mirror of
https://github.com/pocket-id/pocket-id.git
synced 2026-09-20 01:49:05 +02:00
fix: session revoke fails if orphaned session tokens exist
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -17,6 +17,7 @@ type OAuth2Session struct {
|
||||
Kind string
|
||||
Key string
|
||||
RequestID string
|
||||
ClientID string
|
||||
AccessTokenSignature string
|
||||
Active bool
|
||||
RequestData string
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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;
|
||||
@@ -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);
|
||||
@@ -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;
|
||||
@@ -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;
|
||||
@@ -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"
|
||||
}
|
||||
],
|
||||
|
||||
Reference in New Issue
Block a user