From 2075de32347f5b31c517298fb69efecbd49afada Mon Sep 17 00:00:00 2001 From: Alec Rubin <8213066+alecrubin@users.noreply.github.com> Date: Wed, 23 Sep 2026 15:34:38 -0400 Subject: [PATCH] feat: add OIDC back-channel logout (#1734) Co-authored-by: Alessandro (Ale) Segala <43508+ItalyPaleAle@users.noreply.github.com> Co-authored-by: Elias Schneider --- backend/internal/backchannellogout/actor.go | 67 ++++ backend/internal/backchannellogout/service.go | 263 +++++++++++++++ .../backchannellogout/service_test.go | 318 ++++++++++++++++++ .../internal/bootstrap/services_bootstrap.go | 29 +- .../controller/well_known_controller.go | 2 + backend/internal/dto/oidc_dto.go | 2 + backend/internal/dto/validations.go | 11 + backend/internal/dto/validations_test.go | 34 ++ backend/internal/ldapsync/module.go | 19 +- backend/internal/ldapsync/service.go | 172 +++++++--- backend/internal/ldapsync/service_test.go | 36 +- .../middleware/auth_middleware_test.go | 2 +- backend/internal/model/oidc.go | 1 + backend/internal/oidc/store.go | 2 + backend/internal/service/e2etest_service.go | 30 +- backend/internal/service/jwt_service.go | 47 +++ backend/internal/service/jwt_service_test.go | 40 +++ backend/internal/service/oidc_service.go | 57 +++- backend/internal/service/oidc_service_test.go | 28 +- .../internal/service/user_group_service.go | 80 ++++- backend/internal/service/user_service.go | 48 ++- backend/internal/service/user_service_test.go | 3 +- ...idc_client_backchannel_logout_url.down.sql | 1 + ..._oidc_client_backchannel_logout_url.up.sql | 1 + ...idc_client_backchannel_logout_url.down.sql | 1 + ..._oidc_client_backchannel_logout_url.up.sql | 1 + frontend/messages/en.json | 2 + frontend/src/lib/types/oidc.type.ts | 1 + .../oidc-clients/oidc-client-form.svelte | 26 +- tests/data.ts | 2 + tests/resources/export/database.json | 9 +- tests/setup/docker-compose.yml | 2 + tests/specs/oidc-backchannel-logout.spec.ts | 252 ++++++++++++++ 33 files changed, 1493 insertions(+), 96 deletions(-) create mode 100644 backend/internal/backchannellogout/actor.go create mode 100644 backend/internal/backchannellogout/service.go create mode 100644 backend/internal/backchannellogout/service_test.go create mode 100644 backend/resources/migrations/postgres/20260923183637_add_oidc_client_backchannel_logout_url.down.sql create mode 100644 backend/resources/migrations/postgres/20260923183637_add_oidc_client_backchannel_logout_url.up.sql create mode 100644 backend/resources/migrations/sqlite/20260923183637_add_oidc_client_backchannel_logout_url.down.sql create mode 100644 backend/resources/migrations/sqlite/20260923183637_add_oidc_client_backchannel_logout_url.up.sql create mode 100644 tests/specs/oidc-backchannel-logout.spec.ts diff --git a/backend/internal/backchannellogout/actor.go b/backend/internal/backchannellogout/actor.go new file mode 100644 index 00000000..4d87670c --- /dev/null +++ b/backend/internal/backchannellogout/actor.go @@ -0,0 +1,67 @@ +package backchannellogout + +import ( + "context" + "fmt" + "log/slog" + + "github.com/italypaleale/francis/actor" +) + +// ActorType is the actor type that delivers back-channel logout tokens as durable jobs +const ActorType = "BackchannelLogoutNotifier" + +// methodDeliver is the job method that delivers one logout token to one client +const methodDeliver = "deliver" + +const ( + // deliveryConcurrency caps running delivery jobs per host without letting idle actors hold the available capacity + deliveryConcurrency = 4 + + // deliveryMaxAttempts caps the delivery attempts for one notification, so an unreachable client is not retried forever + deliveryMaxAttempts = 3 +) + +// notifierActor delivers the logout tokens scheduled as jobs +// Jobs make the notifications durable: they survive a restart and failed deliveries are retried up to deliveryMaxAttempts times +type notifierActor struct { + service *Service +} + +func (s *Service) newNotifierActor(_ string, _ *actor.Service) actor.Actor { + return ¬ifierActor{service: s} +} + +// Job implements actor.ActorJob +func (a *notifierActor) Job(ctx context.Context, method string, data actor.Envelope) error { + if method != methodDeliver { + return fmt.Errorf("%w: unsupported method '%s'", actor.ErrJobPermanentFailure, method) + } + if data == nil { + return fmt.Errorf("%w: job input is empty", actor.ErrJobPermanentFailure) + } + + var t target + err := data.Decode(&t) + if err != nil { + return fmt.Errorf("%w: job input is not a valid target: %w", actor.ErrJobPermanentFailure, err) + } + + return a.service.sendLogoutToken(ctx, t) +} + +// JobFailed implements actor.ActorJobFailed +func (a *notifierActor) JobFailed(ctx context.Context, _ string, _ string, data actor.Envelope, jobErr error) error { + var t target + if data != nil { + _ = data.Decode(&t) + } + + slog.ErrorContext(ctx, "Giving up on delivering back-channel logout token", + slog.String("clientId", t.ClientID), + slog.String("userId", t.UserID), + slog.String("logoutUrl", t.LogoutURL), + slog.Any("error", jobErr), + ) + return nil +} diff --git a/backend/internal/backchannellogout/service.go b/backend/internal/backchannellogout/service.go new file mode 100644 index 00000000..9dab45ba --- /dev/null +++ b/backend/internal/backchannellogout/service.go @@ -0,0 +1,263 @@ +package backchannellogout + +import ( + "context" + "fmt" + "log/slog" + "net/http" + "net/url" + "strings" + "time" + + "github.com/italypaleale/francis/actor" + francishost "github.com/italypaleale/francis/host" + "github.com/italypaleale/francis/host/local" + "gorm.io/gorm" + + "github.com/pocket-id/pocket-id/backend/internal/model" +) + +// requestTimeout bounds each notification POST, so one unreachable client cannot stall the others +const requestTimeout = 10 * time.Second + +// TokenSigner mints the logout tokens delivered to clients +type TokenSigner interface { + GenerateLogoutToken(userID string, clientID string) (string, error) +} + +// Service sends OIDC Back-Channel Logout 1.0 tokens to clients when a user's access is revoked +// Deliveries are scheduled as durable jobs, so they survive a restart and failed attempts are retried a capped number of times before giving up +type Service struct { + db *gorm.DB + tokenSigner TokenSigner + httpClient *http.Client + actors *actor.Service +} + +func NewService(db *gorm.DB, tokenSigner TokenSigner, httpClient *http.Client, actorsHost francishost.Host) (*Service, error) { + s := &Service{ + db: db, + tokenSigner: tokenSigner, + httpClient: newHTTPClient(httpClient), + actors: actorsHost.Service(), + } + + err := actorsHost.RegisterActor( + ActorType, + s.newNotifierActor, + local.WithCapacityGroup(ActorType, deliveryConcurrency), + local.WithMaxAttempts(deliveryMaxAttempts), + ) + if err != nil { + return nil, fmt.Errorf("error registering the %s actor: %w", ActorType, err) + } + + return s, nil +} + +// newHTTPClient refuses to follow redirects, as Go would turn the POST into a body-less GET and the logout token would be silently dropped +// Returning the redirect response instead makes the delivery fail loudly on the status check +func newHTTPClient(source *http.Client) *http.Client { + if source == nil { + source = http.DefaultClient + } + + return &http.Client{ + Transport: source.Transport, + CheckRedirect: func(_ *http.Request, _ []*http.Request) error { + return http.ErrUseLastResponse + }, + } +} + +// target is a single client to notify that a user's session should end +type target struct { + UserID string + ClientID string + LogoutURL string +} + +// targetsQuery selects the authorizations of clients that are registered for back-channel logout +// Callers narrow it down to the users or the client whose access was revoked +func (s *Service) targetsQuery(ctx context.Context, tx *gorm.DB) *gorm.DB { + return tx. + WithContext(ctx). + Model(&model.UserAuthorizedOidcClient{}). + Select("user_authorized_oidc_clients.user_id", "user_authorized_oidc_clients.client_id", "oidc_clients.backchannel_logout_url AS logout_url"). + Joins("JOIN oidc_clients ON oidc_clients.id = user_authorized_oidc_clients.client_id"). + Where("oidc_clients.backchannel_logout_url <> ''") +} + +// targetsForUsers returns every client the given users have authorized that is registered for back-channel logout +func (s *Service) targetsForUsers(ctx context.Context, tx *gorm.DB, userIDs []string) ([]target, error) { + if len(userIDs) == 0 { + return nil, nil + } + + var targets []target + err := s.targetsQuery(ctx, tx). + Where("user_authorized_oidc_clients.user_id IN (?)", userIDs). + Scan(&targets). + Error + if err != nil { + return nil, err + } + return targets, nil +} + +// targetsForAuthorization returns the client to notify when a single authorization is revoked, and nothing when that client is not registered for back-channel logout +func (s *Service) targetsForAuthorization(ctx context.Context, tx *gorm.DB, userID string, clientID string) ([]target, error) { + var targets []target + err := s.targetsQuery(ctx, tx). + Where("user_authorized_oidc_clients.user_id = ?", userID). + Where("user_authorized_oidc_clients.client_id = ?", clientID). + Scan(&targets). + Error + if err != nil { + return nil, err + } + return targets, nil +} + +// targetsForClient returns every user who has authorized the given client, when that client is registered for back-channel logout +func (s *Service) targetsForClient(ctx context.Context, tx *gorm.DB, clientID string) ([]target, error) { + var targets []target + err := s.targetsQuery(ctx, tx). + Where("user_authorized_oidc_clients.client_id = ?", clientID). + Scan(&targets). + Error + if err != nil { + return nil, err + } + return targets, nil +} + +// targetsForLostGroupAccess returns clients registered for back-channel logout that the matched users have authorized but can no longer access because of the client's group restriction +// It must run after the group membership or allowed-group change has been committed +func (s *Service) targetsForLostGroupAccess(ctx context.Context, tx *gorm.DB, userIDs []string, clientID string) ([]target, error) { + // Require at least one filter, so a caller that computes an empty user list can never match every user of every client + if len(userIDs) == 0 && clientID == "" { + return nil, nil + } + + query := s.targetsQuery(ctx, tx). + Where("oidc_clients.is_group_restricted = ?", true). + Where("NOT EXISTS (SELECT 1 FROM oidc_clients_allowed_user_groups ag JOIN user_groups_users ugu ON ugu.user_group_id = ag.user_group_id WHERE ag.oidc_client_id = oidc_clients.id AND ugu.user_id = user_authorized_oidc_clients.user_id)") + if len(userIDs) > 0 { + query = query.Where("user_authorized_oidc_clients.user_id IN (?)", userIDs) + } + if clientID != "" { + query = query.Where("user_authorized_oidc_clients.client_id = ?", clientID) + } + + var targets []target + err := query.Scan(&targets).Error + if err != nil { + return nil, err + } + return targets, nil +} + +// PrepareUserNotifications resolves, within the given transaction, the logout notifications for users whose access is being revoked +// It exists for callers that delete the users or their authorizations, which are gone once the transaction commits +// The returned function delivers the notifications in the background and must only be called after the transaction has committed +// It is never nil, so callers that treat a failed lookup as non-fatal can call it unconditionally +func (s *Service) PrepareUserNotifications(ctx context.Context, tx *gorm.DB, userIDs []string) (func(), error) { + targets, err := s.targetsForUsers(ctx, tx, userIDs) + if err != nil { + return func() {}, err + } + return func() { s.notifyClients(ctx, targets) }, nil +} + +// PrepareAuthorizationNotification resolves, within the given transaction, the logout notification for a single authorization that is being revoked +// The returned function behaves like the one from PrepareUserNotifications +func (s *Service) PrepareAuthorizationNotification(ctx context.Context, tx *gorm.DB, userID string, clientID string) (func(), error) { + targets, err := s.targetsForAuthorization(ctx, tx, userID, clientID) + if err != nil { + return func() {}, err + } + return func() { s.notifyClients(ctx, targets) }, nil +} + +// PrepareClientNotifications resolves, within the given transaction, the logout notifications for every user of a client that is being deleted +// The returned function behaves like the one from PrepareUserNotifications +func (s *Service) PrepareClientNotifications(ctx context.Context, tx *gorm.DB, clientID string) (func(), error) { + targets, err := s.targetsForClient(ctx, tx, clientID) + if err != nil { + return func() {}, err + } + return func() { s.notifyClients(ctx, targets) }, nil +} + +// NotifyUser delivers logout tokens to every client the user has authorized that is registered for back-channel logout +// It must be called after the change that revoked the user's access has been committed, and logs instead of failing because delivery is best effort +func (s *Service) NotifyUser(ctx context.Context, userID string) { + targets, err := s.targetsForUsers(ctx, s.db, []string{userID}) + if err != nil { + slog.ErrorContext(ctx, "Failed to find clients to notify for back-channel logout", slog.String("userId", userID), slog.Any("error", err)) + return + } + s.notifyClients(ctx, targets) +} + +// NotifyLostGroupAccess delivers logout tokens for group-restricted clients that the matched users can no longer access +// Callers pass the users whose membership changed, the client whose allowed groups changed, or both to narrow the match +// It must be called after the group change has been committed, and logs instead of failing because delivery is best effort +func (s *Service) NotifyLostGroupAccess(ctx context.Context, userIDs []string, clientID string) { + targets, err := s.targetsForLostGroupAccess(ctx, s.db, userIDs, clientID) + if err != nil { + slog.ErrorContext(ctx, "Failed to find clients to notify for back-channel logout", slog.Any("error", err)) + return + } + s.notifyClients(ctx, targets) +} + +// notifyClients schedules a durable delivery job for each of the given clients, so callers are never blocked on slow or unreachable clients +// It must be called after the change that revoked the user's access has been committed +func (s *Service) notifyClients(ctx context.Context, targets []target) { + for _, t := range targets { + // One actor per authorization serializes its deliveries, while the capacity group limits running jobs without counting idle actors + actorID := t.ClientID + ":" + t.UserID + _, _, err := s.actors.Dispatch(ctx, ActorType, actorID, methodDeliver, t) + if err != nil { + slog.ErrorContext(ctx, "Failed to schedule back-channel logout notification", + slog.String("clientId", t.ClientID), + slog.String("userId", t.UserID), + slog.Any("error", err), + ) + } + } +} + +func (s *Service) sendLogoutToken(parentCtx context.Context, t target) error { + logoutToken, err := s.tokenSigner.GenerateLogoutToken(t.UserID, t.ClientID) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(parentCtx, requestTimeout) + defer cancel() + + body := url.Values{"logout_token": []string{logoutToken}} + req, err := http.NewRequestWithContext(ctx, http.MethodPost, t.LogoutURL, strings.NewReader(body.Encode())) + if err != nil { + return fmt.Errorf("%w: invalid logout request: %w", actor.ErrJobPermanentFailure, err) + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + res, err := s.httpClient.Do(req) + if err != nil { + return err + } + defer res.Body.Close() + + if res.StatusCode < 200 || res.StatusCode > 299 { + // Only retry responses that indicate a potentially temporary failure at the client + if res.StatusCode != http.StatusRequestTimeout && res.StatusCode != http.StatusTooManyRequests && res.StatusCode < http.StatusInternalServerError { + return fmt.Errorf("%w: client responded with status %d", actor.ErrJobPermanentFailure, res.StatusCode) + } + return fmt.Errorf("client responded with status %d", res.StatusCode) + } + return nil +} diff --git a/backend/internal/backchannellogout/service_test.go b/backend/internal/backchannellogout/service_test.go new file mode 100644 index 00000000..4b988837 --- /dev/null +++ b/backend/internal/backchannellogout/service_test.go @@ -0,0 +1,318 @@ +package backchannellogout + +import ( + "errors" + "net/http" + "net/http/httptest" + "strconv" + "testing" + "time" + + "github.com/italypaleale/francis/actor" + "github.com/italypaleale/francis/host/local" + "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" +) + +// stubSigner returns a recognizable token so delivery tests can assert what was posted without a real key +// Logout token contents are covered by the JWT service's own tests +type stubSigner struct{} + +func (stubSigner) GenerateLogoutToken(userID string, clientID string) (string, error) { + return "logout-token-" + userID + "-" + clientID, nil +} + +func seedFixtures(t *testing.T, db *gorm.DB) { + t.Helper() + + require.NoError(t, db.Create(&model.User{Base: model.Base{ID: "user-1"}, Username: "user1"}).Error) + require.NoError(t, db.Create(&model.User{Base: model.Base{ID: "user-2"}, Username: "user2"}).Error) + + group := model.UserGroup{Base: model.Base{ID: "group-1"}, Name: "group1", FriendlyName: "Group 1"} + require.NoError(t, db.Create(&group).Error) + require.NoError(t, db.Model(&group).Association("Users").Append(&model.User{Base: model.Base{ID: "user-1"}})) + + // An unrestricted client with a back-channel logout URL + require.NoError(t, db.Create(&model.OidcClient{ + Base: model.Base{ID: "client-open"}, + Name: "Open Client", + BackchannelLogoutURL: "https://open.example.com/logout", + }).Error) + + // An unrestricted client without a back-channel logout URL + require.NoError(t, db.Create(&model.OidcClient{ + Base: model.Base{ID: "client-silent"}, + Name: "Silent Client", + }).Error) + + // A group-restricted client that allows group-1 + restricted := model.OidcClient{ + Base: model.Base{ID: "client-restricted"}, + Name: "Restricted Client", + IsGroupRestricted: true, + BackchannelLogoutURL: "https://restricted.example.com/logout", + } + require.NoError(t, db.Create(&restricted).Error) + require.NoError(t, db.Model(&restricted).Association("AllowedUserGroups").Append(&model.UserGroup{Base: model.Base{ID: "group-1"}})) + + for _, clientID := range []string{"client-open", "client-silent", "client-restricted"} { + require.NoError(t, db.Create(&model.UserAuthorizedOidcClient{UserID: "user-1", ClientID: clientID}).Error) + } + require.NoError(t, db.Create(&model.UserAuthorizedOidcClient{UserID: "user-2", ClientID: "client-restricted"}).Error) +} + +func TestService_TargetsForUsers(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + seedFixtures(t, db) + s := &Service{db: db} + + targets, err := s.targetsForUsers(t.Context(), db, []string{"user-1"}) + require.NoError(t, err) + + // The client without a back-channel logout URL must not be returned + require.Len(t, targets, 2) + byClient := map[string]target{} + for _, tgt := range targets { + byClient[tgt.ClientID] = tgt + } + assert.Equal(t, "https://open.example.com/logout", byClient["client-open"].LogoutURL) + assert.Equal(t, "https://restricted.example.com/logout", byClient["client-restricted"].LogoutURL) + assert.Equal(t, "user-1", byClient["client-open"].UserID) +} + +func TestService_TargetsForAuthorization(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + seedFixtures(t, db) + s := &Service{db: db} + + t.Run("returns the authorized client", func(t *testing.T) { + targets, err := s.targetsForAuthorization(t.Context(), db, "user-1", "client-open") + require.NoError(t, err) + require.Len(t, targets, 1) + assert.Equal(t, "user-1", targets[0].UserID) + assert.Equal(t, "https://open.example.com/logout", targets[0].LogoutURL) + }) + + t.Run("returns nothing for a client without a back-channel logout URL", func(t *testing.T) { + targets, err := s.targetsForAuthorization(t.Context(), db, "user-1", "client-silent") + require.NoError(t, err) + assert.Empty(t, targets) + }) + + t.Run("returns nothing for a client the user has not authorized", func(t *testing.T) { + targets, err := s.targetsForAuthorization(t.Context(), db, "user-2", "client-open") + require.NoError(t, err) + assert.Empty(t, targets) + }) +} + +func TestService_TargetsForClient(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + seedFixtures(t, db) + s := &Service{db: db} + + t.Run("returns every user who authorized the client", func(t *testing.T) { + targets, err := s.targetsForClient(t.Context(), db, "client-restricted") + require.NoError(t, err) + require.Len(t, targets, 2) + userIDs := []string{targets[0].UserID, targets[1].UserID} + assert.ElementsMatch(t, []string{"user-1", "user-2"}, userIDs) + assert.Equal(t, "https://restricted.example.com/logout", targets[0].LogoutURL) + }) + + t.Run("returns nothing for a client without a back-channel logout URL", func(t *testing.T) { + targets, err := s.targetsForClient(t.Context(), db, "client-silent") + require.NoError(t, err) + assert.Empty(t, targets) + }) +} + +func TestService_TargetsForLostGroupAccess(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + seedFixtures(t, db) + s := &Service{db: db} + + t.Run("member of an allowed group is not returned", func(t *testing.T) { + targets, err := s.targetsForLostGroupAccess(t.Context(), db, []string{"user-1"}, "") + require.NoError(t, err) + assert.Empty(t, targets) + }) + + t.Run("user outside every allowed group is returned for restricted clients only", func(t *testing.T) { + targets, err := s.targetsForLostGroupAccess(t.Context(), db, []string{"user-2"}, "") + require.NoError(t, err) + require.Len(t, targets, 1) + assert.Equal(t, "client-restricted", targets[0].ClientID) + assert.Equal(t, "user-2", targets[0].UserID) + assert.Equal(t, "https://restricted.example.com/logout", targets[0].LogoutURL) + }) + + t.Run("client filter matches all users that lost access", func(t *testing.T) { + targets, err := s.targetsForLostGroupAccess(t.Context(), db, nil, "client-restricted") + require.NoError(t, err) + require.Len(t, targets, 1) + assert.Equal(t, "user-2", targets[0].UserID) + }) + + t.Run("no filters matches nothing instead of every user", func(t *testing.T) { + targets, err := s.targetsForLostGroupAccess(t.Context(), db, nil, "") + require.NoError(t, err) + assert.Empty(t, targets) + }) + + t.Run("removing the user from the group makes their restricted client a target", func(t *testing.T) { + require.NoError(t, db.Model(&model.UserGroup{Base: model.Base{ID: "group-1"}}).Association("Users").Clear()) + targets, err := s.targetsForLostGroupAccess(t.Context(), db, []string{"user-1"}, "") + require.NoError(t, err) + require.Len(t, targets, 1) + assert.Equal(t, "client-restricted", targets[0].ClientID) + }) +} + +func TestService_PrepareUserNotifications(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + seedFixtures(t, db) + s := &Service{db: db} + + t.Run("returns a notify function for a user with targets", func(t *testing.T) { + notify, err := s.PrepareUserNotifications(t.Context(), db, []string{"user-1"}) + require.NoError(t, err) + require.NotNil(t, notify) + }) + + t.Run("returns a notify function that delivers nothing for a user without targets", func(t *testing.T) { + require.NoError(t, db.Create(&model.User{Base: model.Base{ID: "user-3"}, Username: "user3"}).Error) + notify, err := s.PrepareUserNotifications(t.Context(), db, []string{"user-3"}) + require.NoError(t, err) + require.NotNil(t, notify) + + // There is nothing to send, so calling the function must be a no-op + assert.NotPanics(t, notify) + }) +} + +func TestService_sendLogoutToken_refusesRedirects(t *testing.T) { + // Following the redirect would turn the POST into a GET without the logout token, so it must be reported as a failure + var redirectTargetHit bool + redirectTarget := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + redirectTargetHit = true + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(redirectTarget.Close) + redirecting := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, redirectTarget.URL, http.StatusFound) + })) + t.Cleanup(redirecting.Close) + + s := &Service{tokenSigner: stubSigner{}, httpClient: newHTTPClient(redirecting.Client())} + err := s.sendLogoutToken(t.Context(), target{UserID: "user-1", ClientID: "client-1", LogoutURL: redirecting.URL}) + require.ErrorContains(t, err, "302") + assert.False(t, redirectTargetHit) +} + +func TestService_notifyClients(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + + const clientCount = 8 + received := make(chan string, clientCount) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "application/x-www-form-urlencoded", r.Header.Get("Content-Type")) + assert.NoError(t, r.ParseForm()) + received <- r.PostForm.Get("logout_token") + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(server.Close) + + // A failing target must not prevent delivery to the remaining targets + failingServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusBadRequest) + })) + t.Cleanup(failingServer.Close) + + var s *Service + testutils.NewActorHostForTest(t, func(t *testing.T, h *local.Host) { + var err error + s, err = NewService(db, stubSigner{}, server.Client(), h) + require.NoError(t, err) + }) + + // Exceed the capacity group so idle actors cannot prevent later clients from receiving logout + targets := []target{{UserID: "user-1", ClientID: "client-fail", LogoutURL: failingServer.URL}} + expected := make([]string, 0, clientCount) + for i := range clientCount { + clientID := "client-" + strconv.Itoa(i) + targets = append(targets, target{UserID: "user-1", ClientID: clientID, LogoutURL: server.URL}) + expected = append(expected, "logout-token-user-1-"+clientID) + } + s.notifyClients(t.Context(), targets) + + // All recipients must receive their own token before the actors' idle timeout can release capacity + timeout := time.NewTimer(10 * time.Second) + defer timeout.Stop() + actual := make([]string, 0, clientCount) + for range clientCount { + select { + case logoutToken := <-received: + actual = append(actual, logoutToken) + case <-timeout.C: + t.Fatalf("received %d of %d logout tokens", len(actual), clientCount) + } + } + assert.ElementsMatch(t, expected, actual) +} + +func TestService_sendLogoutToken_responseClassification(t *testing.T) { + for _, test := range []struct { + status int + permanent bool + }{ + {status: http.StatusOK}, + {status: http.StatusNoContent}, + {status: http.StatusFound, permanent: true}, + {status: http.StatusBadRequest, permanent: true}, + {status: http.StatusUnauthorized, permanent: true}, + {status: http.StatusForbidden, permanent: true}, + {status: http.StatusNotFound, permanent: true}, + {status: http.StatusRequestTimeout}, + {status: http.StatusTooManyRequests}, + {status: http.StatusInternalServerError}, + {status: http.StatusBadGateway}, + {status: http.StatusServiceUnavailable}, + {status: http.StatusGatewayTimeout}, + } { + t.Run(strconv.Itoa(test.status), func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(test.status) + })) + t.Cleanup(server.Close) + s := &Service{tokenSigner: stubSigner{}, httpClient: newHTTPClient(server.Client())} + + err := s.sendLogoutToken(t.Context(), target{UserID: "user-1", ClientID: "client-1", LogoutURL: server.URL}) + if test.status >= 200 && test.status < 300 { + require.NoError(t, err) + return + } + require.Error(t, err) + assert.Equal(t, test.permanent, errors.Is(err, actor.ErrJobPermanentFailure)) + }) + } +} + +func TestService_sendLogoutToken_networkFailureIsRetryable(t *testing.T) { + transportErr := errors.New("connection reset") + s := &Service{ + tokenSigner: stubSigner{}, + httpClient: newHTTPClient(&http.Client{ + Transport: &testutils.MockRoundTripper{Err: transportErr}, + }), + } + + err := s.sendLogoutToken(t.Context(), target{UserID: "user-1", ClientID: "client-1", LogoutURL: "https://rp.example/logout"}) + require.ErrorIs(t, err, transportErr) + assert.NotErrorIs(t, err, actor.ErrJobPermanentFailure) +} diff --git a/backend/internal/bootstrap/services_bootstrap.go b/backend/internal/bootstrap/services_bootstrap.go index 6a732d76..5c392dec 100644 --- a/backend/internal/bootstrap/services_bootstrap.go +++ b/backend/internal/bootstrap/services_bootstrap.go @@ -10,6 +10,7 @@ import ( "github.com/pocket-id/pocket-id/backend/internal/apikey" "github.com/pocket-id/pocket-id/backend/internal/appconfig" "github.com/pocket-id/pocket-id/backend/internal/auditlogs" + "github.com/pocket-id/pocket-id/backend/internal/backchannellogout" "github.com/pocket-id/pocket-id/backend/internal/common" "github.com/pocket-id/pocket-id/backend/internal/devicelogin" "github.com/pocket-id/pocket-id/backend/internal/email" @@ -174,23 +175,29 @@ func initServices( return nil, fmt.Errorf("failed to create OIDC module: %w", err) } - svc.oidcService, err = service.NewOidcService(db, svc.jwtService, svc.oidcModule.Preview, svc.oidcModule, svc.scimSyncModule, httpClient, fileStorage) + backchannelLogoutService, err := backchannellogout.NewService(db, svc.jwtService, httpClient, actors) + if err != nil { + return nil, fmt.Errorf("failed to create back-channel logout service: %w", err) + } + + svc.oidcService, err = service.NewOidcService(db, svc.jwtService, svc.oidcModule.Preview, svc.oidcModule, svc.scimSyncModule, backchannelLogoutService, httpClient, fileStorage) if err != nil { return nil, fmt.Errorf("failed to create OIDC service: %w", err) } - svc.userGroupService = service.NewUserGroupService(db, svc.scimSyncModule) - svc.userService = service.NewUserService(db, svc.jwtService, svc.auditLogService, svc.customClaimService, svc.appImagesService, svc.scimSyncModule, fileStorage) + svc.userGroupService = service.NewUserGroupService(db, svc.scimSyncModule, backchannelLogoutService) + svc.userService = service.NewUserService(db, svc.jwtService, svc.auditLogService, svc.customClaimService, svc.appImagesService, svc.scimSyncModule, backchannelLogoutService, fileStorage) svc.ldapSyncModule, err = ldapsync.New(ldapsync.Dependencies{ - DB: db, - Actors: actors, - HTTPClient: httpClient, - FileStorage: fileStorage, - Users: svc.userService, - Groups: svc.userGroupService, - AppConfig: svc.appConfigService, - ScimSync: svc.scimSyncModule, + DB: db, + Actors: actors, + HTTPClient: httpClient, + FileStorage: fileStorage, + Users: svc.userService, + Groups: svc.userGroupService, + AppConfig: svc.appConfigService, + ScimSync: svc.scimSyncModule, + BackchannelLogout: backchannelLogoutService, // Disable in test environment ScheduleDisabled: common.EnvConfig.AppEnv.IsTest(), }) diff --git a/backend/internal/controller/well_known_controller.go b/backend/internal/controller/well_known_controller.go index 0c318e28..187847ba 100644 --- a/backend/internal/controller/well_known_controller.go +++ b/backend/internal/controller/well_known_controller.go @@ -122,6 +122,8 @@ func (wkc *WellKnownController) computeServerMetadata() ([]byte, error) { "token_endpoint_auth_methods_supported": []string{"client_secret_basic", "client_secret_post", "none"}, "pushed_authorization_request_endpoint": internalAppUrl + "/api/oidc/par", "require_pushed_authorization_requests": false, + "backchannel_logout_supported": true, + "backchannel_logout_session_supported": false, "client_id_metadata_document_supported": cimdSupported, "service_documentation": "https://pocket-id.org/docs", } diff --git a/backend/internal/dto/oidc_dto.go b/backend/internal/dto/oidc_dto.go index c4197229..4bfe3ffd 100644 --- a/backend/internal/dto/oidc_dto.go +++ b/backend/internal/dto/oidc_dto.go @@ -21,6 +21,7 @@ type OidcClientDto struct { OidcClientMetaDataDto CallbackURLs []string `json:"callbackURLs"` LogoutCallbackURLs []string `json:"logoutCallbackURLs"` + BackchannelLogoutURL string `json:"backchannelLogoutURL"` IsPublic bool `json:"isPublic"` PkceEnabled bool `json:"pkceEnabled"` RequiresPushedAuthorizationRequests bool `json:"requiresPushedAuthorizationRequests"` @@ -47,6 +48,7 @@ type OidcClientUpdateDto struct { Description string `json:"description" binding:"omitempty,max=150" unorm:"nfc"` CallbackURLs []string `json:"callbackURLs" binding:"omitempty,dive,callback_url_pattern"` LogoutCallbackURLs []string `json:"logoutCallbackURLs" binding:"omitempty,dive,callback_url_pattern"` + BackchannelLogoutURL string `json:"backchannelLogoutURL" binding:"omitempty,http_url,backchannel_logout_url"` IsPublic bool `json:"isPublic"` PkceEnabled bool `json:"pkceEnabled"` RequiresReauthentication bool `json:"requiresReauthentication"` diff --git a/backend/internal/dto/validations.go b/backend/internal/dto/validations.go index 552d6b7d..3265c384 100644 --- a/backend/internal/dto/validations.go +++ b/backend/internal/dto/validations.go @@ -62,6 +62,15 @@ func init() { "callback_url_pattern": func(fl validator.FieldLevel) bool { return ValidateCallbackURLPattern(fl.Field().String()) }, + "backchannel_logout_url": func(fl validator.FieldLevel) bool { + // Back-Channel Logout §2.2 forbids fragments and permits HTTP only for confidential clients + raw := fl.Field().String() + u, err := url.Parse(raw) + if err != nil || strings.Contains(raw, "#") { + return false + } + return u.Scheme == "https" || !fl.Parent().FieldByName("IsPublic").Bool() + }, "resource_uri": func(fl validator.FieldLevel) bool { return ValidateResourceURI(fl.Field().String()) }, @@ -157,6 +166,8 @@ func ValidationErrorDetails(validationError validator.FieldError) (string, strin return "invalid_format", "must be a valid URL" case "resource_uri": return "invalid_format", "must be an absolute URI without whitespace or a fragment" + case "backchannel_logout_url": + return "invalid_format", "must not contain a fragment and must use HTTPS for public clients" case "min": return "too_short", fmt.Sprintf("must be at least %s characters long", validationError.Param()) case "max": diff --git a/backend/internal/dto/validations_test.go b/backend/internal/dto/validations_test.go index 6047b088..772bfc38 100644 --- a/backend/internal/dto/validations_test.go +++ b/backend/internal/dto/validations_test.go @@ -230,3 +230,37 @@ func TestValidateCallbackURLPattern(t *testing.T) { }) } } + +func TestBackchannelLogoutURLValidation(t *testing.T) { + for _, test := range []struct { + name string + url string + isPublic bool + wantErr bool + }{ + {name: "omitted for confidential client"}, + {name: "omitted for public client", isPublic: true}, + {name: "HTTPS for confidential client", url: "https://rp.example/logout"}, + {name: "HTTPS for public client", url: "https://rp.example/logout", isPublic: true}, + {name: "HTTP for confidential client", url: "http://rp.example:8080/logout?tenant=test"}, + {name: "HTTP for public client", url: "http://rp.example/logout", isPublic: true, wantErr: true}, + {name: "fragment", url: "https://rp.example/logout#fragment", wantErr: true}, + {name: "empty fragment", url: "https://rp.example/logout#", wantErr: true}, + {name: "encoded hash in query", url: "https://rp.example/logout?tenant=%23test", isPublic: true}, + {name: "relative URL", url: "/logout", wantErr: true}, + {name: "missing host", url: "https:///logout", wantErr: true}, + {name: "unsupported scheme", url: "ftp://rp.example/logout", wantErr: true}, + } { + t.Run(test.name, func(t *testing.T) { + input := OidcClientUpdateDto{Name: "Test Client", BackchannelLogoutURL: test.url, IsPublic: test.isPublic} + for _, dto := range []any{input, OidcClientCreateDto{OidcClientUpdateDto: input}} { + err := binding.Validator.ValidateStruct(dto) + if test.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + } + }) + } +} diff --git a/backend/internal/ldapsync/module.go b/backend/internal/ldapsync/module.go index ae71769c..422f7b21 100644 --- a/backend/internal/ldapsync/module.go +++ b/backend/internal/ldapsync/module.go @@ -41,16 +41,27 @@ type ScimSyncScheduler interface { ScheduleSync(ctx context.Context) } +// BackchannelLogoutNotifier tells OIDC clients to end their sessions for users the sync deprovisions +type BackchannelLogoutNotifier interface { + // PrepareUserNotifications resolves the notifications within the sync transaction and returns a function that delivers them, which must only be called after the transaction has committed + PrepareUserNotifications(ctx context.Context, tx *gorm.DB, userIDs []string) (func(), error) + + // NotifyLostGroupAccess delivers logout tokens to group-restricted clients the given users can no longer access, and must be called after the transaction has committed + // The sync passes an empty client ID, matching on the users alone + NotifyLostGroupAccess(ctx context.Context, userIDs []string, clientID string) +} + type Dependencies struct { DB *gorm.DB Actors francishost.Host HTTPClient *http.Client FileStorage storage.FileStorage - Users UserSyncer - Groups GroupSyncer - AppConfig appconfig.AppConfigResolver - ScimSync ScimSyncScheduler + Users UserSyncer + Groups GroupSyncer + AppConfig appconfig.AppConfigResolver + ScimSync ScimSyncScheduler + BackchannelLogout BackchannelLogoutNotifier // ScheduleDisabled keeps the recurring sync from being armed // It's set in the test environment, where syncs are driven explicitly by the end-to-end tests diff --git a/backend/internal/ldapsync/service.go b/backend/internal/ldapsync/service.go index 8bd64abc..32067805 100644 --- a/backend/internal/ldapsync/service.go +++ b/backend/internal/ldapsync/service.go @@ -8,9 +8,11 @@ import ( "fmt" "io" "log/slog" + "maps" "net/http" "net/url" "path" + "slices" "strings" "time" "unicode/utf8" @@ -31,13 +33,14 @@ import ( // Service performs the actual LDAP synchronization // It is deliberately free of any actor concern: the sync actor only decides when a sync runs, while the reconciliation logic lives here and is called directly by the manual "sync now" endpoint too type Service struct { - db *gorm.DB - httpClient *http.Client - users UserSyncer - groups GroupSyncer - scimSync ScimSyncScheduler - fileStorage storage.FileStorage - clientFactory func(dbConfig *appconfig.AppConfigModel) (ldapClient, error) + db *gorm.DB + httpClient *http.Client + users UserSyncer + groups GroupSyncer + scimSync ScimSyncScheduler + backchannelLogout BackchannelLogoutNotifier + fileStorage storage.FileStorage + clientFactory func(dbConfig *appconfig.AppConfigModel) (ldapClient, error) } type savePicture struct { @@ -73,12 +76,13 @@ type ldapClient interface { func newService(deps Dependencies) *Service { service := &Service{ - db: deps.DB, - httpClient: deps.HTTPClient, - users: deps.Users, - groups: deps.Groups, - scimSync: deps.ScimSync, - fileStorage: deps.FileStorage, + db: deps.DB, + httpClient: deps.HTTPClient, + users: deps.Users, + groups: deps.Groups, + scimSync: deps.ScimSync, + backchannelLogout: deps.BackchannelLogout, + fileStorage: deps.FileStorage, } service.clientFactory = service.createClient @@ -129,13 +133,13 @@ func (s *Service) SyncAll(ctx context.Context, dbConfig *appconfig.AppConfigMode defer tx.Rollback() // Reconcile users - savePictures, deleteFiles, err := s.reconcileUsers(ctx, tx, desiredState.users, desiredState.userIDs, dbConfig) + savePictures, deleteFiles, notifyLogout, err := s.reconcileUsers(ctx, tx, desiredState.users, desiredState.userIDs, dbConfig) if err != nil { return fmt.Errorf("failed to sync users: %w", err) } // Reconcile groups - err = s.reconcileGroups(ctx, tx, desiredState.groups, desiredState.groupIDs, dbConfig) + usersRemovedFromGroups, err := s.reconcileGroups(ctx, tx, desiredState.groups, desiredState.groupIDs, dbConfig) if err != nil { return fmt.Errorf("failed to sync groups: %w", err) } @@ -151,6 +155,12 @@ func (s *Service) SyncAll(ctx context.Context, dbConfig *appconfig.AppConfigMode s.scimSync.ScheduleSync(ctx) } + // Tell OIDC clients to end the sessions of users the sync deprovisioned or removed from a group, now that the transaction has committed + notifyLogout() + if s.backchannelLogout != nil { + s.backchannelLogout.NotifyLostGroupAccess(ctx, usersRemovedFromGroups, "") + } + // Now that we've committed the transaction, we can perform operations on the storage layer // First, save all new pictures for _, sp := range savePictures { @@ -422,18 +432,30 @@ func (s *Service) resolveGroupMemberUsername(ctx context.Context, client ldapCli return norm.NFC.String(username) } -func (s *Service) reconcileGroups(ctx context.Context, tx *gorm.DB, desiredGroups []ldapDesiredGroup, ldapGroupIDs map[string]struct{}, dbConfig *appconfig.AppConfigModel) error { +// reconcileGroups returns the IDs of the users this sync removed from a group, which may cost them access to clients restricted to it +func (s *Service) reconcileGroups(ctx context.Context, tx *gorm.DB, desiredGroups []ldapDesiredGroup, ldapGroupIDs map[string]struct{}, dbConfig *appconfig.AppConfigModel) ([]string, error) { // Load the current LDAP-managed state from the database ldapGroupsInDB, ldapGroupsByID, err := s.loadLDAPGroupsInDB(ctx, tx) if err != nil { - return fmt.Errorf("failed to fetch groups from database: %w", err) + return nil, fmt.Errorf("failed to fetch groups from database: %w", err) } _, _, ldapUsersByUsername, err := s.loadLDAPUsersInDB(ctx, tx) if err != nil { - return fmt.Errorf("failed to fetch users from database: %w", err) + return nil, fmt.Errorf("failed to fetch users from database: %w", err) } + // Capture the memberships before they are reconciled, as removals are only visible by comparing against the desired state + var membersByGroup map[string][]string + if s.backchannelLogout != nil { + membersByGroup, err = s.loadGroupMembers(ctx, tx, ldapGroupsInDB) + if err != nil { + // Notifications are best effort and must never fail the sync + slog.Warn("Failed to load group members to notify for back-channel logout", slog.Any("error", err)) + } + } + removedMembers := map[string]struct{}{} + // Apply creates and updates to match the desired LDAP group state for _, desiredGroup := range desiredGroups { memberUserIDs := make([]string, 0, len(desiredGroup.memberUsernames)) @@ -451,26 +473,28 @@ func (s *Service) reconcileGroups(ctx context.Context, tx *gorm.DB, desiredGroup if databaseGroup.ID == "" { newGroup, err := s.groups.CreateInternal(ctx, desiredGroup.input, tx) if err != nil { - return fmt.Errorf("failed to create group '%s': %w", desiredGroup.input.Name, err) + return nil, fmt.Errorf("failed to create group '%s': %w", desiredGroup.input.Name, err) } ldapGroupsByID[desiredGroup.ldapID] = newGroup _, err = s.groups.UpdateUsersInternal(ctx, newGroup.ID, memberUserIDs, tx) if err != nil { - return fmt.Errorf("failed to sync users for group '%s': %w", desiredGroup.input.Name, err) + return nil, fmt.Errorf("failed to sync users for group '%s': %w", desiredGroup.input.Name, err) } continue } _, err = s.groups.UpdateInternal(ctx, dbConfig, databaseGroup.ID, desiredGroup.input, true, tx) if err != nil { - return fmt.Errorf("failed to update group '%s': %w", desiredGroup.input.Name, err) + return nil, fmt.Errorf("failed to update group '%s': %w", desiredGroup.input.Name, err) } _, err = s.groups.UpdateUsersInternal(ctx, databaseGroup.ID, memberUserIDs, tx) if err != nil { - return fmt.Errorf("failed to sync users for group '%s': %w", desiredGroup.input.Name, err) + return nil, fmt.Errorf("failed to sync users for group '%s': %w", desiredGroup.input.Name, err) } + + collectRemovedMembers(removedMembers, membersByGroup[databaseGroup.ID], memberUserIDs) } // Delete groups that are no longer present in LDAP @@ -488,21 +512,68 @@ func (s *Service) reconcileGroups(ctx context.Context, tx *gorm.DB, desiredGroup Delete(&model.UserGroup{}, "ldap_id = ?", *group.LdapID). Error if err != nil { - return fmt.Errorf("failed to delete group '%s': %w", group.Name, err) + return nil, fmt.Errorf("failed to delete group '%s': %w", group.Name, err) } slog.Info("Deleted group", slog.String("group", group.Name)) + + // Deleting the group removes every member from it + collectRemovedMembers(removedMembers, membersByGroup[group.ID], nil) } - return nil + return slices.Collect(maps.Keys(removedMembers)), nil +} + +// collectRemovedMembers adds the previous members that are not part of the group's new member list to removedMembers +func collectRemovedMembers(removedMembers map[string]struct{}, previousMemberIDs []string, memberIDs []string) { + for _, previousMemberID := range previousMemberIDs { + if !slices.Contains(memberIDs, previousMemberID) { + removedMembers[previousMemberID] = struct{}{} + } + } +} + +// loadGroupMembers returns the IDs of the users that are currently members of the given groups, indexed by group ID +func (s *Service) loadGroupMembers(ctx context.Context, tx *gorm.DB, groups []model.UserGroup) (map[string][]string, error) { + if len(groups) == 0 { + return nil, nil + } + + groupIDs := make([]string, len(groups)) + for i, group := range groups { + groupIDs[i] = group.ID + } + + var memberships []struct { + UserGroupID string + UserID string + } + err := tx. + WithContext(ctx). + Table("user_groups_users"). + Select("user_group_id", "user_id"). + Where("user_group_id IN (?)", groupIDs). + Find(&memberships). + Error + if err != nil { + return nil, err + } + + membersByGroup := make(map[string][]string, len(groups)) + for _, membership := range memberships { + membersByGroup[membership.UserGroupID] = append(membersByGroup[membership.UserGroupID], membership.UserID) + } + return membersByGroup, nil } //nolint:gocognit -func (s *Service) reconcileUsers(ctx context.Context, tx *gorm.DB, desiredUsers []ldapDesiredUser, ldapUserIDs map[string]struct{}, dbConfig *appconfig.AppConfigModel) (savePictures []savePicture, deleteFiles []string, err error) { +func (s *Service) reconcileUsers(ctx context.Context, tx *gorm.DB, desiredUsers []ldapDesiredUser, ldapUserIDs map[string]struct{}, dbConfig *appconfig.AppConfigModel) (savePictures []savePicture, deleteFiles []string, notifyLogout func(), err error) { + notifyLogout = func() {} + // Load the current LDAP-managed state from the database ldapUsersInDB, ldapUsersByID, _, err := s.loadLDAPUsersInDB(ctx, tx) if err != nil { - return nil, nil, fmt.Errorf("failed to fetch users from database: %w", err) + return nil, nil, nil, fmt.Errorf("failed to fetch users from database: %w", err) } // Apply creates and updates to match the desired LDAP user state @@ -520,7 +591,7 @@ func (s *Service) reconcileUsers(ctx context.Context, tx *gorm.DB, desiredUsers Update("disabled", false). Error if err != nil { - return nil, nil, fmt.Errorf("failed to enable user %s: %w", databaseUser.Username, err) + return nil, nil, nil, fmt.Errorf("failed to enable user %s: %w", databaseUser.Username, err) } databaseUser.Disabled = false @@ -534,7 +605,7 @@ func (s *Service) reconcileUsers(ctx context.Context, tx *gorm.DB, desiredUsers slog.Warn("Skipping creating LDAP user", slog.String("username", desiredUser.input.Username), slog.Any("error", err)) continue } else if err != nil { - return nil, nil, fmt.Errorf("error creating user '%s': %w", desiredUser.input.Username, err) + return nil, nil, nil, fmt.Errorf("error creating user '%s': %w", desiredUser.input.Username, err) } userID = createdUser.ID @@ -545,7 +616,7 @@ func (s *Service) reconcileUsers(ctx context.Context, tx *gorm.DB, desiredUsers slog.Warn("Skipping updating LDAP user", slog.String("username", desiredUser.input.Username), slog.Any("error", err)) continue } else if err != nil { - return nil, nil, fmt.Errorf("error updating user '%s': %w", desiredUser.input.Username, err) + return nil, nil, nil, fmt.Errorf("error updating user '%s': %w", desiredUser.input.Username, err) } } @@ -558,21 +629,36 @@ func (s *Service) reconcileUsers(ctx context.Context, tx *gorm.DB, desiredUsers } } + // The authorizations of deleted users are gone once the transaction commits, so the clients to notify must be resolved before deprovisioning + // Users that a previous sync already disabled are re-disabled on every run and must not be notified again + if s.backchannelLogout != nil { + deprovisionedUserIDs := make([]string, 0, len(ldapUsersInDB)) + for _, user := range ldapUsersInDB { + if !isDeprovisioned(user, ldapUserIDs) || (dbConfig.LdapSoftDeleteUsers.IsTrue() && user.Disabled) { + continue + } + + deprovisionedUserIDs = append(deprovisionedUserIDs, user.ID) + } + + notifyLogout, err = s.backchannelLogout.PrepareUserNotifications(ctx, tx, deprovisionedUserIDs) + if err != nil { + // Notifications are best effort and must never fail the sync + slog.Warn("Failed to prepare back-channel logout notifications for deprovisioned LDAP users", slog.Any("error", err)) + } + } + // Disable or delete users that are no longer present in LDAP deleteFiles = make([]string, 0, len(ldapUsersInDB)) for _, user := range ldapUsersInDB { - if user.LdapID == nil { - continue - } - - if _, exists := ldapUserIDs[*user.LdapID]; exists { + if !isDeprovisioned(user, ldapUserIDs) { continue } if dbConfig.LdapSoftDeleteUsers.IsTrue() { err = s.users.DisableUserInternal(ctx, tx, user.ID) if err != nil { - return nil, nil, fmt.Errorf("failed to disable user %s: %w", user.Username, err) + return nil, nil, nil, fmt.Errorf("failed to disable user %s: %w", user.Username, err) } slog.Info("Disabled user", slog.String("username", user.Username)) @@ -582,16 +668,26 @@ func (s *Service) reconcileUsers(ctx context.Context, tx *gorm.DB, desiredUsers err = s.users.DeleteUserInternal(ctx, dbConfig, tx, user.ID, true) if err != nil { if apperror.IsCode(err, apperror.CodeLdapUserUpdate) { - return nil, nil, fmt.Errorf("failed to delete user %s: LDAP user must be disabled before deletion", user.Username) + return nil, nil, nil, fmt.Errorf("failed to delete user %s: LDAP user must be disabled before deletion", user.Username) } - return nil, nil, fmt.Errorf("failed to delete user %s: %w", user.Username, err) + return nil, nil, nil, fmt.Errorf("failed to delete user %s: %w", user.Username, err) } slog.Info("Deleted user", slog.String("username", user.Username)) deleteFiles = append(deleteFiles, path.Join("profile-pictures", user.ID+".png")) } - return savePictures, deleteFiles, nil + return savePictures, deleteFiles, notifyLogout, nil +} + +// isDeprovisioned reports whether an LDAP-managed user is no longer present in the directory and is therefore disabled or deleted by this sync +func isDeprovisioned(user model.User, ldapUserIDs map[string]struct{}) bool { + if user.LdapID == nil { + return false + } + + _, exists := ldapUserIDs[*user.LdapID] + return !exists } func (s *Service) loadLDAPUsersInDB(ctx context.Context, tx *gorm.DB) (users []model.User, byLdapID map[string]model.User, byUsername map[string]model.User, err error) { diff --git a/backend/internal/ldapsync/service_test.go b/backend/internal/ldapsync/service_test.go index a9b6efe4..4b9a74fe 100644 --- a/backend/internal/ldapsync/service_test.go +++ b/backend/internal/ldapsync/service_test.go @@ -317,6 +317,39 @@ func TestLdapServiceSyncAllSetsAdminFromGroupMembership(t *testing.T) { } } +func TestLoadGroupMembers(t *testing.T) { + svc, db := newTestLdapService(t, nil) + + require.NoError(t, db.Create(&model.User{Base: model.Base{ID: "user-1"}, Username: "user1"}).Error) + require.NoError(t, db.Create(&model.User{Base: model.Base{ID: "user-2"}, Username: "user2"}).Error) + + group := model.UserGroup{Base: model.Base{ID: "group-1"}, Name: "group1", FriendlyName: "Group 1"} + require.NoError(t, db.Create(&group).Error) + require.NoError(t, db.Model(&group).Association("Users").Append([]model.User{ + {Base: model.Base{ID: "user-1"}}, + {Base: model.Base{ID: "user-2"}}, + })) + + emptyGroup := model.UserGroup{Base: model.Base{ID: "group-2"}, Name: "group2", FriendlyName: "Group 2"} + require.NoError(t, db.Create(&emptyGroup).Error) + + membersByGroup, err := svc.loadGroupMembers(t.Context(), db, []model.UserGroup{group, emptyGroup}) + require.NoError(t, err) + assert.ElementsMatch(t, []string{"user-1", "user-2"}, membersByGroup["group-1"]) + assert.Empty(t, membersByGroup["group-2"]) +} + +func TestCollectRemovedMembers(t *testing.T) { + removedMembers := map[string]struct{}{} + + collectRemovedMembers(removedMembers, []string{"user-1", "user-2"}, []string{"user-2", "user-3"}) + assert.Equal(t, map[string]struct{}{"user-1": {}}, removedMembers) + + // Deleting a group passes no remaining members, and a user removed from two groups is only collected once + collectRemovedMembers(removedMembers, []string{"user-1", "user-4"}, nil) + assert.Equal(t, map[string]struct{}{"user-1": {}, "user-4": {}}, removedMembers) +} + func newTestLdapService(t *testing.T, client ldapClient) (*Service, *gorm.DB) { t.Helper() @@ -326,7 +359,7 @@ func newTestLdapService(t *testing.T, client ldapClient) (*Service, *gorm.DB) { require.NoError(t, err) // The sync is exercised against the real user and group services, so the assertions below can check what actually lands in the database - groupService := service.NewUserGroupService(db, nil) + groupService := service.NewUserGroupService(db, nil, nil) userService := service.NewUserService( db, nil, @@ -334,6 +367,7 @@ func newTestLdapService(t *testing.T, client ldapClient) (*Service, *gorm.DB) { service.NewCustomClaimService(db), service.NewAppImagesService(map[string]string{}, fileStorage), nil, + nil, fileStorage, ) diff --git a/backend/internal/middleware/auth_middleware_test.go b/backend/internal/middleware/auth_middleware_test.go index 16b34035..9a938b0b 100644 --- a/backend/internal/middleware/auth_middleware_test.go +++ b/backend/internal/middleware/auth_middleware_test.go @@ -39,7 +39,7 @@ func TestWithApiKeyAuthDisabled(t *testing.T) { jwtService, err := service.NewJwtService(t.Context(), db, instanceID) require.NoError(t, err) - userService := service.NewUserService(db, jwtService, nil, nil, nil, nil, nil) + userService := service.NewUserService(db, jwtService, nil, nil, nil, nil, nil, nil) apiKeyModule, err := apikey.New(t.Context(), apikey.Dependencies{DB: db, CleanupDisabled: true}) require.NoError(t, err) diff --git a/backend/internal/model/oidc.go b/backend/internal/model/oidc.go index b739e240..58b2cfb0 100644 --- a/backend/internal/model/oidc.go +++ b/backend/internal/model/oidc.go @@ -50,6 +50,7 @@ type OidcClient struct { Description string CallbackURLs datatype.StringList LogoutCallbackURLs datatype.StringList + BackchannelLogoutURL string ImageType *string DarkImageType *string IsPublic bool diff --git a/backend/internal/oidc/store.go b/backend/internal/oidc/store.go index 60f55758..64f367c8 100644 --- a/backend/internal/oidc/store.go +++ b/backend/internal/oidc/store.go @@ -206,6 +206,8 @@ func (s *Store) StoreCIMDClient(ctx context.Context, resolved fosite.Client, _ * } // A security-relevant document change invalidates consent because it changes what the user previously approved + // No back-channel logout tokens are sent here, which is safe only because CIMD clients cannot register a backchannel_logout_url today + // If that URL is ever mapped from the metadata document, resolve the notifications before this delete removes the rows if revokeConsent { err := s.dbFor(ctx). Where("client_id = ?", client.ID). diff --git a/backend/internal/service/e2etest_service.go b/backend/internal/service/e2etest_service.go index bb6939e9..f4c67b34 100644 --- a/backend/internal/service/e2etest_service.go +++ b/backend/internal/service/e2etest_service.go @@ -198,14 +198,15 @@ func (s *TestService) SeedDatabase(baseURL string) error { Base: model.Base{ ID: "3654a746-35d4-4321-ac61-0bdcff2b4055", }, - Name: "Nextcloud", - Description: "This is an example description for Nextcloud", - LaunchURL: new("https://nextcloud.local"), - Credentials: seededClientCredentials("2f1b8f1a-1d3e-4f0c-9c1a-000000000001", "w2mUeZISmEvIDMEDvpY0PnxQIpj1m3zY"), - CallbackURLs: datatype.StringList{"http://nextcloud.localhost/auth/callback"}, - LogoutCallbackURLs: datatype.StringList{"http://nextcloud.localhost/auth/logout/callback"}, - ImageType: new("png"), - CreatedByID: new(users[0].ID), + Name: "Nextcloud", + Description: "This is an example description for Nextcloud", + LaunchURL: new("https://nextcloud.local"), + Credentials: seededClientCredentials("2f1b8f1a-1d3e-4f0c-9c1a-000000000001", "w2mUeZISmEvIDMEDvpY0PnxQIpj1m3zY"), + CallbackURLs: datatype.StringList{"http://nextcloud.localhost/auth/callback"}, + LogoutCallbackURLs: datatype.StringList{"http://nextcloud.localhost/auth/logout/callback"}, + BackchannelLogoutURL: "http://host.docker.internal:18124/nextcloud", + ImageType: new("png"), + CreatedByID: new(users[0].ID), }, { Base: model.Base{ @@ -225,12 +226,13 @@ func (s *TestService) SeedDatabase(baseURL string) error { Base: model.Base{ ID: "7c21a609-96b5-4011-9900-272b8d31a9d1", }, - Name: "Tailscale", - Credentials: seededClientCredentials("2f1b8f1a-1d3e-4f0c-9c1a-000000000003", "n4VfQeXlTzA6yKpWbR9uJcMdSx2qH0Lo"), - CallbackURLs: datatype.StringList{"http://tailscale.localhost/auth/callback"}, - LogoutCallbackURLs: datatype.StringList{"http://tailscale.localhost/auth/logout/callback"}, - IsGroupRestricted: true, - CreatedByID: new(users[0].ID), + Name: "Tailscale", + Credentials: seededClientCredentials("2f1b8f1a-1d3e-4f0c-9c1a-000000000003", "n4VfQeXlTzA6yKpWbR9uJcMdSx2qH0Lo"), + CallbackURLs: datatype.StringList{"http://tailscale.localhost/auth/callback"}, + LogoutCallbackURLs: datatype.StringList{"http://tailscale.localhost/auth/logout/callback"}, + BackchannelLogoutURL: "http://host.docker.internal:18124/tailscale", + IsGroupRestricted: true, + CreatedByID: new(users[0].ID), AllowedUserGroups: []model.UserGroup{ userGroups[0], }, diff --git a/backend/internal/service/jwt_service.go b/backend/internal/service/jwt_service.go index 5a07e90a..d23f2895 100644 --- a/backend/internal/service/jwt_service.go +++ b/backend/internal/service/jwt_service.go @@ -11,6 +11,7 @@ import ( "github.com/lestrrat-go/jwx/v4/jwa" "github.com/lestrrat-go/jwx/v4/jwk" + "github.com/lestrrat-go/jwx/v4/jws" "github.com/lestrrat-go/jwx/v4/jwt" "gorm.io/gorm" @@ -40,6 +41,15 @@ const ( // AccessTokenJWTType identifies a JWT as an access token used by Pocket ID AccessTokenJWTType = "access-token" + // LogoutTokenJWTTyp is the JOSE "typ" header for OIDC back-channel logout tokens + LogoutTokenJWTTyp = "logout+jwt" //nolint:gosec // this is a JOSE typ header, not a credential + + // BackchannelLogoutEvent is the member of the "events" claim that marks a JWT as a back-channel logout token + BackchannelLogoutEvent = "http://schemas.openid.net/event/backchannel-logout" + + // logoutTokenDuration is the lifetime of a logout token, which only needs to survive the delivery to the client + logoutTokenDuration = 2 * time.Minute + // Acceptable clock skew for verifying tokens clockSkew = time.Minute ) @@ -355,6 +365,43 @@ func (s *JwtService) GenerateAccessToken(user model.User, authenticationMethod s return string(signed), nil } +// GenerateLogoutToken creates a logout token for OIDC Back-Channel Logout 1.0 +// The token identifies the user with a "sub" claim only, as Pocket ID does not track per-session "sid" values +func (s *JwtService) GenerateLogoutToken(userID string, clientID string) (string, error) { + now := time.Now() + token, err := jwt.NewBuilder(). + Subject(userID). + Audience([]string{clientID}). + Expiration(now.Add(logoutTokenDuration)). + IssuedAt(now). + Issuer(s.envConfig.AppURL). + JwtID(uuid.NewV4().String()). + Claim("events", map[string]any{BackchannelLogoutEvent: struct{}{}}). + Build() + if err != nil { + return "", fmt.Errorf("failed to build token: %w", err) + } + + // The spec requires the "typ" header so clients can tell logout tokens apart from ID tokens + headers := jws.NewHeaders() + err = headers.Set(jws.TypeKey, LogoutTokenJWTTyp) + if err != nil { + return "", fmt.Errorf("failed to set 'typ' header: %w", err) + } + + alg, err := s.GetKeyAlg() + if err != nil { + return "", err + } + + signed, err := jwt.Sign(token, jwt.WithKey(alg, s.privateKey, jws.WithProtectedHeaders(headers))) + if err != nil { + return "", fmt.Errorf("failed to sign token: %w", err) + } + + return string(signed), nil +} + func (s *JwtService) VerifyAccessToken(tokenString string) (jwt.Token, error) { if s.sessionKey == nil { return nil, errors.New("session key is not initialized") diff --git a/backend/internal/service/jwt_service_test.go b/backend/internal/service/jwt_service_test.go index 74a3b6bb..0362f01b 100644 --- a/backend/internal/service/jwt_service_test.go +++ b/backend/internal/service/jwt_service_test.go @@ -815,3 +815,43 @@ func createEdDSAKeyJWK(t *testing.T, db *gorm.DB, instanceID string, envConfig * // Import as JWK and save it return importKey(t, db, instanceID, envConfig, privateKeyRaw) } + +func TestGenerateLogoutToken(t *testing.T) { + db := testutils.NewDatabaseForTest(t) + envConfig := newTestEnvConfig() + jwtService := initJwtService(t, db, newInstanceID(t, db), nil, envConfig) + + signed, err := jwtService.GenerateLogoutToken("user-id", "client-id") + require.NoError(t, err) + + // The token must carry the "logout+jwt" typ header required by the spec + message, err := jws.Parse([]byte(signed)) + require.NoError(t, err) + require.Len(t, message.Signatures(), 1) + typ, ok := message.Signatures()[0].ProtectedHeaders().Type() + require.True(t, ok) + assert.Equal(t, LogoutTokenJWTTyp, typ) + + alg, err := jwtService.GetKeyAlg() + require.NoError(t, err) + publicKey, err := jwtService.GetPublicJWK() + require.NoError(t, err) + token, err := jwt.ParseString(signed, jwt.WithValidate(true), jwt.WithKey(alg, publicKey)) + require.NoError(t, err) + + subject, _ := token.Subject() + assert.Equal(t, "user-id", subject) + audience, _ := token.Audience() + assert.Equal(t, []string{"client-id"}, audience) + issuer, _ := token.Issuer() + assert.Equal(t, envConfig.AppURL, issuer) + jti, _ := token.JwtID() + assert.Regexp(t, uuidRegexPattern, jti) + + events, err := jwt.Get[map[string]any](token, "events") + require.NoError(t, err) + assert.Contains(t, events, BackchannelLogoutEvent) + + // The spec forbids a nonce claim in logout tokens + assert.False(t, token.Has("nonce")) +} diff --git a/backend/internal/service/oidc_service.go b/backend/internal/service/oidc_service.go index 049610c5..6a8fd50c 100644 --- a/backend/internal/service/oidc_service.go +++ b/backend/internal/service/oidc_service.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "log/slog" "mime/multipart" "net/http" "net/url" @@ -19,6 +20,7 @@ import ( "gorm.io/gorm/clause" "github.com/pocket-id/pocket-id/backend/internal/apperror" + "github.com/pocket-id/pocket-id/backend/internal/backchannellogout" "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" @@ -45,6 +47,7 @@ type OidcService struct { previewBuilder oidcClientPreviewBuilder metadataRefresher metadataRefresher scimSyncScheduler ScimSyncScheduler + backchannelLogout *backchannellogout.Service httpClient *http.Client fileStorage storage.FileStorage @@ -64,6 +67,7 @@ func NewOidcService( previewBuilder oidcClientPreviewBuilder, metadataRefresher metadataRefresher, scimSyncScheduler ScimSyncScheduler, + backchannelLogout *backchannellogout.Service, httpClient *http.Client, fileStorage storage.FileStorage, ) (s *OidcService, err error) { @@ -73,6 +77,7 @@ func NewOidcService( previewBuilder: previewBuilder, metadataRefresher: metadataRefresher, scimSyncScheduler: scimSyncScheduler, + backchannelLogout: backchannelLogout, httpClient: httpClient, fileStorage: fileStorage, } @@ -198,6 +203,7 @@ func (s *OidcService) UpdateClient(ctx context.Context, clientID string, input d if err != nil { return model.OidcClient{}, err } + wasGroupRestricted := client.IsGroupRestricted err = updateOIDCClientModelFromDto(&client, &input) if err != nil { @@ -239,6 +245,11 @@ func (s *OidcService) UpdateClient(ctx context.Context, clientID string, input d return model.OidcClient{}, err } + // Turning on the group restriction revokes access for every authorized user until groups are assigned, so tell their clients to end the sessions + if s.backchannelLogout != nil && !wasGroupRestricted && client.IsGroupRestricted { + s.backchannelLogout.NotifyLostGroupAccess(ctx, nil, client.ID) + } + // All storage operations must be executed outside of a transaction if input.LogoURL != nil { err = s.downloadAndSaveLogoFromURL(ctx, client.ID, *input.LogoURL, true) @@ -279,6 +290,7 @@ func updateOIDCClientModelFromDto(client *model.OidcClient, input *dto.OidcClien client.Name = input.Name client.CallbackURLs = input.CallbackURLs client.LogoutCallbackURLs = input.LogoutCallbackURLs + client.BackchannelLogoutURL = input.BackchannelLogoutURL client.IsPublic = input.IsPublic // PKCE is required for public clients client.PkceEnabled = input.IsPublic || input.PkceEnabled @@ -314,8 +326,24 @@ func updateOIDCClientModelFromDto(client *model.OidcClient, input *dto.OidcClien } func (s *OidcService) DeleteClient(ctx context.Context, clientID string) error { + tx := s.db.Begin() + defer func() { + tx.Rollback() + }() + + // The authorizations cascade away with the client, so the users to notify must be resolved inside the transaction + notifyLogout := func() {} + if s.backchannelLogout != nil { + var prepareErr error + notifyLogout, prepareErr = s.backchannelLogout.PrepareClientNotifications(ctx, tx, clientID) + if prepareErr != nil { + // Notifications are best effort and must never block the deletion itself + slog.ErrorContext(ctx, "Failed to prepare back-channel logout notifications for client", slog.String("clientId", clientID), slog.Any("error", prepareErr)) + } + } + var client model.OidcClient - result := s.db. + result := tx. WithContext(ctx). Where("id = ?", clientID). Clauses(clause.Returning{}). @@ -327,6 +355,14 @@ func (s *OidcService) DeleteClient(ctx context.Context, clientID string) error { return apperror.NotFound("OIDC client") } + err := tx.Commit().Error + if err != nil { + return err + } + + // The deleted client keeps serving its signed-in users, so tell it to end their sessions + notifyLogout() + // Delete images if present // Note that storage operations must be done outside of a transaction if client.ImageType != nil && *client.ImageType != "" { @@ -649,6 +685,12 @@ func (s *OidcService) UpdateAllowedUserGroups(ctx context.Context, id string, in if s.scimSyncScheduler != nil { s.scimSyncScheduler.ScheduleSync(ctx) } + + // Notify users who authorized this client but are no longer in any allowed group + if s.backchannelLogout != nil && client.IsGroupRestricted { + s.backchannelLogout.NotifyLostGroupAccess(ctx, nil, client.ID) + } + return client, nil } @@ -711,6 +753,16 @@ func (s *OidcService) RevokeAuthorizedClient(ctx context.Context, userID string, return err } + // The authorization is gone after the delete, so the client to notify must be resolved inside the transaction + notifyLogout := func() {} + if s.backchannelLogout != nil { + notifyLogout, err = s.backchannelLogout.PrepareAuthorizationNotification(ctx, tx, userID, clientID) + if err != nil { + // Notifications are best effort and must never block the revocation itself + slog.ErrorContext(ctx, "Failed to prepare back-channel logout notification for authorization", slog.String("userId", userID), slog.String("clientId", clientID), slog.Any("error", err)) + } + } + err = tx.WithContext(ctx).Delete(&authorizedClient).Error if err != nil { return err @@ -725,6 +777,9 @@ func (s *OidcService) RevokeAuthorizedClient(ctx context.Context, userID string, return err } + // Tell the client to end the user's session there as well + notifyLogout() + return nil } diff --git a/backend/internal/service/oidc_service_test.go b/backend/internal/service/oidc_service_test.go index 156042ca..ba3590ab 100644 --- a/backend/internal/service/oidc_service_test.go +++ b/backend/internal/service/oidc_service_test.go @@ -521,7 +521,7 @@ func TestOidcService_downloadAndSaveLogoFromURL(t *testing.T) { func TestOidcService_CreateClient_withDescription(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil, nil) require.NoError(t, err) description := "A test client description" @@ -546,7 +546,7 @@ func TestOidcService_CreateClient_withDescription(t *testing.T) { func TestOidcService_CreateClient_withoutDescription(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil, nil) require.NoError(t, err) input := dto.OidcClientCreateDto{ @@ -595,7 +595,7 @@ func TestOidcService_CreateClient_tokenLifetimes(t *testing.T) { t.Run(test.name, func(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil, nil) require.NoError(t, err) input := dto.OidcClientCreateDto{ @@ -622,7 +622,7 @@ func TestOidcService_CreateClient_tokenLifetimes(t *testing.T) { func TestOidcService_UpdateClient_tokenLifetimes(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil, nil) require.NoError(t, err) client := model.OidcClient{ @@ -659,7 +659,7 @@ func TestOidcService_UpdateClient_tokenLifetimes(t *testing.T) { func TestOidcService_CreateClientSecret_withCustomSecret(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil, nil) require.NoError(t, err) client := model.OidcClient{Name: "Test Client"} @@ -688,7 +688,7 @@ func TestOidcService_CreateClientSecret_withCustomSecret(t *testing.T) { func TestOidcService_CreateClientSecret_multipleSecrets(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil, nil) require.NoError(t, err) client := model.OidcClient{Name: "Test Client"} @@ -726,7 +726,7 @@ func TestOidcService_CreateClientSecret_multipleSecrets(t *testing.T) { func TestOidcService_CreateClientSecret_expirationInThePast(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil, nil) require.NoError(t, err) client := model.OidcClient{Name: "Test Client"} @@ -741,7 +741,7 @@ func TestOidcService_CreateClientSecret_expirationInThePast(t *testing.T) { func TestOidcService_CreateClientSecret_limit(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil, nil) require.NoError(t, err) client := model.OidcClient{Name: "Test Client"} @@ -760,7 +760,7 @@ func TestOidcService_CreateClientSecret_limit(t *testing.T) { func TestOidcService_CreateClientSecret_preservesFederatedIdentities(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil, nil) require.NoError(t, err) client := model.OidcClient{ @@ -796,7 +796,7 @@ func TestOidcService_CreateClientSecret_preservesFederatedIdentities(t *testing. func TestOidcService_UpdateClient_description(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil, nil) require.NoError(t, err) // Create a client without a description @@ -837,7 +837,7 @@ func TestOidcService_UpdateClient_description(t *testing.T) { func TestOidcService_UpdateClient_CIMDPreservesMetadataFields(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil, nil) require.NoError(t, err) client := model.OidcClient{ @@ -906,7 +906,7 @@ func TestOidcService_UpdateClient_CIMDPreservesMetadataFields(t *testing.T) { func TestOidcService_UpdateClient_CIMDDoesNotOverwriteConcurrentMetadataRefresh(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil, nil) require.NoError(t, err) client := model.OidcClient{ @@ -939,7 +939,7 @@ func TestOidcService_UpdateClient_CIMDDoesNotOverwriteConcurrentMetadataRefresh( func TestOidcService_ListAccessibleOidcClients_requiresExplicitGroupPermission(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil, nil) require.NoError(t, err) allowedGroup := model.UserGroup{Name: "allowed", FriendlyName: "Allowed"} @@ -973,7 +973,7 @@ func TestOidcService_ListAccessibleOidcClients_requiresExplicitGroupPermission(t func TestOidcService_ListClientViewsFilterByLaunchURLPresence(t *testing.T) { db := testutils.NewDatabaseForTest(t) - s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil) + s, err := NewOidcService(db, nil, nil, nil, nil, nil, nil, nil) require.NoError(t, err) user := model.User{Username: "launch-url-filter"} diff --git a/backend/internal/service/user_group_service.go b/backend/internal/service/user_group_service.go index dc2e7db2..44eef737 100644 --- a/backend/internal/service/user_group_service.go +++ b/backend/internal/service/user_group_service.go @@ -3,6 +3,8 @@ package service import ( "context" "errors" + "log/slog" + "slices" "time" "github.com/pocket-id/pocket-id/backend/internal/appconfig" @@ -10,6 +12,7 @@ import ( "gorm.io/gorm" "github.com/pocket-id/pocket-id/backend/internal/apperror" + "github.com/pocket-id/pocket-id/backend/internal/backchannellogout" "github.com/pocket-id/pocket-id/backend/internal/dto" "github.com/pocket-id/pocket-id/backend/internal/model" "github.com/pocket-id/pocket-id/backend/internal/utils" @@ -18,10 +21,11 @@ import ( type UserGroupService struct { db *gorm.DB scimSyncScheduler ScimSyncScheduler + backchannelLogout *backchannellogout.Service } -func NewUserGroupService(db *gorm.DB, scimSyncScheduler ScimSyncScheduler) *UserGroupService { - return &UserGroupService{db: db, scimSyncScheduler: scimSyncScheduler} +func NewUserGroupService(db *gorm.DB, scimSyncScheduler ScimSyncScheduler, backchannelLogout *backchannellogout.Service) *UserGroupService { + return &UserGroupService{db: db, scimSyncScheduler: scimSyncScheduler, backchannelLogout: backchannelLogout} } func (s *UserGroupService) List(ctx context.Context, name string, listRequestOptions utils.ListRequestOptions) (groups []model.UserGroup, response utils.PaginationResponse, err error) { @@ -89,6 +93,16 @@ func (s *UserGroupService) Delete(ctx context.Context, cfg *appconfig.AppConfigM return apperror.LdapUserGroupUpdate() } + // Capture the members before the delete, as they may lose access to clients restricted to this group + // Notifications are best effort and must never block the deletion itself + var memberIDs []string + if s.backchannelLogout != nil { + memberIDs, err = s.memberIDs(ctx, tx, id) + if err != nil { + slog.ErrorContext(ctx, "Failed to find group members to notify for back-channel logout", slog.String("groupId", id), slog.Any("error", err)) + } + } + err = tx. WithContext(ctx). Delete(&group). @@ -106,9 +120,29 @@ func (s *UserGroupService) Delete(ctx context.Context, cfg *appconfig.AppConfigM s.scimSyncScheduler.ScheduleSync(ctx) } + // Tell group-restricted clients that former members can no longer access to end their sessions + if s.backchannelLogout != nil { + s.backchannelLogout.NotifyLostGroupAccess(ctx, memberIDs, "") + } + return nil } +// memberIDs returns the IDs of the users that are currently members of the group +func (s *UserGroupService) memberIDs(ctx context.Context, tx *gorm.DB, groupID string) ([]string, error) { + var userIDs []string + err := tx. + WithContext(ctx). + Table("user_groups_users"). + Where("user_group_id = ?", groupID). + Pluck("user_id", &userIDs). + Error + if err != nil { + return nil, err + } + return userIDs, nil +} + func (s *UserGroupService) Create(ctx context.Context, input dto.UserGroupCreateDto) (group model.UserGroup, err error) { group, err = s.CreateInternal(ctx, input, s.db) if err != nil { @@ -212,6 +246,16 @@ func (s *UserGroupService) UpdateUsers(ctx context.Context, id string, userIds [ tx.Rollback() }() + // Capture the previous members to work out who is removed from the group by this update + // Notifications are best effort and must never block the update itself + var previousMemberIDs []string + if s.backchannelLogout != nil { + previousMemberIDs, err = s.memberIDs(ctx, tx, id) + if err != nil { + slog.ErrorContext(ctx, "Failed to find group members to notify for back-channel logout", slog.String("groupId", id), slog.Any("error", err)) + } + } + group, err = s.UpdateUsersInternal(ctx, id, userIds, tx) if err != nil { return model.UserGroup{}, err @@ -225,6 +269,22 @@ func (s *UserGroupService) UpdateUsers(ctx context.Context, id string, userIds [ s.scimSyncScheduler.ScheduleSync(ctx) } + // Removed members may lose access to clients restricted to this group, so tell those clients to end their sessions + if s.backchannelLogout != nil { + remainingMembers := make(map[string]struct{}, len(userIds)) + for _, userID := range userIds { + remainingMembers[userID] = struct{}{} + } + + removedUserIDs := make([]string, 0, len(previousMemberIDs)) + for _, memberID := range previousMemberIDs { + if _, remains := remainingMembers[memberID]; !remains { + removedUserIDs = append(removedUserIDs, memberID) + } + } + s.backchannelLogout.NotifyLostGroupAccess(ctx, removedUserIDs, "") + } + return group, nil } @@ -312,6 +372,17 @@ func (s *UserGroupService) UpdateAllowedOidcClient(ctx context.Context, id strin return model.UserGroup{}, err } + // Dropping a client from the group's allowed list revokes access for the members that reach it through this group only + // Clients that are not group restricted are reachable either way, so they are left out + var removedClientIDs []string + if s.backchannelLogout != nil { + for _, client := range group.AllowedOidcClients { + if client.IsGroupRestricted && !slices.Contains(input.OidcClientIDs, client.ID) { + removedClientIDs = append(removedClientIDs, client.ID) + } + } + } + // Fetch the clients based on the client IDs var clients []model.OidcClient if len(input.OidcClientIDs) > 0 { @@ -353,5 +424,10 @@ func (s *UserGroupService) UpdateAllowedOidcClient(ctx context.Context, id strin s.scimSyncScheduler.ScheduleSync(ctx) } + // Tell the clients that lost this group that the members who can no longer reach them should be signed out + for _, clientID := range removedClientIDs { + s.backchannelLogout.NotifyLostGroupAccess(ctx, nil, clientID) + } + return group, nil } diff --git a/backend/internal/service/user_service.go b/backend/internal/service/user_service.go index 806a44bd..0edb70a9 100644 --- a/backend/internal/service/user_service.go +++ b/backend/internal/service/user_service.go @@ -10,6 +10,7 @@ import ( "io/fs" "log/slog" "path" + "slices" "time" "uuid" @@ -18,6 +19,7 @@ import ( "github.com/pocket-id/pocket-id/backend/internal/appconfig" "github.com/pocket-id/pocket-id/backend/internal/apperror" + "github.com/pocket-id/pocket-id/backend/internal/backchannellogout" "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" @@ -33,10 +35,11 @@ type UserService struct { customClaimService *CustomClaimService appImagesService *AppImagesService scimSyncScheduler ScimSyncScheduler + backchannelLogout *backchannellogout.Service fileStorage storage.FileStorage } -func NewUserService(db *gorm.DB, jwtService *JwtService, auditLogService *AuditLogService, customClaimService *CustomClaimService, appImagesService *AppImagesService, scimSyncScheduler ScimSyncScheduler, fileStorage storage.FileStorage) *UserService { +func NewUserService(db *gorm.DB, jwtService *JwtService, auditLogService *AuditLogService, customClaimService *CustomClaimService, appImagesService *AppImagesService, scimSyncScheduler ScimSyncScheduler, backchannelLogout *backchannellogout.Service, fileStorage storage.FileStorage) *UserService { return &UserService{ db: db, jwtService: jwtService, @@ -44,6 +47,7 @@ func NewUserService(db *gorm.DB, jwtService *JwtService, auditLogService *AuditL customClaimService: customClaimService, appImagesService: appImagesService, scimSyncScheduler: scimSyncScheduler, + backchannelLogout: backchannelLogout, fileStorage: fileStorage, } } @@ -196,7 +200,18 @@ func (s *UserService) UpdateProfilePicture(ctx context.Context, userID string, f } func (s *UserService) DeleteUser(ctx context.Context, dbConfig *appconfig.AppConfigModel, userID string, allowLdapDelete bool) error { + // The user's authorizations are gone after the delete, so the clients to notify must be resolved inside the transaction + notifyLogout := func() {} err := s.db.Transaction(func(tx *gorm.DB) error { + if s.backchannelLogout != nil { + var prepareErr error + notifyLogout, prepareErr = s.backchannelLogout.PrepareUserNotifications(ctx, tx, []string{userID}) + if prepareErr != nil { + // Notifications are best effort and must never block the deletion itself + slog.ErrorContext(ctx, "Failed to prepare back-channel logout notifications for user", slog.String("userId", userID), slog.Any("error", prepareErr)) + } + } + return s.DeleteUserInternal(ctx, dbConfig, tx, userID, allowLdapDelete) }) if err != nil { @@ -205,6 +220,7 @@ func (s *UserService) DeleteUser(ctx context.Context, dbConfig *appconfig.AppCon if s.scimSyncScheduler != nil { s.scimSyncScheduler.ScheduleSync(ctx) } + notifyLogout() // Storage operations must be executed outside of a transaction profilePicturePath := path.Join("profile-pictures", userID+".png") @@ -450,6 +466,21 @@ func (s *UserService) UpdateUser(ctx context.Context, cfg *appconfig.AppConfigMo tx.Rollback() }() + // Only an admin setting the flag can disable a user, so the previous state is only needed to detect that transition + canDisable := s.backchannelLogout != nil && !updateOwnUser && updatedUser.Disabled + var wasDisabled bool + if canDisable { + err := tx. + WithContext(ctx). + Model(&model.User{}). + Where("id = ?", userID). + Pluck("disabled", &wasDisabled). + Error + if err != nil { + return model.User{}, err + } + } + user, err := s.UpdateUserInternal(ctx, cfg, userID, updatedUser, updateOwnUser, isLdapSync, tx) if err != nil { return model.User{}, err @@ -463,6 +494,11 @@ func (s *UserService) UpdateUser(ctx context.Context, cfg *appconfig.AppConfigMo s.scimSyncScheduler.ScheduleSync(ctx) } + // A disabled user cannot sign in again, so tell their clients to end the sessions as well + if canDisable && !wasDisabled && user.Disabled { + s.backchannelLogout.NotifyUser(ctx, userID) + } + return user, nil } @@ -555,6 +591,11 @@ func (s *UserService) UpdateUserGroups(ctx context.Context, id string, userGroup return model.User{}, err } + // Only a removed group can revoke access, so adding groups skips the notification below + lostGroup := slices.ContainsFunc(user.UserGroups, func(group model.UserGroup) bool { + return !slices.Contains(userGroupIds, group.ID) + }) + // Fetch the groups based on userGroupIds var groups []model.UserGroup if len(userGroupIds) > 0 { @@ -603,6 +644,11 @@ func (s *UserService) UpdateUserGroups(ctx context.Context, id string, userGroup s.scimSyncScheduler.ScheduleSync(ctx) } + // Losing a group can revoke access to group-restricted clients, so tell those clients to end the user's sessions + if s.backchannelLogout != nil && lostGroup { + s.backchannelLogout.NotifyLostGroupAccess(ctx, []string{id}, "") + } + return user, nil } diff --git a/backend/internal/service/user_service_test.go b/backend/internal/service/user_service_test.go index e5a10b09..ada9e1a6 100644 --- a/backend/internal/service/user_service_test.go +++ b/backend/internal/service/user_service_test.go @@ -30,9 +30,10 @@ func newTestUserService(t *testing.T) (*UserService, *UserGroupService) { NewCustomClaimService(db), NewAppImagesService(map[string]string{}, fileStorage), nil, + nil, fileStorage, ) - groupService := NewUserGroupService(db, nil) + groupService := NewUserGroupService(db, nil, nil) return userService, groupService } diff --git a/backend/resources/migrations/postgres/20260923183637_add_oidc_client_backchannel_logout_url.down.sql b/backend/resources/migrations/postgres/20260923183637_add_oidc_client_backchannel_logout_url.down.sql new file mode 100644 index 00000000..a8fbbd61 --- /dev/null +++ b/backend/resources/migrations/postgres/20260923183637_add_oidc_client_backchannel_logout_url.down.sql @@ -0,0 +1 @@ +ALTER TABLE oidc_clients DROP COLUMN backchannel_logout_url; diff --git a/backend/resources/migrations/postgres/20260923183637_add_oidc_client_backchannel_logout_url.up.sql b/backend/resources/migrations/postgres/20260923183637_add_oidc_client_backchannel_logout_url.up.sql new file mode 100644 index 00000000..0a7a357a --- /dev/null +++ b/backend/resources/migrations/postgres/20260923183637_add_oidc_client_backchannel_logout_url.up.sql @@ -0,0 +1 @@ +ALTER TABLE oidc_clients ADD COLUMN backchannel_logout_url TEXT NOT NULL DEFAULT ''; diff --git a/backend/resources/migrations/sqlite/20260923183637_add_oidc_client_backchannel_logout_url.down.sql b/backend/resources/migrations/sqlite/20260923183637_add_oidc_client_backchannel_logout_url.down.sql new file mode 100644 index 00000000..a8fbbd61 --- /dev/null +++ b/backend/resources/migrations/sqlite/20260923183637_add_oidc_client_backchannel_logout_url.down.sql @@ -0,0 +1 @@ +ALTER TABLE oidc_clients DROP COLUMN backchannel_logout_url; diff --git a/backend/resources/migrations/sqlite/20260923183637_add_oidc_client_backchannel_logout_url.up.sql b/backend/resources/migrations/sqlite/20260923183637_add_oidc_client_backchannel_logout_url.up.sql new file mode 100644 index 00000000..0a7a357a --- /dev/null +++ b/backend/resources/migrations/sqlite/20260923183637_add_oidc_client_backchannel_logout_url.up.sql @@ -0,0 +1 @@ +ALTER TABLE oidc_clients ADD COLUMN backchannel_logout_url TEXT NOT NULL DEFAULT ''; diff --git a/frontend/messages/en.json b/frontend/messages/en.json index d344796e..ae337358 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -289,6 +289,8 @@ "add": "Add", "callback_urls": "Callback URLs", "logout_callback_urls": "Logout Callback URLs", + "backchannel_logout_url": "Back-Channel Logout URL", + "backchannel_logout_url_description": "URL that receives a logout token when a user's access is revoked, for example when they are disabled or lose group access.", "public_client": "Public Client", "public_clients_description": "Public clients do not have a client secret. They are designed for mobile, web, and native applications where secrets cannot be securely stored.", "pkce": "PKCE", diff --git a/frontend/src/lib/types/oidc.type.ts b/frontend/src/lib/types/oidc.type.ts index 7f0c7837..51e3e240 100644 --- a/frontend/src/lib/types/oidc.type.ts +++ b/frontend/src/lib/types/oidc.type.ts @@ -54,6 +54,7 @@ export type OidcDiscoveryConfiguration = { export type OidcClient = OidcClientMetaData & { callbackURLs: string[]; logoutCallbackURLs: string[]; + backchannelLogoutURL: string; isPublic: boolean; pkceEnabled: boolean; requiresReauthentication: boolean; diff --git a/frontend/src/routes/settings/admin/oidc-clients/oidc-client-form.svelte b/frontend/src/routes/settings/admin/oidc-clients/oidc-client-form.svelte index a2ecd932..1d7b0d80 100644 --- a/frontend/src/routes/settings/admin/oidc-clients/oidc-client-form.svelte +++ b/frontend/src/routes/settings/admin/oidc-clients/oidc-client-form.svelte @@ -57,6 +57,7 @@ description: existingClient?.description || '', callbackURLs: existingClient?.callbackURLs || [], logoutCallbackURLs: existingClient?.logoutCallbackURLs || [], + backchannelLogoutURL: existingClient?.backchannelLogoutURL || '', isPublic: existingClient?.isPublic || false, pkceEnabled: existingClient?.pkceEnabled || false, requiresReauthentication: existingClient?.requiresReauthentication || false, @@ -84,6 +85,7 @@ description: z.string().max(150), callbackURLs: z.array(callbackUrlSchema).default([]), logoutCallbackURLs: z.array(callbackUrlSchema).default([]), + backchannelLogoutURL: z.url().or(z.literal('')), isPublic: z.boolean(), pkceEnabled: z.boolean(), requiresReauthentication: z.boolean(), @@ -346,15 +348,25 @@ description={m.requires_pushed_authorization_requests_description()} bind:checked={$inputs.requiresPushedAuthorizationRequests.value} /> - {#if mode == 'create'} +
- {/if} + {#if mode == 'create'} + + {/if} +
{/if} diff --git a/tests/data.ts b/tests/data.ts index c384714e..a104e7a1 100644 --- a/tests/data.ts +++ b/tests/data.ts @@ -30,6 +30,7 @@ export const oidcClients = { name: 'Nextcloud', callbackUrl: 'http://nextcloud.localhost/auth/callback', logoutCallbackUrl: 'http://nextcloud.localhost/auth/logout/callback', + backchannelLogoutURL: 'http://host.docker.internal:18124/nextcloud', secret: 'w2mUeZISmEvIDMEDvpY0PnxQIpj1m3zY', launchURL: 'https://nextcloud.local' }, @@ -44,6 +45,7 @@ export const oidcClients = { id: '7c21a609-96b5-4011-9900-272b8d31a9d1', name: 'Tailscale', callbackUrl: 'http://tailscale.localhost/auth/callback', + backchannelLogoutURL: 'http://host.docker.internal:18124/tailscale', secret: 'n4VfQeXlTzA6yKpWbR9uJcMdSx2qH0Lo' }, federated: { diff --git a/tests/resources/export/database.json b/tests/resources/export/database.json index 7e9f4b75..ca926518 100644 --- a/tests/resources/export/database.json +++ b/tests/resources/export/database.json @@ -1,6 +1,6 @@ { "provider": "sqlite", - "version": 20260814120000, + "version": 20260923183637, "tableOrder": [ "users", "user_groups", @@ -108,6 +108,7 @@ "oidc_clients": [ { "access_token_duration_minutes": 60, + "backchannel_logout_url": "http://host.docker.internal:18124/nextcloud", "callback_urls": "WyJodHRwOi8vbmV4dGNsb3VkLmxvY2FsaG9zdC9hdXRoL2NhbGxiYWNrIl0=", "client_type": "standard", "created_at": "2025-11-25T12:39:02Z", @@ -133,6 +134,7 @@ }, { "access_token_duration_minutes": 60, + "backchannel_logout_url": "", "callback_urls": "WyJodHRwOi8vaW1taWNoLmxvY2FsaG9zdC9hdXRoL2NhbGxiYWNrIl0=", "client_type": "standard", "created_at": "2025-11-25T12:39:02Z", @@ -158,6 +160,7 @@ }, { "access_token_duration_minutes": 60, + "backchannel_logout_url": "http://host.docker.internal:18124/tailscale", "callback_urls": "WyJodHRwOi8vdGFpbHNjYWxlLmxvY2FsaG9zdC9hdXRoL2NhbGxiYWNrIl0=", "client_type": "standard", "created_at": "2025-11-25T12:39:02Z", @@ -183,6 +186,7 @@ }, { "access_token_duration_minutes": 60, + "backchannel_logout_url": "", "callback_urls": "WyJodHRwOi8vZmVkZXJhdGVkLmxvY2FsaG9zdC9hdXRoL2NhbGxiYWNrIl0=", "client_type": "standard", "created_at": "2025-11-25T12:39:02Z", @@ -208,6 +212,7 @@ }, { "access_token_duration_minutes": 60, + "backchannel_logout_url": "", "callback_urls": "WyJodHRwOi8vc2NpbWNsaWVudC5sb2NhbGhvc3QvYXV0aC9jYWxsYmFjayJd", "client_type": "standard", "created_at": "2025-11-25T12:39:02Z", @@ -233,6 +238,7 @@ }, { "access_token_duration_minutes": 60, + "backchannel_logout_url": "", "callback_urls": "WyJodHRwOi8vcGFyLWNsaWVudC5sb2NhbGhvc3QvYXV0aC9jYWxsYmFjayJd", "client_type": "standard", "created_at": "2025-11-25T12:39:02Z", @@ -258,6 +264,7 @@ }, { "access_token_duration_minutes": 60, + "backchannel_logout_url": "", "callback_urls": "WyJodHRwOi8vc2tpcC1jb25zZW50LmxvY2FsaG9zdC9hdXRoL2NhbGxiYWNrIl0=", "client_type": "standard", "created_at": "2025-11-25T12:39:02Z", diff --git a/tests/setup/docker-compose.yml b/tests/setup/docker-compose.yml index 27c3f461..873a9e2f 100644 --- a/tests/setup/docker-compose.yml +++ b/tests/setup/docker-compose.yml @@ -15,6 +15,8 @@ services: - "18123:8080" pocket-id: image: pocket-id:test + extra_hosts: + - "host.docker.internal:host-gateway" ports: - "1411:1411" environment: diff --git a/tests/specs/oidc-backchannel-logout.spec.ts b/tests/specs/oidc-backchannel-logout.spec.ts new file mode 100644 index 00000000..db134362 --- /dev/null +++ b/tests/specs/oidc-backchannel-logout.spec.ts @@ -0,0 +1,252 @@ +import base, { expect, type APIRequestContext } from '@playwright/test'; +import { createLocalJWKSet, jwtVerify } from 'jose'; +import { createServer } from 'node:http'; +import { oidcClients, userGroups, users } from '../data'; +import { cleanupBackend } from '../utils/cleanup.util'; +import { saveUnsavedChanges } from '../utils/unsaved-changes.util'; + +type Delivery = { + path: string; + method: string; + contentType: string; + body: URLSearchParams; +}; + +type LogoutReceiver = { + url: string; + deliveries: Delivery[]; + respond: (path: string, attempt: number) => number; +}; + +const test = base.extend<{ receiver: LogoutReceiver }>({ + receiver: async ({}, use) => { + const receiverURL = new URL(oidcClients.nextcloud.backchannelLogoutURL); + const receiver: LogoutReceiver = { + url: receiverURL.origin, + deliveries: [], + respond: () => 204 + }; + const server = createServer(async (request, response) => { + const chunks: Buffer[] = []; + for await (const chunk of request) chunks.push(Buffer.from(chunk)); + const path = request.url!; + receiver.deliveries.push({ + path, + method: request.method!, + contentType: request.headers['content-type'] ?? '', + body: new URLSearchParams(Buffer.concat(chunks).toString()) + }); + const attempt = receiver.deliveries.filter((delivery) => delivery.path === path).length; + response.writeHead(receiver.respond(path, attempt)).end(); + }); + + // Docker reaches this real RP endpoint through the host gateway on both local machines and CI + await new Promise((resolve, reject) => { + server.once('error', reject); + server.listen(Number(receiverURL.port), '0.0.0.0', resolve); + }); + try { + await use(receiver); + } finally { + server.closeAllConnections(); + await new Promise((resolve, reject) => { + server.close((error) => (error ? reject(error) : resolve())); + }); + } + } +}); + +test.beforeEach(async () => cleanupBackend({ skipLdapSetup: true })); + +async function revoke(request: APIRequestContext, clientId: string) { + const response = await request.delete(`/api/oidc/users/me/authorized-clients/${clientId}`); + expect(response.ok()).toBeTruthy(); +} + +async function verifyLogoutToken( + request: APIRequestContext, + delivery: Delivery, + clientId: string, + userId: string +) { + expect(delivery.method).toBe('POST'); + expect(delivery.contentType).toBe('application/x-www-form-urlencoded'); + expect(delivery.body.getAll('logout_token')).toHaveLength(1); + const discoveryResponse = await request.get('/.well-known/openid-configuration'); + expect(discoveryResponse.ok()).toBeTruthy(); + const discovery = await discoveryResponse.json(); + expect(discovery.backchannel_logout_supported).toBe(true); + expect(discovery.backchannel_logout_session_supported).toBe(false); + const jwksResponse = await request.get(discovery.jwks_uri); + expect(jwksResponse.ok()).toBeTruthy(); + const { payload, protectedHeader } = await jwtVerify( + delivery.body.get('logout_token')!, + createLocalJWKSet(await jwksResponse.json()), + { + issuer: discovery.issuer, + audience: clientId, + algorithms: discovery.id_token_signing_alg_values_supported, + typ: 'logout+jwt', + maxTokenAge: '2m', + requiredClaims: ['iss', 'sub', 'aud', 'iat', 'exp', 'jti', 'events'] + } + ); + expect(protectedHeader.kid).toBeTruthy(); + expect(payload.sub).toBe(userId); + expect(payload.aud).toEqual([clientId]); + expect(payload.events).toEqual({ 'http://schemas.openid.net/event/backchannel-logout': {} }); + expect(payload.exp! - payload.iat!).toBe(120); + expect(payload.jti).toBeTruthy(); + expect(payload).not.toHaveProperty('nonce'); + expect(payload).not.toHaveProperty('sid'); + return payload; +} + +test('Saving a logout URL and revoking an app sends a verifiable logout token', async ({ + page, + receiver +}) => { + const client = oidcClients.nextcloud; + const logoutURL = `${receiver.url}/logout?tenant=test`; + await page.goto(`/settings/admin/oidc-clients/${client.id}`); + await page.getByRole('button', { name: 'Show Advanced Options' }).click(); + await page.getByLabel('Back-Channel Logout URL', { exact: true }).fill(logoutURL); + await saveUnsavedChanges(page); + await page.reload(); + await page.getByRole('button', { name: 'Show Advanced Options' }).click(); + await expect(page.getByLabel('Back-Channel Logout URL', { exact: true })).toHaveValue(logoutURL); + + await page.goto('/settings/apps'); + await page + .getByRole('article', { name: client.name }) + .getByRole('button', { name: 'Toggle menu' }) + .click(); + await page.getByRole('menuitem', { name: 'Revoke' }).click(); + await page.getByRole('alertdialog').getByRole('button', { name: 'Revoke' }).click(); + // Other specs can leave retries for the seeded URLs, so observe this client's updated endpoint + const deliveries = () => + receiver.deliveries.filter((delivery) => delivery.path === '/logout?tenant=test'); + await expect.poll(() => deliveries().length).toBe(1); + await verifyLogoutToken(page.request, deliveries()[0], client.id, users.tim.id); +}); + +test('Disabling a user delivers logout to their authorized clients', async ({ + request, + receiver +}) => { + const clients = [oidcClients.nextcloud, oidcClients.tailscale]; + const userResponse = await request.get(`/api/users/${users.tim.id}`); + expect(userResponse.ok()).toBeTruthy(); + const disabled = await request.put(`/api/users/${users.tim.id}`, { + data: { ...(await userResponse.json()), disabled: true } + }); + expect(disabled.ok()).toBeTruthy(); + await expect.poll(() => receiver.deliveries.length).toBe(clients.length); + expect(receiver.deliveries.map((delivery) => delivery.path).sort()).toEqual( + clients.map((client) => new URL(client.backchannelLogoutURL).pathname).sort() + ); + const tokens = await Promise.all( + clients.map((client) => + verifyLogoutToken( + request, + receiver.deliveries.find( + (delivery) => delivery.path === new URL(client.backchannelLogoutURL).pathname + )!, + client.id, + users.tim.id + ) + ) + ); + expect(new Set(tokens.map((token) => token.jti)).size).toBe(clients.length); +}); + +test('Deleting a client still notifies its users after authorizations are deleted', async ({ + request, + receiver +}) => { + const client = oidcClients.nextcloud; + expect((await request.delete(`/api/oidc/clients/${client.id}`)).ok()).toBeTruthy(); + expect((await request.get(`/api/oidc/clients/${client.id}`)).status()).toBe(404); + await expect.poll(() => receiver.deliveries.length).toBe(1); + await verifyLogoutToken(request, receiver.deliveries[0], client.id, users.tim.id); +}); + +test('Group access is retained through another allowed group and revoked after the last one', async ({ + request, + receiver +}) => { + const client = oidcClients.tailscale; + expect( + ( + await request.put(`/api/oidc/clients/${client.id}/allowed-user-groups`, { + data: { userGroupIds: [userGroups.developers.id, userGroups.designers.id] } + }) + ).ok() + ).toBeTruthy(); + expect( + ( + await request.put(`/api/users/${users.tim.id}/user-groups`, { + data: { userGroupIds: [userGroups.designers.id] } + }) + ).ok() + ).toBeTruthy(); + + // Observe the asynchronous delivery window before removing the user's remaining access + await new Promise((resolve) => setTimeout(resolve, 1500)); + expect(receiver.deliveries).toHaveLength(0); + expect( + ( + await request.put(`/api/users/${users.tim.id}/user-groups`, { + data: { userGroupIds: [] } + }) + ).ok() + ).toBeTruthy(); + await expect.poll(() => receiver.deliveries.length).toBe(1); + await verifyLogoutToken(request, receiver.deliveries[0], client.id, users.tim.id); +}); + +test('Deleting a user still notifies their clients after authorizations are deleted', async ({ + request, + receiver +}) => { + const response = await request.delete(`/api/users/${users.tim.id}`); + expect(response.status()).toBe(204); + await expect.poll(() => receiver.deliveries.length).toBe(2); + for (const client of [oidcClients.nextcloud, oidcClients.tailscale]) { + const delivery = receiver.deliveries.find( + (delivery) => delivery.path === new URL(client.backchannelLogoutURL).pathname + ); + expect(delivery).toBeDefined(); + await verifyLogoutToken(request, delivery!, client.id, users.tim.id); + } +}); + +test('Changing a client’s allowed groups logs out users only after their last allowed group is removed', async ({ + request, + receiver +}) => { + const client = oidcClients.tailscale; + const endpoint = `/api/oidc/clients/${client.id}/allowed-user-groups`; + expect( + ( + await request.put(endpoint, { + data: { userGroupIds: [userGroups.developers.id, userGroups.designers.id] } + }) + ).ok() + ).toBeTruthy(); + expect( + ( + await request.put(endpoint, { + data: { userGroupIds: [userGroups.designers.id] } + }) + ).ok() + ).toBeTruthy(); + + // Retaining an allowed group must not schedule a logout for its members + await new Promise((resolve) => setTimeout(resolve, 1500)); + expect(receiver.deliveries).toHaveLength(0); + expect((await request.put(endpoint, { data: { userGroupIds: [] } })).ok()).toBeTruthy(); + await expect.poll(() => receiver.deliveries.length).toBe(1); + expect(receiver.deliveries[0].path).toBe('/tailscale'); + await verifyLogoutToken(request, receiver.deliveries[0], client.id, users.tim.id); +});