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

Use separate jwt's for invoking preview and storing preview state #2691

Merged
merged 5 commits into from
Nov 4, 2024

Conversation

fraxachun
Copy link
Contributor

Description

The JWT to invoke the site preview has to be transferred via a GET-Parameter (see Draft Mode in Next.JS). This PR shortens the validity time of this JWT to 10 seconds as the site creates a new JWT (which is not transferred via GET) for itself which stores the preview state.

Additional, do not throw a server error when JWT is not valid. This also prevents an error when the preview-state jwt is not valid anymore, it just will be ignored.

Changeset

  • I have verified if my change requires a changeset -> No changeset needed for this internal change of a newly added feature.

}

const data = await verifySitePreviewJwt(jwt);
if (!data) {
return NextResponse.json({ error: "JWT-validation failed." });
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this return a 200 status code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now 400: e44e6f3

@@ -11,10 +11,13 @@ async function legacyPagesRouterSitePreviewApiHandler(
const jwt = params.jwt;

if (typeof jwt !== "string") {
throw new Error("Missing jwt parameter");
return res.end("JWT-Parameter is missing.");
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should probably be 400 bad request

Copy link
Contributor

Choose a reason for hiding this comment

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

HTTP 401 Unauthorized would also be an option

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's 400 an json now: e44e6f3

}

const data = await verifySitePreviewJwt(jwt);
if (!data) {
return res.end("JWT-validation failed.");
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should probably be 400 bad request

@johnnyomair johnnyomair self-requested a review October 31, 2024 10:47
@thomasdax98 thomasdax98 added this to the v7.6.0 milestone Nov 4, 2024
@thomasdax98 thomasdax98 merged commit 3ac6c29 into main Nov 4, 2024
11 checks passed
@thomasdax98 thomasdax98 deleted the site-preview-jwt-expire branch November 4, 2024 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants