From 349dd3b08649b727e4b750fcf4d90d09276797a4 Mon Sep 17 00:00:00 2001 From: Nikos Douvlis Date: Thu, 7 Nov 2024 21:17:06 +0200 Subject: [PATCH 1/2] fix(clerk-js,backend): Do not depend on _clerk_synced to terminate satellite sync flow --- .changeset/wild-schools-vanish.md | 6 + packages/backend/src/constants.ts | 1 + .../__tests__/authenticateContext.test.ts | 18 +-- .../backend/src/tokens/authenticateContext.ts | 146 +++++++++--------- packages/backend/src/tokens/request.ts | 23 +-- packages/clerk-js/src/core/clerk.ts | 16 +- packages/clerk-js/src/core/constants.ts | 1 + .../clerk-js/src/utils/getClerkQueryParam.ts | 3 +- 8 files changed, 112 insertions(+), 102 deletions(-) create mode 100644 .changeset/wild-schools-vanish.md diff --git a/.changeset/wild-schools-vanish.md b/.changeset/wild-schools-vanish.md new file mode 100644 index 00000000000..20f8a2e104a --- /dev/null +++ b/.changeset/wild-schools-vanish.md @@ -0,0 +1,6 @@ +--- +'@clerk/clerk-js': patch +'@clerk/backend': patch +--- + +Fixes satellite syncing when both the satellite and the primary apps use server-side enabled frameworks like NextJS diff --git a/packages/backend/src/constants.ts b/packages/backend/src/constants.ts index 766e2f62fba..0e615aed613 100644 --- a/packages/backend/src/constants.ts +++ b/packages/backend/src/constants.ts @@ -26,6 +26,7 @@ const Cookies = { const QueryParameters = { ClerkSynced: '__clerk_synced', + SuffixedCookies: 'suffixed_cookies', ClerkRedirectUrl: '__clerk_redirect_url', // use the reference to Cookies to indicate that it's the same value DevBrowser: Cookies.DevBrowser, diff --git a/packages/backend/src/tokens/__tests__/authenticateContext.test.ts b/packages/backend/src/tokens/__tests__/authenticateContext.test.ts index 48d3cb8c09f..10b4a02bf62 100644 --- a/packages/backend/src/tokens/__tests__/authenticateContext.test.ts +++ b/packages/backend/src/tokens/__tests__/authenticateContext.test.ts @@ -39,7 +39,7 @@ describe('AuthenticateContext', () => { publishableKey: pkLive, }); - expect(context.suffixedCookies).toBe(false); + expect(context.usesSuffixedCookies()).toBe(false); expect(context.sessionTokenInCookie).toBe(session); expect(context.clientUat.toString()).toBe(clientUat); }); @@ -60,7 +60,7 @@ describe('AuthenticateContext', () => { publishableKey: pkLive, }); - expect(context.suffixedCookies).toBe(false); + expect(context.usesSuffixedCookies()).toBe(false); expect(context.sessionTokenInCookie).toBe(newSession); expect(context.clientUat.toString()).toBe(clientUat); expect(context.devBrowserToken).toBe('__clerk_db_jwt'); @@ -80,7 +80,7 @@ describe('AuthenticateContext', () => { publishableKey: pkTest, }); - expect(context.suffixedCookies).toBe(false); + expect(context.usesSuffixedCookies()).toBe(false); expect(context.sessionTokenInCookie).toBe(session); expect(context.clientUat.toString()).toBe(clientUat); }); @@ -99,7 +99,7 @@ describe('AuthenticateContext', () => { publishableKey: pkLive, }); - expect(context.suffixedCookies).toBe(true); + expect(context.usesSuffixedCookies()).toBe(true); expect(context.sessionTokenInCookie).toBe(suffixedSession); expect(context.clientUat.toString()).toBe('0'); }); @@ -119,7 +119,7 @@ describe('AuthenticateContext', () => { const context = await createAuthenticateContext(clerkRequest, { publishableKey: pkLive, }); - expect(context.suffixedCookies).toBe(true); + expect(context.usesSuffixedCookies()).toBe(true); expect(context.sessionTokenInCookie).toBe(suffixedSession); expect(context.clientUat.toString()).toBe(suffixedClientUat); }); @@ -137,7 +137,7 @@ describe('AuthenticateContext', () => { const context = await createAuthenticateContext(clerkRequest, { publishableKey: pkLive, }); - expect(context.suffixedCookies).toBe(true); + expect(context.usesSuffixedCookies()).toBe(true); expect(context.sessionTokenInCookie).toBe(suffixedSession); expect(context.clientUat.toString()).toBe(suffixedClientUat); }); @@ -166,7 +166,7 @@ describe('AuthenticateContext', () => { publishableKey: pkTest, }); - expect(context.suffixedCookies).toBe(true); + expect(context.usesSuffixedCookies()).toBe(true); expect(context.sessionTokenInCookie).toBe(suffixedSession); expect(context.clientUat.toString()).toBe('0'); expect(context.devBrowserToken).toBe('__clerk_db_jwt-suffixed'); @@ -188,7 +188,7 @@ describe('AuthenticateContext', () => { publishableKey: pkTest, }); - expect(context.suffixedCookies).toBe(true); + expect(context.usesSuffixedCookies()).toBe(true); expect(context.sessionTokenInCookie).toBe(suffixedSession); expect(context.clientUat.toString()).toBe('0'); expect(context.devBrowserToken).toBe('__clerk_db_jwt-suffixed'); @@ -207,7 +207,7 @@ describe('AuthenticateContext', () => { publishableKey: pkLive, }); - expect(context.suffixedCookies).toBe(true); + expect(context.usesSuffixedCookies()).toBe(true); expect(context.sessionTokenInCookie).toBeUndefined(); expect(context.clientUat.toString()).toBe('0'); }); diff --git a/packages/backend/src/tokens/authenticateContext.ts b/packages/backend/src/tokens/authenticateContext.ts index 1d534be5950..7ec25670b58 100644 --- a/packages/backend/src/tokens/authenticateContext.ts +++ b/packages/backend/src/tokens/authenticateContext.ts @@ -23,7 +23,6 @@ interface AuthenticateContextInterface extends AuthenticateRequestOptions { sessionTokenInCookie: string | undefined; refreshTokenInCookie: string | undefined; clientUat: number; - suffixedCookies: boolean; // handshake-related values devBrowserToken: string | undefined; handshakeToken: string | undefined; @@ -68,78 +67,7 @@ class AuthenticateContext { this.clerkUrl = this.clerkRequest.clerkUrl; } - private initPublishableKeyValues(options: AuthenticateRequestOptions) { - assertValidPublishableKey(options.publishableKey); - this.publishableKey = options.publishableKey; - - const pk = parsePublishableKey(this.publishableKey, { - fatal: true, - proxyUrl: options.proxyUrl, - domain: options.domain, - }); - this.instanceType = pk.instanceType; - this.frontendApi = pk.frontendApi; - } - - private initHeaderValues() { - this.sessionTokenInHeader = this.stripAuthorizationHeader(this.getHeader(constants.Headers.Authorization)); - this.origin = this.getHeader(constants.Headers.Origin); - this.host = this.getHeader(constants.Headers.Host); - this.forwardedHost = this.getHeader(constants.Headers.ForwardedHost); - this.forwardedProto = - this.getHeader(constants.Headers.CloudFrontForwardedProto) || this.getHeader(constants.Headers.ForwardedProto); - this.referrer = this.getHeader(constants.Headers.Referrer); - this.userAgent = this.getHeader(constants.Headers.UserAgent); - this.secFetchDest = this.getHeader(constants.Headers.SecFetchDest); - this.accept = this.getHeader(constants.Headers.Accept); - } - - private initCookieValues() { - // suffixedCookies needs to be set first because it's used in getMultipleAppsCookie - this.suffixedCookies = this.shouldUseSuffixed(); - this.sessionTokenInCookie = this.getSuffixedOrUnSuffixedCookie(constants.Cookies.Session); - this.refreshTokenInCookie = this.getSuffixedCookie(constants.Cookies.Refresh); - this.clientUat = Number.parseInt(this.getSuffixedOrUnSuffixedCookie(constants.Cookies.ClientUat) || '') || 0; - } - - private initHandshakeValues() { - this.devBrowserToken = - this.getQueryParam(constants.QueryParameters.DevBrowser) || - this.getSuffixedOrUnSuffixedCookie(constants.Cookies.DevBrowser); - // Using getCookie since we don't suffix the handshake token cookie - this.handshakeToken = - this.getQueryParam(constants.QueryParameters.Handshake) || this.getCookie(constants.Cookies.Handshake); - this.handshakeRedirectLoopCounter = Number(this.getCookie(constants.Cookies.RedirectCount)) || 0; - } - - private stripAuthorizationHeader(authValue: string | undefined | null): string | undefined { - return authValue?.replace('Bearer ', ''); - } - - private getQueryParam(name: string) { - return this.clerkRequest.clerkUrl.searchParams.get(name); - } - - private getHeader(name: string) { - return this.clerkRequest.headers.get(name) || undefined; - } - - private getCookie(name: string) { - return this.clerkRequest.cookies.get(name) || undefined; - } - - private getSuffixedCookie(name: string) { - return this.getCookie(getSuffixedCookieName(name, this.cookieSuffix)) || undefined; - } - - private getSuffixedOrUnSuffixedCookie(cookieName: string) { - if (this.suffixedCookies) { - return this.getSuffixedCookie(cookieName); - } - return this.getCookie(cookieName); - } - - private shouldUseSuffixed(): boolean { + public usesSuffixedCookies(): boolean { const suffixedClientUat = this.getSuffixedCookie(constants.Cookies.ClientUat); const clientUat = this.getCookie(constants.Cookies.ClientUat); const suffixedSession = this.getSuffixedCookie(constants.Cookies.Session) || ''; @@ -158,7 +86,7 @@ class AuthenticateContext { return true; } - // If there is no suffixed cookies use un-suffixed + // If there are no suffixed cookies use un-suffixed if (!suffixedClientUat && !suffixedSession) { return false; } @@ -228,6 +156,76 @@ class AuthenticateContext { return true; } + private initPublishableKeyValues(options: AuthenticateRequestOptions) { + assertValidPublishableKey(options.publishableKey); + this.publishableKey = options.publishableKey; + + const pk = parsePublishableKey(this.publishableKey, { + fatal: true, + proxyUrl: options.proxyUrl, + domain: options.domain, + }); + this.instanceType = pk.instanceType; + this.frontendApi = pk.frontendApi; + } + + private initHeaderValues() { + this.sessionTokenInHeader = this.stripAuthorizationHeader(this.getHeader(constants.Headers.Authorization)); + this.origin = this.getHeader(constants.Headers.Origin); + this.host = this.getHeader(constants.Headers.Host); + this.forwardedHost = this.getHeader(constants.Headers.ForwardedHost); + this.forwardedProto = + this.getHeader(constants.Headers.CloudFrontForwardedProto) || this.getHeader(constants.Headers.ForwardedProto); + this.referrer = this.getHeader(constants.Headers.Referrer); + this.userAgent = this.getHeader(constants.Headers.UserAgent); + this.secFetchDest = this.getHeader(constants.Headers.SecFetchDest); + this.accept = this.getHeader(constants.Headers.Accept); + } + + private initCookieValues() { + // suffixedCookies needs to be set first because it's used in getMultipleAppsCookie + this.sessionTokenInCookie = this.getSuffixedOrUnSuffixedCookie(constants.Cookies.Session); + this.refreshTokenInCookie = this.getSuffixedCookie(constants.Cookies.Refresh); + this.clientUat = Number.parseInt(this.getSuffixedOrUnSuffixedCookie(constants.Cookies.ClientUat) || '') || 0; + } + + private initHandshakeValues() { + this.devBrowserToken = + this.getQueryParam(constants.QueryParameters.DevBrowser) || + this.getSuffixedOrUnSuffixedCookie(constants.Cookies.DevBrowser); + // Using getCookie since we don't suffix the handshake token cookie + this.handshakeToken = + this.getQueryParam(constants.QueryParameters.Handshake) || this.getCookie(constants.Cookies.Handshake); + this.handshakeRedirectLoopCounter = Number(this.getCookie(constants.Cookies.RedirectCount)) || 0; + } + + private stripAuthorizationHeader(authValue: string | undefined | null): string | undefined { + return authValue?.replace('Bearer ', ''); + } + + private getQueryParam(name: string) { + return this.clerkRequest.clerkUrl.searchParams.get(name); + } + + private getHeader(name: string) { + return this.clerkRequest.headers.get(name) || undefined; + } + + private getCookie(name: string) { + return this.clerkRequest.cookies.get(name) || undefined; + } + + private getSuffixedCookie(name: string) { + return this.getCookie(getSuffixedCookieName(name, this.cookieSuffix)) || undefined; + } + + private getSuffixedOrUnSuffixedCookie(cookieName: string) { + if (this.usesSuffixedCookies()) { + return this.getSuffixedCookie(cookieName); + } + return this.getCookie(cookieName); + } + private tokenHasIssuer(token: string): boolean { const { data, errors } = decodeJwt(token); if (errors) { diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index dec5afdbc5b..1de8224f2b3 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -124,7 +124,10 @@ export async function authenticateRequest( const url = new URL(`https://${frontendApiNoProtocol}/v1/client/handshake`); url.searchParams.append('redirect_url', redirectUrl?.href || ''); - url.searchParams.append('suffixed_cookies', authenticateContext.suffixedCookies.toString()); + url.searchParams.append( + constants.QueryParameters.SuffixedCookies, + authenticateContext.usesSuffixedCookies().toString(), + ); url.searchParams.append(constants.QueryParameters.HandshakeReason, handshakeReason); if (authenticateContext.instanceType === 'development' && authenticateContext.devBrowserToken) { @@ -472,11 +475,6 @@ ${error.getFullMessage()}`, const hasSessionToken = !!authenticateContext.sessionTokenInCookie; const hasDevBrowserToken = !!authenticateContext.devBrowserToken; - const isRequestEligibleForMultiDomainSync = - authenticateContext.isSatellite && - authenticateContext.secFetchDest === 'document' && - !authenticateContext.clerkUrl.searchParams.has(constants.QueryParameters.ClerkSynced); - /** * If we have a handshakeToken, resolve the handshake and attempt to return a definitive signed in or signed out state. */ @@ -512,6 +510,9 @@ ${error.getFullMessage()}`, return handleMaybeHandshakeStatus(authenticateContext, AuthErrorReason.DevBrowserSync, ''); } + const isRequestEligibleForMultiDomainSync = + authenticateContext.isSatellite && authenticateContext.secFetchDest === 'document'; + /** * Begin multi-domain sync flows */ @@ -529,17 +530,19 @@ ${error.getFullMessage()}`, constants.QueryParameters.ClerkRedirectUrl, authenticateContext.clerkUrl.toString(), ); - const authErrReason = AuthErrorReason.SatelliteCookieNeedsSyncing; - redirectURL.searchParams.append(constants.QueryParameters.HandshakeReason, authErrReason); - + redirectURL.searchParams.append( + constants.QueryParameters.HandshakeReason, + AuthErrorReason.SatelliteCookieNeedsSyncing, + ); const headers = new Headers({ [constants.Headers.Location]: redirectURL.toString() }); - return handleMaybeHandshakeStatus(authenticateContext, authErrReason, '', headers); + return handleMaybeHandshakeStatus(authenticateContext, AuthErrorReason.SatelliteCookieNeedsSyncing, '', headers); } // Multi-domain development sync flow const redirectUrl = new URL(authenticateContext.clerkUrl).searchParams.get( constants.QueryParameters.ClerkRedirectUrl, ); + if (authenticateContext.instanceType === 'development' && !authenticateContext.isSatellite && redirectUrl) { // Dev MD sync from primary, redirect back to satellite w/ dev browser query param const redirectBackToSatelliteUrl = new URL(redirectUrl); diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index d2901141cb7..e1bd8617aff 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -99,7 +99,7 @@ import { assertNoLegacyProp } from '../utils/assertNoLegacyProp'; import { memoizeListenerCallback } from '../utils/memoizeStateListenerCallback'; import { RedirectUrls } from '../utils/redirectUrls'; import { AuthCookieService } from './auth/AuthCookieService'; -import { CLERK_SATELLITE_URL, CLERK_SYNCED, ERROR_CODES } from './constants'; +import { CLERK_SATELLITE_URL, CLERK_SUFFIXED_COOKIES, CLERK_SYNCED, ERROR_CODES } from './constants'; import { clerkErrorInitFailed, clerkInvalidSignInUrlFormat, @@ -1635,9 +1635,6 @@ export class Clerk implements ClerkInterface { return this.navigate(to); } - #hasJustSynced = () => getClerkQueryParam(CLERK_SYNCED) === 'true'; - #clearJustSynced = () => removeClerkQueryParam(CLERK_SYNCED); - #buildSyncUrlForDevelopmentInstances = (): string => { const searchParams = new URLSearchParams({ [CLERK_SATELLITE_URL]: window.location.href, @@ -1661,8 +1658,7 @@ export class Clerk implements ClerkInterface { }; #shouldSyncWithPrimary = (): boolean => { - if (this.#hasJustSynced()) { - this.#clearJustSynced(); + if (getClerkQueryParam(CLERK_SYNCED) === 'true') { return false; } @@ -1818,7 +1814,7 @@ export class Clerk implements ClerkInterface { } } - this.#clearHandshakeFromUrl(); + this.#clearClerkQueryParams(); this.#handleImpersonationFab(); return true; @@ -2010,8 +2006,12 @@ export class Clerk implements ClerkInterface { * The handshake payload is transported in the URL in development. In cases where FAPI is returning the handshake payload, but Clerk is being used in a client-only application, * we remove the handshake associated parameters as they are not necessary. */ - #clearHandshakeFromUrl = () => { + #clearClerkQueryParams = () => { try { + removeClerkQueryParam(CLERK_SYNCED); + // @nikos: we're looking into dropping this param completely + // in the meantime, we're removing it here to keep the URL clean + removeClerkQueryParam(CLERK_SUFFIXED_COOKIES); removeClerkQueryParam('__clerk_handshake'); removeClerkQueryParam('__clerk_help'); } catch (_) { diff --git a/packages/clerk-js/src/core/constants.ts b/packages/clerk-js/src/core/constants.ts index 28c6cfd2034..d8c0b052593 100644 --- a/packages/clerk-js/src/core/constants.ts +++ b/packages/clerk-js/src/core/constants.ts @@ -13,6 +13,7 @@ export const PRESERVED_QUERYSTRING_PARAMS = [ export const CLERK_MODAL_STATE = '__clerk_modal_state'; export const CLERK_SYNCED = '__clerk_synced'; +export const CLERK_SUFFIXED_COOKIES = 'suffixed_cookies'; export const CLERK_SATELLITE_URL = '__clerk_satellite_url'; export const ERROR_CODES = { FORM_IDENTIFIER_NOT_FOUND: 'form_identifier_not_found', diff --git a/packages/clerk-js/src/utils/getClerkQueryParam.ts b/packages/clerk-js/src/utils/getClerkQueryParam.ts index d2973fb6d35..3248d720ad5 100644 --- a/packages/clerk-js/src/utils/getClerkQueryParam.ts +++ b/packages/clerk-js/src/utils/getClerkQueryParam.ts @@ -1,4 +1,4 @@ -import { CLERK_SATELLITE_URL, CLERK_SYNCED } from '../core/constants'; +import { CLERK_SATELLITE_URL, CLERK_SUFFIXED_COOKIES, CLERK_SYNCED } from '../core/constants'; const ClerkQueryParams = [ '__clerk_status', @@ -10,6 +10,7 @@ const ClerkQueryParams = [ '__clerk_help', CLERK_SYNCED, CLERK_SATELLITE_URL, + CLERK_SUFFIXED_COOKIES, ] as const; type ClerkQueryParam = (typeof ClerkQueryParams)[number]; From 92a8a1fafbb65a48ebcfc523ce9901aa0a6d2613 Mon Sep 17 00:00:00 2001 From: Nikos Douvlis Date: Tue, 12 Nov 2024 12:51:47 +0200 Subject: [PATCH 2/2] Fix type --- packages/clerk-js/src/utils/getClerkQueryParam.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/packages/clerk-js/src/utils/getClerkQueryParam.ts b/packages/clerk-js/src/utils/getClerkQueryParam.ts index 3248d720ad5..1126de9c7cb 100644 --- a/packages/clerk-js/src/utils/getClerkQueryParam.ts +++ b/packages/clerk-js/src/utils/getClerkQueryParam.ts @@ -17,15 +17,7 @@ type ClerkQueryParam = (typeof ClerkQueryParams)[number]; type ClerkQueryParamsToValuesMap = { __clerk_status: VerificationStatus; - __clerk_created_session: string; - __clerk_invitation_token: string; - __clerk_ticket: string; - __clerk_modal_state: string; - __clerk_synced: string; - __clerk_satellite_url: string; - __clerk_handshake: string; - __clerk_help: string; -}; +} & Record<(typeof ClerkQueryParams)[number], string>; export type VerificationStatus = | 'expired'