diff --git a/services/proxy/pkg/command/server.go b/services/proxy/pkg/command/server.go index 9e372e4481..7377d95829 100644 --- a/services/proxy/pkg/command/server.go +++ b/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), - // ), ) } diff --git a/services/proxy/pkg/middleware/authentication.go b/services/proxy/pkg/middleware/authentication.go index 651876f35f..6c961657d1 100644 --- a/services/proxy/pkg/middleware/authentication.go +++ b/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), -// ) -// } diff --git a/services/proxy/pkg/middleware/basic_auth.go b/services/proxy/pkg/middleware/basic_auth.go index 4771e57f21..eaccd70425 100644 --- a/services/proxy/pkg/middleware/basic_auth.go +++ b/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 } diff --git a/services/proxy/pkg/middleware/oidc_auth.go b/services/proxy/pkg/middleware/oidc_auth.go index 97a71962ff..5d740843d1 100644 --- a/services/proxy/pkg/middleware/oidc_auth.go +++ b/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 } diff --git a/services/proxy/pkg/middleware/public_share_auth.go b/services/proxy/pkg/middleware/public_share_auth.go index 6d6d0fa747..795063d5d0 100644 --- a/services/proxy/pkg/middleware/public_share_auth.go +++ b/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 } diff --git a/services/proxy/pkg/middleware/signed_url_auth.go b/services/proxy/pkg/middleware/signed_url_auth.go index 094c279ad4..80a613aace 100644 --- a/services/proxy/pkg/middleware/signed_url_auth.go +++ b/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 }