From f953398b0ab82de28cf925d4c7e4088f968b0fcf Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Wed, 5 Jan 2022 10:11:44 +0100 Subject: [PATCH] Fixes for apps in public shares, project spaces for EOS driver (#2371) --- changelog/unreleased/eos-fixes.md | 3 ++ .../publicstorageprovider.go | 6 ++- internal/http/interceptors/auth/auth.go | 15 ++++--- pkg/app/provider/wopi/wopi.go | 14 +++++- pkg/auth/manager/publicshares/publicshares.go | 14 +++++- pkg/auth/scope/publicshare.go | 10 ++++- pkg/auth/scope/resourceinfo.go | 10 ++++- pkg/cbox/favorite/sql/sql.go | 2 +- pkg/cbox/storage/eoswrapper/eoswrapper.go | 1 + pkg/cbox/utils/conversions.go | 15 ++++++- pkg/eosclient/eosbinary/eosbinary.go | 12 +++-- pkg/eosclient/eosclient.go | 2 +- pkg/eosclient/eosgrpc/eosgrpc.go | 3 +- pkg/storage/utils/eosfs/eosfs.go | 45 +++++++++++++++++-- pkg/storage/utils/grants/grants.go | 10 ++--- pkg/storage/utils/localfs/localfs.go | 2 +- 16 files changed, 128 insertions(+), 36 deletions(-) create mode 100644 changelog/unreleased/eos-fixes.md diff --git a/changelog/unreleased/eos-fixes.md b/changelog/unreleased/eos-fixes.md new file mode 100644 index 0000000000..e0d0cbac7a --- /dev/null +++ b/changelog/unreleased/eos-fixes.md @@ -0,0 +1,3 @@ +Bugfix: Fixes for apps in public shares, project spaces for EOS driver + +https://github.com/cs3org/reva/pull/2370 \ No newline at end of file diff --git a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go index 9aa3aed324..9bdc72057c 100644 --- a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go +++ b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go @@ -596,8 +596,10 @@ func (s *service) augmentStatResponse(ctx context.Context, res *provider.StatRes } func (s *service) setPublicStorageID(info *provider.ResourceInfo, shareToken string) { - info.Id.StorageId = s.mountID - info.Id.OpaqueId = shareToken + "/" + info.Id.OpaqueId + if s.mountID != "" { + info.Id.StorageId = s.mountID + info.Id.OpaqueId = shareToken + "/" + info.Id.OpaqueId + } } func addShare(i *provider.ResourceInfo, ls *link.PublicShare) error { diff --git a/internal/http/interceptors/auth/auth.go b/internal/http/interceptors/auth/auth.go index 42988a0973..4e5a07c80e 100644 --- a/internal/http/interceptors/auth/auth.go +++ b/internal/http/interceptors/auth/auth.go @@ -21,6 +21,7 @@ package auth import ( "fmt" "net/http" + "strings" "time" "github.com/bluele/gcache" @@ -297,14 +298,16 @@ func getCredsForUserAgent(ua string, uam map[string]string, creds []string) []st return creds } - cred, ok := uam[ua] - if ok { - for _, v := range creds { - if v == cred { - return []string{cred} + for u, cred := range uam { + if strings.Contains(ua, u) { + for _, v := range creds { + if v == cred { + return []string{cred} + } } + return creds + } - return creds } return creds diff --git a/pkg/app/provider/wopi/wopi.go b/pkg/app/provider/wopi/wopi.go index 77e49817e4..5f2a38add1 100644 --- a/pkg/app/provider/wopi/wopi.go +++ b/pkg/app/provider/wopi/wopi.go @@ -136,10 +136,20 @@ func (p *wopiProvider) GetAppURL(ctx context.Context, resource *provider.Resourc q.Add("fileid", resource.GetId().OpaqueId) q.Add("endpoint", resource.GetId().StorageId) q.Add("viewmode", viewMode.String()) + u, ok := ctxpkg.ContextGetUser(ctx) if ok { // else defaults to "Guest xyz" - q.Add("username", u.Username) - q.Add("userid", u.Id.OpaqueId+"@"+u.Id.Idp) + var isPublicShare bool + if u.Opaque != nil { + if _, ok := u.Opaque.Map["public-share-role"]; ok { + isPublicShare = true + } + } + + if !isPublicShare { + q.Add("username", u.Username) + q.Add("userid", u.Id.OpaqueId+"@"+u.Id.Idp) + } } q.Add("appname", p.conf.AppName) diff --git a/pkg/auth/manager/publicshares/publicshares.go b/pkg/auth/manager/publicshares/publicshares.go index c00f37f996..0dc4de28be 100644 --- a/pkg/auth/manager/publicshares/publicshares.go +++ b/pkg/auth/manager/publicshares/publicshares.go @@ -136,15 +136,27 @@ func (m *manager) Authenticate(ctx context.Context, token, secret string) (*user share := publicShareResponse.GetShare() role := authpb.Role_ROLE_VIEWER + roleStr := "viewer" if share.Permissions.Permissions.InitiateFileUpload { role = authpb.Role_ROLE_EDITOR + roleStr = "editor" } scope, err := scope.AddPublicShareScope(share, role, nil) if err != nil { return nil, nil, err } - return getUserResponse.GetUser(), scope, nil + u := getUserResponse.GetUser() + u.Opaque = &types.Opaque{ + Map: map[string]*types.OpaqueEntry{ + "public-share-role": { + Decoder: "plain", + Value: []byte(roleStr), + }, + }, + } + + return u, scope, nil } // ErrPasswordNotProvided is returned when the public share is password protected, but there was no password on the request diff --git a/pkg/auth/scope/publicshare.go b/pkg/auth/scope/publicshare.go index 7de007ca87..bfa538040f 100644 --- a/pkg/auth/scope/publicshare.go +++ b/pkg/auth/scope/publicshare.go @@ -23,8 +23,10 @@ import ( "fmt" "strings" + appprovider "github.com/cs3org/go-cs3apis/cs3/app/provider/v1beta1" appregistry "github.com/cs3org/go-cs3apis/cs3/app/registry/v1beta1" authpb "github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1" + gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" @@ -52,9 +54,13 @@ func publicshareScope(ctx context.Context, scope *authpb.Scope, resource interfa return checkStorageRef(ctx, &share, v.GetRef()), nil case *provider.InitiateFileDownloadRequest: return checkStorageRef(ctx, &share, v.GetRef()), nil + case *appprovider.OpenInAppRequest: + return checkStorageRef(ctx, &share, &provider.Reference{ResourceId: v.ResourceInfo.Id}), nil + case *gateway.OpenInAppRequest: + return checkStorageRef(ctx, &share, v.GetRef()), nil - // Editor role - // need to return appropriate status codes in the ocs/ocdav layers. + // Editor role + // need to return appropriate status codes in the ocs/ocdav layers. case *provider.CreateContainerRequest: return hasRoleEditor(*scope) && checkStorageRef(ctx, &share, v.GetRef()), nil case *provider.TouchFileRequest: diff --git a/pkg/auth/scope/resourceinfo.go b/pkg/auth/scope/resourceinfo.go index d2ad4240fc..2066ce5667 100644 --- a/pkg/auth/scope/resourceinfo.go +++ b/pkg/auth/scope/resourceinfo.go @@ -23,7 +23,9 @@ import ( "fmt" "strings" + appprovider "github.com/cs3org/go-cs3apis/cs3/app/provider/v1beta1" authpb "github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1" + gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" registry "github.com/cs3org/go-cs3apis/cs3/storage/registry/v1beta1" "github.com/rs/zerolog" @@ -50,9 +52,13 @@ func resourceinfoScope(_ context.Context, scope *authpb.Scope, resource interfac return checkResourceInfo(&r, v.GetRef()), nil case *provider.InitiateFileDownloadRequest: return checkResourceInfo(&r, v.GetRef()), nil + case *appprovider.OpenInAppRequest: + return checkResourceInfo(&r, &provider.Reference{ResourceId: v.ResourceInfo.Id}), nil + case *gateway.OpenInAppRequest: + return checkResourceInfo(&r, v.GetRef()), nil - // Editor role - // need to return appropriate status codes in the ocs/ocdav layers. + // Editor role + // need to return appropriate status codes in the ocs/ocdav layers. case *provider.CreateContainerRequest: return hasRoleEditor(*scope) && checkResourceInfo(&r, v.GetRef()), nil case *provider.TouchFileRequest: diff --git a/pkg/cbox/favorite/sql/sql.go b/pkg/cbox/favorite/sql/sql.go index 3a49b8c99c..ebb6c85ec9 100644 --- a/pkg/cbox/favorite/sql/sql.go +++ b/pkg/cbox/favorite/sql/sql.go @@ -97,7 +97,7 @@ func (m *mgr) SetFavorite(ctx context.Context, userID *user.UserId, resourceInfo // The primary key is just the ID in the table, it should ideally be (uid, fileid_prefix, fileid, tag_key) // For the time being, just check if the favorite already exists. If it does, return early var id int - query := `"SELECT id FROM cbox_metadata WHERE uid=? AND fileid_prefix=? AND fileid=? AND tag_key="fav"` + query := `SELECT id FROM cbox_metadata WHERE uid=? AND fileid_prefix=? AND fileid=? AND tag_key="fav"` if err := m.db.QueryRow(query, user.Id.OpaqueId, resourceInfo.Id.StorageId, resourceInfo.Id.OpaqueId).Scan(&id); err == nil { // Favorite is already set, return return nil diff --git a/pkg/cbox/storage/eoswrapper/eoswrapper.go b/pkg/cbox/storage/eoswrapper/eoswrapper.go index bc55aa30c7..bc4467ba5d 100644 --- a/pkg/cbox/storage/eoswrapper/eoswrapper.go +++ b/pkg/cbox/storage/eoswrapper/eoswrapper.go @@ -166,6 +166,7 @@ func (w *wrapper) setProjectSharingPermissions(ctx context.Context, r *provider. r.PermissionSet.RemoveGrant = true r.PermissionSet.UpdateGrant = true r.PermissionSet.ListGrants = true + r.PermissionSet.GetQuota = true return nil } } diff --git a/pkg/cbox/utils/conversions.go b/pkg/cbox/utils/conversions.go index 787880ec89..1391df675a 100644 --- a/pkg/cbox/utils/conversions.go +++ b/pkg/cbox/utils/conversions.go @@ -113,9 +113,12 @@ func ResourceTypeToItemInt(r provider.ResourceType) int { // SharePermToInt maps read/write permissions to an integer func SharePermToInt(p *provider.ResourcePermissions) int { var perm int - if p.CreateContainer || p.InitiateFileUpload { + switch { + case p.InitiateFileUpload && !p.InitiateFileDownload: + perm = 4 + case p.InitiateFileUpload: perm = 15 - } else if p.ListContainer || p.InitiateFileDownload { + case p.InitiateFileDownload: perm = 1 } // TODO map denials and resharing; currently, denials are mapped to 0 @@ -158,6 +161,14 @@ func IntTosharePerm(p int, itemType string) *provider.ResourcePermissions { perm.PurgeRecycle = true } return perm + case 4: + return &provider.ResourcePermissions{ + Stat: true, + ListContainer: true, + GetPath: true, + CreateContainer: true, + InitiateFileUpload: true, + } default: // TODO we may have other options, for now this is a denial return &provider.ResourcePermissions{} diff --git a/pkg/eosclient/eosbinary/eosbinary.go b/pkg/eosclient/eosbinary/eosbinary.go index b9c1f6a5a4..91b45dd037 100644 --- a/pkg/eosclient/eosbinary/eosbinary.go +++ b/pkg/eosclient/eosbinary/eosbinary.go @@ -535,12 +535,16 @@ func (c *Client) SetAttr(ctx context.Context, auth eosclient.Authorization, attr } // UnsetAttr unsets an extended attribute on a path. -func (c *Client) UnsetAttr(ctx context.Context, auth eosclient.Authorization, attr *eosclient.Attribute, path string) error { +func (c *Client) UnsetAttr(ctx context.Context, auth eosclient.Authorization, attr *eosclient.Attribute, recursive bool, path string) error { if !isValidAttribute(attr) { return errors.New("eos: attr is invalid: " + serializeAttribute(attr)) } - - args := []string{"attr", "-r", "rm", fmt.Sprintf("%d.%s", attr.Type, attr.Key), path} + var args []string + if recursive { + args = []string{"attr", "-r", "rm", fmt.Sprintf("%s.%s", attrTypeToString(attr.Type), attr.Key), path} + } else { + args = []string{"attr", "rm", fmt.Sprintf("%s.%s", attrTypeToString(attr.Type), attr.Key), path} + } _, _, err := c.executeEOS(ctx, args, auth) if err != nil { return err @@ -911,7 +915,7 @@ func (c *Client) parseFind(ctx context.Context, auth eosclient.Authorization, di for _, fi := range finfos { // For files, inherit ACLs from the parent // And set the inode to that of their version folder - if !fi.IsDir { + if !fi.IsDir && !isVersionFolder(dirPath) { if parent != nil { fi.SysACL.Entries = append(fi.SysACL.Entries, parent.SysACL.Entries...) } diff --git a/pkg/eosclient/eosclient.go b/pkg/eosclient/eosclient.go index c4e47e7b23..5e52854c78 100644 --- a/pkg/eosclient/eosclient.go +++ b/pkg/eosclient/eosclient.go @@ -36,7 +36,7 @@ type EOSClient interface { GetFileInfoByFXID(ctx context.Context, auth Authorization, fxid string) (*FileInfo, error) GetFileInfoByPath(ctx context.Context, auth Authorization, path string) (*FileInfo, error) SetAttr(ctx context.Context, auth Authorization, attr *Attribute, recursive bool, path string) error - UnsetAttr(ctx context.Context, auth Authorization, attr *Attribute, path string) error + UnsetAttr(ctx context.Context, auth Authorization, attr *Attribute, recursive bool, path string) error GetAttr(ctx context.Context, auth Authorization, key, path string) (*Attribute, error) GetQuota(ctx context.Context, username string, rootAuth Authorization, path string) (*QuotaInfo, error) SetQuota(ctx context.Context, rooAuth Authorization, info *SetQuotaInfo) error diff --git a/pkg/eosclient/eosgrpc/eosgrpc.go b/pkg/eosclient/eosgrpc/eosgrpc.go index ea906c9de5..8cd4c62b6a 100644 --- a/pkg/eosclient/eosgrpc/eosgrpc.go +++ b/pkg/eosclient/eosgrpc/eosgrpc.go @@ -572,7 +572,7 @@ func (c *Client) SetAttr(ctx context.Context, auth eosclient.Authorization, attr } // UnsetAttr unsets an extended attribute on a path. -func (c *Client) UnsetAttr(ctx context.Context, auth eosclient.Authorization, attr *eosclient.Attribute, path string) error { +func (c *Client) UnsetAttr(ctx context.Context, auth eosclient.Authorization, attr *eosclient.Attribute, recursive bool, path string) error { log := appctx.GetLogger(ctx) log.Info().Str("func", "UnsetAttr").Str("uid,gid", auth.Role.UID+","+auth.Role.GID).Str("path", path).Msg("") @@ -586,6 +586,7 @@ func (c *Client) UnsetAttr(ctx context.Context, auth eosclient.Authorization, at var ktd = []string{attr.Key} msg.Keystodelete = ktd + msg.Recursive = recursive msg.Id = new(erpc.MDId) msg.Id.Path = []byte(path) diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go index 89cd109cdc..028e7d90d7 100644 --- a/pkg/storage/utils/eosfs/eosfs.go +++ b/pkg/storage/utils/eosfs/eosfs.go @@ -526,7 +526,7 @@ func (fs *eosfs) UnsetArbitraryMetadata(ctx context.Context, ref *provider.Refer Key: k, } - err := fs.c.UnsetAttr(ctx, auth, attr, fn) + err := fs.c.UnsetAttr(ctx, auth, attr, false, fn) if err != nil { return errors.Wrap(err, "eosfs: error unsetting xattr in eos driver") } @@ -796,7 +796,7 @@ func (fs *eosfs) ListGrants(ctx context.Context, ref *provider.Reference) ([]*pr grantList = append(grantList, &provider.Grant{ Grantee: grantee, - Permissions: grants.GetGrantPermissionSet(a.Permissions, true), + Permissions: grants.GetGrantPermissionSet(a.Permissions), }) } @@ -1670,7 +1670,7 @@ func (fs *eosfs) convertToFileReference(ctx context.Context, eosFileInfo *eoscli return nil, err } info.Type = provider.ResourceType_RESOURCE_TYPE_REFERENCE - val, ok := eosFileInfo.Attrs["user.reva.target"] + val, ok := eosFileInfo.Attrs["reva.target"] if !ok || val == "" { return nil, errtypes.InternalError("eosfs: reference does not contain target: target=" + val + " file=" + eosFileInfo.File) } @@ -1688,6 +1688,43 @@ func (fs *eosfs) permissionSet(ctx context.Context, eosFileInfo *eosclient.FileI } if owner != nil && u.Id.OpaqueId == owner.OpaqueId && u.Id.Idp == owner.Idp { + // The logged-in user is the owner but we may be impersonating them + // on behalf of a public share accessor. + + if u.Opaque != nil { + if publicShare, ok := u.Opaque.Map["public-share-role"]; ok { + if string(publicShare.Value) == "editor" { + return &provider.ResourcePermissions{ + CreateContainer: true, + Delete: true, + GetPath: true, + GetQuota: true, + InitiateFileDownload: true, + InitiateFileUpload: true, + ListContainer: true, + ListFileVersions: true, + ListGrants: true, + ListRecycle: true, + Move: true, + PurgeRecycle: true, + RestoreFileVersion: true, + RestoreRecycleItem: true, + Stat: true, + } + } + return &provider.ResourcePermissions{ + GetPath: true, + GetQuota: true, + InitiateFileDownload: true, + ListContainer: true, + ListFileVersions: true, + ListRecycle: true, + ListGrants: true, + Stat: true, + } + } + } + return &provider.ResourcePermissions{ // owner has all permissions AddGrant: true, @@ -1738,7 +1775,7 @@ func (fs *eosfs) permissionSet(ctx context.Context, eosFileInfo *eosclient.FileI } if (e.Type == acl.TypeUser && e.Qualifier == auth.Role.UID) || (e.Type == acl.TypeLightweight && e.Qualifier == u.Id.OpaqueId) || userInGroup { - mergePermissions(&perm, grants.GetGrantPermissionSet(e.Permissions, eosFileInfo.IsDir)) + mergePermissions(&perm, grants.GetGrantPermissionSet(e.Permissions)) } } diff --git a/pkg/storage/utils/grants/grants.go b/pkg/storage/utils/grants/grants.go index 57efbdb8da..72dbdc31db 100644 --- a/pkg/storage/utils/grants/grants.go +++ b/pkg/storage/utils/grants/grants.go @@ -66,7 +66,7 @@ func GetACLPerm(set *provider.ResourcePermissions) (string, error) { // TODO(labkode): add more fine grained controls. // EOS acls are a mix of ACLs and POSIX permissions. More details can be found in // https://github.com/cern-eos/eos/blob/master/doc/configuration/permission.rst -func GetGrantPermissionSet(perm string, isDir bool) *provider.ResourcePermissions { +func GetGrantPermissionSet(perm string) *provider.ResourcePermissions { var rp provider.ResourcePermissions // default to 0 == all denied if strings.Contains(perm, "r") && !strings.Contains(perm, "!r") { @@ -82,17 +82,13 @@ func GetGrantPermissionSet(perm string, isDir bool) *provider.ResourcePermission rp.InitiateFileUpload = true rp.RestoreFileVersion = true rp.RestoreRecycleItem = true - if isDir { - rp.CreateContainer = true - } + rp.CreateContainer = true } if strings.Contains(perm, "x") && !strings.Contains(perm, "!x") { rp.ListFileVersions = true rp.ListRecycle = true - if isDir { - rp.ListContainer = true - } + rp.ListContainer = true } if strings.Contains(perm, "!d") { diff --git a/pkg/storage/utils/localfs/localfs.go b/pkg/storage/utils/localfs/localfs.go index af70f404e3..aa55d27715 100644 --- a/pkg/storage/utils/localfs/localfs.go +++ b/pkg/storage/utils/localfs/localfs.go @@ -497,7 +497,7 @@ func (fs *localfs) ListGrants(ctx context.Context, ref *provider.Reference) ([]* } else if grantSplit[0] == acl.TypeGroup { grantee.Id = &provider.Grantee_GroupId{GroupId: &grouppb.GroupId{OpaqueId: parts[0], Idp: parts[1]}} } - permissions := grants.GetGrantPermissionSet(role, true) + permissions := grants.GetGrantPermissionSet(role) grantList = append(grantList, &provider.Grant{ Grantee: grantee,