diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index e7701734..b40094c7 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -13,3 +13,6 @@ Style/EachWithObject: # Once we drop Rubies that lack support for __dir__ we can turn this on. Style/ExpandPathArguments: Enabled: false + +Metrics/ClassLength: + Max: 105 diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b673445..52def80b 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,9 @@ 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] +- Take minimum validity into consideration when checking expired? [#391] ## [1.4.0] - 2017-06-09 @@ -49,14 +53,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 +96,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 diff --git a/lib/oauth2/access_token.rb b/lib/oauth2/access_token.rb index 2c152b89..aba9f345 100644 --- a/lib/oauth2/access_token.rb +++ b/lib/oauth2/access_token.rb @@ -1,6 +1,8 @@ module OAuth2 class AccessToken - attr_reader :client, :token, :expires_in, :expires_at, :params + MIN_VALIDITY = 30 + + attr_reader :client, :token, :expires_in, :issued_at, :expires_at, :params, :time_skew attr_accessor :options, :refresh_token, :response class << self @@ -37,17 +39,26 @@ 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 opts = opts.dup + [: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 + + @issued_at = token_payload.fetch('iat', local_now) + @expires_at ||= token_payload.fetch('exp', nil) + @expires_at &&= @expires_at.to_i - @expires_at ||= Time.now.to_i + @expires_in if @expires_in + @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', :param_name => opts.delete(:param_name) || 'access_token'} @@ -72,7 +83,7 @@ def expires? # # @return [Boolean] def expired? - expires? && (expires_at <= Time.now.to_i) + expires? && (expires_at + time_skew - MIN_VALIDITY <= Time.now.to_i) end # Refreshes the current Access Token @@ -150,6 +161,39 @@ 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" + # } + # + # + # @return [Hash] a hash of token property values + def token_payload + @token_payload ||= decoded_payload + end + + # A JWT token consists of three dot separated parts: + # 1) header + # 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) + JSON Base64.decode64(jwt_payload) + rescue StandardError + {} + 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 diff --git a/spec/oauth2/access_token_spec.rb b/spec/oauth2/access_token_spec.rb index 699b31d9..21ef938e 100644 --- a/spec/oauth2/access_token_spec.rb +++ b/spec/oauth2/access_token_spec.rb @@ -151,6 +151,28 @@ def assert_initialized_token(target) # rubocop:disable Metrics/AbcSize allow(Time).to receive(:now).and_return(@now) expect(access).to be_expired end + + describe 'min validity' do + let(:old_now) { 1_528_454_438 } + let(:expires_in) { 300 } + 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 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 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 + end end describe '#refresh' do @@ -184,4 +206,70 @@ 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 + { + '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 + { + '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 - AccessToken::MIN_VALIDITY } + + 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 - AccessToken::MIN_VALIDITY - 1 } + + it 'access is not expired' do + allow(Time).to receive(:now).and_return(local_now) + expect(access).not_to be_expired + end + end + end + end end