From 7a0869addaf79ac58d224bc04a8130f8ed1b1fc9 Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Wed, 8 Nov 2023 15:24:20 +0100 Subject: [PATCH 1/3] WIP sharing NG links --- .../services/publicshareprovider/publicshareprovider.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/grpc/services/publicshareprovider/publicshareprovider.go b/internal/grpc/services/publicshareprovider/publicshareprovider.go index e8f18aea6c..c993f30f61 100644 --- a/internal/grpc/services/publicshareprovider/publicshareprovider.go +++ b/internal/grpc/services/publicshareprovider/publicshareprovider.go @@ -24,6 +24,7 @@ import ( rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" + typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/cs3org/reva/v2/pkg/appctx" ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" "github.com/cs3org/reva/v2/pkg/errtypes" @@ -31,7 +32,9 @@ import ( "github.com/cs3org/reva/v2/pkg/publicshare/manager/registry" "github.com/cs3org/reva/v2/pkg/rgrpc" "github.com/cs3org/reva/v2/pkg/rgrpc/status" + "github.com/cs3org/reva/v2/pkg/utils" "github.com/mitchellh/mapstructure" + graph "github.com/owncloud/ocis/v2/services/graph/pkg/service/v0/errorcode" "github.com/pkg/errors" "google.golang.org/grpc" ) @@ -44,6 +47,7 @@ type config struct { Driver string `mapstructure:"driver"` Drivers map[string]map[string]interface{} `mapstructure:"drivers"` AllowedPathsForShares []string `mapstructure:"allowed_paths_for_shares"` + EnableExpiredSharesCleanup bool `mapstructure:"enable_expired_shares_cleanup"` WriteableShareMustHavePassword bool `mapstructure:"writeable_share_must_have_password"` } @@ -159,10 +163,12 @@ func (s *service) CreatePublicShare(ctx context.Context, req *link.CreatePublicS if err != nil { log.Debug().Err(err).Str("createShare", "shares").Msg("error connecting to storage provider") } - + opaque := &typesv1beta1.Opaque{Map: map[string]*typesv1beta1.OpaqueEntry{}} + opaque = utils.AppendJSONToOpaque(opaque, "errorcode", graph.New(graph.InvalidAuthenticationToken, "test")) res := &link.CreatePublicShareResponse{ Status: status.NewOK(ctx), Share: share, + Opaque: opaque, } return res, nil } From ebcf4ffd782054637680035ccad2191279dda81e Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Mon, 13 Nov 2023 20:08:15 +0100 Subject: [PATCH 2/3] add sufficient permissions check --- .../publicshareprovider.go | 14 ++- pkg/conversions/role.go | 31 +++++ pkg/conversions/role_test.go | 107 ++++++++++++++++++ 3 files changed, 146 insertions(+), 6 deletions(-) create mode 100644 pkg/conversions/role_test.go diff --git a/internal/grpc/services/publicshareprovider/publicshareprovider.go b/internal/grpc/services/publicshareprovider/publicshareprovider.go index c993f30f61..f142d76fa0 100644 --- a/internal/grpc/services/publicshareprovider/publicshareprovider.go +++ b/internal/grpc/services/publicshareprovider/publicshareprovider.go @@ -24,17 +24,15 @@ import ( rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" - typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/cs3org/reva/v2/pkg/appctx" + "github.com/cs3org/reva/v2/pkg/conversions" ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" "github.com/cs3org/reva/v2/pkg/errtypes" "github.com/cs3org/reva/v2/pkg/publicshare" "github.com/cs3org/reva/v2/pkg/publicshare/manager/registry" "github.com/cs3org/reva/v2/pkg/rgrpc" "github.com/cs3org/reva/v2/pkg/rgrpc/status" - "github.com/cs3org/reva/v2/pkg/utils" "github.com/mitchellh/mapstructure" - graph "github.com/owncloud/ocis/v2/services/graph/pkg/service/v0/errorcode" "github.com/pkg/errors" "google.golang.org/grpc" ) @@ -140,6 +138,12 @@ func (s *service) CreatePublicShare(ctx context.Context, req *link.CreatePublicS log := appctx.GetLogger(ctx) log.Info().Str("publicshareprovider", "create").Msg("create public share") + if !conversions.SufficientCS3Permissions(req.GetResourceInfo().GetPermissionSet(), req.GetGrant().GetPermissions().GetPermissions()) { + return &link.CreatePublicShareResponse{ + Status: status.NewInvalid(ctx, "insufficient permissions to create that kind of share"), + }, nil + } + if !s.isPathAllowed(req.ResourceInfo.Path) { return &link.CreatePublicShareResponse{ Status: status.NewInvalid(ctx, "share creation is not allowed for the specified path"), @@ -163,12 +167,10 @@ func (s *service) CreatePublicShare(ctx context.Context, req *link.CreatePublicS if err != nil { log.Debug().Err(err).Str("createShare", "shares").Msg("error connecting to storage provider") } - opaque := &typesv1beta1.Opaque{Map: map[string]*typesv1beta1.OpaqueEntry{}} - opaque = utils.AppendJSONToOpaque(opaque, "errorcode", graph.New(graph.InvalidAuthenticationToken, "test")) + res := &link.CreatePublicShareResponse{ Status: status.NewOK(ctx), Share: share, - Opaque: opaque, } return res, nil } diff --git a/pkg/conversions/role.go b/pkg/conversions/role.go index 6971957c8f..d842036b11 100644 --- a/pkg/conversions/role.go +++ b/pkg/conversions/role.go @@ -21,6 +21,7 @@ package conversions import ( "fmt" + "reflect" "strings" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" @@ -508,3 +509,33 @@ func RoleFromResourcePermissions(rp *provider.ResourcePermissions, islink bool) // TODO what about even more granular cs3 permissions?, eg. only stat return r } + +// SufficientCS3Permissions returns true if the `existing` permissions contain the `requested` permissions +func SufficientCS3Permissions(existing, requested *provider.ResourcePermissions) bool { + if existing == nil || requested == nil { + return false + } + // empty permissions represent a denial + if grants.PermissionsEqual(requested, &provider.ResourcePermissions{}) { + return existing.DenyGrant + } + requestedPermissionsType := reflect.TypeOf(provider.ResourcePermissions{}) + numFields := requestedPermissionsType.NumField() + requestedPermissionsValues := reflect.ValueOf(requested) + existingPermissionsValues := reflect.ValueOf(existing) + + for i := 0; i < numFields; i++ { + permissionName := requestedPermissionsType.Field(i).Name + // filter out irrelevant fields + if strings.Contains(permissionName, "XXX") { + continue + } + existingPermission := reflect.Indirect(existingPermissionsValues).FieldByName(permissionName).Bool() + requestedPermission := requestedPermissionsValues.Elem().Field(i).Bool() + // every requested permission needs to exist for the creator + if requestedPermission && !existingPermission { + return false + } + } + return true +} diff --git a/pkg/conversions/role_test.go b/pkg/conversions/role_test.go new file mode 100644 index 0000000000..44c8499fa3 --- /dev/null +++ b/pkg/conversions/role_test.go @@ -0,0 +1,107 @@ +package conversions + +import ( + "testing" + + providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/stretchr/testify/assert" +) + +func TestSufficientPermissions(t *testing.T) { + type testData struct { + Existing *providerv1beta1.ResourcePermissions + Requested *providerv1beta1.ResourcePermissions + Sufficient bool + } + table := []testData{ + { + Existing: nil, + Requested: nil, + Sufficient: false, + }, + { + Existing: RoleFromName("editor", true).CS3ResourcePermissions(), + Requested: nil, + Sufficient: false, + }, + { + Existing: nil, + Requested: RoleFromName("viewer", true).CS3ResourcePermissions(), + Sufficient: false, + }, + { + Existing: RoleFromName("editor", true).CS3ResourcePermissions(), + Requested: RoleFromName("viewer", true).CS3ResourcePermissions(), + Sufficient: true, + }, + { + Existing: RoleFromName("viewer", true).CS3ResourcePermissions(), + Requested: RoleFromName("editor", true).CS3ResourcePermissions(), + Sufficient: false, + }, + { + Existing: RoleFromName("spaceviewer", true).CS3ResourcePermissions(), + Requested: RoleFromName("spaceeditor", true).CS3ResourcePermissions(), + Sufficient: false, + }, + { + Existing: RoleFromName("manager", true).CS3ResourcePermissions(), + Requested: RoleFromName("spaceeditor", true).CS3ResourcePermissions(), + Sufficient: true, + }, + { + Existing: RoleFromName("manager", true).CS3ResourcePermissions(), + Requested: RoleFromName("spaceviewer", true).CS3ResourcePermissions(), + Sufficient: true, + }, + { + Existing: RoleFromName("manager", true).CS3ResourcePermissions(), + Requested: RoleFromName("manager", true).CS3ResourcePermissions(), + Sufficient: true, + }, + { + Existing: RoleFromName("manager", true).CS3ResourcePermissions(), + Requested: RoleFromName("denied", true).CS3ResourcePermissions(), + Sufficient: true, + }, + { + Existing: RoleFromName("spaceeditor", true).CS3ResourcePermissions(), + Requested: RoleFromName("denied", true).CS3ResourcePermissions(), + Sufficient: false, + }, + { + Existing: RoleFromName("editor", true).CS3ResourcePermissions(), + Requested: RoleFromName("denied", true).CS3ResourcePermissions(), + Sufficient: false, + }, + { + Existing: &providerv1beta1.ResourcePermissions{ + // all permissions, used for personal space owners + AddGrant: true, + CreateContainer: true, + Delete: true, + GetPath: true, + GetQuota: true, + InitiateFileDownload: true, + InitiateFileUpload: true, + ListContainer: true, + ListFileVersions: true, + ListGrants: true, + ListRecycle: true, + Move: true, + PurgeRecycle: true, + RemoveGrant: true, + RestoreFileVersion: true, + RestoreRecycleItem: true, + Stat: true, + UpdateGrant: true, + DenyGrant: true, + }, + Requested: RoleFromName("denied", true).CS3ResourcePermissions(), + Sufficient: true, + }, + } + for _, test := range table { + assert.Equal(t, test.Sufficient, SufficientCS3Permissions(test.Existing, test.Requested)) + } +} From 3d7572e4b41b637bea1893460a0defbe837f2fa5 Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Mon, 13 Nov 2023 20:30:05 +0100 Subject: [PATCH 3/3] add changelog --- changelog/unreleased/fix-shares-cleanup-config.md | 5 +++++ changelog/unreleased/new-permission-helper-function.md | 6 ++++++ 2 files changed, 11 insertions(+) create mode 100644 changelog/unreleased/fix-shares-cleanup-config.md create mode 100644 changelog/unreleased/new-permission-helper-function.md diff --git a/changelog/unreleased/fix-shares-cleanup-config.md b/changelog/unreleased/fix-shares-cleanup-config.md new file mode 100644 index 0000000000..967eac8997 --- /dev/null +++ b/changelog/unreleased/fix-shares-cleanup-config.md @@ -0,0 +1,5 @@ +Bugfix: Fix public shares cleanup config + +The public shares cleanup for expired shares was not configurable via ocis. + +https://github.com/cs3org/reva/pull/4335/ diff --git a/changelog/unreleased/new-permission-helper-function.md b/changelog/unreleased/new-permission-helper-function.md new file mode 100644 index 0000000000..4e1914a12f --- /dev/null +++ b/changelog/unreleased/new-permission-helper-function.md @@ -0,0 +1,6 @@ +Enhancement: Add sufficient permissions check function + +We added a helper function to check for sufficient CS3 resource permissions. + +https://github.com/cs3org/reva/pull/4335/ +https://github.com/owncloud/ocis/issues/6993