Skip to content
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

omniauth-oauth2 from 1.7.0 to 1.7.2 broke cognito authentication #423

Closed
mTouhid opened this issue Nov 26, 2021 · 2 comments · Fixed by #408
Closed

omniauth-oauth2 from 1.7.0 to 1.7.2 broke cognito authentication #423

mTouhid opened this issue Nov 26, 2021 · 2 comments · Fixed by #408
Assignees
Labels
bug Something isn't working ruby Pull requests that update Ruby code

Comments

@mTouhid
Copy link
Contributor

mTouhid commented Nov 26, 2021

Updating omniauth-oauth2 from 1.7.0 to 1.7.2 broke the cognito authentication passthru.

@mTouhid mTouhid linked a pull request Nov 26, 2021 that will close this issue
@mTouhid mTouhid added bug Something isn't working ruby Pull requests that update Ruby code labels Nov 26, 2021
@Gary-H9
Copy link
Contributor

Gary-H9 commented Nov 26, 2021

Background:
Dependabot opened a dependency relating to upgrading omniaith-oauth2 from v1.7.0 to v1.7.2. As such numerous dependencies were also upgraded. (omniauth-rails_csrf_protection was not included in the upgraded gems)

Steps taken:

  • Initially we pulled the branch and ran make serve locally, this resulted in login failing.
  • In addition to this running rSpec resulted in spec/acceptance/sign_in_spec.rb failing.

Running developer tools in Chrome on the locally served page, showed that after the cognito request there was no authorisation and callback requests.

Furthermore, in rSpec the following error occurred when removing the mock auth data in the sign_in_spec.rb (line 14-18).

WARN -- omniauth: Attack prevented by OmniAuth::AuthenticityTokenProtection
ERROR -- omniauth: (cognito) Authentication failure! Forbidden: OmniAuth::AuthenticityError, Forbidden redirects to the root path (FAILED - 1)

At this point we spent a few hours considering the possibility that the /sign_in route should POST and not GET. This was largely influenced by numerous mentions of this throughout other peoples troubleshooting and documentation. For example this response, this and also this "Hey all, please note that using POST as the method is the new default in OmniAuth 2+, and that is to increase security and is definitely recommended.".

Reading numerous comments further down made the team consider upgrading the CSRF protection.

gem "omniauth-rails_csrf_protection", "~> 0.1"
gem "omniauth-rails_csrf_protection", "~> 1.0.0"

We changed this, rebuilt the container and tested locally successfully (via make serve and rSpec). After this we ran the new update through the pipeline to Development and again tested successfully before allow the change to propagate into Live.

This resolution does not exclude the possibility that in future we will need to upgrade the sign_in method to use POST and not GET.

@mTouhid
Copy link
Contributor Author

mTouhid commented Nov 26, 2021

Thank you @Gary-H9. I have tested the dev and I was able to login using Azure Devl.

@Gary-H9 Gary-H9 closed this as completed Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ruby Pull requests that update Ruby code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants