mirror/opencloud
1
0
Fork 0
mirror of https://github.com/opencloud-eu/opencloud.git synced 2026-04-26 10:18:42 -04:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #6220 from 2403905/issue-5031

Browse Source 
fix Graph delete request leaks existence of space #5031
This commit is contained in:
Michael Barz
2023-05-08 17:13:47 +02:00
committed by GitHub
parent 428e696eb0 8bbf4ababe
commit e4d674a30d
11 changed files with 48 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
.gitignore vendored
View File

@@ -50,5 +50,6 @@ protogen/buf.sha1.lock
# misc
go.work
go.work.sum
.env
.envrc

6
changelog/unreleased/fix-leaks-existence.md Normal file
View File

@@ -0,0 +1,6 @@
Bugfix: Hide the existence of space when deleting/updating
The "code": "notAllowed" changed to "code": "itemNotFound"
https://github.com/owncloud/ocis/issues/5031
https://github.com/owncloud/ocis/pull/6220

2
go.mod
View File

@@ -13,7 +13,7 @@ require (
github.com/coreos/go-oidc v2.2.1+incompatible
github.com/coreos/go-oidc/v3 v3.4.0
github.com/cs3org/go-cs3apis v0.0.0-20221012090518-ef2996678965
github.com/cs3org/reva/v2 v2.13.2-0.20230504093557-756a84314af0
github.com/cs3org/reva/v2 v2.13.2-0.20230508134639-beefb0242916
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

4
go.sum
View File

@@ -627,8 +627,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.13.2-0.20230504093557-756a84314af0 h1:KJHTdnQpEB3hcOSNXtMFedAvplpNRepo3RPArWFiSYo=
github.com/cs3org/reva/v2 v2.13.2-0.20230504093557-756a84314af0/go.mod h1:VxBmpOvIKlgKLPOsHun+fABopzX+3ZELPAp3N5bQMsM=
github.com/cs3org/reva/v2 v2.13.2-0.20230508134639-beefb0242916 h1:ouFQ/dE5UyHN3kAzL12JnGZmzF0P1LfeByUyXVYKb/A=
github.com/cs3org/reva/v2 v2.13.2-0.20230508134639-beefb0242916/go.mod h1:VxBmpOvIKlgKLPOsHun+fABopzX+3ZELPAp3N5bQMsM=
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=

12
services/graph/pkg/service/v0/drives.go
View File

@@ -468,11 +468,11 @@ func (g Graph) UpdateDrive(w http.ResponseWriter, r *http.Request) {
switch resp.Status.GetCode() {
case cs3rpc.Code_CODE_NOT_FOUND:
logger.Debug().Interface("id", rid).Msg("could not update drive: drive not found")
errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, resp.GetStatus().GetMessage())
errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, "drive not found")
return
case cs3rpc.Code_CODE_PERMISSION_DENIED:
logger.Debug().Interface("id", rid).Msg("could not update drive, permission denied")
errorcode.NotAllowed.Render(w, r, http.StatusForbidden, resp.GetStatus().GetMessage())
errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, "drive not found")
return
case cs3rpc.Code_CODE_INVALID_ARGUMENT:
logger.Debug().Interface("id", rid).Msg("could not update drive, invalid argument")
@@ -480,7 +480,7 @@ func (g Graph) UpdateDrive(w http.ResponseWriter, r *http.Request) {
return
default:
logger.Debug().Interface("id", rid).Str("grpc", resp.GetStatus().GetMessage()).Msg("could not update drive: grpc error")
errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, resp.GetStatus().GetMessage())
errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, "grpc error")
return
}
}
@@ -1054,7 +1054,11 @@ func (g Graph) DeleteDrive(w http.ResponseWriter, r *http.Request) {
return
case cs3rpc.Code_CODE_PERMISSION_DENIED:
logger.Debug().Interface("id", rid).Msg("could not delete drive: permission denied")
errorcode.NotAllowed.Render(w, r, http.StatusForbidden, "permission denied to delete drive")
errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, "drive not found")
return
case cs3rpc.Code_CODE_NOT_FOUND:
logger.Debug().Interface("id", rid).Msg("could not delete drive: drive not found")
errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, "drive not found")
return
// don't expose internal error codes to the outside world
default:

