diff --git a/changelog/unreleased/auth-fixes.md b/changelog/unreleased/auth-fixes.md new file mode 100644 index 0000000000..8ab4594a37 --- /dev/null +++ b/changelog/unreleased/auth-fixes.md @@ -0,0 +1,6 @@ +Enhancement: improve authentication routing logic + +Provides a safer approach to route requests, both in HTTP and gRPC land +when authentication is needed. + +https://github.com/cs3org/reva/pull/4266 diff --git a/internal/grpc/interceptors/auth/auth.go b/internal/grpc/interceptors/auth/auth.go index ed42c2939e..27bb5ed447 100644 --- a/internal/grpc/interceptors/auth/auth.go +++ b/internal/grpc/interceptors/auth/auth.go @@ -98,23 +98,10 @@ func NewUnary(m map[string]interface{}, unprotected []string) (grpc.UnaryServerI if utils.Skip(info.FullMethod, unprotected) { log.Debug().Str("method", info.FullMethod).Msg("skipping auth") - - // If a token is present, set it anyway, as we might need the user info - // to decide the storage provider. - tkn, ok := appctx.ContextGetToken(ctx) - if ok { - u, scopes, err := dismantleToken(ctx, tkn, req, tokenManager, conf.GatewayAddr, true) - if err == nil { - if blockedUsers.IsBlocked(u.Username) { - return nil, status.Errorf(codes.PermissionDenied, "user %s blocked", u.Username) - } - ctx = appctx.ContextSetUser(ctx, u) - ctx = appctx.ContextSetScopes(ctx, scopes) - } - } return handler(ctx, req) } + // from this point on, the method must be authenticated tkn, ok := appctx.ContextGetToken(ctx) if !ok || tkn == "" { @@ -137,6 +124,7 @@ func NewUnary(m map[string]interface{}, unprotected []string) (grpc.UnaryServerI ctx = appctx.ContextSetScopes(ctx, scopes) return handler(ctx, req) } + return interceptor, nil } @@ -171,19 +159,6 @@ func NewStream(m map[string]interface{}, unprotected []string) (grpc.StreamServe if utils.Skip(info.FullMethod, unprotected) { log.Debug().Str("method", info.FullMethod).Msg("skipping auth") - - // If a token is present, set it anyway, as we might need the user info - // to decide the storage provider. - tkn, ok := appctx.ContextGetToken(ctx) - if ok { - u, scopes, err := dismantleToken(ctx, tkn, ss, tokenManager, conf.GatewayAddr, true) - if err == nil { - ctx = appctx.ContextSetUser(ctx, u) - ctx = appctx.ContextSetScopes(ctx, scopes) - ss = newWrappedServerStream(ctx, ss) - } - } - return handler(srv, ss) } diff --git a/internal/grpc/services/gateway/gateway.go b/internal/grpc/services/gateway/gateway.go index a85d18fdf1..7d190ef9bb 100644 --- a/internal/grpc/services/gateway/gateway.go +++ b/internal/grpc/services/gateway/gateway.go @@ -170,7 +170,42 @@ func (s *svc) Close() error { } func (s *svc) UnprotectedEndpoints() []string { - return []string{"/cs3.gateway.v1beta1.GatewayAPI"} + return []string{ + "/cs3.gateway.v1beta1.GatewayAPI/ListShare", + "/cs3.gateway.v1beta1.GatewayAPI/GetAppPassword", + "/cs3.gateway.v1beta1.GatewayAPI/AddAppProvider", + "/cs3.gateway.v1beta1.GatewayAPI/ListSupportedMimeTypes", + "/cs3.gateway.v1beta1.GatewayAPI/Authenticate", + "/cs3.gateway.v1beta1.GatewayAPI/GetAuthProvider", + "/cs3.gateway.v1beta1.GatewayAPI/ListAuthProviders", + "/cs3.gateway.v1beta1.GatewayAPI/CreateOCMCoreShare", + "/cs3.gateway.v1beta1.GatewayAPI/AcceptInvite", + "/cs3.gateway.v1beta1.GatewayAPI/GetAcceptedUser", + "/cs3.gateway.v1beta1.GatewayAPI/IsProviderAllowed", + "/cs3.gateway.v1beta1.GatewayAPI/ListAllProviders", + "/cs3.gateway.v1beta1.GatewayAPI/GetOCMShareByToken", + "/cs3.gateway.v1beta1.GatewayAPI/GetPublicShareByToken", + "/cs3.gateway.v1beta1.GatewayAPI/GetUser", + "/cs3.gateway.v1beta1.GatewayAPI/GetUserByClaim", + "/cs3.gateway.v1beta1.GatewayAPI/GetUserGroups", + + "/cs3.auth.applications.v1beta1.ApplicationsAPI/GetAppPassword", + "/cs3.app.registry.v1beta1.RegistryAPI/AddAppProvider", + "/cs3.app.registry.v1beta1.RegistryAPI/ListSupportedMimeTypes", + "/cs3.auth.provider.v1beta1.ProviderAPI/Authenticate", + "/cs3.auth.registry.v1beta1.RegistryAPI/GetAuthProvider", + "/cs3.auth.registry.v1beta1.RegistryAPI/ListAuthProviders", + "/cs3.ocm.core.v1beta1.OcmCoreAPI/CreateOCMCoreShare", + "/cs3.ocm.invite.v1beta1.InviteAPI/AcceptInvite", + "/cs3.ocm.invite.v1beta1.InviteAPI/GetAcceptedUser", + "/cs3.ocm.provider.v1beta1.ProviderAPI/IsProviderAllowed", + "/cs3.ocm.provider.v1beta1.ProviderAPI/ListAllProviders", + "/cs3.sharing.ocm.v1beta1.OcmAPI/GetOCMShareByToken", + "/cs3.sharing.link.v1beta1.LinkAPI/GetPublicShareByToken", + "/cs3.identity.user.v1beta1.UserAPI/GetUser", + "/cs3.identity.user.v1beta1.UserAPI/GetUserByClaim", + "/cs3.identity.user.v1beta1.UserAPI/GetUserGroups", + } } func getTokenManager(manager string, m map[string]map[string]interface{}) (token.Manager, error) { diff --git a/internal/http/interceptors/auth/auth.go b/internal/http/interceptors/auth/auth.go index 6062c52397..4c89466778 100644 --- a/internal/http/interceptors/auth/auth.go +++ b/internal/http/interceptors/auth/auth.go @@ -168,10 +168,8 @@ func New(m map[string]interface{}, unprotected []string) (global.Middleware, err log := appctx.GetLogger(r.Context()) - // For unprotected URLs, we try to authenticate the request in case some service needs it, - // but don't return any errors if it fails. if utils.Skip(r.URL.Path, unprotected) { - log.Info().Msg("skipping auth check for: " + r.URL.Path) + log.Info().Interface("unprotected", unprotected).Msg("skipping auth check for: " + r.URL.Path) } else { ctx, err := authenticateUser(w, r, conf, tokenStrategyChain, tokenManager, tokenWriter, credChain, false) if err != nil { diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 4c7ce12133..6e94407580 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -59,7 +59,7 @@ var ( // i.e: /a/b/c/d/e contains prefix /a/b/c. func Skip(source string, prefixes []string) bool { for i := range prefixes { - if strings.HasPrefix(source, prefixes[i]) { + if strings.HasPrefix(path.Join(source, "/"), path.Join(prefixes[i], "/")) { return true } }