Three more Copilot review nits on the colon-syntax middleware:
- CS3 Stat returning UNAUTHENTICATED now surfaces as HTTP 401 (was 500
via the default-case fallback). Distinct sentinel + handler branch.
- Item-anchored form (/drives/{driveID}/items/{itemID}:/...) now
validates that driveID's storage/space prefix matches the itemID's,
short-circuiting to 400 InvalidRequest on mismatch instead of doing
a CS3 Stat that would only fail at the handler layer.
- ParseID failures on the anchor id now surface as 400 (was 404). In
practice this branch is defensive — storagespace.ParseID is lenient
and only errors on empty input, which the regex already filters out
— but the right semantic for malformed client input is 400, not 404.
Tests: added two new cases (UNAUTHENTICATED → 401, drive/item id
mismatch → 400). The "malformed drive id" case Copilot suggested
isn't reachable in practice given ParseID's leniency, so it's not
covered.
Two follow-up Copilot review nits on the colon-syntax middleware:
- Don't blank r.URL.RawPath after the rewrite. Graph.ServeHTTP sets
RawPath = EscapedPath() as a workaround for chi's parameter-binding
quirks with `$`/`!` in IDs (see go-chi/chi#641). Clearing RawPath
for rewritten requests negates that workaround. Re-establish the
same invariant after the rewrite.
- Tests previously used log.NewLogger(), which mutates global zerolog
state. Switch to log.NopLogger() — order-independent, no global
side effects.
- Drop double-decoding of URL path components. r.URL.Path is already
decoded by net/http; calling url.PathUnescape again would let crafted
inputs like "%252F" become "/", changing path semantics. Path and
anchor-id strings now go straight to utils.MakeRelativePath /
storagespace.ParseID.
- Distinguish operational errors from "not found". Gateway selection
failure, RPC transport errors, and unexpected CS3 status codes now
surface as 500 (don't mask outages); only NOT_FOUND and PERMISSION_DENIED
collapse to 404 (no existence disclosure). Implemented via a sentinel
errPathNotFound + a 500 fall-through.
- Refactor the two regex-handling branches in rewriteColonPath to share a
resolution path via a small colonMatch struct + matchInto/extract helpers.
Removes the SonarCloud duplication finding without changing behavior.
- Update the docstring on ResolveGraphPath to reflect that the rewrite
preserves the requested API version (/{version}/...) rather than
hard-coding /v1beta1/...
- Test cleanup: register t.Cleanup to remove the per-subtest selector
entry from pool's global selectors map after the subtest, and update
the "unexpected status" case to expect 500 (matches the new error
semantics).
Adds a chi middleware that detects MS Graph colon-syntax URLs and rewrites
them to the canonical /items/{itemID}/... form before chi performs route
matching. Existing handlers, routes, and GetDriveAndItemIDParam stay
unchanged.
Two URL shapes are recognized at both /v1.0 and /v1beta1:
/drives/{driveID}/root:/<path>[:/<suffix>][:]
/drives/{driveID}/items/{itemID}:/<relativePath>[:/<suffix>][:]
Path resolution runs as the request user via CS3 Stat. Both NOT_FOUND and
PERMISSION_DENIED collapse to a 404 response so existence isn't disclosed
to unauthorized callers. URLs without colon syntax fast-path through with
a single substring check. The original URL is stashed in request context
under OriginalPathContextKey for downstream tracing/logging.
The middleware is registered as a top-level mux.Use so it runs before any
route matching: chi middleware on a sub-router runs after the prefix is
matched but cannot redirect to a different leaf route. Top-level
middleware lets URL rewriting actually re-route the request.
Tests cover regex matching across versions, all rewrite variants
(root/items anchored, with/without suffix, with/without trailing colon,
deep paths), NOT_FOUND -> 404, PERMISSION_DENIED -> 404 (security: no
existence disclosure), and original-URL preservation in request context.
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.
This fixes error logs like
RR error when calling Createhome error="gateway: grpc failed with code CODE_INVALID_ARGUMENT" line=github.com/opencloud-eu/opencloud/services/proxy/pkg/middleware/create_home.go:87 service=proxy
e.g. during internal requests to the data provider.
GetRoleDefinition/s does now handle l10n correctly. Previsouly it just
returned the non-localized string. What made things worse was that
ListPermissions() mutated global list of available roles and replaced
some strings with translated values depending on the `accept-language`
header. Which resulted in GetRoleDefinition returning results in mixed
localization depending on who/what called ListPermissions before.
Fixes: #2800
