javascript icon indicating copy to clipboard operation
javascript copied to clipboard

Paths with a "." are ignored by `authMiddleware()`

Open IGassmann opened this issue 2 years ago • 10 comments

  • [x] I have reviewed the documentation
  • [x] I have searched for existing issues
  • [x] I have ensured I am on the most recent version, and checked the changelog for that version
  • [x] I have provided the Frontend API key from my application dashboard: pk_test_dGVuZGVyLXF1YWdnYS0zMC5jbGVyay5hY2NvdW50cy5kZXYk
  • [x] I have provided a minimal reproduction or replay recording below

Brief Summary of the Issue

Next.js routes that include a . in their path are ignored by the authMiddleware(), even though . is a valid character to have within a URL path:

// `encodeURIComponent()` encodes characters such as ?, =, /, &, :
console.log(`paths/with/${encodeURIComponent('a.dot')}/are/okay`);
// Expected output: "paths/with/a.dot/are/okay"

This makes those pages error in Next.js if they use auth() in one of their Server Components.

Potential Solution

The Clerk's default proposed middleware config matcher doesn't match URL paths that include a . in their path.

Editing the matcher to allow . doesn't solve the issue, because the authMiddleware() also ignores those URL paths by default due to its DEFAULT_IGNORED_ROUTES.

Both the DEFAULT_CONFIG_MATCHER and the DEFAULT_IGNORED_ROUTES need to be updated in the authMiddleware() code and in the docs to allow for . in URL paths.

Minimal Reproduction or Replay

  • git clone https://github.com/IGassmann/dot-clerk-issue
  • npm install the required dependencies.
  • npm run dev to launch the development server.
  • Open http://localhost:3000
  • Click in "View Demo"
  • This will open http://localhost:3000/dot.path/dashboard which will throw:
Error: 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

Package + Version

  • [ ] @clerk/clerk-js
  • [ ] @clerk/clerk-react
  • [x] @clerk/nextjs
  • [ ] @clerk/remix
  • [ ] @clerk/types
  • [ ] @clerk/themes
  • [ ] @clerk/localizations
  • [ ] @clerk/clerk-expo
  • [ ] @clerk/backend
  • [ ] @clerk/clerk-sdk-node
  • [ ] @clerk/shared
  • [ ] @clerk/fastify
  • [ ] @clerk/chrome-extension
  • [ ] gatsby-plugin-clerk
  • [ ] build/tooling/chore
  • [ ] other:

IGassmann avatar Aug 30 '23 08:08 IGassmann

Hey @IGassmann - just want to drop a little context here. The reason we ignore paths with a . currently is an attempt to avoid running auth on requests to assets like images, css files, etc, which have a wide range of extensions and all have a . in them. It would probably produce a number of issues if we tried to guess what actual extensions people use for files and got it wrong in any situation, so the . detection felt like casting a wider net, given that using . is relatively uncommon for non-asset paths.

However, what you have brought up here is entirely valid and we're going to think about how we can provide a solution for you here. Any ideas you have are of course welcome as well!

jescalan avatar Aug 31 '23 21:08 jescalan

Hey @IGassmann we have recently updated our default matcher and ignored routes patterns to be more tolerant of . in path segments. Can you update @clerk/nextjs, update your middleware matcher to match what we have in our documentation, and let us know if this addresses your issue? 🙏

brkalow avatar Sep 26 '23 14:09 brkalow

Hey @BRKalow! Unfortunately, this doesn't solve our use case because we do have URLs in the following format:

  • /events/page.viewed
  • /events/user.signed.up

Our event names follow the dot case notation and can appear in the last segment of a URL. The Clerk's matcher would exclude those.

I wonder if this Next.js middleware config is enough to avoid running auth on requests to assets:

export const config = {
  matcher: [
    /*
     * Match all request paths except for the ones starting with:
     * - _next/static (static files)
     * - _next/image (image optimization files)
     * - favicon.ico (favicon file)
     */
    '/((?!_next/static|_next/image|favicon.ico).*)',
  ],
}

Next.js's static files (JS, CSS, fonts...) files that are statically imported in code are all served from _next/static, while images that are used with the <Image /> are all served from _next/image. The only assets that I know of that would be outside of those directories would be file assets placed in the public/ directory that aren't being statically imported in code:

  • public/robots.txt
  • public/sitemap.xml
  • public/logo.png with <img src="/logo.png" /> over import logo from '../public/logo.png'; <Image src={logo} />
  • ...

