Use postMessage to redirect to Shopify during authentication #366
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kevinhughes27
approved these changes
Dec 15, 2016
peterjm
approved these changes
Dec 15, 2016
|
Requires the beta flag to be removed from this feature, but I'm sure you know that 👍 |
Hammadk
changed the title
Hammadk
changed the title
|
Updated and tested in the following browsers:
|
Hammadk
force-pushed
the
use-post-message-to-redirect
branch
from
3d738fc to
644a3eb
Compare
kevinhughes27
approved these changes
Jan 6, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A recent change was proposed in Chrome Canary 57.0.2933.0 that will break embedded app authentication.
Proposed solution:
window.location.originto redirect to the authentication URL, we should usewindow.parent.postMessage(this logic is in#fullpage_redirect_to).This PR won't be merged until extensively tested
Related refactor:
This PR also replaces
#redirect_to_with_fallbackwith the Rails method:redirect_to.#redirect_to_with_fallbackdoesn't work like we think it does. The status and location headers take priority over the inline response. The fallback only happens if the browser ignores the status and location headers -- which doesn't happen on any modern browser.This PR moves
#sanitized_shop_nameand#sanitize_shop_param(params)fromSessionsConcerntoLoginProtection. LoginProtection is already includedSessionController.Please see the following issue for reference: https://github.com/Shopify/shopify/issues/93680
@nwtn @peterjm @kevinhughes27 Please review