Skip to content

Commit

Permalink
fix(clerk-js): Extend client cookie lifetime (#4312)
Browse files Browse the repository at this point in the history
  • Loading branch information
issuedat authored Oct 29, 2024
1 parent 17c5990 commit f875463
Show file tree
Hide file tree
Showing 27 changed files with 260 additions and 88 deletions.
9 changes: 9 additions & 0 deletions .changeset/eighty-guests-change.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@clerk/clerk-js": minor
"@clerk/types": minor
"@clerk/elements": patch
"@clerk/clerk-react": patch
---

- Introduce `redirectUrl` property on `setActive` as a replacement for `beforeEmit`.
- Deprecates `beforeEmit` property on `setActive`.
125 changes: 105 additions & 20 deletions packages/clerk-js/src/core/__tests__/clerk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,65 @@ describe('Clerk singleton', () => {
});
});

it('redirects the user to the /v1/client/touch endpoint if the cookie_expires_at is less than 8 days away', async () => {
mockSession.touch.mockReturnValue(Promise.resolve());
mockClientFetch.mockReturnValue(
Promise.resolve({
activeSessions: [mockSession],
cookieExpiresAt: new Date(Date.now() + 2 * 24 * 60 * 60 * 1000), // 2 days from now
isEligibleForTouch: () => true,
buildTouchUrl: () =>
`https://clerk.example.com/v1/client/touch?redirect_url=${mockWindowLocation.href}/redirect-url-path`,
}),
);

const sut = new Clerk(productionPublishableKey);
sut.navigate = jest.fn();
await sut.load();
await sut.setActive({ session: mockSession as any as ActiveSessionResource, redirectUrl: '/redirect-url-path' });
const redirectUrl = new URL((sut.navigate as jest.Mock).mock.calls[0]);
expect(redirectUrl.pathname).toEqual('/v1/client/touch');
expect(redirectUrl.searchParams.get('redirect_url')).toEqual(`${mockWindowLocation.href}/redirect-url-path`);
});

it('does not redirect the user to the /v1/client/touch endpoint if the cookie_expires_at is more than 8 days away', async () => {
mockSession.touch.mockReturnValue(Promise.resolve());
mockClientFetch.mockReturnValue(
Promise.resolve({
activeSessions: [mockSession],
cookieExpiresAt: new Date(Date.now() + 10 * 24 * 60 * 60 * 1000), // 10 days from now
isEligibleForTouch: () => false,
buildTouchUrl: () =>
`https://clerk.example.com/v1/client/touch?redirect_url=${mockWindowLocation.href}/redirect-url-path`,
}),
);

const sut = new Clerk(productionPublishableKey);
sut.navigate = jest.fn();
await sut.load();
await sut.setActive({ session: mockSession as any as ActiveSessionResource, redirectUrl: '/redirect-url-path' });
expect(sut.navigate).toHaveBeenCalledWith('/redirect-url-path');
});

it('does not redirect the user to the /v1/client/touch endpoint if the cookie_expires_at is not set', async () => {
mockSession.touch.mockReturnValue(Promise.resolve());
mockClientFetch.mockReturnValue(
Promise.resolve({
activeSessions: [mockSession],
cookieExpiresAt: null,
isEligibleForTouch: () => false,
buildTouchUrl: () =>
`https://clerk.example.com/v1/client/touch?redirect_url=${mockWindowLocation.href}/redirect-url-path`,
}),
);

const sut = new Clerk(productionPublishableKey);
sut.navigate = jest.fn();
await sut.load();
await sut.setActive({ session: mockSession as any as ActiveSessionResource, redirectUrl: '/redirect-url-path' });
expect(sut.navigate).toHaveBeenCalledWith('/redirect-url-path');
});

mockNativeRuntime(() => {
it('calls session.touch in a non-standard browser', async () => {
mockClientFetch.mockReturnValue(Promise.resolve({ activeSessions: [mockSession] }));
Expand Down Expand Up @@ -496,6 +555,7 @@ describe('Clerk singleton', () => {
expect(sut.setActive).toHaveBeenCalledWith({
session: null,
beforeEmit: expect.any(Function),
redirectUrl: '/',
});
});
});
Expand All @@ -518,7 +578,11 @@ describe('Clerk singleton', () => {
expect(mockClientDestroy).not.toHaveBeenCalled();
expect(mockClientRemoveSessions).toHaveBeenCalled();
expect(mockSession1.remove).not.toHaveBeenCalled();
expect(sut.setActive).toHaveBeenCalledWith({ session: null, beforeEmit: expect.any(Function) });
expect(sut.setActive).toHaveBeenCalledWith({
session: null,
beforeEmit: expect.any(Function),
redirectUrl: '/',
});
});
});

Expand Down Expand Up @@ -561,7 +625,11 @@ describe('Clerk singleton', () => {
await waitFor(() => {
expect(mockSession1.remove).toHaveBeenCalled();
expect(mockClientDestroy).not.toHaveBeenCalled();
expect(sut.setActive).toHaveBeenCalledWith({ session: null, beforeEmit: expect.any(Function) });
expect(sut.setActive).toHaveBeenCalledWith({
session: null,
beforeEmit: expect.any(Function),
redirectUrl: '/',
});
});
});

