Skip to content

Commit

Permalink
Add expires_at, to match OmniAuth auth schema
Browse files Browse the repository at this point in the history
OmniAuth's [Auth Hash Schema] should return an `expires_at` field as a
timestamp, but this gem returns `expires_in`.  For compatibility with
other `oauth2` OmniAuth strategies, we should also return `expires_at`.

I'm not sure if the best place to fix it is here or upstream, in
`Rack::OAuth2::AccessToken`.  On the one hand, the `oauth2` gem handles it
in `OAuth2::AccessToken`.  On the other hand, the OmniAuth strategy is
the only place we can ensure minimal latency between the server response
and `expires_at` computation.  I chose here. 🙂

[Auth Hash Schema]: https://github.com/omniauth/omniauth/wiki/Auth-Hash-Schema

n.b. I would have assumed that "timestamp" in the schema meant a Time
object, but all of the gems that inherit from `omniauth-oauth2` return
`Time#to_i`, which is also appropriate.
  • Loading branch information
nevans committed Dec 6, 2022
1 parent 147b0ab commit 2e8758d
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
7 changes: 7 additions & 0 deletions lib/omniauth/strategies/openid_connect.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class OpenIDConnect # rubocop:disable Metrics/ClassLength
},
code_challenge_method: 'S256',
}
option :expires_latency # seconds taken from credentials expires_at

def uid
user_info.raw_attributes[options.uid_field.to_sym] || user_info.sub
Expand Down Expand Up @@ -95,6 +96,7 @@ def uid
token: access_token.access_token,
refresh_token: access_token.refresh_token,
expires_in: access_token.expires_in,
expires_at: @access_token_expires_at,
scope: access_token.scope,
}
end
Expand Down Expand Up @@ -262,6 +264,11 @@ def access_token
@access_token = client.access_token!(token_request_params)
verify_id_token!(@access_token.id_token) if configured_response_type == 'code'

if @access_token.expires_in
@access_token_expires_at = Time.now.to_i + @access_token.expires_in
@access_token_expires_at -= options.expires_latency if options.expires_latency
end

@access_token
end

Expand Down
11 changes: 9 additions & 2 deletions test/lib/omniauth/strategies/openid_connect_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,7 @@ def test_credentials
strategy.options.issuer = 'example.com'
strategy.options.client_signing_alg = :RS256
strategy.options.client_jwk_signing_key = jwks.to_json
strategy.options.expires_latency = 60

id_token = stub('OpenIDConnect::ResponseObject::IdToken')
id_token.stubs(:verify!).returns(true)
Expand All @@ -530,14 +531,20 @@ def test_credentials
access_token = stub('OpenIDConnect::AccessToken')
access_token.stubs(:access_token).returns(SecureRandom.hex(16))
access_token.stubs(:refresh_token).returns(SecureRandom.hex(16))
access_token.stubs(:expires_in).returns(Time.now)
access_token.stubs(:expires_in).returns(3599)
access_token.stubs(:scope).returns('openidconnect')
access_token.stubs(:id_token).returns(jwt.to_s)

client.expects(:access_token!).returns(access_token)
access_token.expects(:refresh_token).returns(access_token.refresh_token)
access_token.expects(:expires_in).returns(access_token.expires_in)

start = Time.now.to_i + access_token.expires_in - 60
creds = strategy.credentials
stop = Time.now.to_i + access_token.expires_in - 60
expires_at = creds.delete(:expires_at)
assert_includes start..stop, expires_at

assert_equal(
{
id_token: access_token.id_token,
Expand All @@ -546,7 +553,7 @@ def test_credentials
expires_in: access_token.expires_in,
scope: access_token.scope,
},
strategy.credentials
creds
)
end

Expand Down

0 comments on commit 2e8758d

Please sign in to comment.