Skip to content
This repository has been archived by the owner on Apr 11, 2024. It is now read-only.

Extend cookie OAuth session to allow initial app loads #70

Merged
merged 1 commit into from
Jan 13, 2021

Conversation

paulomarg
Copy link
Contributor

WHY are these changes introduced?

While updating shopify-app-node to use this library, I ran into an issue where OAuth worked normally, but since we destroyed the cookie session right away, we were unable to run the initial request to the app in a logged in state, so that the frontend can build its App Bridge and get a JWT.

These changes extend the OAuth cookie session by 30 seconds instead of deleting it right away, which allows the app to load itself to cover the above scenario.

WHAT is this pull request doing?

Extending the cookie session, and changing loadCurrentSession to fall back to the cookie version if JWT isn't available yet. Since the cookie session is only extended for a short period of time and there is no support for 3rd party cookies baked into the library, any requests made without a JWT from an embedded app would fail outright due to the OAuth session cookie not being available to the server.

Type of change

  • Patch: Bug (non-breaking change which fixes an issue)

@paulomarg paulomarg requested a review from a team as a code owner January 13, 2021 18:49
@paulomarg paulomarg force-pushed the extend_oauth_cookie_session branch 2 times, most recently from dfa39c5 to 0e75466 Compare January 13, 2021 19:02
Copy link
Contributor

@thecodepixi thecodepixi left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@paulomarg paulomarg merged commit 1a736b3 into main Jan 13, 2021
@paulomarg paulomarg deleted the extend_oauth_cookie_session branch January 13, 2021 19:24
@paulomarg paulomarg temporarily deployed to production January 13, 2021 19:45 Inactive
@paulomarg paulomarg temporarily deployed to production January 27, 2021 19:19 Inactive
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants