Skip to content

Commit

Permalink
#authorize now authenticates every time #RSA10a
Browse files Browse the repository at this point in the history
  • Loading branch information
mattheworiordan committed Oct 2, 2016
1 parent 24eeb49 commit 3322c78
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 52 deletions.
35 changes: 17 additions & 18 deletions lib/ably/auth.rb
Expand Up @@ -65,8 +65,6 @@ def initialize(client, token_params, auth_options)
@token_params = token_params.dup
@token_option = options[:token] || options[:token_details]

@options.delete :force # Forcing token auth for every request is not a valid default

if options[:key] && (options[:key_secret] || options[:key_name])
raise ArgumentError, 'key and key_name or key_secret are mutually exclusive. Provider either a key or key_name & key_secret'
end
Expand Down Expand Up @@ -107,7 +105,6 @@ def initialize(client, token_params, auth_options)
#
# @param [Hash, nil] token_params the token params used for future token requests. When nil, previously configured token params are used
# @param [Hash, nil] auth_options the authentication options used for future token requests. When nil, previously configure authentication options are used
# @option auth_options [Boolean] :force obtains a new token even if the current token is valid. If the provided +auth_options+ Hash contains only this +:force+ attribute, the existing configured authentication options are not overwriten
# @option (see #request_token)
#
# @return (see #create_token_request)
Expand All @@ -124,10 +121,8 @@ def initialize(client, token_params, auth_options)
# end
#
def authorize(token_params = nil, auth_options = nil)
if auth_options == { force: true }
auth_options = options.merge(force: true)
elsif auth_options.nil?
auth_options = options
if auth_options.nil?
auth_options = options # Use default options
else
ensure_valid_auth_attributes auth_options

Expand All @@ -146,29 +141,24 @@ def authorize(token_params = nil, auth_options = nil)

@options = auth_options.clone

# Force reauth and query the server time only happens once
# the otpions remain in auth_options though so they are passed to request_token
# Query the server time only happens once
# the options remain in auth_options though so they are passed to request_token
@options.delete(:query_time)
@options.delete(:force)

@options.freeze
end

# Unless provided, defaults are used
unless token_params.nil?
@token_params = token_params
@token_params.freeze
end

if current_token_details && !auth_options[:force]
return current_token_details unless current_token_details.expired?
end

authorize_with_token(request_token(@token_params, auth_options)).tap do |new_token_details|
logger.debug "Auth: new token following authorisation: #{new_token_details}"

# If authorize was forced allow a block to be called so that the realtime library
# can force upgrade the authorisation
if auth_options[:force] && block_given?
# If authorize the realtime library required auth, then yield the token in a block
if block_given?
yield new_token_details
end
end
Expand Down Expand Up @@ -455,6 +445,14 @@ def token_option
@token_option
end

def authorize_when_necessary
if current_token_details && !current_token_details.expired?
return current_token_details
else
authorize
end
end

# Returns the current device clock time unless the
# the server time has previously been requested with query_time: true
# and the @server_time_offset is configured
Expand Down Expand Up @@ -539,13 +537,14 @@ def store_and_delete_basic_auth_key_from_options!(options)
# Returns the current token if it exists or authorizes and retrieves a token
def token_auth_string
if !current_token_details && token_option
logger.debug "Auth: Token auth string missing, authorizing implicitly now"
# A TokenRequest was configured in the ClientOptions +:token field+ and no current token exists
# Note: If a Token or TokenDetails is provided in the initializer, the token is stored in +current_token_details+
authorize_with_token send_token_request(token_option)
current_token_details.token
else
# Authorize will use the current token if one exists and is not expired, otherwise a new token will be issued
authorize.token
authorize_when_necessary.token
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/ably/realtime/auth.rb
Expand Up @@ -214,7 +214,7 @@ def client
def upgrade_authentication_block(new_token)
# This block is called if the authorisation was forced
if client.connection.connected? || client.connection.connecting?
logger.debug "Realtime::Auth - authorize called with { force: true } so forcibly disconnecting transport to initiate auth upgrade"
logger.debug "Realtime::Auth - authorize was called so forcibly disconnecting transport to initiate auth upgrade"
block = Proc.new do
if client.connection.transport
logger.debug "Realtime::Auth - current transport disconnected"
Expand Down
2 changes: 1 addition & 1 deletion lib/ably/realtime/connection/connection_manager.rb
Expand Up @@ -402,7 +402,7 @@ def renew_token_and_reconnect(error)
@renewing_token = true
logger.info "ConnectionManager: Token has expired and is renewable, renewing token now"

client.auth.authorize(nil, force: true).tap do |authorize_deferrable|
client.auth.authorize.tap do |authorize_deferrable|
authorize_deferrable.callback do |token_details|
logger.info 'ConnectionManager: Token renewed succesfully following expiration'

