MARTECH-32 Implement GitHub authentication endpoint #101672

robertsreberski · 2025-03-20T14:24:14Z

Related to 177769-ghe-Automattic/wpcom

Proposed Changes

Add auth endpoint and callback handling for direct GitHub authentication with redirect_to param

Why are these changes being made?

We're building new onboarding flow for Jetpack that will simplify the process for new users. The user will now choose their authentication method directly in Jetpack plugin thus we needed to make some adjustments on Calypso end.

Testing Instructions

You need this change (177607-ghe-Automattic/wpcom) on your sandbox and point wordpress.com to your sandbox
You also need this 177769-ghe-Automattic/wpcom
Then it's quite easy
Be logged out and go to calypso.localhost:3000/log-in/jetpack/github?redirect_to=/success
You should be able to go through github authentication and land on calypso.localhost:3000/success
Otherwise, you should land on calypso.localhost:3000/log-in/jetpack?redirect_to=/success

github-actions · 2025-03-20T14:24:26Z

Calypso Live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-138365

Jetpack Cloud live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-138365&env=jetpack

Automattic for Agencies live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-138365&env=a8c-for-agencies

matticbot · 2025-03-20T14:32:46Z

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

notifications
wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug add/jetpack-auth-github-direct on your sandbox.

matticbot · 2025-03-20T14:35:41Z

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~324 bytes added 📈 [gzipped])

name         parsed_size           gzip_size
entry-login      +2036 B  (+0.1%)     +324 B  (+0.1%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

IanRamosC

The code looks good. I just left some minor comments that are not blockers.

IanRamosC · 2025-03-21T13:27:28Z

client/login/controller.js

+			ux_mode: 'redirect',
+			redirect_to: query?.redirect_to || '/',
+		} );
+		window.location.href = `https://public-api.wordpress.com/wpcom/v2/hosting/github/app-authorize?${ params.toString() }`;


Should we add a return after this redirect so the context.primary code doesn't run if we reached this point and the redirect happens?

IanRamosC · 2025-03-21T13:29:31Z

client/login/controller.js

