[Website] Improve OAuth flow for private GitHub repositories#3181
Merged
[Website] Improve OAuth flow for private GitHub repositories#3181
Conversation
When startPlaygroundWeb throws an error (e.g., GitAuthenticationError), the catch block handles it but execution continues past the catch to run client setup code (setupPostMessageRelay, addClientInfo, etc.). This happens because the `playground` variable may already be set via the onClientConnected callback before the error is thrown, so the `if (!playground) return` check doesn't prevent continued execution. Adding an early return at the end of the catch block ensures we don't attempt to set up a client that failed to boot properly.
…n at call time
Two fixes for the GitHub OAuth flow:
1. buildOAuthRedirectUrl: Converts URL fragment blueprints (e.g., #{"steps":...})
to blueprint-url query parameters before the OAuth redirect. URL fragments
are not sent to servers in HTTP requests, so they would be lost during the
OAuth redirect flow.
2. createGitAuthHeaders: Moves token retrieval inside the returned function
so the token is captured at call time, not at creation time. This ensures
the OAuth token acquired during the redirect flow is used for subsequent
GitHub API requests.
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.
Motivation for the change, related issues
When using blueprints that reference private GitHub repositories, the OAuth authentication flow has several issues:
Hash blueprints are lost: URL fragments (e.g.,
#{"steps":...}) are not sent to servers in HTTP requests, so they're lost during the OAuth redirect to GitHub and back.Token not used after OAuth: The
createGitAuthHeaders()function captured the OAuth token at creation time, but the token is only available after the OAuth redirect completes. It still works here because the page reloads after OAuth, and the token is acquired before boot starts on the fresh page. The bug would only show when we remove the blueprint parameters as we do inpersonal-wp.startPlaygroundWebthrows an error (e.g.,GitAuthenticationError), the catch block handles it but execution continues to run client setup code, which shouldn't happen for failed boots.Implementation details
Add
buildOAuthRedirectUrl(): Converts URL fragment blueprints toblueprint-urlquery parameters before the OAuth redirect, preserving the blueprint through the round-trip.Fix token capture timing: Move token retrieval inside the returned function so it's captured at call time, not creation time.
Add early return in catch block: Prevent client setup code from running after boot errors.
Testing Instructions (or ideally a Blueprint)