From c4f9f18a39b6b06054a3d93ad5d53ae9b2660bb9 Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Thu, 15 Jun 2023 15:50:17 +0200 Subject: [PATCH 1/5] Show changes between old an new values in audit log Co-authored-by: Julian Koberg Signed-off-by: Christian Richter --- .../graph/pkg/service/v0/educationuser.go | 5 +- services/graph/pkg/service/v0/users.go | 58 +++++++++++++++++-- 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/services/graph/pkg/service/v0/educationuser.go b/services/graph/pkg/service/v0/educationuser.go index 6f8e1c2523..3ab8572a28 100644 --- a/services/graph/pkg/service/v0/educationuser.go +++ b/services/graph/pkg/service/v0/educationuser.go @@ -361,8 +361,9 @@ func (g Graph) PatchEducationUser(w http.ResponseWriter, r *http.Request) { } e := events.UserFeatureChanged{ - UserID: nameOrID, - Features: features, + UserID: nameOrID, + Features: features, + Timestamp: utils.TSNow(), } if currentUser, ok := revactx.ContextGetUser(r.Context()); ok { e.Executant = currentUser.GetId() diff --git a/services/graph/pkg/service/v0/users.go b/services/graph/pkg/service/v0/users.go index 4421e78822..ed3b962a37 100644 --- a/services/graph/pkg/service/v0/users.go +++ b/services/graph/pkg/service/v0/users.go @@ -9,6 +9,7 @@ import ( "reflect" "regexp" "sort" + "strconv" "strings" "github.com/CiscoM31/godata" @@ -612,6 +613,22 @@ func (g Graph) PatchUser(w http.ResponseWriter, r *http.Request) { return } + sanitizedPath := strings.TrimPrefix(r.URL.Path, "/graph/v1.0/") + + odataReq, err := godata.ParseRequest(r.Context(), sanitizedPath, r.URL.Query()) + if err != nil { + logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("could not get users: query error") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error()) + return + } + + oldUserValues, err := g.identityBackend.GetUser(r.Context(), nameOrID, odataReq) + if err != nil { + logger.Debug().Err(err).Str("id", nameOrID).Msg("could not get user: backend error") + errorcode.RenderError(w, r, err) + return + } + if nameOrID == "" { logger.Debug().Msg("could not update user: missing user id") errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "missing user id") @@ -627,7 +644,7 @@ func (g Graph) PatchUser(w http.ResponseWriter, r *http.Request) { } if reflect.ValueOf(*changes).IsZero() { - logger.Debug().Interface("body", r.Body).Msg("ignoring empyt request body") + logger.Debug().Interface("body", r.Body).Msg("ignoring empty request body") render.Status(r, http.StatusNoContent) render.NoContent(w, r) return @@ -649,19 +666,47 @@ func (g Graph) PatchUser(w http.ResponseWriter, r *http.Request) { fmt.Sprintf("'%s' is not a valid email address", *mail)) return } - features = append(features, events.UserFeature{Name: "email", Value: *mail}) + features = append(features, events.UserFeature{ + Name: "email", + Value: *mail, + OldValue: oldUserValues.Mail, + }) } if name, ok := changes.GetDisplayNameOk(); ok { - features = append(features, events.UserFeature{Name: "displayname", Value: *name}) + features = append(features, events.UserFeature{ + Name: "displayname", + Value: *name, + OldValue: oldUserValues.DisplayName, + }) } - if changes.HasUserType() { + if userType, ok := changes.GetUserTypeOk(); ok { if !isValidUserType(*changes.UserType) { logger.Debug().Interface("user", changes).Msg("invalid userType attribute") errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "invalid userType attribute, valid options are 'Member' or 'Guest'") return } + features = append(features, events.UserFeature{ + Name: "changeUserType", + Value: *userType, + OldValue: oldUserValues.UserType, + }) + } + + if accEnabled, ok := changes.GetAccountEnabledOk(); ok { + oldAccVal := strconv.FormatBool(oldUserValues.GetAccountEnabled()) + features = append(features, events.UserFeature{ + Name: "accountEnabled", + Value: strconv.FormatBool(*accEnabled), + OldValue: &oldAccVal, + }) + } + + if changes.HasPasswordProfile() { + features = append(features, events.UserFeature{ + Name: "passwordChanged", + }) } logger.Debug().Str("nameid", nameOrID).Interface("changes", *changes).Msg("calling update user on backend") @@ -673,8 +718,9 @@ func (g Graph) PatchUser(w http.ResponseWriter, r *http.Request) { } e := events.UserFeatureChanged{ - UserID: nameOrID, - Features: features, + UserID: nameOrID, + Features: features, + Timestamp: utils.TSNow(), } if currentUser, ok := revactx.ContextGetUser(r.Context()); ok { e.Executant = currentUser.GetId() From dd862cdf06dee810ea379b624943438325b805e1 Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Thu, 15 Jun 2023 16:09:54 +0200 Subject: [PATCH 2/5] Add event for role changes Co-authored-by: Julian Koberg Signed-off-by: Christian Richter --- .../pkg/service/v0/approleassignments.go | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/services/graph/pkg/service/v0/approleassignments.go b/services/graph/pkg/service/v0/approleassignments.go index 81fb693f37..1927d05aee 100644 --- a/services/graph/pkg/service/v0/approleassignments.go +++ b/services/graph/pkg/service/v0/approleassignments.go @@ -4,6 +4,9 @@ import ( "fmt" "net/http" + revactx "github.com/cs3org/reva/v2/pkg/ctx" + "github.com/cs3org/reva/v2/pkg/events" + "github.com/cs3org/reva/v2/pkg/utils" "github.com/go-chi/chi/v5" "github.com/go-chi/render" libregraph "github.com/owncloud/libre-graph-api-go" @@ -54,6 +57,11 @@ func (g Graph) CreateAppRoleAssignment(w http.ResponseWriter, r *http.Request) { userID := chi.URLParam(r, "userID") + // We can ignore the error, in the worst case the old role will be empty + oldRoles, _ := g.roleService.ListRoleAssignments(r.Context(), &settingssvc.ListRoleAssignmentsRequest{ + AccountUuid: userID, + }) + if appRoleAssignment.GetPrincipalId() != userID { errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, fmt.Sprintf("user id %s does not match principal id %v", userID, appRoleAssignment.GetPrincipalId())) return @@ -76,6 +84,27 @@ func (g Graph) CreateAppRoleAssignment(w http.ResponseWriter, r *http.Request) { return } + var role string + roles := oldRoles.GetAssignments() + if len(roles) > 0 { + role = roles[0].GetRoleId() + } + + e := events.UserFeatureChanged{ + UserID: userID, + Features: []events.UserFeature{ + { + Name: "roleChanged", + Value: appRoleAssignment.AppRoleId, + OldValue: &role, + }, + }, + Timestamp: utils.TSNow(), + } + if currentUser, ok := revactx.ContextGetUser(r.Context()); ok { + e.Executant = currentUser.GetId() + } + g.publishEvent(e) render.Status(r, http.StatusCreated) render.JSON(w, r, g.assignmentToAppRoleAssignment(artur.GetAssignment())) } From b64af5947791ff6088c17303f7a392294c02e41a Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Thu, 15 Jun 2023 16:12:47 +0200 Subject: [PATCH 3/5] Add changelog Co-authored-by Julian Koberg Signed-off-by: Christian Richter --- changelog/unreleased/audit-events.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/unreleased/audit-events.md diff --git a/changelog/unreleased/audit-events.md b/changelog/unreleased/audit-events.md new file mode 100644 index 0000000000..b1bbef4f51 --- /dev/null +++ b/changelog/unreleased/audit-events.md @@ -0,0 +1,6 @@ +Enhancement: Add old & new values to audit logs + +We have added old & new values to the audit logs +We have added the missing events for role changes + +https://github.com/owncloud/ocis/pull/6537 \ No newline at end of file From 4aa9b2e416807b4d4058e43d517d3d50aa6950dd Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Thu, 15 Jun 2023 16:14:23 +0200 Subject: [PATCH 4/5] Bump reva Co-authored-by Julian Koberg Signed-off-by: Christian Richter --- go.mod | 2 +- go.sum | 4 +-- .../cs3org/reva/v2/pkg/events/users.go | 6 ++-- .../utils/decomposedfs/decomposedfs.go | 28 +++++++++---------- .../pkg/storage/utils/decomposedfs/grants.go | 10 +++---- .../storage/utils/decomposedfs/metadata.go | 4 +-- .../storage/utils/decomposedfs/node/node.go | 26 +++++++++-------- .../utils/decomposedfs/node/permissions.go | 26 ++++++++++++----- .../pkg/storage/utils/decomposedfs/recycle.go | 14 +++++----- .../storage/utils/decomposedfs/revisions.go | 8 +++--- .../utils/decomposedfs/spacepermissions.go | 6 ++++ .../pkg/storage/utils/decomposedfs/spaces.go | 4 +-- .../utils/decomposedfs/upload/processing.go | 2 +- vendor/modules.txt | 2 +- 14 files changed, 81 insertions(+), 61 deletions(-) diff --git a/go.mod b/go.mod index b2567838e1..ea4e50ab1c 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/coreos/go-oidc v2.2.1+incompatible github.com/coreos/go-oidc/v3 v3.6.0 github.com/cs3org/go-cs3apis v0.0.0-20230516150832-730ac860c71d - github.com/cs3org/reva/v2 v2.14.1-0.20230614134159-186dd7ce3161 + github.com/cs3org/reva/v2 v2.14.1-0.20230615141102-1906a729ee5a github.com/disintegration/imaging v1.6.2 github.com/dutchcoders/go-clamd v0.0.0-20170520113014-b970184f4d9e github.com/egirna/icap-client v0.1.1 diff --git a/go.sum b/go.sum index ca0da6a6cf..350e288fff 100644 --- a/go.sum +++ b/go.sum @@ -625,8 +625,8 @@ github.com/crewjam/httperr v0.2.0 h1:b2BfXR8U3AlIHwNeFFvZ+BV1LFvKLlzMjzaTnZMybNo github.com/crewjam/httperr v0.2.0/go.mod h1:Jlz+Sg/XqBQhyMjdDiC+GNNRzZTD7x39Gu3pglZ5oH4= github.com/crewjam/saml v0.4.13 h1:TYHggH/hwP7eArqiXSJUvtOPNzQDyQ7vwmwEqlFWhMc= github.com/crewjam/saml v0.4.13/go.mod h1:igEejV+fihTIlHXYP8zOec3V5A8y3lws5bQBFsTm4gA= -github.com/cs3org/reva/v2 v2.14.1-0.20230614134159-186dd7ce3161 h1:rtitZ/GDtdkP+R56mK7DXzrFuNhi5JGdC0SO3JV7ZJU= -github.com/cs3org/reva/v2 v2.14.1-0.20230614134159-186dd7ce3161/go.mod h1:E32krZG159YflDSjDWfx/QGIC2529PS5LiPnGNHu3d0= +github.com/cs3org/reva/v2 v2.14.1-0.20230615141102-1906a729ee5a h1:T+504loI0dvksiutkzF4ciLfIJD/LJtJFdFJ6d7I7dA= +github.com/cs3org/reva/v2 v2.14.1-0.20230615141102-1906a729ee5a/go.mod h1:E32krZG159YflDSjDWfx/QGIC2529PS5LiPnGNHu3d0= github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8 h1:Z9lwXumT5ACSmJ7WGnFl+OMLLjpz5uR2fyz7dC255FI= github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8/go.mod h1:4abs/jPXcmJzYoYGF91JF9Uq9s/KL5n1jvFDix8KcqY= github.com/cyberdelia/templates v0.0.0-20141128023046-ca7fffd4298c/go.mod h1:GyV+0YP4qX0UQ7r2MoYZ+AvYDp12OF5yg4q8rGnyNh4= diff --git a/vendor/github.com/cs3org/reva/v2/pkg/events/users.go b/vendor/github.com/cs3org/reva/v2/pkg/events/users.go index 205af508a0..1a118b7199 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/events/users.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/events/users.go @@ -55,9 +55,9 @@ func (UserDeleted) Unmarshal(v []byte) (interface{}, error) { // UserFeature represents a user feature type UserFeature struct { - Name string - Value string - Timestamp *types.Timestamp + Name string + Value string + OldValue *string } // UserFeatureChanged is emitted when a user feature was changed diff --git a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/decomposedfs.go b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/decomposedfs.go index 3065a168aa..ebcafa4f3b 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/decomposedfs.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/decomposedfs.go @@ -427,7 +427,7 @@ func (fs *Decomposedfs) GetQuota(ctx context.Context, ref *provider.Reference) ( rp, err := fs.p.AssemblePermissions(ctx, n) switch { case err != nil: - return 0, 0, 0, errtypes.InternalError(err.Error()) + return 0, 0, 0, err case !rp.GetQuota && !fs.p.ListAllSpaces(ctx): f, _ := storagespace.FormatReference(ref) if rp.Stat { @@ -534,7 +534,7 @@ func (fs *Decomposedfs) GetPathByID(ctx context.Context, id *provider.ResourceId rp, err := fs.p.AssemblePermissions(ctx, n) switch { case err != nil: - return "", errtypes.InternalError(err.Error()) + return "", err case !rp.GetPath: f := storagespace.FormatResourceID(*id) if rp.Stat { @@ -581,7 +581,7 @@ func (fs *Decomposedfs) CreateDir(ctx context.Context, ref *provider.Reference) rp, err := fs.p.AssemblePermissions(ctx, n) switch { case err != nil: - return errtypes.InternalError(err.Error()) + return err case !rp.CreateContainer: f, _ := storagespace.FormatReference(ref) if rp.Stat { @@ -637,7 +637,7 @@ func (fs *Decomposedfs) TouchFile(ctx context.Context, ref *provider.Reference, rp, err := fs.p.AssemblePermissions(ctx, n) switch { case err != nil: - return errtypes.InternalError(err.Error()) + return err case !rp.InitiateFileUpload: f, _ := storagespace.FormatReference(ref) if rp.Stat { @@ -680,7 +680,7 @@ func (fs *Decomposedfs) Move(ctx context.Context, oldRef, newRef *provider.Refer rp, err := fs.p.AssemblePermissions(ctx, oldNode) switch { case err != nil: - return errtypes.InternalError(err.Error()) + return err case !rp.Move: f, _ := storagespace.FormatReference(oldRef) if rp.Stat { @@ -700,7 +700,7 @@ func (fs *Decomposedfs) Move(ctx context.Context, oldRef, newRef *provider.Refer rp, err = fs.p.AssemblePermissions(ctx, newNode) switch { case err != nil: - return errtypes.InternalError(err.Error()) + return err case oldNode.IsDir() && !rp.CreateContainer: f, _ := storagespace.FormatReference(newRef) if rp.Stat { @@ -746,7 +746,7 @@ func (fs *Decomposedfs) GetMD(ctx context.Context, ref *provider.Reference, mdKe rp, err := fs.p.AssemblePermissions(ctx, node) switch { case err != nil: - return nil, errtypes.InternalError(err.Error()) + return nil, err case !rp.Stat: f, _ := storagespace.FormatReference(ref) return nil, errtypes.NotFound(f) @@ -790,7 +790,7 @@ func (fs *Decomposedfs) ListFolder(ctx context.Context, ref *provider.Reference, rp, err := fs.p.AssemblePermissions(ctx, n) switch { case err != nil: - return nil, errtypes.InternalError(err.Error()) + return nil, err case !rp.ListContainer: f, _ := storagespace.FormatReference(ref) if rp.Stat { @@ -881,7 +881,7 @@ func (fs *Decomposedfs) Delete(ctx context.Context, ref *provider.Reference) (er rp, err := fs.p.AssemblePermissions(ctx, node) switch { case err != nil: - return errtypes.InternalError(err.Error()) + return err case !rp.Delete: f, _ := storagespace.FormatReference(ref) if rp.Stat { @@ -921,7 +921,7 @@ func (fs *Decomposedfs) Download(ctx context.Context, ref *provider.Reference) ( rp, err := fs.p.AssemblePermissions(ctx, node) switch { case err != nil: - return nil, errtypes.InternalError(err.Error()) + return nil, err case !rp.InitiateFileDownload: f, _ := storagespace.FormatReference(ref) if rp.Stat { @@ -952,7 +952,7 @@ func (fs *Decomposedfs) GetLock(ctx context.Context, ref *provider.Reference) (* rp, err := fs.p.AssemblePermissions(ctx, node) switch { case err != nil: - return nil, errtypes.InternalError(err.Error()) + return nil, err case !rp.InitiateFileDownload: f, _ := storagespace.FormatReference(ref) if rp.Stat { @@ -978,7 +978,7 @@ func (fs *Decomposedfs) SetLock(ctx context.Context, ref *provider.Reference, lo rp, err := fs.p.AssemblePermissions(ctx, node) switch { case err != nil: - return errtypes.InternalError(err.Error()) + return err case !rp.InitiateFileUpload: f, _ := storagespace.FormatReference(ref) if rp.Stat { @@ -1008,7 +1008,7 @@ func (fs *Decomposedfs) RefreshLock(ctx context.Context, ref *provider.Reference rp, err := fs.p.AssemblePermissions(ctx, node) switch { case err != nil: - return errtypes.InternalError(err.Error()) + return err case !rp.InitiateFileUpload: f, _ := storagespace.FormatReference(ref) if rp.Stat { @@ -1038,7 +1038,7 @@ func (fs *Decomposedfs) Unlock(ctx context.Context, ref *provider.Reference, loc rp, err := fs.p.AssemblePermissions(ctx, node) switch { case err != nil: - return errtypes.InternalError(err.Error()) + return err case !rp.InitiateFileUpload: // TODO do we need a dedicated permission? f, _ := storagespace.FormatReference(ref) if rp.Stat { diff --git a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/grants.go b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/grants.go index 0e162e6aab..3c053d71da 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/grants.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/grants.go @@ -63,7 +63,7 @@ func (fs *Decomposedfs) DenyGrant(ctx context.Context, ref *provider.Reference, switch { case err != nil: - return errtypes.InternalError(err.Error()) + return err case !rp.DenyGrant: return errtypes.PermissionDenied(filepath.Join(grantNode.ParentID, grantNode.Name)) } @@ -102,7 +102,7 @@ func (fs *Decomposedfs) AddGrant(ctx context.Context, ref *provider.Reference, g rp, err := fs.p.AssemblePermissions(ctx, grantNode) switch { case err != nil: - return errtypes.InternalError(err.Error()) + return err case !rp.AddGrant: f, _ := storagespace.FormatReference(ref) if rp.Stat { @@ -128,7 +128,7 @@ func (fs *Decomposedfs) ListGrants(ctx context.Context, ref *provider.Reference) rp, err := fs.p.AssemblePermissions(ctx, grantNode) switch { case err != nil: - return nil, errtypes.InternalError(err.Error()) + return nil, err case !rp.ListGrants && !rp.Stat: f, _ := storagespace.FormatReference(ref) return nil, errtypes.NotFound(f) @@ -186,7 +186,7 @@ func (fs *Decomposedfs) RemoveGrant(ctx context.Context, ref *provider.Reference rp, err := fs.p.AssemblePermissions(ctx, grantNode) switch { case err != nil: - return errtypes.InternalError(err.Error()) + return err case !rp.RemoveGrant: f, _ := storagespace.FormatReference(ref) if rp.Stat { @@ -262,7 +262,7 @@ func (fs *Decomposedfs) UpdateGrant(ctx context.Context, ref *provider.Reference rp, err := fs.p.AssemblePermissions(ctx, grantNode) switch { case err != nil: - return errtypes.InternalError(err.Error()) + return err case !rp.UpdateGrant: f, _ := storagespace.FormatReference(ref) if rp.Stat { diff --git a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/metadata.go b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/metadata.go index 31c96fef0a..97a35b346d 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/metadata.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/metadata.go @@ -52,7 +52,7 @@ func (fs *Decomposedfs) SetArbitraryMetadata(ctx context.Context, ref *provider. rp, err := fs.p.AssemblePermissions(ctx, n) switch { case err != nil: - return errtypes.InternalError(err.Error()) + return err case !rp.InitiateFileUpload: // TODO add explicit SetArbitraryMetadata grant to CS3 api, tracked in https://github.com/cs3org/cs3apis/issues/91 f, _ := storagespace.FormatReference(ref) if rp.Stat { @@ -145,7 +145,7 @@ func (fs *Decomposedfs) UnsetArbitraryMetadata(ctx context.Context, ref *provide rp, err := fs.p.AssemblePermissions(ctx, n) switch { case err != nil: - return errtypes.InternalError(err.Error()) + return err case !rp.InitiateFileUpload: // TODO use SetArbitraryMetadata grant to CS3 api, tracked in https://github.com/cs3org/cs3apis/issues/91 f, _ := storagespace.FormatReference(ref) if rp.Stat { diff --git a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node/node.go b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node/node.go index ea86aaee29..f154d2db02 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node/node.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node/node.go @@ -406,11 +406,11 @@ func (n *Node) Child(ctx context.Context, name string) (*Node, error) { } // ParentWithReader returns the parent node -func (n *Node) ParentWithReader(r io.Reader) (p *Node, err error) { +func (n *Node) ParentWithReader(r io.Reader) (*Node, error) { if n.ParentID == "" { return nil, fmt.Errorf("decomposedfs: root has no parent") } - p = &Node{ + p := &Node{ SpaceID: n.SpaceID, lu: n.lu, ID: n.ParentID, @@ -418,17 +418,19 @@ func (n *Node) ParentWithReader(r io.Reader) (p *Node, err error) { } // fill metadata cache using the reader - _, _ = p.XattrsWithReader(r) - - // lookup name and parent id in extended attributes - p.ParentID, _ = p.XattrString(prefixes.ParentidAttr) - p.Name, _ = p.XattrString(prefixes.NameAttr) - - // check node exists - if _, err := os.Stat(p.InternalPath()); err == nil { - p.Exists = true + attrs, err := p.XattrsWithReader(r) + switch { + case metadata.IsNotExist(err): + return p, nil // swallow not found, the node defaults to exists = false + case err != nil: + return nil, err } - return + p.Exists = true + + p.Name = attrs.String(prefixes.NameAttr) + p.ParentID = attrs.String(prefixes.ParentidAttr) + + return p, err } // Parent returns the parent node diff --git a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node/permissions.go b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node/permissions.go index 0899ba2c69..2401510f35 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node/permissions.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node/permissions.go @@ -98,6 +98,16 @@ func NewPermissions(lu PathLookup) *Permissions { // AssemblePermissions will assemble the permissions for the current user on the given node, taking into account all parent nodes func (p *Permissions) AssemblePermissions(ctx context.Context, n *Node) (ap provider.ResourcePermissions, err error) { + return p.assemblePermissions(ctx, n, true) +} + +// AssembleTrashPermissions will assemble the permissions for the current user on the given node, taking into account all parent nodes +func (p *Permissions) AssembleTrashPermissions(ctx context.Context, n *Node) (ap provider.ResourcePermissions, err error) { + return p.assemblePermissions(ctx, n, false) +} + +// assemblePermissions will assemble the permissions for the current user on the given node, taking into account all parent nodes +func (p *Permissions) assemblePermissions(ctx context.Context, n *Node, failOnTrashedSubtree bool) (ap provider.ResourcePermissions, err error) { u, ok := ctxpkg.ContextGetUser(ctx) if !ok { return NoPermissions(), nil @@ -114,16 +124,9 @@ func (p *Permissions) AssemblePermissions(ctx context.Context, n *Node) (ap prov n.ID = kp[0] } - // check if the current user is the owner - if utils.UserIDEqual(u.Id, n.Owner()) { - return OwnerPermissions(), nil - } // determine root - rn := n.SpaceRoot - cn := n - ap = provider.ResourcePermissions{} // for an efficient group lookup convert the list of groups to a map @@ -154,6 +157,10 @@ func (p *Permissions) AssemblePermissions(ctx context.Context, n *Node) (ap prov // We do not have a parent, so we assume the next valid parent is the spaceRoot (which must always exist) cn = n.SpaceRoot } + if failOnTrashedSubtree && !cn.Exists { + return NoPermissions(), errtypes.NotFound(n.ID) + } + } // for the root node @@ -167,6 +174,11 @@ func (p *Permissions) AssemblePermissions(ctx context.Context, n *Node) (ap prov appctx.GetLogger(ctx).Error().Err(err).Interface("node", cn.ID).Msg("error reading root node permissions") } + // check if the current user is the owner + if utils.UserIDEqual(u.Id, n.Owner()) { + return OwnerPermissions(), nil + } + appctx.GetLogger(ctx).Debug().Interface("permissions", ap).Interface("node", n.ID).Interface("user", u).Msg("returning agregated permissions") return ap, nil } diff --git a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/recycle.go b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/recycle.go index 18598ef980..8cbbc120fa 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/recycle.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/recycle.go @@ -62,10 +62,10 @@ func (fs *Decomposedfs) ListRecycle(ctx context.Context, ref *provider.Reference if err != nil { return nil, err } - rp, err := fs.p.AssemblePermissions(ctx, trashnode) + rp, err := fs.p.AssembleTrashPermissions(ctx, trashnode) switch { case err != nil: - return nil, errtypes.InternalError(err.Error()) + return nil, err case !rp.ListRecycle: if rp.Stat { return nil, errtypes.PermissionDenied(key) @@ -300,10 +300,10 @@ func (fs *Decomposedfs) RestoreRecycleItem(ctx context.Context, ref *provider.Re } // check permissions of deleted node - rp, err := fs.p.AssemblePermissions(ctx, rn) + rp, err := fs.p.AssembleTrashPermissions(ctx, rn) switch { case err != nil: - return errtypes.InternalError(err.Error()) + return err case !rp.RestoreRecycleItem: if rp.Stat { return errtypes.PermissionDenied(key) @@ -318,7 +318,7 @@ func (fs *Decomposedfs) RestoreRecycleItem(ctx context.Context, ref *provider.Re pp, err := fs.p.AssemblePermissions(ctx, parent) switch { case err != nil: - return errtypes.InternalError(err.Error()) + return err case !pp.InitiateFileUpload: // share receiver cannot restore to a shared resource to which she does not have write permissions. if rp.Stat { @@ -346,10 +346,10 @@ func (fs *Decomposedfs) PurgeRecycleItem(ctx context.Context, ref *provider.Refe } // check permissions of deleted node - rp, err := fs.p.AssemblePermissions(ctx, rn) + rp, err := fs.p.AssembleTrashPermissions(ctx, rn) switch { case err != nil: - return errtypes.InternalError(err.Error()) + return err case !rp.PurgeRecycle: if rp.Stat { return errtypes.PermissionDenied(key) diff --git a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/revisions.go b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/revisions.go index e023ee98ac..be40c727f9 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/revisions.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/revisions.go @@ -57,7 +57,7 @@ func (fs *Decomposedfs) ListRevisions(ctx context.Context, ref *provider.Referen rp, err := fs.p.AssemblePermissions(ctx, n) switch { case err != nil: - return nil, errtypes.InternalError(err.Error()) + return nil, err case !rp.ListFileVersions: f, _ := storagespace.FormatReference(ref) if rp.Stat { @@ -136,7 +136,7 @@ func (fs *Decomposedfs) DownloadRevision(ctx context.Context, ref *provider.Refe rp, err := fs.p.AssemblePermissions(ctx, n) switch { case err != nil: - return nil, errtypes.InternalError(err.Error()) + return nil, err case !rp.ListFileVersions || !rp.InitiateFileDownload: // TODO add explicit permission in the CS3 api? f, _ := storagespace.FormatReference(ref) if rp.Stat { @@ -190,7 +190,7 @@ func (fs *Decomposedfs) RestoreRevision(ctx context.Context, ref *provider.Refer rp, err := fs.p.AssemblePermissions(ctx, n) switch { case err != nil: - return errtypes.InternalError(err.Error()) + return err case !rp.RestoreFileVersion: f, _ := storagespace.FormatReference(ref) if rp.Stat { @@ -327,7 +327,7 @@ func (fs *Decomposedfs) getRevisionNode(ctx context.Context, ref *provider.Refer p, err := fs.p.AssemblePermissions(ctx, n) switch { case err != nil: - return nil, errtypes.InternalError(err.Error()) + return nil, err case !hasPermission(&p): return nil, errtypes.PermissionDenied(filepath.Join(n.ParentID, n.Name)) } diff --git a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/spacepermissions.go b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/spacepermissions.go index 010484a066..0a8d990b86 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/spacepermissions.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/spacepermissions.go @@ -17,6 +17,7 @@ import ( // PermissionsChecker defines an interface for checking permissions on a Node type PermissionsChecker interface { AssemblePermissions(ctx context.Context, n *node.Node) (ap provider.ResourcePermissions, err error) + AssembleTrashPermissions(ctx context.Context, n *node.Node) (ap provider.ResourcePermissions, err error) } // CS3PermissionsClient defines an interface for checking permissions against the CS3 permissions service @@ -40,6 +41,11 @@ func (p Permissions) AssemblePermissions(ctx context.Context, n *node.Node) (pro return p.item.AssemblePermissions(ctx, n) } +// AssembleTrashPermissions is used to assemble file permissions +func (p Permissions) AssembleTrashPermissions(ctx context.Context, n *node.Node) (provider.ResourcePermissions, error) { + return p.item.AssembleTrashPermissions(ctx, n) +} + // CreateSpace returns true when the user is allowed to create the space func (p Permissions) CreateSpace(ctx context.Context, spaceid string) bool { return p.checkPermission(ctx, "Drives.Create", spaceRef(spaceid)) diff --git a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/spaces.go b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/spaces.go index 77fda61361..b78ec624e6 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/spaces.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/spaces.go @@ -554,7 +554,7 @@ func (fs *Decomposedfs) UpdateStorageSpace(ctx context.Context, req *provider.Up sp, err := fs.p.AssemblePermissions(ctx, spaceNode) if err != nil { return &provider.UpdateStorageSpaceResponse{ - Status: &v1beta11.Status{Code: v1beta11.Code_CODE_INVALID, Message: err.Error()}, + Status: status.NewStatusFromErrType(ctx, "assembling permissions failed", err), }, nil } @@ -793,7 +793,7 @@ func (fs *Decomposedfs) storageSpaceFromNode(ctx context.Context, n *node.Node, rp, err := fs.p.AssemblePermissions(ctx, n) switch { case err != nil: - return nil, errtypes.InternalError(err.Error()) + return nil, err case !rp.Stat: return nil, errtypes.NotFound(fmt.Sprintf("space %s not found", n.ID)) } diff --git a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/upload/processing.go b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/upload/processing.go index 3ba1a04451..1af0cfb6e4 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/upload/processing.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/upload/processing.go @@ -114,7 +114,7 @@ func New(ctx context.Context, info tusd.FileInfo, lu *lookup.Lookup, tp Tree, p rp, err := p.AssemblePermissions(ctx, checkNode) switch { case err != nil: - return nil, errtypes.InternalError(err.Error()) + return nil, err case !rp.InitiateFileUpload: return nil, errtypes.PermissionDenied(path) } diff --git a/vendor/modules.txt b/vendor/modules.txt index 0a25c1e199..c8906b155b 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -352,7 +352,7 @@ github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1 github.com/cs3org/go-cs3apis/cs3/storage/registry/v1beta1 github.com/cs3org/go-cs3apis/cs3/tx/v1beta1 github.com/cs3org/go-cs3apis/cs3/types/v1beta1 -# github.com/cs3org/reva/v2 v2.14.1-0.20230614134159-186dd7ce3161 +# github.com/cs3org/reva/v2 v2.14.1-0.20230615141102-1906a729ee5a ## explicit; go 1.20 github.com/cs3org/reva/v2/cmd/revad/internal/grace github.com/cs3org/reva/v2/cmd/revad/runtime From 9198dd9a67cea9675d7c25c49f0d01bdbb080489 Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Fri, 16 Jun 2023 11:56:03 +0200 Subject: [PATCH 5/5] Fix tests Co-authored-by: Julian Koberg Signed-off-by: Christian Richter --- services/graph/pkg/service/v0/approleassignments_test.go | 6 ++++++ services/graph/pkg/service/v0/users_test.go | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/services/graph/pkg/service/v0/approleassignments_test.go b/services/graph/pkg/service/v0/approleassignments_test.go index b67165c526..7315df703e 100644 --- a/services/graph/pkg/service/v0/approleassignments_test.go +++ b/services/graph/pkg/service/v0/approleassignments_test.go @@ -139,6 +139,12 @@ var _ = Describe("AppRoleAssignments", func() { AccountUuid: user.GetId(), RoleId: "some-appRole-ID", } + roleService.On("ListRoleAssignments", mock.Anything, mock.Anything, mock.Anything).Return(&settings.ListRoleAssignmentsResponse{ + Assignments: []*settingsmsg.UserRoleAssignment{ + userRoleAssignment, + }, + }, nil) + roleService.On("AssignRoleToUser", mock.Anything, mock.Anything, mock.Anything).Return(&settings.AssignRoleToUserResponse{Assignment: userRoleAssignment}, nil) ara := libregraph.NewAppRoleAssignmentWithDefaults() diff --git a/services/graph/pkg/service/v0/users_test.go b/services/graph/pkg/service/v0/users_test.go index 5dbe88f5f7..38ee843499 100644 --- a/services/graph/pkg/service/v0/users_test.go +++ b/services/graph/pkg/service/v0/users_test.go @@ -809,7 +809,7 @@ var _ = Describe("Users", func() { user.SetMail("user@example.com") user.SetId("/users/user") - identityBackend.On("GetUser", mock.Anything, mock.Anything, mock.Anything).Return(&user, nil) + identityBackend.On("GetUser", mock.Anything, mock.Anything, mock.Anything).Return(user, nil) }) It("handles missing userids", func() { @@ -866,7 +866,7 @@ var _ = Describe("Users", func() { data, err := json.Marshal(user) Expect(err).ToNot(HaveOccurred()) - r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/users?$invalid=true", bytes.NewBuffer(data)) + r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/users", bytes.NewBuffer(data)) rctx := chi.NewRouteContext() rctx.URLParams.Add("userID", user.GetId()) r = r.WithContext(context.WithValue(revactx.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))