+					tos: JSON.stringify( getToSAcceptancePayload() ),
+				} );
+			} catch {
+				// Silently fail; when id_token is not present, the endpoint fails when the user already exists


Is this something we'd like to log to logstash if it fails? Or perhaps check the error code if there's any, so we know we are failing silently only when user already exists

IanRamosC · 2025-03-21T13:38:01Z

client/login/controller.js

+	try {
+		// GitHub supports localhost auth; and we allowlist the jetpack callback path
+		const redirectUri = `${ window.location.origin }/log-in/jetpack/github/callback`;
+
+		// Exchange auth code for tokens
+		const response = await postLoginRequest( 'exchange-social-auth-code', {
+			service: 'github',
+			auth_code: code,
+			redirect_uri: redirectUri,
+			client_id: config( 'wpcom_signup_id' ),
+			client_secret: config( 'wpcom_signup_key' ),
+		} );
+
+		const { access_token } = response.body.data;
+
+		// Try to create a new WordPress.com account (if it doesn't exist) - then, log in the user
+		try {
+			try {


These nested try...catch blocks might make it harder to read the code, I wonder whether letting the error propagate, or creating helper functions would help to achieve a similar behavior while improving readability

IanRamosC · 2025-03-21T13:39:17Z

client/login/controller.js

+			await context.store.dispatch( rebootAfterLogin() );
+			return;
+		} catch {
+			// If both connection and creation fail, show warning and redirect back to login page


Thanks for the comments, they help understand what each catch is for

IanRamosC · 2025-03-21T14:08:29Z

client/login/controller.js

+					redirect_to
+				)
+			);
+			const url = new URL( redirect_to );


Here, is redirect_to an internal URL? Does it make set the second parameter (the base of the URL) if it's an internal URL?

This reverts commit bec8c74.

* Revert "Properly handle redirect_to in fallback login page (#102117)" This reverts commit 24f8f5e. * Revert "Jetpack Onboarding: various auth improvements (#102106)" This reverts commit da747fd. * Revert "Improve error handling in auth flows (#102027)" This reverts commit fd52d8c. * Revert "My Jetpack: redirect logged in users to the connection page (#102644)" This reverts commit 6a41e3e. * Revert "Improve log in handling for Google (#102018)" This reverts commit 30b8af0. * Revert "Add back logging out (#102015)" This reverts commit 54afb7e. * Revert "Fix Apple and GitHub redirects (#102011)" This reverts commit f39b9e4. * Revert "Next bunch on auth improvements (#101996)" This reverts commit 3de680b. * Revert "Various improvements of direct Jetpack auth (#101988)" This reverts commit 7a3558a. * Revert "Improve error redirects (#101865)" This reverts commit e2c6064. * Revert "MARTECH-42 Various Jetpack direct auth improvements and fixes (#101859)" This reverts commit d49fc76. * Revert "Add direct GitHub auth and callback server-side (#101672)" This reverts commit bec8c74. * Revert "Do not run Jetpack social auth endpoints on serverside (#101832)" This reverts commit 2d915a9. * Revert "Remove oauth env checks in social auth Jetpack (#101802)" This reverts commit 0639270. * Revert "Revert the improvement to the original code (#101801)" This reverts commit d959108. * Revert "Fix nonce handling in auth request (#101800)" This reverts commit 013bd4e. * Revert "JPMT-31 Apple authentication endpoint for Jetpack (#101658)" This reverts commit 0c55e09. * Revert "JPMT-30 Add direct Google auth endpoint for Jetpack (#101588)" This reverts commit 93dbf58. * Revert "Support auto trigger of magiclink auth (#101601)" This reverts commit 2984e31. * Revert "JPMT-29 Improve Google Auth handling based on testing (#101508)" This reverts commit 7011770. * Revert "JPMT-29 Add direct authentication endpoints for social providers (#101427)" This reverts commit b45e821.

IanRamosC approved these changes Mar 21, 2025

View reviewed changes

robertsreberski force-pushed the add/jetpack-auth-apple-direct branch from 6680194 to 26f9b16 Compare March 22, 2025 16:39

IanRamosC mentioned this pull request Mar 24, 2025

JPMT-30 Add direct Google auth endpoint for Jetpack #101588

Merged

robertsreberski changed the base branch from add/jetpack-auth-apple-direct to trunk March 24, 2025 20:24

Add direct GitHub auth and callback server-side

a66165e

robertsreberski force-pushed the add/jetpack-auth-github-direct branch from 47ef171 to a66165e Compare March 25, 2025 16:53

robertsreberski changed the title ~~JPMT-32 Implement GitHub authentication endpoint~~ MARTECH-32 Implement GitHub authentication endpoint Mar 25, 2025

robertsreberski merged commit bec8c74 into trunk Mar 25, 2025
14 checks passed

robertsreberski deleted the add/jetpack-auth-github-direct branch March 25, 2025 17:58

robertsreberski mentioned this pull request Mar 25, 2025

MARTECH-42 Various Jetpack direct auth improvements and fixes #101859

Merged

manzoorwanijk added a commit that referenced this pull request Apr 25, 2025

Revert "Add direct GitHub auth and callback server-side (#101672)"

8c3962d

This reverts commit bec8c74.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

MARTECH-32 Implement GitHub authentication endpoint #101672

MARTECH-32 Implement GitHub authentication endpoint #101672

Uh oh!

robertsreberski commented Mar 20, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Mar 20, 2025 •

edited by matticbot

Loading

Uh oh!

matticbot commented Mar 20, 2025 •

edited

Loading

Uh oh!

matticbot commented Mar 20, 2025 •

edited

Loading

Uh oh!

IanRamosC left a comment

Uh oh!

IanRamosC Mar 21, 2025

Uh oh!

IanRamosC Mar 21, 2025

Uh oh!

IanRamosC Mar 21, 2025

Uh oh!

IanRamosC Mar 21, 2025

Uh oh!

IanRamosC Mar 21, 2025

Uh oh!

Uh oh!

Uh oh!

MARTECH-32 Implement GitHub authentication endpoint #101672

MARTECH-32 Implement GitHub authentication endpoint #101672

Uh oh!

Conversation

robertsreberski commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Why are these changes being made?

Testing Instructions

Uh oh!

github-actions bot commented Mar 20, 2025 • edited by matticbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matticbot commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matticbot commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IanRamosC left a comment

Choose a reason for hiding this comment

Uh oh!

IanRamosC Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

IanRamosC Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

IanRamosC Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

IanRamosC Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

IanRamosC Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

robertsreberski commented Mar 20, 2025 •

edited

Loading

github-actions bot commented Mar 20, 2025 •

edited by matticbot

Loading

matticbot commented Mar 20, 2025 •

edited

Loading

matticbot commented Mar 20, 2025 •

edited

Loading