From 7e060844831057d09848c057a1a72a12fe933266 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Wed, 29 Sep 2021 15:52:17 +0200 Subject: [PATCH 1/3] Differentiate share types when retrieving received shares in sql driver --- changelog/unreleased/shares-sql-received-fix.md | 3 +++ pkg/cbox/share/sql/sql.go | 12 ++++++------ 2 files changed, 9 insertions(+), 6 deletions(-) create mode 100644 changelog/unreleased/shares-sql-received-fix.md diff --git a/changelog/unreleased/shares-sql-received-fix.md b/changelog/unreleased/shares-sql-received-fix.md new file mode 100644 index 0000000000..6690f3638c --- /dev/null +++ b/changelog/unreleased/shares-sql-received-fix.md @@ -0,0 +1,3 @@ +Bugfix: Differentiate share types when retrieving received shares in sql driver + +https://github.com/cs3org/reva/pull/2116 diff --git a/pkg/cbox/share/sql/sql.go b/pkg/cbox/share/sql/sql.go index 675f40ba0a..7fb5c0a4f2 100644 --- a/pkg/cbox/share/sql/sql.go +++ b/pkg/cbox/share/sql/sql.go @@ -328,9 +328,9 @@ func (m *mgr) ListReceivedShares(ctx context.Context, filters []*collaboration.F FROM oc_share ts LEFT JOIN oc_share_acl tr ON (ts.id = tr.id AND tr.rejected_by = ?) WHERE (orphan = 0 or orphan IS NULL) AND (uid_owner != ? AND uid_initiator != ?)` if len(user.Groups) > 0 { - query += " AND (share_with=? OR share_with in (?" + strings.Repeat(",?", len(user.Groups)-1) + "))" + query += " AND ((share_with=? AND share_type = 0) OR (share_type = 1 AND share_with in (?" + strings.Repeat(",?", len(user.Groups)-1) + ")))" } else { - query += " AND (share_with=?)" + query += " AND (share_with=? AND share_type = 0)" } for _, f := range filters { @@ -375,9 +375,9 @@ func (m *mgr) getReceivedByID(ctx context.Context, id *collaboration.ShareId) (* s := conversions.DBShare{ID: id.OpaqueId} query := "select coalesce(uid_owner, '') as uid_owner, coalesce(uid_initiator, '') as uid_initiator, coalesce(share_with, '') as share_with, coalesce(fileid_prefix, '') as fileid_prefix, coalesce(item_source, '') as item_source, stime, permissions, share_type, accepted, coalesce(tr.rejected_by, '') as rejected_by FROM oc_share ts LEFT JOIN oc_share_acl tr ON (ts.id = tr.id AND tr.rejected_by = ?) WHERE (orphan = 0 or orphan IS NULL) AND ts.id=? " if len(user.Groups) > 0 { - query += "AND (share_with=? OR share_with in (?" + strings.Repeat(",?", len(user.Groups)-1) + "))" + query += "AND ((share_with=? AND share_type = 0) OR (share_type = 1 AND share_with in (?" + strings.Repeat(",?", len(user.Groups)-1) + ")))" } else { - query += "AND (share_with=?)" + query += "AND (share_with=? AND share_type = 0)" } if err := m.db.QueryRow(query, params...).Scan(&s.UIDOwner, &s.UIDInitiator, &s.ShareWith, &s.Prefix, &s.ItemSource, &s.STime, &s.Permissions, &s.ShareType, &s.State, &s.RejectedBy); err != nil { if err == sql.ErrNoRows { @@ -401,9 +401,9 @@ func (m *mgr) getReceivedByKey(ctx context.Context, key *collaboration.ShareKey) s := conversions.DBShare{} query := "select coalesce(uid_owner, '') as uid_owner, coalesce(uid_initiator, '') as uid_initiator, coalesce(share_with, '') as share_with, coalesce(fileid_prefix, '') as fileid_prefix, coalesce(item_source, '') as item_source, ts.id, stime, permissions, share_type, accepted, coalesce(tr.rejected_by, '') as rejected_by FROM oc_share ts LEFT JOIN oc_share_acl tr ON (ts.id = tr.id AND tr.rejected_by = ?) WHERE (orphan = 0 or orphan IS NULL) AND uid_owner=? AND fileid_prefix=? AND item_source=? AND share_type=? AND share_with=? " if len(user.Groups) > 0 { - query += "AND (share_with=? OR share_with in (?" + strings.Repeat(",?", len(user.Groups)-1) + "))" + query += "AND ((share_with=? AND share_type = 0) OR (share_type = 1 AND share_with in (?" + strings.Repeat(",?", len(user.Groups)-1) + ")))" } else { - query += "AND (share_with=?)" + query += "AND (share_with=? AND share_type = 0)" } if err := m.db.QueryRow(query, params...).Scan(&s.UIDOwner, &s.UIDInitiator, &s.ShareWith, &s.Prefix, &s.ItemSource, &s.ID, &s.STime, &s.Permissions, &s.ShareType, &s.State, &s.RejectedBy); err != nil { From 22611c0d9c6d692c1a37f17ff3811910f2d45f3d Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Tue, 5 Oct 2021 11:55:19 +0200 Subject: [PATCH 2/3] Make copy of user object when removing groups --- internal/grpc/services/gateway/authprovider.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/grpc/services/gateway/authprovider.go b/internal/grpc/services/gateway/authprovider.go index 17f8499d07..6a2eb5ae3e 100644 --- a/internal/grpc/services/gateway/authprovider.go +++ b/internal/grpc/services/gateway/authprovider.go @@ -99,7 +99,7 @@ func (s *svc) Authenticate(ctx context.Context, req *gateway.AuthenticateRequest }, nil } - u := res.User + u := *res.User if sharedconf.SkipUserGroupsInToken() { u.Groups = []string{} } @@ -109,7 +109,7 @@ func (s *svc) Authenticate(ctx context.Context, req *gateway.AuthenticateRequest // the resources referenced by these. Since the current scope can do that, // mint a temporary token based on that and expand the scope. Then set the // token obtained from the updated scope in the context. - token, err := s.tokenmgr.MintToken(ctx, u, res.TokenScope) + token, err := s.tokenmgr.MintToken(ctx, &u, res.TokenScope) if err != nil { err = errors.Wrap(err, "authsvc: error in MintToken") res := &gateway.AuthenticateResponse{ @@ -129,7 +129,7 @@ func (s *svc) Authenticate(ctx context.Context, req *gateway.AuthenticateRequest }, nil } - token, err = s.tokenmgr.MintToken(ctx, u, scope) + token, err = s.tokenmgr.MintToken(ctx, &u, scope) if err != nil { err = errors.Wrap(err, "authsvc: error in MintToken") res := &gateway.AuthenticateResponse{ From 321a9219f684bf85267c6c64bbbb8fc5385a0447 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Tue, 5 Oct 2021 12:05:35 +0200 Subject: [PATCH 3/3] Return invalid file ID in appprovider --- internal/http/services/appprovider/appprovider.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/http/services/appprovider/appprovider.go b/internal/http/services/appprovider/appprovider.go index 30f949a714..e4dc55d57d 100644 --- a/internal/http/services/appprovider/appprovider.go +++ b/internal/http/services/appprovider/appprovider.go @@ -332,12 +332,12 @@ func (s *svc) getStatInfo(ctx context.Context, fileID string, client gateway.Gat decodedID, err := base64.URLEncoding.DecodeString(fileID) if err != nil { - return nil, ocmd.APIErrorInvalidParameter, errors.Wrap(err, "fileID doesn't follow the required format") + return nil, ocmd.APIErrorInvalidParameter, errors.Wrap(err, fmt.Sprintf("fileID %s doesn't follow the required format", fileID)) } parts := strings.Split(string(decodedID), idDelimiter) if !utf8.ValidString(parts[0]) || !utf8.ValidString(parts[1]) { - return nil, ocmd.APIErrorInvalidParameter, errors.New("fileID contains illegal characters") + return nil, ocmd.APIErrorInvalidParameter, errtypes.BadRequest(fmt.Sprintf("fileID %s contains illegal characters", fileID)) } res := &provider.ResourceId{ StorageId: parts[0],