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

Update to omniauth-github 2. #1314

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

simi
Copy link

@simi simi commented Jul 26, 2023

ℹ️ I'm trying to fix security report reported to RubyGems.org instance of shipit instance. Sadly, since there is no testing shipit instance at RubyGems.org side, I have not tested this deployed, I just did quick test the app boots locally. Is there any chance to test this at Shopify side? If not, feel free to ping me, I'll try to setup proper testing environment and test the whole GH flow.

@casperisfine
Copy link
Contributor

Is there any chance to test this at Shopify side?

Hum, I haven't done this in a very long time, but it's possible to test this locally, IIRC you don't need a publicly available instance for authentication.

per https://github.com/omniauth/omniauth/wiki/Upgrading-to-2.0 there are some backward incompatibilities

OmniAuth now defaults to only POST as allowed request_phase methods.

I think this will be an issue, as we directly redirect to authentication, without submitting a form or anything.

So we'll either have to disable that default and keep the old behavior (meaning CVE-2015-9284 is still a thing) or add an authentication page.

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