supabase / auth-js

An isomorphic Javascript library for Supabase Auth.
MIT License
368 stars 165 forks source link

Expired JWT allows database table update #983

Open paulschneider opened 3 days ago

paulschneider commented 3 days ago

Bug report

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

Additional context

j4w8n commented 6 hours ago

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 commented 6 hours ago

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

j4w8n commented 6 hours ago

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.