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

Expired JWT allows database table update #983

Open
2 tasks done
paulschneider opened this issue Nov 19, 2024 · 3 comments
Open
2 tasks done

Expired JWT allows database table update #983

paulschneider opened this issue Nov 19, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@paulschneider
Copy link

paulschneider commented Nov 19, 2024

Bug report

  • I confirm this is a bug with Supabase, not with my own application.
  • I confirm I have searched the Docs, GitHub Discussions, and Discord.

Describe the bug

It is possible to use the JS library to make a call to "database.update" and have requested changes commit to the table using an expired JWT. Making a check of session.getUser() returns a null value indicating that the user is not authenticated but the underlying operation to make the update is successful.

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

[In the context of a NestJS API]

  1. Make an API PATCH request using a client such as Insomnia or Postman (I'm using Insomnia)
  2. Provide a previously valid JWT (I.e now expired) within the Authorisation header as a bearer token
  3. (Checks for the validity of the token are implemented within a middleware):
if ('authorization' in req.headers && 'refreshtoken' in req.headers) {
      const token = req.headers.authorization.split(' ')[1]

      const { data: { user } } = await Client.connection.auth.getUser(token)

      // make sure the user is authenticated
      if (!user) {
        next(res.status(403).json({ message: "User is not logged in" }))
      }

      // set the current session with the provided tokens
      await Client.connection.auth.setSession({
        access_token: token,
        refresh_token: req.headers.refreshtoken as string
      })

      return next()
    }
  1. Checks for user availability in the session "fail" correctly when implemented (I.e no user returned by SupaBase as expected):
if (!user) {
        next(res.status(403).json({ message: "User is not logged in" }))
      }
  1. However commenting out these lines ^^
  2. ..and making a call to update the database:
const response = await Client.connection
        .from(this.tableName)
        .update({
          "given_name": updateProfileDto.givenName,
          "family_name": updateProfileDto.familyName
        })
        .eq("user_id", updateProfileDto.userId)
        .select()

      return response
  1. is successful (but shouldn't be permitted)

Expected behavior

The database change (or any operation requiring an authenticated user) should be denied and not occur.

System information

  • OS: MacOS
  • Version of supabase-js: 2.46.1
  • Version of Node.js: v21.0

Additional context

  • A test of this workflow is successful when a valid JWT is provided.
  • I confirmed the expiration time of the JWT via jwt.io and it confirmed the time was in the past
@paulschneider paulschneider added the bug Something isn't working label Nov 19, 2024
@j4w8n
Copy link
Contributor

j4w8n commented Nov 22, 2024

There is a small window of time where an expired JWT will still be accepted. I don't recall the exact timeframe though - maybe 30 seconds? How long are you waiting before you try the expired JWT?

@j4w8n
Copy link
Contributor

j4w8n commented Nov 22, 2024

Err... I was wrong there. I was thinking of the refresh token being able to be used more than once.

@j4w8n
Copy link
Contributor

j4w8n commented Nov 22, 2024

Calling setSession with an expired access_token and a valid refresh_token will cause the session to be refreshed, giving you a valid access_token for your query.

If you want to avoid this, then I'd manually set the client's authorization header when creating the client, instead of using setSession.

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

2 participants