Merge pull request #6537 from dragonchaser/issue-3753-issues-with-audit-userfeaturechanged

Show changes between old an new values in audit log

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

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

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=

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()))
 }

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()

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()

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()

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))

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

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 {

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 {

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 {

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

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
 }

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)

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))
 	}

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))

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))
 		}

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)
 	}

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