From 457284885ba69bdfa645c469b2f17122eaedd45c Mon Sep 17 00:00:00 2001 From: Florian Schade Date: Tue, 19 May 2026 18:37:16 +0200 Subject: [PATCH] fix: remove unnecessary error log it the oidc access token verify method is set to none --- services/proxy/.mockery.yaml | 2 +- services/proxy/pkg/command/server.go | 12 +-- services/proxy/pkg/middleware/oidc_auth.go | 73 +++++++++++-------- .../pkg/staticroutes/backchannellogout.go | 6 +- .../backchannellogout/backchannellogout.go | 0 .../backchannellogout_test.go | 2 +- .../backchannellogout/mocks/store.go | 0 7 files changed, 53 insertions(+), 42 deletions(-) rename services/proxy/pkg/staticroutes/{internal => }/backchannellogout/backchannellogout.go (100%) rename services/proxy/pkg/staticroutes/{internal => }/backchannellogout/backchannellogout_test.go (99%) rename services/proxy/pkg/staticroutes/{internal => }/backchannellogout/mocks/store.go (100%) diff --git a/services/proxy/.mockery.yaml b/services/proxy/.mockery.yaml index d3ae0a3817..c363c34b61 100644 --- a/services/proxy/.mockery.yaml +++ b/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: {} diff --git a/services/proxy/pkg/command/server.go b/services/proxy/pkg/command/server.go index 2debb13dd2..01e485733c 100644 --- a/services/proxy/pkg/command/server.go +++ b/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), diff --git a/services/proxy/pkg/middleware/oidc_auth.go b/services/proxy/pkg/middleware/oidc_auth.go index eb1ca83e27..7a565ed1bd 100644 --- a/services/proxy/pkg/middleware/oidc_auth.go +++ b/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") } }() diff --git a/services/proxy/pkg/staticroutes/backchannellogout.go b/services/proxy/pkg/staticroutes/backchannellogout.go index 136b598b68..83d864f03e 100644 --- a/services/proxy/pkg/staticroutes/backchannellogout.go +++ b/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 diff --git a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go b/services/proxy/pkg/staticroutes/backchannellogout/backchannellogout.go similarity index 100% rename from services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout.go rename to services/proxy/pkg/staticroutes/backchannellogout/backchannellogout.go diff --git a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go b/services/proxy/pkg/staticroutes/backchannellogout/backchannellogout_test.go similarity index 99% rename from services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go rename to services/proxy/pkg/staticroutes/backchannellogout/backchannellogout_test.go index 40f99f6a4c..fead1c0310 100644 --- a/services/proxy/pkg/staticroutes/internal/backchannellogout/backchannellogout_test.go +++ b/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 { diff --git a/services/proxy/pkg/staticroutes/internal/backchannellogout/mocks/store.go b/services/proxy/pkg/staticroutes/backchannellogout/mocks/store.go similarity index 100% rename from services/proxy/pkg/staticroutes/internal/backchannellogout/mocks/store.go rename to services/proxy/pkg/staticroutes/backchannellogout/mocks/store.go