Skip to content

Commit c5969ed

Browse files
authored
fix: check session state for admin tokens (#2555)
## What kind of change does this PR introduce? Bug fix ## What is the current behavior? Middleware validates the session token has not expired. ## What is the new behavior? Validates the token has not expired and that the session has not been revoked. ## Additional context Middleware checks for admin tokens should validate that the session is still live. JWTs may be revoked but stay valid until the expiry time.
1 parent 01f136d commit c5969ed

2 files changed

Lines changed: 142 additions & 0 deletions

File tree

internal/api/middleware.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,31 @@ func (a *API) requireAdminCredentials(w http.ResponseWriter, req *http.Request)
195195
return nil, err
196196
}
197197

198+
// If the token references a real user session, confirm the session
199+
// still exists and is valid in the DB — a JWT remains usable past
200+
// logout or revocation otherwise. Sessionless admin tokens (e.g.
201+
// service_role) skip this check.
202+
claims := getClaims(ctx)
203+
if claims != nil && claims.SessionId != "" && claims.SessionId != uuid.Nil.String() {
204+
ctx, err = a.maybeLoadUserOrSession(ctx)
205+
if err != nil {
206+
return nil, err
207+
}
208+
209+
session := getSession(ctx)
210+
user := getUser(ctx)
211+
if session != nil && user != nil {
212+
validity := session.CheckValidity(models.SessionValidityConfig{
213+
Timebox: a.config.Sessions.Timebox,
214+
InactivityTimeout: a.config.Sessions.InactivityTimeout,
215+
AllowLowAAL: a.config.Sessions.AllowLowAAL,
216+
}, time.Now(), nil, user.HighestPossibleAAL())
217+
if validity != models.SessionValid {
218+
return nil, apierrors.NewForbiddenError(apierrors.ErrorCodeSessionExpired, "Session is no longer valid")
219+
}
220+
}
221+
}
222+
198223
return a.requireAdmin(ctx)
199224
}
200225

