From 1b5ed3c5e9b58a439dfdae153b536dda70257748 Mon Sep 17 00:00:00 2001 From: Roman Perekhod <2403905@gmail.com> Date: Tue, 24 Mar 2026 16:30:01 +0100 Subject: [PATCH 1/5] feat: Separate the storage-users and graph to handle vault storage update ListStorageProviders cache update findProvidersForFilter fix the finishUpload event restict webdav copy/move from the vault --- .../grpc/services/gateway/storageprovider.go | 17 ++++++++++++ .../services/gateway/storageprovidercache.go | 15 ++++++++--- .../storageprovider/storageprovider.go | 3 ++- internal/http/services/owncloud/ocdav/copy.go | 7 +++++ internal/http/services/owncloud/ocdav/move.go | 7 +++++ .../http/services/owncloud/ocdav/ocdav.go | 12 +++++++++ .../handlers/apps/sharing/shares/spaces.go | 16 ++--------- pkg/events/postprocessing.go | 2 ++ pkg/storage/registry/spaces/spaces.go | 27 ++++++++++++++++--- .../utils/decomposedfs/decomposedfs.go | 14 +++++++++- .../utils/decomposedfs/options/options.go | 9 +++++-- .../utils/decomposedfs/upload/upload.go | 2 +- pkg/utils/utils.go | 3 +++ 13 files changed, 107 insertions(+), 27 deletions(-) diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index 4a8d15df4f4..c45f26e3061 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -143,6 +143,12 @@ func (s *svc) CreateHome(ctx context.Context, req *provider.CreateHomeRequest) ( }, } } + + // pass storage_id to the storage provider to handle vault storage id + if storageId := utils.ReadPlainFromOpaque(req.GetOpaque(), "storage_id"); storageId != "" { + createReq.Opaque = utils.AppendPlainToOpaque(createReq.Opaque, "storage_id", storageId) + } + res, err := s.CreateStorageSpace(ctx, createReq) if err != nil { return &provider.CreateHomeResponse{ @@ -170,6 +176,11 @@ func (s *svc) CreateStorageSpace(ctx context.Context, req *provider.CreateStorag } } + if storageId := utils.ReadPlainFromOpaque(req.GetOpaque(), "storage_id"); storageId != "" { + space.Root = &provider.ResourceId{StorageId: storageId} + req.Opaque = utils.AppendPlainToOpaque(req.Opaque, "storage_id", storageId) + } + srClient, err := s.getStorageRegistryClient(ctx, s.c.StorageRegistryEndpoint) if err != nil { return &provider.CreateStorageSpaceResponse{ @@ -247,6 +258,7 @@ func (s *svc) ListStorageSpaces(ctx context.Context, req *provider.ListStorageSp filters["path"] = path } + hasFileIdFilter := false for _, f := range req.Filters { switch f.Type { case provider.ListStorageSpacesRequest_Filter_TYPE_ID: @@ -255,6 +267,7 @@ func (s *svc) ListStorageSpaces(ctx context.Context, req *provider.ListStorageSp continue } filters["storage_id"], filters["space_id"], filters["opaque_id"] = sid, spid, oid + hasFileIdFilter = true case provider.ListStorageSpacesRequest_Filter_TYPE_OWNER: filters["owner_idp"] = f.GetOwner().GetIdp() filters["owner_id"] = f.GetOwner().GetOpaqueId() @@ -270,6 +283,10 @@ func (s *svc) ListStorageSpaces(ctx context.Context, req *provider.ListStorageSp } } + if !hasFileIdFilter && utils.ReadPlainFromOpaque(req.Opaque, "storage_id") != "" { + filters["storage_id"] = utils.ReadPlainFromOpaque(req.Opaque, "storage_id") + } + c, err := s.getStorageRegistryClient(ctx, s.c.StorageRegistryEndpoint) if err != nil { return &provider.ListStorageSpacesResponse{ diff --git a/internal/grpc/services/gateway/storageprovidercache.go b/internal/grpc/services/gateway/storageprovidercache.go index ba76690e235..855a7832cfc 100644 --- a/internal/grpc/services/gateway/storageprovidercache.go +++ b/internal/grpc/services/gateway/storageprovidercache.go @@ -24,8 +24,8 @@ import ( rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" registry "github.com/cs3org/go-cs3apis/cs3/storage/registry/v1beta1" ctxpkg "github.com/owncloud/reva/v2/pkg/ctx" - sdk "github.com/owncloud/reva/v2/pkg/sdk/common" "github.com/owncloud/reva/v2/pkg/storage/cache" + "github.com/owncloud/reva/v2/pkg/storagespace" "github.com/owncloud/reva/v2/pkg/utils" "github.com/pkg/errors" "google.golang.org/grpc" @@ -41,15 +41,22 @@ type cachedRegistryClient struct { } func (c *cachedRegistryClient) ListStorageProviders(ctx context.Context, in *registry.ListStorageProvidersRequest, opts ...grpc.CallOption) (*registry.ListStorageProvidersResponse, error) { - - spaceID := sdk.DecodeOpaqueMap(in.Opaque)["space_id"] + spaceID := utils.ReadPlainFromOpaque(in.GetOpaque(), "space_id") + resourceID := spaceID + if storageID := utils.ReadPlainFromOpaque(in.GetOpaque(), "storage_id"); storageID != "" { + if spaceID != "" { + resourceID = storagespace.FormatStorageID(storageID, spaceID) + } else { + resourceID = storageID + } + } u, ok := ctxpkg.ContextGetUser(ctx) if !ok { return nil, errors.New("user not found in context") } - key := c.cache.GetKey(u.GetId(), spaceID) + key := c.cache.GetKey(u.GetId(), resourceID) if key != "" { s := ®istry.ListStorageProvidersResponse{} if err := c.cache.PullFromCache(key, s); err == nil { diff --git a/internal/grpc/services/storageprovider/storageprovider.go b/internal/grpc/services/storageprovider/storageprovider.go index d790bf2c1d5..2bf7025e9b8 100644 --- a/internal/grpc/services/storageprovider/storageprovider.go +++ b/internal/grpc/services/storageprovider/storageprovider.go @@ -33,6 +33,7 @@ import ( rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + "github.com/mitchellh/mapstructure" "github.com/owncloud/reva/v2/pkg/appctx" "github.com/owncloud/reva/v2/pkg/conversions" ctxpkg "github.com/owncloud/reva/v2/pkg/ctx" @@ -47,7 +48,6 @@ import ( "github.com/owncloud/reva/v2/pkg/storage/fs/registry" "github.com/owncloud/reva/v2/pkg/storagespace" "github.com/owncloud/reva/v2/pkg/utils" - "github.com/mitchellh/mapstructure" "github.com/pkg/errors" "github.com/rs/zerolog" "go.opentelemetry.io/otel/attribute" @@ -787,6 +787,7 @@ func (s *Service) Stat(ctx context.Context, req *provider.StatRequest) (*provide s.addMissingStorageProviderID(md.GetId(), nil) s.addMissingStorageProviderID(md.GetParentId(), nil) s.addMissingStorageProviderID(md.GetSpace().GetRoot(), nil) + s.addMissingStorageProviderID(md.GetSpace().GetRootInfo().GetId(), nil) return &provider.StatResponse{ Status: status.NewOK(ctx), diff --git a/internal/http/services/owncloud/ocdav/copy.go b/internal/http/services/owncloud/ocdav/copy.go index 307bed917f2..737e010e2cb 100644 --- a/internal/http/services/owncloud/ocdav/copy.go +++ b/internal/http/services/owncloud/ocdav/copy.go @@ -553,6 +553,13 @@ func (s *svc) executeSpacesCopy(ctx context.Context, w http.ResponseWriter, sele } func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Request, srcRef, dstRef *provider.Reference, log *zerolog.Logger, destInShareJail bool) *copy { + // restict copy from the vault + if destinationIsNotAllowed(srcRef, dstRef) { + w.WriteHeader(http.StatusConflict) + b, err := errors.Marshal(http.StatusBadRequest, "destination is not allowed", "", "") + errors.HandleWebdavError(log, w, b, err) + return nil + } isChild, err := s.referenceIsChildOf(ctx, s.gatewaySelector, dstRef, srcRef) if err != nil { switch err.(type) { diff --git a/internal/http/services/owncloud/ocdav/move.go b/internal/http/services/owncloud/ocdav/move.go index 65192cfa3a6..f9a0e30999b 100644 --- a/internal/http/services/owncloud/ocdav/move.go +++ b/internal/http/services/owncloud/ocdav/move.go @@ -141,6 +141,13 @@ func (s *svc) handleSpacesMove(w http.ResponseWriter, r *http.Request, srcSpaceI } func (s *svc) handleMove(ctx context.Context, w http.ResponseWriter, r *http.Request, src, dst *provider.Reference, log zerolog.Logger) { + // restrict move from the vault + if destinationIsNotAllowed(src, dst) { + w.WriteHeader(http.StatusConflict) + b, err := errors.Marshal(http.StatusBadRequest, "destination is not allowed", "", "") + errors.HandleWebdavError(&log, w, b, err) + return + } isChild, err := s.referenceIsChildOf(ctx, s.gatewaySelector, dst, src) if err != nil { switch err.(type) { diff --git a/internal/http/services/owncloud/ocdav/ocdav.go b/internal/http/services/owncloud/ocdav/ocdav.go index fee2dd86b01..850cccbf75c 100644 --- a/internal/http/services/owncloud/ocdav/ocdav.go +++ b/internal/http/services/owncloud/ocdav/ocdav.go @@ -359,6 +359,10 @@ func (s *svc) sspReferenceIsChildOf(ctx context.Context, selector pool.Selectabl } func (s *svc) referenceIsChildOf(ctx context.Context, selector pool.Selectable[gateway.GatewayAPIClient], child, parent *provider.Reference) (bool, error) { + if child.ResourceId.StorageId != parent.ResourceId.StorageId { + return false, nil // Not on the same storage -> not a child + } + if child.ResourceId.SpaceId != parent.ResourceId.SpaceId { return false, nil // Not on the same storage -> not a child } @@ -414,3 +418,11 @@ func isBodyEmpty(r *http.Request) bool { } return true } + +func destinationIsNotAllowed(srcRef, dstRef *provider.Reference) bool { + if srcRef.GetResourceId().GetStorageId() == utils.VaultStorageProviderID && + dstRef.GetResourceId().GetStorageId() != utils.VaultStorageProviderID { + return true + } + return false +} diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/spaces.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/spaces.go index 41c92b936c6..12093c61230 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/spaces.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/spaces.go @@ -137,8 +137,7 @@ func (h *Handler) addSpaceMember(w http.ResponseWriter, r *http.Request, info *p response.WriteOCSError(w, r, response.MetaNotFound.StatusCode, "error getting storage provider", err) return } - - providerClient, err := h.getStorageProviderClient(p) + providerClient, err := pool.GetStorageProviderServiceClient(p.Address) if err != nil { response.WriteOCSError(w, r, response.MetaNotFound.StatusCode, "error getting storage provider client", err) return @@ -244,8 +243,7 @@ func (h *Handler) removeSpaceMember(w http.ResponseWriter, r *http.Request, spac if ref.ResourceId.OpaqueId == "" { ref.ResourceId.OpaqueId = ref.ResourceId.SpaceId } - - providerClient, err := h.getStorageProviderClient(prov) + providerClient, err := pool.GetStorageProviderServiceClient(prov.Address) if err != nil { response.WriteOCSError(w, r, response.MetaNotFound.StatusCode, "error getting storage provider client", err) return @@ -290,16 +288,6 @@ func (h *Handler) removeSpaceMember(w http.ResponseWriter, r *http.Request, spac response.WriteOCSSuccess(w, r, nil) } -func (h *Handler) getStorageProviderClient(p *registry.ProviderInfo) (provider.ProviderAPIClient, error) { - c, err := pool.GetStorageProviderServiceClient(p.Address) - if err != nil { - err = errors.Wrap(err, "shares spaces: error getting a storage provider client") - return nil, err - } - - return c, nil -} - func (h *Handler) findProvider(ctx context.Context, ref *provider.Reference) (*registry.ProviderInfo, error) { c, err := pool.GetStorageRegistryClient(h.storageRegistryAddr) if err != nil { diff --git a/pkg/events/postprocessing.go b/pkg/events/postprocessing.go index f4268920a3d..64318cb9487 100644 --- a/pkg/events/postprocessing.go +++ b/pkg/events/postprocessing.go @@ -103,6 +103,7 @@ type PostprocessingStepFinished struct { UploadID string ExecutingUser *user.User Filename string + ResourceID *provider.ResourceId FinishedStep Postprocessingstep // name of the step Result interface{} // result information see VirusscanResult for example @@ -145,6 +146,7 @@ type VirusscanResult struct { type PostprocessingFinished struct { UploadID string Filename string + ResourceID *provider.ResourceId SpaceOwner *user.UserId ExecutingUser *user.User Result map[Postprocessingstep]interface{} // it is a map[step]Event diff --git a/pkg/storage/registry/spaces/spaces.go b/pkg/storage/registry/spaces/spaces.go index ac586e96d39..d2b0cbc7fb7 100644 --- a/pkg/storage/registry/spaces/spaces.go +++ b/pkg/storage/registry/spaces/spaces.go @@ -34,6 +34,7 @@ import ( providerpb "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" registrypb "github.com/cs3org/go-cs3apis/cs3/storage/registry/v1beta1" typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + "github.com/mitchellh/mapstructure" "github.com/owncloud/reva/v2/pkg/appctx" ctxpkg "github.com/owncloud/reva/v2/pkg/ctx" "github.com/owncloud/reva/v2/pkg/errtypes" @@ -44,7 +45,6 @@ import ( pkgregistry "github.com/owncloud/reva/v2/pkg/storage/registry/registry" "github.com/owncloud/reva/v2/pkg/storagespace" "github.com/owncloud/reva/v2/pkg/utils" - "github.com/mitchellh/mapstructure" "google.golang.org/grpc" ) @@ -195,6 +195,18 @@ func (r *registry) GetProvider(ctx context.Context, space *providerpb.StorageSpa if space.SpaceType != "" && spaceType != space.SpaceType { continue } + + // Filter out vault spaces if no storageId is provided + if space.GetRoot().GetStorageId() != "" { + if space.GetRoot().GetStorageId() != provider.ProviderID { + continue + } + } else { + if strings.HasPrefix(sc.MountPoint, "/vault/") { + continue + } + } + if space.Owner != nil { user := ctxpkg.ContextMustGetUser(ctx) spacePath, err = sc.SpacePath(user, space) @@ -289,7 +301,7 @@ func (r *registry) ListProviders(ctx context.Context, filters map[string]string) // return all providers return r.findAllProviders(ctx, mask), nil default: - return r.findProvidersForFilter(ctx, r.buildFilters(filters), unrestricted, mask), nil + return r.findProvidersForFilter(ctx, r.buildFilters(filters), filters["storage_id"], unrestricted, mask), nil } } @@ -340,7 +352,7 @@ func (r *registry) buildFilters(filterMap map[string]string) []*providerpb.ListS return filters } -func (r *registry) findProvidersForFilter(ctx context.Context, filters []*providerpb.ListStorageSpacesRequest_Filter, unrestricted bool, _ string) []*registrypb.ProviderInfo { +func (r *registry) findProvidersForFilter(ctx context.Context, filters []*providerpb.ListStorageSpacesRequest_Filter, storageId string, unrestricted bool, _ string) []*registrypb.ProviderInfo { var requestedSpaceType string for _, f := range filters { @@ -352,7 +364,10 @@ func (r *registry) findProvidersForFilter(ctx context.Context, filters []*provid currentUser := ctxpkg.ContextMustGetUser(ctx) providerInfos := []*registrypb.ProviderInfo{} for address, provider := range r.c.Providers { - + // skip mismatching storageproviders + if storageId != "" && storageId != provider.ProviderID { + continue + } // when a specific space type is requested we may skip this provider altogether if it is not configured for that type // we have to ignore a space type filter with +grant or +mountpoint type because they can live on any provider if requestedSpaceType != "" && !strings.HasPrefix(requestedSpaceType, "+") { @@ -385,6 +400,10 @@ func (r *registry) findProvidersForFilter(ctx context.Context, filters []*provid if sc, ok = provider.Spaces[space.SpaceType]; !ok { continue } + // Filter out vault spaces if no storageId is provided + if storageId == "" && strings.HasPrefix(sc.MountPoint, "/vault/") { + continue + } spacePath, err = sc.SpacePath(currentUser, space) if err != nil { appctx.GetLogger(ctx).Error().Err(err).Interface("provider", provider).Interface("space", space).Msg("failed to execute template, continuing") diff --git a/pkg/storage/utils/decomposedfs/decomposedfs.go b/pkg/storage/utils/decomposedfs/decomposedfs.go index 79dcc454a76..c4c4fd1e08f 100644 --- a/pkg/storage/utils/decomposedfs/decomposedfs.go +++ b/pkg/storage/utils/decomposedfs/decomposedfs.go @@ -258,7 +258,7 @@ func New(o *options.Options, aspects aspects.Aspects, log *zerolog.Logger) (stor return nil, errors.New("need nats for async file processing") } - ch, err := events.Consume(fs.stream, "dcfs", _registeredEvents...) + ch, err := events.Consume(fs.stream, o.Events.ConsumerGroup, _registeredEvents...) if err != nil { return nil, err } @@ -285,6 +285,10 @@ func (fs *Decomposedfs) Postprocessing(ch <-chan events.Event) { switch ev := event.Event.(type) { case events.PostprocessingFinished: sublog := log.With().Str("event", "PostprocessingFinished").Str("uploadid", ev.UploadID).Logger() + if ev.ResourceID != nil && ev.ResourceID.GetStorageId() != "" && ev.ResourceID.GetStorageId() != fs.o.MountID { + sublog.Debug().Msg("ignoring event for different storage") + continue + } session, err := fs.sessionStore.Get(ctx, ev.UploadID) if err != nil { sublog.Error().Err(err).Msg("Failed to get upload") @@ -450,6 +454,10 @@ func (fs *Decomposedfs) Postprocessing(ch <-chan events.Event) { session.Cleanup(true, !ev.KeepUpload, !ev.KeepUpload, true) case events.RevertRevision: sublog := log.With().Str("event", "RevertRevision").Interface("nodeid", ev.ResourceID).Logger() + if ev.ResourceID != nil && ev.ResourceID.GetStorageId() != "" && ev.ResourceID.GetStorageId() != fs.o.MountID { + sublog.Debug().Msg("ignoring event for different storage") + continue + } n, err := fs.lu.NodeFromID(ctx, ev.ResourceID) if err != nil { sublog.Error().Err(err).Msg("Failed to get node") @@ -462,6 +470,10 @@ func (fs *Decomposedfs) Postprocessing(ch <-chan events.Event) { } case events.PostprocessingStepFinished: sublog := log.With().Str("event", "PostprocessingStepFinished").Str("uploadid", ev.UploadID).Logger() + if ev.ResourceID != nil && ev.ResourceID.GetStorageId() != "" && ev.ResourceID.GetStorageId() != fs.o.MountID { + sublog.Debug().Msg("ignoring event for different storage") + continue + } if ev.FinishedStep != events.PPStepAntivirus { // atm we are only interested in antivirus results continue diff --git a/pkg/storage/utils/decomposedfs/options/options.go b/pkg/storage/utils/decomposedfs/options/options.go index 5c76a383eac..210f2068130 100644 --- a/pkg/storage/utils/decomposedfs/options/options.go +++ b/pkg/storage/utils/decomposedfs/options/options.go @@ -23,10 +23,10 @@ import ( "strings" "time" + "github.com/mitchellh/mapstructure" "github.com/owncloud/reva/v2/pkg/rgrpc/todo/pool" "github.com/owncloud/reva/v2/pkg/sharedconf" "github.com/owncloud/reva/v2/pkg/storage/cache" - "github.com/mitchellh/mapstructure" "github.com/pkg/errors" ) @@ -103,7 +103,8 @@ type AsyncPropagatorOptions struct { // EventOptions are the configurable options for events type EventOptions struct { - NumConsumers int `mapstructure:"numconsumers"` + NumConsumers int `mapstructure:"numconsumers"` + ConsumerGroup string `mapstructure:"consumer_group"` } // TokenOptions are the configurable option for tokens @@ -172,5 +173,9 @@ func New(m map[string]interface{}) (*Options, error) { o.UploadDirectory = filepath.Join(o.Root, "uploads") } + if o.Events.ConsumerGroup == "" { + o.Events.ConsumerGroup = "dcfs" + } + return o, nil } diff --git a/pkg/storage/utils/decomposedfs/upload/upload.go b/pkg/storage/utils/decomposedfs/upload/upload.go index 97b506d855c..6e91c8a409f 100644 --- a/pkg/storage/utils/decomposedfs/upload/upload.go +++ b/pkg/storage/utils/decomposedfs/upload/upload.go @@ -224,7 +224,7 @@ func (session *OcisSession) FinishUploadDecomposed(ctx context.Context) error { URL: s, SpaceOwner: n.SpaceOwnerOrManager(session.Context(ctx)), ExecutingUser: u, - ResourceID: &provider.ResourceId{SpaceId: n.SpaceID, OpaqueId: n.ID}, + ResourceID: &provider.ResourceId{StorageId: session.ProviderID(), SpaceId: n.SpaceID, OpaqueId: n.ID}, Filename: session.Filename(), Filesize: uint64(session.Size()), ImpersonatingUser: iu, diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index c1031368743..c562636e8b0 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -64,6 +64,9 @@ var ( // OCMStorageSpaceID is the space id used by the ocmreceived storageprovider OCMStorageSpaceID = "89f37a33-858b-45fa-8890-a1f2b27d90e1" + // VaultStorageProviderID is the storage id used by the vault storageprovider + VaultStorageProviderID = "1a01c2c4-4309-4483-a845-842fd56d8622" + // SpaceGrant is used to signal the storageprovider that the grant is on a space SpaceGrant struct{} ) From be87e5bbeba932baee3743cade6981aa5f2b71b3 Mon Sep 17 00:00:00 2001 From: Roman Perekhod <2403905@gmail.com> Date: Tue, 14 Apr 2026 18:33:38 +0200 Subject: [PATCH 2/5] feat: provide the mfa to webdav and grpc --- internal/grpc/interceptors/auth/auth.go | 22 ++++- internal/grpc/interceptors/loader/loader.go | 1 + internal/grpc/interceptors/mfa/mfa.go | 102 ++++++++++++++++++++ internal/grpc/interceptors/token/token.go | 14 +++ internal/http/interceptors/auth/auth.go | 9 ++ internal/http/services/archiver/handler.go | 10 +- pkg/auth/manager/oidc/oidc.go | 14 ++- pkg/ctx/mfactx.go | 21 ++++ pkg/ctx/userctx.go | 1 + 9 files changed, 190 insertions(+), 4 deletions(-) create mode 100644 internal/grpc/interceptors/mfa/mfa.go create mode 100644 pkg/ctx/mfactx.go diff --git a/internal/grpc/interceptors/auth/auth.go b/internal/grpc/interceptors/auth/auth.go index b3ff6171481..4a2a63d1c0d 100644 --- a/internal/grpc/interceptors/auth/auth.go +++ b/internal/grpc/interceptors/auth/auth.go @@ -27,6 +27,7 @@ import ( authpb "github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1" gatewayv1beta1 "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + "github.com/mitchellh/mapstructure" "github.com/owncloud/reva/v2/pkg/appctx" "github.com/owncloud/reva/v2/pkg/auth/scope" ctxpkg "github.com/owncloud/reva/v2/pkg/ctx" @@ -36,12 +37,12 @@ import ( "github.com/owncloud/reva/v2/pkg/token" tokenmgr "github.com/owncloud/reva/v2/pkg/token/manager/registry" "github.com/owncloud/reva/v2/pkg/utils" - "github.com/mitchellh/mapstructure" "github.com/pkg/errors" semconv "go.opentelemetry.io/otel/semconv/v1.20.0" "go.opentelemetry.io/otel/trace" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" "google.golang.org/grpc/status" ) @@ -154,6 +155,7 @@ func NewUnary(m map[string]interface{}, unprotected []string, tp trace.TracerPro // store user and scopes in context ctx = ctxpkg.ContextSetUser(ctx, u) ctx = ctxpkg.ContextSetScopes(ctx, tokenScope) + ctx = grantMFAForServiceAccount(ctx, u) span.SetAttributes(semconv.EnduserIDKey.String(u.Id.OpaqueId)) @@ -243,6 +245,7 @@ func NewStream(m map[string]interface{}, unprotected []string, tp trace.TracerPr // store user and scopes in context ctx = ctxpkg.ContextSetUser(ctx, u) ctx = ctxpkg.ContextSetScopes(ctx, tokenScope) + ctx = grantMFAForServiceAccount(ctx, u) wrapped := newWrappedServerStream(ctx, ss) span.SetAttributes(semconv.EnduserIDKey.String(u.Id.OpaqueId)) @@ -300,6 +303,23 @@ func dismantleToken(ctx context.Context, tkn string, req interface{}, mgr token. return u, tokenScope, nil } +// grantMFAForServiceAccount automatically sets MFA=true for service accounts. +// Service accounts are trusted internal processes that never authenticate via OIDC and +// therefore never carry an acr/MFA claim. Granting them implicit MFA allows +// them to access MFA-gated resources such as vault storage without +// compromising the MFA requirement for regular users. +func grantMFAForServiceAccount(ctx context.Context, u *userpb.User) context.Context { + if u.GetId().GetType() != userpb.UserType_USER_TYPE_SERVICE { + return ctx + } + if _, alreadySet := ctxpkg.ContextGetMFA(ctx); alreadySet { + return ctx + } + ctx = ctxpkg.ContextSetMFA(ctx, true) + ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.MFAHeader, "true") + return ctx +} + func getUserGroups(ctx context.Context, u *userpb.User, client gatewayv1beta1.GatewayAPIClient) ([]string, error) { if groupsIf, err := userGroupsCache.Get(u.Id.OpaqueId); err == nil { log := appctx.GetLogger(ctx) diff --git a/internal/grpc/interceptors/loader/loader.go b/internal/grpc/interceptors/loader/loader.go index c5707555d2b..98b94fd5c81 100644 --- a/internal/grpc/interceptors/loader/loader.go +++ b/internal/grpc/interceptors/loader/loader.go @@ -21,6 +21,7 @@ package loader import ( // Load core GRPC services _ "github.com/owncloud/reva/v2/internal/grpc/interceptors/eventsmiddleware" + _ "github.com/owncloud/reva/v2/internal/grpc/interceptors/mfa" _ "github.com/owncloud/reva/v2/internal/grpc/interceptors/prometheus" _ "github.com/owncloud/reva/v2/internal/grpc/interceptors/readonly" // Add your own service here diff --git a/internal/grpc/interceptors/mfa/mfa.go b/internal/grpc/interceptors/mfa/mfa.go new file mode 100644 index 00000000000..e43161de060 --- /dev/null +++ b/internal/grpc/interceptors/mfa/mfa.go @@ -0,0 +1,102 @@ +package mfa + +import ( + "context" + + userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/owncloud/reva/v2/pkg/appctx" + ctxpkg "github.com/owncloud/reva/v2/pkg/ctx" + "github.com/owncloud/reva/v2/pkg/rgrpc" + rstatus "github.com/owncloud/reva/v2/pkg/rgrpc/status" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + grpcstatus "google.golang.org/grpc/status" +) + +const ( + // defaultPriority places mfa before readonly (200) so the MFA check + // runs first and returns a clear PermissionDenied rather than a readonly error. + defaultPriority = 150 +) + +func init() { + rgrpc.RegisterUnaryInterceptor("mfa", NewUnary) +} + +// NewUnary returns a new unary interceptor that requires MFA to be satisfied +// for every gRPC call on the vault storage provider. +// Service accounts (UserType_USER_TYPE_SERVICE) are exempt because they are +// used for internal operations (postprocessing, event handling, etc.) that +// never carry an MFA claim. +func NewUnary(map[string]interface{}) (grpc.UnaryServerInterceptor, int, error) { + return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) { + log := appctx.GetLogger(ctx) + + // Bypass for service accounts — they perform internal operations and + // never carry an MFA claim. + if u, ok := ctxpkg.ContextGetUser(ctx); ok { + if u.GetId().GetType() == userpb.UserType_USER_TYPE_SERVICE { + return handler(ctx, req) + } + } + + hasMFA, _ := ctxpkg.ContextGetMFA(ctx) + if hasMFA { + return handler(ctx, req) + } + + log.Warn().Str("method", info.FullMethod).Msg("mfa: access denied, MFA required") + + const msg = "MFA required to access vault storage" + switch req.(type) { + case *provider.StatRequest: + return &provider.StatResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.ListContainerRequest: + return &provider.ListContainerResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.GetPathRequest: + return &provider.GetPathResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.GetQuotaRequest: + return &provider.GetQuotaResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.InitiateFileDownloadRequest: + return &provider.InitiateFileDownloadResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.InitiateFileUploadRequest: + return &provider.InitiateFileUploadResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.CreateContainerRequest: + return &provider.CreateContainerResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.TouchFileRequest: + return &provider.TouchFileResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.DeleteRequest: + return &provider.DeleteResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.MoveRequest: + return &provider.MoveResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.CreateHomeRequest: + return &provider.CreateHomeResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.AddGrantRequest: + return &provider.AddGrantResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.RemoveGrantRequest: + return &provider.RemoveGrantResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.UpdateGrantRequest: + return &provider.UpdateGrantResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.ListGrantsRequest: + return &provider.ListGrantsResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.ListFileVersionsRequest: + return &provider.ListFileVersionsResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.RestoreFileVersionRequest: + return &provider.RestoreFileVersionResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.ListRecycleRequest: + return &provider.ListRecycleResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.RestoreRecycleItemRequest: + return &provider.RestoreRecycleItemResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.PurgeRecycleRequest: + return &provider.PurgeRecycleResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.SetArbitraryMetadataRequest: + return &provider.SetArbitraryMetadataResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.UnsetArbitraryMetadataRequest: + return &provider.UnsetArbitraryMetadataResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + default: + log.Debug().Str("method", info.FullMethod).Msg("mfa: blocking unknown request type") + return nil, grpcstatus.Errorf(codes.PermissionDenied, "mfa: %s: %T", msg, req) + } + }, defaultPriority, nil +} diff --git a/internal/grpc/interceptors/token/token.go b/internal/grpc/interceptors/token/token.go index 2dcd4b66639..a00cc87db12 100644 --- a/internal/grpc/interceptors/token/token.go +++ b/internal/grpc/interceptors/token/token.go @@ -47,6 +47,13 @@ func NewUnary() grpc.UnaryServerInterceptor { ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.InitiatorHeader, initiatorID) } } + + if val, ok := md[ctxpkg.MFAHeader]; ok { + if len(val) > 0 && val[0] != "" { + ctx = ctxpkg.ContextSetMFA(ctx, val[0] == "true") + ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.MFAHeader, val[0]) + } + } } return handler(ctx, req) @@ -77,6 +84,13 @@ func NewStream() grpc.StreamServerInterceptor { ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.InitiatorHeader, initiatorID) } } + + if val, ok := md[ctxpkg.MFAHeader]; ok { + if len(val) > 0 && val[0] != "" { + ctx = ctxpkg.ContextSetMFA(ctx, val[0] == "true") + ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.MFAHeader, val[0]) + } + } } wrapped := newWrappedServerStream(ctx, ss) diff --git a/internal/http/interceptors/auth/auth.go b/internal/http/interceptors/auth/auth.go index f7b494c684d..f7cce842835 100644 --- a/internal/http/interceptors/auth/auth.go +++ b/internal/http/interceptors/auth/auth.go @@ -354,6 +354,15 @@ func ctxWithUserInfo(ctx context.Context, r *http.Request, user *userpb.User, to ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.UserAgentHeader, r.UserAgent()) ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.InitiatorHeader, initiatorid) ctx = ctxpkg.ContextSetScopes(ctx, tokenScope) + + // Forward MFA status from the proxy's HTTP header to outgoing gRPC metadata. + // The proxy MultiFactor middleware always sets X-Multi-Factor-Authentication: + // "true" when MFA is disabled globally, or the real outcome when enabled. + if mfaVal := r.Header.Get("X-Multi-Factor-Authentication"); mfaVal != "" { + ctx = ctxpkg.ContextSetMFA(ctx, mfaVal == "true") + ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.MFAHeader, mfaVal) + } + return ctx } diff --git a/internal/http/services/archiver/handler.go b/internal/http/services/archiver/handler.go index c2e2dfdc73a..7cd3d2b31ff 100644 --- a/internal/http/services/archiver/handler.go +++ b/internal/http/services/archiver/handler.go @@ -23,6 +23,7 @@ import ( "errors" "fmt" "net/http" + "strings" "time" "regexp" @@ -204,8 +205,15 @@ func (s *svc) writeHTTPError(rw http.ResponseWriter, err error) { s.log.Error().Msg(err.Error()) switch err.(type) { - case errtypes.NotFound, errtypes.PermissionDenied: + case errtypes.NotFound: rw.WriteHeader(http.StatusNotFound) + case errtypes.PermissionDenied: + if strings.Contains(err.Error(), "MFA required") { + rw.Header().Set("X-Ocis-Mfa-Required", "true") + rw.WriteHeader(http.StatusForbidden) + } else { + rw.WriteHeader(http.StatusNotFound) + } case manager.ErrMaxSize, manager.ErrMaxFileCount: rw.WriteHeader(http.StatusRequestEntityTooLarge) case errtypes.BadRequest: diff --git a/pkg/auth/manager/oidc/oidc.go b/pkg/auth/manager/oidc/oidc.go index bbe950dc191..f716e1c86e7 100644 --- a/pkg/auth/manager/oidc/oidc.go +++ b/pkg/auth/manager/oidc/oidc.go @@ -32,6 +32,8 @@ import ( authpb "github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1" user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" + "github.com/juliangruber/go-intersect" + "github.com/mitchellh/mapstructure" "github.com/owncloud/reva/v2/pkg/appctx" "github.com/owncloud/reva/v2/pkg/auth" "github.com/owncloud/reva/v2/pkg/auth/manager/registry" @@ -41,8 +43,7 @@ import ( "github.com/owncloud/reva/v2/pkg/rgrpc/todo/pool" "github.com/owncloud/reva/v2/pkg/rhttp" "github.com/owncloud/reva/v2/pkg/sharedconf" - "github.com/juliangruber/go-intersect" - "github.com/mitchellh/mapstructure" + "github.com/owncloud/reva/v2/pkg/utils" "github.com/pkg/errors" "golang.org/x/oauth2" ) @@ -241,6 +242,15 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string) GidNumber: claims[am.c.GIDClaim].(int64), } + // Embed the OIDC acr (Authentication Context Class Reference) claim into + // User.Opaque so it is cryptographically bound to the minted reva JWT. + // Downstream services (e.g. the mfa gRPC interceptor) can use this + // as a secondary MFA proof independent of the X-Multi-Factor-Authentication + // HTTP header propagation path. + if acr, ok := claims["acr"].(string); ok && acr != "" { + u.Opaque = utils.AppendPlainToOpaque(u.Opaque, "acr", acr) + } + var scopes map[string]*authpb.Scope if userID != nil && (userID.Type == user.UserType_USER_TYPE_LIGHTWEIGHT || userID.Type == user.UserType_USER_TYPE_FEDERATED) { scopes, err = scope.AddLightweightAccountScope(authpb.Role_ROLE_OWNER, nil) diff --git a/pkg/ctx/mfactx.go b/pkg/ctx/mfactx.go new file mode 100644 index 00000000000..cb2d4024e68 --- /dev/null +++ b/pkg/ctx/mfactx.go @@ -0,0 +1,21 @@ +package ctx + +import "context" + +// MFAHeader is the gRPC metadata key used to propagate MFA status across +// service boundaries. Lowercased to satisfy gRPC metadata requirements. +// The corresponding HTTP header set by the proxy is "X-Multi-Factor-Authentication". +const MFAHeader = "x-mfa-authenticated" + +// ContextGetMFA returns the MFA status stored in the context, and whether it +// was set at all. A missing value (second return = false) should be treated as +// MFA not satisfied. +func ContextGetMFA(ctx context.Context) (bool, bool) { + v, ok := ctx.Value(mfaKey).(bool) + return v, ok +} + +// ContextSetMFA stores the MFA status in the context. +func ContextSetMFA(ctx context.Context, mfa bool) context.Context { + return context.WithValue(ctx, mfaKey, mfa) +} diff --git a/pkg/ctx/userctx.go b/pkg/ctx/userctx.go index bb0c8076757..c32e96910a7 100644 --- a/pkg/ctx/userctx.go +++ b/pkg/ctx/userctx.go @@ -33,6 +33,7 @@ const ( lockIDKey scopeKey initiatorKey + mfaKey ) // ContextGetUser returns the user if set in the given context. From 3f827ba4fe4e057d4c5b9af50d228e4cdc9159d5 Mon Sep 17 00:00:00 2001 From: Roman Perekhod <2403905@gmail.com> Date: Thu, 16 Apr 2026 11:06:36 +0200 Subject: [PATCH 3/5] feat: refactoring: provide the mfa to webdav and grpc --- internal/grpc/interceptors/auth/auth.go | 24 +++-- internal/grpc/interceptors/auth/mfa.go | 65 +++++++++++++ internal/grpc/interceptors/loader/loader.go | 1 - internal/grpc/interceptors/mfa/mfa.go | 102 -------------------- internal/grpc/interceptors/token/token.go | 14 --- internal/http/interceptors/auth/auth.go | 11 +-- pkg/ctx/mfactx.go | 24 ++--- pkg/ctx/userctx.go | 1 - 8 files changed, 94 insertions(+), 148 deletions(-) create mode 100644 internal/grpc/interceptors/auth/mfa.go delete mode 100644 internal/grpc/interceptors/mfa/mfa.go diff --git a/internal/grpc/interceptors/auth/auth.go b/internal/grpc/interceptors/auth/auth.go index 4a2a63d1c0d..db5ea3c5b1e 100644 --- a/internal/grpc/interceptors/auth/auth.go +++ b/internal/grpc/interceptors/auth/auth.go @@ -20,6 +20,7 @@ package auth import ( "context" + "slices" "sync" "time" @@ -63,6 +64,7 @@ type config struct { GatewayAddr string `mapstructure:"gateway_addr"` UserGroupsCacheSize int `mapstructure:"usergroups_cache_size"` ScopeExpansionCacheSize int `mapstructure:"scope_expansion_cache_size"` + MFAEnabled bool `mapstructure:"mfa_enabled"` } func parseConfig(m map[string]interface{}) (*config, error) { @@ -156,6 +158,12 @@ func NewUnary(m map[string]interface{}, unprotected []string, tp trace.TracerPro ctx = ctxpkg.ContextSetUser(ctx, u) ctx = ctxpkg.ContextSetScopes(ctx, tokenScope) ctx = grantMFAForServiceAccount(ctx, u) + if conf.MFAEnabled { + if mfav := metadata.ValueFromIncomingContext(ctx, ctxpkg.MFAOutgoingHeader); !slices.Contains(mfav, "true") { + log.Warn().Err(err).Msg("access token is invalid: MFA is required") + return mfaResponse(ctx, req, info) + } + } span.SetAttributes(semconv.EnduserIDKey.String(u.Id.OpaqueId)) @@ -246,6 +254,12 @@ func NewStream(m map[string]interface{}, unprotected []string, tp trace.TracerPr ctx = ctxpkg.ContextSetUser(ctx, u) ctx = ctxpkg.ContextSetScopes(ctx, tokenScope) ctx = grantMFAForServiceAccount(ctx, u) + if conf.MFAEnabled { + if mfav := metadata.ValueFromIncomingContext(ctx, ctxpkg.MFAOutgoingHeader); !slices.Contains(mfav, "true") { + log.Warn().Err(err).Msg("access token is invalid: MFA is required") + return status.Errorf(codes.PermissionDenied, "MFA required to access vault storage") + } + } wrapped := newWrappedServerStream(ctx, ss) span.SetAttributes(semconv.EnduserIDKey.String(u.Id.OpaqueId)) @@ -303,7 +317,8 @@ func dismantleToken(ctx context.Context, tkn string, req interface{}, mgr token. return u, tokenScope, nil } -// grantMFAForServiceAccount automatically sets MFA=true for service accounts. +// grantMFAForServiceAccount automatically grants MFA for service accounts by +// injecting the autoprop-prefixed metadata key into outgoing context. // Service accounts are trusted internal processes that never authenticate via OIDC and // therefore never carry an acr/MFA claim. Granting them implicit MFA allows // them to access MFA-gated resources such as vault storage without @@ -312,12 +327,7 @@ func grantMFAForServiceAccount(ctx context.Context, u *userpb.User) context.Cont if u.GetId().GetType() != userpb.UserType_USER_TYPE_SERVICE { return ctx } - if _, alreadySet := ctxpkg.ContextGetMFA(ctx); alreadySet { - return ctx - } - ctx = ctxpkg.ContextSetMFA(ctx, true) - ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.MFAHeader, "true") - return ctx + return metadata.AppendToOutgoingContext(ctx, ctxpkg.MFAOutgoingHeader, "true") } func getUserGroups(ctx context.Context, u *userpb.User, client gatewayv1beta1.GatewayAPIClient) ([]string, error) { diff --git a/internal/grpc/interceptors/auth/mfa.go b/internal/grpc/interceptors/auth/mfa.go new file mode 100644 index 00000000000..513ed50d1bd --- /dev/null +++ b/internal/grpc/interceptors/auth/mfa.go @@ -0,0 +1,65 @@ +package auth + +import ( + "context" + + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + rstatus "github.com/owncloud/reva/v2/pkg/rgrpc/status" + "github.com/rs/zerolog/log" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + grpcstatus "google.golang.org/grpc/status" +) + +func mfaResponse(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo) (interface{}, error) { + const msg = "MFA required to access vault storage" + switch req.(type) { + case *provider.StatRequest: + return &provider.StatResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.ListContainerRequest: + return &provider.ListContainerResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.GetPathRequest: + return &provider.GetPathResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.GetQuotaRequest: + return &provider.GetQuotaResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.InitiateFileDownloadRequest: + return &provider.InitiateFileDownloadResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.InitiateFileUploadRequest: + return &provider.InitiateFileUploadResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.CreateContainerRequest: + return &provider.CreateContainerResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.TouchFileRequest: + return &provider.TouchFileResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.DeleteRequest: + return &provider.DeleteResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.MoveRequest: + return &provider.MoveResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.CreateHomeRequest: + return &provider.CreateHomeResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.AddGrantRequest: + return &provider.AddGrantResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.RemoveGrantRequest: + return &provider.RemoveGrantResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.UpdateGrantRequest: + return &provider.UpdateGrantResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.ListGrantsRequest: + return &provider.ListGrantsResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.ListFileVersionsRequest: + return &provider.ListFileVersionsResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.RestoreFileVersionRequest: + return &provider.RestoreFileVersionResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.ListRecycleRequest: + return &provider.ListRecycleResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.RestoreRecycleItemRequest: + return &provider.RestoreRecycleItemResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.PurgeRecycleRequest: + return &provider.PurgeRecycleResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.SetArbitraryMetadataRequest: + return &provider.SetArbitraryMetadataResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.UnsetArbitraryMetadataRequest: + return &provider.UnsetArbitraryMetadataResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + default: + log.Debug().Str("method", info.FullMethod).Msg("mfa: blocking unknown request type") + return nil, grpcstatus.Errorf(codes.PermissionDenied, "mfa: %s: %T", msg, req) + } +} diff --git a/internal/grpc/interceptors/loader/loader.go b/internal/grpc/interceptors/loader/loader.go index 98b94fd5c81..c5707555d2b 100644 --- a/internal/grpc/interceptors/loader/loader.go +++ b/internal/grpc/interceptors/loader/loader.go @@ -21,7 +21,6 @@ package loader import ( // Load core GRPC services _ "github.com/owncloud/reva/v2/internal/grpc/interceptors/eventsmiddleware" - _ "github.com/owncloud/reva/v2/internal/grpc/interceptors/mfa" _ "github.com/owncloud/reva/v2/internal/grpc/interceptors/prometheus" _ "github.com/owncloud/reva/v2/internal/grpc/interceptors/readonly" // Add your own service here diff --git a/internal/grpc/interceptors/mfa/mfa.go b/internal/grpc/interceptors/mfa/mfa.go deleted file mode 100644 index e43161de060..00000000000 --- a/internal/grpc/interceptors/mfa/mfa.go +++ /dev/null @@ -1,102 +0,0 @@ -package mfa - -import ( - "context" - - userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" - provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" - "github.com/owncloud/reva/v2/pkg/appctx" - ctxpkg "github.com/owncloud/reva/v2/pkg/ctx" - "github.com/owncloud/reva/v2/pkg/rgrpc" - rstatus "github.com/owncloud/reva/v2/pkg/rgrpc/status" - "google.golang.org/grpc" - "google.golang.org/grpc/codes" - grpcstatus "google.golang.org/grpc/status" -) - -const ( - // defaultPriority places mfa before readonly (200) so the MFA check - // runs first and returns a clear PermissionDenied rather than a readonly error. - defaultPriority = 150 -) - -func init() { - rgrpc.RegisterUnaryInterceptor("mfa", NewUnary) -} - -// NewUnary returns a new unary interceptor that requires MFA to be satisfied -// for every gRPC call on the vault storage provider. -// Service accounts (UserType_USER_TYPE_SERVICE) are exempt because they are -// used for internal operations (postprocessing, event handling, etc.) that -// never carry an MFA claim. -func NewUnary(map[string]interface{}) (grpc.UnaryServerInterceptor, int, error) { - return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) { - log := appctx.GetLogger(ctx) - - // Bypass for service accounts — they perform internal operations and - // never carry an MFA claim. - if u, ok := ctxpkg.ContextGetUser(ctx); ok { - if u.GetId().GetType() == userpb.UserType_USER_TYPE_SERVICE { - return handler(ctx, req) - } - } - - hasMFA, _ := ctxpkg.ContextGetMFA(ctx) - if hasMFA { - return handler(ctx, req) - } - - log.Warn().Str("method", info.FullMethod).Msg("mfa: access denied, MFA required") - - const msg = "MFA required to access vault storage" - switch req.(type) { - case *provider.StatRequest: - return &provider.StatResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.ListContainerRequest: - return &provider.ListContainerResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.GetPathRequest: - return &provider.GetPathResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.GetQuotaRequest: - return &provider.GetQuotaResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.InitiateFileDownloadRequest: - return &provider.InitiateFileDownloadResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.InitiateFileUploadRequest: - return &provider.InitiateFileUploadResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.CreateContainerRequest: - return &provider.CreateContainerResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.TouchFileRequest: - return &provider.TouchFileResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.DeleteRequest: - return &provider.DeleteResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.MoveRequest: - return &provider.MoveResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.CreateHomeRequest: - return &provider.CreateHomeResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.AddGrantRequest: - return &provider.AddGrantResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.RemoveGrantRequest: - return &provider.RemoveGrantResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.UpdateGrantRequest: - return &provider.UpdateGrantResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.ListGrantsRequest: - return &provider.ListGrantsResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.ListFileVersionsRequest: - return &provider.ListFileVersionsResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.RestoreFileVersionRequest: - return &provider.RestoreFileVersionResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.ListRecycleRequest: - return &provider.ListRecycleResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.RestoreRecycleItemRequest: - return &provider.RestoreRecycleItemResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.PurgeRecycleRequest: - return &provider.PurgeRecycleResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.SetArbitraryMetadataRequest: - return &provider.SetArbitraryMetadataResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - case *provider.UnsetArbitraryMetadataRequest: - return &provider.UnsetArbitraryMetadataResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil - default: - log.Debug().Str("method", info.FullMethod).Msg("mfa: blocking unknown request type") - return nil, grpcstatus.Errorf(codes.PermissionDenied, "mfa: %s: %T", msg, req) - } - }, defaultPriority, nil -} diff --git a/internal/grpc/interceptors/token/token.go b/internal/grpc/interceptors/token/token.go index a00cc87db12..2dcd4b66639 100644 --- a/internal/grpc/interceptors/token/token.go +++ b/internal/grpc/interceptors/token/token.go @@ -47,13 +47,6 @@ func NewUnary() grpc.UnaryServerInterceptor { ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.InitiatorHeader, initiatorID) } } - - if val, ok := md[ctxpkg.MFAHeader]; ok { - if len(val) > 0 && val[0] != "" { - ctx = ctxpkg.ContextSetMFA(ctx, val[0] == "true") - ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.MFAHeader, val[0]) - } - } } return handler(ctx, req) @@ -84,13 +77,6 @@ func NewStream() grpc.StreamServerInterceptor { ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.InitiatorHeader, initiatorID) } } - - if val, ok := md[ctxpkg.MFAHeader]; ok { - if len(val) > 0 && val[0] != "" { - ctx = ctxpkg.ContextSetMFA(ctx, val[0] == "true") - ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.MFAHeader, val[0]) - } - } } wrapped := newWrappedServerStream(ctx, ss) diff --git a/internal/http/interceptors/auth/auth.go b/internal/http/interceptors/auth/auth.go index f7cce842835..ebfbdf02ccf 100644 --- a/internal/http/interceptors/auth/auth.go +++ b/internal/http/interceptors/auth/auth.go @@ -355,12 +355,11 @@ func ctxWithUserInfo(ctx context.Context, r *http.Request, user *userpb.User, to ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.InitiatorHeader, initiatorid) ctx = ctxpkg.ContextSetScopes(ctx, tokenScope) - // Forward MFA status from the proxy's HTTP header to outgoing gRPC metadata. - // The proxy MultiFactor middleware always sets X-Multi-Factor-Authentication: - // "true" when MFA is disabled globally, or the real outcome when enabled. - if mfaVal := r.Header.Get("X-Multi-Factor-Authentication"); mfaVal != "" { - ctx = ctxpkg.ContextSetMFA(ctx, mfaVal == "true") - ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.MFAHeader, mfaVal) + // Forward MFA status from the proxy's HTTP header into outgoing gRPC metadata. + // Using the autoprop-prefixed key causes the metadata interceptor to propagate + // it automatically at every subsequent gRPC hop. + if mfaVal := r.Header.Get(ctxpkg.MFAHeader); mfaVal != "" { + ctx = metadata.AppendToOutgoingContext(ctx, ctxpkg.MFAOutgoingHeader, mfaVal) } return ctx diff --git a/pkg/ctx/mfactx.go b/pkg/ctx/mfactx.go index cb2d4024e68..39b94b6f772 100644 --- a/pkg/ctx/mfactx.go +++ b/pkg/ctx/mfactx.go @@ -1,21 +1,11 @@ package ctx -import "context" +// MFAOutgoingHeader is the gRPC metadata key used to propagate MFA status across +// service boundaries. The "autoprop-" prefix causes the metadata interceptor +// (internal/grpc/interceptors/metadata) to forward it automatically at every +// gRPC hop, so no manual re-forwarding is required. +// The const rgrpc.AutoPropPrefix causes the cycle import +const MFAOutgoingHeader = "autoprop-mfa-authenticated" -// MFAHeader is the gRPC metadata key used to propagate MFA status across -// service boundaries. Lowercased to satisfy gRPC metadata requirements. // The corresponding HTTP header set by the proxy is "X-Multi-Factor-Authentication". -const MFAHeader = "x-mfa-authenticated" - -// ContextGetMFA returns the MFA status stored in the context, and whether it -// was set at all. A missing value (second return = false) should be treated as -// MFA not satisfied. -func ContextGetMFA(ctx context.Context) (bool, bool) { - v, ok := ctx.Value(mfaKey).(bool) - return v, ok -} - -// ContextSetMFA stores the MFA status in the context. -func ContextSetMFA(ctx context.Context, mfa bool) context.Context { - return context.WithValue(ctx, mfaKey, mfa) -} +const MFAHeader = "X-Multi-Factor-Authentication" diff --git a/pkg/ctx/userctx.go b/pkg/ctx/userctx.go index c32e96910a7..bb0c8076757 100644 --- a/pkg/ctx/userctx.go +++ b/pkg/ctx/userctx.go @@ -33,7 +33,6 @@ const ( lockIDKey scopeKey initiatorKey - mfaKey ) // ContextGetUser returns the user if set in the given context. From d76b0d672c57bdb99d03dff05f11ae99f079dd0b Mon Sep 17 00:00:00 2001 From: Roman Perekhod <2403905@gmail.com> Date: Sat, 18 Apr 2026 22:50:37 +0200 Subject: [PATCH 4/5] draft: mfa collaboration --- internal/grpc/interceptors/auth/auth.go | 4 ++-- internal/grpc/interceptors/auth/mfa.go | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/internal/grpc/interceptors/auth/auth.go b/internal/grpc/interceptors/auth/auth.go index db5ea3c5b1e..c48b061c4bb 100644 --- a/internal/grpc/interceptors/auth/auth.go +++ b/internal/grpc/interceptors/auth/auth.go @@ -160,7 +160,7 @@ func NewUnary(m map[string]interface{}, unprotected []string, tp trace.TracerPro ctx = grantMFAForServiceAccount(ctx, u) if conf.MFAEnabled { if mfav := metadata.ValueFromIncomingContext(ctx, ctxpkg.MFAOutgoingHeader); !slices.Contains(mfav, "true") { - log.Warn().Err(err).Msg("access token is invalid: MFA is required") + log.Warn().Str("user_id", u.Id.OpaqueId).Strs("mfa_values", mfav).Msg("MFA is required") return mfaResponse(ctx, req, info) } } @@ -256,7 +256,7 @@ func NewStream(m map[string]interface{}, unprotected []string, tp trace.TracerPr ctx = grantMFAForServiceAccount(ctx, u) if conf.MFAEnabled { if mfav := metadata.ValueFromIncomingContext(ctx, ctxpkg.MFAOutgoingHeader); !slices.Contains(mfav, "true") { - log.Warn().Err(err).Msg("access token is invalid: MFA is required") + log.Warn().Str("user_id", u.Id.OpaqueId).Strs("mfa_values", mfav).Msg("MFA is required") return status.Errorf(codes.PermissionDenied, "MFA required to access vault storage") } } diff --git a/internal/grpc/interceptors/auth/mfa.go b/internal/grpc/interceptors/auth/mfa.go index 513ed50d1bd..98cd97e3a82 100644 --- a/internal/grpc/interceptors/auth/mfa.go +++ b/internal/grpc/interceptors/auth/mfa.go @@ -58,6 +58,14 @@ func mfaResponse(ctx context.Context, req interface{}, info *grpc.UnaryServerInf return &provider.SetArbitraryMetadataResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil case *provider.UnsetArbitraryMetadataRequest: return &provider.UnsetArbitraryMetadataResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.ListStorageSpacesRequest: + return &provider.ListStorageSpacesResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.CreateStorageSpaceRequest: + return &provider.CreateStorageSpaceResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.UpdateStorageSpaceRequest: + return &provider.UpdateStorageSpaceResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil + case *provider.DeleteStorageSpaceRequest: + return &provider.DeleteStorageSpaceResponse{Status: rstatus.NewPermissionDenied(ctx, nil, msg)}, nil default: log.Debug().Str("method", info.FullMethod).Msg("mfa: blocking unknown request type") return nil, grpcstatus.Errorf(codes.PermissionDenied, "mfa: %s: %T", msg, req) From 0300dc8978e08fca29167b0664c1ab3bd7db7ca1 Mon Sep 17 00:00:00 2001 From: Roman Perekhod <2403905@gmail.com> Date: Wed, 22 Apr 2026 23:13:12 +0200 Subject: [PATCH 5/5] remove redundant code --- internal/grpc/interceptors/auth/auth.go | 15 --------------- pkg/auth/manager/oidc/oidc.go | 10 ---------- .../manager/serviceaccounts/serviceaccounts.go | 2 +- 3 files changed, 1 insertion(+), 26 deletions(-) diff --git a/internal/grpc/interceptors/auth/auth.go b/internal/grpc/interceptors/auth/auth.go index c48b061c4bb..deb8fa5b79a 100644 --- a/internal/grpc/interceptors/auth/auth.go +++ b/internal/grpc/interceptors/auth/auth.go @@ -157,7 +157,6 @@ func NewUnary(m map[string]interface{}, unprotected []string, tp trace.TracerPro // store user and scopes in context ctx = ctxpkg.ContextSetUser(ctx, u) ctx = ctxpkg.ContextSetScopes(ctx, tokenScope) - ctx = grantMFAForServiceAccount(ctx, u) if conf.MFAEnabled { if mfav := metadata.ValueFromIncomingContext(ctx, ctxpkg.MFAOutgoingHeader); !slices.Contains(mfav, "true") { log.Warn().Str("user_id", u.Id.OpaqueId).Strs("mfa_values", mfav).Msg("MFA is required") @@ -253,7 +252,6 @@ func NewStream(m map[string]interface{}, unprotected []string, tp trace.TracerPr // store user and scopes in context ctx = ctxpkg.ContextSetUser(ctx, u) ctx = ctxpkg.ContextSetScopes(ctx, tokenScope) - ctx = grantMFAForServiceAccount(ctx, u) if conf.MFAEnabled { if mfav := metadata.ValueFromIncomingContext(ctx, ctxpkg.MFAOutgoingHeader); !slices.Contains(mfav, "true") { log.Warn().Str("user_id", u.Id.OpaqueId).Strs("mfa_values", mfav).Msg("MFA is required") @@ -317,19 +315,6 @@ func dismantleToken(ctx context.Context, tkn string, req interface{}, mgr token. return u, tokenScope, nil } -// grantMFAForServiceAccount automatically grants MFA for service accounts by -// injecting the autoprop-prefixed metadata key into outgoing context. -// Service accounts are trusted internal processes that never authenticate via OIDC and -// therefore never carry an acr/MFA claim. Granting them implicit MFA allows -// them to access MFA-gated resources such as vault storage without -// compromising the MFA requirement for regular users. -func grantMFAForServiceAccount(ctx context.Context, u *userpb.User) context.Context { - if u.GetId().GetType() != userpb.UserType_USER_TYPE_SERVICE { - return ctx - } - return metadata.AppendToOutgoingContext(ctx, ctxpkg.MFAOutgoingHeader, "true") -} - func getUserGroups(ctx context.Context, u *userpb.User, client gatewayv1beta1.GatewayAPIClient) ([]string, error) { if groupsIf, err := userGroupsCache.Get(u.Id.OpaqueId); err == nil { log := appctx.GetLogger(ctx) diff --git a/pkg/auth/manager/oidc/oidc.go b/pkg/auth/manager/oidc/oidc.go index f716e1c86e7..757aca3b9cb 100644 --- a/pkg/auth/manager/oidc/oidc.go +++ b/pkg/auth/manager/oidc/oidc.go @@ -43,7 +43,6 @@ import ( "github.com/owncloud/reva/v2/pkg/rgrpc/todo/pool" "github.com/owncloud/reva/v2/pkg/rhttp" "github.com/owncloud/reva/v2/pkg/sharedconf" - "github.com/owncloud/reva/v2/pkg/utils" "github.com/pkg/errors" "golang.org/x/oauth2" ) @@ -242,15 +241,6 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string) GidNumber: claims[am.c.GIDClaim].(int64), } - // Embed the OIDC acr (Authentication Context Class Reference) claim into - // User.Opaque so it is cryptographically bound to the minted reva JWT. - // Downstream services (e.g. the mfa gRPC interceptor) can use this - // as a secondary MFA proof independent of the X-Multi-Factor-Authentication - // HTTP header propagation path. - if acr, ok := claims["acr"].(string); ok && acr != "" { - u.Opaque = utils.AppendPlainToOpaque(u.Opaque, "acr", acr) - } - var scopes map[string]*authpb.Scope if userID != nil && (userID.Type == user.UserType_USER_TYPE_LIGHTWEIGHT || userID.Type == user.UserType_USER_TYPE_FEDERATED) { scopes, err = scope.AddLightweightAccountScope(authpb.Role_ROLE_OWNER, nil) diff --git a/pkg/auth/manager/serviceaccounts/serviceaccounts.go b/pkg/auth/manager/serviceaccounts/serviceaccounts.go index 2d3fabfa68d..a7238352739 100644 --- a/pkg/auth/manager/serviceaccounts/serviceaccounts.go +++ b/pkg/auth/manager/serviceaccounts/serviceaccounts.go @@ -6,10 +6,10 @@ import ( authpb "github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + "github.com/mitchellh/mapstructure" "github.com/owncloud/reva/v2/pkg/auth" "github.com/owncloud/reva/v2/pkg/auth/manager/registry" "github.com/owncloud/reva/v2/pkg/auth/scope" - "github.com/mitchellh/mapstructure" "github.com/pkg/errors" )