From 44006efb630def444d94dbdf0f65c7a5da58e8b4 Mon Sep 17 00:00:00 2001 From: Firas Frikha Date: Thu, 3 Sep 2026 15:48:56 +0100 Subject: [PATCH 1/2] fix(gateway): [OCISDEV-1200] enforce vault mode on the share grantee --- .../fix-vault-share-grantee-permission.md | 15 +++++ .../services/gateway/usershareprovider.go | 60 +++++++++++++++++ .../gateway/usershareprovider_test.go | 64 +++++++++++++++++++ pkg/permission/permission.go | 2 + 4 files changed, 141 insertions(+) create mode 100644 changelog/unreleased/fix-vault-share-grantee-permission.md diff --git a/changelog/unreleased/fix-vault-share-grantee-permission.md b/changelog/unreleased/fix-vault-share-grantee-permission.md new file mode 100644 index 00000000000..6d0a8251df0 --- /dev/null +++ b/changelog/unreleased/fix-vault-share-grantee-permission.md @@ -0,0 +1,15 @@ +Bugfix: Only share vault resources with grantees allowed to use vault mode + +Vault ("Safe") resources could be shared with any grantee, including users that lack +the `VaultMode.ReadWriteEnabled` permission and can therefore never enter vault mode. +Such a grant was useless to the grantee but still produced a share notification that +disclosed the resource name to them. + +Share creation on a vault resource now verifies that the grantee holds the vault mode +permission and is denied otherwise. The check is applied in the gateway ahead of the +space root branch, so it covers plain shares as well as space memberships, and it fails +closed if the permission cannot be determined. Group grantees are rejected, because +settings role assignments are held by individual accounts and group membership can +change after the share has been created. + +https://github.com/owncloud/reva/pull/TODO-fill-in-after-opening-the-pr diff --git a/internal/grpc/services/gateway/usershareprovider.go b/internal/grpc/services/gateway/usershareprovider.go index 7d0980f71ca..5a7b07fc5c7 100644 --- a/internal/grpc/services/gateway/usershareprovider.go +++ b/internal/grpc/services/gateway/usershareprovider.go @@ -23,6 +23,7 @@ import ( "slices" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + permissions "github.com/cs3org/go-cs3apis/cs3/permissions/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" @@ -31,6 +32,7 @@ import ( "github.com/owncloud/reva/v2/pkg/conversions" ctxpkg "github.com/owncloud/reva/v2/pkg/ctx" "github.com/owncloud/reva/v2/pkg/errtypes" + "github.com/owncloud/reva/v2/pkg/permission" "github.com/owncloud/reva/v2/pkg/rgrpc/status" "github.com/owncloud/reva/v2/pkg/rgrpc/todo/pool" "github.com/owncloud/reva/v2/pkg/storage/utils/grants" @@ -48,6 +50,24 @@ func hasGrantManagementBits(p *provider.ResourcePermissions) bool { // TODO(labkode): add multi-phase commit logic when commit share or commit ref is enabled. func (s *svc) CreateShare(ctx context.Context, req *collaboration.CreateShareRequest) (*collaboration.CreateShareResponse, error) { + // Vault resources must only be shared with grantees that may use vault mode. + if req.GetResourceInfo().GetId().GetStorageId() == utils.VaultStorageProviderID { + allowed, err := s.granteeMayAccessVault(ctx, req.GetGrant().GetGrantee()) + if err != nil { + appctx.GetLogger(ctx).Error().Err(err). + Interface("grantee", req.GetGrant().GetGrantee()). + Msg("could not determine whether the grantee may access vault resources") + return &collaboration.CreateShareResponse{ + Status: status.NewInternal(ctx, "error checking grantee vault permission"), + }, nil + } + if !allowed { + return &collaboration.CreateShareResponse{ + Status: status.NewPermissionDenied(ctx, nil, "grantee is not allowed to access vault resources"), + }, nil + } + } + // Don't use the share manager when sharing a space root if !s.c.UseCommonSpaceRootShareLogic && refIsSpaceRoot(req.ResourceInfo.Id) { return s.addSpaceShare(ctx, req) @@ -55,6 +75,46 @@ func (s *svc) CreateShare(ctx context.Context, req *collaboration.CreateShareReq return s.addShare(ctx, req) } +// granteeMayAccessVault reports whether the grantee may be granted access to a vault resource. +// A non-nil error means undetermined, never denied, so callers must fail closed. +func (s *svc) granteeMayAccessVault(ctx context.Context, g *provider.Grantee) (bool, error) { + ref := vaultPermissionSubject(g) + if ref == nil { + return false, nil + } + + res, err := s.CheckPermission(ctx, &permissions.CheckPermissionRequest{ + SubjectRef: ref, + Permission: permission.VaultMode, + }) + if err != nil { + return false, err + } + return vaultPermissionGranted(res.GetStatus()) +} + +// vaultPermissionSubject returns the subject to check VaultMode for, or nil for grantees that can never hold it, like groups. +func vaultPermissionSubject(g *provider.Grantee) *permissions.SubjectReference { + if g.GetType() != provider.GranteeType_GRANTEE_TYPE_USER { + return nil + } + return &permissions.SubjectReference{ + Spec: &permissions.SubjectReference_UserId{UserId: g.GetUserId()}, + } +} + +// vaultPermissionGranted interprets the status of a VaultMode check, erring on anything but a definitive answer. +func vaultPermissionGranted(st *rpc.Status) (bool, error) { + switch code := st.GetCode(); code { + case rpc.Code_CODE_OK: + return true, nil + case rpc.Code_CODE_PERMISSION_DENIED: + return false, nil + default: + return false, errors.Errorf("vault permission check returned non-authoritative status %q: %s", code, st.GetMessage()) + } +} + func (s *svc) RemoveShare(ctx context.Context, req *collaboration.RemoveShareRequest) (*collaboration.RemoveShareResponse, error) { key := req.GetRef().GetKey() if !s.c.UseCommonSpaceRootShareLogic && shareIsSpaceRoot(key) { diff --git a/internal/grpc/services/gateway/usershareprovider_test.go b/internal/grpc/services/gateway/usershareprovider_test.go index 3735b97a9e7..3462a2db2bb 100644 --- a/internal/grpc/services/gateway/usershareprovider_test.go +++ b/internal/grpc/services/gateway/usershareprovider_test.go @@ -21,7 +21,9 @@ package gateway import ( "testing" + grouppb "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + 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/owncloud/reva/v2/pkg/conversions" @@ -132,3 +134,65 @@ func TestSpaceShareRejectsPartialGrantManagement(t *testing.T) { t.Error("space editor grant should be accepted on a space root") } } + +func groupGrantee(id string) *provider.Grantee { + return &provider.Grantee{ + Type: provider.GranteeType_GRANTEE_TYPE_GROUP, + Id: &provider.Grantee_GroupId{GroupId: &grouppb.GroupId{OpaqueId: id}}, + } +} + +// TestVaultPermissionSubject covers which grantees are eligible for a VaultMode check at all. +func TestVaultPermissionSubject(t *testing.T) { + tests := []struct { + name string + grantee *provider.Grantee + wantSubject bool + }{ + {"user grantee is checked", userGrantee("alice"), true}, + {"group grantee is denied outright", groupGrantee("students"), false}, + {"invalid grantee is denied outright", &provider.Grantee{Type: provider.GranteeType_GRANTEE_TYPE_INVALID}, false}, + {"nil grantee is denied outright", nil, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := vaultPermissionSubject(tt.grantee) + if (got != nil) != tt.wantSubject { + t.Fatalf("vaultPermissionSubject() subject = %v, want subject %v", got, tt.wantSubject) + } + if tt.wantSubject && got.GetUserId().GetOpaqueId() != tt.grantee.GetUserId().GetOpaqueId() { + t.Errorf("vaultPermissionSubject() checked %q, want the grantee %q", + got.GetUserId().GetOpaqueId(), tt.grantee.GetUserId().GetOpaqueId()) + } + }) + } +} + +// TestVaultPermissionGranted covers the fail-closed contract: only a definitive OK grants access. +func TestVaultPermissionGranted(t *testing.T) { + tests := []struct { + name string + status *rpc.Status + want bool + wantErr bool + }{ + {"ok grants", &rpc.Status{Code: rpc.Code_CODE_OK}, true, false}, + {"permission denied denies", &rpc.Status{Code: rpc.Code_CODE_PERMISSION_DENIED}, false, false}, + {"internal error fails closed", &rpc.Status{Code: rpc.Code_CODE_INTERNAL}, false, true}, + {"not found fails closed", &rpc.Status{Code: rpc.Code_CODE_NOT_FOUND}, false, true}, + {"unset status fails closed", nil, false, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := vaultPermissionGranted(tt.status) + if (err != nil) != tt.wantErr { + t.Fatalf("vaultPermissionGranted() error = %v, wantErr %v", err, tt.wantErr) + } + if got != tt.want { + t.Errorf("vaultPermissionGranted() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/permission/permission.go b/pkg/permission/permission.go index fb82add4439..607198931b5 100644 --- a/pkg/permission/permission.go +++ b/pkg/permission/permission.go @@ -37,6 +37,8 @@ const ( WriteFavorites string = "Favorites.Write" // DeleteReadOnlyPassword is the hardcoded name for the ReadOnlyPublicLinkPassword.Delete permission DeleteReadOnlyPassword string = "ReadOnlyPublicLinkPassword.Delete" + // VaultMode is the hardcoded name for the VaultMode.ReadWriteEnabled permission + VaultMode string = "VaultMode.ReadWriteEnabled" ) // Manager defines the interface for the permission service driver From 00db32b1a2cdde49108cd454436e9c83e23ec533 Mon Sep 17 00:00:00 2001 From: Firas Frikha Date: Thu, 3 Sep 2026 15:51:24 +0100 Subject: [PATCH 2/2] fix: add changelog file --- changelog/unreleased/fix-vault-share-grantee-permission.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/unreleased/fix-vault-share-grantee-permission.md b/changelog/unreleased/fix-vault-share-grantee-permission.md index 6d0a8251df0..1160b9e86e9 100644 --- a/changelog/unreleased/fix-vault-share-grantee-permission.md +++ b/changelog/unreleased/fix-vault-share-grantee-permission.md @@ -12,4 +12,4 @@ closed if the permission cannot be determined. Group grantees are rejected, beca settings role assignments are held by individual accounts and group membership can change after the share has been created. -https://github.com/owncloud/reva/pull/TODO-fill-in-after-opening-the-pr +https://github.com/owncloud/reva/pull/727