Expand Down
2 changes: 1 addition & 1 deletion lib/ably/rest/client.rb
Expand Up @@ -468,7 +468,7 @@ def reauthorize_on_authorisation_failure
yield
rescue Ably::Exceptions::TokenExpired => e
if auth.token_renewable?
auth.authorize(nil, force: true)
auth.authorize
yield
else
raise e
Expand Down
28 changes: 14 additions & 14 deletions spec/acceptance/realtime/auth_spec.rb
Expand Up @@ -242,7 +242,7 @@
context 'and an incompatible client_id in a TokenDetails object passed to the auth callback' do
it 'rejects a TokenDetails object with an incompatible client_id and raises an exception' do
client.connection.once(:connected) do
client.auth.authorize({}, force: true)
client.auth.authorize({})
client.connection.on(:error) do |error|
expect(error).to be_a(Ably::Exceptions::IncompatibleClientId)
EventMachine.add_timer(0.1) do
Expand All @@ -255,7 +255,7 @@
end
end

context 'with force: true to trigger an authentication upgrade' do
context 'when already authenticated with a valid token' do
let(:rest_client) { Ably::Rest::Client.new(default_options) }
let(:client_publisher) { auto_close Ably::Realtime::Client.new(default_options) }
let(:basic_capability) { JSON.dump("foo" => ["subscribe"]) }
Expand All @@ -279,7 +279,7 @@
it 'forces the connection to disconnect and reconnect with a new token when in the CONNECTED state' do
client.connection.once(:connected) do
existing_token = client.auth.current_token_details
client.auth.authorize(nil, force: true)
client.auth.authorize(nil)
client.connection.once(:disconnected) do
client.connection.once(:connected) do
expect(existing_token).to_not eql(client.auth.current_token_details)
Expand All @@ -292,7 +292,7 @@
it 'forces the connection to disconnect and reconnect with a new token when in the CONNECTING state' do
client.connection.once(:connecting) do
existing_token = client.auth.current_token_details
client.auth.authorize(nil, force: true)
client.auth.authorize(nil)
client.connection.once(:disconnected) do
client.connection.once(:connected) do
expect(existing_token).to_not eql(client.auth.current_token_details)
Expand All @@ -311,7 +311,7 @@

it 'transisitions the connection state to FAILED if the client_id changes' do
client.connection.once(:connected) do
client.auth.authorize(nil, auth_callback: identified_token_cb, force: true)
client.auth.authorize(nil, auth_callback: identified_token_cb)
client.connection.once(:failed) do
expect(client.connection.error_reason.message).to match(/incompatible.*client ID/)
stop_reactor
Expand All @@ -329,7 +329,7 @@
channel.publish('not-allowed').errback do |error|
expect(error.code).to eql(40160)
expect(error.message).to match(/permission denied/)
client.auth.authorize(nil, auth_callback: upgraded_token_cb, force: true)
client.auth.authorize(nil, auth_callback: upgraded_token_cb)
client.connection.once(:connected) do
expect(client.connection.error_reason).to be_nil
channel.subscribe('allowed') do |message|
Expand All @@ -349,7 +349,7 @@
client.connection.once(:connected) do
channel = client.channels.get('foo')
channel.attach do
client.auth.authorize(nil, auth_callback: downgraded_token_cb, force: true)
client.auth.authorize(nil, auth_callback: downgraded_token_cb)
channel.once(:failed) do
expect(channel.error_reason.code).to eql(40160)
expect(channel.error_reason.message).to match(/Channel denied access/)
Expand All @@ -372,7 +372,7 @@
publisher_channel.publish('foo') do
EventMachine.add_timer(2) do
expect(received_messages.length).to eql(1)
client.auth.authorize(nil, force: true)
client.auth.authorize(nil)
client.connection.once(:disconnected) do
publisher_channel.publish('bar') do
expect(received_messages.length).to eql(1)
Expand All @@ -393,9 +393,9 @@
it 'does not change the connection state if current connection state is closing' do
client.connection.once(:connected) do
client.connection.once(:closing) do
client.auth.authorize(nil, force: true)
client.auth.authorize(nil)
client.connection.once(:connected) do
raise "Should not reconnect following auth force: true"
raise "Should not reconnect following #authorize"
end
EventMachine.add_timer(4) do
expect(client.connection).to be_closed
Expand All @@ -409,9 +409,9 @@
it 'does not change the connection state if current connection state is closed' do
client.connection.once(:connected) do
client.connection.once(:closed) do
client.auth.authorize(nil, force: true)
client.auth.authorize(nil)
client.connection.once(:connected) do
raise "Should not reconnect following auth force: true"
raise "Should not reconnect following #authorize"
end
EventMachine.add_timer(4) do
expect(client.connection).to be_closed
Expand All @@ -428,9 +428,9 @@
it 'does not change the connection state' do
client.connection.once(:connected) do
client.connection.once(:failed) do
client.auth.authorize(nil, force: true)
client.auth.authorize(nil)
client.connection.once(:connected) do
raise "Should not reconnect following auth force: true"
raise "Should not reconnect following #authorize"
end
EventMachine.add_timer(4) do
expect(client.connection).to be_failed
Expand Down
29 changes: 12 additions & 17 deletions spec/acceptance/rest/auth_spec.rb
Expand Up @@ -589,7 +589,7 @@ def coerce_if_time_value(field_name, value, params = {})
end
end

