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

Proper way to whitelist/publicize a health api route with authMiddleware? #1441

Open
dcyoung opened this issue Jul 1, 2023 · 7 comments
Open
Labels
documentation Improvements or additions to documentation interstitial prioritized This issue has been triaged and the team is working on it

Comments

@dcyoung
Copy link

dcyoung commented Jul 1, 2023

With the old middleware, i performed a "public" check on my /api/healthz route, and allowed public traffic before ever calling useAuth. With the new authMiddleware, adding the /api/healthz route to the publicRoute list does not accomplish this behavior.

Instead I'm using a negative look ahead like so:

 apiRoutes: ['/api/(\\?!healthz)(.*)', '/trpc/(.*)'],

Is this the recommended method?

@dimkl
Copy link
Member

dimkl commented Jul 4, 2023

Hello @dcyoung
I would suggest to either ignore the middleware for the health endpoint if you don't consider the middleware part of the health checks. Otherwise I would ignore the health endpoint for authMiddleware specifically by using the ignoredRoutes property.

@dimkl dimkl self-assigned this Jul 4, 2023
@dimkl dimkl added the documentation Improvements or additions to documentation label Jul 4, 2023
@dcyoung
Copy link
Author

dcyoung commented Jul 4, 2023

Perfect. Thanks!

@dcyoung dcyoung closed this as completed Jul 4, 2023
@dcyoung dcyoung reopened this Jul 4, 2023
@dcyoung
Copy link
Author

dcyoung commented Jul 4, 2023

@dimkl Using the ignoredRoutes works and is definitely more explicit, but I'm seeing the following log whenever I hit the health endpoint (even in a prod build):

Clerk: The middleware was skipped for this request URL: http://0.0.0.0:3000/api/healthz. For performance reasons, it's recommended to your middleware matcher to:
export const config = {
  matcher: ["/((?!.*\\..*|_next).*)","/","/(api|trpc)(.*)"],
};

Alternatively, you can set your own ignoredRoutes. See https://clerk.com/docs/nextjs/middleware

We run health checks in our orchestration layer at frequent intervals (~15s), and this may pollute our logs. Anyway to suppress this log?

Also, the config recommended by the message is identical to the one exported in my middleware file.

@dimkl
Copy link
Member

dimkl commented Jul 4, 2023

🤔 We cannot determine if the request is for ignored route or not before receiving a request, so I couldn't find a way to skip this log.

We are taking a deeper look at some conditions we have, to allow adding the /api/healthz endpoint to both publicRoutes and apiRoutes and make it work. Eg :

export default authMiddleware({
    apiRoutes: [/\/api/],
    publicRoutes: [/\/api\/healthz/]
});

Until we make the above or something similar available, I would suggest you to ignore the /api/healthz in the middleware exported config.matcher.

Let me know if this works for you.

@dcyoung
Copy link
Author

dcyoung commented Aug 1, 2023

Hi there @dimkl
We've been ok with polluted logs in the short term, but we're still hoping for a resolution on this.

Until we make the above or something similar available, I would suggest you to add ignore the /api/healthz in the middleware exported config.matcher.

Can you provide a snippet/example of that matcher regex.

Our current setup looks like:

export default authMiddleware({
    publishableKey: env.XXX,
    ....,
    ignoredRoutes: ['/api/healthz(.*)'],
});

export const config = {
    matcher: ["/((?!.*\\..*|_next).*)", "/", "/(api|trpc)(.*)"],
};

@dcyoung
Copy link
Author

dcyoung commented Aug 18, 2023

@dimkl ^ friendly bump

@jescalan jescalan added the needs-triage A ticket that needs to be triaged by a team member label Sep 6, 2023
@jescalan jescalan added the linear Created by Linear-GitHub Sync label Nov 16, 2023
@jescalan
Copy link
Contributor

I'd like to note that we're working on an easier way to accomplish this with middleware as a first class solution in our upcoming major release. We don't have an exact release date yet but sometime within the next couple months. Hopefully that will make this way easier for you all 😁

@jescalan jescalan added prioritized This issue has been triaged and the team is working on it and removed linear Created by Linear-GitHub Sync needs-triage A ticket that needs to be triaged by a team member labels Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation interstitial prioritized This issue has been triaged and the team is working on it
Projects
None yet
Development

No branches or pull requests

3 participants