Repository navigation
feat: [OCISDEV-1516] allow group shares in the vault #13260
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| Enhancement: Allow group shares in the vault | ||
|
|
||
| Users can now invite a group to a resource in the vault (Safe). Before this | ||
| change, the server accepted only users as vault share recipients. The share | ||
| dialog in Web now also searches for groups in vault mode. The | ||
| notifications and userlog services now send a vault notification only to the | ||
| group members who have the permission to use the vault. The other group | ||
| members get no email and no in-app notification. | ||
|
|
||
| https://github.com/owncloud/ocis/pull/13260 | ||
| https://github.com/owncloud/reva/pull/787 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| // Package vault provides helpers for resources in the vault storage provider. | ||
| package vault | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
|
|
||
| gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" | ||
| user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" | ||
| permissions "github.com/cs3org/go-cs3apis/cs3/permissions/v1beta1" | ||
| rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" | ||
| "github.com/owncloud/reva/v2/pkg/permission" | ||
| "github.com/owncloud/reva/v2/pkg/storagespace" | ||
| "github.com/owncloud/reva/v2/pkg/utils" | ||
| "go.opentelemetry.io/otel/attribute" | ||
| "go.opentelemetry.io/otel/trace" | ||
| "golang.org/x/sync/errgroup" | ||
| ) | ||
|
|
||
| // maxConcurrentChecks bounds how many VaultMode checks run at the same time, so that a large | ||
| // group does not send all its checks to the gateway at once. | ||
| const maxConcurrentChecks = 10 | ||
|
|
||
| const tracerName = "github.com/owncloud/ocis/v2/ocis-pkg/vault" | ||
|
|
||
| // IsVaultStorage reports whether the storage ID belongs to the vault storage provider. | ||
| func IsVaultStorage(storageID string) bool { | ||
| return storageID == utils.VaultStorageProviderID | ||
| } | ||
|
|
||
| // IsVaultSpace reports whether the space ID belongs to the vault storage provider. | ||
| // It returns true for a space ID that it cannot parse, so that callers fail closed. | ||
| func IsVaultSpace(spaceID string) bool { | ||
| rid, err := storagespace.ParseID(spaceID) | ||
| if err != nil { | ||
| return true | ||
| } | ||
| return IsVaultStorage(rid.GetStorageId()) | ||
| } | ||
|
|
||
| // HasVaultMode reports whether the user holds the VaultMode permission. | ||
| // A non-nil error means undetermined, so callers must treat the user as not allowed. | ||
| func HasVaultMode(ctx context.Context, gwc gateway.GatewayAPIClient, userID *user.UserId) (bool, error) { | ||
| res, err := gwc.CheckPermission(ctx, &permissions.CheckPermissionRequest{ | ||
| SubjectRef: &permissions.SubjectReference{ | ||
| Spec: &permissions.SubjectReference_UserId{UserId: userID}, | ||
| }, | ||
| Permission: permission.VaultMode, | ||
| }) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
| switch code := res.GetStatus().GetCode(); code { | ||
| case rpc.Code_CODE_OK: | ||
| return true, nil | ||
| case rpc.Code_CODE_PERMISSION_DENIED: | ||
| return false, nil | ||
| default: | ||
| return false, fmt.Errorf("vault permission check returned status %q: %s", code, res.GetStatus().GetMessage()) | ||
| } | ||
| } | ||
|
|
||
| // FilterVaultModeUsers returns the users from userIDs who hold the VaultMode permission, in the | ||
| // same order. A group can contain users without the permission, because the gateway does not | ||
| // check the group members. The checks run in parallel, with a bound. When a check fails, the | ||
| // function calls onError for that user and leaves the user out, so that the caller fails closed. | ||
| func FilterVaultModeUsers(ctx context.Context, gwc gateway.GatewayAPIClient, userIDs []*user.UserId, onError func(*user.UserId, error)) []*user.UserId { | ||
| ctx, span := trace.SpanFromContext(ctx).TracerProvider().Tracer(tracerName).Start(ctx, "FilterVaultModeUsers", | ||
| trace.WithAttributes(attribute.Int("vault.users.checked", len(userIDs)))) | ||
| defer span.End() | ||
|
|
||
| allowed := make([]bool, len(userIDs)) | ||
| errs := make([]error, len(userIDs)) | ||
| var eg errgroup.Group | ||
| eg.SetLimit(maxConcurrentChecks) | ||
| for i, id := range userIDs { | ||
| eg.Go(func() error { | ||
| allowed[i], errs[i] = HasVaultMode(ctx, gwc, id) | ||
| return nil | ||
| }) | ||
| } | ||
| _ = eg.Wait() | ||
|
|
||
| filtered := make([]*user.UserId, 0, len(userIDs)) | ||
| failed := 0 | ||
| for i, id := range userIDs { | ||
| if errs[i] != nil { | ||
| failed++ | ||
| if onError != nil { | ||
| onError(id, errs[i]) | ||
| } | ||
| continue | ||
| } | ||
| if allowed[i] { | ||
| filtered = append(filtered, id) | ||
| } | ||
| } | ||
| span.SetAttributes( | ||
| attribute.Int("vault.users.allowed", len(filtered)), | ||
| attribute.Int("vault.users.failed", failed), | ||
| ) | ||
| return filtered | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| package vault | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "fmt" | ||
| "slices" | ||
| "testing" | ||
|
|
||
| user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" | ||
| permissions "github.com/cs3org/go-cs3apis/cs3/permissions/v1beta1" | ||
| rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" | ||
| "github.com/owncloud/reva/v2/pkg/permission" | ||
| "github.com/owncloud/reva/v2/pkg/utils" | ||
| cs3mocks "github.com/owncloud/reva/v2/tests/cs3mocks/mocks" | ||
| "github.com/stretchr/testify/mock" | ||
| "google.golang.org/grpc" | ||
| ) | ||
|
|
||
| func TestIsVaultSpace(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| spaceID string | ||
| want bool | ||
| }{ | ||
| {"vault space", utils.VaultStorageProviderID + "$space-id", true}, | ||
| {"regular space", "storage-id$space-id", false}, | ||
| {"unparsable space id fails closed", "", true}, | ||
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| if got := IsVaultSpace(tt.spaceID); got != tt.want { | ||
| t.Errorf("IsVaultSpace(%q) = %v, want %v", tt.spaceID, got, tt.want) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestHasVaultMode(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| res *permissions.CheckPermissionResponse | ||
| err error | ||
| want bool | ||
| wantErr bool | ||
| }{ | ||
| {"ok grants", &permissions.CheckPermissionResponse{Status: &rpc.Status{Code: rpc.Code_CODE_OK}}, nil, true, false}, | ||
| {"permission denied denies", &permissions.CheckPermissionResponse{Status: &rpc.Status{Code: rpc.Code_CODE_PERMISSION_DENIED}}, nil, false, false}, | ||
| {"internal status fails closed", &permissions.CheckPermissionResponse{Status: &rpc.Status{Code: rpc.Code_CODE_INTERNAL}}, nil, false, true}, | ||
| {"transport error fails closed", nil, errors.New("unavailable"), false, true}, | ||
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| gwc := &cs3mocks.GatewayAPIClient{} | ||
| gwc.On("CheckPermission", mock.Anything, mock.MatchedBy(func(req *permissions.CheckPermissionRequest) bool { | ||
| return req.GetPermission() == permission.VaultMode && req.GetSubjectRef().GetUserId().GetOpaqueId() == "alice" | ||
| })).Return(tt.res, tt.err) | ||
|
|
||
| got, err := HasVaultMode(context.Background(), gwc, &user.UserId{OpaqueId: "alice"}) | ||
| if (err != nil) != tt.wantErr { | ||
| t.Fatalf("HasVaultMode() error = %v, wantErr %v", err, tt.wantErr) | ||
| } | ||
| if got != tt.want { | ||
| t.Errorf("HasVaultMode() = %v, want %v", got, tt.want) | ||
| } | ||
| gwc.AssertExpectations(t) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestFilterVaultModeUsers(t *testing.T) { | ||
| // alice and every "ok-" user hold the permission, bob does not, and the check for carol fails | ||
| gwc := &cs3mocks.GatewayAPIClient{} | ||
| gwc.On("CheckPermission", mock.Anything, mock.Anything).Return(func(_ context.Context, r *permissions.CheckPermissionRequest, _ ...grpc.CallOption) (*permissions.CheckPermissionResponse, error) { | ||
| switch id := r.GetSubjectRef().GetUserId().GetOpaqueId(); { | ||
| case id == "carol": | ||
| return nil, errors.New("unavailable") | ||
| case id == "bob": | ||
| return &permissions.CheckPermissionResponse{Status: &rpc.Status{Code: rpc.Code_CODE_PERMISSION_DENIED}}, nil | ||
| default: | ||
| return &permissions.CheckPermissionResponse{Status: &rpc.Status{Code: rpc.Code_CODE_OK}}, nil | ||
| } | ||
| }) | ||
|
|
||
| ids := []*user.UserId{{OpaqueId: "alice"}, {OpaqueId: "bob"}, {OpaqueId: "carol"}} | ||
| for i := range 50 { | ||
| ids = append(ids, &user.UserId{OpaqueId: fmt.Sprintf("ok-%02d", i)}) | ||
| } | ||
|
|
||
| var failed []string | ||
| got := FilterVaultModeUsers(context.Background(), gwc, ids, func(id *user.UserId, err error) { | ||
| failed = append(failed, id.GetOpaqueId()) | ||
| }) | ||
|
|
||
| want := []string{"alice"} | ||
| for i := range 50 { | ||
| want = append(want, fmt.Sprintf("ok-%02d", i)) | ||
| } | ||
| gotIDs := make([]string, 0, len(got)) | ||
| for _, id := range got { | ||
| gotIDs = append(gotIDs, id.GetOpaqueId()) | ||
| } | ||
| if !slices.Equal(gotIDs, want) { | ||
| t.Errorf("FilterVaultModeUsers() = %v, want %v", gotIDs, want) | ||
| } | ||
| if !slices.Equal(failed, []string{"carol"}) { | ||
| t.Errorf("onError called for %v, want [carol]", failed) | ||
| } | ||
| gwc.AssertNumberOfCalls(t, "CheckPermission", len(ids)) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this is fine because each goroutine will write in a different memory address, but I'd rather have some confirmation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is safe. The code makes both slices at full length before the loop starts, and they never grow. Each goroutine writes only its own index, and eg.Wait() makes the writes visible before the reads. Go 1.22+ gives each loop pass its own i and id.