Merge pull request #2795 from fschade/fix-missing-oidc-sub-session

fix: remove unnecessary error log it the oidc access token verify method is set to none

services/proxy/.mockery.yaml

@@ -14,6 +14,6 @@ packages:
       UserRoleAssigner: {}
   go-micro.dev/v4/store:
     config:
-      dir: pkg/staticroutes/internal/backchannellogout/mocks
+      dir: pkg/staticroutes/backchannellogout/mocks
     interfaces:
       Store: {}

services/proxy/pkg/command/server.go

@@ -11,6 +11,12 @@ import (
 	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
 	"github.com/justinas/alice"
 
+	"github.com/opencloud-eu/reva/v2/pkg/events"
+	"github.com/opencloud-eu/reva/v2/pkg/events/stream"
+	"github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool"
+	"github.com/opencloud-eu/reva/v2/pkg/signedurl"
+	"github.com/opencloud-eu/reva/v2/pkg/store"
+
 	"github.com/opencloud-eu/opencloud/pkg/config/configlog"
 	"github.com/opencloud-eu/opencloud/pkg/generators"
 	"github.com/opencloud-eu/opencloud/pkg/log"
@@ -34,11 +40,6 @@ import (
 	"github.com/opencloud-eu/opencloud/services/proxy/pkg/staticroutes"
 	"github.com/opencloud-eu/opencloud/services/proxy/pkg/user/backend"
 	"github.com/opencloud-eu/opencloud/services/proxy/pkg/userroles"
-	"github.com/opencloud-eu/reva/v2/pkg/events"
-	"github.com/opencloud-eu/reva/v2/pkg/events/stream"
-	"github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool"
-	"github.com/opencloud-eu/reva/v2/pkg/signedurl"
-	"github.com/opencloud-eu/reva/v2/pkg/store"
 
 	chimiddleware "github.com/go-chi/chi/v5/middleware"
 	"github.com/spf13/cobra"
@@ -298,6 +299,7 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config,
 		middleware.DefaultAccessTokenTTL(cfg.OIDC.UserinfoCache.TTL),
 		middleware.HTTPClient(oidcHTTPClient),
 		middleware.OIDCIss(cfg.OIDC.Issuer),
+		middleware.AccessTokenVerifyMethod(cfg.OIDC.AccessTokenVerifyMethod),
 		middleware.OIDCClient(oidc.NewOIDCClient(
 			oidc.WithAccessTokenVerifyMethod(cfg.OIDC.AccessTokenVerifyMethod),
 			oidc.WithLogger(logger),

services/proxy/pkg/middleware/oidc_auth.go

@@ -16,7 +16,8 @@ import (
 
 	"github.com/opencloud-eu/opencloud/pkg/log"
 	"github.com/opencloud-eu/opencloud/pkg/oidc"
-	"github.com/opencloud-eu/opencloud/services/proxy/pkg/staticroutes"
+	"github.com/opencloud-eu/opencloud/services/proxy/pkg/config"
+	bcl "github.com/opencloud-eu/opencloud/services/proxy/pkg/staticroutes/backchannellogout"
 )
 
 const (
@@ -31,7 +32,6 @@ func NewOIDCAuthenticator(opts ...Option) *OIDCAuthenticator {
 	return &OIDCAuthenticator{
 		Logger:                  options.Logger,
 		userInfoCache:           options.UserInfoCache,
-		DefaultTokenCacheTTL:    options.DefaultAccessTokenTTL,
 		HTTPClient:              options.HTTPClient,
 		OIDCIss:                 options.OIDCIss,
 		oidcClient:              options.OIDCClient,
@@ -104,38 +104,49 @@ func (m *OIDCAuthenticator) getClaims(token string, req *http.Request) (map[stri
 	// always set an exp claim
 	claims["exp"] = expiration.Unix()
 	go func() {
-		if d, err := msgpack.Marshal(claims); err != nil {
+		d, err := msgpack.Marshal(claims)
+		if err != nil {
 			m.Logger.Error().Err(err).Msg("failed to marshal claims for userinfo cache")
-		} else {
-			err = m.userInfoCache.Write(&store.Record{
-				Key:    encodedHash,
-				Value:  d,
-				Expiry: time.Until(expiration),
-			})
-			if err != nil {
-				m.Logger.Error().Err(err).Msg("failed to write to userinfo cache")
-			}
+			return
+		}
 
-			// 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
-			}
+		err = m.userInfoCache.Write(&store.Record{
+			Key:    encodedHash,
+			Value:  d,
+			Expiry: time.Until(expiration),
+		})
+		if err != nil {
+			m.Logger.Error().Err(err).Msg("failed to write to userinfo cache")
+		}
 
-			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")
-			}
+		// 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 := bcl.NewKey(aClaims.Subject, aClaims.SessionID)
+		switch {
+		// fails if the verify method is set to `none`, in that case the oidc client verification returns
+		// an empty oidcclient.RegClaimsWithSID but no err.
+		//
+		// revisit once:
+		//   - Authelia OpenID Connect Back-Channel Logout 1.0 is implemented,
+		//     e.g. https://www.authelia.com/roadmap/active/openid-connect-1.0-provider/#beta-9
+		case m.AccessTokenVerifyMethod == config.AccessTokenVerificationNone && errors.Is(err, bcl.ErrInvalidKey):
+			return
+		case 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

@@ -10,13 +10,11 @@ import (
 	"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"
-)
 
-// NewRecordKey converts the subject and session to a base64 encoded key
-var NewRecordKey = bcl.NewKey
+	bcl "github.com/opencloud-eu/opencloud/services/proxy/pkg/staticroutes/backchannellogout"
+)
 
 // 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

services/proxy/pkg/staticroutes/backchannellogout/backchannellogout_test.go

@@ -9,7 +9,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"go-micro.dev/v4/store"
 
-	"github.com/opencloud-eu/opencloud/services/proxy/pkg/staticroutes/internal/backchannellogout/mocks"
+	"github.com/opencloud-eu/opencloud/services/proxy/pkg/staticroutes/backchannellogout/mocks"
 )
 
 func mustNewKey(t *testing.T, subject, session string) string {