Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Bearer Token #132

Merged
merged 12 commits into from
Dec 26, 2019
Merged

Conversation

smaeda-ks
Copy link
Collaborator

@smaeda-ks smaeda-ks commented Dec 15, 2019

Problem

Twurl doesn't support application-only token (OAuth2).
There were some efforts before such as #48 (#47) but it wasn't merged, unfortunately.

Solution

Using @mdeschamps 's base implementation with a bit of a customized approach.

So the key point is, how to store a bearer token in ~/.twurlrc file. In #48, I see the original implementation was to store token by associating it to a hard-coded username app-only:

profiles:
  manuel:
    XXX:
      consumer_key: XXX
      .....
  app-only:
    XXX:
      consumer_key: XXX
      .....

but this is not an ideal approach as described in #48 (comment).

So my approach is, store bearer tokens under a new top-level object bearer_tokens like this:

profiles:
  user1:
    AAA:
      consumer_key: AAA
      .....
    BBB:
      consumer_key: BBB
      .....
  user2:
    AAA:
      consumer_key: AAA
      .....
    CCC:
      consumer_key: CCC
      .....
configuration:
  default_profile:
  - user1
  - AAA
aliases:
  h: "/1.1/statuses/home_timeline.json"
bearer_tokens:
  AAA: ${bearer_token_1}
  BBB: ${bearer_token_2}

Since a bearer token doesn't have an association with user but App (consumer_key + consumer_secret), it should not link to a user object. In the above case, each bearer token only has a link to its App (consumer_key). That is, both user1 and user2 can make an OAuth2 request using a stored bearer token if the current profile's consumer_key has a link to one of those stored tokens.

Example:

profiles:
  user1:
    AAA:
      consumer_key: AAA
      .....
    BBB:
      consumer_key: BBB
      .....
  user2:
    AAA:
      consumer_key: AAA
      .....
    CCC:
      consumer_key: CCC
      .....
configuration:
  default_profile:
  - user1
  - AAA
aliases:
  h: "/1.1/statuses/home_timeline.json"
bearer_tokens:
  AAA: ${bearer_token_1}
  BBB: ${bearer_token_2}
  1. SUCCESS profile: user1, consumer_key: AAA
    This should success as the consumer_key AAA has a stored bearer token (${bearer_token_1})
$ twurl set default user1 AAA
$ twurl --oauth2 '/1.1/search/tweets.json?q=hello'
  # => success
  1. SUCCESS profile: user1, consumer_key: BBB
    This should success as the consumer_key BBB has a stored bearer token (${bearer_token_2})
$ twurl set default user1 BBB
$ twurl --oauth2 '/1.1/search/tweets.json?q=hello'
  # => success
  1. SUCCESS profile: user2, consumer_key: AAA
    This should success as the consumer_key AAA has a stored bearer token (${bearer_token_1})
$ twurl set default user2 BBB
$ twurl --oauth2 '/1.1/search/tweets.json?q=hello'
  # => success
  1. ERROR profile: user2, consumer_key: CCC
    This should fail as the consumer_key CCC has no stored bearer token.
$ twurl set default user2 CCC
$ twurl --oauth2 '/1.1/search/tweets.json?q=hello'
No available bearer token found for consumer_key:CCC  # => error

Auth flow

$ twurl authorize --oauth2 --consumer-key ${consumer_key} --consumer-secret ${consumer_secret}
Authorization successful

List OAuth2 tokens

$ twurl oauth2_tokens
[consumer_key: bearer_token]
AAA: (omitted)
BBB: (omitted)
  • the (omitted) part is actual output, the oauth2_tokens doesn't print bearer_tokens for security reasons.

Limitations

The --oauth2 option requires at least one user profile created (in ~/.twurlrc). So if a first-time install user who only issued an OAuth2 token performs an OAuth2 request then it fails:

$ gem install twurl
$ twurl authorize --oauth2 --consumer-key ${consumer_key} --consumer-secret ${consumer_secret}
Authorization successful
$ cat ~/.twurlrc
---
profiles: {}
configuration: {}
bearer_tokens:
  AAA: token
