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(backend): Add more error reasons for refresh token query param #4193

Merged
merged 6 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
5 changes: 5 additions & 0 deletions .changeset/weak-trees-perform.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@clerk/backend": patch
---

Introduce more refresh token error reasons.
75 changes: 63 additions & 12 deletions packages/backend/src/tokens/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,18 @@ import { verifyHandshakeToken } from './handshake';
import type { AuthenticateRequestOptions } from './types';
import { verifyToken } from './verify';

const RefreshTokenErrorReason = {
NoCookie: 'no-cookie',
NonEligible: 'non-eligible',
InvalidSessionToken: 'invalid-session-token',
MissingApiClient: 'missing-api-client',
MissingSessionToken: 'missing-session-token',
MissingRefreshToken: 'missing-refresh-token',
SessionTokenDecodeFailed: 'session-token-decode-failed',
FetchNetworkError: 'fetch-network-error',
UnexpectedRefreshError: 'unexpected-refresh-error',
} as const;

function assertSignInUrlExists(signInUrl: string | undefined, key: string): asserts signInUrl is string {
if (!signInUrl && isDevelopmentFromSecretKey(key)) {
throw new Error(`Missing signInUrl. Pass a signInUrl for dev instances if an app is satellite`);
Expand Down Expand Up @@ -195,16 +207,36 @@ ${error.getFullMessage()}`,
}

async function refreshToken(authenticateContext: AuthenticateContext): Promise<string> {
// To perform a token refresh, apiClient must be defined.
assertApiClient(options.apiClient);
try {
// To perform a token refresh, apiClient must be defined.
assertApiClient(options.apiClient);
} catch (err: any) {
throw { errors: [{ code: RefreshTokenErrorReason.MissingApiClient, message: err?.message || '' }] };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the mix between single error and errors array? I've only seen single errors, not multiple, in your code below 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LekoArts an errors array is returned on API errors.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just refactored this, to be clearer.

}
const { sessionToken: expiredSessionToken, refreshTokenInCookie: refreshToken } = authenticateContext;
if (!expiredSessionToken || !refreshToken) {
throw new Error('Clerk: refreshTokenInCookie and sessionToken must be provided.');
if (!expiredSessionToken) {
throw {
errors: [
{
code: RefreshTokenErrorReason.MissingSessionToken,
message: 'Session token must be provided.',
},
],
};
}
if (!refreshToken) {
throw {
errors: [{ code: RefreshTokenErrorReason.MissingRefreshToken, message: 'Refresh token must be provided.' }],
};
}
// The token refresh endpoint requires a sessionId, so we decode that from the expired token.
const { data: decodeResult, errors: decodedErrors } = decodeJwt(expiredSessionToken);
if (!decodeResult || decodedErrors) {
throw new Error(`Clerk: unable to decode session token.`);
throw {
errors: [
{ code: RefreshTokenErrorReason.SessionTokenDecodeFailed, message: 'Unable to decode session token.' },
],
};
}
// Perform the actual token refresh.
const tokenResponse = await options.apiClient.sessions.refreshSession(decodeResult.payload.sid, {
Expand All @@ -226,10 +258,19 @@ ${error.getFullMessage()}`,
sessionToken = await refreshToken(authenticateContext);
} catch (err: any) {
if (err?.errors?.length) {
if (err.errors[0].code === 'unexpected_error') {
return {
data: null,
error: {
message: `Fetch unexpected error`,
cause: { reason: RefreshTokenErrorReason.FetchNetworkError, errors: err.errors },
},
};
}
return {
data: null,
error: {
message: `Clerk: unable to refresh session token.`,
message: err.errors[0].code,
cause: { reason: err.errors[0].code, errors: err.errors },
},
};
Expand All @@ -247,7 +288,7 @@ ${error.getFullMessage()}`,
data: null,
error: {
message: `Clerk: unable to verify refreshed session token.`,
cause: { reason: 'invalid-session-token', errors },
cause: { reason: RefreshTokenErrorReason.InvalidSessionToken, errors },
},
};
}
Expand All @@ -263,7 +304,11 @@ ${error.getFullMessage()}`,
): SignedInState | SignedOutState | HandshakeState {
if (isRequestEligibleForHandshake(authenticateContext)) {
// If a refresh error is not passed in, we default to 'no-cookie' or 'non-eligible'.
refreshError = refreshError || (authenticateContext.refreshTokenInCookie ? 'non-eligible' : 'no-cookie');
refreshError =
refreshError ||
(authenticateContext.refreshTokenInCookie
? RefreshTokenErrorReason.NonEligible
: RefreshTokenErrorReason.NoCookie);

// Right now the only usage of passing in different headers is for multi-domain sync, which redirects somewhere else.
// In the future if we want to decorate the handshake redirect with additional headers per call we need to tweak this logic.
Expand Down Expand Up @@ -398,7 +443,9 @@ ${error.getFullMessage()}`,
);
const authErrReason = AuthErrorReason.SatelliteCookieNeedsSyncing;
redirectURL.searchParams.append(constants.QueryParameters.HandshakeReason, authErrReason);
const refreshTokenError = authenticateContext.refreshTokenInCookie ? 'non-eligible' : 'no-cookie';
const refreshTokenError = authenticateContext.refreshTokenInCookie
? RefreshTokenErrorReason.NonEligible
: RefreshTokenErrorReason.NoCookie;
redirectURL.searchParams.append(constants.QueryParameters.RefreshTokenError, refreshTokenError);

const headers = new Headers({ [constants.Headers.Location]: redirectURL.toString() });
Expand All @@ -422,7 +469,9 @@ ${error.getFullMessage()}`,
redirectBackToSatelliteUrl.searchParams.append(constants.QueryParameters.ClerkSynced, 'true');
const authErrReason = AuthErrorReason.PrimaryRespondsToSyncing;
redirectBackToSatelliteUrl.searchParams.append(constants.QueryParameters.HandshakeReason, authErrReason);
const refreshTokenError = authenticateContext.refreshTokenInCookie ? 'non-eligible' : 'no-cookie';
const refreshTokenError = authenticateContext.refreshTokenInCookie
? RefreshTokenErrorReason.NonEligible
: RefreshTokenErrorReason.NoCookie;
redirectBackToSatelliteUrl.searchParams.append(constants.QueryParameters.RefreshTokenError, refreshTokenError);

const headers = new Headers({ [constants.Headers.Location]: redirectBackToSatelliteUrl.toString() });
Expand Down Expand Up @@ -480,7 +529,9 @@ ${error.getFullMessage()}`,
return signedOut(authenticateContext, AuthErrorReason.UnexpectedError);
}

let refreshError: string = authenticateContext.refreshTokenInCookie ? 'non-eligible' : 'no-cookie';
let refreshError: string = authenticateContext.refreshTokenInCookie
? RefreshTokenErrorReason.NonEligible
: RefreshTokenErrorReason.NoCookie;

if (isRequestEligibleForRefresh(err, authenticateContext, request)) {
const { data, error } = await attemptRefresh(authenticateContext);
Expand All @@ -493,7 +544,7 @@ ${error.getFullMessage()}`,
if (error?.cause?.reason) {
refreshError = error.cause.reason;
} else {
refreshError = 'unexpected-refresh-error';
refreshError = RefreshTokenErrorReason.UnexpectedRefreshError;
}
}

Expand Down
Loading