From deda825da102a2e45c80a394d6182285ea4fe5b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20=C5=9Aliwa?= Date: Mon, 20 Dec 2021 15:58:35 +0100 Subject: [PATCH 1/2] [#268] Resolves RSA4b1. Updated TokenDetails#expired?. Added :from attribute and started using current_time when checking expiration and authorize for new token. --- lib/ably/auth.rb | 6 +++--- lib/ably/models/token_details.rb | 9 +++++++-- spec/acceptance/rest/auth_spec.rb | 27 ++++++++++++++++++++++++++ spec/unit/models/token_details_spec.rb | 14 +++++++++++++ 4 files changed, 51 insertions(+), 5 deletions(-) diff --git a/lib/ably/auth.rb b/lib/ably/auth.rb index 653270005..2b6ddff82 100644 --- a/lib/ably/auth.rb +++ b/lib/ably/auth.rb @@ -510,11 +510,11 @@ def token_option end def authorize_when_necessary - if current_token_details && !current_token_details.expired? + if current_token_details && !current_token_details.expired?(from: current_time) return current_token_details - else - authorize end + + authorize end # Returns the current device clock time unless the diff --git a/lib/ably/models/token_details.rb b/lib/ably/models/token_details.rb index 8bc0169ff..7745b90e8 100644 --- a/lib/ably/models/token_details.rb +++ b/lib/ably/models/token_details.rb @@ -95,10 +95,15 @@ def client_id # Returns true if token is expired or about to expire # For tokens that have not got an explicit expires attribute expired? will always return true # + # @param attributes [Hash] + # @option attributes [Time] :from Sets a current time from which token expires + # # @return [Boolean] - def expired? + def expired?(attributes = {}) return false if !expires - expires < Time.now + TOKEN_EXPIRY_BUFFER + + from = attributes[:from] || Time.now + expires < from + TOKEN_EXPIRY_BUFFER end # True if the TokenDetails was created from an opaque string i.e. no metadata exists for this token diff --git a/spec/acceptance/rest/auth_spec.rb b/spec/acceptance/rest/auth_spec.rb index 6f079c7a8..f22f3b9ad 100644 --- a/spec/acceptance/rest/auth_spec.rb +++ b/spec/acceptance/rest/auth_spec.rb @@ -1165,6 +1165,33 @@ def coerce_if_time_value(field_name, value, params = {}) end end + context 'when token does not expire' do + let(:client_options) { default_options.merge(use_token_auth: true, key: api_key, query_time: true) } + let(:channel) { client.channels.get(random_str) } + + context 'for the next 2 hours' do + let(:local_time) { Time.now - 2 * 60 * 60 } + + before do + @now = Time.now + allow(Time).to receive(:now).and_return(local_time) + end + + it 'saves a round-trip request (#RSA4b1)' do + expect(auth.current_token_details).to be_nil + channel.publish 'event' + expect(auth.current_token_details).not_to be_nil + + token = auth.current_token_details + + sleep 2.5 + channel.publish 'event' + expect(auth.current_token_details).to eql(token) + end + end + + end + context 'when :client_id is provided in a token' do let(:client_id) { '123' } let(:token) do diff --git a/spec/unit/models/token_details_spec.rb b/spec/unit/models/token_details_spec.rb index 932fdd775..c9bdc40d2 100644 --- a/spec/unit/models/token_details_spec.rb +++ b/spec/unit/models/token_details_spec.rb @@ -80,6 +80,20 @@ expect(subject.expired?).to eql(false) end end + + context 'with :from attribute' do + subject { Ably::Models::TokenDetails.new(expires: expire_time) } + + let(:server_offset_time) { 2 * 60 * 60 } # 2 hours + + it 'is false' do + expect(subject.expired?(from: (Time.now - server_offset_time))).to eql(false) + end + + it 'is true' do + expect(subject.expired?(from: Time.now)).to eql(true) + end + end end end From 63b023fc121086e7507b4c45aa2f3b0980d48de6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20=C5=9Aliwa?= Date: Tue, 21 Dec 2021 19:13:49 +0100 Subject: [PATCH 2/2] Simplified test for RSA4b1 --- spec/acceptance/rest/auth_spec.rb | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/spec/acceptance/rest/auth_spec.rb b/spec/acceptance/rest/auth_spec.rb index f22f3b9ad..ef07af9f2 100644 --- a/spec/acceptance/rest/auth_spec.rb +++ b/spec/acceptance/rest/auth_spec.rb @@ -1172,24 +1172,15 @@ def coerce_if_time_value(field_name, value, params = {}) context 'for the next 2 hours' do let(:local_time) { Time.now - 2 * 60 * 60 } - before do - @now = Time.now - allow(Time).to receive(:now).and_return(local_time) - end - - it 'saves a round-trip request (#RSA4b1)' do - expect(auth.current_token_details).to be_nil - channel.publish 'event' - expect(auth.current_token_details).not_to be_nil + before { allow(Time).to receive(:now).and_return(local_time) } - token = auth.current_token_details - - sleep 2.5 - channel.publish 'event' - expect(auth.current_token_details).to eql(token) + it 'should not request for the new token (#RSA4b1)' do + expect { channel.publish 'event' }.to change { auth.current_token_details } + expect do + expect { channel.publish 'event' }.not_to change { auth.current_token_details } + end.not_to change { auth.current_token_details.expires } end end - end context 'when :client_id is provided in a token' do