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

Update Manual JWT verification page #925

Merged
merged 15 commits into from
Jul 3, 2024

Conversation

octoper
Copy link
Member

@octoper octoper commented Apr 19, 2024

Warning

Requires clerk/clerk#481 to be merged

This PR updates the example in Manual JWT verification to use @clerk/backend instead of relying on external libraries

@octoper octoper requested a review from a team as a code owner April 19, 2024 19:20
@octoper octoper force-pushed the vaggelis/update-manual-jwt-verification branch from cbea9d5 to f6d8653 Compare April 21, 2024 08:37
@octoper octoper force-pushed the vaggelis/update-manual-jwt-verification branch from f6d8653 to b0f5d73 Compare April 23, 2024 12:01
Copy link

github-actions bot commented Apr 24, 2024

Hey, here’s your docs preview: https://clerk.com/docs/pr/925

Copy link
Contributor

@S3Prototype S3Prototype left a comment

Choose a reason for hiding this comment

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

Approved with suggested changes

docs/backend-requests/handling/manual-jwt.mdx Outdated Show resolved Hide resolved
docs/backend-requests/handling/manual-jwt.mdx Outdated Show resolved Hide resolved
docs/backend-requests/handling/manual-jwt.mdx Outdated Show resolved Hide resolved
Copy link
Contributor

@dimkl dimkl left a comment

Choose a reason for hiding this comment

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

There is already another PR to update this file but it's been outdated.
ref: https://github.com/clerk/clerk-docs/blob/207c8b87517ed4c96b1577163d97e62563ef8f6d/docs/backend-requests/handling/manual-jwt.mdx#example-usage

I would expect to suggest customers to use the clerkClient.authenticateRequest() instead of manually retrieving the token from cookies or authorization header.
If the docs team prefers the current changes (using low level utils), ignore my comment and proceed with merging it.

@alexisintech
Copy link
Member

There is already another PR to update this file but it's been outdated. ref: https://github.com/clerk/clerk-docs/blob/207c8b87517ed4c96b1577163d97e62563ef8f6d/docs/backend-requests/handling/manual-jwt.mdx#example-usage

I would expect to suggest customers to use the clerkClient.authenticateRequest() instead of manually retrieving the token from cookies or authorization header. If the docs team prefers the current changes (using low level utils), ignore my comment and proceed with merging it.

I agree that it can be simpler for the user by using our product - so utilizing our authenticateRequest() method instead of manually retrieving the token. Thank you for thinking of this!!
@octoper would you mind making this change?

@S3Prototype
Copy link
Contributor

@octoper Gentle reminder about this PR

@alexisintech
Copy link
Member

import { clerkClient } from '@clerk/nextjs/server';
import { NextRequest, NextResponse } from 'next/server';

export async function GET(req: NextRequest) {
const { isSignedIn, token } = await clerkClient.authenticateRequest(req);

if (!isSignedIn) {
return NextResponse.json({ status: 401 });
}

// Perform protected actions

return NextResponse.json({ message: token });
}

@alexisintech
Copy link
Member

@alexisintech
Copy link
Member

I've removed the hefty instructions for using verifyToken() - in clerk/docs#1184, the example was moved to its reference page with an in-depth explanation.

The Manual JWT Verification page will now advocate for using authenticateRequest()

@dimkl @octoper can you guys review and let me know your thoughts before I merge 🫶

@alexisintech alexisintech requested a review from dimkl June 28, 2024 17:44
}
}
```
The `authenticateRequest()` method from Clerk's JavaScript Backend SDK does all of this for you. It accepts the request object and authenticates the session token in it. See the [reference page](/docs/references/backend/sessions/authenticate-request) for example usage and more information.
Copy link
Contributor

Choose a reason for hiding this comment

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

I have some notes that should apply before merging this PR.

  • I believe that authenticate-request should not be part of the /docs/references/backend/sessions as it's not exactly related to the sessions and it's definitely not related to our backend endpoints grouped in that url path.
  • code example in authenticate request page should be updated to use @clerk/backend package instead of nextjs
  • I think we should keep a code example of how authenticateRequest() can be used in this page, otherwise does this page make much sense? It's just an intro and a ref to another page.

cc: @alexisintech , @octoper

Copy link
Member

Choose a reason for hiding this comment

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

yeah I agree - this page will definitely get reworked in the upcoming IA. for now, I've made the changes you requested :)

@alexisintech alexisintech requested a review from dimkl July 1, 2024 18:14
@alexisintech alexisintech merged commit a47de71 into main Jul 3, 2024
3 checks passed
@alexisintech alexisintech deleted the vaggelis/update-manual-jwt-verification branch July 3, 2024 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants