clerk / clerk-sdk-go

Access the Clerk Backend API from Go
MIT License
72 stars 11 forks source link

Basic Auth Incompatibility in WithSessionV2 middleware. #282

Open L-four opened 2 months ago

L-four commented 2 months ago

When I use basic auth I.E. https://github.com/go-chi/chi/blob/v5.0.12/middleware/basic_auth.go#L10

The browser adds this header Authorization: Basic <XXXXXX>

Which cause this code path to be taken:

https://github.com/clerk/clerk-sdk-go/blob/59c1229871a66eebbc8bba4cc0855a3da1357e6f/clerk/middleware_v2.go#L54-L77

And as basic auth is not a valid clerk token it bails out here:

https://github.com/clerk/clerk-sdk-go/blob/59c1229871a66eebbc8bba4cc0855a3da1357e6f/clerk/middleware_v2.go#L60-L65

This skips reading the Cookie tokens and prevents login working for my Dev site.

Can we check if the if the Authorization header is a Bearer before we take this path. E.G.

headerToken := strings.TrimSpace(r.Header.Get("Authorization"))
if strings.HasPrefix(headerToken, "Bearer ") {
  // existing logic.
}

Or is this code required to handle "Authorization" headers without the "Bearer " prefix?

gkats commented 2 months ago

Hi @L-four, thanks for the feedback.

I think you're right, we're currently expecting that if there's an Authorization header present, it must include a bearer token. Perhaps there's more uses for the Authorization header that we might be preventing here.

If you don't mind me asking, could you provide more details about your setup?

Your website has some handlers that set the "Set-Cookie" header with valid Clerk cookies but they set the "Authorization" header as well?

If that's the case, do you actually need to set both? Would it work if you didn't set the "Authorization" header at all for routes that authenticate with cookies?

L-four commented 2 months ago

Hi @gkats,

Thanks for the quick response.

I am not setting any cookies explicitly anywhere, I can repro this with just the clerk and basic auth middle wares, to that end i have created a git repo with a reproduction of this issue https://github.com/L-four/clerk-sdk-go-282/blob/master/main.go

Usage: server <api_token> <public_key> <frontend_api_url>

For context am using test credentials I.E. sktest and pktest if that effects anything.

I don't have deep knowledge of how Clerk is using headers to authenticate between the JS client and the go client. I was under the assumption that the JS client handled the authentication with clerk.com and set a cookie in JS for the baclend go code to use for backend API requests to clerk.com.

gkats commented 2 months ago

Hi @L-four thank you for the public repo, this helps a lot!

The Clerk Go SDK can be used in two types of authentication scenarios:

  1. Header based authentication
  2. Cookie based authentication

The header based authentication is suitable for "machine-to-machine" communication and something you would usually want to use to authenticate your API routes. We picked the Bearer token authentication scheme as we felt it's the most commonly used scheme for API authentication.

The cookie based authentication should be used when you want to authenticate your web pages. The browser and Clerk-JS will use cookies, and the middleware will guard authenticated pages, allowing access only for valid cookies.

The way the RequireSessionV2 middleware is currently written, you cannot use both [1]. Header-based authentication takes precedence.

In your setup, isn't it possible to use the RequireSessionV2 middleware only for those routes that do not authenticate via basic auth?

The RequireSessionV2 middleware should not necessarily run for your whole route tree, but only for the routes where it makes sense.

Example (hypothetical scenario, pseudo-code):

r := chi.NewRouter()
r.Get("/", handleRoot)
// Other public routes here...
r.Route("/protected", func(r chi.Router) {
  // Protected routes use the Clerk middleware
  r.Use(clerk.RequireSessionV2())
  r.Get("/profile", handleProfile)
})

Does the above help?

[1] By the way, the newest version of our Go SDK splits header based authentication to its own middleware and doesn't mix concerns.