IGassmann avatar Oct 02 '23 13:10 IGassmann

Thanks for providing that additional information 👍

We currently have these tests: https://github.com/clerkinc/javascript/blob/164f3aac7928bc69301846130cc77986569d4e91/packages/nextjs/src/server/authMiddleware.test.ts#L134-L147

When I add your usage example to them, they fail:

✕ does not match /protected/hello.example
✕ does not match /protected/hello.world.example
✕ does not match /protected/hello.world.example.here

So we have more fine-tuning here to do.

Your suggestion for the matcher is the same as https://nextjs.org/docs/pages/building-your-application/routing/middleware#matcher - we should try if we can make it simpler like that 👍

LekoArts avatar Oct 04 '23 12:10 LekoArts

I also ran into this issue, and after going down a rabbit hole through the source code I figured out that it was due to .s being in the path, and ultimately ended up here.

In the app I'm building, I have pages for domains in the format /domains/:domain - e.g. /domains/example.com. As a workaround, I'm instead modeling the URL as /domains/:domain/page (e.g. /domains/example.com/page) but I'd prefer not to have to do that.

Is this being worked on? Any expectation on when this will be supported?

codyjk avatar Nov 04 '23 06:11 codyjk

Hi @codyjk! This can be worked around by updating your middleware matcher and passing a custom ignoredRoutes option to authMiddleware(). The default ignoredRoutes value is:

ignoredRoutes: [`/((?!api|trpc))(_next.*|.+\\.[\\w]+$)`]

We do a best-effort match here, but as you have seen it doesn't handle all edge cases. Feel free to tweak the middleware matcher and ignoredRoutes pattern to handle your use case. 🙏

brkalow avatar Nov 06 '23 21:11 brkalow

Hi @codyjk! This can be worked around by updating your middleware matcher and passing a custom ignoredRoutes option to authMiddleware(). The default ignoredRoutes value is:

ignoredRoutes: [`/((?!api|trpc))(_next.*|.+\\.[\\w]+$)`]

We do a best-effort match here, but as you have seen it doesn't handle all edge cases. Feel free to tweak the middleware matcher and ignoredRoutes pattern to handle your use case. 🙏

Thanks for the suggestion - unless I'm missing something, any path that matches ignoredRoutes won't be able to use auth(), correct? Unfortunately that won't work for my use case.

Are there any plans to formally support paths in the form /path/containing.periods?

codyjk avatar Nov 11 '23 07:11 codyjk

@codyjk Correct, but you can provide your own ignoredRoutes matchers to support your specific path format:

ignoredRoutes: [`/((?!api|trpc|domains))(_next.*|.+\\.[\\w]+$)`]

And updating the middleware matcher:

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

This would prevent any routes starting with /domains from being ignored, allowing you to use auth() in those pages.

brkalow avatar Nov 13 '23 18:11 brkalow

@BRKalow - makes sense, that did the trick. Thanks for the help!

codyjk avatar Nov 20 '23 01:11 codyjk

Hi,

Just got caught by the same issue. I believe this could potentially lead to security issues where dev modifies matcher to include routes with . and these routes are not properly redirected for sign-in. I understand we'd preferably ignore static assets here. Though, the current behavior is very implicit and confusing right now because from a dev perspective when ignoredRoutes is not set, we assume that matcher would properly include set routes, but it doesn't. Could DEFAULT_IGNORED_ROUTES be empty instead and let devs decide what to match in matcher? At least, updating the Options table with default values in the documentation would be super useful.

I have to say Next.js doesn't make it easy to filter public assets in the middleware. They seem to ignore public assets in their doc:

  matcher: [
    /*
     * Match all request paths except for the ones starting with:
     * - api (API routes)
     * - _next/static (static files)
     * - _next/image (image optimization files)
     * - favicon.ico (favicon file)
     */
    '/((?!api|_next/static|_next/image|favicon.ico).*)',
  ],
}```

reginaldl avatar Mar 07 '24 19:03 reginaldl

Hi all, we've recently adjusted our recommend matcher to more explicitly ignore common static asset extensions, you can see it here: https://clerk.com/docs/quickstarts/nextjs#add-middleware-to-your-application

brkalow avatar Jul 24 '24 13:07 brkalow

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

clerk-cookie avatar Jul 25 '25 00:07 clerk-cookie