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

Fix Issues with EnsureAuthenticatedLinks and updated OAuth flow #1549

Merged
merged 1 commit into from
Nov 4, 2022

Conversation

rdillensnyder
Copy link
Contributor

@rdillensnyder rdillensnyder commented Oct 25, 2022

What this PR does

The 'embedded' param from the updated OAuth flow is lost by the EnsureAuthenticatedLinks concern during redirects for deep linked pages or Admin extensions. Adding this param to ensure the flow completes successfully.

Reviewer's guide to testing

Directly link to a page for an app using the EnsureAuthenticatedLinks concern such as Order Printer

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.

@andyw8 andyw8 merged commit feced71 into main Nov 4, 2022
@andyw8 andyw8 deleted the authenticatedlinks-patch branch November 4, 2022 14:54
@rdillensnyder rdillensnyder linked an issue Nov 4, 2022 that may be closed by this pull request
@netwire88
Copy link

Thanks all, looking forward to this fix.

nelsonwittwer pushed a commit that referenced this pull request Nov 12, 2022
nelsonwittwer added a commit that referenced this pull request Dec 7, 2022
* session stored in rails instead of library

* move responsiblity for session persistence

* move load_current_session to login_protection

* FIXME notes

* Move session concerns back to session utils

* Add `embedded` param to `splash_page` (#1549)

* use online tokens appropriately if there is a user session storage available

* move load_current_session to login_protection

* Move session concerns back to session utils

* refactor login protection and tests with sesison ownership change

* rubocop'd

* no more session_storage in generated example

* add x86_65-linux platform for CI

* no more local pointer to api lib

* more gemlock

Co-authored-by: rdillensnyder <101280580+rdillensnyder@users.noreply.github.com>
Co-authored-by: Teddy Hwang <teddy.hwang@shopify.com>
fabriazza pushed a commit to fabriazza/shopify_app that referenced this pull request Feb 1, 2023
fabriazza pushed a commit to fabriazza/shopify_app that referenced this pull request Feb 1, 2023
…ify#1563)

* session stored in rails instead of library

* move responsiblity for session persistence

* move load_current_session to login_protection

* FIXME notes

* Move session concerns back to session utils

* Add `embedded` param to `splash_page` (Shopify#1549)

* use online tokens appropriately if there is a user session storage available

* move load_current_session to login_protection

* Move session concerns back to session utils

* refactor login protection and tests with sesison ownership change

* rubocop'd

* no more session_storage in generated example

* add x86_65-linux platform for CI

* no more local pointer to api lib

* more gemlock

Co-authored-by: rdillensnyder <101280580+rdillensnyder@users.noreply.github.com>
Co-authored-by: Teddy Hwang <teddy.hwang@shopify.com>
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.

EnsureAuthenticatedLinks doesn't understand deep linked pages
3 participants