describe '#authorize' do
describe '#authorize (#RSA10, #RSA10j)' do
context 'when called for the first time since the client has been instantiated' do
let(:auth_options) do
{ auth_url: 'http://somewhere.com/' }
Expand All @@ -607,8 +607,8 @@ def coerce_if_time_value(field_name, value, params = {})
expect(auth.authorize).to be_a(Ably::Models::TokenDetails)
end

it 'issues a new token if option :force => true' do
expect { auth.authorize(force: true) }.to change { auth.current_token_details }
it 'issues a new token every time (#RSA10a)' do
expect { auth.authorize }.to change { auth.current_token_details }
end
end

Expand All @@ -625,7 +625,7 @@ def coerce_if_time_value(field_name, value, params = {})
expect(client).to receive(:time).once.and_return(server_time)

auth.authorize({}, query_time: true)
auth.authorize({}, force: true)
auth.authorize({})
expect(auth.auth_options).to_not have_key(:query_time)
end
end
Expand All @@ -639,21 +639,21 @@ def coerce_if_time_value(field_name, value, params = {})

it 'has no effect on the defaults when null and TokenParam defaults remain the same' do
old_token = auth.current_token_details
auth.authorize(nil, force: true)
auth.authorize
expect(old_token).to_not eql(auth.current_token_details)
expect(auth.token_params[:ttl]).to eql(23)
end

it 'updates defaults when present and all previous configured TokenParams are discarded' do
old_token = auth.current_token_details
auth.authorize({ client_id: 'bob' }, { force: true })
auth.authorize({ client_id: 'bob' })
expect(old_token).to_not eql(auth.current_token_details)
expect(auth.token_params[:ttl]).to_not eql(23)
expect(auth.token_params[:client_id]).to eql('bob')
end

it 'updates Auth#token_params attribute with an immutable hash' do
auth.authorize({ client_id: 'bob' }, { force: true })
auth.authorize({ client_id: 'bob' })
expect { auth.token_params['key_name'] = 'new_name' }.to raise_error RuntimeError, /can't modify frozen.*Hash/
end
end
Expand Down Expand Up @@ -699,19 +699,14 @@ def coerce_if_time_value(field_name, value, params = {})
expect(auth.current_token_details).to_not be_expired
end

it 'does not request a token if current_token_details has not expired' do
expect(auth).to_not receive(:request_token)
auth.authorize
end

it 'requests a new token if token is expired' do
allow(auth.current_token_details).to receive(:expired?).and_return(true)
expect(auth).to receive(:request_token)
expect { auth.authorize }.to change { auth.current_token_details }
end

it 'issues a new token if option :force => true' do
expect { auth.authorize({}, force: true) }.to change { auth.current_token_details }
it 'issues a new token every time #authorize is called' do
expect { auth.authorize({}) }.to change { auth.current_token_details }
end
end

Expand Down Expand Up @@ -800,23 +795,23 @@ def coerce_if_time_value(field_name, value, params = {})
let(:auth_token_object) { auth_client.auth.request_token }

it 'rejects a TokenDetails object with an incompatible client_id and raises an exception' do
expect { client.auth.authorize({}, force: true) }.to raise_error Ably::Exceptions::IncompatibleClientId
expect { client.auth.authorize({}) }.to raise_error Ably::Exceptions::IncompatibleClientId
end
end

context 'and an incompatible client_id in a TokenRequest object passed to the auth callback and raises an exception' do
let(:auth_token_object) { auth_client.auth.create_token_request }

it 'rejects a TokenRequests object with an incompatible client_id and raises an exception' do
expect { client.auth.authorize({}, force: true) }.to raise_error Ably::Exceptions::IncompatibleClientId
expect { client.auth.authorize({}) }.to raise_error Ably::Exceptions::IncompatibleClientId
end
end

context 'and a token string without any retrievable client_id' do
let(:auth_token_object) { auth_client.auth.request_token(client_id: 'different').token }

it 'rejects a TokenRequests object with an incompatible client_id and raises an exception' do
client.auth.authorize({}, force: true)
client.auth.authorize({})
expect(client.client_id).to eql(client_id)
end
end
Expand Down

0 comments on commit 3322c78

Please sign in to comment.