$ twurl --oauth2 '/1.1/search/tweets.json?q=hello'
To use --oauth2 option, you need to authorize (OAuth1.0a) and create at least one user profile (~/.twurlrc):

twurl authorize -c key -s secret

or, you can specify issued token's consumer_key directly:
(to see your issued tokens: 'twurl oauth2_tokens')

twurl --oauth2 -c key '/path/to/api'

In this case, the user can still make an OAuth2 request by specifying the -c (--consumer_key) option without creating a user profile as described in the above error message.

$ twurl --oauth2 -c AAA '/1.1/search/tweets.json?q=hello'
  # => success

@smaeda-ks smaeda-ks self-assigned this Dec 15, 2019
@coveralls
Copy link

coveralls commented Dec 15, 2019

Coverage Status

Coverage decreased (-0.06%) to 88.571% when pulling 5d91c1b on smaeda-ks:smaeda-ks/app-only-auth into 3a1a055 on twitter:master.

@smaeda-ks smaeda-ks added this to the Target v0.9.5 milestone Dec 15, 2019
@smaeda-ks smaeda-ks changed the title Support Bearer Token [WIP] Support Bearer Token Dec 15, 2019
@smaeda-ks smaeda-ks requested a review from a team December 16, 2019 00:13
so users can make an OAuth2 request without creating a user profile if they want... (e.g., a user who only use bearer token)
Copy link
Contributor

@andypiper andypiper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested several scenarios here (standard, premium and Labs APIs) and this works. It also doesn't seem to have corrupted my .twurlrc file which is positive :-) So, this is a tentative approval, but I do have a few things we should discuss before merging.

A few thoughts:

  • I'm concerned about the complexity and clarity of the .twurlrc file, given it is now possible to have multiple accounts and apps, and also (separately, but still requiring an account to exist), bearer tokens. I wonder if there is any way to make it more clear, which tokens potentially work with which default account.
  • we're using "OAuth 2", "bearer token" and "application-only" to describe this functionality in different places (twurlrc, help text, etc), and I think there's a danger of this making it more confusing. We've got some auth doc updates being worked on as well (Hamza might be able to show you these) and maybe we could find a way of only using one term (I think "app-only" or "bearer token" would be most clear).
  • needs a big update to the README
  • I'm not sure of the value of hiding the bearer token in the output from the oauth2_tokens command, because they are stored in plain text in the twurlrc file

@smaeda-ks
Copy link
Collaborator Author

@andypiper Thanks!

  • I'm concerned about the complexity and clarity of the .twurlrc file, given it is now possible to have multiple accounts and apps, and also (separately, but still requiring an account to exist), bearer tokens. I wonder if there is any way to make it more clear, which tokens potentially work with which default account.

Agree. But, with the current interface and implementation by having a backward-compatibility, it is certainly difficult to provide such a way, is my view. For now, all we can do is to provide solid documentation and eliminate confusion as much as possible, I think.

  • we're using "OAuth 2", "bearer token" and "application-only" to describe this functionality in different places (twurlrc, help text, etc), and I think there's a danger of this making it more confusing. We've got some auth doc updates being worked on as well (Hamza might be able to show you these) and maybe we could find a way of only using one term (I think "app-only" or "bearer token" would be most clear).

100%, this was one of the critical ask I wanted to collect feedback from folks. When I start this implementation, since the "application-only" is our own term and not widely common, I thought "--oauth2" is more user-friendly and intuitive. But now, I'm also feeling --app-only could be more appropriate from both users and maintainers' perspective after wrote some tests code and was forced to name function names getting so confusing.

  • needs a big update to the README

Will do.

  • I'm not sure of the value of hiding the bearer token in the output from the oauth2_tokens command, because they are stored in plain text in the twurlrc file.