internal/api/middleware_test.go

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,15 @@ import (
1212

1313
"github.com/didip/tollbooth/v5"
1414
"github.com/didip/tollbooth/v5/limiter"
15+
"github.com/gofrs/uuid"
1516
jwt "github.com/golang-jwt/jwt/v5"
1617
"github.com/stretchr/testify/assert"
1718
"github.com/stretchr/testify/mock"
1819
"github.com/stretchr/testify/require"
1920
"github.com/stretchr/testify/suite"
2021
"github.com/supabase/auth/internal/api/apierrors"
2122
"github.com/supabase/auth/internal/conf"
23+
"github.com/supabase/auth/internal/models"
2224
"github.com/supabase/auth/internal/sbff"
2325
"github.com/supabase/auth/internal/security"
2426
"github.com/supabase/auth/internal/storage"
@@ -831,3 +833,118 @@ func (ts *MiddlewareTestSuite) TestDatabaseCleanup() {
831833
}
832834
mockCleanup.AssertNumberOfCalls(ts.T(), "Clean", 1)
833835
}
836+
837+
func (ts *MiddlewareTestSuite) TestRequireAdminCredentialsSessionCheck() {
838+
models.TruncateAll(ts.API.db)
839+
840+
user, err := models.NewUser("", "admin-session@example.com", "password", ts.Config.JWT.Aud, nil)
841+
require.NoError(ts.T(), err)
842+
require.NoError(ts.T(), ts.API.db.Create(user))
843+
844+
session, err := models.NewSession(user.ID, nil)
845+
require.NoError(ts.T(), err)
846+
require.NoError(ts.T(), ts.API.db.Create(session))
847+
848+
signClaims := func(claims *AccessTokenClaims) string {
849+
signed, err := jwt.NewWithClaims(jwt.SigningMethodHS256, claims).SignedString([]byte(ts.Config.JWT.Secret))
850+
require.NoError(ts.T(), err)
851+
return signed
852+
}
853+
854+
newRequest := func(token string) *http.Request {
855+
req := httptest.NewRequest(http.MethodGet, "http://localhost", nil)
856+
req.Header.Set("Authorization", "Bearer "+token)
857+
return req
858+
}
859+
860+
ts.Run("admin token with valid session is accepted", func() {
861+
token := signClaims(&AccessTokenClaims{
862+
Role: "supabase_admin",
863+
SessionId: session.ID.String(),
864+
RegisteredClaims: jwt.RegisteredClaims{
865+
Subject: user.ID.String(),
866+
ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour)),
867+
},
868+
})
869+
870+
_, err := ts.API.requireAdminCredentials(httptest.NewRecorder(), newRequest(token))
871+
require.NoError(ts.T(), err)
872+
})
873+
874+
ts.Run("admin token whose session was revoked is rejected", func() {
875+
token := signClaims(&AccessTokenClaims{
876+
Role: "supabase_admin",
877+
SessionId: session.ID.String(),
878+
RegisteredClaims: jwt.RegisteredClaims{
879+
Subject: user.ID.String(),
880+
ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour)),
881+
},
882+
})
883+
require.NoError(ts.T(), models.LogoutSession(ts.API.db, session.ID))
884+
885+
_, err := ts.API.requireAdminCredentials(httptest.NewRecorder(), newRequest(token))
886+
require.Error(ts.T(), err)
887+
httpErr, ok := err.(*HTTPError)
888+
require.True(ts.T(), ok, "expected HTTPError, got %T", err)
889+
require.Equal(ts.T(), apierrors.ErrorCodeSessionNotFound, httpErr.ErrorCode)
890+
})
891+
892+
ts.Run("admin token past session timebox is rejected", func() {
893+
freshSession, err := models.NewSession(user.ID, nil)
894+
require.NoError(ts.T(), err)
895+
require.NoError(ts.T(), ts.API.db.Create(freshSession))
896+
897+
// Backdate the session past the timebox — pop manages created_at
898+
// automatically on Update, so go through raw SQL.
899+
require.NoError(ts.T(), ts.API.db.RawQuery(
900+
"UPDATE auth.sessions SET created_at = ? WHERE id = ?",
901+
time.Now().Add(-48*time.Hour), freshSession.ID,
902+
).Exec())
903+
904+
timebox := time.Hour
905+
original := ts.Config.Sessions.Timebox
906+
ts.Config.Sessions.Timebox = &timebox
907+
defer func() { ts.Config.Sessions.Timebox = original }()
908+
909+
token := signClaims(&AccessTokenClaims{
910+
Role: "supabase_admin",
911+
SessionId: freshSession.ID.String(),
912+
RegisteredClaims: jwt.RegisteredClaims{
913+
Subject: user.ID.String(),
914+
ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour)),
915+
},
916+
})
917+
918+
_, err = ts.API.requireAdminCredentials(httptest.NewRecorder(), newRequest(token))
919+
require.Error(ts.T(), err)
920+
httpErr, ok := err.(*HTTPError)
921+
require.True(ts.T(), ok, "expected HTTPError, got %T", err)
922+
require.Equal(ts.T(), apierrors.ErrorCodeSessionExpired, httpErr.ErrorCode)
923+
})
924+
925+
ts.Run("sessionless service_role token still passes", func() {
926+
token := signClaims(&AccessTokenClaims{
927+
Role: "service_role",
928+
RegisteredClaims: jwt.RegisteredClaims{
929+
ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour)),
930+
},
931+
})
932+
933+
_, err := ts.API.requireAdminCredentials(httptest.NewRecorder(), newRequest(token))
934+
require.NoError(ts.T(), err)
935+
})
936+
937+
ts.Run("admin token with nil-uuid session_id passes (treated as sessionless)", func() {
938+
token := signClaims(&AccessTokenClaims{
939+
Role: "supabase_admin",
940+
SessionId: uuid.Nil.String(),
941+
RegisteredClaims: jwt.RegisteredClaims{
942+
Subject: user.ID.String(),
943+
ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour)),
944+
},
945+
})
946+
947+
_, err := ts.API.requireAdminCredentials(httptest.NewRecorder(), newRequest(token))
948+
require.NoError(ts.T(), err)
949+
})
950+
}

0 commit comments

Comments
 (0)