From 30bcf320628f11424272477c25cef550137643c6 Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Wed, 19 Apr 2023 17:17:23 +0200 Subject: [PATCH] incorporate requested changes Signed-off-by: Christian Richter --- ocis-pkg/middleware/oidc.go | 6 ++-- ocis-pkg/oidc/client.go | 38 +++++++++++----------- ocis-pkg/oidc/client_test.go | 26 +++++++-------- ocis-pkg/oidc/metadata.go | 3 +- ocis-pkg/oidc/options.go | 14 ++++---- services/proxy/pkg/command/server.go | 19 ++++++----- services/proxy/pkg/middleware/oidc_auth.go | 6 ++-- 7 files changed, 57 insertions(+), 55 deletions(-) diff --git a/ocis-pkg/middleware/oidc.go b/ocis-pkg/middleware/oidc.go index 2c1dd5b194..4bab746e2b 100644 --- a/ocis-pkg/middleware/oidc.go +++ b/ocis-pkg/middleware/oidc.go @@ -6,7 +6,7 @@ import ( "strings" "sync" - gOidc "github.com/coreos/go-oidc/v3/oidc" + goidc "github.com/coreos/go-oidc/v3/oidc" "github.com/owncloud/ocis/v2/ocis-pkg/oidc" "golang.org/x/oauth2" ) @@ -24,7 +24,7 @@ func newOidcOptions(opts ...Option) Options { // OIDCProvider used to mock the oidc provider during tests type OIDCProvider interface { - UserInfo(ctx context.Context, ts oauth2.TokenSource) (*gOidc.UserInfo, error) + UserInfo(ctx context.Context, ts oauth2.TokenSource) (*goidc.UserInfo, error) } // OidcAuth provides a middleware to authenticate a bearer auth with an OpenID Connect identity provider @@ -38,7 +38,7 @@ func OidcAuth(opts ...Option) func(http.Handler) http.Handler { // Initialize a provider by specifying the issuer URL. // it will fetch the keys from the issuer using the .well-known // endpoint - return gOidc.NewProvider( + return goidc.NewProvider( context.WithValue(context.Background(), oauth2.HTTPClient, http.Client{}), opt.OidcIssuer, ) diff --git a/ocis-pkg/oidc/client.go b/ocis-pkg/oidc/client.go index 83ec35210a..0a27708363 100644 --- a/ocis-pkg/oidc/client.go +++ b/ocis-pkg/oidc/client.go @@ -15,7 +15,7 @@ import ( "time" "github.com/MicahParks/keyfunc" - gOidc "github.com/coreos/go-oidc/v3/oidc" + goidc "github.com/coreos/go-oidc/v3/oidc" "github.com/golang-jwt/jwt/v4" "github.com/owncloud/ocis/v2/ocis-pkg/log" "github.com/owncloud/ocis/v2/services/proxy/pkg/config" @@ -30,7 +30,7 @@ type OIDCClient interface { VerifyLogoutToken(ctx context.Context, token string) (*LogoutToken, error) } -// KeySet is a set of publc JSON Web Keys that can be used to validate the signature +// KeySet is a set of public JSON Web Keys that can be used to validate the signature // of JSON web tokens. This is expected to be backed by a remote key set through // provider metadata discovery or an in-memory set of keys delivered out-of-band. type KeySet interface { @@ -66,10 +66,10 @@ type oidcClient struct { httpClient *http.Client } -// supportedAlgorithms is a list of algorithms explicitly supported by this +// _supportedAlgorithms is a list of algorithms explicitly supported by this // package. If a provider supports other algorithms, such as HS256 or none, // those values won't be passed to the IDTokenVerifier. -var supportedAlgorithms = map[string]bool{ +var _supportedAlgorithms = map[string]bool{ RS256: true, RS384: true, RS512: true, @@ -81,13 +81,13 @@ var supportedAlgorithms = map[string]bool{ PS512: true, } -// NewOIDCClient returns an OIDC client for the given issuer +// NewOIDCClient returns an OIDClient instance for the given issuer func NewOIDCClient(opts ...Option) OIDCClient { options := newOptions(opts...) return &oidcClient{ Logger: options.Logger, - issuer: options.OidcIssuer, + issuer: options.OIDCIssuer, httpClient: options.HTTPClient, accessTokenVerifyMethod: options.AccessTokenVerifyMethod, JWKSOptions: options.JWKSOptions, // TODO I don't like that we pass down config options ... @@ -135,13 +135,13 @@ func (c *oidcClient) lookupWellKnownOpenidConfiguration(ctx context.Context) err } var algs []string for _, a := range p.IDTokenSigningAlgValuesSupported { - if supportedAlgorithms[a] { + if _supportedAlgorithms[a] { algs = append(algs, a) } } c.provider = &p c.algorithms = algs - c.remoteKeySet = gOidc.NewRemoteKeySet(gOidc.ClientContext(ctx, c.httpClient), p.JwksURI) + c.remoteKeySet = goidc.NewRemoteKeySet(goidc.ClientContext(ctx, c.httpClient), p.JwksURI) } return nil } @@ -174,6 +174,7 @@ func (c *oidcClient) getKeyfunc() *keyfunc.JWKS { type stringAsBool bool +// Claims unmarshals the raw JSON string into a bool. func (sb *stringAsBool) UnmarshalJSON(b []byte) error { switch string(b) { case "true", `"true"`: @@ -214,15 +215,14 @@ func (u *UserInfo) Claims(v interface{}) error { return json.Unmarshal(u.claims, v) } +// UserInfo retrieves the userinfo from a Token func (c *oidcClient) UserInfo(ctx context.Context, tokenSource oauth2.TokenSource) (*UserInfo, error) { if err := c.lookupWellKnownOpenidConfiguration(ctx); err != nil { return nil, err } if c.provider.UserinfoEndpoint == "" { - if c.provider.UserinfoEndpoint == "" { - return nil, errors.New("oidc: user info endpoint is not supported by this provider") - } + return nil, errors.New("oidc: user info endpoint is not supported by this provider") } req, err := http.NewRequest("GET", c.provider.UserinfoEndpoint, nil) @@ -250,9 +250,9 @@ func (c *oidcClient) UserInfo(ctx context.Context, tokenSource oauth2.TokenSourc } ct := resp.Header.Get("Content-Type") - mediaType, _, parseErr := mime.ParseMediaType(ct) - if parseErr == nil && mediaType == "application/jwt" { - payload, err := c.remoteKeySet.VerifySignature(gOidc.ClientContext(ctx, c.httpClient), string(body)) + mediaType, _, err := mime.ParseMediaType(ct) + if err == nil && mediaType == "application/jwt" { + payload, err := c.remoteKeySet.VerifySignature(goidc.ClientContext(ctx, c.httpClient), string(body)) if err != nil { return nil, fmt.Errorf("oidc: invalid userinfo jwt signature %v", err) } @@ -348,10 +348,9 @@ func (c *oidcClient) VerifyLogoutToken(ctx context.Context, rawToken string) (*L return nil, fmt.Errorf("oidc: logout token must contain logout event") } //6. Verify that the Logout Token does not contain a nonce Claim. - type nonce struct { + var n struct { Nonce *string `json:"nonce"` } - var n nonce json.Unmarshal(payload, &n) if n.Nonce != nil { return nil, fmt.Errorf("oidc: nonce on logout token MUST NOT be present") @@ -378,6 +377,7 @@ func (c *oidcClient) VerifyLogoutToken(ctx context.Context, rawToken string) (*L case 0: return nil, fmt.Errorf("oidc: logout token not signed") case 1: + // do nothing default: return nil, fmt.Errorf("oidc: multiple signatures on logout token not supported") } @@ -392,7 +392,7 @@ func (c *oidcClient) VerifyLogoutToken(ctx context.Context, rawToken string) (*L return nil, fmt.Errorf("oidc: logout token signed with unsupported algorithm, expected %q got %q", supportedSigAlgs, sig.Header.Algorithm) } - gotPayload, err := c.remoteKeySet.VerifySignature(gOidc.ClientContext(ctx, c.httpClient), rawToken) + gotPayload, err := c.remoteKeySet.VerifySignature(goidc.ClientContext(ctx, c.httpClient), rawToken) if err != nil { return nil, fmt.Errorf("failed to verify signature: %v", err) } @@ -411,8 +411,8 @@ func unmarshalResp(r *http.Response, body []byte, v interface{}) error { return nil } ct := r.Header.Get("Content-Type") - mediaType, _, parseErr := mime.ParseMediaType(ct) - if parseErr == nil && mediaType == "application/json" { + mediaType, _, err := mime.ParseMediaType(ct) + if err == nil && mediaType == "application/json" { return fmt.Errorf("got Content-Type = application/json, but could not unmarshal as JSON: %v", err) } return fmt.Errorf("expected Content-Type = application/json, got %q: %v", ct, err) diff --git a/ocis-pkg/oidc/client_test.go b/ocis-pkg/oidc/client_test.go index 25d77dc4bf..e33dbf0d7a 100644 --- a/ocis-pkg/oidc/client_test.go +++ b/ocis-pkg/oidc/client_test.go @@ -9,7 +9,7 @@ import ( "fmt" "testing" - gOidc "github.com/coreos/go-oidc/v3/oidc" + goidc "github.com/coreos/go-oidc/v3/oidc" "github.com/owncloud/ocis/v2/ocis-pkg/oidc" "gopkg.in/square/go-jose.v2" ) @@ -60,7 +60,7 @@ func TestLogoutVerify(t *testing.T) { "http://schemas.openid.net/event/backchannel-logout": {} } }`, - config: gOidc.Config{ + config: goidc.Config{ SkipClientIDCheck: true, }, signKey: newRSAKey(t), @@ -69,7 +69,7 @@ func TestLogoutVerify(t *testing.T) { name: "invalid issuer", issuer: "https://bar", logoutToken: `{"iss":"https://foo"}`, - config: gOidc.Config{ + config: goidc.Config{ SkipClientIDCheck: true, SkipExpiryCheck: true, }, @@ -89,7 +89,7 @@ func TestLogoutVerify(t *testing.T) { "http://schemas.openid.net/event/backchannel-logout": {} } }`, - config: gOidc.Config{ + config: goidc.Config{ SkipClientIDCheck: true, SkipExpiryCheck: true, }, @@ -108,7 +108,7 @@ func TestLogoutVerify(t *testing.T) { "http://schemas.openid.net/event/backchannel-logout": {} } }`, - config: gOidc.Config{ + config: goidc.Config{ SkipClientIDCheck: true, }, signKey: newRSAKey(t), @@ -127,7 +127,7 @@ func TestLogoutVerify(t *testing.T) { "http://schemas.openid.net/event/backchannel-logout": {} } }`, - config: gOidc.Config{ + config: goidc.Config{ SkipClientIDCheck: true, }, signKey: newRSAKey(t), @@ -146,7 +146,7 @@ func TestLogoutVerify(t *testing.T) { "not a logout event": {} } }`, - config: gOidc.Config{ + config: goidc.Config{ SkipClientIDCheck: true, }, signKey: newRSAKey(t), @@ -162,7 +162,7 @@ func TestLogoutVerify(t *testing.T) { "jti": "bWJq", "sid": "08a5019c-17e1-4977-8f42-65a12843ea02", }`, - config: gOidc.Config{ + config: goidc.Config{ SkipClientIDCheck: true, }, signKey: newRSAKey(t), @@ -182,7 +182,7 @@ func TestVerifyAudienceLogout(t *testing.T) { "http://schemas.openid.net/event/backchannel-logout": {} } }`, - config: gOidc.Config{ + config: goidc.Config{ ClientID: "client1", SkipExpiryCheck: true, }, @@ -193,7 +193,7 @@ func TestVerifyAudienceLogout(t *testing.T) { logoutToken: `{"iss":"https://foo","aud":"client2","sub":"subject","events": { "http://schemas.openid.net/event/backchannel-logout": {} }}`, - config: gOidc.Config{ + config: goidc.Config{ ClientID: "client1", SkipExpiryCheck: true, }, @@ -205,7 +205,7 @@ func TestVerifyAudienceLogout(t *testing.T) { logoutToken: `{"iss":"https://foo","aud":["client1","client2"],"sub":"subject","events": { "http://schemas.openid.net/event/backchannel-logout": {} }}`, - config: gOidc.Config{ + config: goidc.Config{ ClientID: "client2", SkipExpiryCheck: true, }, @@ -233,7 +233,7 @@ type logoutVerificationTest struct { // testing invalid signatures. verificationKey *signingKey - config gOidc.Config + config goidc.Config wantErr bool } @@ -255,7 +255,7 @@ func (v logoutVerificationTest) runGetToken(t *testing.T) (*oidc.LogoutToken, er ctx, cancel := context.WithCancel(context.Background()) defer cancel() issuer := "https://foo" - var ks gOidc.KeySet + var ks goidc.KeySet if v.verificationKey == nil { ks = &testVerifier{v.signKey.jwk()} } else { diff --git a/ocis-pkg/oidc/metadata.go b/ocis-pkg/oidc/metadata.go index 52d1b918ce..c10db68b6e 100644 --- a/ocis-pkg/oidc/metadata.go +++ b/ocis-pkg/oidc/metadata.go @@ -54,7 +54,7 @@ type ProviderMetadata struct { //claim_types_supported } -// Logout Token +// Logout Token defines an logout Token type LogoutToken struct { // The URL of the server which issued this token. OpenID Connect // requires this value always be identical to the URL used for @@ -87,6 +87,7 @@ type LogoutToken struct { JwtID string `json:"jti"` } +// LogoutEvent defines a logout Event type LogoutEvent struct { Event *struct{} `json:"http://schemas.openid.net/event/backchannel-logout"` } diff --git a/ocis-pkg/oidc/options.go b/ocis-pkg/oidc/options.go index 5e0aafd7fe..86439532ae 100644 --- a/ocis-pkg/oidc/options.go +++ b/ocis-pkg/oidc/options.go @@ -6,7 +6,7 @@ import ( "github.com/owncloud/ocis/v2/ocis-pkg/log" "github.com/owncloud/ocis/v2/services/proxy/pkg/config" - gOidc "github.com/coreos/go-oidc/v3/oidc" + goidc "github.com/coreos/go-oidc/v3/oidc" ) // Option defines a single option function. @@ -19,7 +19,7 @@ type Options struct { // Logger to use for logging, must be set Logger log.Logger // The OpenID Connect Issuer URL - OidcIssuer string + OIDCIssuer string // JWKSOptions to use when retrieving keys JWKSOptions config.JWKS // KeySet to use when verifiing signatures @@ -33,7 +33,7 @@ type Options struct { // SkipClientIDCheck must be true if ClientID is empty SkipClientIDCheck bool // Config to use - Config *gOidc.Config + Config *goidc.Config // ProviderMetadata to use ProviderMetadata *ProviderMetadata @@ -50,10 +50,10 @@ func newOptions(opts ...Option) Options { return opt } -// WithLogger provides a function to set the openid connect issuer option. +// WithOidcIssuer provides a function to set the openid connect issuer option. func WithOidcIssuer(val string) Option { return func(o *Options) { - o.OidcIssuer = val + o.OIDCIssuer = val } } @@ -106,8 +106,8 @@ func WithSkipClientIDCheck(val bool) Option { } } -// WithSkipClientIDCheck provides a function to set the skipClientIDCheck option. -func WithConfig(val *gOidc.Config) Option { +// WithConfig provides a function to set the Config option. +func WithConfig(val *goidc.Config) Option { return func(o *Options) { o.Config = val } diff --git a/services/proxy/pkg/command/server.go b/services/proxy/pkg/command/server.go index 85b550689e..e61dd9ff60 100644 --- a/services/proxy/pkg/command/server.go +++ b/services/proxy/pkg/command/server.go @@ -41,15 +41,6 @@ import ( microstore "go-micro.dev/v4/store" ) -type StaticRouteHandler struct { - prefix string - proxy http.Handler - userInfoCache microstore.Store - logger log.Logger - config config.Config - oidcClient oidc.OIDCClient -} - // Server is the entrypoint for the server command. func Server(cfg *config.Config) *cli.Command { return &cli.Command{ @@ -188,6 +179,16 @@ func Server(cfg *config.Config) *cli.Command { } } +// StaticRouteHandler defines a Route Handler for static routes +type StaticRouteHandler struct { + prefix string + proxy http.Handler + userInfoCache microstore.Store + logger log.Logger + config config.Config + oidcClient oidc.OIDCClient +} + func (h *StaticRouteHandler) handler() http.Handler { m := chi.NewMux() var methods = []string{"PROPFIND", "DELETE", "PROPPATCH", "MKCOL", "COPY", "MOVE", "LOCK", "UNLOCK", "REPORT"} diff --git a/services/proxy/pkg/middleware/oidc_auth.go b/services/proxy/pkg/middleware/oidc_auth.go index 4e46dc70c1..b8a57e1c6a 100644 --- a/services/proxy/pkg/middleware/oidc_auth.go +++ b/services/proxy/pkg/middleware/oidc_auth.go @@ -112,9 +112,9 @@ func (m *OIDCAuthenticator) getClaims(token string, req *http.Request) (map[stri Value: []byte(encodedHash), Expiry: time.Until(expiration), }) - } - if err != nil { - m.Logger.Error().Err(err).Msg("failed to write session lookup cache") + if err != nil { + m.Logger.Error().Err(err).Msg("failed to write session lookup cache") + } } } }()