From 516a5784809c8a05c9004934a13eb7c572b2a989 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Tue, 12 Oct 2021 09:40:02 +0200 Subject: [PATCH 1/7] Change condition for setting sharing permissions --- pkg/storage/utils/grants/grants.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/storage/utils/grants/grants.go b/pkg/storage/utils/grants/grants.go index 32f18a8d8a..aa65e83c89 100644 --- a/pkg/storage/utils/grants/grants.go +++ b/pkg/storage/utils/grants/grants.go @@ -96,7 +96,7 @@ func GetGrantPermissionSet(perm string, isDir bool) *provider.ResourcePermission rp.Delete = false } - if strings.Contains(perm, "m") && !strings.Contains(perm, "!m") { + if strings.Contains(perm, "w") && strings.Contains(perm, "x") && !strings.Contains(perm, "!m") { rp.AddGrant = true rp.ListGrants = true rp.RemoveGrant = true From 839bcb6c39e15a968239d5b0bf2591b41c69c1f4 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Tue, 12 Oct 2021 10:34:50 +0200 Subject: [PATCH 2/7] Add changelog --- changelog/1.14.0_2021-10-12/grants-share-perms.md | 3 +++ pkg/cbox/share/sql/sql.go | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 changelog/1.14.0_2021-10-12/grants-share-perms.md diff --git a/changelog/1.14.0_2021-10-12/grants-share-perms.md b/changelog/1.14.0_2021-10-12/grants-share-perms.md new file mode 100644 index 0000000000..3771706ea3 --- /dev/null +++ b/changelog/1.14.0_2021-10-12/grants-share-perms.md @@ -0,0 +1,3 @@ +Bugfix: Update checks for setting sharing permissions from EOS grants + +https://github.com/cs3org/reva/pull/2153 \ No newline at end of file diff --git a/pkg/cbox/share/sql/sql.go b/pkg/cbox/share/sql/sql.go index 7ab01b26d7..0987aa711b 100644 --- a/pkg/cbox/share/sql/sql.go +++ b/pkg/cbox/share/sql/sql.go @@ -531,7 +531,7 @@ func translateFilters(filters []*collaboration.Filter) (string, []interface{}, e case collaboration.Filter_TYPE_RESOURCE_ID: filterQuery += "(" for i, f := range filters { - filterQuery += "(fileid_prefix =? AND item_source=?" + filterQuery += "(fileid_prefix =? AND item_source=?)" params = append(params, f.GetResourceId().StorageId, f.GetResourceId().OpaqueId) if i != len(filters)-1 { From c938d17a35e3d554fd6cfc12e6a602ca51516b1d Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Tue, 12 Oct 2021 11:33:53 +0200 Subject: [PATCH 3/7] Restrict EOS project spaces sharing permissions to admins and writers --- .../eos-projects-share-perm.md | 3 ++ .../grpc/services/gateway/storageprovider.go | 8 ++-- pkg/cbox/storage/eoswrapper/eoswrapper.go | 44 ++++++++++++++++++- pkg/storage/utils/grants/grants.go | 2 +- 4 files changed, 51 insertions(+), 6 deletions(-) create mode 100644 changelog/1.14.0_2021-10-12/eos-projects-share-perm.md diff --git a/changelog/1.14.0_2021-10-12/eos-projects-share-perm.md b/changelog/1.14.0_2021-10-12/eos-projects-share-perm.md new file mode 100644 index 0000000000..41605fb6c0 --- /dev/null +++ b/changelog/1.14.0_2021-10-12/eos-projects-share-perm.md @@ -0,0 +1,3 @@ +Bugfix: Restrict EOS project spaces sharing permissions to admins and writers + +https://github.com/cs3org/reva/pull/2153 \ No newline at end of file diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index 974349cac9..249aac85e6 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -28,16 +28,16 @@ import ( "sync" "time" - rtrace "github.com/cs3org/reva/pkg/trace" "google.golang.org/protobuf/types/known/fieldmaskpb" - collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" - types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" - gateway "github.com/cs3org/go-cs3apis/cs3/gateway/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" registry "github.com/cs3org/go-cs3apis/cs3/storage/registry/v1beta1" + types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + rtrace "github.com/cs3org/reva/pkg/trace" + "github.com/cs3org/reva/pkg/appctx" "github.com/cs3org/reva/pkg/errtypes" "github.com/cs3org/reva/pkg/rgrpc/status" diff --git a/pkg/cbox/storage/eoswrapper/eoswrapper.go b/pkg/cbox/storage/eoswrapper/eoswrapper.go index 2f27c42230..018d88b886 100644 --- a/pkg/cbox/storage/eoswrapper/eoswrapper.go +++ b/pkg/cbox/storage/eoswrapper/eoswrapper.go @@ -21,10 +21,12 @@ package eoshome import ( "bytes" "context" + "strings" "text/template" "github.com/Masterminds/sprig" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + ctxpkg "github.com/cs3org/reva/pkg/ctx" "github.com/cs3org/reva/pkg/storage" "github.com/cs3org/reva/pkg/storage/fs/registry" "github.com/cs3org/reva/pkg/storage/utils/eosfs" @@ -36,8 +38,18 @@ func init() { registry.Register("eoswrapper", New) } +const ( + eosProjectsNamespace = "/eos/project" + + // We can use a regex for these, but that might have inferior performance + projectSpaceGroupsPrefix = "cernbox-project-" + projectSpaceAdminGroups = "-admins" + projectSpaceWriterGroups = "-writers" +) + type wrapper struct { storage.FS + config *eosfs.Config mountIDTemplate *template.Template } @@ -79,7 +91,7 @@ func New(m map[string]interface{}) (storage.FS, error) { return nil, err } - return &wrapper{FS: eos, mountIDTemplate: mountIDTemplate}, nil + return &wrapper{FS: eos, config: c, mountIDTemplate: mountIDTemplate}, nil } // We need to override the two methods, GetMD and ListFolder to fill the @@ -96,6 +108,11 @@ func (w *wrapper) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []s // Take the first letter of the resource path after the namespace has been removed. // If it's empty, leave it empty to be filled by storageprovider. res.Id.StorageId = w.getMountID(ctx, res) + + if err = w.setProjectSharingPermissions(ctx, res); err != nil { + return nil, err + } + return res, nil } @@ -107,6 +124,9 @@ func (w *wrapper) ListFolder(ctx context.Context, ref *provider.Reference, mdKey } for _, r := range res { r.Id.StorageId = w.getMountID(ctx, r) + if err = w.setProjectSharingPermissions(ctx, r); err != nil { + continue + } } return res, nil } @@ -121,3 +141,25 @@ func (w *wrapper) getMountID(ctx context.Context, r *provider.ResourceInfo) stri } return b.String() } + +func (w *wrapper) setProjectSharingPermissions(ctx context.Context, r *provider.ResourceInfo) error { + if strings.HasPrefix(w.config.Namespace, eosProjectsNamespace) { + var userHasSharingAccess bool + user := ctxpkg.ContextMustGetUser(ctx) + + for _, g := range user.Groups { + // Check if user is present in the admins or writers groups + if strings.HasPrefix(g, projectSpaceGroupsPrefix) && (strings.HasSuffix(g, projectSpaceAdminGroups) || strings.HasSuffix(g, projectSpaceWriterGroups)) { + userHasSharingAccess = true + break + } + } + + if userHasSharingAccess { + r.PermissionSet.AddGrant = true + r.PermissionSet.RemoveGrant = true + r.PermissionSet.UpdateGrant = true + } + } + return nil +} diff --git a/pkg/storage/utils/grants/grants.go b/pkg/storage/utils/grants/grants.go index aa65e83c89..32f18a8d8a 100644 --- a/pkg/storage/utils/grants/grants.go +++ b/pkg/storage/utils/grants/grants.go @@ -96,7 +96,7 @@ func GetGrantPermissionSet(perm string, isDir bool) *provider.ResourcePermission rp.Delete = false } - if strings.Contains(perm, "w") && strings.Contains(perm, "x") && !strings.Contains(perm, "!m") { + if strings.Contains(perm, "m") && !strings.Contains(perm, "!m") { rp.AddGrant = true rp.ListGrants = true rp.RemoveGrant = true From 502c09ca2dfff67bb640b2e4bcddcf653ab0ae23 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Tue, 12 Oct 2021 12:02:17 +0200 Subject: [PATCH 4/7] Restrict sharing to only admins --- pkg/cbox/storage/eoswrapper/eoswrapper.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/cbox/storage/eoswrapper/eoswrapper.go b/pkg/cbox/storage/eoswrapper/eoswrapper.go index 018d88b886..b07b81cf26 100644 --- a/pkg/cbox/storage/eoswrapper/eoswrapper.go +++ b/pkg/cbox/storage/eoswrapper/eoswrapper.go @@ -44,7 +44,6 @@ const ( // We can use a regex for these, but that might have inferior performance projectSpaceGroupsPrefix = "cernbox-project-" projectSpaceAdminGroups = "-admins" - projectSpaceWriterGroups = "-writers" ) type wrapper struct { @@ -148,8 +147,8 @@ func (w *wrapper) setProjectSharingPermissions(ctx context.Context, r *provider. user := ctxpkg.ContextMustGetUser(ctx) for _, g := range user.Groups { - // Check if user is present in the admins or writers groups - if strings.HasPrefix(g, projectSpaceGroupsPrefix) && (strings.HasSuffix(g, projectSpaceAdminGroups) || strings.HasSuffix(g, projectSpaceWriterGroups)) { + // Check if user is present in the admins groups + if strings.HasPrefix(g, projectSpaceGroupsPrefix) && strings.HasSuffix(g, projectSpaceAdminGroups) { userHasSharingAccess = true break } From c85b55b6a8920751b34cadcc5c77b9b423e115f3 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Tue, 12 Oct 2021 18:01:22 +0200 Subject: [PATCH 5/7] Check if user belongs to admin group for the specific project --- pkg/cbox/storage/eoswrapper/eoswrapper.go | 24 +++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/pkg/cbox/storage/eoswrapper/eoswrapper.go b/pkg/cbox/storage/eoswrapper/eoswrapper.go index b07b81cf26..a9a6e387ff 100644 --- a/pkg/cbox/storage/eoswrapper/eoswrapper.go +++ b/pkg/cbox/storage/eoswrapper/eoswrapper.go @@ -27,6 +27,7 @@ import ( "github.com/Masterminds/sprig" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" ctxpkg "github.com/cs3org/reva/pkg/ctx" + "github.com/cs3org/reva/pkg/errtypes" "github.com/cs3org/reva/pkg/storage" "github.com/cs3org/reva/pkg/storage/fs/registry" "github.com/cs3org/reva/pkg/storage/utils/eosfs" @@ -39,16 +40,15 @@ func init() { } const ( - eosProjectsNamespace = "/eos/project" + eosProjectsNamespace = "/eos/project/" // We can use a regex for these, but that might have inferior performance - projectSpaceGroupsPrefix = "cernbox-project-" - projectSpaceAdminGroups = "-admins" + projectSpaceGroupsPrefix = "cernbox-project-" + projectSpaceAdminGroupsSuffix = "-admins" ) type wrapper struct { storage.FS - config *eosfs.Config mountIDTemplate *template.Template } @@ -90,7 +90,7 @@ func New(m map[string]interface{}) (storage.FS, error) { return nil, err } - return &wrapper{FS: eos, config: c, mountIDTemplate: mountIDTemplate}, nil + return &wrapper{FS: eos, mountIDTemplate: mountIDTemplate}, nil } // We need to override the two methods, GetMD and ListFolder to fill the @@ -142,13 +142,21 @@ func (w *wrapper) getMountID(ctx context.Context, r *provider.ResourceInfo) stri } func (w *wrapper) setProjectSharingPermissions(ctx context.Context, r *provider.ResourceInfo) error { - if strings.HasPrefix(w.config.Namespace, eosProjectsNamespace) { + if strings.HasPrefix(r.Path, eosProjectsNamespace) { + + // Extract project name from the path resembling /eos/project/c/cernbox/minutes/.. + path := strings.TrimPrefix(r.Path, eosProjectsNamespace) + parts := strings.SplitN(path, "/", 3) + if len(parts) != 3 { + return errtypes.BadRequest("eoswrapper: path does not follow the allowed format") + } + adminGroup := projectSpaceGroupsPrefix + parts[1] + projectSpaceAdminGroupsSuffix + var userHasSharingAccess bool user := ctxpkg.ContextMustGetUser(ctx) for _, g := range user.Groups { - // Check if user is present in the admins groups - if strings.HasPrefix(g, projectSpaceGroupsPrefix) && strings.HasSuffix(g, projectSpaceAdminGroups) { + if g == adminGroup { userHasSharingAccess = true break } From 453aa705d1b6f7c8ae88f9e519de84ba6f136d24 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Tue, 12 Oct 2021 18:04:06 +0200 Subject: [PATCH 6/7] Fix changelog --- changelog/1.14.0_2021-10-12/grants-share-perms.md | 3 --- .../eos-projects-share-perm.md | 0 2 files changed, 3 deletions(-) delete mode 100644 changelog/1.14.0_2021-10-12/grants-share-perms.md rename changelog/{1.14.0_2021-10-12 => unreleased}/eos-projects-share-perm.md (100%) diff --git a/changelog/1.14.0_2021-10-12/grants-share-perms.md b/changelog/1.14.0_2021-10-12/grants-share-perms.md deleted file mode 100644 index 3771706ea3..0000000000 --- a/changelog/1.14.0_2021-10-12/grants-share-perms.md +++ /dev/null @@ -1,3 +0,0 @@ -Bugfix: Update checks for setting sharing permissions from EOS grants - -https://github.com/cs3org/reva/pull/2153 \ No newline at end of file diff --git a/changelog/1.14.0_2021-10-12/eos-projects-share-perm.md b/changelog/unreleased/eos-projects-share-perm.md similarity index 100% rename from changelog/1.14.0_2021-10-12/eos-projects-share-perm.md rename to changelog/unreleased/eos-projects-share-perm.md From d8fbb7f88b60249c2f7d5d62685baa81e424325b Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Wed, 13 Oct 2021 09:07:17 +0200 Subject: [PATCH 7/7] Return early if we found admin group --- pkg/cbox/storage/eoswrapper/eoswrapper.go | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/pkg/cbox/storage/eoswrapper/eoswrapper.go b/pkg/cbox/storage/eoswrapper/eoswrapper.go index a9a6e387ff..9f7bfa0fb2 100644 --- a/pkg/cbox/storage/eoswrapper/eoswrapper.go +++ b/pkg/cbox/storage/eoswrapper/eoswrapper.go @@ -151,22 +151,17 @@ func (w *wrapper) setProjectSharingPermissions(ctx context.Context, r *provider. return errtypes.BadRequest("eoswrapper: path does not follow the allowed format") } adminGroup := projectSpaceGroupsPrefix + parts[1] + projectSpaceAdminGroupsSuffix - - var userHasSharingAccess bool user := ctxpkg.ContextMustGetUser(ctx) for _, g := range user.Groups { if g == adminGroup { - userHasSharingAccess = true - break + r.PermissionSet.AddGrant = true + r.PermissionSet.RemoveGrant = true + r.PermissionSet.UpdateGrant = true + r.PermissionSet.ListGrants = true + return nil } } - - if userHasSharingAccess { - r.PermissionSet.AddGrant = true - r.PermissionSet.RemoveGrant = true - r.PermissionSet.UpdateGrant = true - } } return nil }