Skip to content

fix status codes for Space Disable/Delete, also bump reva to latest main#2975

Open
rhafer wants to merge 2 commits into
opencloud-eu:mainfrom
rhafer:bump-reva
Open

fix status codes for Space Disable/Delete, also bump reva to latest main#2975
rhafer wants to merge 2 commits into
opencloud-eu:mainfrom
rhafer:bump-reva

Conversation

@rhafer

@rhafer rhafer commented Jun 18, 2026

Copy link
Copy Markdown
Member

fix: Status codes for Space Disable/Delete

Allow a "permission denied error" from reva to bubble up to the client.
Reva was fixed to return "permission denied" only when the space to be
delete can actually be listed by the user. Other wise it will return
"not found". See reva commit 1bf72cb76394671f373e87f15f23f978cf41ab08.

So when a user with the 'can manage' role tries to purge an already
disabled space it will now get "Forbidden" status instead of a "Not
found".

Also fixes the expected status codes in the tests.

rhafer added 2 commits June 18, 2026 16:37
Allow a "permission denied error" from reva to bubble up to the client.
Reva was fixed to return "permission denied" only when the space to be
delete can actually be listed by the user. Other wise it will return
"not found". See reva commit 1bf72cb76394671f373e87f15f23f978cf41ab08.

So when a user with the 'can manage' role tries to purge an already
disabled space it will now get "Forbidden" status instead of a "Not
found".

Also fixes the expected status codes in the tests.
@rhafer rhafer requested review from ScharfViktor and aduffeck June 18, 2026 15:18
@codacy-production

codacy-production Bot commented Jun 18, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity · 0 duplication

Metric Results
Complexity 0
Duplication 0

View in Codacy

🟢 Coverage 0.00% diff coverage · -0.01% coverage variation

Metric Results
Coverage variation -0.01% coverage variation (-1.00%)
Diff coverage 0.00% diff coverage

View coverage diff in Codacy

Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (5b1b84a) 82262 18955 23.04%
Head commit (e86381d) 82262 (+0) 18943 (-12) 23.03% (-0.01%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#2975) 1 0 0.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@ScharfViktor ScharfViktor mentioned this pull request Jun 18, 2026
2 tasks
@rhafer

rhafer commented Jun 18, 2026

Copy link
Copy Markdown
Member Author
Scenario Outline: space admin user tries to disable the personal space       # /woodpecker/src/github.com/opencloud-eu/opencloud/tests/acceptance/features/apiSpaces/spaceManagement.feature:153
    When user "<user>" disables a space "Alice Hansen" owned by user "Alice" # SpacesContext::userDisablesSpace()
    Then the HTTP status code should be "404"                                # FeatureContext::thenTheHTTPStatusCodeShouldBe()

    Examples:
      | user  |
      | Brian |
        Failed step: Then the HTTP status code should be "404"
        HTTP status code 403 is not the expected value 404
        Failed asserting that 403 matches expected '404'.
      | Carol |

This needs more investigation. This seems to imply that SpaceManagers can list Personal Spaces, which they shouldn't be able to. Might be another reva issue.

@ScharfViktor

Copy link
Copy Markdown
Contributor

This needs more investigation. This seems to imply that SpaceManagers can list Personal Spaces, which they shouldn't be able to. Might be another reva issue.

unfortunately that’s true:
Screenshot 2026-06-18 at 18 38 34

@ScharfViktor

Copy link
Copy Markdown
Contributor

but it's not new.
checked on demo 7.1.0 rolling and localy on stable 4.0.7 - same behavior

@rhafer

rhafer commented Jun 19, 2026

Copy link
Copy Markdown
Member Author

but it's not new.
checked on demo 7.1.0 rolling and localy on stable 4.0.7 - same behavior

I've created a new bug report for that: #2979
For Space Admins it's currently also possible to update space attributes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants