Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(clerk-js): Extend client cookie lifetime #4312

Merged
merged 18 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
}
issuedat marked this conversation as resolved.
Show resolved Hide resolved

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 @@ -1459,12 +1475,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
Loading