incorporate requested changes

Signed-off-by: Christian Richter <crichter@owncloud.com>

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,
 		)

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)

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 {

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"`
 }

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
 	}

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"}

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")
+				}
 			}
 		}
 	}()