Skip to content

Update gem to work with omniauth-oauth2 1.8.0 #19

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jaedonfarrugia
Copy link

@jaedonfarrugia jaedonfarrugia commented Jun 10, 2024

authentication flow was failing on authorization code step due to omniauth / omniauth-oauth2 updates

given the previous requirement to use omniauth-oauth2 1.7.0, this gem could not be used in combination with google/microsoft strategies which rely on versions above 1.8.0

s.add_dependency 'omniauth-oauth2', '~> 1.7.1'
spec.add_runtime_dependency 'jwt', '~> 2.0'
spec.add_runtime_dependency 'omniauth', '>= 2.0.0', '< 2.2.0'
spec.add_runtime_dependency 'omniauth-oauth2', '~> 1.8.0'
Copy link
Author

Choose a reason for hiding this comment

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

this dependency has been bumped to 1.8.0


def build_access_token
redirect_uri = request.params['redirect_uri'] || callback_url
authorization_code = request.params['code'] || JSON.parse(request.body.read)['code']
Copy link
Author

Choose a reason for hiding this comment

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

this strategy has been updated to read the code from the request body

@jaedonfarrugia jaedonfarrugia changed the title Update gem to work with omniauth-oauth2 1.8 Update gem to work with omniauth-oauth2 1.8.0 Jun 10, 2024
authorization code was no longer working due to omniauth /
omniauth-oauth2 updates, there were also conflicts with other gems using
a later version of omniauth-oauth2 (1.8), meaning this gem could not be
used in combination with google/microsoft strategies
@pond
Copy link

pond commented Jul 10, 2024

It would be great if this could be merged ASAP - it is a rather overdue change. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants