clean up the authenticators middlewares

services/proxy/pkg/command/server.go

@@ -19,6 +19,7 @@ import (
 	"github.com/owncloud/ocis/v2/ocis-pkg/sync"
 	"github.com/owncloud/ocis/v2/ocis-pkg/version"
 	settingssvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/settings/v0"
+	storesvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/store/v0"
 	"github.com/owncloud/ocis/v2/services/proxy/pkg/config"
 	"github.com/owncloud/ocis/v2/services/proxy/pkg/config/parser"
 	"github.com/owncloud/ocis/v2/services/proxy/pkg/cs3"
@@ -149,7 +150,7 @@ func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config)
 		logger.Fatal().Msgf("Invalid accounts backend type '%s'", cfg.AccountBackend)
 	}
 
-	// storeClient := storesvc.NewStoreService("com.owncloud.api.store", grpc.DefaultClient)
+	storeClient := storesvc.NewStoreService("com.owncloud.api.store", grpc.DefaultClient)
 	if err != nil {
 		logger.Error().Err(err).
 			Str("gateway", cfg.Reva.Address).
@@ -193,10 +194,17 @@ func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config)
 		JWKSOptions:             cfg.OIDC.JWKS,
 		AccessTokenVerifyMethod: cfg.OIDC.AccessTokenVerifyMethod,
 	})
-	// authenticators = append(authenticators, middleware.PublicShareAuthenticator{
-	// 	Logger:            logger,
-	// 	RevaGatewayClient: revaClient,
-	// })
+	authenticators = append(authenticators, middleware.PublicShareAuthenticator{
+		Logger:            logger,
+		RevaGatewayClient: revaClient,
+	})
+
+	authenticators = append(authenticators, middleware.SignedURLAuthenticator{
+		Logger:             logger,
+		PreSignedURLConfig: cfg.PreSignedURL,
+		UserProvider:       userProvider,
+		Store:              storeClient,
+	})
 
 	return alice.New(
 		// first make sure we log all requests and redirect to https if necessary
@@ -211,18 +219,6 @@ func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config)
 			oidcHTTPClient,
 		),
 
-		// middleware.AuthenticationOld(
-		// 	// OIDC Options
-		// 	middleware.OIDCProviderFunc(func() (middleware.OIDCProvider, error) {
-		// 		// Initialize a provider by specifying the issuer URL.
-		// 		// it will fetch the keys from the issuer using the .well-known
-		// 		// endpoint
-		// 		return oidc.NewProvider(
-		// 			context.WithValue(ctx, oauth2.HTTPClient, oidcHTTPClient),
-		// 			cfg.OIDC.Issuer,
-		// 		)
-		// 	}),
-
 		middleware.Authentication(
 			authenticators,
 			middleware.CredentialsByUserAgent(cfg.AuthMiddleware.CredentialsByUserAgent),
@@ -230,25 +226,6 @@ func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config)
 			middleware.OIDCIss(cfg.OIDC.Issuer),
 			middleware.EnableBasicAuth(cfg.EnableBasicAuth),
 		),
-		// 	middleware.HTTPClient(oidcHTTPClient),
-		// 	middleware.TokenCacheSize(cfg.OIDC.UserinfoCache.Size),
-		// 	middleware.TokenCacheTTL(time.Second*time.Duration(cfg.OIDC.UserinfoCache.TTL)),
-		//
-		// 	// basic Options
-		// 	middleware.Logger(logger),
-		// 	middleware.EnableBasicAuth(cfg.EnableBasicAuth),
-		// 	middleware.UserProvider(userProvider),
-		// 	middleware.OIDCIss(cfg.OIDC.Issuer),
-		// 	middleware.UserOIDCClaim(cfg.UserOIDCClaim),
-		// 	middleware.UserCS3Claim(cfg.UserCS3Claim),
-		// 	middleware.CredentialsByUserAgent(cfg.AuthMiddleware.CredentialsByUserAgent),
-		// ),
-		// middleware.SignedURLAuth(
-		// 	middleware.Logger(logger),
-		// 	middleware.PreSignedURLConfig(cfg.PreSignedURL),
-		// 	middleware.UserProvider(userProvider),
-		// 	middleware.Store(storeClient),
-		// ),
 		middleware.AccountResolver(
 			middleware.Logger(logger),
 			middleware.UserProvider(userProvider),
@@ -270,9 +247,5 @@ func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config)
 			middleware.TokenManagerConfig(*cfg.TokenManager),
 			middleware.RevaGatewayClient(revaClient),
 		),
-		// middleware.PublicShareAuth(
-		// 	middleware.Logger(logger),
-		// 	middleware.RevaGatewayClient(revaClient),
-		// ),
 	)
 }

services/proxy/pkg/middleware/authentication.go

@@ -18,7 +18,7 @@ var (
 	// regexp.Regexp which are safe to use concurrently.
 	ProxyWwwAuthenticate = []regexp.Regexp{*regexp.MustCompile("/ocs/v[12].php/cloud/")}
 
-	_publicPaths = []string{
+	_publicPaths = [...]string{
 		"/dav/public-files/",
 		"/remote.php/dav/public-files/",
 		"/remote.php/ocs/apps/files_sharing/api/v1/tokeninfo/unprotected",
@@ -164,57 +164,3 @@ func evalRequestURI(l userAgentLocker, r regexp.Regexp) {
 	}
 	l.w.Header().Add(WwwAuthenticate, fmt.Sprintf("%v realm=\"%s\", charset=\"UTF-8\"", strings.Title(l.fallback), l.r.Host))
 }
-
-// AuthenticationOld is a higher order authentication middleware.
-func AuthenticationOld(opts ...Option) func(next http.Handler) http.Handler {
-	options := newOptions(opts...)
-
-	configureSupportedChallenges(options)
-	oidc := newOIDCAuth(options)
-	// basic := newBasicAuth(options)
-
-	return func(next http.Handler) http.Handler {
-		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			if options.OIDCIss != "" && options.EnableBasicAuth {
-				//oidc(basic(next)).ServeHTTP(w, r)
-				oidc(next).ServeHTTP(w, r)
-			}
-
-			if options.OIDCIss != "" && !options.EnableBasicAuth {
-				oidc(next).ServeHTTP(w, r)
-			}
-
-			// if options.OIDCIss == "" && options.EnableBasicAuth {
-			// 	basic(next).ServeHTTP(w, r)
-			// }
-		})
-	}
-}
-
-// newOIDCAuth returns a configured oidc middleware
-func newOIDCAuth(options Options) func(http.Handler) http.Handler {
-	return OIDCAuth(
-		Logger(options.Logger),
-		OIDCProviderFunc(options.OIDCProviderFunc),
-		HTTPClient(options.HTTPClient),
-		OIDCIss(options.OIDCIss),
-		TokenCacheSize(options.UserinfoCacheSize),
-		TokenCacheTTL(options.UserinfoCacheTTL),
-		CredentialsByUserAgent(options.CredentialsByUserAgent),
-		AccessTokenVerifyMethod(options.AccessTokenVerifyMethod),
-		JWKSOptions(options.JWKS),
-	)
-}
-
-// // newBasicAuth returns a configured basic middleware
-// func newBasicAuth(options Options) func(http.Handler) http.Handler {
-// 	return BasicAuth(
-// 		UserProvider(options.UserProvider),
-// 		Logger(options.Logger),
-// 		EnableBasicAuth(options.EnableBasicAuth),
-// 		OIDCIss(options.OIDCIss),
-// 		UserOIDCClaim(options.UserOIDCClaim),
-// 		UserCS3Claim(options.UserCS3Claim),
-// 		CredentialsByUserAgent(options.CredentialsByUserAgent),
-// 	)
-// }

services/proxy/pkg/middleware/basic_auth.go

@@ -15,22 +15,26 @@ type BasicAuthenticator struct {
 	UserOIDCClaim string
 }
 
-func (m BasicAuthenticator) Authenticate(req *http.Request) (*http.Request, bool) {
-	if isPublicPath(req.URL.Path) {
+func (m BasicAuthenticator) Authenticate(r *http.Request) (*http.Request, bool) {
+	if isPublicPath(r.URL.Path) {
 		// The authentication of public path requests is handled by another authenticator.
 		// Since we can't guarantee the order of execution of the authenticators, we better
 		// implement an early return here for paths we can't authenticate in this authenticator.
 		return nil, false
 	}
 
-	login, password, ok := req.BasicAuth()
+	login, password, ok := r.BasicAuth()
 	if !ok {
 		return nil, false
 	}
 
-	user, _, err := m.UserProvider.Authenticate(req.Context(), login, password)
+	user, _, err := m.UserProvider.Authenticate(r.Context(), login, password)
 	if err != nil {
-		// TODO add log line
+		m.Logger.Error().
+			Err(err).
+			Str("authenticator", "basic").
+			Str("path", r.URL.Path).
+			Msg("failed to authenticate request")
 		return nil, false
 	}
 
@@ -48,5 +52,9 @@ func (m BasicAuthenticator) Authenticate(req *http.Request) (*http.Request, bool
 		claims[m.UserOIDCClaim] = user.Id.OpaqueId
 
 	}
-	return req.WithContext(oidc.NewContext(req.Context(), claims)), true
+	m.Logger.Debug().
+		Str("authenticator", "basic").
+		Str("path", r.URL.Path).
+		Msg("successfully authenticated request")
+	return r.WithContext(oidc.NewContext(r.Context(), claims)), true
 }

services/proxy/pkg/middleware/oidc_auth.go

@@ -3,7 +3,6 @@ package middleware
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"io/ioutil"
 	"net/http"
 	"strings"
@@ -17,9 +16,20 @@ import (
 	"github.com/owncloud/ocis/v2/ocis-pkg/oidc"
 	osync "github.com/owncloud/ocis/v2/ocis-pkg/sync"
 	"github.com/owncloud/ocis/v2/services/proxy/pkg/config"
+	"github.com/pkg/errors"
 	"golang.org/x/oauth2"
 )
 
+const (
+	_headerAuthorization = "Authorization"
+	_bearerPrefix        = "Bearer "
+)
+
+var (
+	// _unauthenticatePaths contains paths which don't need to be authenticated.
+	_unauthenticatePaths = [...]string{"/konnect/v1/userinfo", "/status.php"}
+)
+
 // OIDCProvider used to mock the oidc provider during tests
 type OIDCProvider interface {
 	UserInfo(ctx context.Context, ts oauth2.TokenSource) (*gOidc.UserInfo, error)
@@ -42,14 +52,13 @@ type OIDCAuthenticator struct {
 	JWKS     *keyfunc.JWKS
 }
 
-func (m OIDCAuthenticator) getClaims(token string, req *http.Request) (claims map[string]interface{}, status int) {
+func (m OIDCAuthenticator) getClaims(token string, req *http.Request) (map[string]interface{}, error) {
+	var claims map[string]interface{}
 	hit := m.TokenCache.Load(token)
 	if hit == nil {
 		aClaims, err := m.verifyAccessToken(token)
 		if err != nil {
-			m.Logger.Error().Err(err).Msg("Failed to verify access token")
-			status = http.StatusUnauthorized
-			return
+			return nil, errors.Wrap(err, "failed to verify access token")
 		}
 
 		oauth2Token := &oauth2.Token{
@@ -61,31 +70,25 @@ func (m OIDCAuthenticator) getClaims(token string, req *http.Request) (claims ma
 			oauth2.StaticTokenSource(oauth2Token),
 		)
 		if err != nil {
-			m.Logger.Error().Err(err).Msg("Failed to get userinfo")
-			status = http.StatusUnauthorized
-			return
+			return nil, errors.Wrap(err, "failed to get userinfo")
 		}
-
 		if err := userInfo.Claims(&claims); err != nil {
-			m.Logger.Error().Err(err).Interface("userinfo", userInfo).Msg("failed to unmarshal userinfo claims")
-			status = http.StatusInternalServerError
-			return
+			return nil, errors.Wrap(err, "failed to unmarshal userinfo claims")
 		}
 
 		expiration := m.extractExpiration(aClaims)
 		m.TokenCache.Store(token, claims, expiration)
 
 		m.Logger.Debug().Interface("claims", claims).Interface("userInfo", userInfo).Time("expiration", expiration.UTC()).Msg("unmarshalled and cached userinfo")
-		return
+		return claims, nil
 	}
 
 	var ok bool
 	if claims, ok = hit.V.(map[string]interface{}); !ok {
-		status = http.StatusInternalServerError
-		return
+		return nil, errors.New("failed to cast claims from the cache")
 	}
 	m.Logger.Debug().Interface("claims", claims).Msg("cache hit for userinfo")
-	return
+	return claims, nil
 }
 
 func (m OIDCAuthenticator) verifyAccessToken(token string) (jwt.RegisteredClaims, error) {
@@ -139,21 +142,20 @@ func (m OIDCAuthenticator) extractExpiration(aClaims jwt.RegisteredClaims) time.
 }
 
 func (m OIDCAuthenticator) shouldServe(req *http.Request) bool {
-	header := req.Header.Get("Authorization")
-
 	if m.OIDCIss == "" {
 		return false
 	}
 
 	// todo: looks dirty, check later
 	// TODO: make a PR to coreos/go-oidc for exposing userinfo endpoint on provider, see https://github.com/coreos/go-oidc/issues/248
-	for _, ignoringPath := range []string{"/konnect/v1/userinfo", "/status.php"} {
+	for _, ignoringPath := range _unauthenticatePaths {
 		if req.URL.Path == ignoringPath {
 			return false
 		}
 	}
 
-	return strings.HasPrefix(header, "Bearer ")
+	header := req.Header.Get(_headerAuthorization)
+	return strings.HasPrefix(header, _bearerPrefix)
 }
 
 type jwksJSON struct {
@@ -234,14 +236,10 @@ func (m *OIDCAuthenticator) getProvider() OIDCProvider {
 func (m OIDCAuthenticator) Authenticate(r *http.Request) (*http.Request, bool) {
 	// there is no bearer token on the request,
 	if !m.shouldServe(r) {
-		// // oidc supported but token not present, add header and handover to the next middleware.
-		// userAgentAuthenticateLockIn(w, r, options.CredentialsByUserAgent, "bearer")
-		// next.ServeHTTP(w, r)
 		return nil, false
 	}
 
 	if m.getProvider() == nil {
-		// w.WriteHeader(http.StatusInternalServerError)
 		return nil, false
 	}
 	// Force init of jwks keyfunc if needed (contacts the .well-known and jwks endpoints on first call)
@@ -249,13 +247,20 @@ func (m OIDCAuthenticator) Authenticate(r *http.Request) (*http.Request, bool) {
 		return nil, false
 	}
 
-	token := strings.TrimPrefix(r.Header.Get("Authorization"), "Bearer ")
+	token := strings.TrimPrefix(r.Header.Get(_headerAuthorization), _bearerPrefix)
 
-	claims, status := m.getClaims(token, r)
-	if status != 0 {
-		// w.WriteHeader(status)
-		// TODO log
+	claims, err := m.getClaims(token, r)
+	if err != nil {
+		m.Logger.Error().
+			Err(err).
+			Str("authenticator", "oidc").
+			Str("path", r.URL.Path).
+			Msg("failed to authenticate the request")
 		return nil, false
 	}
+	m.Logger.Debug().
+		Str("authenticator", "oidc").
+		Str("path", r.URL.Path).
+		Msg("successfully authenticated request")
 	return r.WithContext(oidc.NewContext(r.Context(), claims)), true
 }

services/proxy/pkg/middleware/public_share_auth.go

@@ -13,6 +13,9 @@ const (
 	headerShareToken        = "public-token"
 	basicAuthPasswordPrefix = "password|"
 	authenticationType      = "publicshares"
+
+	_paramSignature  = "signature"
+	_paramExpiration = "expiration"
 )
 
 type PublicShareAuthenticator struct {
@@ -38,8 +41,8 @@ func (a PublicShareAuthenticator) Authenticate(r *http.Request) (*http.Request,
 	}
 
 	var sharePassword string
-	if signature := query.Get("signature"); signature != "" {
-		expiration := query.Get("expiration")
+	if signature := query.Get(_paramSignature); signature != "" {
+		expiration := query.Get(_paramExpiration)
 		if expiration == "" {
 			a.Logger.Warn().Str("signature", signature).Msg("cannot do signature auth without the expiration")
 			return nil, false
@@ -62,11 +65,20 @@ func (a PublicShareAuthenticator) Authenticate(r *http.Request) (*http.Request,
 	})
 
 	if err != nil {
-		a.Logger.Debug().Err(err).Str("public_share_token", shareToken).Msg("could not authenticate public share")
-		// try another middleware
+		a.Logger.Error().
+			Err(err).
+			Str("authenticator", "public_share").
+			Str("public_share_token", shareToken).
+			Str("path", r.URL.Path).
+			Msg("failed to authenticate request")
 		return nil, false
 	}
 
 	r.Header.Add(headerRevaAccessToken, authResp.Token)
+
+	a.Logger.Debug().
+		Str("authenticator", "public_share").
+		Str("path", r.URL.Path).
+		Msg("successfully authenticated request")
 	return r, false
 }

services/proxy/pkg/middleware/signed_url_auth.go

@@ -20,58 +20,36 @@ import (
 	"golang.org/x/crypto/pbkdf2"
 )
 
-// SignedURLAuth provides a middleware to check access secured by a signed URL.
-func SignedURLAuth(optionSetters ...Option) func(next http.Handler) http.Handler {
-	options := newOptions(optionSetters...)
+const (
+	_paramOCSignature  = "OC-Signature"
+	_paramOCCredential = "OC-Credential"
+	_paramOCDate       = "OC-Date"
+	_paramOCExpires    = "OC-Expires"
+	_paramOCVerb       = "OC-Verb"
+)
 
-	return func(next http.Handler) http.Handler {
-		return &SignedURLAuthenticator{
-			next:               next,
-			logger:             options.Logger,
-			preSignedURLConfig: options.PreSignedURLConfig,
-			store:              options.Store,
-			userProvider:       options.UserProvider,
-		}
+var (
+	_requiredParams = [...]string{
+		_paramOCSignature,
+		_paramOCCredential,
+		_paramOCDate,
+		_paramOCExpires,
+		_paramOCVerb,
 	}
-}
+)
 
 type SignedURLAuthenticator struct {
-	next               http.Handler
-	logger             log.Logger
-	preSignedURLConfig config.PreSignedURL
-	userProvider       backend.UserBackend
-	store              storesvc.StoreService
-}
-
-func (m SignedURLAuthenticator) ServeHTTP(w http.ResponseWriter, req *http.Request) {
-	if !m.shouldServe(req) {
-		m.next.ServeHTTP(w, req)
-		return
-	}
-
-	user, _, err := m.userProvider.GetUserByClaims(req.Context(), "username", req.URL.Query().Get("OC-Credential"), true)
-	if err != nil {
-		m.logger.Error().Err(err).Msg("Could not get user by claim")
-		w.WriteHeader(http.StatusInternalServerError)
-	}
-
-	ctx := revactx.ContextSetUser(req.Context(), user)
-
-	req = req.WithContext(ctx)
-
-	if err := m.validate(req); err != nil {
-		http.Error(w, "Invalid url signature", http.StatusUnauthorized)
-		return
-	}
-
-	m.next.ServeHTTP(w, req)
+	Logger             log.Logger
+	PreSignedURLConfig config.PreSignedURL
+	UserProvider       backend.UserBackend
+	Store              storesvc.StoreService
 }
 
 func (m SignedURLAuthenticator) shouldServe(req *http.Request) bool {
-	if !m.preSignedURLConfig.Enabled {
+	if !m.PreSignedURLConfig.Enabled {
 		return false
 	}
-	return req.URL.Query().Get("OC-Signature") != ""
+	return req.URL.Query().Get(_paramOCSignature) != ""
 }
 
 func (m SignedURLAuthenticator) validate(req *http.Request) (err error) {
@@ -107,13 +85,7 @@ func (m SignedURLAuthenticator) allRequiredParametersArePresent(query url.Values
 	// OC-Date - defined the date the url was signed (ISO 8601 UTC) REQUIRED
 	// OC-Expires - defines the expiry interval in seconds (between 1 and 604800 = 7 days) REQUIRED
 	// TODO OC-Verb - defines for which http verb the request is valid - defaults to GET OPTIONAL
-	for _, p := range []string{
-		"OC-Signature",
-		"OC-Credential",
-		"OC-Date",
-		"OC-Expires",
-		"OC-Verb",
-	} {
+	for _, p := range _requiredParams {
 		if query.Get(p) == "" {
 			return false, fmt.Errorf("required %s parameter not found", p)
 		}
@@ -124,7 +96,7 @@ func (m SignedURLAuthenticator) allRequiredParametersArePresent(query url.Values
 
 func (m SignedURLAuthenticator) requestMethodMatches(meth string, query url.Values) (ok bool, err error) {
 	// check if given url query parameter OC-Verb matches given request method
-	if !strings.EqualFold(meth, query.Get("OC-Verb")) {
+	if !strings.EqualFold(meth, query.Get(_paramOCVerb)) {
 		return false, errors.New("required OC-Verb parameter did not match request method")
 	}
 
@@ -134,7 +106,7 @@ func (m SignedURLAuthenticator) requestMethodMatches(meth string, query url.Valu
 func (m SignedURLAuthenticator) requestMethodIsAllowed(meth string) (ok bool, err error) {
 	//  check if given request method is allowed
 	methodIsAllowed := false
-	for _, am := range m.preSignedURLConfig.AllowedHTTPMethods {
+	for _, am := range m.PreSignedURLConfig.AllowedHTTPMethods {
 		if strings.EqualFold(meth, am) {
 			methodIsAllowed = true
 			break
@@ -147,14 +119,15 @@ func (m SignedURLAuthenticator) requestMethodIsAllowed(meth string) (ok bool, er
 
 	return true, nil
 }
+
 func (m SignedURLAuthenticator) urlIsExpired(query url.Values, now func() time.Time) (expired bool, err error) {
 	// check if url is expired by checking if given date (OC-Date) + expires in seconds (OC-Expires) is after now
-	validFrom, err := time.Parse(time.RFC3339, query.Get("OC-Date"))
+	validFrom, err := time.Parse(time.RFC3339, query.Get(_paramOCDate))
 	if err != nil {
 		return true, err
 	}
 
-	requestExpiry, err := time.ParseDuration(query.Get("OC-Expires") + "s")
+	requestExpiry, err := time.ParseDuration(query.Get(_paramOCExpires) + "s")
 	if err != nil {
 		return true, err
 	}
@@ -168,16 +141,16 @@ func (m SignedURLAuthenticator) signatureIsValid(req *http.Request) (ok bool, er
 	u := revactx.ContextMustGetUser(req.Context())
 	signingKey, err := m.getSigningKey(req.Context(), u.Id.OpaqueId)
 	if err != nil {
-		m.logger.Error().Err(err).Msg("could not retrieve signing key")
+		m.Logger.Error().Err(err).Msg("could not retrieve signing key")
 		return false, err
 	}
 	if len(signingKey) == 0 {
-		m.logger.Error().Err(err).Msg("signing key empty")
+		m.Logger.Error().Err(err).Msg("signing key empty")
 		return false, err
 	}
 	q := req.URL.Query()
-	signature := q.Get("OC-Signature")
-	q.Del("OC-Signature")
+	signature := q.Get(_paramOCSignature)
+	q.Del(_paramOCSignature)
 	req.URL.RawQuery = q.Encode()
 	url := req.URL.String()
 	if !req.URL.IsAbs() {
@@ -198,7 +171,7 @@ func (m SignedURLAuthenticator) createSignature(url string, signingKey []byte) s
 }
 
 func (m SignedURLAuthenticator) getSigningKey(ctx context.Context, ocisID string) ([]byte, error) {
-	res, err := m.store.Read(ctx, &storesvc.ReadRequest{
+	res, err := m.Store.Read(ctx, &storesvc.ReadRequest{
 		Options: &storemsg.ReadOptions{
 			Database: "proxy",
 			Table:    "signing-keys",
@@ -206,7 +179,7 @@ func (m SignedURLAuthenticator) getSigningKey(ctx context.Context, ocisID string
 		Key: ocisID,
 	})
 	if err != nil || len(res.Records) < 1 {
-		return []byte{}, err
+		return nil, err
 	}
 
 	return res.Records[0].Value, nil
@@ -217,9 +190,13 @@ func (m SignedURLAuthenticator) Authenticate(r *http.Request) (*http.Request, bo
 		return nil, false
 	}
 
-	user, _, err := m.userProvider.GetUserByClaims(r.Context(), "username", r.URL.Query().Get("OC-Credential"), true)
+	user, _, err := m.UserProvider.GetUserByClaims(r.Context(), "username", r.URL.Query().Get(_paramOCCredential), true)
 	if err != nil {
-		m.logger.Error().Err(err).Msg("Could not get user by claim")
+		m.Logger.Error().
+			Err(err).
+			Str("authenticator", "signed_url").
+			Str("path", r.URL.Path).
+			Msg("Could not get user by claim")
 		return nil, false
 	}
 
@@ -228,9 +205,17 @@ func (m SignedURLAuthenticator) Authenticate(r *http.Request) (*http.Request, bo
 	r = r.WithContext(ctx)
 
 	if err := m.validate(r); err != nil {
-		// http.Error(w, "Invalid url signature", http.StatusUnauthorized)
+		m.Logger.Error().
+			Err(err).
+			Str("authenticator", "signed_url").
+			Str("path", r.URL.Path).
+			Msg("Could not get user by claim")
 		return nil, false
 	}
 
+	m.Logger.Debug().
+		Str("authenticator", "signed_url").
+		Str("path", r.URL.Path).
+		Msg("successfully authenticated request")
 	return r, true
 }