resolve logout token subject:sessions for the idp backchannel logout

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: {}

services/proxy/pkg/command/server.go

@@ -10,6 +10,7 @@ import (
 
 	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
 	"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"
@@ -72,6 +73,7 @@ func Server(cfg *config.Config) *cobra.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),
 			)
 

services/proxy/pkg/middleware/oidc_auth.go

@@ -8,13 +8,15 @@ import (
 	"strings"
 	"time"
 
-	"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"
+	"github.com/opencloud-eu/opencloud/services/proxy/pkg/staticroutes"
 )
 
 const (
@@ -114,16 +116,25 @@ 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")
-				}
+			// 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")
+				return
+			}
+
+			if err := m.userInfoCache.Write(&store.Record{
+				Key:    subjectSessionKey,
+				Value:  []byte(encodedHash),
+				Expiry: time.Until(expiration),
+			}); err != nil {
+				m.Logger.Error().Err(err).Msg("failed to write session lookup cache")
 			}
 		}
 	}()

services/proxy/pkg/staticroutes/backchannellogout.go

@@ -6,17 +6,40 @@ import (
 	"net/http"
 
 	"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
+// 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
+//
+// 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")
@@ -27,87 +50,127 @@ 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
+	}
+
+	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, jse{Error: "invalid_request", ErrorDescription: msg})
 		return
 	}
 
-	records, err := s.UserInfoCache.Read(logoutToken.SessionId)
-	if errors.Is(err, microstore.ErrNotFound) || len(records) == 0 {
+	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)
 		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
 	}
 
-	for _, record := range records {
-		err := s.publishBackchannelLogoutEvent(r.Context(), record, logoutToken)
+	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, err := bcl.NewSuSe(key)
+		if err != nil {
+			// never leak any key-related information
+			logger.Warn().Err(err).Msgf("failed to parse key: %s", key)
+			continue
+		}
+
+		session, err := subjectSession.Session()
 		if err != nil {
-			s.Logger.Warn().Err(err).Msg("could not publish backchannel logout event")
+			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(string(record.Value))
+
+		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("could not delete user info from cache")
+			// 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
+			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
 		}
-		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")
+		// we can ignore errors when deleting the lookup record
+		err = s.UserInfoCache.Delete(key)
+		if err != nil {
+			logger.Debug().Err(err).Msgf("failed to delete record for: %s", key)
+		}
 	}
 
 	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, sessionId, claimKey string) error {
 	if s.EventsPublisher == nil {
-		return fmt.Errorf("the events publisher is not set")
-	}
-	urecords, err := s.UserInfoCache.Read(string(record.Value))
-	if err != nil {
-		return fmt.Errorf("reading userinfo cache: %w", err)
+		return errors.New("events publisher not set")
 	}
-	if len(urecords) == 0 {
-		return fmt.Errorf("userinfo not found")
+
+	claimRecords, err := s.UserInfoCache.Read(claimKey)
+	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(urecords[0].Value, &claims); err != nil {
-		return fmt.Errorf("could not unmarshal userinfo: %w", err)
+	if err = msgpack.Unmarshal(claimRecords[0].Value, &claims); err != nil {
+		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{
 		Executant: user.GetId(),
-		SessionId: logoutToken.SessionId,
+		SessionId: sessionId,
 		Timestamp: utils.TSNow(),
 	}
 
 	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
 }