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

feat(nextjs): Clock skew error message #1661

Merged
merged 4 commits into from
Sep 1, 2023
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
6 changes: 6 additions & 0 deletions .changeset/green-mails-cover.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@clerk/backend': patch
'@clerk/nextjs': patch
---

Improve error messaging when clock skew is detected.
1 change: 1 addition & 0 deletions packages/backend/src/tokens/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export enum TokenVerificationErrorAction {
EnsureClerkJWT = 'Make sure that this is a valid Clerk generate JWT.',
SetClerkJWTKey = 'Set the CLERK_JWT_KEY environment variable.',
SetClerkSecretKeyOrAPIKey = 'Set the CLERK_SECRET_KEY or CLERK_API_KEY environment variable.',
EnsureClockSync = 'Make sure your system clock is in sync (e.g. turn off and on automatic time synchronization).',
}

export class TokenVerificationError extends Error {
Expand Down
1 change: 1 addition & 0 deletions packages/backend/src/tokens/jwt/verifyJwt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ export async function verifyJwt(
const early = notBeforeDate.getTime() > currentDate.getTime() + clockSkew;
if (early) {
throw new TokenVerificationError({
action: TokenVerificationErrorAction.EnsureClockSync,
reason: TokenVerificationErrorReason.TokenNotActiveYet,
message: `JWT cannot be used prior to not before date claim (nbf). Not before date: ${notBeforeDate}; Current date: ${currentDate};`,
});
Expand Down
35 changes: 32 additions & 3 deletions packages/nextjs/src/server/authMiddleware.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { AuthObject } from '@clerk/backend';
import type { AuthObject, RequestState } from '@clerk/backend';
import { buildRequestUrl, constants } from '@clerk/backend';
import type Link from 'next/link';
import type { NextFetchEvent, NextMiddleware, NextRequest } from 'next/server';
Expand All @@ -9,7 +9,12 @@ import { withLogger } from '../utils/debugLogger';
import { authenticateRequest, handleInterstitialState, handleUnknownState } from './authenticateRequest';
import { SECRET_KEY } from './clerkClient';
import { DEV_BROWSER_JWT_MARKER, setDevBrowserJWTInURL } from './devBrowser';
import { infiniteRedirectLoopDetected, informAboutProtectedRouteInfo, receivedRequestForIgnoredRoute } from './errors';
import {
clockSkewDetected,
infiniteRedirectLoopDetected,
informAboutProtectedRouteInfo,
receivedRequestForIgnoredRoute,
} from './errors';
import { redirectToSignIn } from './redirect';
import type { NextMiddlewareResult, WithAuthOptions } from './types';
import { isDevAccountPortalOrigin } from './url';
Expand Down Expand Up @@ -187,8 +192,11 @@ const authMiddleware: AuthMiddleware = (...args: unknown[]) => {
return handleUnknownState(requestState);
} else if (requestState.isInterstitial) {
logger.debug('authenticateRequest state is interstitial', requestState);

assertClockSkew(requestState, options);

const res = handleInterstitialState(requestState, options);
return assertInfiniteRedirectionLoop(req, res, options);
return assertInfiniteRedirectionLoop(req, res, options, requestState);
}

const auth = Object.assign(requestState.toAuth(), {
Expand Down Expand Up @@ -334,20 +342,41 @@ const isRequestMethodIndicatingApiRoute = (req: NextRequest): boolean => {
return !['get', 'head', 'options'].includes(requestMethod);
};

/**
* In development, attempt to detect clock skew based on the requestState. This check should run when requestState.isInterstitial is true. If detected, we throw an error.
*/
const assertClockSkew = (requestState: RequestState, opts: AuthMiddlewareParams): void => {
if (!isDevelopmentFromApiKey(opts.secretKey || SECRET_KEY)) {
return;
}

if (requestState.reason === 'token-not-active-yet') {
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

@nikosdouvlis I would have to import from the dist, what's our stance on that?

import { TokenVerificationErrorReason } from '@clerk/backend/dist/types/tokens/errors';

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use the requestState.action: TokenVerificationErrorAction.EnsureClockSync ?

throw new Error(clockSkewDetected(requestState.message));
}
};

// When in development, we want to prevent infinite interstitial redirection loops.
// We incrementally set a `__clerk_redirection_loop` cookie, and when it loops 6 times, we throw an error.
// We also utilize the `referer` header to skip the prefetch requests.
const assertInfiniteRedirectionLoop = (
req: NextRequest,
res: NextResponse,
opts: AuthMiddlewareParams,
requestState: RequestState,
): NextResponse => {
if (!isDevelopmentFromApiKey(opts.secretKey || SECRET_KEY)) {
return res;
}

const infiniteRedirectsCounter = Number(req.cookies.get(INFINITE_REDIRECTION_LOOP_COOKIE)?.value) || 0;
if (infiniteRedirectsCounter === 6) {
// Infinite redirect detected, is it clock skew?
// We check for token-expired here because it can be a valid, recoverable scenario, but in a redirect loop a token-expired error likely indicates clock skew.
if (requestState.reason === 'token-expired') {
throw new Error(clockSkewDetected(requestState.message));
}

// Not clock skew, return general error
throw new Error(infiniteRedirectLoopDetected());
}

Expand Down
16 changes: 10 additions & 6 deletions packages/nextjs/src/server/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,24 @@ export const getAuthAuthHeaderMissing = () =>
export const authAuthHeaderMissing = () =>
"Clerk: auth() was called but it looks like you aren't using `authMiddleware` in your middleware file. Please use `authMiddleware` and make sure your middleware matcher is configured correctly and it matches this route or page. See https://clerk.com/docs/quickstarts/get-started-with-nextjs";

export const clockSkewDetected = (verifyMessage: string) =>
`Clerk: Clock skew detected. This usually means that your system clock is inaccurate. Clerk will continuously try to issue new tokens, as the existing ones will be treated as "expired" due to clock skew.

To resolve this issue, make sure your system's clock is set to the correct time (e.g. turn off and on automatic time synchronization).

---

${verifyMessage}`;

export const infiniteRedirectLoopDetected = () =>
`Clerk: Infinite redirect loop detected. That usually means that we were not able to determine the auth state for this request. A list of common causes and solutions follows.

Reason 1:
Your server's system clock is inaccurate. Clerk will continuously try to issue new tokens, as the existing ones will be treated as "expired" due to clock skew.
How to resolve:
-> Make sure your system's clock is set to the correct time (e.g. turn off and on automatic time synchronization).

Reason 2:
Your Clerk instance keys are incorrect, or you recently changed keys (Publishable Key, Secret Key).
How to resolve:
-> Make sure you're using the correct keys from the Clerk Dashboard. If you changed keys recently, make sure to clear your browser application data and cookies.

Reason 3:
Reason 2:
A bug that may have already been fixed in the latest version of Clerk NextJS package.
How to resolve:
-> Make sure you are using the latest version of '@clerk/nextjs' and 'next'.
Expand Down