Skip to content

Commit

Permalink
fix(backend): Trigger handshake in dev when no dev browser present (#…
Browse files Browse the repository at this point in the history 
…3175)

* fix(backend): Trigger handshake when no dev browser token exists on the current request for dev

* fix(backend): Fix tests for new dev browser check

* chore(repo): Adds changeset

* chore(express): Fix tests
  • Loading branch information
@brkalow
brkalow authored Apr 12, 2024
1 parent 94519aa commit 7cb1241
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 16 deletions.
5 changes: 5 additions & 0 deletions .changeset/rare-shoes-hunt.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/backend': patch
---

Trigger the handshake when no dev browser token exists in development.
5 changes: 4 additions & 1 deletion integration/tests/handshake.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,10 @@ test.describe('Client handshake @generic', () => {
}),
redirect: 'manual',
});
expect(res.status).toBe(200);
expect(res.status).toBe(307);
expect(res.headers.get('location')).toBe(
`https://${config.pkHost}/v1/client/handshake?redirect_url=${encodeURIComponent(`${app.serverUrl}/`)}`,
);
});

test('Test redirect url - path and qs - dev', async () => {
Expand Down
53 changes: 44 additions & 9 deletions packages/backend/src/tokens/__tests__/request.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import { AuthErrorReason, type AuthReason, AuthStatus, type RequestState } from
import { authenticateRequest } from '../request';
import type { AuthenticateRequestOptions } from '../types';

const PK_TEST = 'pk_test_Y2xlcmsuaW5jbHVkZWQua2F0eWRpZC05Mi5sY2wuZGV2JA';
const PK_LIVE = 'pk_live_Y2xlcmsuaW5jbHVkZWQua2F0eWRpZC05Mi5sY2wuZGV2JA';

function assertSignedOut(
assert,
requestState: RequestState,
Expand All @@ -21,7 +24,6 @@ function assertSignedOut(
},
) {
assert.propContains(requestState, {
publishableKey: 'pk_test_Y2xlcmsuaW5jbHVkZWQua2F0eWRpZC05Mi5sY2wuZGV2JA',
proxyUrl: '',
status: AuthStatus.SignedOut,
isSignedIn: false,
Expand Down Expand Up @@ -61,7 +63,6 @@ function assertHandshake(
},
) {
assert.propContains(requestState, {
publishableKey: 'pk_test_Y2xlcmsuaW5jbHVkZWQua2F0eWRpZC05Mi5sY2wuZGV2JA',
proxyUrl: '',
status: AuthStatus.Handshake,
isSignedIn: false,
Expand Down Expand Up @@ -99,7 +100,6 @@ function assertSignedIn(
},
) {
assert.propContains(requestState, {
publishableKey: 'pk_test_Y2xlcmsuaW5jbHVkZWQua2F0eWRpZC05Mi5sY2wuZGV2JA',
proxyUrl: '',
status: AuthStatus.SignedIn,
isSignedIn: true,
Expand Down Expand Up @@ -132,7 +132,7 @@ export default (QUnit: QUnit) => {
secretKey: 'deadbeef',
apiUrl: 'https://api.clerk.test',
apiVersion: 'v1',
publishableKey: 'pk_test_Y2xlcmsuaW5jbHVkZWQua2F0eWRpZC05Mi5sY2wuZGV2JA',
publishableKey: PK_TEST,
proxyUrl: '',
skipJwksCache: true,
isSatellite: false,
Expand Down Expand Up @@ -300,6 +300,7 @@ export default (QUnit: QUnit) => {
{ __client_uat: '0' },
),
mockOptions({
publishableKey: PK_LIVE,
secretKey: 'deadbeef',
isSatellite: true,
signInUrl: 'https://primary.dev/sign-in',
Expand Down Expand Up @@ -361,6 +362,7 @@ export default (QUnit: QUnit) => {
const requestState = await authenticateRequest(
mockRequestWithCookies(),
mockOptions({
publishableKey: 'pk_live_Y2xlcmsuaW5jbHVkZWQua2F0eWRpZC05Mi5sY2wuZGV2JA',
secretKey: 'live_deadbeef',
}),
);
Expand All @@ -371,19 +373,45 @@ export default (QUnit: QUnit) => {
assertSignedOutToAuth(assert, requestState);
});

test('cookieToken: returns handshake when no clientUat in development [5y]', async assert => {
test('cookieToken: returns handshake when no dev browser in development', async assert => {
const requestState = await authenticateRequest(
mockRequestWithCookies({}, { __session: mockJwt }),
mockOptions({
secretKey: 'test_deadbeef',
}),
);

assertHandshake(assert, requestState, { reason: AuthErrorReason.DevBrowserMissing });
assert.equal(requestState.message, '');
assert.strictEqual(requestState.toAuth(), null);
});

test('cookieToken: returns handshake when no clientUat in development [5y]', async assert => {
const requestState = await authenticateRequest(
mockRequestWithCookies({}, { __clerk_db_jwt: 'deadbeef', __session: mockJwt }),
mockOptions({
secretKey: 'test_deadbeef',
}),
);

assertHandshake(assert, requestState, { reason: AuthErrorReason.SessionTokenWithoutClientUAT });
assert.equal(requestState.message, '');
assert.strictEqual(requestState.toAuth(), null);
});

test('cookieToken: returns handshake when no cookies in development [5y]', async assert => {
const requestState = await authenticateRequest(
mockRequestWithCookies({}),
mockOptions({
secretKey: 'test_deadbeef',
}),
);

assertHandshake(assert, requestState, { reason: AuthErrorReason.DevBrowserMissing });
assert.equal(requestState.message, '');
assert.strictEqual(requestState.toAuth(), null);
});

test('cookieToken: returns signedIn when satellite but valid token and clientUat', async assert => {
// Scenario: after auth action on Clerk-hosted UIs
const requestState = await authenticateRequest(
Expand All @@ -394,7 +422,7 @@ export default (QUnit: QUnit) => {
// this is not a typo, it's intentional to be `referer` to match HTTP header key
referer: 'https://clerk.com',
},
{ __client_uat: '12345', __session: mockJwt },
{ __clerk_db_jwt: 'deadbeef', __client_uat: '12345', __session: mockJwt },
),
mockOptions({
secretKey: 'pk_test_deadbeef',
Expand All @@ -415,7 +443,7 @@ export default (QUnit: QUnit) => {
test('cookieToken: returns handshake when clientUat > 0 and no cookieToken [8y]', async assert => {
const requestState = await authenticateRequest(
mockRequestWithCookies({}, { __client_uat: '12345' }),
mockOptions({ secretKey: 'deadbeef' }),
mockOptions({ secretKey: 'deadbeef', publishableKey: PK_LIVE }),
);

assertHandshake(assert, requestState, { reason: AuthErrorReason.ClientUATWithoutSessionToken });
Expand All @@ -424,7 +452,10 @@ export default (QUnit: QUnit) => {
});

test('cookieToken: returns signed out when clientUat = 0 and no cookieToken [9y]', async assert => {
const requestState = await authenticateRequest(mockRequestWithCookies({}, { __client_uat: '0' }), mockOptions());
const requestState = await authenticateRequest(
mockRequestWithCookies({}, { __client_uat: '0' }),
mockOptions({ publishableKey: PK_LIVE }),
);

assertSignedOut(assert, requestState, {
reason: AuthErrorReason.SessionTokenAndUATMissing,
Expand All @@ -437,6 +468,7 @@ export default (QUnit: QUnit) => {
mockRequestWithCookies(
{},
{
__clerk_db_jwt: 'deadbeef',
__client_uat: `${mockJwtPayload.iat + 10}`,
__session: mockJwt,
},
Expand All @@ -454,6 +486,7 @@ export default (QUnit: QUnit) => {
mockRequestWithCookies(
{},
{
__clerk_db_jwt: 'deadbeef',
__client_uat: `${mockJwtPayload.iat - 10}`,
__session: mockMalformedJwt,
},
Expand All @@ -475,6 +508,7 @@ export default (QUnit: QUnit) => {
mockRequestWithCookies(
{},
{
__clerk_db_jwt: 'deadbeef',
__client_uat: `${mockJwtPayload.iat - 10}`,
__session: mockJwt,
},
Expand All @@ -501,6 +535,7 @@ export default (QUnit: QUnit) => {
mockRequestWithCookies(
{},
{
__clerk_db_jwt: 'deadbeef',
__client_uat: `${mockJwtPayload.iat - 10}`,
__session: mockJwt,
},
Expand All @@ -522,7 +557,7 @@ export default (QUnit: QUnit) => {
},
{ __client_uat: `12345`, __session: mockJwt },
),
mockOptions({ secretKey: 'test_deadbeef' }),
mockOptions({ secretKey: 'test_deadbeef', publishableKey: PK_LIVE }),
);

assertSignedIn(assert, requestState);
Expand Down
1 change: 1 addition & 0 deletions packages/backend/src/tokens/authStatus.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export type HandshakeState = Omit<SignedOutState, 'status' | 'toAuth'> & {

export const AuthErrorReason = {
ClientUATWithoutSessionToken: 'client-uat-but-no-session-token',
DevBrowserMissing: 'dev-browser-missing',
DevBrowserSync: 'dev-browser-sync',
PrimaryRespondsToSyncing: 'primary-responds-to-syncing',
SatelliteCookieNeedsSyncing: 'satellite-needs-syncing',
Expand Down
5 changes: 5 additions & 0 deletions packages/backend/src/tokens/request.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ ${error.getFullMessage()}`,
async function authenticateRequestWithTokenInCookie() {
const hasActiveClient = authenticateContext.clientUat;
const hasSessionToken = !!authenticateContext.sessionTokenInCookie;
const hasDevBrowserToken = !!authenticateContext.devBrowserToken;

const isRequestEligibleForMultiDomainSync =
authenticateContext.isSatellite &&
Expand Down Expand Up @@ -293,6 +294,10 @@ ${error.getFullMessage()}`,
* End multi-domain sync flows
*/

if (instanceType === 'development' && !hasDevBrowserToken) {
return handleMaybeHandshakeStatus(authenticateContext, AuthErrorReason.DevBrowserMissing, '');
}

if (!hasActiveClient && !hasSessionToken) {
return signedOut(authenticateContext, AuthErrorReason.SessionTokenAndUATMissing, '');
}
Expand Down
29 changes: 23 additions & 6 deletions packages/express/src/__tests__/clerkMiddleware.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ describe('clerkMiddleware', () => {
it('works if secretKey is passed as parameter', async () => {
const options = { secretKey: 'sk_test_....' };

const response = await runMiddleware(clerkMiddleware(options)).expect(200, 'Hello world!');
const response = await runMiddleware(clerkMiddleware(options), { Cookie: '__clerk_db_jwt=deadbeef;' }).expect(
200,
'Hello world!',
);

assertSignedOutDebugHeaders(response);
});
Expand All @@ -54,7 +57,10 @@ describe('clerkMiddleware', () => {
it('works if publishableKey is passed as parameter', async () => {
const options = { publishableKey: 'pk_test_Y2xlcmsuZXhhbXBsZS5jb20k' };

const response = await runMiddleware(clerkMiddleware(options)).expect(200, 'Hello world!');
const response = await runMiddleware(clerkMiddleware(options), { Cookie: '__clerk_db_jwt=deadbeef;' }).expect(
200,
'Hello world!',
);

assertSignedOutDebugHeaders(response);
});
Expand All @@ -63,15 +69,21 @@ describe('clerkMiddleware', () => {
it.todo('supports usage without invocation: app.use(clerkMiddleware)');

it('supports usage without parameters: app.use(clerkMiddleware())', async () => {
const response = await runMiddleware(clerkMiddleware()).expect(200, 'Hello world!');
const response = await runMiddleware(clerkMiddleware(), { Cookie: '__clerk_db_jwt=deadbeef;' }).expect(
200,
'Hello world!',
);

assertSignedOutDebugHeaders(response);
});

it('supports usage with parameters: app.use(clerkMiddleware(options))', async () => {
const options = { publishableKey: 'pk_test_Y2xlcmsuZXhhbXBsZS5jb20k' };

const response = await runMiddleware(clerkMiddleware(options)).expect(200, 'Hello world!');
const response = await runMiddleware(clerkMiddleware(options), { Cookie: '__clerk_db_jwt=deadbeef;' }).expect(
200,
'Hello world!',
);

assertSignedOutDebugHeaders(response);
});
Expand All @@ -82,7 +94,10 @@ describe('clerkMiddleware', () => {
return next();
};

const response = await runMiddleware(clerkMiddleware(handler)).expect(200, 'Hello world!');
const response = await runMiddleware(clerkMiddleware(handler), { Cookie: '__clerk_db_jwt=deadbeef;' }).expect(
200,
'Hello world!',
);

expect(response.header).toHaveProperty('x-clerk-auth-custom', 'custom-value');
assertSignedOutDebugHeaders(response);
Expand All @@ -95,7 +110,9 @@ describe('clerkMiddleware', () => {
};
const options = { publishableKey: 'pk_test_Y2xlcmsuZXhhbXBsZS5jb20k' };

const response = await runMiddleware(clerkMiddleware(handler, options)).expect(200, 'Hello world!');
const response = await runMiddleware(clerkMiddleware(handler, options), {
Cookie: '__clerk_db_jwt=deadbeef;',
}).expect(200, 'Hello world!');

expect(response.header).toHaveProperty('x-clerk-auth-custom', 'custom-value');
assertSignedOutDebugHeaders(response);
Expand Down

0 comments on commit 7cb1241

Please sign in to comment.