8
tests/acceptance/features/apiSpaces/changeSpaces.feature
View File

@@ -1,4 +1,4 @@
@api
@api
Feature: Change data of space
As a user with space admin rights
I want to be able to change the meta-data of a created space (increase the quota, change name, etc.)
@@ -51,7 +51,7 @@ Feature: Change data of space
Scenario Outline: user other than space manager role can't change the name of a Space via the Graph API
When user "<user>" changes the name of the "Project Jupiter" space to "Project Jupiter"
Then the HTTP status code should be "403"
Then the HTTP status code should be "404"
Examples:
| user |
| Brian |
@@ -90,7 +90,7 @@ Feature: Change data of space
Scenario Outline: viewer and editor cannot change the description(subtitle) of a space via the Graph API
When user "<user>" changes the description of the "Project Jupiter" space to "The Death Star is a fictional mobile space station"
Then the HTTP status code should be "403"
Then the HTTP status code should be "404"
Examples:
| user |
| Brian |
@@ -334,7 +334,7 @@ Feature: Change data of space
Given user "Alice" has created a folder ".space" in space "Project Jupiter"
And user "Alice" has uploaded a file inside space "Project Jupiter" with content "" to ".space/someImageFile.jpg"
When user "Bob" sets the file ".space/someImageFile.jpg" as a space image in a special section of the "Project Jupiter" space
Then the HTTP status code should be "403"
Then the HTTP status code should be "404"
Scenario Outline: user set new readme file as description of the space via the graph API

6
tests/acceptance/features/apiSpaces/disableAndDeleteSpaces.feature
View File

@@ -1,4 +1,4 @@
@api
@api
Feature: Disabling and deleting space
As a manager of space
I want to be able to disable the space first, then delete it.
@@ -41,7 +41,7 @@ Feature: Disabling and deleting space
Scenario Outline: user with role user and guest cannot disable other space via the Graph API
Given the administrator has given "Carol" the role "<role>" using the settings api
When user "Carol" tries to disable a space "Project Moon" owned by user "Alice"
Then the HTTP status code should be "403"
Then the HTTP status code should be "404"
And the user "Brian" should have a space called "Project Moon"
And the user "Bob" should have a space called "Project Moon"
Examples:
@@ -115,7 +115,7 @@ Feature: Disabling and deleting space
Given the administrator has given "Carol" the role "<role>" using the settings api
And user "Alice" has disabled a space "Project Moon"
When user "Carol" tries to delete a space "Project Moon" owned by user "Alice"
Then the HTTP status code should be "403"
Then the HTTP status code should be "404"
Examples:
| role |
| User |

12
tests/acceptance/features/apiSpaces/spaceManagement.feature
View File

@@ -1,4 +1,4 @@
@api
@api
Feature: Space management
As a user with space admin permission
I want to be able to manage all existing project spaces
@@ -113,7 +113,7 @@ Feature: Space management
Scenario: user without space admin permission tries to change the name of the project space
When user "Carol" tries to change the name of the "Project" space to "New Name" owned by user "Alice"
Then the HTTP status code should be "403"
Then the HTTP status code should be "404"
And the user "Alice" should have a space called "Project"
@skipOnStable2.0
@@ -140,7 +140,7 @@ Feature: Space management
Scenario: user without space admin permission tries to change the description of the project space
Given user "Alice" has changed the description of the "Project" space to "old description"
When user "Carol" tries to change the description of the "Project" space to "New description" owned by user "Alice"
Then the HTTP status code should be "403"
Then the HTTP status code should be "404"
@skipOnStable2.0
Scenario: space admin user disables the project space
@@ -151,12 +151,12 @@ Feature: Space management
Scenario: user without space admin permission tries to disable the project space
When user "Carol" tries to disable a space "Project" owned by user "Alice"
Then the HTTP status code should be "403"
Then the HTTP status code should be "404"
Scenario Outline: space admin user tries to disable the personal space
When user "<user>" disables a space "Alice Hansen" owned by user "Alice"
Then the HTTP status code should be "403"
Then the HTTP status code should be "404"
Examples:
| user |
| Brian |
@@ -173,7 +173,7 @@ Feature: Space management
Scenario: user without space admin permission tries to delete the project space
Given user "Alice" has disabled a space "Project"
When user "Carol" tries to delete a space "Project" owned by user "Alice"
Then the HTTP status code should be "403"
Then the HTTP status code should be "404"
@skipOnStable2.0
Scenario: space admin user enables the project space

