From 67c8168fdeb3dfb7e107703708af7a4073d58d0d Mon Sep 17 00:00:00 2001 From: Mathias Klippinge Date: Thu, 7 Jun 2018 15:52:36 +0200 Subject: [PATCH 01/11] Take time skew into consideration --- lib/oauth2/access_token.rb | 20 +++++++++++++++----- spec/oauth2/access_token_spec.rb | 24 ++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/lib/oauth2/access_token.rb b/lib/oauth2/access_token.rb index 2c152b89..14ca15bf 100644 --- a/lib/oauth2/access_token.rb +++ b/lib/oauth2/access_token.rb @@ -1,6 +1,6 @@ module OAuth2 class AccessToken - attr_reader :client, :token, :expires_in, :expires_at, :params + attr_reader :client, :token, :expires_in, :expires_at, :params, :time_skew attr_accessor :options, :refresh_token, :response class << self @@ -37,17 +37,27 @@ def from_kvform(client, kvform) # @option opts [String] :header_format ('Bearer %s') the string format to use for the Authorization header # @option opts [String] :param_name ('access_token') the parameter name to use for transmission of the # Access Token value in :body or :query transmission mode - def initialize(client, token, opts = {}) # rubocop:disable Metrics/AbcSize + def initialize(client, token, opts = {}) # rubocop:disable Metrics/AbcSize, Metrics/MethodLength + local_now = Time.now.to_i + opts = opts.dup @client = client @token = token.to_s - opts = opts.dup + @time_skew = 0 + [:refresh_token, :expires_in, :expires_at].each do |arg| instance_variable_set("@#{arg}", opts.delete(arg) || opts.delete(arg.to_s)) end + @expires_in ||= opts.delete('expires') @expires_in &&= @expires_in.to_i @expires_at &&= @expires_at.to_i - @expires_at ||= Time.now.to_i + @expires_in if @expires_in + + if @expires_in + @expires_at ||= local_now + @expires_in + calculated_issued_at = @expires_at - @expires_in + @time_skew = local_now - calculated_issued_at + end + @options = {:mode => opts.delete(:mode) || :header, :header_format => opts.delete(:header_format) || 'Bearer %s', :param_name => opts.delete(:param_name) || 'access_token'} @@ -72,7 +82,7 @@ def expires? # # @return [Boolean] def expired? - expires? && (expires_at <= Time.now.to_i) + expires? && (expires_at + time_skew <= Time.now.to_i) end # Refreshes the current Access Token diff --git a/spec/oauth2/access_token_spec.rb b/spec/oauth2/access_token_spec.rb index 699b31d9..57fad9cb 100644 --- a/spec/oauth2/access_token_spec.rb +++ b/spec/oauth2/access_token_spec.rb @@ -151,6 +151,30 @@ def assert_initialized_token(target) # rubocop:disable Metrics/AbcSize allow(Time).to receive(:now).and_return(@now) expect(access).to be_expired end + + describe 'time skew' do + let(:time_skew) { 10 } + let(:expires_in) { 300 } + let(:expires_at) { Time.now.to_i - 10 + expires_in } + let!(:access) { described_class.new(client, token, :refresh_token => 'abaca', :expires_at => expires_at, :expires_in => expires_in) } + + context 'when not within time skew correction' do + let(:now) { Time.at(expires_at) + time_skew + 1 } + + it 'access is expired' do + allow(Time).to receive(:now).and_return(now) + expect(access).to be_expired + end + end + + context 'when within time skew correction' do + let(:now) { Time.at(expires_at) + time_skew - 1 } + + it 'access is not expired' do + expect(access).to_not be_expired + end + end + end end describe '#refresh' do From 8ab6a3779bfb8831a59ab3fc365c7066025c08ec Mon Sep 17 00:00:00 2001 From: Mathias Klippinge Date: Fri, 8 Jun 2018 11:06:51 +0200 Subject: [PATCH 02/11] Read issued_at from token --- lib/oauth2/access_token.rb | 60 +++++++++++++++++++++++++++++++------- lib/oauth2/response.rb | 2 +- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/lib/oauth2/access_token.rb b/lib/oauth2/access_token.rb index 14ca15bf..2ea76bb3 100644 --- a/lib/oauth2/access_token.rb +++ b/lib/oauth2/access_token.rb @@ -1,6 +1,6 @@ module OAuth2 class AccessToken - attr_reader :client, :token, :expires_in, :expires_at, :params, :time_skew + attr_reader :client, :token, :expires_in, :expires_at, :params, :token_payload, :time_skew attr_accessor :options, :refresh_token, :response class << self @@ -37,12 +37,11 @@ def from_kvform(client, kvform) # @option opts [String] :header_format ('Bearer %s') the string format to use for the Authorization header # @option opts [String] :param_name ('access_token') the parameter name to use for transmission of the # Access Token value in :body or :query transmission mode - def initialize(client, token, opts = {}) # rubocop:disable Metrics/AbcSize, Metrics/MethodLength + def initialize(client, token, opts = {}) # rubocop:disable Metrics/AbcSize local_now = Time.now.to_i - opts = opts.dup @client = client @token = token.to_s - @time_skew = 0 + opts = opts.dup [:refresh_token, :expires_in, :expires_at].each do |arg| instance_variable_set("@#{arg}", opts.delete(arg) || opts.delete(arg.to_s)) @@ -51,12 +50,9 @@ def initialize(client, token, opts = {}) # rubocop:disable Metrics/AbcSize, Metr @expires_in ||= opts.delete('expires') @expires_in &&= @expires_in.to_i @expires_at &&= @expires_at.to_i - - if @expires_in - @expires_at ||= local_now + @expires_in - calculated_issued_at = @expires_at - @expires_in - @time_skew = local_now - calculated_issued_at - end + @expires_at ||= local_now + @expires_in if @expires_in + issued_at = token_payload.fetch('iat', nil) + @time_skew = issued_at ? local_now - issued_at : 0 @options = {:mode => opts.delete(:mode) || :header, :header_format => opts.delete(:header_format) || 'Bearer %s', @@ -160,6 +156,50 @@ def headers {'Authorization' => options[:header_format] % token} end + # For JWT tokens, this will store a Hash on the form + # { + # "exp": exp, + # "nbf": 0, + # "iat": now.to_i, + # "iss": "https://example.com/auth/realms/issuer", + # "aud": "client-identifier", + # "sub": "subject-identifier", + # "typ": "Bearer", + # "azp": "client-identifier" + # } + # + # For non-JWT tokens, an empty Hash is stored + # + # @return [Hash] a hash of token property values + def token_payload + @token_payload ||= decoded_token(:payload) + end + + # A JWT token consists of three dot separated parts: + # 1) header + # 2) payload + # 3) verify_signature + # + # Some access tokens are not JWT tokens, + # in which case nil is returned + def decoded_token(part) + token_chunks = token.split('.') + selected_chunk = case part + when :header + token_chunks.fetch(0) + when :payload + token_chunks.fetch(1) + when :verify_signature + token_chunks.fetch(2) + else + raise(ArgumentError, "#{part} is not a valid token chunk, valid arguments: [:header, :payload, :verify_signature]") + end + + JSON Base64.decode64(selected_chunk) + rescue + {} + end + private def configure_authentication!(opts) # rubocop:disable MethodLength, Metrics/AbcSize diff --git a/lib/oauth2/response.rb b/lib/oauth2/response.rb index e97442da..d8b27d51 100644 --- a/lib/oauth2/response.rb +++ b/lib/oauth2/response.rb @@ -86,7 +86,7 @@ def content_type end # Determines the parser (a Proc or other Object which responds to #call) - # that will be passed the {#body} (and optionall {#response}) to supply + # that will be passed the {#body} (and optional {#response}) to supply # {#parsed}. # # The parser can be supplied as the +:parse+ option in the form of a Proc From c6c3f7fd59cdc294395c97bfec2c06e09a9c482e Mon Sep 17 00:00:00 2001 From: Mathias Klippinge Date: Fri, 8 Jun 2018 13:56:27 +0200 Subject: [PATCH 03/11] Add a spec for JWT tokens --- spec/oauth2/access_token_spec.rb | 70 ++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/spec/oauth2/access_token_spec.rb b/spec/oauth2/access_token_spec.rb index 57fad9cb..2d40ddc6 100644 --- a/spec/oauth2/access_token_spec.rb +++ b/spec/oauth2/access_token_spec.rb @@ -208,4 +208,74 @@ def assert_initialized_token(target) # rubocop:disable Metrics/AbcSize expect(access_token.to_hash).to eq(hash) end end + + context 'when token is a JWT token' do + let(:rsa_private) { OpenSSL::PKey::RSA.generate 2048 } + let(:rsa_public) { rsa_private.public_key } + let(:token_payload) do + Hash( + { + "exp": exp, + "nbf": 0, + "iat": now.to_i, + "iss": "https://example.com/auth/realms/issuer", + "aud": "client-identifier", + "sub": "subject-identifier", + "typ": "Bearer", + "azp": "client-identifier" + } + ) + end + let(:token) { @token ||= JWT.encode(token_payload, rsa_private, 'RS256') } + + let(:refresh_token_payload) do + Hash( + { + "exp": exp_refresh, + "nbf": 0, + "iat": refreshed_now.to_i, + "iss": "https://example.com/auth/realms/issuer", + "aud": "client-identifier", + "sub": "subject-identifier", + "typ": "Bearer", + "azp": "client-identifier" + } + ) + end + let(:refresh_token) { @refresh_token ||= JWT.encode(refresh_token_payload, rsa_private, 'RS256') } + + let(:now) { Time.now } + let(:exp) { now.to_i + 300 } + let(:exp_refresh) { refreshed_now.to_i + 300 } + let(:refreshed_now) { Time.now + 300 } + + let(:refresh_body) { MultiJson.encode(:access_token => token, :expires_in => 600, :refresh_token => refresh_token) } + + describe 'time skew' do + let(:time_skew) { 10 } + let!(:access) do + described_class.new(client, token, :expires_at => exp).tap do |access| + access.instance_variable_set(:@time_skew, time_skew) + end + end + + context 'when not within time skew correction' do + let(:local_now) { Time.at(exp) + time_skew + 1 } + + it 'access is expired' do + allow(Time).to receive(:now).and_return(local_now) + expect(access).to be_expired + end + end + + context 'when within time skew correction' do + let(:local_now) { Time.at(exp) + time_skew - 1 } + + it 'access is not expired' do + allow(Time).to receive(:now).and_return(local_now) + expect(access).to_not be_expired + end + end + end + end end From 5ab6eca125e347484ef51ce556262c4569de3cf5 Mon Sep 17 00:00:00 2001 From: Mathias Klippinge Date: Fri, 8 Jun 2018 13:58:49 +0200 Subject: [PATCH 04/11] Remove :token_payload from attr_reader --- lib/oauth2/access_token.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/oauth2/access_token.rb b/lib/oauth2/access_token.rb index 2ea76bb3..cf20629d 100644 --- a/lib/oauth2/access_token.rb +++ b/lib/oauth2/access_token.rb @@ -1,6 +1,6 @@ module OAuth2 class AccessToken - attr_reader :client, :token, :expires_in, :expires_at, :params, :token_payload, :time_skew + attr_reader :client, :token, :expires_in, :expires_at, :params, :time_skew attr_accessor :options, :refresh_token, :response class << self From 254519fd88441705089242cc68d16b271dbf1f67 Mon Sep 17 00:00:00 2001 From: Mathias Klippinge Date: Fri, 8 Jun 2018 14:16:29 +0200 Subject: [PATCH 05/11] Reduce complexity --- lib/oauth2/access_token.rb | 24 +++++-------------- spec/oauth2/access_token_spec.rb | 40 ++++++++++++++------------------ 2 files changed, 24 insertions(+), 40 deletions(-) diff --git a/lib/oauth2/access_token.rb b/lib/oauth2/access_token.rb index cf20629d..9bb0fad4 100644 --- a/lib/oauth2/access_token.rb +++ b/lib/oauth2/access_token.rb @@ -172,7 +172,7 @@ def headers # # @return [Hash] a hash of token property values def token_payload - @token_payload ||= decoded_token(:payload) + @token_payload ||= decoded_payload end # A JWT token consists of three dot separated parts: @@ -180,23 +180,11 @@ def token_payload # 2) payload # 3) verify_signature # - # Some access tokens are not JWT tokens, - # in which case nil is returned - def decoded_token(part) - token_chunks = token.split('.') - selected_chunk = case part - when :header - token_chunks.fetch(0) - when :payload - token_chunks.fetch(1) - when :verify_signature - token_chunks.fetch(2) - else - raise(ArgumentError, "#{part} is not a valid token chunk, valid arguments: [:header, :payload, :verify_signature]") - end - - JSON Base64.decode64(selected_chunk) - rescue + # @return [Hash] a hash of token property values + def decoded_payload + jwt_payload = token.split('.').fetch(1) + JSON Base64.decode64(jwt_payload) + rescue StandardError {} end diff --git a/spec/oauth2/access_token_spec.rb b/spec/oauth2/access_token_spec.rb index 2d40ddc6..9a6f8778 100644 --- a/spec/oauth2/access_token_spec.rb +++ b/spec/oauth2/access_token_spec.rb @@ -171,7 +171,7 @@ def assert_initialized_token(target) # rubocop:disable Metrics/AbcSize let(:now) { Time.at(expires_at) + time_skew - 1 } it 'access is not expired' do - expect(access).to_not be_expired + expect(access).not_to be_expired end end end @@ -214,32 +214,28 @@ def assert_initialized_token(target) # rubocop:disable Metrics/AbcSize let(:rsa_public) { rsa_private.public_key } let(:token_payload) do Hash( - { - "exp": exp, - "nbf": 0, - "iat": now.to_i, - "iss": "https://example.com/auth/realms/issuer", - "aud": "client-identifier", - "sub": "subject-identifier", - "typ": "Bearer", - "azp": "client-identifier" - } + 'exp' => exp, + 'nbf' => 0, + 'iat' => now.to_i, + 'iss' => 'https://example.com/auth/realms/issuer', + 'aud' => 'client-identifier', + 'sub' => 'subject-identifier', + 'typ' => 'Bearer', + 'azp' => 'client-identifier' ) end let(:token) { @token ||= JWT.encode(token_payload, rsa_private, 'RS256') } let(:refresh_token_payload) do Hash( - { - "exp": exp_refresh, - "nbf": 0, - "iat": refreshed_now.to_i, - "iss": "https://example.com/auth/realms/issuer", - "aud": "client-identifier", - "sub": "subject-identifier", - "typ": "Bearer", - "azp": "client-identifier" - } + 'exp' => exp_refresh, + 'nbf' => 0, + 'iat' => refreshed_now.to_i, + 'iss' => 'https://example.com/auth/realms/issuer', + 'aud' => 'client-identifier', + 'sub' => 'subject-identifier', + 'typ' => 'Bearer', + 'azp' => 'client-identifier' ) end let(:refresh_token) { @refresh_token ||= JWT.encode(refresh_token_payload, rsa_private, 'RS256') } @@ -273,7 +269,7 @@ def assert_initialized_token(target) # rubocop:disable Metrics/AbcSize it 'access is not expired' do allow(Time).to receive(:now).and_return(local_now) - expect(access).to_not be_expired + expect(access).not_to be_expired end end end From b9bb7479418d703d47ed7fcf7265bb95c57d8cdd Mon Sep 17 00:00:00 2001 From: Mathias Klippinge Date: Fri, 8 Jun 2018 14:23:12 +0200 Subject: [PATCH 06/11] Rubocop changes In order to get around a few linting errors which were too hard to deal with, disable or increase the metrics temporarily --- .rubocop_todo.yml | 6 ++++++ lib/oauth2/access_token.rb | 5 +++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index e7701734..aaaef51d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -13,3 +13,9 @@ Style/EachWithObject: # Once we drop Rubies that lack support for __dir__ we can turn this on. Style/ExpandPathArguments: Enabled: false + +Metrics/CyclomaticComplexity: + Max: 7 + +Metrics/ClassLength: + Max: 103 diff --git a/lib/oauth2/access_token.rb b/lib/oauth2/access_token.rb index 9bb0fad4..398e50a6 100644 --- a/lib/oauth2/access_token.rb +++ b/lib/oauth2/access_token.rb @@ -37,7 +37,7 @@ def from_kvform(client, kvform) # @option opts [String] :header_format ('Bearer %s') the string format to use for the Authorization header # @option opts [String] :param_name ('access_token') the parameter name to use for transmission of the # Access Token value in :body or :query transmission mode - def initialize(client, token, opts = {}) # rubocop:disable Metrics/AbcSize + def initialize(client, token, opts = {}) # rubocop:disable Metrics/AbcSize, Metrics/MethodLength local_now = Time.now.to_i @client = client @token = token.to_s @@ -168,7 +168,6 @@ def headers # "azp": "client-identifier" # } # - # For non-JWT tokens, an empty Hash is stored # # @return [Hash] a hash of token property values def token_payload @@ -180,6 +179,8 @@ def token_payload # 2) payload # 3) verify_signature # + # For non-JWT tokens, an empty Hash is returned + # # @return [Hash] a hash of token property values def decoded_payload jwt_payload = token.split('.').fetch(1) From 2f9842433bb1f37194aaccff1d9368e6772639ac Mon Sep 17 00:00:00 2001 From: Mathias Klippinge Date: Fri, 8 Jun 2018 15:04:29 +0200 Subject: [PATCH 07/11] Read issued_at from token as well, or default to local time --- lib/oauth2/access_token.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/oauth2/access_token.rb b/lib/oauth2/access_token.rb index 398e50a6..44315795 100644 --- a/lib/oauth2/access_token.rb +++ b/lib/oauth2/access_token.rb @@ -1,6 +1,6 @@ module OAuth2 class AccessToken - attr_reader :client, :token, :expires_in, :expires_at, :params, :time_skew + attr_reader :client, :token, :expires_in, :issued_at, :expires_at, :params, :time_skew attr_accessor :options, :refresh_token, :response class << self @@ -49,10 +49,13 @@ def initialize(client, token, opts = {}) # rubocop:disable Metrics/AbcSize, Metr @expires_in ||= opts.delete('expires') @expires_in &&= @expires_in.to_i + + @issued_at = token_payload.fetch('iat', local_now) + @expires_at ||= token_payload.fetch('exp', nil) + @expires_at &&= @expires_at.to_i - @expires_at ||= local_now + @expires_in if @expires_in - issued_at = token_payload.fetch('iat', nil) - @time_skew = issued_at ? local_now - issued_at : 0 + @expires_at ||= @issued_at + @expires_in if @expires_in + @time_skew = local_now - @issued_at @options = {:mode => opts.delete(:mode) || :header, :header_format => opts.delete(:header_format) || 'Bearer %s', From 6a07b6a16c80a83466ec52c3ee0106782b3d53ee Mon Sep 17 00:00:00 2001 From: Mathias Klippinge Date: Fri, 8 Jun 2018 15:09:20 +0200 Subject: [PATCH 08/11] Use hash style compatible with ruby 1.9 --- spec/oauth2/access_token_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/oauth2/access_token_spec.rb b/spec/oauth2/access_token_spec.rb index 9a6f8778..d5adf9d0 100644 --- a/spec/oauth2/access_token_spec.rb +++ b/spec/oauth2/access_token_spec.rb @@ -213,7 +213,7 @@ def assert_initialized_token(target) # rubocop:disable Metrics/AbcSize let(:rsa_private) { OpenSSL::PKey::RSA.generate 2048 } let(:rsa_public) { rsa_private.public_key } let(:token_payload) do - Hash( + { 'exp' => exp, 'nbf' => 0, 'iat' => now.to_i, @@ -222,12 +222,12 @@ def assert_initialized_token(target) # rubocop:disable Metrics/AbcSize 'sub' => 'subject-identifier', 'typ' => 'Bearer', 'azp' => 'client-identifier' - ) + } end let(:token) { @token ||= JWT.encode(token_payload, rsa_private, 'RS256') } let(:refresh_token_payload) do - Hash( + { 'exp' => exp_refresh, 'nbf' => 0, 'iat' => refreshed_now.to_i, @@ -236,7 +236,7 @@ def assert_initialized_token(target) # rubocop:disable Metrics/AbcSize 'sub' => 'subject-identifier', 'typ' => 'Bearer', 'azp' => 'client-identifier' - ) + } end let(:refresh_token) { @refresh_token ||= JWT.encode(refresh_token_payload, rsa_private, 'RS256') } From 5a1d55a5fe275bf49f6edf1f7510d615e533a226 Mon Sep 17 00:00:00 2001 From: Mathias Klippinge Date: Fri, 8 Jun 2018 15:11:10 +0200 Subject: [PATCH 09/11] Rubocop --- .rubocop_todo.yml | 2 +- spec/oauth2/access_token_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index aaaef51d..9b42f824 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -18,4 +18,4 @@ Metrics/CyclomaticComplexity: Max: 7 Metrics/ClassLength: - Max: 103 + Max: 104 diff --git a/spec/oauth2/access_token_spec.rb b/spec/oauth2/access_token_spec.rb index d5adf9d0..f5c16e54 100644 --- a/spec/oauth2/access_token_spec.rb +++ b/spec/oauth2/access_token_spec.rb @@ -221,7 +221,7 @@ def assert_initialized_token(target) # rubocop:disable Metrics/AbcSize 'aud' => 'client-identifier', 'sub' => 'subject-identifier', 'typ' => 'Bearer', - 'azp' => 'client-identifier' + 'azp' => 'client-identifier', } end let(:token) { @token ||= JWT.encode(token_payload, rsa_private, 'RS256') } @@ -235,7 +235,7 @@ def assert_initialized_token(target) # rubocop:disable Metrics/AbcSize 'aud' => 'client-identifier', 'sub' => 'subject-identifier', 'typ' => 'Bearer', - 'azp' => 'client-identifier' + 'azp' => 'client-identifier', } end let(:refresh_token) { @refresh_token ||= JWT.encode(refresh_token_payload, rsa_private, 'RS256') } From 0cc3bc7e09c4801411b3c04f294955865bb25a4b Mon Sep 17 00:00:00 2001 From: Mathias Klippinge Date: Fri, 8 Jun 2018 15:13:08 +0200 Subject: [PATCH 10/11] Update changelog --- CHANGELOG.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b673445..cbe7cdb5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,5 @@ # Change Log + All notable changes to this project will be documented in this file. ## [unreleased] @@ -12,6 +13,8 @@ All notable changes to this project will be documented in this file. - _Dependency_: Upgrade Faraday to 0.13.x (@zacharywelch) - _Dependency_: Upgrade jwt to 2.x.x (@travisofthenorth) - Fix logging to `$stdout` of request and response bodies via Faraday's logger and `ENV["OAUTH_DEBUG"] == 'true'` +- Read issued_at and expires_at from the token instead of using Time.now [#391] +- Take clock skew into consideration when checking expired? [#391] ## [1.4.0] - 2017-06-09 @@ -49,14 +52,17 @@ All notable changes to this project will be documented in this file. ## [1.0.0] - 2014-07-09 ### Added + - Add an implementation of the MAC token spec. ### Fixed + - Fix Base64.strict_encode64 incompatibility with Ruby 1.8.7. ## [0.5.0] - 2011-07-29 ### Changed + - [breaking] `oauth_token` renamed to `oauth_bearer`. - [breaking] `authorize_path` Client option renamed to `authorize_url`. - [breaking] `access_token_path` Client option renamed to `token_url`. @@ -89,7 +95,6 @@ All notable changes to this project will be documented in this file. ## [0.0.4] + [0.0.3] + [0.0.2] + [0.0.1] - 2010-04-22 - [0.0.1]: https://github.com/oauth-xx/oauth2/compare/311d9f4...v0.0.1 [0.0.2]: https://github.com/oauth-xx/oauth2/compare/v0.0.1...v0.0.2 [0.0.3]: https://github.com/oauth-xx/oauth2/compare/v0.0.2...v0.0.3 From ad6db038a630d9f7caf65cbe0ddfeadf83e7eb66 Mon Sep 17 00:00:00 2001 From: Mathias Klippinge Date: Fri, 8 Jun 2018 15:33:39 +0200 Subject: [PATCH 11/11] Take min validity into consideration --- .rubocop_todo.yml | 5 +---- CHANGELOG.md | 1 + lib/oauth2/access_token.rb | 4 +++- spec/oauth2/access_token_spec.rb | 20 +++++++++----------- 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 9b42f824..b40094c7 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -14,8 +14,5 @@ Style/EachWithObject: Style/ExpandPathArguments: Enabled: false -Metrics/CyclomaticComplexity: - Max: 7 - Metrics/ClassLength: - Max: 104 + Max: 105 diff --git a/CHANGELOG.md b/CHANGELOG.md index cbe7cdb5..52def80b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ All notable changes to this project will be documented in this file. - Fix logging to `$stdout` of request and response bodies via Faraday's logger and `ENV["OAUTH_DEBUG"] == 'true'` - Read issued_at and expires_at from the token instead of using Time.now [#391] - Take clock skew into consideration when checking expired? [#391] +- Take minimum validity into consideration when checking expired? [#391] ## [1.4.0] - 2017-06-09 diff --git a/lib/oauth2/access_token.rb b/lib/oauth2/access_token.rb index 44315795..aba9f345 100644 --- a/lib/oauth2/access_token.rb +++ b/lib/oauth2/access_token.rb @@ -1,5 +1,7 @@ module OAuth2 class AccessToken + MIN_VALIDITY = 30 + attr_reader :client, :token, :expires_in, :issued_at, :expires_at, :params, :time_skew attr_accessor :options, :refresh_token, :response @@ -81,7 +83,7 @@ def expires? # # @return [Boolean] def expired? - expires? && (expires_at + time_skew <= Time.now.to_i) + expires? && (expires_at + time_skew - MIN_VALIDITY <= Time.now.to_i) end # Refreshes the current Access Token diff --git a/spec/oauth2/access_token_spec.rb b/spec/oauth2/access_token_spec.rb index f5c16e54..21ef938e 100644 --- a/spec/oauth2/access_token_spec.rb +++ b/spec/oauth2/access_token_spec.rb @@ -152,25 +152,23 @@ def assert_initialized_token(target) # rubocop:disable Metrics/AbcSize expect(access).to be_expired end - describe 'time skew' do - let(:time_skew) { 10 } + describe 'min validity' do + let(:old_now) { 1_528_454_438 } let(:expires_in) { 300 } - let(:expires_at) { Time.now.to_i - 10 + expires_in } + let(:expires_at) { 1_528_454_438 + expires_in } let!(:access) { described_class.new(client, token, :refresh_token => 'abaca', :expires_at => expires_at, :expires_in => expires_in) } + let(:now) { Time.at(expires_at) - AccessToken::MIN_VALIDITY } - context 'when not within time skew correction' do - let(:now) { Time.at(expires_at) + time_skew + 1 } - + context 'when not within min validity correction' do it 'access is expired' do allow(Time).to receive(:now).and_return(now) expect(access).to be_expired end end - context 'when within time skew correction' do - let(:now) { Time.at(expires_at) + time_skew - 1 } - + context 'when within min validity correction' do it 'access is not expired' do + allow(Time).to receive(:now).and_return(now - 1) expect(access).not_to be_expired end end @@ -256,7 +254,7 @@ def assert_initialized_token(target) # rubocop:disable Metrics/AbcSize end context 'when not within time skew correction' do - let(:local_now) { Time.at(exp) + time_skew + 1 } + let(:local_now) { Time.at(exp) + time_skew - AccessToken::MIN_VALIDITY } it 'access is expired' do allow(Time).to receive(:now).and_return(local_now) @@ -265,7 +263,7 @@ def assert_initialized_token(target) # rubocop:disable Metrics/AbcSize end context 'when within time skew correction' do - let(:local_now) { Time.at(exp) + time_skew - 1 } + let(:local_now) { Time.at(exp) + time_skew - AccessToken::MIN_VALIDITY - 1 } it 'access is not expired' do allow(Time).to receive(:now).and_return(local_now)