Right. I just followed the same way as the twurl accounts command does. But I don't think it's so critical for now, and we can perhaps emphasize the purpose/existence of the ~/twurlrc file. From what I've been seeing from users' inquiries, most of them don't know much about the ~/.twurlrc file (It is useful to know, I believe).

@smaeda-ks
Copy link
Collaborator Author

Well, I actually prefer --bearer than --app-only.
This looks much better...

$ twurl -h
Usage: twurl authorize --consumer-key key --consumer-secret secret
       twurl [options] /1.1/statuses/home_timeline.json

Supported Commands: accounts, alias, authorize, bearer_tokens, set

Getting started:
    -T, --tutorial                   Narrative overview of how to get started using Twurl

Authorization options:
    -u, --username [username]        Username of account to authorize (required)
    -p, --password [password]        Password of account to authorize (required)
    -c, --consumer-key [key]         Your consumer key (required)
    -s, --consumer-secret [secret]   Your consumer secret (required)
    -a, --access-token [token]       Your access token
    -S, --token-secret [secret]      Your token secret

Common options:
    -t, --[no-]trace                 Trace request/response traffic (default: --no-trace)
    -d, --data [data]                Sends the specified data in a POST request to the HTTP server.
    -r, --raw-data [data]            Sends the specified data as it is in a POST request to the HTTP server.
    -A, --header [header]            Adds the specified header to the request to the HTTP server.
    -H, --host [host]                Specify host to make requests to (default: api.twitter.com)
    -q, --quiet                      Suppress all output (default: output is printed to STDOUT)
    -U, --no-ssl                     Disable SSL (default: SSL is enabled)
    -X, --request-method [method]    Request method (default: GET)
    -P, --proxy [proxy]              Specify HTTP proxy to forward requests to (default: No proxy)
    -f, --file [path_to_file]        Specify the path to the file to upload
    -F, --file-field [field_name]    Specify the POST parameter name for the file upload data (default: media)
    -b, --base64                     Encode the uploaded file as base64 (default: false)
    -j, --json-pretty                Format response body to JSON pretty style
        --timeout [sec]              Number of seconds to wait for the request to be read (default: 60)
        --connection-timeout [sec]   Number of seconds to wait for the connection to open (default: 60)
        --bearer                     Use application-only authentication (Bearer Token)
    -h, --help                       Show this message
    -v, --version                    Show version
$ twurl bearer_tokens
No issued application-only (Bearer) tokens
$ twurl --bearer '/1.1/search/tweets.json?q=hello'
No available bearer token found for consumer_key:AAA
$ twurl authorize --bearer -c AAA -s secret
Authorization successful
$ twurl bearer_tokens
[consumer_key: bearer_token]
AAA: (omitted)

@andypiper
Copy link
Contributor

🚀

@andypiper
Copy link
Contributor

I think once we've got some documentation about this in place, we can probably get this merged.

@smaeda-ks smaeda-ks changed the title [WIP] Support Bearer Token Support Bearer Token Dec 21, 2019
@smaeda-ks smaeda-ks force-pushed the smaeda-ks/app-only-auth branch from f545d6b to 4814044 Compare December 21, 2019 14:41
@smaeda-ks smaeda-ks force-pushed the smaeda-ks/app-only-auth branch from 4814044 to 7cfa1fc Compare December 21, 2019 14:45
@smaeda-ks
Copy link
Collaborator Author

@andypiper Updated README, I'll wait for your review, thanks!

@smaeda-ks smaeda-ks force-pushed the smaeda-ks/app-only-auth branch from 7cfa1fc to a8edbce Compare December 21, 2019 14:52
@smaeda-ks smaeda-ks requested a review from andypiper December 21, 2019 14:53
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@andypiper andypiper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One typo in the README - after that I approve merging and shipping.

@smaeda-ks smaeda-ks requested a review from andypiper December 24, 2019 22:29
@smaeda-ks
Copy link
Collaborator Author

@andypiper Done! Thanks for your review

@smaeda-ks smaeda-ks merged commit e9cb1a0 into twitter:master Dec 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants