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

Adding Bearer Token support #387

Merged
merged 9 commits into from
Apr 18, 2013
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion lib/twitter/api/oauth.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ module Twitter
module API
module OAuth
include Twitter::API::Utils

def token
object_from_response(Twitter::Token, :bearer_request, "/oauth2/token", :grant_type => "client_credentials")
end
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe alias this method to bearer_token?

# Allows a registered application to revoke an issued OAuth 2 Bearer Token by presenting its client credentials.
#
# @see https://dev.twitter.com/docs/api/1.1/post/oauth2/invalidate_token
Expand All @@ -19,6 +21,19 @@ module OAuth
def invalidate_token(access_token)
object_from_response(Twitter::Token, :post, "/oauth2/invalidate_token", :access_token => access_token)
end

private
def bearer_request(path, params={})
connection.send(:post, path, params) do |request|
request.headers[:accept] = "*/*"
request.headers[:authorization] = "Basic #{encoded_bearer_token_credentials}"
request.headers[:content_type] = "application/x-www-form-urlencoded; charset=UTF-8"
end.env
rescue Faraday::Error::ClientError
raise Twitter::Error::ClientError
rescue MultiJson::DecodeError
raise Twitter::Error::DecodeError
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand why you put this here, but feels like this is also misplaced. Does it make sense to put bearer_request and encoded_bearer_token_credentials on Twitter::Client alongside the rest of the request-based methods.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If I understand you correctly, you propose I move bearer_request and encoded_bearer_token_credentials to client.rb as private methods. I could certainly do that.

However, I am not happy supporting two separate pathways that both call connection.send in ways that are fundamentally very similar. My initial attempt was to include an extra parameter (e.g :bearer_token_request => true in the call, ultimately, to request method. Then inside the request method to check for bearer request (by calling params.delete(:bearer_token_request) and in return to change the block given to connection.send. That would certainly localize connection calls but will also incur an extra hash lookup and if statement for every connection request, and even worse where most of the calls will not even be using the alternative pathway. What do you think?

end
end
end
8 changes: 6 additions & 2 deletions lib/twitter/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,12 @@ def connection
end

def auth_header(method, path, params={})
uri = URI(@endpoint + path)
SimpleOAuth::Header.new(method, uri, params, credentials)
if application_only_auth?
"Bearer #{@bearer_token}"
else
uri = URI(@endpoint + path)
SimpleOAuth::Header.new(method, uri, params, credentials)
end
end

end
Expand Down
13 changes: 11 additions & 2 deletions lib/twitter/configurable.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
require 'forwardable'
require 'twitter/error/configuration_error'
require 'base64'
Copy link
Owner

Choose a reason for hiding this comment

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

This require statement belongs in client.rb, where the Base64 module is used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed.


module Twitter
module Configurable
extend Forwardable
attr_writer :consumer_key, :consumer_secret, :oauth_token, :oauth_token_secret
attr_writer :consumer_key, :consumer_secret, :oauth_token, :oauth_token_secret, :bearer_token
attr_accessor :endpoint, :connection_options, :identity_map, :middleware
def_delegator :options, :hash

Expand All @@ -16,6 +17,7 @@ def keys
:consumer_secret,
:oauth_token,
:oauth_token_secret,
:bearer_token,
:endpoint,
:connection_options,
:identity_map,
Expand All @@ -37,7 +39,7 @@ def configure

# @return [Boolean]
def credentials?
credentials.values.all?
credentials.values.all? || @bearer_token
end

def reset!
Expand All @@ -49,6 +51,13 @@ def reset!
alias setup reset!

private
def application_only_auth?
not @bearer_token.nil?
Copy link
Collaborator

Choose a reason for hiding this comment

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

stylistic preference for me, but i loathe double negatives like this, perhaps change to:

!!@bearer_token

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, that sounds perfectly fine, will change.

end

def encoded_bearer_token_credentials
Copy link
Collaborator

Choose a reason for hiding this comment

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

Twitter::Configurable doesn't seem like the right place for this.

Base64.strict_encode64("#{@consumer_key}:#{@consumer_secret}")
end

# @return [Hash]
def credentials
Expand Down
5 changes: 5 additions & 0 deletions lib/twitter/default.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ def oauth_token_secret
ENV['TWITTER_OAUTH_TOKEN_SECRET']
end

# @return [String]
def bearer_token
ENV['TWITTER_BEARER_TOKEN']
end

# @note This is configurable in case you want to use a Twitter-compatible endpoint.
# @see http://status.net/wiki/Twitter-compatible_API
# @see http://en.blog.wordpress.com/2009/12/12/twitter-api/
Expand Down
13 changes: 13 additions & 0 deletions spec/twitter/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
:middleware => Proc.new{},
:oauth_token => 'OT',
:oauth_token_secret => 'OS',
:bearer_token => 'BT',
:identity_map => ::Hash
}
end
Expand Down Expand Up @@ -152,4 +153,16 @@
end
end

describe "#auth_header" do
subject do
Twitter::Client.new(:bearer_token => "BT")
end

it "creates the correct auth headers with supplied bearer_token" do
uri = "/1.1/direct_messages.json"
authorization = subject.send(:auth_header, :get, uri)
expect(authorization).to eq "Bearer BT"
end
end

end
8 changes: 7 additions & 1 deletion spec/twitter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,13 @@
end

describe ".credentials?" do
it "returns true if all credentials are present" do
it "returns true if only bearer_token is supplied" do
Twitter.configure do |config|
config.bearer_token = 'BT'
end
expect(Twitter.credentials?).to be_true
end
it "returns true if all oauth credentials are present" do
Twitter.configure do |config|
config.consumer_key = 'CK'
config.consumer_secret = 'CS'
Expand Down