From ecf3249f4d13aa23ce27400fc65aafbc2a92d60e Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Mon, 10 Oct 2022 11:43:18 +0200 Subject: [PATCH 01/21] rewrote lightweight account in eos to be atomic --- pkg/eosclient/eosclient.go | 1 + pkg/storage/utils/eosfs/eosfs.go | 144 ++++++++++++++++++----------- pkg/storage/utils/grants/grants.go | 2 +- 3 files changed, 93 insertions(+), 54 deletions(-) diff --git a/pkg/eosclient/eosclient.go b/pkg/eosclient/eosclient.go index b62c413458..c9d52d4d87 100644 --- a/pkg/eosclient/eosclient.go +++ b/pkg/eosclient/eosclient.go @@ -39,6 +39,7 @@ type EOSClient interface { SetAttr(ctx context.Context, auth Authorization, attr *Attribute, errorIfExists, recursive bool, 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) + GetAttrs(ctx context.Context, auth Authorization, 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 Touch(ctx context.Context, auth Authorization, path string) error diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go index 64f8981b25..2465be061d 100644 --- a/pkg/storage/utils/eosfs/eosfs.go +++ b/pkg/storage/utils/eosfs/eosfs.go @@ -62,6 +62,7 @@ import ( const ( refTargetAttrKey = "reva.target" + lwShareAttrKey = "reva.lwshare" ) const ( @@ -1000,15 +1001,28 @@ func (fs *eosfs) AddGrant(ctx context.Context, ref *provider.Reference, g *provi return err } - // position where put the ACL - position := eosclient.StartPosition - eosACL, err := fs.getEosACL(ctx, g) if err != nil { return err } - err = fs.c.AddACL(ctx, auth, rootAuth, fn, position, eosACL) + if eosACL.Type == acl.TypeLightweight { + // The ACLs for a lightweight is understandable by EOS + // directly, but only from reva. So we have to store them + // in an xattr named sys.reva.lwshare., with value + // the permissions. + attr := &eosclient.Attribute{ + Type: SystemAttr, + Key: fmt.Sprintf("%s.%s", lwShareAttrKey, eosACL.Qualifier), + Val: eosACL.Permissions, + } + if err := fs.c.SetAttr(ctx, rootAuth, attr, false, true, fn); err != nil { + return errors.Wrap(err, "eosfs: error adding acl for lightweight account") + } + return nil + } + + err = fs.c.AddACL(ctx, auth, rootAuth, fn, eosclient.StartPosition, eosACL) if err != nil { return errors.Wrap(err, "eosfs: error adding acl") } @@ -1086,45 +1100,29 @@ func (fs *eosfs) getEosACL(ctx context.Context, g *provider.Grant) (*acl.Entry, } func (fs *eosfs) RemoveGrant(ctx context.Context, ref *provider.Reference, g *provider.Grant) error { - eosACLType, err := grants.GetACLType(g.Grantee.Type) + fn, auth, err := fs.resolveRefAndGetAuth(ctx, ref) if err != nil { return err } - var recipient string - if eosACLType == acl.TypeUser { - // if the grantee is a lightweight account, we need to set it accordingly - if g.Grantee.GetUserId().Type == userpb.UserType_USER_TYPE_LIGHTWEIGHT || - g.Grantee.GetUserId().Type == userpb.UserType_USER_TYPE_FEDERATED { - eosACLType = acl.TypeLightweight - recipient = g.Grantee.GetUserId().OpaqueId - } else { - // since EOS Citrine ACLs are stored with uid, we need to convert username to uid - auth, err := fs.getUIDGateway(ctx, g.Grantee.GetUserId()) - if err != nil { - return err - } - recipient = auth.Role.UID - } - } else { - recipient = g.Grantee.GetGroupId().OpaqueId - } - - eosACL := &acl.Entry{ - Qualifier: recipient, - Type: eosACLType, - } - - fn, auth, err := fs.resolveRefAndGetAuth(ctx, ref) + rootAuth, err := fs.getRootAuth(ctx) if err != nil { return err } - rootAuth, err := fs.getRootAuth(ctx) + eosACL, err := fs.getEosACL(ctx, g) if err != nil { return err } + if eosACL.Type == acl.TypeLightweight { + attr := &eosclient.Attribute{} + if err := fs.c.UnsetAttr(ctx, rootAuth, attr, true, fn); err != nil { + return errors.Wrap(err, "eosfs: error removing acl for lightweight account") + } + return nil + } + err = fs.c.RemoveACL(ctx, auth, rootAuth, fn, eosACL) if err != nil { return errors.Wrap(err, "eosfs: error removing acl") @@ -1136,19 +1134,9 @@ func (fs *eosfs) UpdateGrant(ctx context.Context, ref *provider.Reference, g *pr return fs.AddGrant(ctx, ref, g) } -func (fs *eosfs) ListGrants(ctx context.Context, ref *provider.Reference) ([]*provider.Grant, error) { - fn, auth, err := fs.resolveRefAndGetAuth(ctx, ref) - if err != nil { - return nil, err - } - - acls, err := fs.c.ListACLs(ctx, auth, fn) - if err != nil { - return nil, err - } - - grantList := []*provider.Grant{} - for _, a := range acls { +func (fs *eosfs) convertACLsToGrants(ctx context.Context, acls *acl.ACLs) ([]*provider.Grant, error) { + res := make([]*provider.Grant, 0, len(acls.Entries)) + for _, a := range acls.Entries { var grantee *provider.Grantee switch { case a.Type == acl.TypeUser: @@ -1162,24 +1150,74 @@ func (fs *eosfs) ListGrants(ctx context.Context, ref *provider.Reference) ([]*pr Id: &provider.Grantee_UserId{UserId: qualifier}, Type: grants.GetGranteeType(a.Type), } - case a.Type == acl.TypeLightweight: - a.Type = acl.TypeUser - grantee = &provider.Grantee{ - Id: &provider.Grantee_UserId{UserId: &userpb.UserId{OpaqueId: a.Qualifier}}, - Type: grants.GetGranteeType(a.Type), - } - default: + case a.Type == acl.TypeGroup: grantee = &provider.Grantee{ Id: &provider.Grantee_GroupId{GroupId: &grouppb.GroupId{OpaqueId: a.Qualifier}}, Type: grants.GetGranteeType(a.Type), } + default: + return nil, errtypes.InternalError(fmt.Sprintf("eosfs: acl type %s not recognised", a.Type)) } - - grantList = append(grantList, &provider.Grant{ + res = append(res, &provider.Grant{ Grantee: grantee, Permissions: grants.GetGrantPermissionSet(a.Permissions), }) } + return res, nil +} + +func isSysACLs(a *eosclient.Attribute) bool { + return a.Type == SystemAttr && a.Key == "sys" +} + +func isLightweightACL(a *eosclient.Attribute) bool { + return a.Type == SystemAttr && strings.HasPrefix(a.Key, lwShareAttrKey) +} + +func parseLightweightACL(a *eosclient.Attribute) *provider.Grant { + qualifier := strings.TrimPrefix(a.Key, lwShareAttrKey+".") + return &provider.Grant{ + Grantee: &provider.Grantee{ + Id: &provider.Grantee_UserId{UserId: &userpb.UserId{ + // FIXME: idp missing, maybe get the user_id from the user provider? + Type: userpb.UserType_USER_TYPE_LIGHTWEIGHT, + OpaqueId: qualifier, + }}, + Type: grants.GetGranteeType(acl.TypeLightweight), + }, + Permissions: grants.GetGrantPermissionSet(a.Val), + } +} + +func (fs *eosfs) ListGrants(ctx context.Context, ref *provider.Reference) ([]*provider.Grant, error) { + fn, auth, err := fs.resolveRefAndGetAuth(ctx, ref) + if err != nil { + return nil, err + } + + attrs, err := fs.c.GetAttrs(ctx, auth, fn) + if err != nil { + return nil, err + } + + grantList := []*provider.Grant{} + for _, a := range attrs { + switch { + case isSysACLs(a): + // EOS ACLs + acls, err := acl.Parse(a.Val, acl.ShortTextForm) + if err != nil { + return nil, err + } + grants, err := fs.convertACLsToGrants(ctx, acls) + if err != nil { + return nil, err + } + grantList = append(grantList, grants...) + case isLightweightACL(a): + grantList = append(grantList, parseLightweightACL(a)) + } + } return grantList, nil } diff --git a/pkg/storage/utils/grants/grants.go b/pkg/storage/utils/grants/grants.go index 72dbdc31db..f7185ba8de 100644 --- a/pkg/storage/utils/grants/grants.go +++ b/pkg/storage/utils/grants/grants.go @@ -124,7 +124,7 @@ func GetACLType(gt provider.GranteeType) (string, error) { // GetGranteeType returns the grantee type from a char func GetGranteeType(aclType string) provider.GranteeType { switch aclType { - case acl.TypeUser: + case acl.TypeUser, acl.TypeLightweight: return provider.GranteeType_GRANTEE_TYPE_USER case acl.TypeGroup: return provider.GranteeType_GRANTEE_TYPE_GROUP From 59da7cbb6363d45c0603220c269ee97a3d16df18 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Mon, 10 Oct 2022 11:43:51 +0200 Subject: [PATCH 02/21] implemented getattrs in eos binary --- pkg/eosclient/eosbinary/eosbinary.go | 90 +++++++++------------------- 1 file changed, 27 insertions(+), 63 deletions(-) diff --git a/pkg/eosclient/eosbinary/eosbinary.go b/pkg/eosclient/eosbinary/eosbinary.go index 0f80ec55c6..7a45c30748 100644 --- a/pkg/eosclient/eosbinary/eosbinary.go +++ b/pkg/eosclient/eosbinary/eosbinary.go @@ -45,7 +45,6 @@ import ( const ( versionPrefix = ".sys.v#." - lwShareAttrKey = "reva.lwshare" userACLEvalKey = "eval.useracl" favoritesKey = "http://owncloud.org/ns/favorite" ) @@ -280,30 +279,6 @@ func (c *Client) AddACL(ctx context.Context, auth, rootAuth eosclient.Authorizat return err } - if a.Type == acl.TypeLightweight { - sysACL := "" - aclStr, ok := finfo.Attrs["sys."+lwShareAttrKey] - if ok { - acls, err := acl.Parse(aclStr, acl.ShortTextForm) - if err != nil { - return err - } - err = acls.SetEntry(a.Type, a.Qualifier, a.Permissions) - if err != nil { - return err - } - sysACL = acls.Serialize() - } else { - sysACL = a.CitrineSerialize() - } - sysACLAttr := &eosclient.Attribute{ - Type: eosclient.SystemAttr, - Key: lwShareAttrKey, - Val: sysACL, - } - return c.SetAttr(ctx, auth, sysACLAttr, false, finfo.IsDir, path) - } - sysACL := a.CitrineSerialize() args := []string{"acl", "--sys"} if finfo.IsDir { @@ -330,30 +305,6 @@ func (c *Client) RemoveACL(ctx context.Context, auth, rootAuth eosclient.Authori return err } - if a.Type == acl.TypeLightweight { - sysACL := "" - aclStr, ok := finfo.Attrs["sys."+lwShareAttrKey] - if ok { - acls, err := acl.Parse(aclStr, acl.ShortTextForm) - if err != nil { - return err - } - acls.DeleteEntry(a.Type, a.Qualifier) - if err != nil { - return err - } - sysACL = acls.Serialize() - } else { - sysACL = a.CitrineSerialize() - } - sysACLAttr := &eosclient.Attribute{ - Type: eosclient.SystemAttr, - Key: lwShareAttrKey, - Val: sysACL, - } - return c.SetAttr(ctx, auth, sysACLAttr, false, finfo.IsDir, path) - } - sysACL := a.CitrineSerialize() args := []string{"acl", "--sys"} if finfo.IsDir { @@ -645,6 +596,33 @@ func (c *Client) GetAttr(ctx context.Context, auth eosclient.Authorization, key, return attr, nil } +func (c *Client) GetAttrs(ctx context.Context, auth eosclient.Authorization, path string) ([]*eosclient.Attribute, error) { + info, err := c.getRawFileInfoByPath(ctx, auth, path) + if err != nil { + return nil, err + } + if !info.IsDir { + path = getVersionFolder(path) + } + + args := []string{"attr", "ls", path} + attrOut, _, err := c.executeEOS(ctx, args, auth) + if err != nil { + return nil, err + } + + attrsStr := strings.Split(attrOut, "\n") + attrs := make([]*eosclient.Attribute, 0, len(attrsStr)) + for _, line := range attrsStr { + attr, err := deserializeAttribute(line) + if err != nil { + return nil, err + } + attrs = append(attrs, attr) + } + return attrs, nil +} + func deserializeAttribute(attrStr string) (*eosclient.Attribute, error) { // the string is in the form sys.forced.checksum="adler" keyValue := strings.SplitN(strings.TrimSpace(attrStr), "=", 2) // keyValue = ["sys.forced.checksum", "\"adler\""] @@ -1222,20 +1200,6 @@ func (c *Client) mapToFileInfo(ctx context.Context, kv, attrs map[string]string, } } - // Read lightweight ACLs recognized by the sys.reva.lwshare attr - if lwACLStr, ok := attrs["sys."+lwShareAttrKey]; ok { - lwAcls, err := acl.Parse(lwACLStr, acl.ShortTextForm) - if err != nil { - return nil, err - } - for _, e := range lwAcls.Entries { - err = sysACL.SetEntry(e.Type, e.Qualifier, e.Permissions) - if err != nil { - return nil, err - } - } - } - // Read the favorite attr if parseFavoriteKey { parseAndSetFavoriteAttr(ctx, attrs) From 8eaf2602a522f4f260f1b02b13c014eb9d67add1 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Mon, 10 Oct 2022 11:44:03 +0200 Subject: [PATCH 03/21] implemented getattrs in eos grpc --- pkg/eosclient/eosgrpc/eosgrpc.go | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/pkg/eosclient/eosgrpc/eosgrpc.go b/pkg/eosclient/eosgrpc/eosgrpc.go index 47bd3c4a7f..de295f00d4 100644 --- a/pkg/eosclient/eosgrpc/eosgrpc.go +++ b/pkg/eosclient/eosgrpc/eosgrpc.go @@ -49,7 +49,6 @@ import ( const ( versionPrefix = ".sys.v#." - // lwShareAttrKey = "reva.lwshare" ) const ( @@ -629,6 +628,24 @@ func (c *Client) GetAttr(ctx context.Context, auth eosclient.Authorization, key, return nil, errtypes.NotFound(fmt.Sprintf("key %s not found", key)) } +func (c *Client) GetAttrs(ctx context.Context, auth eosclient.Authorization, path string) ([]*eosclient.Attribute, error) { + info, err := c.GetFileInfoByPath(ctx, auth, path) + if err != nil { + return nil, err + } + + attrs := make([]*eosclient.Attribute, 0, len(info.Attrs)) + for k, v := range info.Attrs { + attr, err := getAttribute(k, v) + if err != nil { + return nil, errors.Wrap(err, fmt.Sprintf("eosgrpc: cannot parse attribute key=%s value=%s", k, v)) + } + attrs = append(attrs, attr) + } + + return attrs, nil +} + func getAttribute(key, val string) (*eosclient.Attribute, error) { // key is in the form sys.forced.checksum type2key := strings.SplitN(key, ".", 2) // type2key = ["sys", "forced.checksum"] @@ -1238,8 +1255,10 @@ func (c *Client) List(ctx context.Context, auth eosclient.Authorization, dpath s // Read reads a file from the mgm and returns a handle to read it // This handle could be directly the body of the response or a local tmp file -// returning a handle to the body is nice, yet it gives less control on the transaction -// itself, e.g. strange timeouts or TCP issues may be more difficult to trace +// +// returning a handle to the body is nice, yet it gives less control on the transaction +// itself, e.g. strange timeouts or TCP issues may be more difficult to trace +// // Let's consider this experimental for the moment, maybe I'll like to add a config // parameter to choose between the two behaviours func (c *Client) Read(ctx context.Context, auth eosclient.Authorization, path string) (io.ReadCloser, error) { From 51ad401bdffaa63fe2c885fb6ee40154309bcab2 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Mon, 10 Oct 2022 14:39:24 +0200 Subject: [PATCH 04/21] fix permissions for lightweight accounts --- pkg/storage/utils/eosfs/eosfs.go | 76 ++++++++++++++++++++++---------- 1 file changed, 52 insertions(+), 24 deletions(-) diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go index 2465be061d..bfe8fabbce 100644 --- a/pkg/storage/utils/eosfs/eosfs.go +++ b/pkg/storage/utils/eosfs/eosfs.go @@ -2079,23 +2079,17 @@ 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 conversions.NewEditorRole().CS3ResourcePermissions() - } else if string(publicShare.Value) == "uploader" { - return conversions.NewUploaderRole().CS3ResourcePermissions() - } - // Default to viewer role - return conversions.NewViewerRole().CS3ResourcePermissions() - } + if role, ok := utils.HasPublicShareRole(u); ok { + switch role { + case "editor": + return conversions.NewEditorRole().CS3ResourcePermissions() + case "uploader": + return conversions.NewUploaderRole().CS3ResourcePermissions() } + return conversions.NewViewerRole().CS3ResourcePermissions() + } - // owner has all permissions + if utils.UserEqual(u.Id, owner) { return conversions.NewManagerRole().CS3ResourcePermissions() } @@ -2113,16 +2107,19 @@ func (fs *eosfs) permissionSet(ctx context.Context, eosFileInfo *eosclient.FileI } var perm provider.ResourcePermissions - for _, e := range eosFileInfo.SysACL.Entries { - var userInGroup bool - if e.Type == acl.TypeGroup { - for _, g := range u.Groups { - if e.Qualifier == g { - userInGroup = true - break - } - } + // as the lightweight acl are stored as normal attrs, + // we need to add them in the sysacl entries + + for k, v := range eosFileInfo.Attrs { + if e, ok := attrForLightweightACL(k, v); ok { + eosFileInfo.SysACL.Entries = append(eosFileInfo.SysACL.Entries, e) } + } + + userGroupsSet := makeSet(u.Groups) + + for _, e := range eosFileInfo.SysACL.Entries { + userInGroup := e.Type == acl.TypeGroup && userGroupsSet.in(strings.ToLower(e.Qualifier)) 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)) @@ -2132,6 +2129,37 @@ func (fs *eosfs) permissionSet(ctx context.Context, eosFileInfo *eosclient.FileI return &perm } +func attrForLightweightACL(k, v string) (*acl.Entry, bool) { + ok := strings.HasPrefix(k, "sys."+lwShareAttrKey) + if !ok { + return nil, false + } + + qualifier := strings.TrimPrefix(k, fmt.Sprintf("sys.%s.", lwShareAttrKey)) + + attr := &acl.Entry{ + Type: acl.TypeLightweight, + Qualifier: qualifier, + Permissions: v, + } + return attr, true +} + +type groupSet map[string]struct{} + +func makeSet(lst []string) groupSet { + s := make(map[string]struct{}, len(lst)) + for _, e := range lst { + s[e] = struct{}{} + } + return s +} + +func (s groupSet) in(group string) bool { + _, ok := s[group] + return ok +} + func mergePermissions(l *provider.ResourcePermissions, r *provider.ResourcePermissions) { l.AddGrant = l.AddGrant || r.AddGrant l.CreateContainer = l.CreateContainer || r.CreateContainer From d5c9ddf2cd87e370fce08817e12a0f4240fe492e Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Mon, 10 Oct 2022 17:33:56 +0200 Subject: [PATCH 05/21] allow thumbnails to be used by lightweight accounts --- pkg/auth/scope/lightweight.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/auth/scope/lightweight.go b/pkg/auth/scope/lightweight.go index 5ebc8d2225..c828f149b5 100644 --- a/pkg/auth/scope/lightweight.go +++ b/pkg/auth/scope/lightweight.go @@ -56,6 +56,7 @@ func checkLightweightPath(path string) bool { "/ocs/v1.php/cloud/user", "/remote.php/webdav", "/remote.php/dav/files", + "/thumbnails", "/app/open", "/app/new", "/archiver", From e3a42773e0b4770a14c70647bc7422411728ab4e Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Tue, 11 Oct 2022 13:33:57 +0200 Subject: [PATCH 06/21] do not check lightweight account scope into gateway --- internal/grpc/interceptors/auth/scope.go | 65 ------------------------ pkg/auth/scope/lightweight.go | 24 +++++++-- 2 files changed, 20 insertions(+), 69 deletions(-) diff --git a/internal/grpc/interceptors/auth/scope.go b/internal/grpc/interceptors/auth/scope.go index 1bb5581ab7..a939416067 100644 --- a/internal/grpc/interceptors/auth/scope.go +++ b/internal/grpc/interceptors/auth/scope.go @@ -77,80 +77,15 @@ func expandAndVerifyScope(ctx context.Context, req interface{}, tokenScope map[s if err = resolveUserShare(ctx, ref, tokenScope[k], client, mgr); err == nil { return nil } - - case strings.HasPrefix(k, "lightweight"): - if err = resolveLightweightScope(ctx, ref, tokenScope[k], user, client, mgr); err == nil { - return nil - } } log.Err(err).Msgf("error resolving reference %s under scope %+v", ref.String(), k) } - } else if ref, ok := extractShareRef(req); ok { - // It's a share ref - // The request might be coming from a share created for a lightweight account - // after the token was minted. - log.Info().Msgf("resolving share reference against received shares to verify token scope %+v", ref.String()) - for k := range tokenScope { - if strings.HasPrefix(k, "lightweight") { - // Check if this ID is cached - key := "lw:" + user.Id.OpaqueId + scopeDelimiter + ref.GetId().OpaqueId - if _, err := scopeExpansionCache.Get(key); err == nil { - return nil - } - - shares, err := client.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{}) - if err != nil || shares.Status.Code != rpc.Code_CODE_OK { - log.Warn().Err(err).Msg("error listing received shares") - continue - } - for _, s := range shares.Shares { - shareKey := "lw:" + user.Id.OpaqueId + scopeDelimiter + s.Share.Id.OpaqueId - _ = scopeExpansionCache.SetWithExpire(shareKey, nil, scopeCacheExpiration*time.Second) - - if ref.GetId() != nil && ref.GetId().OpaqueId == s.Share.Id.OpaqueId { - return nil - } - if key := ref.GetKey(); key != nil && (utils.UserEqual(key.Owner, s.Share.Owner) || utils.UserEqual(key.Owner, s.Share.Creator)) && - utils.ResourceIDEqual(key.ResourceId, s.Share.ResourceId) && utils.GranteeEqual(key.Grantee, s.Share.Grantee) { - return nil - } - } - } - } } return errtypes.PermissionDenied("access to resource not allowed within the assigned scope") } -func resolveLightweightScope(ctx context.Context, ref *provider.Reference, scope *authpb.Scope, user *userpb.User, client gateway.GatewayAPIClient, mgr token.Manager) error { - // Check if this ref is cached - key := "lw:" + user.Id.OpaqueId + scopeDelimiter + getRefKey(ref) - if _, err := scopeExpansionCache.Get(key); err == nil { - return nil - } - - shares, err := client.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{}) - if err != nil || shares.Status.Code != rpc.Code_CODE_OK { - return errtypes.InternalError("error listing received shares") - } - - for _, share := range shares.Shares { - shareKey := "lw:" + user.Id.OpaqueId + scopeDelimiter + resourceid.OwnCloudResourceIDWrap(share.Share.ResourceId) - _ = scopeExpansionCache.SetWithExpire(shareKey, nil, scopeCacheExpiration*time.Second) - - if ref.ResourceId != nil && utils.ResourceIDEqual(share.Share.ResourceId, ref.ResourceId) { - return nil - } - if ok, err := checkIfNestedResource(ctx, ref, share.Share.ResourceId, client, mgr); err == nil && ok { - _ = scopeExpansionCache.SetWithExpire(key, nil, scopeCacheExpiration*time.Second) - return nil - } - } - - return errtypes.PermissionDenied("request is not for a nested resource") -} - func resolvePublicShare(ctx context.Context, ref *provider.Reference, scope *authpb.Scope, client gateway.GatewayAPIClient, mgr token.Manager) error { var share link.PublicShare err := utils.UnmarshalJSONToProtoV1(scope.Resource.Value, &share) diff --git a/pkg/auth/scope/lightweight.go b/pkg/auth/scope/lightweight.go index c828f149b5..ada902aaf1 100644 --- a/pkg/auth/scope/lightweight.go +++ b/pkg/auth/scope/lightweight.go @@ -22,22 +22,38 @@ import ( "context" "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" 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" "github.com/cs3org/reva/pkg/utils" "github.com/rs/zerolog" ) func lightweightAccountScope(_ context.Context, scope *authpb.Scope, resource interface{}, _ *zerolog.Logger) (bool, error) { - // Lightweight accounts have access to resources shared with them. - // These cannot be resolved from here, but need to be added to the scope from - // where the call to mint tokens is made. - // From here, we only allow ListReceivedShares calls switch v := resource.(type) { case *collaboration.ListReceivedSharesRequest: return true, nil + // Editing role for shares + case *provider.CreateContainerRequest, + *provider.TouchFileRequest, + *provider.DeleteRequest, + *provider.MoveRequest, + *provider.InitiateFileUploadRequest, + *provider.SetArbitraryMetadataRequest, + *provider.UnsetArbitraryMetadataRequest: + return true, nil + // Viewer role for shares + case *registry.GetStorageProvidersRequest, + *provider.StatRequest, + *provider.ListContainerRequest, + *provider.InitiateFileDownloadRequest, + *appprovider.OpenInAppRequest, + *gateway.OpenInAppRequest: + return true, nil case string: return checkLightweightPath(v), nil } From ff178e0699f1f3b00e05b458b05003e002409959 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Tue, 11 Oct 2022 13:52:48 +0200 Subject: [PATCH 07/21] add check lightweight account method --- pkg/utils/utils.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 54033e3386..3b36eb2db2 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -19,6 +19,7 @@ package utils import ( + "context" "fmt" "math/rand" "net" @@ -36,6 +37,7 @@ import ( userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + ctxpkg "github.com/cs3org/reva/pkg/ctx" "github.com/cs3org/reva/pkg/registry" "github.com/cs3org/reva/pkg/registry/memory" "github.com/golang/protobuf/proto" @@ -354,3 +356,21 @@ func GetViewMode(viewMode string) gateway.OpenInAppRequest_ViewMode { return gateway.OpenInAppRequest_VIEW_MODE_INVALID } } + +// HasPublicShareRole return true if the user has a public share role. +// If yes, the string is the type of role, viewer, editor or uploader +func HasPublicShareRole(u *userpb.User) (string, bool) { + if u.Opaque == nil { + return "", false + } + if publicShare, ok := u.Opaque.Map["public-share-role"]; ok { + return string(publicShare.Value), true + } + return "", false +} + +func IsLightweithAccountInCtx(ctx context.Context) bool { + user := ctxpkg.ContextMustGetUser(ctx) + return user.Id.Type == userpb.UserType_USER_TYPE_FEDERATED || + user.Id.Type == userpb.UserType_USER_TYPE_LIGHTWEIGHT +} From 565f98803c4a3ea0560953ff065d9b76f5738379 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Tue, 11 Oct 2022 17:24:16 +0200 Subject: [PATCH 08/21] Revert "add check lightweight account method" This reverts commit 79ee54c91e8ae0ad25fa98e5105f8a787bfc6208. --- pkg/utils/utils.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 3b36eb2db2..d0fb8ef423 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -19,7 +19,6 @@ package utils import ( - "context" "fmt" "math/rand" "net" @@ -37,7 +36,6 @@ import ( userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" - ctxpkg "github.com/cs3org/reva/pkg/ctx" "github.com/cs3org/reva/pkg/registry" "github.com/cs3org/reva/pkg/registry/memory" "github.com/golang/protobuf/proto" @@ -368,9 +366,3 @@ func HasPublicShareRole(u *userpb.User) (string, bool) { } return "", false } - -func IsLightweithAccountInCtx(ctx context.Context) bool { - user := ctxpkg.ContextMustGetUser(ctx) - return user.Id.Type == userpb.UserType_USER_TYPE_FEDERATED || - user.Id.Type == userpb.UserType_USER_TYPE_LIGHTWEIGHT -} From aa58a1a3ec0f0a8b39be7a9b58d9f52211979652 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Tue, 11 Oct 2022 17:38:38 +0200 Subject: [PATCH 09/21] in verify scope check only list share request --- pkg/auth/scope/lightweight.go | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/pkg/auth/scope/lightweight.go b/pkg/auth/scope/lightweight.go index ada902aaf1..d0356cd93e 100644 --- a/pkg/auth/scope/lightweight.go +++ b/pkg/auth/scope/lightweight.go @@ -22,12 +22,9 @@ import ( "context" "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" 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" "github.com/cs3org/reva/pkg/utils" "github.com/rs/zerolog" @@ -37,23 +34,6 @@ func lightweightAccountScope(_ context.Context, scope *authpb.Scope, resource in switch v := resource.(type) { case *collaboration.ListReceivedSharesRequest: return true, nil - // Editing role for shares - case *provider.CreateContainerRequest, - *provider.TouchFileRequest, - *provider.DeleteRequest, - *provider.MoveRequest, - *provider.InitiateFileUploadRequest, - *provider.SetArbitraryMetadataRequest, - *provider.UnsetArbitraryMetadataRequest: - return true, nil - // Viewer role for shares - case *registry.GetStorageProvidersRequest, - *provider.StatRequest, - *provider.ListContainerRequest, - *provider.InitiateFileDownloadRequest, - *appprovider.OpenInAppRequest, - *gateway.OpenInAppRequest: - return true, nil case string: return checkLightweightPath(v), nil } From 38c1d45c4bf7c2eeb310aab08a2acd06c9101c08 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Tue, 11 Oct 2022 17:40:17 +0200 Subject: [PATCH 10/21] Revert "do not check lightweight account scope into gateway" This reverts commit e1a4817265f2c1ad48f69a464738addb714e3e65. --- internal/grpc/interceptors/auth/scope.go | 65 ++++++++++++++++++++++++ pkg/auth/scope/lightweight.go | 4 ++ 2 files changed, 69 insertions(+) diff --git a/internal/grpc/interceptors/auth/scope.go b/internal/grpc/interceptors/auth/scope.go index a939416067..1bb5581ab7 100644 --- a/internal/grpc/interceptors/auth/scope.go +++ b/internal/grpc/interceptors/auth/scope.go @@ -77,15 +77,80 @@ func expandAndVerifyScope(ctx context.Context, req interface{}, tokenScope map[s if err = resolveUserShare(ctx, ref, tokenScope[k], client, mgr); err == nil { return nil } + + case strings.HasPrefix(k, "lightweight"): + if err = resolveLightweightScope(ctx, ref, tokenScope[k], user, client, mgr); err == nil { + return nil + } } log.Err(err).Msgf("error resolving reference %s under scope %+v", ref.String(), k) } + } else if ref, ok := extractShareRef(req); ok { + // It's a share ref + // The request might be coming from a share created for a lightweight account + // after the token was minted. + log.Info().Msgf("resolving share reference against received shares to verify token scope %+v", ref.String()) + for k := range tokenScope { + if strings.HasPrefix(k, "lightweight") { + // Check if this ID is cached + key := "lw:" + user.Id.OpaqueId + scopeDelimiter + ref.GetId().OpaqueId + if _, err := scopeExpansionCache.Get(key); err == nil { + return nil + } + + shares, err := client.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{}) + if err != nil || shares.Status.Code != rpc.Code_CODE_OK { + log.Warn().Err(err).Msg("error listing received shares") + continue + } + for _, s := range shares.Shares { + shareKey := "lw:" + user.Id.OpaqueId + scopeDelimiter + s.Share.Id.OpaqueId + _ = scopeExpansionCache.SetWithExpire(shareKey, nil, scopeCacheExpiration*time.Second) + + if ref.GetId() != nil && ref.GetId().OpaqueId == s.Share.Id.OpaqueId { + return nil + } + if key := ref.GetKey(); key != nil && (utils.UserEqual(key.Owner, s.Share.Owner) || utils.UserEqual(key.Owner, s.Share.Creator)) && + utils.ResourceIDEqual(key.ResourceId, s.Share.ResourceId) && utils.GranteeEqual(key.Grantee, s.Share.Grantee) { + return nil + } + } + } + } } return errtypes.PermissionDenied("access to resource not allowed within the assigned scope") } +func resolveLightweightScope(ctx context.Context, ref *provider.Reference, scope *authpb.Scope, user *userpb.User, client gateway.GatewayAPIClient, mgr token.Manager) error { + // Check if this ref is cached + key := "lw:" + user.Id.OpaqueId + scopeDelimiter + getRefKey(ref) + if _, err := scopeExpansionCache.Get(key); err == nil { + return nil + } + + shares, err := client.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{}) + if err != nil || shares.Status.Code != rpc.Code_CODE_OK { + return errtypes.InternalError("error listing received shares") + } + + for _, share := range shares.Shares { + shareKey := "lw:" + user.Id.OpaqueId + scopeDelimiter + resourceid.OwnCloudResourceIDWrap(share.Share.ResourceId) + _ = scopeExpansionCache.SetWithExpire(shareKey, nil, scopeCacheExpiration*time.Second) + + if ref.ResourceId != nil && utils.ResourceIDEqual(share.Share.ResourceId, ref.ResourceId) { + return nil + } + if ok, err := checkIfNestedResource(ctx, ref, share.Share.ResourceId, client, mgr); err == nil && ok { + _ = scopeExpansionCache.SetWithExpire(key, nil, scopeCacheExpiration*time.Second) + return nil + } + } + + return errtypes.PermissionDenied("request is not for a nested resource") +} + func resolvePublicShare(ctx context.Context, ref *provider.Reference, scope *authpb.Scope, client gateway.GatewayAPIClient, mgr token.Manager) error { var share link.PublicShare err := utils.UnmarshalJSONToProtoV1(scope.Resource.Value, &share) diff --git a/pkg/auth/scope/lightweight.go b/pkg/auth/scope/lightweight.go index d0356cd93e..c828f149b5 100644 --- a/pkg/auth/scope/lightweight.go +++ b/pkg/auth/scope/lightweight.go @@ -31,6 +31,10 @@ import ( ) func lightweightAccountScope(_ context.Context, scope *authpb.Scope, resource interface{}, _ *zerolog.Logger) (bool, error) { + // Lightweight accounts have access to resources shared with them. + // These cannot be resolved from here, but need to be added to the scope from + // where the call to mint tokens is made. + // From here, we only allow ListReceivedShares calls switch v := resource.(type) { case *collaboration.ListReceivedSharesRequest: return true, nil From 1bc8b41d20ae34fdf81fc4dcaf550924fb455347 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Tue, 11 Oct 2022 18:50:06 +0200 Subject: [PATCH 11/21] add method to check subset of permissions --- pkg/utils/utils.go | 14 +++++++++ pkg/utils/utils_test.go | 69 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index d0fb8ef423..e7d84bd432 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -27,6 +27,7 @@ import ( "os/user" "path" "path/filepath" + "reflect" "regexp" "strings" "time" @@ -366,3 +367,16 @@ func HasPublicShareRole(u *userpb.User) (string, bool) { } return "", false } + +func HasPermissions(target, toCheck *provider.ResourcePermissions) bool { + targetStruct := reflect.ValueOf(target).Elem() + toCheckStruct := reflect.ValueOf(toCheck).Elem() + + for i := 0; i < toCheckStruct.NumField(); i++ { + fieldToCheck := toCheckStruct.Field(i) + if fieldToCheck.Kind() == reflect.Bool && fieldToCheck.Bool() && !targetStruct.Field(i).Bool() { + return false + } + } + return true +} diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index 29bbf66112..091e8463ea 100644 --- a/pkg/utils/utils_test.go +++ b/pkg/utils/utils_test.go @@ -207,3 +207,72 @@ func TestParseStorageSpaceReference(t *testing.T) { } } } + +func TestHasPermissions(t *testing.T) { + tests := []struct { + name string + target *provider.ResourcePermissions + toCheck *provider.ResourcePermissions + expected bool + }{ + { + name: "both empty", + target: &provider.ResourcePermissions{}, + toCheck: &provider.ResourcePermissions{}, + expected: true, + }, + { + name: "empty target", + target: &provider.ResourcePermissions{}, + toCheck: &provider.ResourcePermissions{ + AddGrant: true, + }, + expected: false, + }, + { + name: "empty to_check", + target: &provider.ResourcePermissions{ + AddGrant: true, + }, + toCheck: &provider.ResourcePermissions{}, + expected: true, + }, + { + name: "to_check is a subset", + target: &provider.ResourcePermissions{ + AddGrant: true, + CreateContainer: true, + Delete: true, + GetPath: true, + }, + toCheck: &provider.ResourcePermissions{ + CreateContainer: true, + GetPath: true, + }, + expected: true, + }, + { + name: "to_check contains permissions to in target", + target: &provider.ResourcePermissions{ + AddGrant: true, + CreateContainer: true, + Delete: true, + GetPath: true, + }, + toCheck: &provider.ResourcePermissions{ + CreateContainer: true, + GetPath: true, + Move: true, + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if res := HasPermissions(tt.target, tt.toCheck); res != tt.expected { + t.Fatalf("got unexpected result: target=%+v to_check=%+v res=%+v expected=%+v", tt.target, tt.toCheck, res, tt.expected) + } + }) + } +} From a9d6741c4369417396ad5a5cced38025408c069d Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Tue, 11 Oct 2022 18:51:10 +0200 Subject: [PATCH 12/21] implemented scope checking for lightweight accounts --- internal/grpc/interceptors/auth/scope.go | 125 +++++++++++++---------- 1 file changed, 71 insertions(+), 54 deletions(-) diff --git a/internal/grpc/interceptors/auth/scope.go b/internal/grpc/interceptors/auth/scope.go index 1bb5581ab7..684d250ff4 100644 --- a/internal/grpc/interceptors/auth/scope.go +++ b/internal/grpc/interceptors/auth/scope.go @@ -78,77 +78,94 @@ func expandAndVerifyScope(ctx context.Context, req interface{}, tokenScope map[s return nil } - case strings.HasPrefix(k, "lightweight"): - if err = resolveLightweightScope(ctx, ref, tokenScope[k], user, client, mgr); err == nil { - return nil - } } log.Err(err).Msgf("error resolving reference %s under scope %+v", ref.String(), k) } - } else if ref, ok := extractShareRef(req); ok { - // It's a share ref - // The request might be coming from a share created for a lightweight account - // after the token was minted. - log.Info().Msgf("resolving share reference against received shares to verify token scope %+v", ref.String()) - for k := range tokenScope { - if strings.HasPrefix(k, "lightweight") { - // Check if this ID is cached - key := "lw:" + user.Id.OpaqueId + scopeDelimiter + ref.GetId().OpaqueId - if _, err := scopeExpansionCache.Get(key); err == nil { - return nil - } + } - shares, err := client.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{}) - if err != nil || shares.Status.Code != rpc.Code_CODE_OK { - log.Warn().Err(err).Msg("error listing received shares") - continue - } - for _, s := range shares.Shares { - shareKey := "lw:" + user.Id.OpaqueId + scopeDelimiter + s.Share.Id.OpaqueId - _ = scopeExpansionCache.SetWithExpire(shareKey, nil, scopeCacheExpiration*time.Second) - - if ref.GetId() != nil && ref.GetId().OpaqueId == s.Share.Id.OpaqueId { - return nil - } - if key := ref.GetKey(); key != nil && (utils.UserEqual(key.Owner, s.Share.Owner) || utils.UserEqual(key.Owner, s.Share.Creator)) && - utils.ResourceIDEqual(key.ResourceId, s.Share.ResourceId) && utils.GranteeEqual(key.Grantee, s.Share.Grantee) { - return nil - } - } - } - } + if checkLightweightScope(ctx, req, tokenScope, client) { + return nil } return errtypes.PermissionDenied("access to resource not allowed within the assigned scope") } -func resolveLightweightScope(ctx context.Context, ref *provider.Reference, scope *authpb.Scope, user *userpb.User, client gateway.GatewayAPIClient, mgr token.Manager) error { - // Check if this ref is cached - key := "lw:" + user.Id.OpaqueId + scopeDelimiter + getRefKey(ref) - if _, err := scopeExpansionCache.Get(key); err == nil { - return nil +func hasLightweightScope(tokenScope map[string]*authpb.Scope) bool { + for scope := range tokenScope { + if strings.HasPrefix(scope, "lightweight") { + return true + } } + return false +} - shares, err := client.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{}) - if err != nil || shares.Status.Code != rpc.Code_CODE_OK { - return errtypes.InternalError("error listing received shares") +func checkLightweightScope(ctx context.Context, req interface{}, tokenScope map[string]*authpb.Scope, client gateway.GatewayAPIClient) bool { + if !hasLightweightScope(tokenScope) { + return false } - for _, share := range shares.Shares { - shareKey := "lw:" + user.Id.OpaqueId + scopeDelimiter + resourceid.OwnCloudResourceIDWrap(share.Share.ResourceId) - _ = scopeExpansionCache.SetWithExpire(shareKey, nil, scopeCacheExpiration*time.Second) + switch r := req.(type) { + // Viewer role + case *registry.GetStorageProvidersRequest: + return true + case *provider.StatRequest: + return true + case *provider.ListContainerRequest: + return hasPermissions(ctx, client, r.GetRef(), &provider.ResourcePermissions{ + ListContainer: true, + }) + case *provider.InitiateFileDownloadRequest: + return hasPermissions(ctx, client, r.GetRef(), &provider.ResourcePermissions{ + InitiateFileDownload: true, + }) + case *appprovider.OpenInAppRequest: + return hasPermissions(ctx, client, &provider.Reference{ResourceId: r.ResourceInfo.Id}, &provider.ResourcePermissions{ + InitiateFileDownload: true, + }) + case *gateway.OpenInAppRequest: + return hasPermissions(ctx, client, r.GetRef(), &provider.ResourcePermissions{ + InitiateFileDownload: true, + }) - if ref.ResourceId != nil && utils.ResourceIDEqual(share.Share.ResourceId, ref.ResourceId) { - return nil - } - if ok, err := checkIfNestedResource(ctx, ref, share.Share.ResourceId, client, mgr); err == nil && ok { - _ = scopeExpansionCache.SetWithExpire(key, nil, scopeCacheExpiration*time.Second) - return nil - } + // Editor role + case *provider.CreateContainerRequest: + return hasPermissions(ctx, client, r.GetRef(), &provider.ResourcePermissions{ + CreateContainer: true, + }) + case *provider.TouchFileRequest: + return hasPermissions(ctx, client, r.GetRef(), &provider.ResourcePermissions{ + InitiateFileDownload: true, + }) + case *provider.DeleteRequest: + return hasPermissions(ctx, client, r.GetRef(), &provider.ResourcePermissions{ + InitiateFileDownload: true, + }) + case *provider.MoveRequest: + return hasPermissions(ctx, client, r.Source, &provider.ResourcePermissions{ + InitiateFileDownload: true, + }) && hasPermissions(ctx, client, r.Destination, &provider.ResourcePermissions{ + InitiateFileUpload: true, + }) + case *provider.InitiateFileUploadRequest: + return hasPermissions(ctx, client, r.GetRef(), &provider.ResourcePermissions{ + InitiateFileUpload: true, + }) } - return errtypes.PermissionDenied("request is not for a nested resource") + return false +} + +func hasPermissions(ctx context.Context, client gateway.GatewayAPIClient, ref *provider.Reference, permissionSet *provider.ResourcePermissions) bool { + statRes, err := client.Stat(ctx, &provider.StatRequest{ + Ref: ref, + }) + + if err != nil || statRes.Status.Code != rpc.Code_CODE_OK { + return false + } + + return utils.HasPermissions(statRes.Info.PermissionSet, permissionSet) } func resolvePublicShare(ctx context.Context, ref *provider.Reference, scope *authpb.Scope, client gateway.GatewayAPIClient, mgr token.Manager) error { From e269748fe922ee69ddb9ea08f0667e145c6923dd Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Wed, 12 Oct 2022 10:42:16 +0200 Subject: [PATCH 13/21] check parent on new file creation for lightweight accounts --- internal/grpc/interceptors/auth/scope.go | 57 +++++++++++++++++++++--- 1 file changed, 50 insertions(+), 7 deletions(-) diff --git a/internal/grpc/interceptors/auth/scope.go b/internal/grpc/interceptors/auth/scope.go index 684d250ff4..8b171ee640 100644 --- a/internal/grpc/interceptors/auth/scope.go +++ b/internal/grpc/interceptors/auth/scope.go @@ -20,6 +20,7 @@ package auth import ( "context" + "path/filepath" "strings" "time" @@ -130,11 +131,19 @@ func checkLightweightScope(ctx context.Context, req interface{}, tokenScope map[ // Editor role case *provider.CreateContainerRequest: - return hasPermissions(ctx, client, r.GetRef(), &provider.ResourcePermissions{ + parent, err := parentOfResource(ctx, client, r.GetRef()) + if err != nil { + return false + } + return hasPermissions(ctx, client, parent, &provider.ResourcePermissions{ CreateContainer: true, }) case *provider.TouchFileRequest: - return hasPermissions(ctx, client, r.GetRef(), &provider.ResourcePermissions{ + parent, err := parentOfResource(ctx, client, r.GetRef()) + if err != nil { + return false + } + return hasPermissions(ctx, client, parent, &provider.ResourcePermissions{ InitiateFileDownload: true, }) case *provider.DeleteRequest: @@ -148,7 +157,11 @@ func checkLightweightScope(ctx context.Context, req interface{}, tokenScope map[ InitiateFileUpload: true, }) case *provider.InitiateFileUploadRequest: - return hasPermissions(ctx, client, r.GetRef(), &provider.ResourcePermissions{ + parent, err := parentOfResource(ctx, client, r.GetRef()) + if err != nil { + return false + } + return hasPermissions(ctx, client, parent, &provider.ResourcePermissions{ InitiateFileUpload: true, }) } @@ -156,16 +169,46 @@ func checkLightweightScope(ctx context.Context, req interface{}, tokenScope map[ return false } -func hasPermissions(ctx context.Context, client gateway.GatewayAPIClient, ref *provider.Reference, permissionSet *provider.ResourcePermissions) bool { +func parentOfResource(ctx context.Context, client gateway.GatewayAPIClient, ref *provider.Reference) (*provider.Reference, error) { + if utils.IsAbsolutePathReference(ref) { + parent := filepath.Dir(ref.GetPath()) + info, err := stat(ctx, client, &provider.Reference{Path: parent}) + if err != nil { + return nil, err + } + return &provider.Reference{ResourceId: info.Id}, nil + } + + info, err := stat(ctx, client, ref) + if err != nil { + return nil, err + } + return &provider.Reference{ResourceId: info.ParentId}, nil +} + +func stat(ctx context.Context, client gateway.GatewayAPIClient, ref *provider.Reference) (*provider.ResourceInfo, error) { statRes, err := client.Stat(ctx, &provider.StatRequest{ Ref: ref, }) - if err != nil || statRes.Status.Code != rpc.Code_CODE_OK { - return false + switch { + case err != nil: + return nil, err + case statRes.Status.Code == rpc.Code_CODE_NOT_FOUND: + return nil, errtypes.NotFound(statRes.Status.Message) + case statRes.Status.Code != rpc.Code_CODE_OK: + return nil, errtypes.InternalError(statRes.Status.Message) } - return utils.HasPermissions(statRes.Info.PermissionSet, permissionSet) + return statRes.Info, nil +} + +func hasPermissions(ctx context.Context, client gateway.GatewayAPIClient, ref *provider.Reference, permissionSet *provider.ResourcePermissions) bool { + info, err := stat(ctx, client, ref) + if err != nil { + return false + } + return utils.HasPermissions(info.PermissionSet, permissionSet) } func resolvePublicShare(ctx context.Context, ref *provider.Reference, scope *authpb.Scope, client gateway.GatewayAPIClient, mgr token.Manager) error { From d2191560383e7be23e6dce2df49d3ad0b588d5fc Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Wed, 12 Oct 2022 11:35:51 +0200 Subject: [PATCH 14/21] get acl from parent dir for lightweight accounts --- pkg/storage/utils/eosfs/eosfs.go | 12 ++++++++++++ pkg/utils/utils.go | 5 +++++ 2 files changed, 17 insertions(+) diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go index bfe8fabbce..7909d661ad 100644 --- a/pkg/storage/utils/eosfs/eosfs.go +++ b/pkg/storage/utils/eosfs/eosfs.go @@ -2126,6 +2126,18 @@ func (fs *eosfs) permissionSet(ctx context.Context, eosFileInfo *eosclient.FileI } } + // for normal files, we need to inherit also the lw acls + // from the parent folder, as these, when creating a new + // file are not inherited + + if utils.UserIsLightweight(u) && !eosFileInfo.IsDir { + if parentPath, err := fs.unwrap(ctx, filepath.Dir(eosFileInfo.File)); err == nil { + if parent, err := fs.GetMD(ctx, &provider.Reference{Path: parentPath}, nil); err == nil { + mergePermissions(&perm, parent.PermissionSet) + } + } + } + return &perm } diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index e7d84bd432..b62278cb0c 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -380,3 +380,8 @@ func HasPermissions(target, toCheck *provider.ResourcePermissions) bool { } return true } + +func UserIsLightweight(u *userpb.User) bool { + return u.Id.Type == userpb.UserType_USER_TYPE_FEDERATED || + u.Id.Type == userpb.UserType_USER_TYPE_LIGHTWEIGHT +} From 34febfb9d49fd1ec2d483f067c21cdf24e24271c Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Wed, 12 Oct 2022 15:41:00 +0200 Subject: [PATCH 15/21] allow open for lightweight accounts --- pkg/auth/scope/lightweight.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/auth/scope/lightweight.go b/pkg/auth/scope/lightweight.go index c828f149b5..5ae5bb79f9 100644 --- a/pkg/auth/scope/lightweight.go +++ b/pkg/auth/scope/lightweight.go @@ -62,6 +62,7 @@ func checkLightweightPath(path string) bool { "/archiver", "/dataprovider", "/data", + "/open", } for _, p := range paths { if strings.HasPrefix(path, p) { From 0fe12648926073975c3c4814c3d7bd5ed82c7142 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Wed, 12 Oct 2022 17:50:42 +0200 Subject: [PATCH 16/21] fix url for apps --- pkg/auth/scope/lightweight.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/auth/scope/lightweight.go b/pkg/auth/scope/lightweight.go index 5ae5bb79f9..cc314ce1db 100644 --- a/pkg/auth/scope/lightweight.go +++ b/pkg/auth/scope/lightweight.go @@ -62,7 +62,7 @@ func checkLightweightPath(path string) bool { "/archiver", "/dataprovider", "/data", - "/open", + "/app/open", } for _, p := range paths { if strings.HasPrefix(path, p) { From 1f7982f22fe07e55ef3e09b326ece5a843b46453 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Mon, 17 Oct 2022 09:57:13 +0200 Subject: [PATCH 17/21] fix --- pkg/storage/utils/eosfs/eosfs.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go index 7909d661ad..fac31a04dd 100644 --- a/pkg/storage/utils/eosfs/eosfs.go +++ b/pkg/storage/utils/eosfs/eosfs.go @@ -26,6 +26,7 @@ import ( "net/url" "os" "path" + "path/filepath" "regexp" "strconv" "strings" From 03d28ef08e0c0834782f38d54bb7cca1cb51ce36 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Mon, 17 Oct 2022 10:01:03 +0200 Subject: [PATCH 18/21] add comments --- pkg/eosclient/eosbinary/eosbinary.go | 1 + pkg/eosclient/eosgrpc/eosgrpc.go | 1 + pkg/utils/utils.go | 4 ++++ 3 files changed, 6 insertions(+) diff --git a/pkg/eosclient/eosbinary/eosbinary.go b/pkg/eosclient/eosbinary/eosbinary.go index 7a45c30748..2a56a863c4 100644 --- a/pkg/eosclient/eosbinary/eosbinary.go +++ b/pkg/eosclient/eosbinary/eosbinary.go @@ -596,6 +596,7 @@ func (c *Client) GetAttr(ctx context.Context, auth eosclient.Authorization, key, return attr, nil } +// GetAttrs returns all the attributes of a resource func (c *Client) GetAttrs(ctx context.Context, auth eosclient.Authorization, path string) ([]*eosclient.Attribute, error) { info, err := c.getRawFileInfoByPath(ctx, auth, path) if err != nil { diff --git a/pkg/eosclient/eosgrpc/eosgrpc.go b/pkg/eosclient/eosgrpc/eosgrpc.go index de295f00d4..2c4f47c269 100644 --- a/pkg/eosclient/eosgrpc/eosgrpc.go +++ b/pkg/eosclient/eosgrpc/eosgrpc.go @@ -628,6 +628,7 @@ func (c *Client) GetAttr(ctx context.Context, auth eosclient.Authorization, key, return nil, errtypes.NotFound(fmt.Sprintf("key %s not found", key)) } +// GetAttrs returns all the attributes of a resource func (c *Client) GetAttrs(ctx context.Context, auth eosclient.Authorization, path string) ([]*eosclient.Attribute, error) { info, err := c.GetFileInfoByPath(ctx, auth, path) if err != nil { diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index b62278cb0c..be4fedf7db 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -368,6 +368,8 @@ func HasPublicShareRole(u *userpb.User) (string, bool) { return "", false } +// HasPermissions returns true if all permissions defined in the stuict toCheck +// are set in the target func HasPermissions(target, toCheck *provider.ResourcePermissions) bool { targetStruct := reflect.ValueOf(target).Elem() toCheckStruct := reflect.ValueOf(toCheck).Elem() @@ -381,6 +383,8 @@ func HasPermissions(target, toCheck *provider.ResourcePermissions) bool { return true } +// UserIsLightweight returns true if the user is a lightweith +// or federated account func UserIsLightweight(u *userpb.User) bool { return u.Id.Type == userpb.UserType_USER_TYPE_FEDERATED || u.Id.Type == userpb.UserType_USER_TYPE_LIGHTWEIGHT From c2494a22ff3de3fb59baff7324dc3f195d290d86 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Mon, 17 Oct 2022 10:03:08 +0200 Subject: [PATCH 19/21] add changelog --- changelog/unreleased/lightweigh-accounts.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changelog/unreleased/lightweigh-accounts.md diff --git a/changelog/unreleased/lightweigh-accounts.md b/changelog/unreleased/lightweigh-accounts.md new file mode 100644 index 0000000000..7984bbff28 --- /dev/null +++ b/changelog/unreleased/lightweigh-accounts.md @@ -0,0 +1,8 @@ +Enhancement: Revamp lightweigth accounts + +Re-implements the lighweight account scope check, making +it more efficient. +Also, the ACLs for the EOS storage driver for the lw +accounts are set atomically. + +https://github.com/cs3org/reva/pull/3348 \ No newline at end of file From 911fbc27d40ae5e1db3a5ed450d55ab8e270c357 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Mon, 17 Oct 2022 10:31:42 +0200 Subject: [PATCH 20/21] removed unused function --- internal/grpc/interceptors/auth/scope.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/internal/grpc/interceptors/auth/scope.go b/internal/grpc/interceptors/auth/scope.go index 8b171ee640..78d481ad36 100644 --- a/internal/grpc/interceptors/auth/scope.go +++ b/internal/grpc/interceptors/auth/scope.go @@ -389,16 +389,6 @@ func extractRef(req interface{}, tokenScope map[string]*authpb.Scope) (*provider return nil, false } -func extractShareRef(req interface{}) (*collaboration.ShareReference, bool) { - switch v := req.(type) { - case *collaboration.GetReceivedShareRequest: - return v.GetRef(), true - case *collaboration.UpdateReceivedShareRequest: - return &collaboration.ShareReference{Spec: &collaboration.ShareReference_Id{Id: v.GetShare().GetShare().GetId()}}, true - } - return nil, false -} - func getRefKey(ref *provider.Reference) string { if ref.Path != "" { return ref.Path From 55946e1959e80841fd087fdfc2e909c375bd2982 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Mon, 17 Oct 2022 10:33:23 +0200 Subject: [PATCH 21/21] fix comment in eosfs --- pkg/storage/utils/eosfs/eosfs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go index fac31a04dd..f6aa0e1103 100644 --- a/pkg/storage/utils/eosfs/eosfs.go +++ b/pkg/storage/utils/eosfs/eosfs.go @@ -1008,7 +1008,7 @@ func (fs *eosfs) AddGrant(ctx context.Context, ref *provider.Reference, g *provi } if eosACL.Type == acl.TypeLightweight { - // The ACLs for a lightweight is understandable by EOS + // The ACLs for a lightweight are not understandable by EOS // directly, but only from reva. So we have to store them // in an xattr named sys.reva.lwshare., with value // the permissions.