Expand All @@ -582,7 +650,11 @@ describe('Clerk singleton', () => {
await waitFor(() => {
expect(mockSession1.remove).toHaveBeenCalled();
expect(mockClientDestroy).not.toHaveBeenCalled();
expect(sut.setActive).toHaveBeenCalledWith({ session: null, beforeEmit: expect.any(Function) });
expect(sut.setActive).toHaveBeenCalledWith({
session: null,
beforeEmit: expect.any(Function),
redirectUrl: '/after-sign-out',
});
expect(sut.navigate).toHaveBeenCalledWith('/after-sign-out');
});
});
Expand Down Expand Up @@ -1096,6 +1168,16 @@ describe('Clerk singleton', () => {
});

it('redirects the user to the signInForceRedirectUrl if one was provided', async () => {
const sessionId = 'sess_1yDceUR8SIKtQ0gIOO8fNsW7nhe';
const mockSession = {
id: sessionId,
remove: jest.fn(),
status: 'active',
user: {},
touch: jest.fn(() => Promise.resolve()),
getToken: jest.fn(),
lastActiveToken: { getRawString: () => 'mocked-token' },
};
mockEnvironmentFetch.mockReturnValue(
Promise.resolve({
authConfig: {},
Expand All @@ -1110,6 +1192,7 @@ describe('Clerk singleton', () => {

mockClientFetch.mockReturnValue(
Promise.resolve({
sessions: [mockSession],
activeSessions: [],
signIn: new SignIn(null),
signUp: new SignUp({
Expand All @@ -1124,24 +1207,19 @@ describe('Clerk singleton', () => {
long_message: "You're already signed in",
message: "You're already signed in",
meta: {
session_id: 'sess_1yDceUR8SIKtQ0gIOO8fNsW7nhe',
session_id: sessionId,
},
},
},
},
} as any as SignUpJSON),
isEligibleForTouch: () => false,
}),
);

const mockSetActive = jest.fn(async (setActiveOpts: any) => {
await setActiveOpts.beforeEmit();
});

const sut = new Clerk(productionPublishableKey);
await sut.load(mockedLoadOptions);
sut.setActive = mockSetActive as any;

sut.handleRedirectCallback({
await sut.handleRedirectCallback({
signInForceRedirectUrl: '/custom-sign-in',
});

Expand All @@ -1151,6 +1229,16 @@ describe('Clerk singleton', () => {
});

it('gives priority to signInForceRedirectUrl if signInForceRedirectUrl and signInFallbackRedirectUrl were provided ', async () => {
const sessionId = 'sess_1yDceUR8SIKtQ0gIOO8fNsW7nhe';
const mockSession = {
id: sessionId,
remove: jest.fn(),
status: 'active',
user: {},
touch: jest.fn(() => Promise.resolve()),
getToken: jest.fn(),
lastActiveToken: { getRawString: () => 'mocked-token' },
};
mockEnvironmentFetch.mockReturnValue(
Promise.resolve({
authConfig: {},
Expand All @@ -1165,6 +1253,7 @@ describe('Clerk singleton', () => {

mockClientFetch.mockReturnValue(
Promise.resolve({
sessions: [mockSession],
activeSessions: [],
signIn: new SignIn(null),
signUp: new SignUp({
Expand All @@ -1179,24 +1268,20 @@ describe('Clerk singleton', () => {
long_message: "You're already signed in",
message: "You're already signed in",
meta: {
session_id: 'sess_1yDceUR8SIKtQ0gIOO8fNsW7nhe',
session_id: sessionId,
},
},
},
},
} as any as SignUpJSON),
isEligibleForTouch: () => false,
}),
);

const mockSetActive = jest.fn(async (setActiveOpts: any) => {
await setActiveOpts.beforeEmit();
});

const sut = new Clerk(productionPublishableKey);
await sut.load(mockedLoadOptions);
sut.setActive = mockSetActive as any;

sut.handleRedirectCallback({
await sut.handleRedirectCallback({
signInForceRedirectUrl: '/custom-sign-in',
signInFallbackRedirectUrl: '/redirect-to',
} as any);
Expand Down Expand Up @@ -1654,7 +1739,7 @@ describe('Clerk singleton', () => {
await waitFor(() => {
expect(mockSetActive).toHaveBeenCalledWith({
session: createdSessionId,
beforeEmit: expect.any(Function),
redirectUrl: redirectUrlComplete,
});
});
});
Expand Down Expand Up @@ -1714,7 +1799,7 @@ describe('Clerk singleton', () => {
await waitFor(() => {
expect(mockSetActive).toHaveBeenCalledWith({
session: createdSessionId,
beforeEmit: expect.any(Function),
redirectUrl: redirectUrlComplete,
});
});
});
Expand Down
43 changes: 27 additions & 16 deletions packages/clerk-js/src/core/clerk.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
addClerkPrefix,
ClerkRuntimeError,
deprecated,
handleValueOrFn,
inBrowser as inClientSide,
is4xxError,
Expand Down Expand Up @@ -354,6 +355,7 @@ export class Clerk implements ClerkInterface {
return this.setActive({
session: null,
beforeEmit: ignoreEventValue(cb),
redirectUrl,
});
}

Expand All @@ -364,6 +366,7 @@ export class Clerk implements ClerkInterface {
return this.setActive({
session: null,
beforeEmit: ignoreEventValue(cb),
redirectUrl,
});
}
};
Expand Down Expand Up @@ -746,7 +749,7 @@ export class Clerk implements ClerkInterface {
/**
* `setActive` can be used to set the active session and/or organization.
*/
public setActive = async ({ session, organization, beforeEmit }: SetActiveParams): Promise<void> => {
public setActive = async ({ session, organization, beforeEmit, redirectUrl }: SetActiveParams): Promise<void> => {
if (!this.client) {
throw new Error('setActive is being called before the client is loaded. Wait for init.');
}
Expand Down Expand Up @@ -831,10 +834,26 @@ export class Clerk implements ClerkInterface {
if (beforeEmit) {
beforeUnloadTracker?.startTracking();
this.#setTransitiveState();
deprecated('beforeEmit', 'Use the `redirectUrl` property instead.');
await beforeEmit(newSession);
beforeUnloadTracker?.stopTracking();
}

if (redirectUrl) {
beforeUnloadTracker?.startTracking();
this.#setTransitiveState();

if (this.client.isEligibleForTouch()) {
const absoluteRedirectUrl = new URL(redirectUrl, window.location.href);

await this.navigate(this.buildUrlWithAuth(this.client.buildTouchUrl({ redirectUrl: absoluteRedirectUrl })));
} else {
await this.navigate(redirectUrl);
}

beforeUnloadTracker?.stopTracking();
}

//3. Check if hard reloading (onbeforeunload). If not, set the user/session and emit
if (beforeUnloadTracker?.isUnloading()) {
return;
Expand Down Expand Up @@ -1105,13 +1124,12 @@ export class Clerk implements ClerkInterface {
const navigate = (to: string) =>
customNavigate && typeof customNavigate === 'function' ? customNavigate(to) : this.navigate(to);

const redirectComplete = params.redirectUrlComplete ? () => navigate(params.redirectUrlComplete as string) : noop;
const redirectContinue = params.redirectUrl ? () => navigate(params.redirectUrl as string) : noop;

if (shouldCompleteOnThisDevice) {
return this.setActive({
session: newSessionId,
beforeEmit: redirectComplete,
redirectUrl: params.redirectUrlComplete,
});
} else if (shouldContinueOnThisDevice) {
return redirectContinue();
Expand Down Expand Up @@ -1206,8 +1224,6 @@ export class Clerk implements ClerkInterface {
);

const redirectUrls = new RedirectUrls(this.#options, params);
const navigateAfterSignIn = makeNavigate(redirectUrls.getAfterSignInUrl());
const navigateAfterSignUp = makeNavigate(redirectUrls.getAfterSignUpUrl());

const navigateToContinueSignUp = makeNavigate(
params.continueSignUpUrl ||
Expand Down Expand Up @@ -1240,7 +1256,7 @@ export class Clerk implements ClerkInterface {
if (si.status === 'complete') {
return this.setActive({
session: si.sessionId,
beforeEmit: navigateAfterSignIn,
redirectUrl: redirectUrls.getAfterSignInUrl(),
});
}

Expand All @@ -1253,7 +1269,7 @@ export class Clerk implements ClerkInterface {
case 'complete':
return this.setActive({
session: res.createdSessionId,
beforeEmit: navigateAfterSignIn,
redirectUrl: redirectUrls.getAfterSignInUrl(),
});
case 'needs_first_factor':
return navigateToFactorOne();
Expand Down Expand Up @@ -1301,7 +1317,7 @@ export class Clerk implements ClerkInterface {
case 'complete':
return this.setActive({
session: res.createdSessionId,
beforeEmit: navigateAfterSignUp,
redirectUrl: redirectUrls.getAfterSignUpUrl(),
});
case 'missing_requirements':
return navigateToNextStepSignUp({ missingFields: res.missingFields });
Expand All @@ -1313,7 +1329,7 @@ export class Clerk implements ClerkInterface {
if (su.status === 'complete') {
return this.setActive({
session: su.sessionId,
beforeEmit: navigateAfterSignUp,
redirectUrl: redirectUrls.getAfterSignUpUrl(),
});
}

Expand All @@ -1337,7 +1353,7 @@ export class Clerk implements ClerkInterface {
if (sessionId) {
return this.setActive({
session: sessionId,
beforeEmit: navigateAfterSignIn,
redirectUrl: redirectUrls.getAfterSignInUrl(),
});
}
}
Expand Down Expand Up @@ -1462,12 +1478,7 @@ export class Clerk implements ClerkInterface {
if (signInOrSignUp.createdSessionId) {
await this.setActive({
session: signInOrSignUp.createdSessionId,
beforeEmit: () => {
if (redirectUrl) {
return navigate(redirectUrl);
}
return Promise.resolve();
},
redirectUrl,
});
}
};
Expand Down
Loading

0 comments on commit f875463

Please sign in to comment.