From ca559bc3fb7f04a87c6b1e7ba03eb7f8e50338cc Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Thu, 12 Feb 2026 12:42:10 +0100 Subject: [PATCH 01/15] add additional validation to logout token Signed-off-by: Christian Richter Co-authored-by: Michael Barz --- .../pkg/staticroutes/backchannellogout.go | 71 ++++++++++++++----- 1 file changed, 53 insertions(+), 18 deletions(-) diff --git a/services/proxy/pkg/staticroutes/backchannellogout.go b/services/proxy/pkg/staticroutes/backchannellogout.go index 0c67a4d951..f016b6c437 100644 --- a/services/proxy/pkg/staticroutes/backchannellogout.go +++ b/services/proxy/pkg/staticroutes/backchannellogout.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/http" + "strings" "github.com/go-chi/render" "github.com/opencloud-eu/opencloud/pkg/oidc" @@ -25,6 +26,12 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re return } + if r.PostFormValue("logout_token") == "" { + logger.Warn().Msg("logout_token is missing") + render.Status(r, http.StatusBadRequest) + render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: "logout_token is missing"}) + } + logoutToken, err := s.OidcClient.VerifyLogoutToken(r.Context(), r.PostFormValue("logout_token")) if err != nil { logger.Warn().Err(err).Msg("VerifyLogoutToken failed") @@ -33,24 +40,41 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re return } - records, err := s.UserInfoCache.Read(logoutToken.SessionId) - if errors.Is(err, microstore.ErrNotFound) || len(records) == 0 { - render.Status(r, http.StatusOK) - render.JSON(w, r, nil) - return - } - if err != nil { - logger.Error().Err(err).Msg("Error reading userinfo cache") + var records []*microstore.Record + + if strings.TrimSpace(logoutToken.SessionId) != "" { + records, err = s.UserInfoCache.Read(logoutToken.SessionId) + if errors.Is(err, microstore.ErrNotFound) || len(records) == 0 { + render.Status(r, http.StatusOK) + render.JSON(w, r, nil) + return + } + if err != nil { + logger.Error().Err(err).Msg("Error reading userinfo cache") + render.Status(r, http.StatusBadRequest) + render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) + return + } + } else if strings.TrimSpace(logoutToken.Subject) != "" { + records, err = s.UserInfoCache.Read(logoutToken.Subject) + if errors.Is(err, microstore.ErrNotFound) || len(records) == 0 { + render.Status(r, http.StatusOK) + render.JSON(w, r, nil) + return + } + if err != nil { + logger.Error().Err(err).Msg("Error reading userinfo cache") + render.Status(r, http.StatusBadRequest) + render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) + return + } + } else { + logger.Warn().Msg("invalid logout token") render.Status(r, http.StatusBadRequest) - render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) - return + render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: "invalid logout token"}) } for _, record := range records { - err := s.publishBackchannelLogoutEvent(r.Context(), record, logoutToken) - if err != nil { - s.Logger.Warn().Err(err).Msg("could not publish backchannel logout event") - } err = s.UserInfoCache.Delete(string(record.Value)) if err != nil && !errors.Is(err, microstore.ErrNotFound) { // Spec requires us to return a 400 BadRequest when the session could not be destroyed @@ -59,13 +83,24 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) return } + err := s.publishBackchannelLogoutEvent(r.Context(), record, logoutToken) + if err != nil { + s.Logger.Warn().Err(err).Msg("could not publish backchannel logout event") + } logger.Debug().Msg("Deleted userinfo from cache") } - // we can ignore errors when cleaning up the lookup table - err = s.UserInfoCache.Delete(logoutToken.SessionId) - if err != nil { - logger.Debug().Err(err).Msg("Failed to cleanup sessionid lookup entry") + if strings.TrimSpace(logoutToken.SessionId) != "" { + // we can ignore errors when cleaning up the lookup table + err = s.UserInfoCache.Delete(logoutToken.SessionId) + if err != nil { + logger.Debug().Err(err).Msg("Failed to cleanup sessionid lookup entry") + } + } else if strings.TrimSpace(logoutToken.Subject) != "" { + err = s.UserInfoCache.Delete(logoutToken.Subject) + if err != nil { + logger.Debug().Err(err).Msg("Failed to cleanup subject lookup entry") + } } render.Status(r, http.StatusOK) From 5aefcb9544cc29a782010a5e44bb0e11efc58fbe Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Thu, 12 Feb 2026 16:41:19 +0100 Subject: [PATCH 02/15] add mapping to backchannel logout for subject => sessionid Signed-off-by: Christian Richter --- .../pkg/staticroutes/backchannellogout.go | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/services/proxy/pkg/staticroutes/backchannellogout.go b/services/proxy/pkg/staticroutes/backchannellogout.go index f016b6c437..ad680813ee 100644 --- a/services/proxy/pkg/staticroutes/backchannellogout.go +++ b/services/proxy/pkg/staticroutes/backchannellogout.go @@ -30,6 +30,7 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re logger.Warn().Msg("logout_token is missing") render.Status(r, http.StatusBadRequest) render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: "logout_token is missing"}) + return } logoutToken, err := s.OidcClient.VerifyLogoutToken(r.Context(), r.PostFormValue("logout_token")) @@ -56,6 +57,7 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re return } } else if strings.TrimSpace(logoutToken.Subject) != "" { + // TODO: enter a mapping table between subject and sessionid when the oidc session is refreshed records, err = s.UserInfoCache.Read(logoutToken.Subject) if errors.Is(err, microstore.ErrNotFound) || len(records) == 0 { render.Status(r, http.StatusOK) @@ -68,10 +70,21 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) return } + for _, record := range records { + // take all previous records retrieved for this subject, and fetch the corresponding sessions + rs, err := s.UserInfoCache.Read(string(record.Value)) + if errors.Is(err, microstore.ErrNotFound) || len(rs) == 0 { + // we do not care about errors here, since we already have entries from the subjects that need to be addressed + continue + } + // we append the additional sessions found through the mapping for later deletion + records = append(records, rs...) + } } else { logger.Warn().Msg("invalid logout token") render.Status(r, http.StatusBadRequest) render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: "invalid logout token"}) + return } for _, record := range records { @@ -95,11 +108,18 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re err = s.UserInfoCache.Delete(logoutToken.SessionId) if err != nil { logger.Debug().Err(err).Msg("Failed to cleanup sessionid lookup entry") + render.Status(r, http.StatusBadRequest) + render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) + return } } else if strings.TrimSpace(logoutToken.Subject) != "" { + // TODO: do a lookup subject => sessionid and delete both entries err = s.UserInfoCache.Delete(logoutToken.Subject) if err != nil { logger.Debug().Err(err).Msg("Failed to cleanup subject lookup entry") + render.Status(r, http.StatusBadRequest) + render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) + return } } From 06708d94a986c2e8b337c43b3b06d2b1c72779af Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Thu, 12 Feb 2026 16:51:42 +0100 Subject: [PATCH 03/15] create mapping in cache for subject => sessionid Signed-off-by: Christian Richter --- services/proxy/pkg/middleware/oidc_auth.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/services/proxy/pkg/middleware/oidc_auth.go b/services/proxy/pkg/middleware/oidc_auth.go index b797fcac0d..20a3c60af0 100644 --- a/services/proxy/pkg/middleware/oidc_auth.go +++ b/services/proxy/pkg/middleware/oidc_auth.go @@ -124,6 +124,18 @@ func (m *OIDCAuthenticator) getClaims(token string, req *http.Request) (map[stri if err != nil { m.Logger.Error().Err(err).Msg("failed to write session lookup cache") } + + // create an additional entry mapping subject to session id + if sub := aClaims.Subject; sub != "" { + err = m.userInfoCache.Write(&store.Record{ + Key: sub, + Value: []byte(sid), + Expiry: time.Until(expiration), + }) + if err != nil { + m.Logger.Error().Err(err).Msg("failed to write subject lookup cache") + } + } } } }() From 2e36859816bf918c7cca516af9f9e7014f52432e Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Thu, 12 Feb 2026 17:46:04 +0100 Subject: [PATCH 04/15] refactor deletion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jörn Dreyer Co-authored-by: Michael Barz Signed-off-by: Christian Richter --- services/proxy/pkg/middleware/oidc_auth.go | 3 +- .../pkg/staticroutes/backchannellogout.go | 88 ++++++++----------- 2 files changed, 38 insertions(+), 53 deletions(-) diff --git a/services/proxy/pkg/middleware/oidc_auth.go b/services/proxy/pkg/middleware/oidc_auth.go index 20a3c60af0..dcb82f54d4 100644 --- a/services/proxy/pkg/middleware/oidc_auth.go +++ b/services/proxy/pkg/middleware/oidc_auth.go @@ -3,6 +3,7 @@ package middleware import ( "context" "encoding/base64" + "fmt" "net" "net/http" "strings" @@ -128,7 +129,7 @@ func (m *OIDCAuthenticator) getClaims(token string, req *http.Request) (map[stri // create an additional entry mapping subject to session id if sub := aClaims.Subject; sub != "" { err = m.userInfoCache.Write(&store.Record{ - Key: sub, + Key: fmt.Sprintf("%s.%s", sub, sid), Value: []byte(sid), Expiry: time.Until(expiration), }) diff --git a/services/proxy/pkg/staticroutes/backchannellogout.go b/services/proxy/pkg/staticroutes/backchannellogout.go index ad680813ee..eb5574112c 100644 --- a/services/proxy/pkg/staticroutes/backchannellogout.go +++ b/services/proxy/pkg/staticroutes/backchannellogout.go @@ -41,29 +41,17 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re return } - var records []*microstore.Record + var cacheKeys map[string]bool if strings.TrimSpace(logoutToken.SessionId) != "" { - records, err = s.UserInfoCache.Read(logoutToken.SessionId) + cacheKeys[logoutToken.SessionId] = true + } else if strings.TrimSpace(logoutToken.Subject) != "" { + records, err := s.UserInfoCache.Read(fmt.Sprintf("%s.*", logoutToken.Subject)) if errors.Is(err, microstore.ErrNotFound) || len(records) == 0 { - render.Status(r, http.StatusOK) - render.JSON(w, r, nil) - return - } - if err != nil { - logger.Error().Err(err).Msg("Error reading userinfo cache") render.Status(r, http.StatusBadRequest) render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) return } - } else if strings.TrimSpace(logoutToken.Subject) != "" { - // TODO: enter a mapping table between subject and sessionid when the oidc session is refreshed - records, err = s.UserInfoCache.Read(logoutToken.Subject) - if errors.Is(err, microstore.ErrNotFound) || len(records) == 0 { - render.Status(r, http.StatusOK) - render.JSON(w, r, nil) - return - } if err != nil { logger.Error().Err(err).Msg("Error reading userinfo cache") render.Status(r, http.StatusBadRequest) @@ -71,14 +59,8 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re return } for _, record := range records { - // take all previous records retrieved for this subject, and fetch the corresponding sessions - rs, err := s.UserInfoCache.Read(string(record.Value)) - if errors.Is(err, microstore.ErrNotFound) || len(rs) == 0 { - // we do not care about errors here, since we already have entries from the subjects that need to be addressed - continue - } - // we append the additional sessions found through the mapping for later deletion - records = append(records, rs...) + cacheKeys[string(record.Value)] = true + cacheKeys[record.Key] = false } } else { logger.Warn().Msg("invalid logout token") @@ -87,52 +69,54 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re return } - for _, record := range records { - err = s.UserInfoCache.Delete(string(record.Value)) + for key, isSID := range cacheKeys { + if isSID { + records, err := s.UserInfoCache.Read(key) + if err != nil && !errors.Is(err, microstore.ErrNotFound) { + logger.Error().Err(err).Msg("Error reading userinfo cache") + render.Status(r, http.StatusBadRequest) + render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) + return + } + for _, record := range records { + err = s.UserInfoCache.Delete(string(record.Value)) + if err != nil && !errors.Is(err, microstore.ErrNotFound) { + logger.Error().Err(err).Msg("Error deleting userinfo cache") + render.Status(r, http.StatusBadRequest) + render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) + return + } + } + } + + err = s.UserInfoCache.Delete(key) if err != nil && !errors.Is(err, microstore.ErrNotFound) { // Spec requires us to return a 400 BadRequest when the session could not be destroyed - logger.Err(err).Msg("could not delete user info from cache") + logger.Err(err).Msg(fmt.Errorf("could not delete session from cache (%s)", key).Error()) + // We only return on requests that do only attempt to destroy a single session, not multiple render.Status(r, http.StatusBadRequest) render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) return } - err := s.publishBackchannelLogoutEvent(r.Context(), record, logoutToken) - if err != nil { - s.Logger.Warn().Err(err).Msg("could not publish backchannel logout event") + if isSID { + err := s.publishBackchannelLogoutEvent(r.Context(), key, logoutToken) + if err != nil { + s.Logger.Warn().Err(err).Msg("could not publish backchannel logout event") + } } logger.Debug().Msg("Deleted userinfo from cache") } - if strings.TrimSpace(logoutToken.SessionId) != "" { - // we can ignore errors when cleaning up the lookup table - err = s.UserInfoCache.Delete(logoutToken.SessionId) - if err != nil { - logger.Debug().Err(err).Msg("Failed to cleanup sessionid lookup entry") - render.Status(r, http.StatusBadRequest) - render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) - return - } - } else if strings.TrimSpace(logoutToken.Subject) != "" { - // TODO: do a lookup subject => sessionid and delete both entries - err = s.UserInfoCache.Delete(logoutToken.Subject) - if err != nil { - logger.Debug().Err(err).Msg("Failed to cleanup subject lookup entry") - render.Status(r, http.StatusBadRequest) - render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) - return - } - } - render.Status(r, http.StatusOK) render.JSON(w, r, nil) } // publishBackchannelLogoutEvent publishes a backchannel logout event when the callback revived from the identity provider -func (s StaticRouteHandler) publishBackchannelLogoutEvent(ctx context.Context, record *microstore.Record, logoutToken *oidc.LogoutToken) error { +func (s StaticRouteHandler) publishBackchannelLogoutEvent(ctx context.Context, cacheKey string, logoutToken *oidc.LogoutToken) error { if s.EventsPublisher == nil { return fmt.Errorf("the events publisher is not set") } - urecords, err := s.UserInfoCache.Read(string(record.Value)) + urecords, err := s.UserInfoCache.Read(cacheKey) if err != nil { return fmt.Errorf("reading userinfo cache: %w", err) } From 66d220ff442f44d579919cfde2c54f69e2353495 Mon Sep 17 00:00:00 2001 From: Florian Schade Date: Fri, 13 Feb 2026 20:41:16 +0100 Subject: [PATCH 05/15] enhancement: finalize backchannel logout --- services/proxy/pkg/command/server.go | 2 + .../pkg/config/defaults/defaultconfig.go | 2 + services/proxy/pkg/middleware/oidc_auth.go | 50 ++-- .../pkg/staticroutes/backchannellogout.go | 129 +++++------ .../backchannellogout/backchannellogout.go | 125 ++++++++++ .../backchannellogout_test.go | 213 ++++++++++++++++++ .../proxy/pkg/staticroutes/staticroutes.go | 3 +- 7 files changed, 430 insertions(+), 94 deletions(-) create mode 100644 services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go create mode 100644 services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go diff --git a/services/proxy/pkg/command/server.go b/services/proxy/pkg/command/server.go index a1f2f842dd..b3e8332480 100644 --- a/services/proxy/pkg/command/server.go +++ b/services/proxy/pkg/command/server.go @@ -11,6 +11,7 @@ import ( gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" chimiddleware "github.com/go-chi/chi/v5/middleware" "github.com/justinas/alice" + "github.com/opencloud-eu/opencloud/pkg/config/configlog" "github.com/opencloud-eu/opencloud/pkg/generators" "github.com/opencloud-eu/opencloud/pkg/log" @@ -73,6 +74,7 @@ func Server(cfg *config.Config) *cli.Command { microstore.Nodes(cfg.PreSignedURL.SigningKeys.Nodes...), microstore.Database("proxy"), microstore.Table("signing-keys"), + store.DisablePersistence(cfg.PreSignedURL.SigningKeys.DisablePersistence), store.Authentication(cfg.PreSignedURL.SigningKeys.AuthUsername, cfg.PreSignedURL.SigningKeys.AuthPassword), ) diff --git a/services/proxy/pkg/config/defaults/defaultconfig.go b/services/proxy/pkg/config/defaults/defaultconfig.go index 9e0985003a..bbdd82a272 100644 --- a/services/proxy/pkg/config/defaults/defaultconfig.go +++ b/services/proxy/pkg/config/defaults/defaultconfig.go @@ -45,6 +45,8 @@ func DefaultConfig() *config.Config { AccessTokenVerifyMethod: config.AccessTokenVerificationJWT, SkipUserInfo: false, UserinfoCache: &config.Cache{ + // toDo: + // check if "memory" is the right default or if "nats-js-kv" is a better match Store: "memory", Nodes: []string{"127.0.0.1:9233"}, Database: "cache-userinfo", diff --git a/services/proxy/pkg/middleware/oidc_auth.go b/services/proxy/pkg/middleware/oidc_auth.go index dcb82f54d4..342e5bbbe9 100644 --- a/services/proxy/pkg/middleware/oidc_auth.go +++ b/services/proxy/pkg/middleware/oidc_auth.go @@ -3,20 +3,20 @@ package middleware import ( "context" "encoding/base64" - "fmt" "net" "net/http" "strings" "time" "github.com/golang-jwt/jwt/v5" - "github.com/opencloud-eu/opencloud/pkg/log" - "github.com/opencloud-eu/opencloud/pkg/oidc" "github.com/pkg/errors" "github.com/vmihailenco/msgpack/v5" - store "go-micro.dev/v4/store" + "go-micro.dev/v4/store" "golang.org/x/crypto/sha3" "golang.org/x/oauth2" + + "github.com/opencloud-eu/opencloud/pkg/log" + "github.com/opencloud-eu/opencloud/pkg/oidc" ) const ( @@ -115,28 +115,26 @@ func (m *OIDCAuthenticator) getClaims(token string, req *http.Request) (map[stri m.Logger.Error().Err(err).Msg("failed to write to userinfo cache") } - if sid := aClaims.SessionID; sid != "" { - // reuse user cache for session id lookup - err = m.userInfoCache.Write(&store.Record{ - Key: sid, - Value: []byte(encodedHash), - Expiry: time.Until(expiration), - }) - if err != nil { - m.Logger.Error().Err(err).Msg("failed to write session lookup cache") - } - - // create an additional entry mapping subject to session id - if sub := aClaims.Subject; sub != "" { - err = m.userInfoCache.Write(&store.Record{ - Key: fmt.Sprintf("%s.%s", sub, sid), - Value: []byte(sid), - Expiry: time.Until(expiration), - }) - if err != nil { - m.Logger.Error().Err(err).Msg("failed to write subject lookup cache") - } - } + subject, sessionId := aClaims.Subject, aClaims.SessionID + // if no session id is present, we can't do a session lookup, + // so we can skip the cache entry for that. + if sessionId == "" { + return + } + + // if the claim has no subject, we can leave it empty, + // it's important to keep the dot in the key to prevent + // sufix and prefix exploration in the cache. + // + // ok: {key: ".sessionId"} + // ok: {key: "subject.sessionId"} + key := strings.Join([]string{subject, sessionId}, ".") + if err := m.userInfoCache.Write(&store.Record{ + Key: key, + Value: []byte(encodedHash), + Expiry: time.Until(expiration), + }); err != nil { + m.Logger.Error().Err(err).Msg("failed to write session lookup cache") } } }() diff --git a/services/proxy/pkg/staticroutes/backchannellogout.go b/services/proxy/pkg/staticroutes/backchannellogout.go index eb5574112c..4dee667607 100644 --- a/services/proxy/pkg/staticroutes/backchannellogout.go +++ b/services/proxy/pkg/staticroutes/backchannellogout.go @@ -7,15 +7,24 @@ import ( "strings" "github.com/go-chi/render" - "github.com/opencloud-eu/opencloud/pkg/oidc" - "github.com/opencloud-eu/reva/v2/pkg/events" - "github.com/opencloud-eu/reva/v2/pkg/utils" "github.com/pkg/errors" "github.com/vmihailenco/msgpack/v5" microstore "go-micro.dev/v4/store" + + bcl "github.com/opencloud-eu/opencloud/services/proxy/pkg/staticroutes/internal/backchannellogout" + "github.com/opencloud-eu/reva/v2/pkg/events" + "github.com/opencloud-eu/reva/v2/pkg/utils" ) -// handle backchannel logout requests as per https://openid.net/specs/openid-connect-backchannel-1_0.html#BCRequest +// BackchannelLogout handles backchannel logout requests from the identity provider and invalidates the related sessions in the cache +// spec: https://openid.net/specs/openid-connect-backchannel-1_0.html#BCRequest +// +// toDo: +// - keyCloak "Sign out all active sessions" fails to log out, no incoming request +// - if the keycloak setting "Backchannel logout session required" is disabled, +// we resolve the session by the subject which can lead to multiple session records, +// we then send a logout event to each connected client and delete our stored record (subject.session & claim). +// but the session still exists in the identity provider. func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Request) { // parse the application/x-www-form-urlencoded POST request logger := s.Logger.SubloggerWithRequestID(r.Context()) @@ -26,13 +35,6 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re return } - if r.PostFormValue("logout_token") == "" { - logger.Warn().Msg("logout_token is missing") - render.Status(r, http.StatusBadRequest) - render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: "logout_token is missing"}) - return - } - logoutToken, err := s.OidcClient.VerifyLogoutToken(r.Context(), r.PostFormValue("logout_token")) if err != nil { logger.Warn().Err(err).Msg("VerifyLogoutToken failed") @@ -41,70 +43,63 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re return } - var cacheKeys map[string]bool + subject, session := strings.Join(strings.Fields(logoutToken.Subject), ""), strings.Join(strings.Fields(logoutToken.SessionId), "") + if subject == "" && session == "" { + jseErr := jse{Error: "invalid_request", ErrorDescription: "invalid logout token: subject and session id are empty"} + logger.Warn().Msg(jseErr.ErrorDescription) + render.Status(r, http.StatusBadRequest) + render.JSON(w, r, jseErr) + return + } - if strings.TrimSpace(logoutToken.SessionId) != "" { - cacheKeys[logoutToken.SessionId] = true - } else if strings.TrimSpace(logoutToken.Subject) != "" { - records, err := s.UserInfoCache.Read(fmt.Sprintf("%s.*", logoutToken.Subject)) - if errors.Is(err, microstore.ErrNotFound) || len(records) == 0 { - render.Status(r, http.StatusBadRequest) - render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) - return - } - if err != nil { - logger.Error().Err(err).Msg("Error reading userinfo cache") - render.Status(r, http.StatusBadRequest) - render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) - return - } - for _, record := range records { - cacheKeys[string(record.Value)] = true - cacheKeys[record.Key] = false - } - } else { - logger.Warn().Msg("invalid logout token") + requestSubjectAndSession := bcl.SuSe{Session: session, Subject: subject} + // find out which mode of backchannel logout we are in + // by checking if the session or subject is present in the token + logoutMode := bcl.GetLogoutMode(requestSubjectAndSession) + lookupRecords, err := bcl.GetLogoutRecords(requestSubjectAndSession, logoutMode, s.UserInfoCache) + if errors.Is(err, microstore.ErrNotFound) || len(lookupRecords) == 0 { + render.Status(r, http.StatusOK) + render.JSON(w, r, nil) + return + } + if err != nil { + logger.Error().Err(err).Msg("Error reading userinfo cache") render.Status(r, http.StatusBadRequest) - render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: "invalid logout token"}) + render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) return } - for key, isSID := range cacheKeys { - if isSID { - records, err := s.UserInfoCache.Read(key) - if err != nil && !errors.Is(err, microstore.ErrNotFound) { - logger.Error().Err(err).Msg("Error reading userinfo cache") - render.Status(r, http.StatusBadRequest) - render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) - return - } - for _, record := range records { - err = s.UserInfoCache.Delete(string(record.Value)) - if err != nil && !errors.Is(err, microstore.ErrNotFound) { - logger.Error().Err(err).Msg("Error deleting userinfo cache") - render.Status(r, http.StatusBadRequest) - render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) - return - } - } + for _, record := range lookupRecords { + // the record key is in the format "subject.session" or ".session" + // the record value is the key of the record that contains the claim in its value + key, value := record.Key, string(record.Value) + + subjectSession, ok := bcl.NewSuSe(key) + if !ok { + logger.Warn().Msgf("invalid logout record key: %s", key) + continue } - err = s.UserInfoCache.Delete(key) + err := s.publishBackchannelLogoutEvent(r.Context(), subjectSession.Session, value) + if err != nil { + s.Logger.Warn().Err(err).Msg("could not publish backchannel logout event") + } + + err = s.UserInfoCache.Delete(value) if err != nil && !errors.Is(err, microstore.ErrNotFound) { - // Spec requires us to return a 400 BadRequest when the session could not be destroyed - logger.Err(err).Msg(fmt.Errorf("could not delete session from cache (%s)", key).Error()) - // We only return on requests that do only attempt to destroy a single session, not multiple + // we have to return a 400 BadRequest when we fail to delete the session + // https://openid.net/specs/openid-connect-backchannel-1_0.html#rfc.section.2.8 + logger.Err(err).Msg("could not delete user info from cache") render.Status(r, http.StatusBadRequest) render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) return } - if isSID { - err := s.publishBackchannelLogoutEvent(r.Context(), key, logoutToken) - if err != nil { - s.Logger.Warn().Err(err).Msg("could not publish backchannel logout event") - } + + // we can ignore errors when deleting the lookup record + err = s.UserInfoCache.Delete(key) + if err != nil { + logger.Debug().Err(err).Msg("Failed to cleanup sessionid lookup entry") } - logger.Debug().Msg("Deleted userinfo from cache") } render.Status(r, http.StatusOK) @@ -112,20 +107,20 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re } // publishBackchannelLogoutEvent publishes a backchannel logout event when the callback revived from the identity provider -func (s StaticRouteHandler) publishBackchannelLogoutEvent(ctx context.Context, cacheKey string, logoutToken *oidc.LogoutToken) error { +func (s *StaticRouteHandler) publishBackchannelLogoutEvent(ctx context.Context, sessionId, claimKey string) error { if s.EventsPublisher == nil { return fmt.Errorf("the events publisher is not set") } - urecords, err := s.UserInfoCache.Read(cacheKey) + claimRecords, err := s.UserInfoCache.Read(claimKey) if err != nil { return fmt.Errorf("reading userinfo cache: %w", err) } - if len(urecords) == 0 { + if len(claimRecords) == 0 { return fmt.Errorf("userinfo not found") } var claims map[string]interface{} - if err = msgpack.Unmarshal(urecords[0].Value, &claims); err != nil { + if err = msgpack.Unmarshal(claimRecords[0].Value, &claims); err != nil { return fmt.Errorf("could not unmarshal userinfo: %w", err) } @@ -141,7 +136,7 @@ func (s StaticRouteHandler) publishBackchannelLogoutEvent(ctx context.Context, c e := events.BackchannelLogout{ Executant: user.GetId(), - SessionId: logoutToken.SessionId, + SessionId: sessionId, Timestamp: utils.TSNow(), } diff --git a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go new file mode 100644 index 0000000000..92f9ab0d59 --- /dev/null +++ b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go @@ -0,0 +1,125 @@ +// package backchannellogout provides functions to classify and lookup +// backchannel logout records from the cache store. + +package backchannellogout + +import ( + "fmt" + "strings" + + "github.com/pkg/errors" + microstore "go-micro.dev/v4/store" +) + +// SuSe 🦎 ;) is a struct that groups the subject and session together +// to prevent mix-ups for ('session, subject' || 'subject, session') +// return values. +type SuSe struct { + Subject string + Session string +} + +// NewSuSe parses the subject and session id from the given key and returns a SuSe struct +func NewSuSe(key string) (SuSe, bool) { + var subject, session string + switch keys := strings.Split(strings.Join(strings.Fields(key), ""), "."); { + case len(keys) == 2 && keys[0] == "" && keys[1] != "": + session = keys[1] + case len(keys) == 2 && keys[0] != "" && keys[1] == "": + subject = keys[0] + case len(keys) == 2 && keys[0] != "" && keys[1] != "": + subject = keys[0] + session = keys[1] + case len(keys) == 1 && keys[0] != "": + session = keys[0] + default: + return SuSe{}, false + } + + return SuSe{Session: session, Subject: subject}, true +} + +// LogoutMode defines the mode of backchannel logout, either by session or by subject +type LogoutMode int + +const ( + // LogoutModeUnknown is used when the logout mode cannot be determined + LogoutModeUnknown LogoutMode = iota + // LogoutModeSession is used when the logout mode is determined by the session id + LogoutModeSession + // LogoutModeSubject is used when the logout mode is determined by the subject + LogoutModeSubject +) + +// GetLogoutMode determines the backchannel logout mode based on the presence of subject and session in the SuSe struct +func GetLogoutMode(suse SuSe) LogoutMode { + switch { + case suse.Session != "": + return LogoutModeSession + case suse.Subject != "": + return LogoutModeSubject + default: + return LogoutModeUnknown + } +} + +// ErrSuspiciousCacheResult is returned when the cache result is suspicious +var ErrSuspiciousCacheResult = errors.New("suspicious cache result") + +// GetLogoutRecords retrieves the records from the user info cache based on the backchannel +// logout mode and the provided SuSe struct. +// it uses a seperator to prevent sufix and prefix exploration in the cache and checks +// if the retrieved records match the requested subject and or session id as well, to prevent false positives. +func GetLogoutRecords(suse SuSe, mode LogoutMode, store microstore.Store) ([]*microstore.Record, error) { + var key string + var opts []microstore.ReadOption + switch mode { + case LogoutModeSession: + // the dot at the beginning prevents sufix exploration in the cache, + // so only keys that end with '*.session' will be returned, but not '*sion'. + key = "." + suse.Session + opts = append(opts, microstore.ReadSuffix()) + case LogoutModeSubject: + // the dot at the end prevents prefix exploration in the cache, + // so only keys that start with 'subject.*' will be returned, but not 'sub*'. + key = suse.Subject + "." + opts = append(opts, microstore.ReadPrefix()) + default: + return nil, fmt.Errorf("%w: cannot determine logout mode", ErrSuspiciousCacheResult) + } + + records, err := store.Read(key, append(opts, microstore.ReadLimit(1000))...) + if err != nil { + return nil, err + } + + if len(records) == 0 { + return nil, microstore.ErrNotFound + } + + if mode == LogoutModeSession && len(records) > 1 { + return nil, fmt.Errorf("%w: multiple session records found", ErrSuspiciousCacheResult) + } + + // double-check if the found records match the requested subject and or session id as well, + // to prevent false positives. + for _, record := range records { + recordSuSe, ok := NewSuSe(record.Key) + if !ok { + return nil, microstore.ErrNotFound + } + + switch { + // in session mode, the session id must match, but the subject can be different + case mode == LogoutModeSession && suse.Session == recordSuSe.Session: + continue + // in subject mode, the subject must match, but the session id can be different + case mode == LogoutModeSubject && suse.Subject == recordSuSe.Subject: + continue + } + + return nil, fmt.Errorf("%w: record key does not match the requested subject or session", ErrSuspiciousCacheResult) + } + + return records, nil +} diff --git a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go new file mode 100644 index 0000000000..9250a4885b --- /dev/null +++ b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go @@ -0,0 +1,213 @@ +package backchannellogout + +import ( + "slices" + "strings" + "testing" + + "github.com/stretchr/testify/require" + "go-micro.dev/v4/store" +) + +func TestNewSuSe(t *testing.T) { + tests := []struct { + name string + key string + wantsuSe SuSe + wantOk bool + }{ + { + name: ".session", + key: ".session", + wantsuSe: SuSe{Session: "session", Subject: ""}, + wantOk: true, + }, + { + name: ".session", + key: ".session", + wantsuSe: SuSe{Session: "session", Subject: ""}, + wantOk: true, + }, + { + name: "session", + key: "session", + wantsuSe: SuSe{Session: "session", Subject: ""}, + wantOk: true, + }, + { + name: "subject.", + key: "subject.", + wantsuSe: SuSe{Session: "", Subject: "subject"}, + wantOk: true, + }, + { + name: "subject.session", + key: "subject.session", + wantsuSe: SuSe{Session: "session", Subject: "subject"}, + wantOk: true, + }, + { + name: "dot", + key: ".", + wantsuSe: SuSe{Session: "", Subject: ""}, + wantOk: false, + }, + { + name: "empty", + key: "", + wantsuSe: SuSe{Session: "", Subject: ""}, + wantOk: false, + }, + { + name: "whitespace . whitespace", + key: " . ", + wantsuSe: SuSe{Session: "", Subject: ""}, + wantOk: false, + }, + { + name: "whitespace subject whitespace . whitespace", + key: " subject . ", + wantsuSe: SuSe{Session: "", Subject: "subject"}, + wantOk: true, + }, + { + name: "whitespace . whitespace session whitespace", + key: " . session ", + wantsuSe: SuSe{Session: "session", Subject: ""}, + wantOk: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + suSe, ok := NewSuSe(tt.key) + require.Equal(t, tt.wantOk, ok) + require.Equal(t, tt.wantsuSe, suSe) + }) + } +} + +func TestGetLogoutMode(t *testing.T) { + tests := []struct { + name string + suSe SuSe + want LogoutMode + }{ + { + name: ".session", + suSe: SuSe{Session: "session", Subject: ""}, + want: LogoutModeSession, + }, + { + name: "subject.session", + suSe: SuSe{Session: "session", Subject: "subject"}, + want: LogoutModeSession, + }, + { + name: "subject.", + suSe: SuSe{Session: "", Subject: "subject"}, + want: LogoutModeSubject, + }, + { + name: "", + suSe: SuSe{Session: "", Subject: ""}, + want: LogoutModeUnknown, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mode := GetLogoutMode(tt.suSe) + require.Equal(t, tt.want, mode) + }) + } +} + +func TestGetLogoutRecords(t *testing.T) { + sessionStore := store.NewMemoryStore() + + recordClaimA := &store.Record{Key: "claim-a", Value: []byte("claim-a-data")} + recordClaimB := &store.Record{Key: "claim-b", Value: []byte("claim-b-data")} + recordClaimC := &store.Record{Key: "claim-c", Value: []byte("claim-c-data")} + recordClaimD := &store.Record{Key: "claim-d", Value: []byte("claim-d-data")} + recordSessionA := &store.Record{Key: "session-a", Value: []byte(recordClaimA.Key)} + recordSessionB := &store.Record{Key: "session-b", Value: []byte(recordClaimB.Key)} + recordSubjectASessionC := &store.Record{Key: "subject-a.session-c", Value: []byte(recordSessionA.Key)} + recordSubjectASessionD := &store.Record{Key: "subject-a.session-d", Value: []byte(recordSessionB.Key)} + + for _, r := range []*store.Record{ + recordClaimA, + recordClaimB, + recordClaimC, + recordClaimD, + recordSessionA, + recordSessionB, + recordSubjectASessionC, + recordSubjectASessionD, + } { + require.NoError(t, sessionStore.Write(r)) + } + + tests := []struct { + name string + suSe SuSe + mode LogoutMode + store store.Store + wantRecords []*store.Record + wantError error + }{ + { + name: "session-c", + suSe: SuSe{Session: "session-c"}, + mode: LogoutModeSession, + store: sessionStore, + wantRecords: []*store.Record{recordSubjectASessionC}, + }, + { + name: "ession-c", + suSe: SuSe{Session: "ession-c"}, + mode: LogoutModeSession, + store: sessionStore, + wantError: store.ErrNotFound, + wantRecords: []*store.Record{}, + }, + { + name: "subject-a", + suSe: SuSe{Subject: "subject-a"}, + mode: LogoutModeSubject, + store: sessionStore, + wantRecords: []*store.Record{recordSubjectASessionC, recordSubjectASessionD}, + }, + { + name: "subject-", + suSe: SuSe{Subject: "subject-"}, + mode: LogoutModeSubject, + store: sessionStore, + wantError: store.ErrNotFound, + wantRecords: []*store.Record{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + records, err := GetLogoutRecords(tt.suSe, tt.mode, tt.store) + require.ErrorIs(t, err, tt.wantError) + require.Len(t, records, len(tt.wantRecords)) + + sortRecords := func(r []*store.Record) []*store.Record { + slices.SortFunc(r, func(a, b *store.Record) int { + return strings.Compare(a.Key, b.Key) + }) + + return r + } + + records = sortRecords(records) + for i, wantRecords := range sortRecords(tt.wantRecords) { + require.True(t, len(records) >= i+1) + require.Equal(t, wantRecords.Key, records[i].Key) + require.Equal(t, wantRecords.Value, records[i].Value) + } + }) + } +} diff --git a/services/proxy/pkg/staticroutes/staticroutes.go b/services/proxy/pkg/staticroutes/staticroutes.go index f2f76740f1..61d4f0ca5b 100644 --- a/services/proxy/pkg/staticroutes/staticroutes.go +++ b/services/proxy/pkg/staticroutes/staticroutes.go @@ -4,12 +4,13 @@ import ( "net/http" "github.com/go-chi/chi/v5" + microstore "go-micro.dev/v4/store" + "github.com/opencloud-eu/opencloud/pkg/log" "github.com/opencloud-eu/opencloud/pkg/oidc" "github.com/opencloud-eu/opencloud/services/proxy/pkg/config" "github.com/opencloud-eu/opencloud/services/proxy/pkg/user/backend" "github.com/opencloud-eu/reva/v2/pkg/events" - microstore "go-micro.dev/v4/store" ) // StaticRouteHandler defines a Route Handler for static routes From a9660e3e8ca207754c9af5b7ea654a1169db3181 Mon Sep 17 00:00:00 2001 From: Florian Schade Date: Mon, 16 Feb 2026 13:44:01 +0100 Subject: [PATCH 06/15] enhancement: document idp side-effects --- .../pkg/staticroutes/backchannellogout.go | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/services/proxy/pkg/staticroutes/backchannellogout.go b/services/proxy/pkg/staticroutes/backchannellogout.go index 4dee667607..c213e090a1 100644 --- a/services/proxy/pkg/staticroutes/backchannellogout.go +++ b/services/proxy/pkg/staticroutes/backchannellogout.go @@ -16,17 +16,28 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/utils" ) -// BackchannelLogout handles backchannel logout requests from the identity provider and invalidates the related sessions in the cache +// backchannelLogout handles backchannel logout requests from the identity provider and invalidates the related sessions in the cache // spec: https://openid.net/specs/openid-connect-backchannel-1_0.html#BCRequest // -// toDo: -// - keyCloak "Sign out all active sessions" fails to log out, no incoming request -// - if the keycloak setting "Backchannel logout session required" is disabled, -// we resolve the session by the subject which can lead to multiple session records, -// we then send a logout event to each connected client and delete our stored record (subject.session & claim). -// but the session still exists in the identity provider. +// known side effects of backchannel logout in keycloak: +// +// - keyCloak "Sign out all active sessions" does not send a backchannel logout request, +// as the devs mention, this may lead to thousands of backchannel logout requests, +// therefore, they recommend a short token lifetime. +// https://github.com/keycloak/keycloak/issues/27342#issuecomment-2408461913 +// +// - keyCloak user self-service portal, "Sign out all devices" may not send a backchannel +// logout request for each session, it's not mentionex explicitly, +// but maybe the reason for that is the same as for "Sign out all active sessions" +// to prevent a flood of backchannel logout requests. +// +// - if the keycloak setting "Backchannel logout session required" is disabled (or the token has no session id), +// we resolve the session by the subject which can lead to multiple session records (subject.*), +// we then send a logout event (sse) to each connected client and delete our stored cache record (subject.session & claim). +// all sessions besides the one that triggered the backchannel logout continue to exist in the identity provider, +// so the user will not be fully logged out until all sessions are logged out or expired. +// this leads to the situation that web renders the logout view even if the instance is not fully logged out yet. func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Request) { - // parse the application/x-www-form-urlencoded POST request logger := s.Logger.SubloggerWithRequestID(r.Context()) if err := r.ParseForm(); err != nil { logger.Warn().Err(err).Msg("ParseForm failed") From f7a86d681a5488aa40bdee65231d2ee7e05f1d96 Mon Sep 17 00:00:00 2001 From: Florian Schade Date: Mon, 16 Feb 2026 14:02:57 +0100 Subject: [PATCH 07/15] chore: cleanup backchannel logout pr for review --- services/proxy/pkg/config/defaults/defaultconfig.go | 2 -- services/proxy/pkg/middleware/oidc_auth.go | 2 +- services/proxy/pkg/staticroutes/staticroutes.go | 3 +-- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/services/proxy/pkg/config/defaults/defaultconfig.go b/services/proxy/pkg/config/defaults/defaultconfig.go index bbdd82a272..9e0985003a 100644 --- a/services/proxy/pkg/config/defaults/defaultconfig.go +++ b/services/proxy/pkg/config/defaults/defaultconfig.go @@ -45,8 +45,6 @@ func DefaultConfig() *config.Config { AccessTokenVerifyMethod: config.AccessTokenVerificationJWT, SkipUserInfo: false, UserinfoCache: &config.Cache{ - // toDo: - // check if "memory" is the right default or if "nats-js-kv" is a better match Store: "memory", Nodes: []string{"127.0.0.1:9233"}, Database: "cache-userinfo", diff --git a/services/proxy/pkg/middleware/oidc_auth.go b/services/proxy/pkg/middleware/oidc_auth.go index 342e5bbbe9..17f9e8edba 100644 --- a/services/proxy/pkg/middleware/oidc_auth.go +++ b/services/proxy/pkg/middleware/oidc_auth.go @@ -115,7 +115,7 @@ func (m *OIDCAuthenticator) getClaims(token string, req *http.Request) (map[stri m.Logger.Error().Err(err).Msg("failed to write to userinfo cache") } - subject, sessionId := aClaims.Subject, aClaims.SessionID + subject, sessionId := strings.Join(strings.Fields(aClaims.Subject), ""), strings.Join(strings.Fields(aClaims.SessionID), "") // if no session id is present, we can't do a session lookup, // so we can skip the cache entry for that. if sessionId == "" { diff --git a/services/proxy/pkg/staticroutes/staticroutes.go b/services/proxy/pkg/staticroutes/staticroutes.go index 61d4f0ca5b..f2f76740f1 100644 --- a/services/proxy/pkg/staticroutes/staticroutes.go +++ b/services/proxy/pkg/staticroutes/staticroutes.go @@ -4,13 +4,12 @@ import ( "net/http" "github.com/go-chi/chi/v5" - microstore "go-micro.dev/v4/store" - "github.com/opencloud-eu/opencloud/pkg/log" "github.com/opencloud-eu/opencloud/pkg/oidc" "github.com/opencloud-eu/opencloud/services/proxy/pkg/config" "github.com/opencloud-eu/opencloud/services/proxy/pkg/user/backend" "github.com/opencloud-eu/reva/v2/pkg/events" + microstore "go-micro.dev/v4/store" ) // StaticRouteHandler defines a Route Handler for static routes From 6af2c44f7fcc8b657fa674d3409317943e3340ee Mon Sep 17 00:00:00 2001 From: Florian Schade Date: Mon, 16 Feb 2026 23:20:39 +0100 Subject: [PATCH 08/15] test: add more backchannellogout tests --- services/proxy/.mockery.yaml | 5 + .../pkg/staticroutes/backchannellogout.go | 15 +- .../backchannellogout/backchannellogout.go | 16 +- .../backchannellogout_test.go | 222 +++++--- .../internal/backchannellogout/mocks/store.go | 509 ++++++++++++++++++ 5 files changed, 693 insertions(+), 74 deletions(-) create mode 100644 services/proxy/pkg/staticroutes/internal/backchannellogout/mocks/store.go diff --git a/services/proxy/.mockery.yaml b/services/proxy/.mockery.yaml index a490457301..d3ae0a3817 100644 --- a/services/proxy/.mockery.yaml +++ b/services/proxy/.mockery.yaml @@ -12,3 +12,8 @@ packages: github.com/opencloud-eu/opencloud/services/proxy/pkg/userroles: interfaces: UserRoleAssigner: {} + go-micro.dev/v4/store: + config: + dir: pkg/staticroutes/internal/backchannellogout/mocks + interfaces: + Store: {} diff --git a/services/proxy/pkg/staticroutes/backchannellogout.go b/services/proxy/pkg/staticroutes/backchannellogout.go index c213e090a1..0d44e7cd1c 100644 --- a/services/proxy/pkg/staticroutes/backchannellogout.go +++ b/services/proxy/pkg/staticroutes/backchannellogout.go @@ -37,6 +37,9 @@ import ( // all sessions besides the one that triggered the backchannel logout continue to exist in the identity provider, // so the user will not be fully logged out until all sessions are logged out or expired. // this leads to the situation that web renders the logout view even if the instance is not fully logged out yet. +// +// toDo: +// - check logs and errors to not contain any sensitive information like session ids or user ids (keys too) func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Request) { logger := s.Logger.SubloggerWithRequestID(r.Context()) if err := r.ParseForm(); err != nil { @@ -85,14 +88,14 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re // the record value is the key of the record that contains the claim in its value key, value := record.Key, string(record.Value) - subjectSession, ok := bcl.NewSuSe(key) - if !ok { - logger.Warn().Msgf("invalid logout record key: %s", key) + subjectSession, err := bcl.NewSuSe(key) + if err != nil { + // never leak any key-related information + logger.Warn().Err(err).Msgf("invalid logout record key: %s", "XXX") continue } - err := s.publishBackchannelLogoutEvent(r.Context(), subjectSession.Session, value) - if err != nil { + if err := s.publishBackchannelLogoutEvent(r.Context(), subjectSession.Session, value); err != nil { s.Logger.Warn().Err(err).Msg("could not publish backchannel logout event") } @@ -109,7 +112,7 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re // we can ignore errors when deleting the lookup record err = s.UserInfoCache.Delete(key) if err != nil { - logger.Debug().Err(err).Msg("Failed to cleanup sessionid lookup entry") + logger.Debug().Err(err).Msg("Failed to cleanup sessionId lookup entry") } } diff --git a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go index 92f9ab0d59..306febd0ca 100644 --- a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go +++ b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go @@ -19,8 +19,11 @@ type SuSe struct { Session string } +// ErrInvalidSessionOrSubject is returned when the provided key does not match the expected key format +var ErrInvalidSessionOrSubject = errors.New("invalid session or subject") + // NewSuSe parses the subject and session id from the given key and returns a SuSe struct -func NewSuSe(key string) (SuSe, bool) { +func NewSuSe(key string) (SuSe, error) { var subject, session string switch keys := strings.Split(strings.Join(strings.Fields(key), ""), "."); { case len(keys) == 2 && keys[0] == "" && keys[1] != "": @@ -33,10 +36,10 @@ func NewSuSe(key string) (SuSe, bool) { case len(keys) == 1 && keys[0] != "": session = keys[0] default: - return SuSe{}, false + return SuSe{}, ErrInvalidSessionOrSubject } - return SuSe{Session: session, Subject: subject}, true + return SuSe{Session: session, Subject: subject}, nil } // LogoutMode defines the mode of backchannel logout, either by session or by subject @@ -104,9 +107,10 @@ func GetLogoutRecords(suse SuSe, mode LogoutMode, store microstore.Store) ([]*mi // double-check if the found records match the requested subject and or session id as well, // to prevent false positives. for _, record := range records { - recordSuSe, ok := NewSuSe(record.Key) - if !ok { - return nil, microstore.ErrNotFound + recordSuSe, err := NewSuSe(record.Key) + if err != nil { + // never leak any key-related information + return nil, fmt.Errorf("%w %w: failed to parse logout record key: %s", err, ErrSuspiciousCacheResult, "XXX") } switch { diff --git a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go index 9250a4885b..238323c728 100644 --- a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go +++ b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go @@ -5,84 +5,80 @@ import ( "strings" "testing" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "go-micro.dev/v4/store" + + "github.com/opencloud-eu/opencloud/services/proxy/pkg/staticroutes/internal/backchannellogout/mocks" ) func TestNewSuSe(t *testing.T) { tests := []struct { name string key string - wantsuSe SuSe - wantOk bool + wantSuSe SuSe + wantErr error }{ { - name: ".session", + name: "key variation: '.session'", key: ".session", - wantsuSe: SuSe{Session: "session", Subject: ""}, - wantOk: true, + wantSuSe: SuSe{Session: "session", Subject: ""}, }, { - name: ".session", + name: "key variation: '.session'", key: ".session", - wantsuSe: SuSe{Session: "session", Subject: ""}, - wantOk: true, + wantSuSe: SuSe{Session: "session", Subject: ""}, }, { - name: "session", + name: "key variation: 'session'", key: "session", - wantsuSe: SuSe{Session: "session", Subject: ""}, - wantOk: true, + wantSuSe: SuSe{Session: "session", Subject: ""}, }, { - name: "subject.", + name: "key variation: 'subject.'", key: "subject.", - wantsuSe: SuSe{Session: "", Subject: "subject"}, - wantOk: true, + wantSuSe: SuSe{Session: "", Subject: "subject"}, }, { - name: "subject.session", + name: "key variation: 'subject.session'", key: "subject.session", - wantsuSe: SuSe{Session: "session", Subject: "subject"}, - wantOk: true, + wantSuSe: SuSe{Session: "session", Subject: "subject"}, }, { - name: "dot", + name: "key variation: 'dot'", key: ".", - wantsuSe: SuSe{Session: "", Subject: ""}, - wantOk: false, + wantSuSe: SuSe{Session: "", Subject: ""}, + wantErr: ErrInvalidSessionOrSubject, }, { - name: "empty", + name: "key variation: 'empty'", key: "", - wantsuSe: SuSe{Session: "", Subject: ""}, - wantOk: false, + wantSuSe: SuSe{Session: "", Subject: ""}, + wantErr: ErrInvalidSessionOrSubject, }, { - name: "whitespace . whitespace", + name: "key variation: 'whitespace . whitespace'", key: " . ", - wantsuSe: SuSe{Session: "", Subject: ""}, - wantOk: false, + wantSuSe: SuSe{Session: "", Subject: ""}, + wantErr: ErrInvalidSessionOrSubject, }, { - name: "whitespace subject whitespace . whitespace", + name: "key variation: 'whitespace subject whitespace . whitespace'", key: " subject . ", - wantsuSe: SuSe{Session: "", Subject: "subject"}, - wantOk: true, + wantSuSe: SuSe{Session: "", Subject: "subject"}, }, { - name: "whitespace . whitespace session whitespace", + name: "key variation: 'whitespace . whitespace session whitespace'", key: " . session ", - wantsuSe: SuSe{Session: "session", Subject: ""}, - wantOk: true, + wantSuSe: SuSe{Session: "session", Subject: ""}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { suSe, ok := NewSuSe(tt.key) - require.Equal(t, tt.wantOk, ok) - require.Equal(t, tt.wantsuSe, suSe) + require.ErrorIs(t, tt.wantErr, ok) + require.Equal(t, tt.wantSuSe, suSe) }) } } @@ -94,22 +90,22 @@ func TestGetLogoutMode(t *testing.T) { want LogoutMode }{ { - name: ".session", + name: "key variation: '.session'", suSe: SuSe{Session: "session", Subject: ""}, want: LogoutModeSession, }, { - name: "subject.session", + name: "key variation: 'subject.session'", suSe: SuSe{Session: "session", Subject: "subject"}, want: LogoutModeSession, }, { - name: "subject.", + name: "key variation: 'subject.'", suSe: SuSe{Session: "", Subject: "subject"}, want: LogoutModeSubject, }, { - name: "", + name: "key variation: 'empty'", suSe: SuSe{Session: "", Subject: ""}, want: LogoutModeUnknown, }, @@ -130,8 +126,8 @@ func TestGetLogoutRecords(t *testing.T) { recordClaimB := &store.Record{Key: "claim-b", Value: []byte("claim-b-data")} recordClaimC := &store.Record{Key: "claim-c", Value: []byte("claim-c-data")} recordClaimD := &store.Record{Key: "claim-d", Value: []byte("claim-d-data")} - recordSessionA := &store.Record{Key: "session-a", Value: []byte(recordClaimA.Key)} - recordSessionB := &store.Record{Key: "session-b", Value: []byte(recordClaimB.Key)} + recordSessionA := &store.Record{Key: ".session-a", Value: []byte(recordClaimA.Key)} + recordSessionB := &store.Record{Key: ".session-b", Value: []byte(recordClaimB.Key)} recordSubjectASessionC := &store.Record{Key: "subject-a.session-c", Value: []byte(recordSessionA.Key)} recordSubjectASessionD := &store.Record{Key: "subject-a.session-d", Value: []byte(recordSessionB.Key)} @@ -152,46 +148,148 @@ func TestGetLogoutRecords(t *testing.T) { name string suSe SuSe mode LogoutMode - store store.Store + store func(t *testing.T) store.Store wantRecords []*store.Record - wantError error + wantErrs []error }{ { - name: "session-c", - suSe: SuSe{Session: "session-c"}, - mode: LogoutModeSession, - store: sessionStore, + name: "fails if mode is unknown", + suSe: SuSe{Session: "session-a"}, + mode: LogoutModeUnknown, + store: func(t *testing.T) store.Store { + return sessionStore + }, + wantRecords: []*store.Record{}, + wantErrs: []error{ErrSuspiciousCacheResult}, + }, + { + name: "fails if mode is any random int", + suSe: SuSe{Session: "session-a"}, + mode: 999, + store: func(t *testing.T) store.Store { + return sessionStore + }, + wantRecords: []*store.Record{}, + wantErrs: []error{ErrSuspiciousCacheResult}}, + { + name: "fails if multiple session records are found", + suSe: SuSe{Session: "session-a"}, + mode: LogoutModeSession, + store: func(t *testing.T) store.Store { + s := mocks.NewStore(t) + s.EXPECT().Read(mock.Anything, mock.Anything).Return([]*store.Record{ + recordSessionA, + recordSessionB, + }, nil) + return s + }, + wantRecords: []*store.Record{}, + wantErrs: []error{ErrSuspiciousCacheResult}}, + { + name: "fails if the record key is not ok", + suSe: SuSe{Session: "session-a"}, + mode: LogoutModeSession, + store: func(t *testing.T) store.Store { + s := mocks.NewStore(t) + s.EXPECT().Read(mock.Anything, mock.Anything).Return([]*store.Record{ + &store.Record{Key: "invalid.record.key"}, + }, nil) + return s + }, + wantRecords: []*store.Record{}, + wantErrs: []error{ErrInvalidSessionOrSubject, ErrSuspiciousCacheResult}, + }, + { + name: "fails if the session does not match the retrieved record", + suSe: SuSe{Session: "session-a"}, + mode: LogoutModeSession, + store: func(t *testing.T) store.Store { + s := mocks.NewStore(t) + s.EXPECT().Read(mock.Anything, mock.Anything).Return([]*store.Record{ + recordSessionB, + }, nil) + return s + }, + wantRecords: []*store.Record{}, + wantErrs: []error{ErrSuspiciousCacheResult}}, + { + name: "fails if the subject does not match the retrieved record", + suSe: SuSe{Subject: "subject-a"}, + mode: LogoutModeSubject, + store: func(t *testing.T) store.Store { + s := mocks.NewStore(t) + s.EXPECT().Read(mock.Anything, mock.Anything).Return([]*store.Record{ + recordSessionB, + }, nil) + return s + }, + wantRecords: []*store.Record{}, + wantErrs: []error{ErrSuspiciousCacheResult}}, + // key variation tests + { + name: "key variation: 'session-a'", + suSe: SuSe{Session: "session-a"}, + mode: LogoutModeSession, + store: func(*testing.T) store.Store { + return sessionStore + }, + wantRecords: []*store.Record{recordSessionA}, + }, + { + name: "key variation: 'session-b'", + suSe: SuSe{Session: "session-b"}, + mode: LogoutModeSession, + store: func(*testing.T) store.Store { + return sessionStore + }, + wantRecords: []*store.Record{recordSessionB}, + }, + { + name: "key variation: 'session-c'", + suSe: SuSe{Session: "session-c"}, + mode: LogoutModeSession, + store: func(*testing.T) store.Store { + return sessionStore + }, wantRecords: []*store.Record{recordSubjectASessionC}, }, { - name: "ession-c", - suSe: SuSe{Session: "ession-c"}, - mode: LogoutModeSession, - store: sessionStore, - wantError: store.ErrNotFound, + name: "key variation: 'ession-c'", + suSe: SuSe{Session: "ession-c"}, + mode: LogoutModeSession, + store: func(*testing.T) store.Store { + return sessionStore + }, wantRecords: []*store.Record{}, + wantErrs: []error{store.ErrNotFound}, }, { - name: "subject-a", - suSe: SuSe{Subject: "subject-a"}, - mode: LogoutModeSubject, - store: sessionStore, + name: "key variation: 'subject-a'", + suSe: SuSe{Subject: "subject-a"}, + mode: LogoutModeSubject, + store: func(*testing.T) store.Store { + return sessionStore + }, wantRecords: []*store.Record{recordSubjectASessionC, recordSubjectASessionD}, }, { - name: "subject-", - suSe: SuSe{Subject: "subject-"}, - mode: LogoutModeSubject, - store: sessionStore, - wantError: store.ErrNotFound, + name: "key variation: 'subject-'", + suSe: SuSe{Subject: "subject-"}, + mode: LogoutModeSubject, + store: func(*testing.T) store.Store { + return sessionStore + }, wantRecords: []*store.Record{}, + wantErrs: []error{store.ErrNotFound}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - records, err := GetLogoutRecords(tt.suSe, tt.mode, tt.store) - require.ErrorIs(t, err, tt.wantError) + records, err := GetLogoutRecords(tt.suSe, tt.mode, tt.store(t)) + for _, wantErr := range tt.wantErrs { + require.ErrorIs(t, err, wantErr) + } require.Len(t, records, len(tt.wantRecords)) sortRecords := func(r []*store.Record) []*store.Record { diff --git a/services/proxy/pkg/staticroutes/internal/backchannellogout/mocks/store.go b/services/proxy/pkg/staticroutes/internal/backchannellogout/mocks/store.go new file mode 100644 index 0000000000..359ea9cc2b --- /dev/null +++ b/services/proxy/pkg/staticroutes/internal/backchannellogout/mocks/store.go @@ -0,0 +1,509 @@ +// Code generated by mockery; DO NOT EDIT. +// github.com/vektra/mockery +// template: testify + +package mocks + +import ( + mock "github.com/stretchr/testify/mock" + "go-micro.dev/v4/store" +) + +// NewStore creates a new instance of Store. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewStore(t interface { + mock.TestingT + Cleanup(func()) +}) *Store { + mock := &Store{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} + +// Store is an autogenerated mock type for the Store type +type Store struct { + mock.Mock +} + +type Store_Expecter struct { + mock *mock.Mock +} + +func (_m *Store) EXPECT() *Store_Expecter { + return &Store_Expecter{mock: &_m.Mock} +} + +// Close provides a mock function for the type Store +func (_mock *Store) Close() error { + ret := _mock.Called() + + if len(ret) == 0 { + panic("no return value specified for Close") + } + + var r0 error + if returnFunc, ok := ret.Get(0).(func() error); ok { + r0 = returnFunc() + } else { + r0 = ret.Error(0) + } + return r0 +} + +// Store_Close_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Close' +type Store_Close_Call struct { + *mock.Call +} + +// Close is a helper method to define mock.On call +func (_e *Store_Expecter) Close() *Store_Close_Call { + return &Store_Close_Call{Call: _e.mock.On("Close")} +} + +func (_c *Store_Close_Call) Run(run func()) *Store_Close_Call { + _c.Call.Run(func(args mock.Arguments) { + run() + }) + return _c +} + +func (_c *Store_Close_Call) Return(err error) *Store_Close_Call { + _c.Call.Return(err) + return _c +} + +func (_c *Store_Close_Call) RunAndReturn(run func() error) *Store_Close_Call { + _c.Call.Return(run) + return _c +} + +// Delete provides a mock function for the type Store +func (_mock *Store) Delete(key string, opts ...store.DeleteOption) error { + var tmpRet mock.Arguments + if len(opts) > 0 { + tmpRet = _mock.Called(key, opts) + } else { + tmpRet = _mock.Called(key) + } + ret := tmpRet + + if len(ret) == 0 { + panic("no return value specified for Delete") + } + + var r0 error + if returnFunc, ok := ret.Get(0).(func(string, ...store.DeleteOption) error); ok { + r0 = returnFunc(key, opts...) + } else { + r0 = ret.Error(0) + } + return r0 +} + +// Store_Delete_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Delete' +type Store_Delete_Call struct { + *mock.Call +} + +// Delete is a helper method to define mock.On call +// - key string +// - opts ...store.DeleteOption +func (_e *Store_Expecter) Delete(key interface{}, opts ...interface{}) *Store_Delete_Call { + return &Store_Delete_Call{Call: _e.mock.On("Delete", + append([]interface{}{key}, opts...)...)} +} + +func (_c *Store_Delete_Call) Run(run func(key string, opts ...store.DeleteOption)) *Store_Delete_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 string + if args[0] != nil { + arg0 = args[0].(string) + } + var arg1 []store.DeleteOption + var variadicArgs []store.DeleteOption + if len(args) > 1 { + variadicArgs = args[1].([]store.DeleteOption) + } + arg1 = variadicArgs + run( + arg0, + arg1..., + ) + }) + return _c +} + +func (_c *Store_Delete_Call) Return(err error) *Store_Delete_Call { + _c.Call.Return(err) + return _c +} + +func (_c *Store_Delete_Call) RunAndReturn(run func(key string, opts ...store.DeleteOption) error) *Store_Delete_Call { + _c.Call.Return(run) + return _c +} + +// Init provides a mock function for the type Store +func (_mock *Store) Init(options ...store.Option) error { + var tmpRet mock.Arguments + if len(options) > 0 { + tmpRet = _mock.Called(options) + } else { + tmpRet = _mock.Called() + } + ret := tmpRet + + if len(ret) == 0 { + panic("no return value specified for Init") + } + + var r0 error + if returnFunc, ok := ret.Get(0).(func(...store.Option) error); ok { + r0 = returnFunc(options...) + } else { + r0 = ret.Error(0) + } + return r0 +} + +// Store_Init_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Init' +type Store_Init_Call struct { + *mock.Call +} + +// Init is a helper method to define mock.On call +// - options ...store.Option +func (_e *Store_Expecter) Init(options ...interface{}) *Store_Init_Call { + return &Store_Init_Call{Call: _e.mock.On("Init", + append([]interface{}{}, options...)...)} +} + +func (_c *Store_Init_Call) Run(run func(options ...store.Option)) *Store_Init_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 []store.Option + var variadicArgs []store.Option + if len(args) > 0 { + variadicArgs = args[0].([]store.Option) + } + arg0 = variadicArgs + run( + arg0..., + ) + }) + return _c +} + +func (_c *Store_Init_Call) Return(err error) *Store_Init_Call { + _c.Call.Return(err) + return _c +} + +func (_c *Store_Init_Call) RunAndReturn(run func(options ...store.Option) error) *Store_Init_Call { + _c.Call.Return(run) + return _c +} + +// List provides a mock function for the type Store +func (_mock *Store) List(opts ...store.ListOption) ([]string, error) { + var tmpRet mock.Arguments + if len(opts) > 0 { + tmpRet = _mock.Called(opts) + } else { + tmpRet = _mock.Called() + } + ret := tmpRet + + if len(ret) == 0 { + panic("no return value specified for List") + } + + var r0 []string + var r1 error + if returnFunc, ok := ret.Get(0).(func(...store.ListOption) ([]string, error)); ok { + return returnFunc(opts...) + } + if returnFunc, ok := ret.Get(0).(func(...store.ListOption) []string); ok { + r0 = returnFunc(opts...) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]string) + } + } + if returnFunc, ok := ret.Get(1).(func(...store.ListOption) error); ok { + r1 = returnFunc(opts...) + } else { + r1 = ret.Error(1) + } + return r0, r1 +} + +// Store_List_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'List' +type Store_List_Call struct { + *mock.Call +} + +// List is a helper method to define mock.On call +// - opts ...store.ListOption +func (_e *Store_Expecter) List(opts ...interface{}) *Store_List_Call { + return &Store_List_Call{Call: _e.mock.On("List", + append([]interface{}{}, opts...)...)} +} + +func (_c *Store_List_Call) Run(run func(opts ...store.ListOption)) *Store_List_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 []store.ListOption + var variadicArgs []store.ListOption + if len(args) > 0 { + variadicArgs = args[0].([]store.ListOption) + } + arg0 = variadicArgs + run( + arg0..., + ) + }) + return _c +} + +func (_c *Store_List_Call) Return(strings []string, err error) *Store_List_Call { + _c.Call.Return(strings, err) + return _c +} + +func (_c *Store_List_Call) RunAndReturn(run func(opts ...store.ListOption) ([]string, error)) *Store_List_Call { + _c.Call.Return(run) + return _c +} + +// Options provides a mock function for the type Store +func (_mock *Store) Options() store.Options { + ret := _mock.Called() + + if len(ret) == 0 { + panic("no return value specified for Options") + } + + var r0 store.Options + if returnFunc, ok := ret.Get(0).(func() store.Options); ok { + r0 = returnFunc() + } else { + r0 = ret.Get(0).(store.Options) + } + return r0 +} + +// Store_Options_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Options' +type Store_Options_Call struct { + *mock.Call +} + +// Options is a helper method to define mock.On call +func (_e *Store_Expecter) Options() *Store_Options_Call { + return &Store_Options_Call{Call: _e.mock.On("Options")} +} + +func (_c *Store_Options_Call) Run(run func()) *Store_Options_Call { + _c.Call.Run(func(args mock.Arguments) { + run() + }) + return _c +} + +func (_c *Store_Options_Call) Return(options store.Options) *Store_Options_Call { + _c.Call.Return(options) + return _c +} + +func (_c *Store_Options_Call) RunAndReturn(run func() store.Options) *Store_Options_Call { + _c.Call.Return(run) + return _c +} + +// Read provides a mock function for the type Store +func (_mock *Store) Read(key string, opts ...store.ReadOption) ([]*store.Record, error) { + var tmpRet mock.Arguments + if len(opts) > 0 { + tmpRet = _mock.Called(key, opts) + } else { + tmpRet = _mock.Called(key) + } + ret := tmpRet + + if len(ret) == 0 { + panic("no return value specified for Read") + } + + var r0 []*store.Record + var r1 error + if returnFunc, ok := ret.Get(0).(func(string, ...store.ReadOption) ([]*store.Record, error)); ok { + return returnFunc(key, opts...) + } + if returnFunc, ok := ret.Get(0).(func(string, ...store.ReadOption) []*store.Record); ok { + r0 = returnFunc(key, opts...) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*store.Record) + } + } + if returnFunc, ok := ret.Get(1).(func(string, ...store.ReadOption) error); ok { + r1 = returnFunc(key, opts...) + } else { + r1 = ret.Error(1) + } + return r0, r1 +} + +// Store_Read_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Read' +type Store_Read_Call struct { + *mock.Call +} + +// Read is a helper method to define mock.On call +// - key string +// - opts ...store.ReadOption +func (_e *Store_Expecter) Read(key interface{}, opts ...interface{}) *Store_Read_Call { + return &Store_Read_Call{Call: _e.mock.On("Read", + append([]interface{}{key}, opts...)...)} +} + +func (_c *Store_Read_Call) Run(run func(key string, opts ...store.ReadOption)) *Store_Read_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 string + if args[0] != nil { + arg0 = args[0].(string) + } + var arg1 []store.ReadOption + var variadicArgs []store.ReadOption + if len(args) > 1 { + variadicArgs = args[1].([]store.ReadOption) + } + arg1 = variadicArgs + run( + arg0, + arg1..., + ) + }) + return _c +} + +func (_c *Store_Read_Call) Return(records []*store.Record, err error) *Store_Read_Call { + _c.Call.Return(records, err) + return _c +} + +func (_c *Store_Read_Call) RunAndReturn(run func(key string, opts ...store.ReadOption) ([]*store.Record, error)) *Store_Read_Call { + _c.Call.Return(run) + return _c +} + +// String provides a mock function for the type Store +func (_mock *Store) String() string { + ret := _mock.Called() + + if len(ret) == 0 { + panic("no return value specified for String") + } + + var r0 string + if returnFunc, ok := ret.Get(0).(func() string); ok { + r0 = returnFunc() + } else { + r0 = ret.Get(0).(string) + } + return r0 +} + +// Store_String_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'String' +type Store_String_Call struct { + *mock.Call +} + +// String is a helper method to define mock.On call +func (_e *Store_Expecter) String() *Store_String_Call { + return &Store_String_Call{Call: _e.mock.On("String")} +} + +func (_c *Store_String_Call) Run(run func()) *Store_String_Call { + _c.Call.Run(func(args mock.Arguments) { + run() + }) + return _c +} + +func (_c *Store_String_Call) Return(s string) *Store_String_Call { + _c.Call.Return(s) + return _c +} + +func (_c *Store_String_Call) RunAndReturn(run func() string) *Store_String_Call { + _c.Call.Return(run) + return _c +} + +// Write provides a mock function for the type Store +func (_mock *Store) Write(r *store.Record, opts ...store.WriteOption) error { + var tmpRet mock.Arguments + if len(opts) > 0 { + tmpRet = _mock.Called(r, opts) + } else { + tmpRet = _mock.Called(r) + } + ret := tmpRet + + if len(ret) == 0 { + panic("no return value specified for Write") + } + + var r0 error + if returnFunc, ok := ret.Get(0).(func(*store.Record, ...store.WriteOption) error); ok { + r0 = returnFunc(r, opts...) + } else { + r0 = ret.Error(0) + } + return r0 +} + +// Store_Write_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Write' +type Store_Write_Call struct { + *mock.Call +} + +// Write is a helper method to define mock.On call +// - r *store.Record +// - opts ...store.WriteOption +func (_e *Store_Expecter) Write(r interface{}, opts ...interface{}) *Store_Write_Call { + return &Store_Write_Call{Call: _e.mock.On("Write", + append([]interface{}{r}, opts...)...)} +} + +func (_c *Store_Write_Call) Run(run func(r *store.Record, opts ...store.WriteOption)) *Store_Write_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 *store.Record + if args[0] != nil { + arg0 = args[0].(*store.Record) + } + var arg1 []store.WriteOption + var variadicArgs []store.WriteOption + if len(args) > 1 { + variadicArgs = args[1].([]store.WriteOption) + } + arg1 = variadicArgs + run( + arg0, + arg1..., + ) + }) + return _c +} + +func (_c *Store_Write_Call) Return(err error) *Store_Write_Call { + _c.Call.Return(err) + return _c +} + +func (_c *Store_Write_Call) RunAndReturn(run func(r *store.Record, opts ...store.WriteOption) error) *Store_Write_Call { + _c.Call.Return(run) + return _c +} From a5f5009f9db77ef6921711bd15698fb8c640173e Mon Sep 17 00:00:00 2001 From: Florian Schade Date: Tue, 17 Feb 2026 11:13:05 +0100 Subject: [PATCH 09/15] chore: change naming --- .../internal/backchannellogout/backchannellogout.go | 11 ++++++++--- .../backchannellogout/backchannellogout_test.go | 6 +++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go index 306febd0ca..fb5c15ff36 100644 --- a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go +++ b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go @@ -26,13 +26,17 @@ var ErrInvalidSessionOrSubject = errors.New("invalid session or subject") func NewSuSe(key string) (SuSe, error) { var subject, session string switch keys := strings.Split(strings.Join(strings.Fields(key), ""), "."); { + // key: '.session' case len(keys) == 2 && keys[0] == "" && keys[1] != "": session = keys[1] + // key: 'subject.' case len(keys) == 2 && keys[0] != "" && keys[1] == "": subject = keys[0] + // key: 'subject.session' case len(keys) == 2 && keys[0] != "" && keys[1] != "": subject = keys[0] session = keys[1] + // key: 'session' case len(keys) == 1 && keys[0] != "": session = keys[0] default: @@ -46,8 +50,8 @@ func NewSuSe(key string) (SuSe, error) { type LogoutMode int const ( - // LogoutModeUnknown is used when the logout mode cannot be determined - LogoutModeUnknown LogoutMode = iota + // LogoutModeUndefined is used when the logout mode cannot be determined + LogoutModeUndefined LogoutMode = iota // LogoutModeSession is used when the logout mode is determined by the session id LogoutModeSession // LogoutModeSubject is used when the logout mode is determined by the subject @@ -62,7 +66,7 @@ func GetLogoutMode(suse SuSe) LogoutMode { case suse.Subject != "": return LogoutModeSubject default: - return LogoutModeUnknown + return LogoutModeUndefined } } @@ -91,6 +95,7 @@ func GetLogoutRecords(suse SuSe, mode LogoutMode, store microstore.Store) ([]*mi return nil, fmt.Errorf("%w: cannot determine logout mode", ErrSuspiciousCacheResult) } + // the go micro memory store requires a limit to work, why??? records, err := store.Read(key, append(opts, microstore.ReadLimit(1000))...) if err != nil { return nil, err diff --git a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go index 238323c728..d442f06184 100644 --- a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go +++ b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go @@ -107,7 +107,7 @@ func TestGetLogoutMode(t *testing.T) { { name: "key variation: 'empty'", suSe: SuSe{Session: "", Subject: ""}, - want: LogoutModeUnknown, + want: LogoutModeUndefined, }, } @@ -155,7 +155,7 @@ func TestGetLogoutRecords(t *testing.T) { { name: "fails if mode is unknown", suSe: SuSe{Session: "session-a"}, - mode: LogoutModeUnknown, + mode: LogoutModeUndefined, store: func(t *testing.T) store.Store { return sessionStore }, @@ -192,7 +192,7 @@ func TestGetLogoutRecords(t *testing.T) { store: func(t *testing.T) store.Store { s := mocks.NewStore(t) s.EXPECT().Read(mock.Anything, mock.Anything).Return([]*store.Record{ - &store.Record{Key: "invalid.record.key"}, + {Key: "invalid.record.key"}, }, nil) return s }, From 8d99cf3f8b8ccb4ccbc9f51a0389e4fd5e680e2a Mon Sep 17 00:00:00 2001 From: Florian Schade Date: Fri, 20 Feb 2026 16:45:38 +0100 Subject: [PATCH 10/15] fix: use base64 record keys to prevent separator clashes with subjects or sessionIds that contain a dot --- services/proxy/pkg/middleware/oidc_auth.go | 18 +- .../pkg/staticroutes/backchannellogout.go | 78 +++++--- .../backchannellogout/backchannellogout.go | 117 ++++++++---- .../backchannellogout_test.go | 174 +++++++++++------- 4 files changed, 253 insertions(+), 134 deletions(-) diff --git a/services/proxy/pkg/middleware/oidc_auth.go b/services/proxy/pkg/middleware/oidc_auth.go index 17f9e8edba..7afdfc70ee 100644 --- a/services/proxy/pkg/middleware/oidc_auth.go +++ b/services/proxy/pkg/middleware/oidc_auth.go @@ -17,6 +17,7 @@ import ( "github.com/opencloud-eu/opencloud/pkg/log" "github.com/opencloud-eu/opencloud/pkg/oidc" + "github.com/opencloud-eu/opencloud/services/proxy/pkg/staticroutes" ) const ( @@ -115,22 +116,21 @@ func (m *OIDCAuthenticator) getClaims(token string, req *http.Request) (map[stri m.Logger.Error().Err(err).Msg("failed to write to userinfo cache") } - subject, sessionId := strings.Join(strings.Fields(aClaims.Subject), ""), strings.Join(strings.Fields(aClaims.SessionID), "") - // if no session id is present, we can't do a session lookup, - // so we can skip the cache entry for that. - if sessionId == "" { - return - } - // if the claim has no subject, we can leave it empty, // it's important to keep the dot in the key to prevent // sufix and prefix exploration in the cache. // // ok: {key: ".sessionId"} + // ok: {key: "subject."} // ok: {key: "subject.sessionId"} - key := strings.Join([]string{subject, sessionId}, ".") + subjectSessionKey, err := staticroutes.NewRecordKey(aClaims.Subject, aClaims.SessionID) + if err != nil { + m.Logger.Error().Err(err).Msg("failed to build subject.session") + return + } + if err := m.userInfoCache.Write(&store.Record{ - Key: key, + Key: subjectSessionKey, Value: []byte(encodedHash), Expiry: time.Until(expiration), }); err != nil { diff --git a/services/proxy/pkg/staticroutes/backchannellogout.go b/services/proxy/pkg/staticroutes/backchannellogout.go index 0d44e7cd1c..d7e10d5da8 100644 --- a/services/proxy/pkg/staticroutes/backchannellogout.go +++ b/services/proxy/pkg/staticroutes/backchannellogout.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "net/http" - "strings" "github.com/go-chi/render" "github.com/pkg/errors" @@ -16,6 +15,9 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/utils" ) +// NewRecordKey converts the subject and session to a base64 encoded key +var NewRecordKey = bcl.NewKey + // backchannelLogout handles backchannel logout requests from the identity provider and invalidates the related sessions in the cache // spec: https://openid.net/specs/openid-connect-backchannel-1_0.html#BCRequest // @@ -37,9 +39,6 @@ import ( // all sessions besides the one that triggered the backchannel logout continue to exist in the identity provider, // so the user will not be fully logged out until all sessions are logged out or expired. // this leads to the situation that web renders the logout view even if the instance is not fully logged out yet. -// -// toDo: -// - check logs and errors to not contain any sensitive information like session ids or user ids (keys too) func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Request) { logger := s.Logger.SubloggerWithRequestID(r.Context()) if err := r.ParseForm(); err != nil { @@ -51,22 +50,31 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re logoutToken, err := s.OidcClient.VerifyLogoutToken(r.Context(), r.PostFormValue("logout_token")) if err != nil { - logger.Warn().Err(err).Msg("VerifyLogoutToken failed") + msg := "failed to verify logout token" + logger.Warn().Err(err).Msg(msg) render.Status(r, http.StatusBadRequest) - render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) + render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: msg}) return } - subject, session := strings.Join(strings.Fields(logoutToken.Subject), ""), strings.Join(strings.Fields(logoutToken.SessionId), "") - if subject == "" && session == "" { - jseErr := jse{Error: "invalid_request", ErrorDescription: "invalid logout token: subject and session id are empty"} - logger.Warn().Msg(jseErr.ErrorDescription) + lookupKey, err := bcl.NewKey(logoutToken.Subject, logoutToken.SessionId) + if err != nil { + msg := "failed to build key from logout token" + logger.Warn().Err(err).Msg(msg) + render.Status(r, http.StatusBadRequest) + render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: msg}) + return + } + + requestSubjectAndSession, err := bcl.NewSuSe(lookupKey) + if err != nil { + msg := "failed to build subjec.session from lookupKey" + logger.Error().Err(err).Msg(msg) render.Status(r, http.StatusBadRequest) - render.JSON(w, r, jseErr) + render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: msg}) return } - requestSubjectAndSession := bcl.SuSe{Session: session, Subject: subject} // find out which mode of backchannel logout we are in // by checking if the session or subject is present in the token logoutMode := bcl.GetLogoutMode(requestSubjectAndSession) @@ -77,9 +85,10 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re return } if err != nil { - logger.Error().Err(err).Msg("Error reading userinfo cache") + msg := "failed to read userinfo cache" + logger.Error().Err(err).Msg(msg) render.Status(r, http.StatusBadRequest) - render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) + render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: msg}) return } @@ -91,28 +100,36 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re subjectSession, err := bcl.NewSuSe(key) if err != nil { // never leak any key-related information - logger.Warn().Err(err).Msgf("invalid logout record key: %s", "XXX") + logger.Warn().Err(err).Msgf("failed to parse key: %s", key) continue } - if err := s.publishBackchannelLogoutEvent(r.Context(), subjectSession.Session, value); err != nil { - s.Logger.Warn().Err(err).Msg("could not publish backchannel logout event") + session, err := subjectSession.Session() + if err != nil { + logger.Warn().Err(err).Msgf("failed to read session for: %s", key) + continue + } + + if err := s.publishBackchannelLogoutEvent(r.Context(), session, value); err != nil { + s.Logger.Warn().Err(err).Msgf("failed to publish backchannel logout event for: %s", key) + continue } err = s.UserInfoCache.Delete(value) if err != nil && !errors.Is(err, microstore.ErrNotFound) { // we have to return a 400 BadRequest when we fail to delete the session // https://openid.net/specs/openid-connect-backchannel-1_0.html#rfc.section.2.8 - logger.Err(err).Msg("could not delete user info from cache") + msg := "failed to delete record" + s.Logger.Warn().Err(err).Msgf("%s for: %s", msg, key) render.Status(r, http.StatusBadRequest) - render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: err.Error()}) + render.JSON(w, r, jse{Error: "invalid_request", ErrorDescription: msg}) return } // we can ignore errors when deleting the lookup record err = s.UserInfoCache.Delete(key) if err != nil { - logger.Debug().Err(err).Msg("Failed to cleanup sessionId lookup entry") + logger.Debug().Err(err).Msgf("failed to delete record for: %s", key) } } @@ -123,29 +140,30 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re // publishBackchannelLogoutEvent publishes a backchannel logout event when the callback revived from the identity provider func (s *StaticRouteHandler) publishBackchannelLogoutEvent(ctx context.Context, sessionId, claimKey string) error { if s.EventsPublisher == nil { - return fmt.Errorf("the events publisher is not set") + return errors.New("events publisher not set") } + claimRecords, err := s.UserInfoCache.Read(claimKey) - if err != nil { - return fmt.Errorf("reading userinfo cache: %w", err) - } - if len(claimRecords) == 0 { - return fmt.Errorf("userinfo not found") + switch { + case err != nil: + return fmt.Errorf("failed to read userinfo cache: %w", err) + case len(claimRecords) == 0: + return fmt.Errorf("no claim found for key: %s", claimKey) } var claims map[string]interface{} if err = msgpack.Unmarshal(claimRecords[0].Value, &claims); err != nil { - return fmt.Errorf("could not unmarshal userinfo: %w", err) + return fmt.Errorf("failed to unmarshal claims: %w", err) } oidcClaim, ok := claims[s.Config.UserOIDCClaim].(string) if !ok { - return fmt.Errorf("could not get claim %w", err) + return fmt.Errorf("failed to get claim %w", err) } user, _, err := s.UserProvider.GetUserByClaims(ctx, s.Config.UserCS3Claim, oidcClaim) if err != nil || user.GetId() == nil { - return fmt.Errorf("could not get user by claims: %w", err) + return fmt.Errorf("failed to get user by claims: %w", err) } e := events.BackchannelLogout{ @@ -155,7 +173,7 @@ func (s *StaticRouteHandler) publishBackchannelLogoutEvent(ctx context.Context, } if err := events.Publish(ctx, s.EventsPublisher, e); err != nil { - return fmt.Errorf("could not publish user created event %w", err) + return fmt.Errorf("failed to publish user logout event %w", err) } return nil } diff --git a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go index fb5c15ff36..1863047031 100644 --- a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go +++ b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go @@ -4,46 +4,97 @@ package backchannellogout import ( - "fmt" + "encoding/base64" + "errors" "strings" - "github.com/pkg/errors" microstore "go-micro.dev/v4/store" ) +// keyEncoding is the base64 encoding used for session and subject keys +var keyEncoding = base64.URLEncoding + +// ErrInvalidKey indicates that the provided key does not conform to the expected format. +var ErrInvalidKey = errors.New("invalid key format") + +// NewKey converts the subject and session to a base64 encoded key +func NewKey(subject, session string) (string, error) { + subjectSession := strings.Join([]string{ + keyEncoding.EncodeToString([]byte(subject)), + keyEncoding.EncodeToString([]byte(session)), + }, ".") + + if subjectSession == "." { + return "", ErrInvalidKey + } + + return subjectSession, nil +} + +// ErrDecoding is returned when decoding fails +var ErrDecoding = errors.New("failed to decode") + // SuSe 🦎 ;) is a struct that groups the subject and session together // to prevent mix-ups for ('session, subject' || 'subject, session') // return values. type SuSe struct { - Subject string - Session string + encodedSubject string + encodedSession string +} + +// Subject decodes and returns the subject or an error +func (suse SuSe) Subject() (string, error) { + subject, err := keyEncoding.DecodeString(suse.encodedSubject) + if err != nil { + return "", errors.Join(errors.New("failed to decode subject"), ErrDecoding, err) + } + + return string(subject), nil +} + +// Session decodes and returns the session or an error +func (suse SuSe) Session() (string, error) { + subject, err := keyEncoding.DecodeString(suse.encodedSession) + if err != nil { + return "", errors.Join(errors.New("failed to decode session"), ErrDecoding, err) + } + + return string(subject), nil } -// ErrInvalidSessionOrSubject is returned when the provided key does not match the expected key format -var ErrInvalidSessionOrSubject = errors.New("invalid session or subject") +// ErrInvalidSubjectOrSession is returned when the provided key does not match the expected key format +var ErrInvalidSubjectOrSession = errors.New("invalid subject or session") // NewSuSe parses the subject and session id from the given key and returns a SuSe struct func NewSuSe(key string) (SuSe, error) { - var subject, session string + suse := SuSe{} switch keys := strings.Split(strings.Join(strings.Fields(key), ""), "."); { // key: '.session' case len(keys) == 2 && keys[0] == "" && keys[1] != "": - session = keys[1] + suse.encodedSession = keys[1] // key: 'subject.' case len(keys) == 2 && keys[0] != "" && keys[1] == "": - subject = keys[0] + suse.encodedSubject = keys[0] // key: 'subject.session' case len(keys) == 2 && keys[0] != "" && keys[1] != "": - subject = keys[0] - session = keys[1] + suse.encodedSubject = keys[0] + suse.encodedSession = keys[1] // key: 'session' case len(keys) == 1 && keys[0] != "": - session = keys[0] + suse.encodedSession = keys[0] default: - return SuSe{}, ErrInvalidSessionOrSubject + return suse, ErrInvalidSubjectOrSession + } + + if _, err := suse.Subject(); err != nil { + return suse, errors.Join(ErrInvalidSubjectOrSession, err) } - return SuSe{Session: session, Subject: subject}, nil + if _, err := suse.Session(); err != nil { + return suse, errors.Join(ErrInvalidSubjectOrSession, err) + } + + return suse, nil } // LogoutMode defines the mode of backchannel logout, either by session or by subject @@ -52,19 +103,19 @@ type LogoutMode int const ( // LogoutModeUndefined is used when the logout mode cannot be determined LogoutModeUndefined LogoutMode = iota - // LogoutModeSession is used when the logout mode is determined by the session id - LogoutModeSession // LogoutModeSubject is used when the logout mode is determined by the subject LogoutModeSubject + // LogoutModeSession is used when the logout mode is determined by the session id + LogoutModeSession ) // GetLogoutMode determines the backchannel logout mode based on the presence of subject and session in the SuSe struct func GetLogoutMode(suse SuSe) LogoutMode { switch { - case suse.Session != "": - return LogoutModeSession - case suse.Subject != "": + case suse.encodedSession == "" && suse.encodedSubject != "": return LogoutModeSubject + case suse.encodedSession != "": + return LogoutModeSession default: return LogoutModeUndefined } @@ -81,18 +132,18 @@ func GetLogoutRecords(suse SuSe, mode LogoutMode, store microstore.Store) ([]*mi var key string var opts []microstore.ReadOption switch mode { - case LogoutModeSession: - // the dot at the beginning prevents sufix exploration in the cache, - // so only keys that end with '*.session' will be returned, but not '*sion'. - key = "." + suse.Session - opts = append(opts, microstore.ReadSuffix()) case LogoutModeSubject: // the dot at the end prevents prefix exploration in the cache, // so only keys that start with 'subject.*' will be returned, but not 'sub*'. - key = suse.Subject + "." + key = suse.encodedSubject + "." opts = append(opts, microstore.ReadPrefix()) + case LogoutModeSession: + // the dot at the beginning prevents sufix exploration in the cache, + // so only keys that end with '*.session' will be returned, but not '*sion'. + key = "." + suse.encodedSession + opts = append(opts, microstore.ReadSuffix()) default: - return nil, fmt.Errorf("%w: cannot determine logout mode", ErrSuspiciousCacheResult) + return nil, errors.Join(errors.New("cannot determine logout mode"), ErrSuspiciousCacheResult) } // the go micro memory store requires a limit to work, why??? @@ -106,7 +157,7 @@ func GetLogoutRecords(suse SuSe, mode LogoutMode, store microstore.Store) ([]*mi } if mode == LogoutModeSession && len(records) > 1 { - return nil, fmt.Errorf("%w: multiple session records found", ErrSuspiciousCacheResult) + return nil, errors.Join(errors.New("multiple session records found"), ErrSuspiciousCacheResult) } // double-check if the found records match the requested subject and or session id as well, @@ -115,19 +166,19 @@ func GetLogoutRecords(suse SuSe, mode LogoutMode, store microstore.Store) ([]*mi recordSuSe, err := NewSuSe(record.Key) if err != nil { // never leak any key-related information - return nil, fmt.Errorf("%w %w: failed to parse logout record key: %s", err, ErrSuspiciousCacheResult, "XXX") + return nil, errors.Join(errors.New("failed to parse key"), ErrSuspiciousCacheResult, err) } switch { - // in session mode, the session id must match, but the subject can be different - case mode == LogoutModeSession && suse.Session == recordSuSe.Session: - continue // in subject mode, the subject must match, but the session id can be different - case mode == LogoutModeSubject && suse.Subject == recordSuSe.Subject: + case mode == LogoutModeSubject && suse.encodedSubject == recordSuSe.encodedSubject: + continue + // in session mode, the session id must match, but the subject can be different + case mode == LogoutModeSession && suse.encodedSession == recordSuSe.encodedSession: continue } - return nil, fmt.Errorf("%w: record key does not match the requested subject or session", ErrSuspiciousCacheResult) + return nil, errors.Join(errors.New("key does not match the requested subject or session"), ErrSuspiciousCacheResult) } return records, nil diff --git a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go index d442f06184..a653be2247 100644 --- a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go +++ b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go @@ -12,73 +12,123 @@ import ( "github.com/opencloud-eu/opencloud/services/proxy/pkg/staticroutes/internal/backchannellogout/mocks" ) -func TestNewSuSe(t *testing.T) { +func mustNewKey(t *testing.T, subject, session string) string { + key, err := NewKey(subject, session) + require.NoError(t, err) + return key +} + +func mustNewSuSe(t *testing.T, subject, session string) SuSe { + suse, err := NewSuSe(mustNewKey(t, subject, session)) + require.NoError(t, err) + return suse +} + +func TestNewKey(t *testing.T) { tests := []struct { - name string - key string - wantSuSe SuSe - wantErr error + name string + subject string + session string + wantKey string + wantErr error }{ { - name: "key variation: '.session'", - key: ".session", - wantSuSe: SuSe{Session: "session", Subject: ""}, + name: "key variation: 'subject.session'", + subject: "subject", + session: "session", + wantKey: "c3ViamVjdA==.c2Vzc2lvbg==", }, { - name: "key variation: '.session'", - key: ".session", - wantSuSe: SuSe{Session: "session", Subject: ""}, + name: "key variation: 'subject.'", + subject: "subject", + wantKey: "c3ViamVjdA==.", }, { - name: "key variation: 'session'", - key: "session", - wantSuSe: SuSe{Session: "session", Subject: ""}, + name: "key variation: '.session'", + session: "session", + wantKey: ".c2Vzc2lvbg==", }, { - name: "key variation: 'subject.'", - key: "subject.", - wantSuSe: SuSe{Session: "", Subject: "subject"}, + name: "key variation: '.'", + wantErr: ErrInvalidKey, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + key, err := NewKey(tt.subject, tt.session) + require.ErrorIs(t, err, tt.wantErr) + require.Equal(t, tt.wantKey, key) + }) + } +} + +func TestNewSuSe(t *testing.T) { + tests := []struct { + name string + key string + wantSubject string + wantSession string + wantErr error + }{ + { + name: "key variation: '.session'", + key: mustNewKey(t, "", "session"), + wantSession: "session", }, { - name: "key variation: 'subject.session'", - key: "subject.session", - wantSuSe: SuSe{Session: "session", Subject: "subject"}, + name: "key variation: 'session'", + key: mustNewKey(t, "", "session"), + wantSession: "session", }, { - name: "key variation: 'dot'", - key: ".", - wantSuSe: SuSe{Session: "", Subject: ""}, - wantErr: ErrInvalidSessionOrSubject, + name: "key variation: 'subject.'", + key: mustNewKey(t, "subject", ""), + wantSubject: "subject", }, { - name: "key variation: 'empty'", - key: "", - wantSuSe: SuSe{Session: "", Subject: ""}, - wantErr: ErrInvalidSessionOrSubject, + name: "key variation: 'subject.session'", + key: mustNewKey(t, "subject", "session"), + wantSubject: "subject", + wantSession: "session", }, { - name: "key variation: 'whitespace . whitespace'", - key: " . ", - wantSuSe: SuSe{Session: "", Subject: ""}, - wantErr: ErrInvalidSessionOrSubject, + name: "key variation: 'dot'", + key: ".", + wantErr: ErrInvalidSubjectOrSession, }, { - name: "key variation: 'whitespace subject whitespace . whitespace'", - key: " subject . ", - wantSuSe: SuSe{Session: "", Subject: "subject"}, + name: "key variation: 'empty'", + key: "", + wantErr: ErrInvalidSubjectOrSession, }, { - name: "key variation: 'whitespace . whitespace session whitespace'", - key: " . session ", - wantSuSe: SuSe{Session: "session", Subject: ""}, + name: "key variation: string('subject.session')", + key: "subject.session", + wantErr: ErrInvalidSubjectOrSession, + }, + { + name: "key variation: string('subject.')", + key: "subject.", + wantErr: ErrInvalidSubjectOrSession, + }, + { + name: "key variation: string('.session')", + key: ".session", + wantErr: ErrInvalidSubjectOrSession, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - suSe, ok := NewSuSe(tt.key) - require.ErrorIs(t, tt.wantErr, ok) - require.Equal(t, tt.wantSuSe, suSe) + suSe, err := NewSuSe(tt.key) + require.ErrorIs(t, err, tt.wantErr) + + subject, _ := suSe.Subject() + require.Equal(t, tt.wantSubject, subject) + + session, _ := suSe.Session() + require.Equal(t, tt.wantSession, session) }) } } @@ -91,22 +141,22 @@ func TestGetLogoutMode(t *testing.T) { }{ { name: "key variation: '.session'", - suSe: SuSe{Session: "session", Subject: ""}, + suSe: mustNewSuSe(t, "", "session"), want: LogoutModeSession, }, { name: "key variation: 'subject.session'", - suSe: SuSe{Session: "session", Subject: "subject"}, + suSe: mustNewSuSe(t, "subject", "session"), want: LogoutModeSession, }, { name: "key variation: 'subject.'", - suSe: SuSe{Session: "", Subject: "subject"}, + suSe: mustNewSuSe(t, "subject", ""), want: LogoutModeSubject, }, { name: "key variation: 'empty'", - suSe: SuSe{Session: "", Subject: ""}, + suSe: SuSe{}, want: LogoutModeUndefined, }, } @@ -126,10 +176,10 @@ func TestGetLogoutRecords(t *testing.T) { recordClaimB := &store.Record{Key: "claim-b", Value: []byte("claim-b-data")} recordClaimC := &store.Record{Key: "claim-c", Value: []byte("claim-c-data")} recordClaimD := &store.Record{Key: "claim-d", Value: []byte("claim-d-data")} - recordSessionA := &store.Record{Key: ".session-a", Value: []byte(recordClaimA.Key)} - recordSessionB := &store.Record{Key: ".session-b", Value: []byte(recordClaimB.Key)} - recordSubjectASessionC := &store.Record{Key: "subject-a.session-c", Value: []byte(recordSessionA.Key)} - recordSubjectASessionD := &store.Record{Key: "subject-a.session-d", Value: []byte(recordSessionB.Key)} + recordSessionA := &store.Record{Key: mustNewKey(t, "", "session-a"), Value: []byte(recordClaimA.Key)} + recordSessionB := &store.Record{Key: mustNewKey(t, "", "session-b"), Value: []byte(recordClaimB.Key)} + recordSubjectASessionC := &store.Record{Key: mustNewKey(t, "subject-a", "session-c"), Value: []byte(recordSessionA.Key)} + recordSubjectASessionD := &store.Record{Key: mustNewKey(t, "subject-a", "session-d"), Value: []byte(recordSessionA.Key)} for _, r := range []*store.Record{ recordClaimA, @@ -154,7 +204,7 @@ func TestGetLogoutRecords(t *testing.T) { }{ { name: "fails if mode is unknown", - suSe: SuSe{Session: "session-a"}, + suSe: mustNewSuSe(t, "", "session-a"), mode: LogoutModeUndefined, store: func(t *testing.T) store.Store { return sessionStore @@ -164,7 +214,7 @@ func TestGetLogoutRecords(t *testing.T) { }, { name: "fails if mode is any random int", - suSe: SuSe{Session: "session-a"}, + suSe: mustNewSuSe(t, "", "session-a"), mode: 999, store: func(t *testing.T) store.Store { return sessionStore @@ -173,7 +223,7 @@ func TestGetLogoutRecords(t *testing.T) { wantErrs: []error{ErrSuspiciousCacheResult}}, { name: "fails if multiple session records are found", - suSe: SuSe{Session: "session-a"}, + suSe: mustNewSuSe(t, "", "session-a"), mode: LogoutModeSession, store: func(t *testing.T) store.Store { s := mocks.NewStore(t) @@ -187,7 +237,7 @@ func TestGetLogoutRecords(t *testing.T) { wantErrs: []error{ErrSuspiciousCacheResult}}, { name: "fails if the record key is not ok", - suSe: SuSe{Session: "session-a"}, + suSe: mustNewSuSe(t, "", "session-a"), mode: LogoutModeSession, store: func(t *testing.T) store.Store { s := mocks.NewStore(t) @@ -197,11 +247,11 @@ func TestGetLogoutRecords(t *testing.T) { return s }, wantRecords: []*store.Record{}, - wantErrs: []error{ErrInvalidSessionOrSubject, ErrSuspiciousCacheResult}, + wantErrs: []error{ErrInvalidSubjectOrSession, ErrSuspiciousCacheResult}, }, { name: "fails if the session does not match the retrieved record", - suSe: SuSe{Session: "session-a"}, + suSe: mustNewSuSe(t, "", "session-a"), mode: LogoutModeSession, store: func(t *testing.T) store.Store { s := mocks.NewStore(t) @@ -214,7 +264,7 @@ func TestGetLogoutRecords(t *testing.T) { wantErrs: []error{ErrSuspiciousCacheResult}}, { name: "fails if the subject does not match the retrieved record", - suSe: SuSe{Subject: "subject-a"}, + suSe: mustNewSuSe(t, "subject-a", ""), mode: LogoutModeSubject, store: func(t *testing.T) store.Store { s := mocks.NewStore(t) @@ -228,7 +278,7 @@ func TestGetLogoutRecords(t *testing.T) { // key variation tests { name: "key variation: 'session-a'", - suSe: SuSe{Session: "session-a"}, + suSe: mustNewSuSe(t, "", "session-a"), mode: LogoutModeSession, store: func(*testing.T) store.Store { return sessionStore @@ -237,7 +287,7 @@ func TestGetLogoutRecords(t *testing.T) { }, { name: "key variation: 'session-b'", - suSe: SuSe{Session: "session-b"}, + suSe: mustNewSuSe(t, "", "session-b"), mode: LogoutModeSession, store: func(*testing.T) store.Store { return sessionStore @@ -246,7 +296,7 @@ func TestGetLogoutRecords(t *testing.T) { }, { name: "key variation: 'session-c'", - suSe: SuSe{Session: "session-c"}, + suSe: mustNewSuSe(t, "", "session-c"), mode: LogoutModeSession, store: func(*testing.T) store.Store { return sessionStore @@ -255,7 +305,7 @@ func TestGetLogoutRecords(t *testing.T) { }, { name: "key variation: 'ession-c'", - suSe: SuSe{Session: "ession-c"}, + suSe: mustNewSuSe(t, "", "ession-c"), mode: LogoutModeSession, store: func(*testing.T) store.Store { return sessionStore @@ -265,7 +315,7 @@ func TestGetLogoutRecords(t *testing.T) { }, { name: "key variation: 'subject-a'", - suSe: SuSe{Subject: "subject-a"}, + suSe: mustNewSuSe(t, "subject-a", ""), mode: LogoutModeSubject, store: func(*testing.T) store.Store { return sessionStore @@ -274,7 +324,7 @@ func TestGetLogoutRecords(t *testing.T) { }, { name: "key variation: 'subject-'", - suSe: SuSe{Subject: "subject-"}, + suSe: mustNewSuSe(t, "subject-", ""), mode: LogoutModeSubject, store: func(*testing.T) store.Store { return sessionStore From 44fc25dbf67abebf72add4feaecb24c65533cc5e Mon Sep 17 00:00:00 2001 From: Florian Schade Date: Fri, 20 Feb 2026 16:52:29 +0100 Subject: [PATCH 11/15] refactor: make the logout mode private --- services/proxy/pkg/middleware/oidc_auth.go | 7 ++- .../pkg/staticroutes/backchannellogout.go | 5 +-- .../backchannellogout/backchannellogout.go | 41 +++++++++-------- .../backchannellogout_test.go | 44 +++---------------- 4 files changed, 33 insertions(+), 64 deletions(-) diff --git a/services/proxy/pkg/middleware/oidc_auth.go b/services/proxy/pkg/middleware/oidc_auth.go index 7afdfc70ee..78c82e8887 100644 --- a/services/proxy/pkg/middleware/oidc_auth.go +++ b/services/proxy/pkg/middleware/oidc_auth.go @@ -8,7 +8,6 @@ import ( "strings" "time" - "github.com/golang-jwt/jwt/v5" "github.com/pkg/errors" "github.com/vmihailenco/msgpack/v5" "go-micro.dev/v4/store" @@ -116,13 +115,13 @@ func (m *OIDCAuthenticator) getClaims(token string, req *http.Request) (map[stri m.Logger.Error().Err(err).Msg("failed to write to userinfo cache") } - // if the claim has no subject, we can leave it empty, - // it's important to keep the dot in the key to prevent - // sufix and prefix exploration in the cache. + // fail if creating the storage key fails, + // it means there is no subject and no session. // // ok: {key: ".sessionId"} // ok: {key: "subject."} // ok: {key: "subject.sessionId"} + // fail: {key: "."} subjectSessionKey, err := staticroutes.NewRecordKey(aClaims.Subject, aClaims.SessionID) if err != nil { m.Logger.Error().Err(err).Msg("failed to build subject.session") diff --git a/services/proxy/pkg/staticroutes/backchannellogout.go b/services/proxy/pkg/staticroutes/backchannellogout.go index d7e10d5da8..53375d63bb 100644 --- a/services/proxy/pkg/staticroutes/backchannellogout.go +++ b/services/proxy/pkg/staticroutes/backchannellogout.go @@ -75,10 +75,7 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re return } - // find out which mode of backchannel logout we are in - // by checking if the session or subject is present in the token - logoutMode := bcl.GetLogoutMode(requestSubjectAndSession) - lookupRecords, err := bcl.GetLogoutRecords(requestSubjectAndSession, logoutMode, s.UserInfoCache) + lookupRecords, err := bcl.GetLogoutRecords(requestSubjectAndSession, s.UserInfoCache) if errors.Is(err, microstore.ErrNotFound) || len(lookupRecords) == 0 { render.Status(r, http.StatusOK) render.JSON(w, r, nil) diff --git a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go index 1863047031..86ee00556b 100644 --- a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go +++ b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go @@ -97,27 +97,27 @@ func NewSuSe(key string) (SuSe, error) { return suse, nil } -// LogoutMode defines the mode of backchannel logout, either by session or by subject -type LogoutMode int +// logoutMode defines the mode of backchannel logout, either by session or by subject +type logoutMode int const ( - // LogoutModeUndefined is used when the logout mode cannot be determined - LogoutModeUndefined LogoutMode = iota - // LogoutModeSubject is used when the logout mode is determined by the subject - LogoutModeSubject - // LogoutModeSession is used when the logout mode is determined by the session id - LogoutModeSession + // logoutModeUndefined is used when the logout mode cannot be determined + logoutModeUndefined logoutMode = iota + // logoutModeSubject is used when the logout mode is determined by the subject + logoutModeSubject + // logoutModeSession is used when the logout mode is determined by the session id + logoutModeSession ) -// GetLogoutMode determines the backchannel logout mode based on the presence of subject and session in the SuSe struct -func GetLogoutMode(suse SuSe) LogoutMode { +// getLogoutMode determines the backchannel logout mode based on the presence of subject and session in the SuSe struct +func getLogoutMode(suse SuSe) logoutMode { switch { case suse.encodedSession == "" && suse.encodedSubject != "": - return LogoutModeSubject + return logoutModeSubject case suse.encodedSession != "": - return LogoutModeSession + return logoutModeSession default: - return LogoutModeUndefined + return logoutModeUndefined } } @@ -128,16 +128,19 @@ var ErrSuspiciousCacheResult = errors.New("suspicious cache result") // logout mode and the provided SuSe struct. // it uses a seperator to prevent sufix and prefix exploration in the cache and checks // if the retrieved records match the requested subject and or session id as well, to prevent false positives. -func GetLogoutRecords(suse SuSe, mode LogoutMode, store microstore.Store) ([]*microstore.Record, error) { +func GetLogoutRecords(suse SuSe, store microstore.Store) ([]*microstore.Record, error) { + // get subject.session mode + mode := getLogoutMode(suse) + var key string var opts []microstore.ReadOption switch mode { - case LogoutModeSubject: + case logoutModeSubject: // the dot at the end prevents prefix exploration in the cache, // so only keys that start with 'subject.*' will be returned, but not 'sub*'. key = suse.encodedSubject + "." opts = append(opts, microstore.ReadPrefix()) - case LogoutModeSession: + case logoutModeSession: // the dot at the beginning prevents sufix exploration in the cache, // so only keys that end with '*.session' will be returned, but not '*sion'. key = "." + suse.encodedSession @@ -156,7 +159,7 @@ func GetLogoutRecords(suse SuSe, mode LogoutMode, store microstore.Store) ([]*mi return nil, microstore.ErrNotFound } - if mode == LogoutModeSession && len(records) > 1 { + if mode == logoutModeSession && len(records) > 1 { return nil, errors.Join(errors.New("multiple session records found"), ErrSuspiciousCacheResult) } @@ -171,10 +174,10 @@ func GetLogoutRecords(suse SuSe, mode LogoutMode, store microstore.Store) ([]*mi switch { // in subject mode, the subject must match, but the session id can be different - case mode == LogoutModeSubject && suse.encodedSubject == recordSuSe.encodedSubject: + case mode == logoutModeSubject && suse.encodedSubject == recordSuSe.encodedSubject: continue // in session mode, the session id must match, but the subject can be different - case mode == LogoutModeSession && suse.encodedSession == recordSuSe.encodedSession: + case mode == logoutModeSession && suse.encodedSession == recordSuSe.encodedSession: continue } diff --git a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go index a653be2247..617bd6d9e0 100644 --- a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go +++ b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go @@ -137,33 +137,33 @@ func TestGetLogoutMode(t *testing.T) { tests := []struct { name string suSe SuSe - want LogoutMode + want logoutMode }{ { name: "key variation: '.session'", suSe: mustNewSuSe(t, "", "session"), - want: LogoutModeSession, + want: logoutModeSession, }, { name: "key variation: 'subject.session'", suSe: mustNewSuSe(t, "subject", "session"), - want: LogoutModeSession, + want: logoutModeSession, }, { name: "key variation: 'subject.'", suSe: mustNewSuSe(t, "subject", ""), - want: LogoutModeSubject, + want: logoutModeSubject, }, { name: "key variation: 'empty'", suSe: SuSe{}, - want: LogoutModeUndefined, + want: logoutModeUndefined, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - mode := GetLogoutMode(tt.suSe) + mode := getLogoutMode(tt.suSe) require.Equal(t, tt.want, mode) }) } @@ -197,34 +197,13 @@ func TestGetLogoutRecords(t *testing.T) { tests := []struct { name string suSe SuSe - mode LogoutMode store func(t *testing.T) store.Store wantRecords []*store.Record wantErrs []error }{ - { - name: "fails if mode is unknown", - suSe: mustNewSuSe(t, "", "session-a"), - mode: LogoutModeUndefined, - store: func(t *testing.T) store.Store { - return sessionStore - }, - wantRecords: []*store.Record{}, - wantErrs: []error{ErrSuspiciousCacheResult}, - }, - { - name: "fails if mode is any random int", - suSe: mustNewSuSe(t, "", "session-a"), - mode: 999, - store: func(t *testing.T) store.Store { - return sessionStore - }, - wantRecords: []*store.Record{}, - wantErrs: []error{ErrSuspiciousCacheResult}}, { name: "fails if multiple session records are found", suSe: mustNewSuSe(t, "", "session-a"), - mode: LogoutModeSession, store: func(t *testing.T) store.Store { s := mocks.NewStore(t) s.EXPECT().Read(mock.Anything, mock.Anything).Return([]*store.Record{ @@ -238,7 +217,6 @@ func TestGetLogoutRecords(t *testing.T) { { name: "fails if the record key is not ok", suSe: mustNewSuSe(t, "", "session-a"), - mode: LogoutModeSession, store: func(t *testing.T) store.Store { s := mocks.NewStore(t) s.EXPECT().Read(mock.Anything, mock.Anything).Return([]*store.Record{ @@ -252,7 +230,6 @@ func TestGetLogoutRecords(t *testing.T) { { name: "fails if the session does not match the retrieved record", suSe: mustNewSuSe(t, "", "session-a"), - mode: LogoutModeSession, store: func(t *testing.T) store.Store { s := mocks.NewStore(t) s.EXPECT().Read(mock.Anything, mock.Anything).Return([]*store.Record{ @@ -265,7 +242,6 @@ func TestGetLogoutRecords(t *testing.T) { { name: "fails if the subject does not match the retrieved record", suSe: mustNewSuSe(t, "subject-a", ""), - mode: LogoutModeSubject, store: func(t *testing.T) store.Store { s := mocks.NewStore(t) s.EXPECT().Read(mock.Anything, mock.Anything).Return([]*store.Record{ @@ -279,7 +255,6 @@ func TestGetLogoutRecords(t *testing.T) { { name: "key variation: 'session-a'", suSe: mustNewSuSe(t, "", "session-a"), - mode: LogoutModeSession, store: func(*testing.T) store.Store { return sessionStore }, @@ -288,7 +263,6 @@ func TestGetLogoutRecords(t *testing.T) { { name: "key variation: 'session-b'", suSe: mustNewSuSe(t, "", "session-b"), - mode: LogoutModeSession, store: func(*testing.T) store.Store { return sessionStore }, @@ -297,7 +271,6 @@ func TestGetLogoutRecords(t *testing.T) { { name: "key variation: 'session-c'", suSe: mustNewSuSe(t, "", "session-c"), - mode: LogoutModeSession, store: func(*testing.T) store.Store { return sessionStore }, @@ -306,7 +279,6 @@ func TestGetLogoutRecords(t *testing.T) { { name: "key variation: 'ession-c'", suSe: mustNewSuSe(t, "", "ession-c"), - mode: LogoutModeSession, store: func(*testing.T) store.Store { return sessionStore }, @@ -316,7 +288,6 @@ func TestGetLogoutRecords(t *testing.T) { { name: "key variation: 'subject-a'", suSe: mustNewSuSe(t, "subject-a", ""), - mode: LogoutModeSubject, store: func(*testing.T) store.Store { return sessionStore }, @@ -325,7 +296,6 @@ func TestGetLogoutRecords(t *testing.T) { { name: "key variation: 'subject-'", suSe: mustNewSuSe(t, "subject-", ""), - mode: LogoutModeSubject, store: func(*testing.T) store.Store { return sessionStore }, @@ -336,7 +306,7 @@ func TestGetLogoutRecords(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - records, err := GetLogoutRecords(tt.suSe, tt.mode, tt.store(t)) + records, err := GetLogoutRecords(tt.suSe, tt.store(t)) for _, wantErr := range tt.wantErrs { require.ErrorIs(t, err, wantErr) } From 9cf02e5c7e03ad8b853d12025b1e54b8ad2db91e Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Thu, 26 Feb 2026 09:11:25 +0100 Subject: [PATCH 12/15] fix: simplify subject.session key parsing Signed-off-by: Christian Richter --- .../backchannellogout/backchannellogout.go | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go index 86ee00556b..fa29c24fb3 100644 --- a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go +++ b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go @@ -68,24 +68,21 @@ var ErrInvalidSubjectOrSession = errors.New("invalid subject or session") // NewSuSe parses the subject and session id from the given key and returns a SuSe struct func NewSuSe(key string) (SuSe, error) { suse := SuSe{} - switch keys := strings.Split(strings.Join(strings.Fields(key), ""), "."); { - // key: '.session' - case len(keys) == 2 && keys[0] == "" && keys[1] != "": - suse.encodedSession = keys[1] - // key: 'subject.' - case len(keys) == 2 && keys[0] != "" && keys[1] == "": - suse.encodedSubject = keys[0] - // key: 'subject.session' - case len(keys) == 2 && keys[0] != "" && keys[1] != "": + keys := strings.Split(key, ".") + switch len(keys) { + case 1: + suse.encodedSession = keys[0] + case 2: suse.encodedSubject = keys[0] suse.encodedSession = keys[1] - // key: 'session' - case len(keys) == 1 && keys[0] != "": - suse.encodedSession = keys[0] default: return suse, ErrInvalidSubjectOrSession } + if suse.encodedSubject == "" && suse.encodedSession == "" { + return suse, ErrInvalidSubjectOrSession + } + if _, err := suse.Subject(); err != nil { return suse, errors.Join(ErrInvalidSubjectOrSession, err) } @@ -134,13 +131,13 @@ func GetLogoutRecords(suse SuSe, store microstore.Store) ([]*microstore.Record, var key string var opts []microstore.ReadOption - switch mode { - case logoutModeSubject: + switch { + case mode == logoutModeSubject && suse.encodedSubject != "": // the dot at the end prevents prefix exploration in the cache, // so only keys that start with 'subject.*' will be returned, but not 'sub*'. key = suse.encodedSubject + "." opts = append(opts, microstore.ReadPrefix()) - case logoutModeSession: + case mode == logoutModeSession && suse.encodedSession != "": // the dot at the beginning prevents sufix exploration in the cache, // so only keys that end with '*.session' will be returned, but not '*sion'. key = "." + suse.encodedSession From 2a2d925ee427bb97c41558dde608fe38ea0193b7 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Wed, 18 Feb 2026 16:38:34 +0100 Subject: [PATCH 13/15] fix(idp): Remove kpop dependency The built package (https://download.kopano.io/community/kapp:/kpop-2.7.2.tgz) seems to be no longer available and upstream lico already switched away from it quite a while ago. Fixes: #2364 --- services/idp/package.json | 10 +-- services/idp/pnpm-lock.yaml | 151 ------------------------------------ services/idp/src/App.jsx | 7 +- services/idp/src/app.css | 10 ++- 4 files changed, 13 insertions(+), 165 deletions(-) diff --git a/services/idp/package.json b/services/idp/package.json index 21da935a91..797bbf38be 100644 --- a/services/idp/package.json +++ b/services/idp/package.json @@ -7,7 +7,7 @@ "analyze": "source-map-explorer 'build/static/js/*.js'", "build": "node --openssl-legacy-provider scripts/build.js && rm -f build/service-worker.js", "licenses": "NODE_PATH=./node_modules node ../scripts/js-license-ranger.js", - "licenses:check": "license-checker-rseidelsohn --summary --relativeLicensePath --onlyAllow 'Python-2.0;Apache*;Apache License, Version 2.0;Apache-2.0;Apache 2.0;Artistic-2.0;BSD;BSD-3-Clause;CC-BY-3.0;CC-BY-4.0;CC0-1.0;ISC;MIT;MPL-2.0;Public Domain;Unicode-TOU;Unlicense;WTFPL;ODC-By-1.0;BlueOak-1.0.0;OFL-1.1' --excludePackages 'identifier;kpop;unicoderegexp' --clarificationsFile license-checker-clarifications.json", + "licenses:check": "license-checker-rseidelsohn --summary --relativeLicensePath --onlyAllow 'Python-2.0;Apache*;Apache License, Version 2.0;Apache-2.0;Apache 2.0;Artistic-2.0;BSD;BSD-3-Clause;CC-BY-3.0;CC-BY-4.0;CC0-1.0;ISC;MIT;MPL-2.0;Public Domain;Unicode-TOU;Unlicense;WTFPL;ODC-By-1.0;BlueOak-1.0.0;OFL-1.1' --excludePackages 'identifier;unicoderegexp' --clarificationsFile license-checker-clarifications.json", "licenses:csv": "license-checker-rseidelsohn --relativeLicensePath --csv --out ../../third-party-licenses/node/idp/third-party-licenses.csv", "licenses:save": "license-checker-rseidelsohn --relativeLicensePath --out /dev/null --files ../../third-party-licenses/node/idp/third-party-licenses", "lint": "eslint ./**/*.{tsx,ts,jsx,js}", @@ -89,7 +89,6 @@ "i18next-browser-languagedetector": "^8.1.0", "i18next-http-backend": "^3.0.2", "i18next-resources-to-backend": "^1.2.1", - "kpop": "https://download.kopano.io/community/kapp:/kpop-2.7.2.tgz", "query-string": "^9.2.0", "react": "^17.0.2", "react-app-polyfill": "^3.0.0", @@ -154,10 +153,5 @@ "webpack-manifest-plugin": "5.0.0", "workbox-webpack-plugin": "7.1.0" }, - "packageManager": "pnpm@9.15.4", - "pnpm": { - "overrides": { - "kpop>cldr": "" - } - } + "packageManager": "pnpm@9.15.4" } diff --git a/services/idp/pnpm-lock.yaml b/services/idp/pnpm-lock.yaml index fae3ee999c..62e9c9372a 100644 --- a/services/idp/pnpm-lock.yaml +++ b/services/idp/pnpm-lock.yaml @@ -4,9 +4,6 @@ settings: autoInstallPeers: true excludeLinksFromLockfile: false -overrides: - kpop>cldr: '' - importers: .: @@ -65,9 +62,6 @@ importers: i18next-resources-to-backend: specifier: ^1.2.1 version: 1.2.1 - kpop: - specifier: https://download.kopano.io/community/kapp:/kpop-2.7.2.tgz - version: https://download.kopano.io/community/kapp:/kpop-2.7.2.tgz(@gluejs/glue@0.3.0)(@material-ui/core@4.12.4(@types/react@17.0.80)(react-dom@17.0.2(react@17.0.2))(react@17.0.2))(@material-ui/icons@4.11.3(@material-ui/core@4.12.4(@types/react@17.0.80)(react-dom@17.0.2(react@17.0.2))(react@17.0.2))(@types/react@17.0.80)(react-dom@17.0.2(react@17.0.2))(react@17.0.2))(notistack@0.8.9(@material-ui/core@4.12.4(@types/react@17.0.80)(react-dom@17.0.2(react@17.0.2))(react@17.0.2))(react-dom@17.0.2(react@17.0.2))(react@17.0.2))(oidc-client@1.11.5)(react-dom@17.0.2(react@17.0.2))(react-intl@2.9.0(prop-types@15.8.1)(react@17.0.2))(react@17.0.2) query-string: specifier: ^9.2.0 version: 9.2.0 @@ -1516,9 +1510,6 @@ packages: '@fontsource/roboto@5.2.5': resolution: {integrity: sha512-70r2UZ0raqLn5W+sPeKhqlf8wGvUXFWlofaDlcbt/S3d06+17gXKr3VNqDODB0I1ASme3dGT5OJj9NABt7OTZQ==} - '@gluejs/glue@0.3.0': - resolution: {integrity: sha512-byvFoZCbZW+A3Pg8JUU+8FjoPuF5l1v7mDeLJQP/YSeEcEDiD/YdUKLBUapPrcuyxclrtS8+peX4cxkh6awwTw==} - '@gulpjs/to-absolute-glob@4.0.0': resolution: {integrity: sha512-kjotm7XJrJ6v+7knhPaRgaT6q8F8K2jiafwYdNHLzmV0uGLuZY43FK6smNSHUPrhq5kX2slCUy+RGG/xGqmIKA==} engines: {node: '>=10.13.0'} @@ -2724,9 +2715,6 @@ packages: core-js@3.40.0: resolution: {integrity: sha512-7vsMc/Lty6AGnn7uFpYT56QesI5D2Y/UkgKounk87OP9Z2H9Z8kj6jzcSGAxFmUtDOS0ntK6lbQz+Nsa0Jj6mQ==} - core-js@3.43.0: - resolution: {integrity: sha512-N6wEbTTZSYOY2rYAn85CuvWWkCK6QweMn7/4Nr3w+gDBeBhk/x4EJeY6FPo4QzDoJZxVTv8U7CMvgWk6pOHHqA==} - core-util-is@1.0.3: resolution: {integrity: sha512-ZQBvi1DcpJ4GDqanjucZ2Hj3wEO5pZDS89BWbkcrvdxksJorwUDDZamX9ldFkp9aw2lmBDLgkObEA4DWNJ9FYQ==} @@ -2738,11 +2726,6 @@ packages: resolution: {integrity: sha512-AdmX6xUzdNASswsFtmwSt7Vj8po9IuqXm0UXz7QKPuEUmPB4XyjGfaAr2PSuELMwkRMVH1EpIkX5bTZGRB3eCA==} engines: {node: '>=10'} - crc32@0.2.2: - resolution: {integrity: sha512-PFZEGbDUeoNbL2GHIEpJRQGheXReDody/9axKTxhXtQqIL443wnNigtVZO9iuCIMPApKZRv7k2xr8euXHqNxQQ==} - engines: {node: '>= 0.4.0'} - hasBin: true - cross-fetch@4.0.0: resolution: {integrity: sha512-e4a5N8lVvuLgAWgnCrLr2PP0YyDOTHa9H/Rj54dirp61qXnNq46m82bRhNqIA5VccJtWBvPTFRV3TtvHUKPB1g==} @@ -2754,9 +2737,6 @@ packages: resolution: {integrity: sha512-uV2QOWP2nWzsy2aMp8aRibhi9dlzF5Hgh5SHaB9OiTGEyDTiJJyx0uy51QXdyWbtAHNua4XJzUKca3OzKUd3vA==} engines: {node: '>= 8'} - crypto-js@4.2.0: - resolution: {integrity: sha512-KALDyEYgpY+Rlob/iriUtjV6d5Eq+Y191A5g4UqLAi8CyGP9N1+FdVbkc1SxKc2r4YAYqG8JzO2KGL+AizD70Q==} - crypto-random-string@2.0.0: resolution: {integrity: sha512-v1plID3y9r/lPhviJ1wrXpLeyUIGAZ2SHNYTEapm7/8A9nLPoyvVp3RK/EPFqn5kEznyWgYZNsRtYYIWbuG8KA==} engines: {node: '>=8'} @@ -3718,9 +3698,6 @@ packages: resolution: {integrity: sha512-r0EI+HBMcXadMrugk0GCQ+6BQV39PiWAZVfq7oIckeGiN7sjRGyQxPdft3nQekFTCQbYxLBH+/axZMeH8UX6+w==} engines: {node: ^14.17.0 || ^16.13.0 || >=18.0.0} - hsv-rgb@1.0.0: - resolution: {integrity: sha512-Azd6IP11LZm0cEczEnJw5B6zIgWdGlE4TSoM2eh+RPRbXSQCy/0JS2POEq0wOtbAZtxTJhEMGm3GUYGbnTIJGw==} - html-escaper@2.0.2: resolution: {integrity: sha512-H2iMtd0I4Mt5eYiapRdIDjp+XzelXQ0tFE4JS7YFwFevXXMmOp9myNrUvCg0D6ws8iqkRPBfKHgbwig1SmlLfg==} @@ -3850,23 +3827,6 @@ packages: resolution: {integrity: sha512-4gd7VpWNQNB4UKKCFFVcp1AVv+FMOgs9NKzjHKusc8jTMhd5eL1NqQqOpE0KzMds804/yHlglp3uxgluOqAPLw==} engines: {node: '>= 0.4'} - intl-format-cache@2.2.9: - resolution: {integrity: sha512-Zv/u8wRpekckv0cLkwpVdABYST4hZNTDaX7reFetrYTJwxExR2VyTqQm+l0WmL0Qo8Mjb9Tf33qnfj0T7pjxdQ==} - - intl-messageformat-parser@1.4.0: - resolution: {integrity: sha512-/XkqFHKezO6UcF4Av2/Lzfrez18R0jyw7kRFhSeB/YRakdrgSc9QfFZUwNJI9swMwMoNPygK1ArC5wdFSjPw+A==} - deprecated: We've written a new parser that's 6x faster and is backwards compatible. Please use @formatjs/icu-messageformat-parser - - intl-messageformat@2.2.0: - resolution: {integrity: sha512-I+tSvHnXqJYjDfNmY95tpFMj30yoakC6OXAo+wu/wTMy6tA/4Fd4mvV7Uzs4cqK/Ap29sHhwjcY+78a8eifcXw==} - - intl-relativeformat@2.2.0: - resolution: {integrity: sha512-4bV/7kSKaPEmu6ArxXf9xjv1ny74Zkwuey8Pm01NH4zggPP7JHwg2STk8Y3JdspCKRDriwIyLRfEXnj2ZLr4Bw==} - deprecated: This package has been deprecated, please see migration guide at 'https://github.com/formatjs/formatjs/tree/master/packages/intl-relativeformat#migration-guide' - - invariant@2.2.4: - resolution: {integrity: sha512-phJfQVBuaJM5raOpJjSfkiD6BpbCE4Ns//LaXl6wGYtUBY83nWS6Rf9tXm2e8VaK60JEjYldbPif/A2B1C2gNA==} - is-arguments@1.2.0: resolution: {integrity: sha512-7bVbi0huj/wrIAOzb8U1aszg9kdi3KN/CyU19CTI7tAoZYEZoL9yCDXpbXN+uPsuWnP02cyug1gleqq+TU+YCA==} engines: {node: '>= 0.4'} @@ -4045,10 +4005,6 @@ packages: isexe@2.0.0: resolution: {integrity: sha512-RHxMLp9lnKHGHRng9QFhRCMbYAcVpn69smSGcq3f36xjgVVWThj4qqLbTLlq7Ssj8B+fIQ1EuCEGI2lKsyQeIw==} - iso-639-1@2.1.15: - resolution: {integrity: sha512-7c7mBznZu2ktfvyT582E2msM+Udc1EjOyhVRE/0ZsjD9LBtWSm23h3PtiRh2a35XoUsTQQjJXaJzuLjXsOdFDg==} - engines: {node: '>=6.0'} - istanbul-lib-coverage@3.2.2: resolution: {integrity: sha512-O8dpsF+r0WV/8MNRKfnmrtCWhuKjxrq2w+jpzBL5UZKTi2LeVWnWOmWRxFlesJONmc+wLAGvKQZEOanko0LFTg==} engines: {node: '>=8'} @@ -4346,20 +4302,6 @@ packages: resolution: {integrity: sha512-dhG34DXATL5hSxJbIexCft8FChFXtmskoZYnoPWjXQuebWYCNkVeV3KkGegCK9CP1oswI/vQibS2GY7Em/sJJA==} engines: {node: '>= 8'} - kpop@https://download.kopano.io/community/kapp:/kpop-2.7.2.tgz: - resolution: {tarball: https://download.kopano.io/community/kapp:/kpop-2.7.2.tgz} - version: 2.7.1 - engines: {node: '>=6.11.0'} - peerDependencies: - '@gluejs/glue': ^0.3.0 - '@material-ui/core': ^4.11.0 - '@material-ui/icons': ^4.9.1 - notistack: ^0.8.8 - oidc-client: ^1.11.0 - react: ^16.8.0 || ^17.0.0 - react-dom: ^16.8.0 || ^17.0.0 - react-intl: ^2.6.0 - language-subtag-registry@0.3.23: resolution: {integrity: sha512-0K65Lea881pHotoGEa5gDlMxt3pctLi2RplBb7Ezh4rRdLEOtgi7n4EwK9lamnUCkKBqaeKRVebTq6BAxSkpXQ==} @@ -4636,13 +4578,6 @@ packages: resolution: {integrity: sha512-bdok/XvKII3nUpklnV6P2hxtMNrCboOjAcyBuQnWEhO665FwrSNRxU+AqpsyvO6LgGYPspN+lu5CLtw4jPRKNA==} engines: {node: '>=0.10.0'} - notistack@0.8.9: - resolution: {integrity: sha512-nRHQVWUfgHnvnKrjRbRX9f+YAnbyh96yRyO5bEP/FCLVLuTZcJOwUr0GZ7Xr/8wK3+hXa9JYpXUkUhSxj1K8NQ==} - peerDependencies: - '@material-ui/core': ^3.2.0 || ^4.0.0 - react: ^16.8.0 - react-dom: ^16.8.0 - now-and-later@3.0.0: resolution: {integrity: sha512-pGO4pzSdaxhWTGkfSfHx3hVzJVslFPwBp2Myq9MYN/ChfJZF87ochMAXnvz6/58RJSf5ik2q9tXprBBrk2cpcg==} engines: {node: '>= 10.13.0'} @@ -4698,9 +4633,6 @@ packages: resolution: {integrity: sha512-gXah6aZrcUxjWg2zR2MwouP2eHlCBzdV4pygudehaKXSGW4v2AsRQUK+lwwXhii6KFZcunEnmSUoYp5CXibxtA==} engines: {node: '>= 0.4'} - oidc-client@1.11.5: - resolution: {integrity: sha512-LcKrKC8Av0m/KD/4EFmo9Sg8fSQ+WFJWBrmtWd+tZkNn3WT/sQG3REmPANE9tzzhbjW6VkTNy4xhAXCfPApAOg==} - once@1.4.0: resolution: {integrity: sha512-lNaJgI+2Q5URQBkccEKHTQOPaXdUxnZZElQTZY0MFUAuaEqe1E+Nyvgdz/aIyNi6Z9MzO5dv1H8n58/GELp3+w==} @@ -5364,12 +5296,6 @@ packages: typescript: optional: true - react-intl@2.9.0: - resolution: {integrity: sha512-27jnDlb/d2A7mSJwrbOBnUgD+rPep+abmoJE511Tf8BnoONIAUehy/U1zZCHGO17mnOwMWxqN4qC0nW11cD6rA==} - peerDependencies: - prop-types: ^15.5.4 - react: ^0.14.9 || ^15.0.0 || ^16.0.0 - react-is@16.13.1: resolution: {integrity: sha512-24e6ynE2H+OKt4kqsOvNd8kBpV65zoxbA4BVsEOB3ARVWQki/DHzaUoC5KuON/BiccDaCCTZBuOcfZs70kR8bQ==} @@ -5698,9 +5624,6 @@ packages: seq@0.3.5: resolution: {integrity: sha512-sisY2Ln1fj43KBkRtXkesnRHYNdswIkIibvNe/0UKm2GZxjMbqmccpiatoKr/k2qX5VKiLU8xm+tz/74LAho4g==} - serialize-javascript@4.0.0: - resolution: {integrity: sha512-GaNA54380uFefWghODBWEGisLZFj00nS5ACs6yHa9nLqlLpVLO8ChDGeKRjZnV4Nh4n0Qi7nhYZD/9fCPzEqkw==} - serialize-javascript@6.0.2: resolution: {integrity: sha512-Saa1xPByTTq2gdeFZYLLo+RFE35NHZkAbqZeWNd3BpzppeVisAqpDjcp8dyf6uIvEqJRd46jemmyA4iFIeVk8g==} @@ -7918,8 +7841,6 @@ snapshots: '@fontsource/roboto@5.2.5': {} - '@gluejs/glue@0.3.0': {} - '@gulpjs/to-absolute-glob@4.0.0': dependencies: is-negated-glob: 1.0.0 @@ -9403,8 +9324,6 @@ snapshots: core-js@3.40.0: {} - core-js@3.43.0: {} - core-util-is@1.0.3: {} cosmiconfig@6.0.0: @@ -9423,8 +9342,6 @@ snapshots: path-type: 4.0.0 yaml: 1.10.2 - crc32@0.2.2: {} - cross-fetch@4.0.0(encoding@0.1.13): dependencies: node-fetch: 2.7.0(encoding@0.1.13) @@ -9443,8 +9360,6 @@ snapshots: shebang-command: 2.0.0 which: 2.0.2 - crypto-js@4.2.0: {} - crypto-random-string@2.0.0: {} css-blank-pseudo@7.0.1(postcss@8.5.4): @@ -10683,8 +10598,6 @@ snapshots: dependencies: lru-cache: 7.18.3 - hsv-rgb@1.0.0: {} - html-escaper@2.0.2: {} html-minifier-terser@6.1.0: @@ -10833,22 +10746,6 @@ snapshots: hasown: 2.0.2 side-channel: 1.1.0 - intl-format-cache@2.2.9: {} - - intl-messageformat-parser@1.4.0: {} - - intl-messageformat@2.2.0: - dependencies: - intl-messageformat-parser: 1.4.0 - - intl-relativeformat@2.2.0: - dependencies: - intl-messageformat: 2.2.0 - - invariant@2.2.4: - dependencies: - loose-envify: 1.4.0 - is-arguments@1.2.0: dependencies: call-bound: 1.0.4 @@ -11007,8 +10904,6 @@ snapshots: isexe@2.0.0: {} - iso-639-1@2.1.15: {} - istanbul-lib-coverage@3.2.2: {} istanbul-lib-instrument@6.0.3: @@ -11539,20 +11434,6 @@ snapshots: klona@2.0.6: {} - kpop@https://download.kopano.io/community/kapp:/kpop-2.7.2.tgz(@gluejs/glue@0.3.0)(@material-ui/core@4.12.4(@types/react@17.0.80)(react-dom@17.0.2(react@17.0.2))(react@17.0.2))(@material-ui/icons@4.11.3(@material-ui/core@4.12.4(@types/react@17.0.80)(react-dom@17.0.2(react@17.0.2))(react@17.0.2))(@types/react@17.0.80)(react-dom@17.0.2(react@17.0.2))(react@17.0.2))(notistack@0.8.9(@material-ui/core@4.12.4(@types/react@17.0.80)(react-dom@17.0.2(react@17.0.2))(react@17.0.2))(react-dom@17.0.2(react@17.0.2))(react@17.0.2))(oidc-client@1.11.5)(react-dom@17.0.2(react@17.0.2))(react-intl@2.9.0(prop-types@15.8.1)(react@17.0.2))(react@17.0.2): - dependencies: - '@gluejs/glue': 0.3.0 - '@material-ui/core': 4.12.4(@types/react@17.0.80)(react-dom@17.0.2(react@17.0.2))(react@17.0.2) - '@material-ui/icons': 4.11.3(@material-ui/core@4.12.4(@types/react@17.0.80)(react-dom@17.0.2(react@17.0.2))(react@17.0.2))(@types/react@17.0.80)(react-dom@17.0.2(react@17.0.2))(react@17.0.2) - crc32: 0.2.2 - hsv-rgb: 1.0.0 - iso-639-1: 2.1.15 - notistack: 0.8.9(@material-ui/core@4.12.4(@types/react@17.0.80)(react-dom@17.0.2(react@17.0.2))(react@17.0.2))(react-dom@17.0.2(react@17.0.2))(react@17.0.2) - oidc-client: 1.11.5 - react: 17.0.2 - react-dom: 17.0.2(react@17.0.2) - react-intl: 2.9.0(prop-types@15.8.1)(react@17.0.2) - language-subtag-registry@0.3.23: {} language-tags@1.0.9: @@ -11787,16 +11668,6 @@ snapshots: normalize-range@0.1.2: {} - notistack@0.8.9(@material-ui/core@4.12.4(@types/react@17.0.80)(react-dom@17.0.2(react@17.0.2))(react@17.0.2))(react-dom@17.0.2(react@17.0.2))(react@17.0.2): - dependencies: - '@material-ui/core': 4.12.4(@types/react@17.0.80)(react-dom@17.0.2(react@17.0.2))(react@17.0.2) - classnames: 2.5.1 - hoist-non-react-statics: 3.3.2 - prop-types: 15.8.1 - react: 17.0.2 - react-dom: 17.0.2(react@17.0.2) - react-is: 16.13.1 - now-and-later@3.0.0: dependencies: once: 1.4.0 @@ -11860,14 +11731,6 @@ snapshots: define-properties: 1.2.1 es-object-atoms: 1.1.1 - oidc-client@1.11.5: - dependencies: - acorn: 7.4.1 - base64-js: 1.5.1 - core-js: 3.43.0 - crypto-js: 4.2.0 - serialize-javascript: 4.0.0 - once@1.4.0: dependencies: wrappy: 1.0.2 @@ -12599,16 +12462,6 @@ snapshots: react-dom: 17.0.2(react@17.0.2) typescript: 5.8.3 - react-intl@2.9.0(prop-types@15.8.1)(react@17.0.2): - dependencies: - hoist-non-react-statics: 3.3.2 - intl-format-cache: 2.2.9 - intl-messageformat: 2.2.0 - intl-relativeformat: 2.2.0 - invariant: 2.2.4 - prop-types: 15.8.1 - react: 17.0.2 - react-is@16.13.1: {} react-is@17.0.2: {} @@ -12975,10 +12828,6 @@ snapshots: chainsaw: 0.0.9 hashish: 0.0.4 - serialize-javascript@4.0.0: - dependencies: - randombytes: 2.1.0 - serialize-javascript@6.0.2: dependencies: randombytes: 2.1.0 diff --git a/services/idp/src/App.jsx b/services/idp/src/App.jsx index 228781534d..315a5876eb 100644 --- a/services/idp/src/App.jsx +++ b/services/idp/src/App.jsx @@ -2,10 +2,7 @@ import React, {ReactElement, Suspense, lazy, useState, useEffect} from 'react'; import PropTypes from 'prop-types'; import {MuiThemeProvider} from '@material-ui/core/styles'; -import {defaultTheme} from 'kpop/es/theme'; - -import 'kpop/static/css/base.css'; -import 'kpop/static/css/scrollbar.css'; +import muiTheme from './theme'; import Spinner from './components/Spinner'; import * as version from './version'; @@ -52,7 +49,7 @@ const App = ({ bgImg }): ReactElement => { className={`oc-login-bg ${bgImg ? 'oc-login-bg-image' : ''}`} style={{backgroundImage: bgImg ? `url(${bgImg})` : undefined}} > - + }> diff --git a/services/idp/src/app.css b/services/idp/src/app.css index 9818ad17d4..f6fd9319f8 100644 --- a/services/idp/src/app.css +++ b/services/idp/src/app.css @@ -1,4 +1,3 @@ -/* additional css on top of kpop */ @font-face { font-family: OpenCloud; src: url('./fonts/OpenCloud500-Regular.woff2') format('woff2'); @@ -17,16 +16,25 @@ html { font-feature-settings: "cv11"; color: #20434f !important; + height: 100%; } body { font-family: OpenCloud, sans-serif; + height: 100%; + margin: 0; + padding: 0; } strong { font-weight: 600; } +#root { + height: 100%; + display: flex; +} + .oc-font-weight-light { font-weight: 300; } From 8a667ff1c7d529be51b65b627052fd2f1a458f2f Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Thu, 26 Feb 2026 09:50:20 +0100 Subject: [PATCH 14/15] readd missing import Signed-off-by: Christian Richter --- services/proxy/pkg/middleware/oidc_auth.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/proxy/pkg/middleware/oidc_auth.go b/services/proxy/pkg/middleware/oidc_auth.go index 78c82e8887..10b5d1f065 100644 --- a/services/proxy/pkg/middleware/oidc_auth.go +++ b/services/proxy/pkg/middleware/oidc_auth.go @@ -13,7 +13,8 @@ import ( "go-micro.dev/v4/store" "golang.org/x/crypto/sha3" "golang.org/x/oauth2" - + "github.com/golang-jwt/jwt/v5" + "github.com/opencloud-eu/opencloud/pkg/log" "github.com/opencloud-eu/opencloud/pkg/oidc" "github.com/opencloud-eu/opencloud/services/proxy/pkg/staticroutes" From f8bed839ce6816a8b740568cc721e0aa875b5dd1 Mon Sep 17 00:00:00 2001 From: Florian Schade Date: Fri, 27 Feb 2026 11:16:53 +0100 Subject: [PATCH 15/15] fix: send the backchannel logout event only if a session exists --- .../pkg/staticroutes/backchannellogout.go | 8 ++- .../backchannellogout/backchannellogout.go | 65 ++++++++++--------- .../backchannellogout_test.go | 65 ++++++------------- 3 files changed, 58 insertions(+), 80 deletions(-) diff --git a/services/proxy/pkg/staticroutes/backchannellogout.go b/services/proxy/pkg/staticroutes/backchannellogout.go index 53375d63bb..28ad2f767e 100644 --- a/services/proxy/pkg/staticroutes/backchannellogout.go +++ b/services/proxy/pkg/staticroutes/backchannellogout.go @@ -107,9 +107,11 @@ func (s *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re continue } - if err := s.publishBackchannelLogoutEvent(r.Context(), session, value); err != nil { - s.Logger.Warn().Err(err).Msgf("failed to publish backchannel logout event for: %s", key) - continue + if requestSubjectAndSession.Mode() == bcl.LogoutModeSession { + if err := s.publishBackchannelLogoutEvent(r.Context(), session, value); err != nil { + s.Logger.Warn().Err(err).Msgf("failed to publish backchannel logout event for: %s", key) + continue + } } err = s.UserInfoCache.Delete(value) diff --git a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go index fa29c24fb3..1d906233cd 100644 --- a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go +++ b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go @@ -31,6 +31,18 @@ func NewKey(subject, session string) (string, error) { return subjectSession, nil } +// LogoutMode defines the mode of backchannel logout, either by session or by subject +type LogoutMode int + +const ( + // LogoutModeUndefined is used when the logout mode cannot be determined + LogoutModeUndefined LogoutMode = iota + // LogoutModeSubject is used when the logout mode is determined by the subject + LogoutModeSubject + // LogoutModeSession is used when the logout mode is determined by the session id + LogoutModeSession +) + // ErrDecoding is returned when decoding fails var ErrDecoding = errors.New("failed to decode") @@ -62,6 +74,18 @@ func (suse SuSe) Session() (string, error) { return string(subject), nil } +// Mode determines the backchannel logout mode based on the presence of subject and session +func (suse SuSe) Mode() LogoutMode { + switch { + case suse.encodedSession == "" && suse.encodedSubject != "": + return LogoutModeSubject + case suse.encodedSession != "": + return LogoutModeSession + default: + return LogoutModeUndefined + } +} + // ErrInvalidSubjectOrSession is returned when the provided key does not match the expected key format var ErrInvalidSubjectOrSession = errors.New("invalid subject or session") @@ -91,31 +115,11 @@ func NewSuSe(key string) (SuSe, error) { return suse, errors.Join(ErrInvalidSubjectOrSession, err) } - return suse, nil -} - -// logoutMode defines the mode of backchannel logout, either by session or by subject -type logoutMode int - -const ( - // logoutModeUndefined is used when the logout mode cannot be determined - logoutModeUndefined logoutMode = iota - // logoutModeSubject is used when the logout mode is determined by the subject - logoutModeSubject - // logoutModeSession is used when the logout mode is determined by the session id - logoutModeSession -) - -// getLogoutMode determines the backchannel logout mode based on the presence of subject and session in the SuSe struct -func getLogoutMode(suse SuSe) logoutMode { - switch { - case suse.encodedSession == "" && suse.encodedSubject != "": - return logoutModeSubject - case suse.encodedSession != "": - return logoutModeSession - default: - return logoutModeUndefined + if mode := suse.Mode(); mode == LogoutModeUndefined { + return suse, ErrInvalidSubjectOrSession } + + return suse, nil } // ErrSuspiciousCacheResult is returned when the cache result is suspicious @@ -126,18 +130,15 @@ var ErrSuspiciousCacheResult = errors.New("suspicious cache result") // it uses a seperator to prevent sufix and prefix exploration in the cache and checks // if the retrieved records match the requested subject and or session id as well, to prevent false positives. func GetLogoutRecords(suse SuSe, store microstore.Store) ([]*microstore.Record, error) { - // get subject.session mode - mode := getLogoutMode(suse) - var key string var opts []microstore.ReadOption switch { - case mode == logoutModeSubject && suse.encodedSubject != "": + case suse.Mode() == LogoutModeSubject && suse.encodedSubject != "": // the dot at the end prevents prefix exploration in the cache, // so only keys that start with 'subject.*' will be returned, but not 'sub*'. key = suse.encodedSubject + "." opts = append(opts, microstore.ReadPrefix()) - case mode == logoutModeSession && suse.encodedSession != "": + case suse.Mode() == LogoutModeSession && suse.encodedSession != "": // the dot at the beginning prevents sufix exploration in the cache, // so only keys that end with '*.session' will be returned, but not '*sion'. key = "." + suse.encodedSession @@ -156,7 +157,7 @@ func GetLogoutRecords(suse SuSe, store microstore.Store) ([]*microstore.Record, return nil, microstore.ErrNotFound } - if mode == logoutModeSession && len(records) > 1 { + if suse.Mode() == LogoutModeSession && len(records) > 1 { return nil, errors.Join(errors.New("multiple session records found"), ErrSuspiciousCacheResult) } @@ -171,10 +172,10 @@ func GetLogoutRecords(suse SuSe, store microstore.Store) ([]*microstore.Record, switch { // in subject mode, the subject must match, but the session id can be different - case mode == logoutModeSubject && suse.encodedSubject == recordSuSe.encodedSubject: + case suse.Mode() == LogoutModeSubject && suse.encodedSubject == recordSuSe.encodedSubject: continue // in session mode, the session id must match, but the subject can be different - case mode == logoutModeSession && suse.encodedSession == recordSuSe.encodedSession: + case suse.Mode() == LogoutModeSession && suse.encodedSession == recordSuSe.encodedSession: continue } diff --git a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go index 617bd6d9e0..40f99f6a4c 100644 --- a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go +++ b/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go @@ -69,28 +69,33 @@ func TestNewSuSe(t *testing.T) { key string wantSubject string wantSession string + wantMode LogoutMode wantErr error }{ { name: "key variation: '.session'", key: mustNewKey(t, "", "session"), wantSession: "session", + wantMode: LogoutModeSession, }, { name: "key variation: 'session'", key: mustNewKey(t, "", "session"), wantSession: "session", + wantMode: LogoutModeSession, }, { name: "key variation: 'subject.'", key: mustNewKey(t, "subject", ""), wantSubject: "subject", + wantMode: LogoutModeSubject, }, { name: "key variation: 'subject.session'", key: mustNewKey(t, "subject", "session"), wantSubject: "subject", wantSession: "session", + wantMode: LogoutModeSession, }, { name: "key variation: 'dot'", @@ -103,19 +108,22 @@ func TestNewSuSe(t *testing.T) { wantErr: ErrInvalidSubjectOrSession, }, { - name: "key variation: string('subject.session')", - key: "subject.session", - wantErr: ErrInvalidSubjectOrSession, + name: "key variation: string('subject.session')", + key: "subject.session", + wantErr: ErrInvalidSubjectOrSession, + wantMode: LogoutModeSession, }, { - name: "key variation: string('subject.')", - key: "subject.", - wantErr: ErrInvalidSubjectOrSession, + name: "key variation: string('subject.')", + key: "subject.", + wantErr: ErrInvalidSubjectOrSession, + wantMode: LogoutModeSubject, }, { - name: "key variation: string('.session')", - key: ".session", - wantErr: ErrInvalidSubjectOrSession, + name: "key variation: string('.session')", + key: ".session", + wantErr: ErrInvalidSubjectOrSession, + wantMode: LogoutModeSession, }, } @@ -124,6 +132,9 @@ func TestNewSuSe(t *testing.T) { suSe, err := NewSuSe(tt.key) require.ErrorIs(t, err, tt.wantErr) + mode := suSe.Mode() + require.Equal(t, tt.wantMode, mode) + subject, _ := suSe.Subject() require.Equal(t, tt.wantSubject, subject) @@ -133,42 +144,6 @@ func TestNewSuSe(t *testing.T) { } } -func TestGetLogoutMode(t *testing.T) { - tests := []struct { - name string - suSe SuSe - want logoutMode - }{ - { - name: "key variation: '.session'", - suSe: mustNewSuSe(t, "", "session"), - want: logoutModeSession, - }, - { - name: "key variation: 'subject.session'", - suSe: mustNewSuSe(t, "subject", "session"), - want: logoutModeSession, - }, - { - name: "key variation: 'subject.'", - suSe: mustNewSuSe(t, "subject", ""), - want: logoutModeSubject, - }, - { - name: "key variation: 'empty'", - suSe: SuSe{}, - want: logoutModeUndefined, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - mode := getLogoutMode(tt.suSe) - require.Equal(t, tt.want, mode) - }) - } -} - func TestGetLogoutRecords(t *testing.T) { sessionStore := store.NewMemoryStore()