12
vendor/github.com/cs3org/reva/v2/internal/grpc/services/storageprovider/storageprovider.go generated vendored
View File

@@ -596,10 +596,12 @@ func (s *service) DeleteStorageSpace(ctx context.Context, req *provider.DeleteSt
id := &provider.StorageSpaceId{OpaqueId: storagespace.FormatResourceID(idraw)}
spaces, err := s.storage.ListStorageSpaces(ctx, []*provider.ListStorageSpacesRequest_Filter{{Type: provider.ListStorageSpacesRequest_Filter_TYPE_ID, Term: &provider.ListStorageSpacesRequest_Filter_Id{Id: id}}}, true)
if err != nil || len(spaces) != 1 {
if err != nil {
var st *rpc.Status
switch err.(type) {
case errtypes.IsNotFound, errtypes.PermissionDenied:
case errtypes.IsNotFound:
st = status.NewNotFound(ctx, "space not found")
case errtypes.PermissionDenied:
st = status.NewPermissionDenied(ctx, err, "permission denied")
case errtypes.BadRequest:
st = status.NewInvalid(ctx, err.Error())
@@ -609,13 +611,17 @@ func (s *service) DeleteStorageSpace(ctx context.Context, req *provider.DeleteSt
return &provider.DeleteStorageSpaceResponse{
Status: st,
}, nil
} else if len(spaces) != 1 {
return &provider.DeleteStorageSpaceResponse{
Status: status.NewNotFound(ctx, "space not found"),
}, nil
}
if err := s.storage.DeleteStorageSpace(ctx, req); err != nil {
var st *rpc.Status
switch err.(type) {
case errtypes.IsNotFound:
st = status.NewNotFound(ctx, "not found when deleting space")
st = status.NewNotFound(ctx, "space not found")
case errtypes.PermissionDenied:
st = status.NewPermissionDenied(ctx, err, "permission denied")
case errtypes.BadRequest:

8
vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node/permissions.go generated vendored
View File

@@ -145,8 +145,14 @@ func (p *Permissions) AssemblePermissions(ctx context.Context, n *Node) (ap prov
appctx.GetLogger(ctx).Error().Err(err).Interface("node", cn.ID).Msg("error reading permissions")
// continue with next segment
}
if cn, err = cn.Parent(); err != nil {
return ap, errors.Wrap(err, "Decomposedfs: error getting parent for node "+cn.ID)
// We get an error but get a parent, but can not read it from disk (eg. it has been deleted already)
if cn != nil {
return ap, errors.Wrap(err, "Decomposedfs: error getting parent for node "+cn.ID)
}
// We do not have a parent, so we assume the next valid parent is the spaceRoot (which must always exist)
cn = n.SpaceRoot
}
}

2
vendor/modules.txt vendored
View File

@@ -349,7 +349,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.13.2-0.20230504093557-756a84314af0
# github.com/cs3org/reva/v2 v2.13.2-0.20230508134639-beefb0242916
## explicit; go 1.19
github.com/cs3org/reva/v2/cmd/revad/internal/grace
github.com/cs3org/reva/v2/cmd/revad/runtime