Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix Graph delete request leaks existence of space #5031 #6220

Merged
merged 3 commits into from
May 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,6 @@ protogen/buf.sha1.lock

# misc
go.work
go.work.sum
.env
.envrc
6 changes: 6 additions & 0 deletions changelog/unreleased/fix-leaks-existence.md
Original file line number Diff line number Diff line change
@@ -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 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
12 changes: 8 additions & 4 deletions services/graph/pkg/service/v0/drives.go
Original file line number Diff line number Diff line change
Expand Up @@ -468,19 +468,19 @@ 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())
micbar marked this conversation as resolved.
Show resolved Hide resolved
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")
micbar marked this conversation as resolved.
Show resolved Hide resolved
return
case cs3rpc.Code_CODE_INVALID_ARGUMENT:
logger.Debug().Interface("id", rid).Msg("could not update drive, invalid argument")
errorcode.NotAllowed.Render(w, r, http.StatusBadRequest, resp.GetStatus().GetMessage())
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
}
}
Expand Down Expand Up @@ -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:
Expand Down
8 changes: 4 additions & 4 deletions tests/acceptance/features/apiSpaces/changeSpaces.feature
Original file line number Diff line number Diff line change
@@ -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.)
Expand Down Expand Up @@ -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 |
Expand Down Expand Up @@ -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 |
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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 |
Expand Down
12 changes: 6 additions & 6 deletions tests/acceptance/features/apiSpaces/spaceManagement.feature
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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 |
Expand All @@ -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
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down