omniauth / omniauth-okta

OAuth2 strategy for Okta
MIT License
41 stars 37 forks source link

Refactor access token rewrite and id_token #25

Closed BobbyMcWho closed 2 years ago

BobbyMcWho commented 2 years ago

While the approach in #24 and #17, worked I was a bit perplexed on why in the first place we were creating a new token from the old one, overwriting Strategies::OAuth2's @access_token attr_reader, and creating a new token that is essentially identical to the old one.

I've removed that method alias/overwrite, rely on the access_token that the base OAuth2 strategy gives us, and properly access the id_token on the access_token.

I've also fleshed out the specs a bit more, to validate that things are working as expected w/ the deleted access_token method.

I've released 2f11b42 on RubyGems as v2.0.0.rc1

BobbyMcWho commented 2 years ago

Closing to run CI

ryanswood commented 2 years ago

@BobbyMcWho Glad you went that extra step to simplify the handling of the access token. The change here looks good. Thank you for working with us to make this happen.

BobbyMcWho commented 2 years ago

@ryanswood if you wouldn't mind trying out the release candidate, I can merge and release an official sometime next week. I don't have access to a production app with Okta oauth these days, else I'd validate

ryanswood commented 2 years ago

@BobbyMcWho I was out Friday and see this has now been merged with an official 2.0 release. Thank you for working with us to make this change. I was able to test it and all is well.