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

getSession should validate the session with the JWT_SECRET #908

Open
remorses opened this issue May 13, 2024 · 4 comments
Open

getSession should validate the session with the JWT_SECRET #908

remorses opened this issue May 13, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@remorses
Copy link

remorses commented May 13, 2024

The function supabase.auth.getSession() is basically a vulnerability in every Supabase app, a lot of people are currently using it in the server, telling users to use getUser in a warning as currently happens is not enough. It would also mean calling supabase API every time an user does a request, which slow everything down and makes the use of JWTs pointless.

What you could do instead is to validate the jwt inside getSession.

This change would require passing the jwt secret as an argument when creating the client, then you would log the warning if the user doesn't pass the jwt secret.

example:

const supabase = createServerClient(
        env.PUBLIC_SUPABASE_URL!,
        env.PUBLIC_SUPABASE_ANON_KEY!,
        {
            jwtSecret: process.env.SUPABASE_JWT_SECRET,
       }
)
@remorses remorses added the bug Something isn't working label May 13, 2024
@remorses
Copy link
Author

remorses commented May 13, 2024

I just got an idea to fix this without having users change their code:

  1. Add a field in the jwt that is a signature created with a global Supabase public key. This signature would contain the project ref
  2. In getSession, validate this field with the Supabase public key and check that the project ref is right (so different projects cannot create JWTs that work in other apps)
  3. When creating a new token, add the signature. I think this should be possible because Supabase controls the JWT creation process and is done on your servers.
  4. Only do the signature check if the token was created since this change took effect, this can be done because a JWT has an expiration limit which means you cannot spoof a JWT by changing the creation date.

@j4w8n
Copy link
Contributor

j4w8n commented May 13, 2024

They're releasing asymmetric jwts "soon," but I've not seen a public timeline.

As part of that, I'm hoping they build in the functionality that you're talking about - pass the public jwt key either to the client or the getSession() method itself.

@marcusklausen
Copy link

marcusklausen commented Nov 29, 2024

@j4w8n any news on this? It's been over a year with this bug and excessive logs of

Using the user object as returned from supabase.auth.getSession() or from some supabase.auth.onAuthStateChange() events could be insecure! This value comes directly from the storage medium (usually cookies on the server) and many not be authentic. Use supabase.auth.getUser() instead which authenticates the data by contacting thez Supabase Auth server.

I was wondering if we could just have a flag to disable the warning at least, like I've suggested in my PR #953

Seems very ambiguous, like the issue suggest to allow the use of getSession without JWT secret, and at the same time log a warning about how dangerous this approach is.

Debugging in nextjs is currently a daily nightmare

Screenshot 2024-11-29 at 19 22 20

@j4w8n
Copy link
Contributor

j4w8n commented Nov 29, 2024

@marcusklausen, the last timeline I saw was Q4 of this year.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants