From 47985e6d77e7eebbbb1dfb1557ea968db83f7a05 Mon Sep 17 00:00:00 2001 From: Hugo Gonzalez Labrador Date: Mon, 2 Aug 2021 12:36:58 +0200 Subject: [PATCH] [cbox-commit-1] Expose capability to deny access in OCS API --- changelog/unreleased/deny-role.md | 3 + .../owncloud/ocs/conversions/permissions.go | 19 +++-- .../ocs/conversions/permissions_test.go | 13 ++-- .../services/owncloud/ocs/conversions/role.go | 73 ++++++++++++++++--- .../handlers/apps/sharing/shares/shares.go | 12 +-- 5 files changed, 93 insertions(+), 27 deletions(-) create mode 100644 changelog/unreleased/deny-role.md diff --git a/changelog/unreleased/deny-role.md b/changelog/unreleased/deny-role.md new file mode 100644 index 00000000000..c050c55da24 --- /dev/null +++ b/changelog/unreleased/deny-role.md @@ -0,0 +1,3 @@ +Enhancement: add support for denying access in OCS layer + +http://github.com/cs3org/reva/pull/1949 diff --git a/internal/http/services/owncloud/ocs/conversions/permissions.go b/internal/http/services/owncloud/ocs/conversions/permissions.go index 4d860fb6943..243dd417fd3 100644 --- a/internal/http/services/owncloud/ocs/conversions/permissions.go +++ b/internal/http/services/owncloud/ocs/conversions/permissions.go @@ -26,7 +26,7 @@ import ( type Permissions uint const ( - // PermissionInvalid grants no permissions on a resource + // PermissionInvalid represents an invalid permission PermissionInvalid Permissions = 0 // PermissionRead grants read permissions on a resource PermissionRead Permissions = 1 << (iota - 1) @@ -38,21 +38,30 @@ const ( PermissionDelete // PermissionShare grants share permissions on a resource PermissionShare + // PermissionDeny grants permissions to deny access on a resource + // The recipient of the resource will then have PermissionNone. + PermissionDeny + // PermissionNone grants no permissions on a resource + PermissionNone + // PermissionMax is to be used within value range checks + PermissionMax Permissions = (1 << (iota - 1)) - 1 // PermissionAll grants all permissions on a resource - PermissionAll Permissions = (1 << (iota - 1)) - 1 + PermissionAll = PermissionMax - PermissionNone + // PermissionMin is to be used within value range checks + PermissionMin = PermissionRead ) var ( // ErrPermissionNotInRange defines a permission specific error. - ErrPermissionNotInRange = fmt.Errorf("The provided permission is not between %d and %d", PermissionInvalid, PermissionAll) + ErrPermissionNotInRange = fmt.Errorf("The provided permission is not between %d and %d", PermissionMin, PermissionMax) ) // NewPermissions creates a new Permissions instance. // The value must be in the valid range. func NewPermissions(val int) (Permissions, error) { if val == int(PermissionInvalid) { - return PermissionInvalid, fmt.Errorf("permissions %d out of range %d - %d", val, PermissionRead, PermissionAll) - } else if val < int(PermissionInvalid) || int(PermissionAll) < val { + return PermissionInvalid, fmt.Errorf("permissions %d out of range %d - %d", val, PermissionMin, PermissionMax) + } else if val < int(PermissionInvalid) || int(PermissionMax) < val { return PermissionInvalid, ErrPermissionNotInRange } return Permissions(val), nil diff --git a/internal/http/services/owncloud/ocs/conversions/permissions_test.go b/internal/http/services/owncloud/ocs/conversions/permissions_test.go index 6604b8deb9e..7d6c112733a 100644 --- a/internal/http/services/owncloud/ocs/conversions/permissions_test.go +++ b/internal/http/services/owncloud/ocs/conversions/permissions_test.go @@ -23,7 +23,7 @@ import ( ) func TestNewPermissions(t *testing.T) { - for val := int(PermissionRead); val <= int(PermissionAll); val++ { + for val := int(PermissionMin); val <= int(PermissionMax); val++ { _, err := NewPermissions(val) if err != nil { t.Errorf("value %d should be a valid permissions", val) @@ -35,7 +35,7 @@ func TestNewPermissionsWithInvalidValueShouldFail(t *testing.T) { vals := []int{ int(PermissionInvalid), -1, - int(PermissionAll) + 1, + int(PermissionMax) + 1, } for _, v := range vals { _, err := NewPermissions(v) @@ -52,10 +52,10 @@ func TestContainPermissionAll(t *testing.T) { 4: PermissionCreate, 8: PermissionDelete, 16: PermissionShare, - 31: PermissionAll, + 63: PermissionAll, } - p, _ := NewPermissions(31) // all permissions should contain all other permissions + p, _ := NewPermissions(63) // all permissions should contain all other permissions for _, value := range table { if !p.Contain(value) { t.Errorf("permissions %d should contain %d", p, value) @@ -68,7 +68,7 @@ func TestContainPermissionRead(t *testing.T) { 4: PermissionCreate, 8: PermissionDelete, 16: PermissionShare, - 31: PermissionAll, + 63: PermissionAll, } p, _ := NewPermissions(1) // read permission should not contain any other permissions @@ -145,10 +145,11 @@ func TestPermissions2Role(t *testing.T) { table := map[Permissions]string{ PermissionRead: RoleViewer, PermissionRead | PermissionWrite | PermissionCreate | PermissionDelete: RoleEditor, - PermissionAll: RoleCoowner, + PermissionAll: RoleCollaborator, PermissionWrite: RoleLegacy, PermissionShare: RoleLegacy, PermissionWrite | PermissionShare: RoleLegacy, + PermissionNone: RoleDenied, } for permissions, role := range table { diff --git a/internal/http/services/owncloud/ocs/conversions/role.go b/internal/http/services/owncloud/ocs/conversions/role.go index f04bd7cabdd..b1f91898a42 100644 --- a/internal/http/services/owncloud/ocs/conversions/role.go +++ b/internal/http/services/owncloud/ocs/conversions/role.go @@ -36,12 +36,14 @@ type Role struct { const ( // RoleViewer grants non-editor role on a resource. RoleViewer = "viewer" + // RoleReader grants non-editor role on a resource + RoleReader = "reader" // RoleEditor grants editor permission on a resource, including folders. RoleEditor = "editor" // RoleFileEditor grants editor permission on a single file. RoleFileEditor = "file-editor" - // RoleCoowner grants co-owner permissions on a resource. - RoleCoowner = "coowner" + // RoleCollaborator grants editor+resharing permissions on a resource. + RoleCollaborator = "coowner" // RoleUploader grants uploader permission to upload onto a resource. RoleUploader = "uploader" // RoleManager grants manager permissions on a resource. Semantically equivalent to co-owner. @@ -51,6 +53,8 @@ const ( RoleUnknown = "unknown" // RoleLegacy provides backwards compatibility. RoleLegacy = "legacy" + // RoleDenied grants no permission at all on a resource + RoleDenied = "denied" ) // CS3ResourcePermissions for the role @@ -92,6 +96,7 @@ func (r *Role) OCSPermissions() Permissions { // S = Shared // R = Shareable // M = Mounted +// Z = Deniable (NEW) func (r *Role) WebDAVPermissions(isDir, isShared, isMountpoint, isPublic bool) string { var b strings.Builder if !isPublic && isShared { @@ -115,20 +120,29 @@ func (r *Role) WebDAVPermissions(isDir, isShared, isMountpoint, isPublic bool) s if isDir && r.ocsPermissions.Contain(PermissionCreate) { fmt.Fprintf(&b, "CK") } + + if r.ocsPermissions.Contain(PermissionDeny) { + fmt.Fprintf(&b, "Z") + } + return b.String() } // RoleFromName creates a role from the name func RoleFromName(name string) *Role { switch name { + case RoleDenied: + return NewDeniedRole() case RoleViewer: return NewViewerRole() + case RoleReader: + return NewReaderRole() case RoleEditor: return NewEditorRole() case RoleFileEditor: return NewFileEditorRole() - case RoleCoowner: - return NewCoownerRole() + case RoleCollaborator: + return NewCollaboratorRole() case RoleUploader: return NewUploaderRole() case RoleManager: @@ -147,6 +161,15 @@ func NewUnknownRole() *Role { } } +// NewDeniedRole creates a fully denied role +func NewDeniedRole() *Role { + return &Role{ + Name: RoleDenied, + cS3ResourcePermissions: &provider.ResourcePermissions{}, + ocsPermissions: PermissionNone, + } +} + // NewViewerRole creates a viewer role func NewViewerRole() *Role { return &Role{ @@ -165,6 +188,25 @@ func NewViewerRole() *Role { } } +// NewReaderRole creates a reader role +func NewReaderRole() *Role { + return &Role{ + Name: RoleViewer, + cS3ResourcePermissions: &provider.ResourcePermissions{ + // read + GetPath: true, + GetQuota: true, + InitiateFileDownload: true, + ListGrants: true, + ListContainer: true, + ListFileVersions: true, + ListRecycle: true, + Stat: true, + }, + ocsPermissions: PermissionRead, + } +} + // NewEditorRole creates an editor role func NewEditorRole() *Role { return &Role{ @@ -211,10 +253,10 @@ func NewFileEditorRole() *Role { } } -// NewCoownerRole creates a coowner role -func NewCoownerRole() *Role { +// NewCollaboratorRole creates a collaborator role +func NewCollaboratorRole() *Role { return &Role{ - Name: RoleCoowner, + Name: RoleCollaborator, cS3ResourcePermissions: &provider.ResourcePermissions{ GetPath: true, GetQuota: true, @@ -289,7 +331,7 @@ func RoleFromOCSPermissions(p Permissions) *Role { if p.Contain(PermissionRead) { if p.Contain(PermissionWrite) && p.Contain(PermissionCreate) && p.Contain(PermissionDelete) { if p.Contain(PermissionShare) { - return NewCoownerRole() + return NewCollaboratorRole() } return NewEditorRole() } @@ -300,6 +342,9 @@ func RoleFromOCSPermissions(p Permissions) *Role { if p == PermissionCreate { return NewUploaderRole() } + if p == PermissionNone { + return NewDeniedRole() + } // legacy return NewLegacyRoleFromOCSPermissions(p) } @@ -390,13 +435,17 @@ func RoleFromResourcePermissions(rp *provider.ResourcePermissions) *Role { rp.UpdateGrant { r.ocsPermissions |= PermissionShare } + if rp.DenyGrant { + r.ocsPermissions |= PermissionDeny + } + if r.ocsPermissions.Contain(PermissionRead) { if r.ocsPermissions.Contain(PermissionWrite) && r.ocsPermissions.Contain(PermissionCreate) && r.ocsPermissions.Contain(PermissionDelete) { r.Name = RoleEditor if r.ocsPermissions.Contain(PermissionShare) { - r.Name = RoleCoowner + r.Name = RoleCollaborator } - return r // editor or coowner + return r // editor or collaborator } if r.ocsPermissions == PermissionRead { r.Name = RoleViewer @@ -407,6 +456,10 @@ func RoleFromResourcePermissions(rp *provider.ResourcePermissions) *Role { r.Name = RoleUploader return r } + if r.ocsPermissions == PermissionNone { + r.Name = RoleDenied + return r + } r.Name = RoleLegacy // at this point other ocs permissions may have been mapped. // TODO what about even more granular cs3 permissions?, eg. only stat diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go index afc3252811d..a4e1574393f 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go @@ -166,23 +166,23 @@ func (h *Handler) CreateShare(w http.ResponseWriter, r *http.Request) { switch shareType { case int(conversions.ShareTypeUser): - // user collaborations default to coowner - if role, val, err := h.extractPermissions(w, r, statRes.Info, conversions.NewCoownerRole()); err == nil { + // user collaborations default to collab + if role, val, err := h.extractPermissions(w, r, statRes.Info, conversions.NewCollaboratorRole()); err == nil { h.createUserShare(w, r, statRes.Info, role, val) } case int(conversions.ShareTypeGroup): - // group collaborations default to coowner - if role, val, err := h.extractPermissions(w, r, statRes.Info, conversions.NewCoownerRole()); err == nil { + // group collaborations default to collab + if role, val, err := h.extractPermissions(w, r, statRes.Info, conversions.NewCollaboratorRole()); err == nil { h.createGroupShare(w, r, statRes.Info, role, val) } case int(conversions.ShareTypePublicLink): // public links default to read only - if _, _, err := h.extractPermissions(w, r, statRes.Info, conversions.NewViewerRole()); err == nil { + if _, _, err := h.extractPermissions(w, r, statRes.Info, conversions.NewReaderRole()); err == nil { h.createPublicLinkShare(w, r, statRes.Info) } case int(conversions.ShareTypeFederatedCloudShare): // federated shares default to read only - if role, val, err := h.extractPermissions(w, r, statRes.Info, conversions.NewViewerRole()); err == nil { + if role, val, err := h.extractPermissions(w, r, statRes.Info, conversions.NewReaderRole()); err == nil { h.createFederatedCloudShare(w, r, statRes.Info, role, val) } default: