diff --git a/backend/internal/oidc/cleanup_test.go b/backend/internal/oidc/cleanup_test.go index df1ff251..f71c9a3d 100644 --- a/backend/internal/oidc/cleanup_test.go +++ b/backend/internal/oidc/cleanup_test.go @@ -17,13 +17,14 @@ import ( // past their expiry are removed. func TestCleanupExpiredOAuth2SessionsKeepsInvalidatedButUnexpiredSessions(t *testing.T) { db := testutils.NewDatabaseForTest(t) + require.NoError(t, db.Create(&model.OidcClient{Base: model.Base{ID: "cleanup-client"}, Name: "Cleanup Client"}).Error) future := datatype.DateTime(time.Now().Add(time.Hour)) rows := []OAuth2Session{ - {Base: model.Base{ID: "expired"}, Kind: "access_token", Key: "k-expired", RequestID: "r1", Active: true, RequestData: "{}", ExpiresAt: new(datatype.DateTime(time.Now().Add(-time.Hour)))}, - {Base: model.Base{ID: "rotated"}, Kind: "refresh_token", Key: "k-rotated", RequestID: "r2", Active: false, RequestData: "{}", ExpiresAt: &future}, - {Base: model.Base{ID: "active"}, Kind: "refresh_token", Key: "k-active", RequestID: "r3", Active: true, RequestData: "{}", ExpiresAt: &future}, + {Base: model.Base{ID: "expired"}, Kind: "access_token", Key: "k-expired", RequestID: "r1", ClientID: "cleanup-client", Active: true, RequestData: `{"client_id":"cleanup-client"}`, ExpiresAt: new(datatype.DateTime(time.Now().Add(-time.Hour)))}, + {Base: model.Base{ID: "rotated"}, Kind: "refresh_token", Key: "k-rotated", RequestID: "r2", ClientID: "cleanup-client", Active: false, RequestData: `{"client_id":"cleanup-client"}`, ExpiresAt: &future}, + {Base: model.Base{ID: "active"}, Kind: "refresh_token", Key: "k-active", RequestID: "r3", ClientID: "cleanup-client", Active: true, RequestData: `{"client_id":"cleanup-client"}`, ExpiresAt: &future}, } for i := range rows { require.NoError(t, db.Create(&rows[i]).Error) diff --git a/backend/internal/oidc/models.go b/backend/internal/oidc/models.go index b7ff9f09..ef3eebc9 100644 --- a/backend/internal/oidc/models.go +++ b/backend/internal/oidc/models.go @@ -17,6 +17,7 @@ type OAuth2Session struct { Kind string Key string RequestID string + ClientID string AccessTokenSignature string Active bool RequestData string diff --git a/backend/internal/oidc/store.go b/backend/internal/oidc/store.go index c3aff4ae..83e4668a 100644 --- a/backend/internal/oidc/store.go +++ b/backend/internal/oidc/store.go @@ -401,7 +401,7 @@ func (s *Store) RevokeAccessToken(ctx context.Context, requestID string) error { } func (s *Store) RevokeSessionsByIDTokenHint(ctx context.Context, userID, clientID, idTokenJTI string) error { - _, jtiMatches, err := s.findUserClientRequestIDs(ctx, userID, clientID, idTokenJTI) + _, jtiMatches, err := s.findActiveRefreshTokenRequestIDsForUserClient(ctx, userID, clientID, idTokenJTI) if err != nil { return err } @@ -411,17 +411,30 @@ func (s *Store) RevokeSessionsByIDTokenHint(ctx context.Context, userID, clientI func RevokeUserClientSessions(ctx context.Context, db *gorm.DB, userID, clientID string) error { s := NewStore(db, nil) - requestIDs, _, err := s.findUserClientRequestIDs(ctx, userID, clientID, "") + requestIDs, _, err := s.findActiveRefreshTokenRequestIDsForUserClient(ctx, userID, clientID, "") if err != nil { return err } return s.revokeRequestIDs(ctx, requestIDs) } -func (s *Store) findUserClientRequestIDs(ctx context.Context, userID, clientID, idTokenJTI string) (candidates []string, jtiMatches []string, err error) { +// findActiveRefreshTokenRequestIDsForUserClient returns request IDs for active refresh-token sessions belonging to the user and client, plus the subset matching the optional ID token JTI +func (s *Store) findActiveRefreshTokenRequestIDsForUserClient(ctx context.Context, userID, clientID, idTokenJTI string) (candidates []string, jtiMatches []string, err error) { var sessions []OAuth2Session - err = s.dbFor(ctx). - Where("kind = ? AND active = ?", sessionKindRefreshToken, true). + query := s.dbFor(ctx). + Select("request_id", "request_data"). + Where("kind = ? AND active = ? AND client_id = ?", sessionKindRefreshToken, true, clientID) + + // Filter by the user ID stored in the JSON request data + switch query.Name() { + case "sqlite": + query = query.Where("json_extract(CAST(request_data AS TEXT), '$.session.subject') = ?", userID) + case "postgres": + query = query.Where("request_data #>> '{session,subject}' = ?", userID) + default: + return nil, nil, fmt.Errorf("unsupported database dialect: %s", query.Name()) + } + err = query. Find(&sessions). Error if err != nil { @@ -431,19 +444,17 @@ func (s *Store) findUserClientRequestIDs(ctx context.Context, userID, clientID, candidateRequestIDs := map[string]struct{}{} matchingRequestIDs := map[string]struct{}{} for _, session := range sessions { - requester, err := s.decodeRequester(ctx, session.RequestData) - if err != nil { - return nil, nil, err - } - requestSession := requester.GetSession() - if requestSession == nil || requester.GetClient().GetID() != clientID || requestSession.GetSubject() != userID { - continue - } - // Add all sessions that match the user and client candidateRequestIDs[session.RequestID] = struct{}{} // Only add sessions that also match the ID token hint JTI - if storedSession, ok := requestSession.(*Session); ok && idTokenJTI != "" && storedSession.IDTokenClaims().JTI == idTokenJTI { + if idTokenJTI == "" { + continue + } + var stored storedRequester + if err := json.Unmarshal([]byte(session.RequestData), &stored); err != nil { + return nil, nil, err + } + if stored.Session != nil && stored.Session.Claims != nil && stored.Session.Claims.JTI == idTokenJTI { matchingRequestIDs[session.RequestID] = struct{}{} } } @@ -715,10 +726,16 @@ func (s *Store) upsertAuthorizeSession(ctx context.Context, kind string, key str } func (s *Store) storeSession(ctx context.Context, kind string, key string, requestID string, accessTokenSignature string, active bool, requestData string, exp *datatype.DateTime) error { + clientID, err := sessionClientID(requestData) + if err != nil { + return err + } + session := OAuth2Session{ Kind: kind, Key: key, RequestID: requestID, + ClientID: clientID, AccessTokenSignature: accessTokenSignature, Active: active, RequestData: requestData, @@ -730,6 +747,7 @@ func (s *Store) storeSession(ctx context.Context, kind string, key string, reque Columns: []clause.Column{{Name: "kind"}, {Name: "key"}}, DoUpdates: clause.AssignmentColumns([]string{ "request_id", + "client_id", "access_token_signature", "active", "request_data", @@ -740,6 +758,15 @@ func (s *Store) storeSession(ctx context.Context, kind string, key string, reque Error } +func sessionClientID(requestData string) (string, error) { + var stored storedRequester + if err := json.Unmarshal([]byte(requestData), &stored); err != nil { + return "", err + } + + return stored.ClientID, nil +} + func (s *Store) getRequesterSession(ctx context.Context, kind string, key string) (fosite.Requester, bool, error) { session, err := s.getSession(ctx, kind, key) if err != nil { diff --git a/backend/internal/oidc/store_test.go b/backend/internal/oidc/store_test.go index 45a95a05..18352041 100644 --- a/backend/internal/oidc/store_test.go +++ b/backend/internal/oidc/store_test.go @@ -9,6 +9,7 @@ import ( fositejwt "github.com/ory/fosite/token/jwt" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gorm.io/gorm" "github.com/pocket-id/pocket-id/backend/internal/model" testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing" @@ -125,7 +126,15 @@ func TestStoreRevokeSessionsByIDTokenHintRevokesMatchingFositeSessions(t *testin require.NoError(t, store.CreateRefreshTokenSession(t.Context(), "other-client-same-jti", "other-client-access", newTestRequester("other-client-request", otherClientID, userID, idTokenJTI))) require.NoError(t, store.CreateAccessTokenSession(t.Context(), "other-client-access", newTestRequester("other-client-request", otherClientID, userID, idTokenJTI))) + queryCount := 0 + const queryCounterCallback = "test:count_session_revocation_queries" + require.NoError(t, db.Callback().Query().Before("gorm:query").Register(queryCounterCallback, func(*gorm.DB) { + queryCount++ + })) + require.NoError(t, store.RevokeSessionsByIDTokenHint(t.Context(), userID, clientID, idTokenJTI)) + assert.Equal(t, 1, queryCount) + require.NoError(t, db.Callback().Query().Remove(queryCounterCallback)) var sessions []OAuth2Session require.NoError(t, db.Order("key").Find(&sessions).Error) diff --git a/backend/internal/service/e2etest_service.go b/backend/internal/service/e2etest_service.go index 58b4f87e..c647a6fe 100644 --- a/backend/internal/service/e2etest_service.go +++ b/backend/internal/service/e2etest_service.go @@ -277,9 +277,10 @@ func (s *TestService) SeedDatabase(baseURL string) error { Kind: "access_token", Key: "cross-database-test-session", RequestID: "cross-database-test-request", + ClientID: oidcClients[0].ID, AccessTokenSignature: "", Active: true, - RequestData: `{"request":"value"}`, + RequestData: `{"client_id":"3654a746-35d4-4321-ac61-0bdcff2b4055","session":{"subject":"f4b89dc2-62fb-46bf-9f5f-c34f4eafe93e","id_token_claims":{"jti":"cross-database-test-id-token-jti"}}}`, ExpiresAt: &farFuture, } if err := tx.Create(&oauth2Session).Error; err != nil { diff --git a/backend/internal/service/export_service.go b/backend/internal/service/export_service.go index c6b0c140..3b7539f7 100644 --- a/backend/internal/service/export_service.go +++ b/backend/internal/service/export_service.go @@ -57,7 +57,7 @@ func (s *ExportService) extractDatabase() (DatabaseExport, error) { Tables: map[string][]map[string]any{}, // These tables need to be inserted in a specific order because of foreign key constraints // Not all tables are listed here, because not all tables are order-dependent - TableOrder: []string{"users", "user_groups", "oidc_clients", "signup_tokens", "apis", "api_permissions", "oidc_clients_allowed_api_permissions"}, + TableOrder: []string{"users", "user_groups", "oidc_clients", "oauth2_sessions", "signup_tokens", "apis", "api_permissions", "oidc_clients_allowed_api_permissions"}, } for table := range schema { diff --git a/backend/internal/service/oidc_service_test.go b/backend/internal/service/oidc_service_test.go index 4ff209bb..fb7bcf77 100644 --- a/backend/internal/service/oidc_service_test.go +++ b/backend/internal/service/oidc_service_test.go @@ -16,11 +16,55 @@ import ( "github.com/pocket-id/pocket-id/backend/internal/dto" "github.com/pocket-id/pocket-id/backend/internal/model" datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" + "github.com/pocket-id/pocket-id/backend/internal/oidc" "github.com/pocket-id/pocket-id/backend/internal/storage" "github.com/pocket-id/pocket-id/backend/internal/utils" testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing" ) +func TestOidcService_DeleteClientDeletesOAuth2Sessions(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + require.NoError(t, db.Exec("PRAGMA foreign_keys = ON").Error) + + client := model.OidcClient{Base: model.Base{ID: "deleted-client"}, Name: "Deleted Client"} + otherClient := model.OidcClient{Base: model.Base{ID: "other-client"}, Name: "Other Client"} + require.NoError(t, db.Create(&client).Error) + require.NoError(t, db.Create(&otherClient).Error) + + for i, kind := range []string{"authorize_code", "access_token", "refresh_token", "par", "device_code"} { + session := oidc.OAuth2Session{ + Base: model.Base{ID: "deleted-client-session-" + strconv.Itoa(i)}, + Kind: kind, + Key: "deleted-client-key-" + strconv.Itoa(i), + RequestID: "deleted-client-request", + ClientID: client.ID, + Active: true, + RequestData: `{"client_id":"deleted-client","session":{"subject":"test-user","id_token_claims":{"jti":"test-jti"}}}`, + } + require.NoError(t, db.Create(&session).Error) + } + require.NoError(t, db.Create(&oidc.OAuth2Session{ + Base: model.Base{ID: "other-client-session"}, + Kind: "refresh_token", + Key: "other-client-key", + RequestID: "other-client-request", + ClientID: otherClient.ID, + Active: true, + RequestData: `{"client_id":"other-client","session":{"subject":"test-user"}}`, + }).Error) + + service := &OidcService{db: db} + require.NoError(t, service.DeleteClient(t.Context(), client.ID)) + + var deletedClientSessionCount int64 + require.NoError(t, db.Model(&oidc.OAuth2Session{}).Where("client_id = ?", client.ID).Count(&deletedClientSessionCount).Error) + assert.Zero(t, deletedClientSessionCount) + + var otherClientSessionCount int64 + require.NoError(t, db.Model(&oidc.OAuth2Session{}).Where("client_id = ?", otherClient.ID).Count(&otherClientSessionCount).Error) + assert.Equal(t, int64(1), otherClientSessionCount) +} + func TestOidcService_updateClientLogoType(t *testing.T) { // Create a test database db := testutils.NewDatabaseForTest(t) diff --git a/backend/resources/migrations/postgres/20260731120000_oauth2_session_ownership.down.sql b/backend/resources/migrations/postgres/20260731120000_oauth2_session_ownership.down.sql new file mode 100644 index 00000000..5373e242 --- /dev/null +++ b/backend/resources/migrations/postgres/20260731120000_oauth2_session_ownership.down.sql @@ -0,0 +1,6 @@ +DROP INDEX IF EXISTS idx_oauth2_sessions_client_subject; + +ALTER TABLE oauth2_sessions + DROP CONSTRAINT IF EXISTS chk_oauth2_sessions_client_id, + DROP CONSTRAINT IF EXISTS fk_oauth2_sessions_client_id, + DROP COLUMN IF EXISTS client_id; diff --git a/backend/resources/migrations/postgres/20260731120000_oauth2_session_ownership.up.sql b/backend/resources/migrations/postgres/20260731120000_oauth2_session_ownership.up.sql new file mode 100644 index 00000000..daf1ced7 --- /dev/null +++ b/backend/resources/migrations/postgres/20260731120000_oauth2_session_ownership.up.sql @@ -0,0 +1,23 @@ +ALTER TABLE oauth2_sessions + ADD COLUMN client_id TEXT; + +UPDATE oauth2_sessions +SET client_id = request_data ->> 'client_id'; + +DELETE FROM oauth2_sessions +WHERE client_id IS NULL + OR NOT EXISTS ( + SELECT 1 + FROM oidc_clients + WHERE oidc_clients.id = oauth2_sessions.client_id + ); + +ALTER TABLE oauth2_sessions + ALTER COLUMN client_id SET NOT NULL, + ADD CONSTRAINT fk_oauth2_sessions_client_id + FOREIGN KEY (client_id) REFERENCES oidc_clients(id) ON DELETE CASCADE, + ADD CONSTRAINT chk_oauth2_sessions_client_id + CHECK (client_id = request_data ->> 'client_id'); + +CREATE INDEX idx_oauth2_sessions_client_subject + ON oauth2_sessions (client_id, (request_data #>> '{session,subject}'), kind, active); diff --git a/backend/resources/migrations/sqlite/20260731120000_oauth2_session_ownership.down.sql b/backend/resources/migrations/sqlite/20260731120000_oauth2_session_ownership.down.sql new file mode 100644 index 00000000..5d30195a --- /dev/null +++ b/backend/resources/migrations/sqlite/20260731120000_oauth2_session_ownership.down.sql @@ -0,0 +1,47 @@ +PRAGMA foreign_keys = OFF; +BEGIN; + +CREATE TABLE oauth2_sessions_old ( + id TEXT NOT NULL PRIMARY KEY, + created_at DATETIME NOT NULL, + kind TEXT NOT NULL, + key TEXT NOT NULL, + request_id TEXT NOT NULL, + access_token_signature TEXT NOT NULL DEFAULT '', + active BOOLEAN NOT NULL DEFAULT TRUE, + request_data BLOB NOT NULL, + expires_at DATETIME +); + +INSERT INTO oauth2_sessions_old ( + id, + created_at, + kind, + key, + request_id, + access_token_signature, + active, + request_data, + expires_at +) +SELECT + id, + created_at, + kind, + key, + request_id, + access_token_signature, + active, + request_data, + expires_at +FROM oauth2_sessions; + +DROP TABLE oauth2_sessions; +ALTER TABLE oauth2_sessions_old RENAME TO oauth2_sessions; + +CREATE UNIQUE INDEX idx_oauth2_sessions_kind_key ON oauth2_sessions (kind, key); +CREATE INDEX idx_oauth2_sessions_kind_request ON oauth2_sessions (kind, request_id); +CREATE INDEX idx_oauth2_sessions_expires_at ON oauth2_sessions (expires_at); + +COMMIT; +PRAGMA foreign_keys = ON; diff --git a/backend/resources/migrations/sqlite/20260731120000_oauth2_session_ownership.up.sql b/backend/resources/migrations/sqlite/20260731120000_oauth2_session_ownership.up.sql new file mode 100644 index 00000000..b84b8325 --- /dev/null +++ b/backend/resources/migrations/sqlite/20260731120000_oauth2_session_ownership.up.sql @@ -0,0 +1,60 @@ +PRAGMA foreign_keys = OFF; +BEGIN; + +CREATE TABLE oauth2_sessions_new ( + id TEXT NOT NULL PRIMARY KEY, + created_at DATETIME NOT NULL, + kind TEXT NOT NULL, + key TEXT NOT NULL, + request_id TEXT NOT NULL, + client_id TEXT NOT NULL REFERENCES oidc_clients(id) ON DELETE CASCADE, + access_token_signature TEXT NOT NULL DEFAULT '', + active BOOLEAN NOT NULL DEFAULT TRUE, + request_data BLOB NOT NULL, + expires_at DATETIME, + CONSTRAINT chk_oauth2_sessions_client_id + CHECK (client_id = json_extract(CAST(request_data AS TEXT), '$.client_id')) +); + +INSERT INTO oauth2_sessions_new ( + id, + created_at, + kind, + key, + request_id, + client_id, + access_token_signature, + active, + request_data, + expires_at +) +SELECT + id, + created_at, + kind, + key, + request_id, + json_extract(CAST(request_data AS TEXT), '$.client_id'), + access_token_signature, + active, + request_data, + expires_at +FROM oauth2_sessions +WHERE json_valid(CAST(request_data AS TEXT)) + AND EXISTS ( + SELECT 1 + FROM oidc_clients + WHERE oidc_clients.id = json_extract(CAST(oauth2_sessions.request_data AS TEXT), '$.client_id') + ); + +DROP TABLE oauth2_sessions; +ALTER TABLE oauth2_sessions_new RENAME TO oauth2_sessions; + +CREATE UNIQUE INDEX idx_oauth2_sessions_kind_key ON oauth2_sessions (kind, key); +CREATE INDEX idx_oauth2_sessions_kind_request ON oauth2_sessions (kind, request_id); +CREATE INDEX idx_oauth2_sessions_expires_at ON oauth2_sessions (expires_at); +CREATE INDEX idx_oauth2_sessions_client_subject + ON oauth2_sessions (client_id, json_extract(CAST(request_data AS TEXT), '$.session.subject'), kind, active); + +COMMIT; +PRAGMA foreign_keys = ON; diff --git a/tests/resources/export/database.json b/tests/resources/export/database.json index eaab7596..76df2888 100644 --- a/tests/resources/export/database.json +++ b/tests/resources/export/database.json @@ -5,6 +5,7 @@ "users", "user_groups", "oidc_clients", + "oauth2_sessions", "signup_tokens", "apis", "api_permissions", @@ -286,9 +287,10 @@ "kind": "access_token", "key": "cross-database-test-session", "request_id": "cross-database-test-request", + "client_id": "3654a746-35d4-4321-ac61-0bdcff2b4055", "access_token_signature": "", "active": true, - "request_data": "eyJyZXF1ZXN0IjoidmFsdWUifQ==", + "request_data": "eyJjbGllbnRfaWQiOiIzNjU0YTc0Ni0zNWQ0LTQzMjEtYWM2MS0wYmRjZmYyYjQwNTUiLCJzZXNzaW9uIjp7InN1YmplY3QiOiJmNGI4OWRjMi02MmZiLTQ2YmYtOWY1Zi1jMzRmNGVhZmU5M2UiLCJpZF90b2tlbl9jbGFpbXMiOnsianRpIjoiY3Jvc3MtZGF0YWJhc2UtdGVzdC1pZC10b2tlbi1qdGkifX19", "expires_at": "2099-01-01T00:00:00Z" } ],