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

Use omniauth 2.x to ensure latest security updates #152

Merged
merged 2 commits into from Jun 18, 2022

Conversation

lucas-aragno
Copy link

My team have been working on some security updates on our app and we noticed omniauth-oauth2 was listing any version from omniauth between 1.9 and 3 as a valid dependency. bc of that we kept running on this security issue. We manually enforced omniauth 2.x on our Gemfile to solve it, but I thought it may be useful to bump the version directly on the gem since the issue seems to exist on all 1.9.x versions

@coveralls
Copy link

coveralls commented May 27, 2022

Pull Request Test Coverage Report for Build 2397096803

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 84.615%

Totals Coverage Status
Change from base Build 1747006164: 0.0%
Covered Lines: 77
Relevant Lines: 91

💛 - Coveralls

@BobbyMcWho
Copy link
Member

So, the reason I left it flexible was that omniauth 2 had some breaking changes, and not all 3rd party omniauth strategies that inherit from this gem had updated and tested against the new omniauth. I'm not opposed to being stricter on this, but it would likely be a major version bump for this gem.

Also, prefer '~> 2.0' to ['>= 2.0', '< 3']

@lucas-aragno
Copy link
Author

I see, yeah I thought that may be the case. I agree this would require a major bump on the gem.

I think '~> 2.0' makes sense

@lucas-aragno
Copy link
Author

@BobbyMcWho Any updates on this? I'm happy to close this PR for now if this isn't something we wanna get it atm

@BobbyMcWho
Copy link
Member

Leave it open, it's just low priority for me at the moment

@BobbyMcWho BobbyMcWho merged commit 043813c into omniauth:master Jun 18, 2022
@BobbyMcWho
Copy link
Member

This has been released in v1.8.0
release notes
rubygems

jessieay added a commit to jessieay/omniauth-azure-activedirectory-v2 that referenced this pull request Sep 7, 2022
* One of the breaking changes in OmniAuth 2.0+ relates to how relative
  URL installations are handled. See: omniauth/omniauth#903 and https://github.com/omniauth/omniauth/wiki/Upgrading-to-2.0#relative-root-apps
* As a result, when `azure_activedirectory_v2` is used with OmniAuth
  2.0+ for an app that lives at a relative URl, the `#callback_url` is incorrect (the relative URL is included twice).
* This is because OmniAuth is now prefixing the default
  Strategy#request_path and Strategy#callback_path with SCRIPT_NAME, but `azure_activedirectory_v2` is also adding `script_name` to `callback_url`.
* This update makes this gem compatible with OmniAuth 2.0+ but will break
  relative URL installatons for OmniAuth 1.x so I've also updated the
  gemspec to rely on a version of omniauth-oauth2 that has a dependency
  on omniauth 2.0: omniauth/omniauth-oauth2#152 (comment)
jessieay added a commit to jessieay/omniauth-azure-activedirectory-v2 that referenced this pull request Sep 7, 2022
* One of the breaking changes in OmniAuth 2.0+ relates to how relative
  URL installations are handled. See: omniauth/omniauth#903 and https://github.com/omniauth/omniauth/wiki/Upgrading-to-2.0#relative-root-apps
* As a result, when `azure_activedirectory_v2` is used with OmniAuth
  2.0+ for an app that lives at a relative URl, the `#callback_url` is incorrect (the relative URL is included twice).
* This is because OmniAuth is now prefixing the default
  Strategy#request_path and Strategy#callback_path with SCRIPT_NAME, but `azure_activedirectory_v2` is also adding `script_name` to `callback_url`.
* This update makes this gem compatible with OmniAuth 2.0+ but will break
  relative URL installatons for OmniAuth 1.x so I've also updated the
  gemspec to rely on a version of omniauth-oauth2 that has a dependency
  on omniauth 2.0: omniauth/omniauth-oauth2#152 (comment)
* Similar change to omniauth-google-oauth2 was made here: zquestz/omniauth-google-oauth2#403
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.

None yet

3 participants