Handle invalid token when adding redirection headers#1945
Merged
randyabson merged 1 commit intomainfrom Mar 26, 2025
Merged
Conversation
2fd002d to
b79036c
Compare
b4a06b4 to
a31fe87
Compare
andru-h
approved these changes
Mar 21, 2025
andru-h
left a comment
There was a problem hiding this comment.
LGTM with my limited context on the gem
ltribolet
approved these changes
Mar 21, 2025
lizkenyon
reviewed
Mar 21, 2025
Contributor
lizkenyon
left a comment
There was a problem hiding this comment.
I believe this looks good!
I am just going to flag this with the App Access team for a quick once over.
Could you please add a note in the CHANGELOG.md
zzooeeyy
reviewed
Mar 25, 2025
Contributor
zzooeeyy
left a comment
There was a problem hiding this comment.
Thanks! The changes makes sense to me, I just had a few questions about the test setup.
lizkenyon
approved these changes
Mar 26, 2025
lizkenyon
approved these changes
Mar 26, 2025
Contributor
lizkenyon
left a comment
There was a problem hiding this comment.
Approving so you can merge, once Zoey's questions are answered.
b670160 to
d81cc9b
Compare
Add changelog Simplify test
d81cc9b to
8c3e282
Compare
This file contains hidden or 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
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.
What this PR does
Currently, while decoding a token while redirecting to login, it's possible for errors to occur. Example error:
Signature has expired.This PR rescues token decoding errors and handles them gracefully. The login url will not include the shop param in this scenario.
Exception:
Reviewer's guide to testing
The test covers this scenario by simulating an error decoding the session token, in this case the error is
Not enough or too many segmentsbut it covers any error while decoding tokens.Things to focus on
I'm unfamiliar with this repo so please ensure the way that the exception is handled won't cause issues in the redirect.
Checklist
Before submitting the PR, please consider if any of the following are needed:
CHANGELOG.mdif the changes would impact usersREADME.md, if appropriate./docs, if necessary