Skip to content

Conversation

rickselby
Copy link
Contributor

@rickselby rickselby commented Jul 18, 2022

I'll start by saying I'm just a user of the gem, I have no idea how the oauth2 process works, we use it with omniauth and cognito for authentication in AWS.

When upgrading to 2.0.6, our logins broke. On investigation, I see the following in the logs:

OAuth2::AccessToken.from_hash: `hash` contained more than one 'token' key (["id_token", "access_token"]); using "id_token".

Looking at the code changes from #624, the access_token was previously being preferred over the id_token, so I'm assuming this is the bug.

I've corrected this and added a test to ensure the gem will still function as it did before.

@codecov-commenter
Copy link

codecov-commenter commented Jul 18, 2022

Codecov Report

Merging #626 (9a88932) into master (8df9452) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 9a88932 differs from pull request most recent head 49b85f5. Consider uploading reports for the commit 49b85f5 to get more accurate results

@@            Coverage Diff            @@
##            master      #626   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           13        13           
  Lines          419       419           
=========================================
  Hits           419       419           
Impacted Files Coverage Δ
lib/oauth2/access_token.rb 100.00% <100.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Copy link
Member

@pboling pboling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pboling pboling self-assigned this Aug 1, 2022
@pboling pboling added the bug label Aug 1, 2022
@pboling pboling added this to the 2.0.7 milestone Aug 1, 2022
@pboling pboling merged commit 5e20fdd into ruby-oauth:master Aug 1, 2022
@vhiairrassary
Copy link

We have encountered the same issue with omniauth-oauth2 and AWS Cognito, and validated this fix:

  • 2.0.5 ✅
  • 2.0.6 🔴
  • master ✅

Thank you 🥳

@pboling
Copy link
Member

pboling commented Aug 22, 2022

@vhiairrassary @rickselby Thanks for the reports. The release 2.0.7 should fix this, and be available now.

bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request Oct 28, 2022
https://build.opensuse.org/request/show/1010030
by user coolo + dimstar_suse
updated to version 2.0.9
 see installed CHANGELOG.md
updated to version 2.0.7
 see installed CHANGELOG.md
  ## [2.0.7] - 2022-08-22
  ### Added
  - [#629](ruby-oauth/oauth2#629) - Allow POST of JSON to get token (@pboling, @terracatta)

  ### Fixed
  - [#626](ruby-oauth/oauth2#626) - Fixes a regression in 2.0.6. Will now prefer the key order from the lookup, not the hash keys (@rickselby)
    - Note: This fixes compatibility with `omniauth-oauth2` and AWS
  - [#625](ruby-oauth/oauth2#625) - Fixes the printed version in the post install message (@hasghari)

updated to version 2.0.6
 see installed CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants