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

redirect to root during oauth#callback if attempted phishing attack #1605

Merged
merged 7 commits into from
Dec 9, 2022

Conversation

nelsonwittwer
Copy link
Contributor

What this PR does

Patches a phishing vulnerability with the OAuth callback flow.

Reviewer's guide to testing

Should still be able to install app and be redirected to app after install.

Checklist

Before submitting the PR, please consider if any of the following are needed:

  • Update CHANGELOG.md if the changes would impact users
  • Update README.md, if appropriate.
  • Update any relevant pages in /docs, if necessary
  • [] For security fixes, the Disclosure Policy must be followed.

@nelsonwittwer nelsonwittwer force-pushed the nelsonwittwer/oauth_security branch from 7680031 to a19ac26 Compare December 8, 2022 17:35
@nelsonwittwer nelsonwittwer changed the title render 404 if attempted phishing attack redirect to root during oauth#callback if attempted phishing attack Dec 8, 2022
@klenotiw
Copy link
Contributor

klenotiw commented Dec 8, 2022

I can't install my app when its pointing to this branch. Looks like something is wrong with the url.

Screenshot 2022-12-08 at 1 45 16 PM

@nelsonwittwer nelsonwittwer force-pushed the nelsonwittwer/oauth_security branch 2 times, most recently from 9d0bf97 to 616993f Compare December 8, 2022 21:23
@nelsonwittwer nelsonwittwer force-pushed the nelsonwittwer/oauth_security branch from 616993f to 685f377 Compare December 8, 2022 21:24
Co-authored-by: Bill Klenotiz <54754550+klenotiw@users.noreply.github.com>
Copy link
Contributor

@klenotiw klenotiw left a comment

Choose a reason for hiding this comment

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

Looks good, I was able to install.

@nelsonwittwer nelsonwittwer merged commit 219dcfc into main Dec 9, 2022
@nelsonwittwer nelsonwittwer deleted the nelsonwittwer/oauth_security branch December 9, 2022 15:13
fabriazza pushed a commit to fabriazza/shopify_app that referenced this pull request Feb 1, 2023
…hopify#1605)

* render 404 if attempted phishing attack

* changelog

* rubocop style fixes

* root address instead of 404

* redirect to param[:host] after sanitization

* Update app/controllers/shopify_app/callback_controller.rb

Co-authored-by: Bill Klenotiz <54754550+klenotiw@users.noreply.github.com>

Co-authored-by: Bill Klenotiz <54754550+klenotiw@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants