diff --git a/services/graph/pkg/middleware/path_lookup.go b/services/graph/pkg/middleware/path_lookup.go new file mode 100644 index 0000000000..1fc89bcd6c --- /dev/null +++ b/services/graph/pkg/middleware/path_lookup.go @@ -0,0 +1,366 @@ +package middleware + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/url" + "strings" + + gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + cs3rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" + storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/go-chi/chi/v5" + + "github.com/rs/zerolog" + + "github.com/opencloud-eu/opencloud/pkg/log" + "github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode" + "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" + "github.com/opencloud-eu/reva/v2/pkg/storagespace" + "github.com/opencloud-eu/reva/v2/pkg/utils" +) + +// The middleware is attached to the /drives/{driveID} sub-routers, so by the +// time it runs chi has already consumed the {version}/drives/{driveID} prefix +// and exposes the remainder via chi.RouteContext().RoutePath. parseColonPath +// therefore only needs to handle the part below the drive: +// +// root-anchored: /root:/[:/][:] +// item-anchored: /items/{itemID}:/[:/][:] + +type contextKey string + +// OriginalPathContextKey holds the pre-rewrite request path for downstream +// tracing/logging consumers. +const OriginalPathContextKey contextKey = "graph.original_path" + +// Sentinels distinguishing the resolution outcomes that map to specific HTTP +// statuses. Anything else surfaces as 500. +// +// errPathNotFound - path doesn't exist or the user lacks permission to +// see it. Both collapse to 404 (no existence disclosure). +// errInvalidRequest - client sent a malformed input (unparseable drive/item +// id, drive/item mismatch in item-anchored form). 400. +// errUnauthenticated - the gateway said the caller isn't authenticated for +// the lookup (token expired, cross-storage auth, etc.). 401. +var ( + errPathNotFound = errors.New("path not found") + errInvalidRequest = errors.New("invalid request") + errUnauthenticated = errors.New("unauthenticated") +) + +// ResolveGraphPath returns middleware that detects MS Graph colon-syntax path +// lookup URLs and rewrites chi's internal route path to the canonical +// /items/{resolvedItemID}{suffix} form so the request lands on the existing +// /drives/{driveID}/items/{itemID}... routes. +// +// It must be attached to the /drives/{driveID} sub-routers (it reads driveID +// from chi.URLParam and matches against chi.RouteContext().RoutePath). chi runs +// a sub-router's middleware chain before that sub-router performs its own route +// matching, so rewriting RoutePath here re-routes the request to a different +// leaf. (Rewriting r.URL.Path would NOT work at this level: once chi has +// descended into a sub-router, routeHTTP matches against rctx.RoutePath and +// ignores r.URL.Path.) +// +// Two URL shapes are recognized: +// +// /drives/{driveID}/root:/[:/][:] +// /drives/{driveID}/items/{itemID}:/[:/][:] +// +// Path resolution runs as the request user via CS3 Stat. NOT_FOUND and +// PERMISSION_DENIED collapse to 404 (no existence disclosure); operational +// failures (gateway selection, RPC transport, unexpected status) surface +// as 5xx so outages aren't masked. +// +// Requests whose RoutePath contains no colon fast-path through untouched. +func ResolveGraphPath(gws pool.Selectable[gateway.GatewayAPIClient], logger log.Logger) func(http.Handler) http.Handler { + l := logger.With().Str("middleware", "graphPathLookup").Logger() + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + rctx := chi.RouteContext(r.Context()) + + // Fast-path: skip anything that can't be colon-syntax. RoutePath + // is the part below /drives/{driveID}, e.g. "/items/{id}/children" + // for a normal request - no colon, so this returns immediately. + if rctx == nil || !strings.Contains(rctx.RoutePath, ":") { + next.ServeHTTP(w, r) + return + } + + driveID := chi.URLParam(r, "driveID") + original := r.URL.Path + rewritten, err := rewriteColonPath(r.Context(), gws, l, driveID, rctx.RoutePath) + switch { + case errors.Is(err, errPathNotFound): + l.Debug().Str("original", original).Msg("colon-path resolution: not found") + errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, "item not found") + return + case errors.Is(err, errInvalidRequest): + l.Debug().Str("original", original).Msg("colon-path resolution: invalid request") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "invalid request") + return + case errors.Is(err, errUnauthenticated): + l.Debug().Str("original", original).Msg("colon-path resolution: unauthenticated") + errorcode.Unauthenticated.Render(w, r, http.StatusUnauthorized, "unauthenticated") + return + case err != nil: + l.Error().Err(err).Str("original", original).Msg("colon-path resolution: internal error") + errorcode.GeneralException.Render( + w, r, http.StatusInternalServerError, "internal error resolving path", + ) + return + case rewritten == "": + // No colon-syntax match - pass through untouched. + next.ServeHTTP(w, r) + return + } + + l.Debug(). + Str("original", original). + Str("rewritten", rewritten). + Msg("colon-path resolution: rewrote") + + // Preserve the original (unmodified) request path for downstream + // tracing/logging. r.URL.Path itself stays untouched; only chi's + // internal RoutePath is rewritten. + r = r.WithContext(context.WithValue(r.Context(), OriginalPathContextKey, original)) + rctx.RoutePath = rewritten + next.ServeHTTP(w, r) + }) + } +} + +// colonMatch is the normalized result of parseColonPath for either the root- +// or item-anchored colon-syntax shape. The two shapes carry different parts; +// this struct erases that difference for downstream resolution. +// +// itemAnchorID, relPath and suffix hold the raw (still percent-encoded) +// substrings from RoutePath; rewriteColonPath decodes them as needed. +type colonMatch struct { + isItemAnchored bool // item-anchored form: anchor is itemAnchorID, validate against driveID + itemAnchorID string // itemID from the path for the item-anchored form (empty for root-anchored) + relPath string // relative path with leading slash + suffix string // suffix with leading slash (e.g. "/children"); may be empty +} + +// rewriteColonPath returns: +// - "" + nil - no colon-syntax pattern matched (passthrough) +// - rewritten + nil - matched and resolved to a canonical RoutePath +// - "" + errPathNotFound - path doesn't exist or user lacks permission (404) +// - "" + errInvalidRequest - malformed input (400) +// - "" + errUnauthenticated - gateway said caller isn't authenticated (401) +// - "" + other error - operational / internal failure (5xx) +// +// driveIDParam is the {driveID} route param (raw chi.URLParam value); routePath +// is chi.RouteContext().RoutePath (the part below /drives/{driveID}). +func rewriteColonPath( + ctx context.Context, + gws pool.Selectable[gateway.GatewayAPIClient], + logger zerolog.Logger, + driveIDParam string, + routePath string, +) (string, error) { + match, ok := parseColonPath(routePath) + if !ok { + return "", nil + } + + // RoutePath follows chi's RawPath, i.e. the percent-encoded wire form + // (e.g. "/Documents/My%20File"). A single PathUnescape reproduces exactly + // what net/http put in r.URL.Path; it is NOT a double-decode (a crafted + // "%252F" decodes once to "%2F", matching the decoded r.URL.Path, not "/"). + driveID, err := url.PathUnescape(driveIDParam) + if err != nil { + logger.Debug().Err(err).Str("driveID", driveIDParam).Msg("undecodable drive id in colon path") + return "", errInvalidRequest + } + + anchorIDStr := driveID + if match.isItemAnchored { + anchorIDStr, err = url.PathUnescape(match.itemAnchorID) + if err != nil { + logger.Debug().Err(err).Str("itemID", match.itemAnchorID).Msg("undecodable item id in colon path") + return "", errInvalidRequest + } + } + + anchor, err := storagespace.ParseID(anchorIDStr) + if err != nil { + // Unparseable input is malformed by the client, not "not found". + logger.Debug().Err(err).Str("anchor", anchorIDStr).Msg("invalid anchor id in colon path") + return "", errInvalidRequest + } + + // Item-anchored form: the itemID comes from the path, driveID from the + // route param. Validate the itemID belongs to the given driveID (storage + + // space prefix) - otherwise the request is malformed and we short-circuit + // instead of doing an unnecessary CS3 Stat. + if match.isItemAnchored { + drive, err := storagespace.ParseID(driveID) + if err != nil { + logger.Debug().Err(err).Str("driveID", driveID).Msg("invalid drive id in colon path") + return "", errInvalidRequest + } + if drive.GetStorageId() != anchor.GetStorageId() || drive.GetSpaceId() != anchor.GetSpaceId() { + logger.Debug(). + Str("driveID", driveID). + Str("itemID", anchorIDStr). + Msg("drive id does not match item id storage/space") + return "", errInvalidRequest + } + } + + relPath, err := url.PathUnescape(match.relPath) + if err != nil { + logger.Debug().Err(err).Str("relPath", match.relPath).Msg("undecodable path in colon path") + return "", errInvalidRequest + } + + itemID, err := resolvePath(ctx, gws, &anchor, relPath) + if err != nil { + return "", err + } + return buildCanonicalRoutePath(itemID, match.suffix), nil +} + +// parseColonPath splits a colon-syntax RoutePath (the part below +// /drives/{driveID}) into its parts, or reports ok=false when the path is not +// colon-syntax and should pass through untouched. +// +// Below the drive the grammar is one of: +// +// /root:/[:/][:] +// /items/:/[:/][:] +// +// The structural delimiter is ":/" (a colon immediately followed by the +// leading slash of the path or suffix); a trailing ":" is the no-suffix +// terminator. For example: +// +// /root:/Documents -> path "/Documents" +// /root:/Documents: -> path "/Documents" +// /root:/Documents:/children -> path "/Documents", suffix "/children" +// /items/{id}:/notes.txt:/children -> itemID "{id}", path "/notes.txt", suffix "/children" +// +// Clients must percent-encode each path segment, exactly as MS Graph requires +// (an unencoded path is ambiguous). The one OpenCloud-specific point: ':' must +// be encoded as "%3A". The split is on the literal ":/", so an encoded ':' is +// never a delimiter and decodes back to ':' with the rest of the path (e.g. +// "/root:/weird%3A/file.txt" -> "/weird:/file.txt"). OpenCloud allows ':' in +// names whereas MS Graph/OneDrive forbid it, hence this extra character. +// +// The returned fields are the raw (still percent-encoded) substrings; the +// caller decodes them. +func parseColonPath(routePath string) (colonMatch, bool) { + var m colonMatch + + // The anchor is separated from the path by the first ":/"; no ":/" at all + // means this isn't colon syntax and should pass through. + anchor, rest, found := strings.Cut(routePath, ":/") + if !found { + return m, false + } + // Cut consumed the leading '/' of the path along with the ":/" delimiter; + // the path is absolute, so put it back. + rest = "/" + rest + + switch { + case anchor == "/root": + // Root-anchored: path resolution later anchors on the {driveID} route + // param, so there's no item id to read from the URL. + case strings.HasPrefix(anchor, "/items/"): + // Item-anchored: the anchor is /items/{itemID} with a single-segment + // id. A '/' inside the id means this is an ordinary /items/{id}/... + // request that just happens to contain a ":/" further along. + itemID := strings.TrimPrefix(anchor, "/items/") + if itemID == "" || strings.Contains(itemID, "/") { + return m, false + } + m.isItemAnchored = true + m.itemAnchorID = itemID + default: + return m, false + } + + // rest is "/[:/][:]". Split path from an optional suffix on + // the next ":/" (the suffix regains the leading '/' Cut consumed), then + // drop a single trailing ':' - the "...:" no-suffix terminator. A suffix + // therefore requires an explicit ":/", so "/root:/foo/children" is the path + // "/foo/children", not "/foo" with suffix "/children". + m.relPath = rest + if path, suffix, found := strings.Cut(rest, ":/"); found { + m.relPath, m.suffix = path, "/"+suffix + } + m.relPath = strings.TrimSuffix(m.relPath, ":") + + // Shape requirements: the path must be absolute and non-empty ("/x"), and a + // present suffix must be absolute, non-empty and colon-free (suffixes are + // fixed API routes like "/children", never file paths). + if len(m.relPath) < 2 || m.relPath[0] != '/' { + return m, false + } + if m.suffix != "" && (len(m.suffix) < 2 || strings.Contains(m.suffix, ":")) { + return m, false + } + return m, true +} + +// resolvePath translates a relative filesystem path (anchored at the given +// CS3 resource id) into the resolved item's id, running with the request +// user's permissions via CS3 Stat. +func resolvePath( + ctx context.Context, + gws pool.Selectable[gateway.GatewayAPIClient], + anchor *storageprovider.ResourceId, + relPath string, +) (string, error) { + gw, err := gws.Next() + if err != nil { + return "", fmt.Errorf("gateway selector: %w", err) + } + + // relPath is already decoded (PathUnescape'd once by the caller), matching + // the form a normal handler would receive from r.URL.Path. + statRes, err := gw.Stat(ctx, &storageprovider.StatRequest{ + Ref: &storageprovider.Reference{ + ResourceId: anchor, + Path: utils.MakeRelativePath(relPath), + }, + }) + if err != nil { + return "", fmt.Errorf("CS3 Stat: %w", err) + } + + switch statRes.GetStatus().GetCode() { + case cs3rpc.Code_CODE_OK: + // fall through + case cs3rpc.Code_CODE_NOT_FOUND, cs3rpc.Code_CODE_PERMISSION_DENIED: + return "", errPathNotFound + case cs3rpc.Code_CODE_UNAUTHENTICATED: + return "", errUnauthenticated + default: + return "", fmt.Errorf( + "CS3 Stat returned %s: %s", + statRes.GetStatus().GetCode(), + statRes.GetStatus().GetMessage(), + ) + } + + id := statRes.GetInfo().GetId() + if id == nil { + return "", fmt.Errorf("CS3 Stat returned OK but missing Info.Id") + } + return storagespace.FormatResourceID(id), nil +} + +// buildCanonicalRoutePath produces the RoutePath chi should match against after +// the rewrite, relative to /drives/{driveID}: /items/{itemID}{suffix}. +// +// itemID is inserted literally (the FormatResourceID output, e.g. with `$` and +// `!` sub-delims). chi binds it verbatim into the {itemID} param; downstream +// handlers (parseIDParam, GetDriveAndItemIDParam) call url.PathUnescape on the +// param, which is a no-op for these literal sub-delims, so the id round-trips. +func buildCanonicalRoutePath(itemID, suffix string) string { + return "/items/" + itemID + suffix +} diff --git a/services/graph/pkg/middleware/path_lookup_test.go b/services/graph/pkg/middleware/path_lookup_test.go new file mode 100644 index 0000000000..fa866d2fed --- /dev/null +++ b/services/graph/pkg/middleware/path_lookup_test.go @@ -0,0 +1,471 @@ +package middleware_test + +import ( + "net/http" + "net/http/httptest" + "net/url" + "testing" + + gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + cs3rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" + storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/go-chi/chi/v5" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "google.golang.org/grpc" + + "github.com/opencloud-eu/opencloud/pkg/log" + "github.com/opencloud-eu/opencloud/services/graph/pkg/middleware" + "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" + cs3mocks "github.com/opencloud-eu/reva/v2/tests/cs3mocks/mocks" +) + +const ( + testDriveID = "storage-users-1$f503f6fe-2656-4b0f-8289-fb3184962dfd" + testItemID = "storage-users-1$f503f6fe-2656-4b0f-8289-fb3184962dfd!f0e20017-9cba-498a-87e5-3467b976604d" +) + +func newTestSelector(t *testing.T, gw *cs3mocks.GatewayAPIClient) pool.Selectable[gateway.GatewayAPIClient] { + t.Helper() + // Unique key per test so pool's selector cache doesn't hand back a stale gw. + // t.Cleanup removes the entry from pool's global selectors map after the + // subtest, so global state doesn't grow across the suite. + svcName := "TestGatewaySelector" + key := "test.gateway." + t.Name() + pool.RemoveSelector(svcName + key) + t.Cleanup(func() { pool.RemoveSelector(svcName + key) }) + return pool.GetSelector[gateway.GatewayAPIClient]( + svcName, + key, + func(cc grpc.ClientConnInterface) gateway.GatewayAPIClient { return gw }, + ) +} + +// statResponse builds a CS3 StatResponse with the given status code, optionally +// returning a resource info populated with testItemID for OK responses. +func statResponse(code cs3rpc.Code, withInfo bool) *storageprovider.StatResponse { + res := &storageprovider.StatResponse{Status: &cs3rpc.Status{Code: code}} + if withInfo { + res.Info = &storageprovider.ResourceInfo{ + Id: &storageprovider.ResourceId{ + StorageId: "storage-users-1", + SpaceId: "f503f6fe-2656-4b0f-8289-fb3184962dfd", + OpaqueId: "f0e20017-9cba-498a-87e5-3467b976604d", + }, + } + } + return res +} + +// leafCapture records what the matched leaf handler saw, so tests can assert +// the middleware rewrote chi's route path correctly and the request reached the +// intended /items/{itemID}... handler with the resolved id bound as a param. +type leafCapture struct { + hit string // which leaf was reached ("" = none) + urlPath string // r.URL.Path as seen by the handler (must stay the original) + driveID string // chi.URLParam(driveID) + itemID string // resolved item id, decoded via PathUnescape + original any // OriginalPathContextKey value +} + +// newGraphTestRouter wires ResolveGraphPath into a chi router that mirrors the +// production /drives/{driveID} nesting for both API versions, including the +// RawPath workaround Graph.ServeHTTP applies. Driving requests through real chi +// routing is deliberate: it exercises chi's actual behavior (sub-router +// middleware ordering, RoutePath encoding, param round-trip) indirectly, so a +// chi upgrade that changes any of it fails these tests instead of silently +// breaking colon-path lookups in production. +func newGraphTestRouter(t *testing.T, gw *cs3mocks.GatewayAPIClient) (http.Handler, *leafCapture) { + t.Helper() + cap := &leafCapture{} + selector := newTestSelector(t, gw) + + leaf := func(name string) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + cap.hit = name + cap.urlPath = r.URL.Path + cap.driveID = chi.URLParam(r, "driveID") + // v1.0 binds the item param as {driveItemID}, v1beta1 as {itemID}. + raw := chi.URLParam(r, "itemID") + if raw == "" { + raw = chi.URLParam(r, "driveItemID") + } + // Downstream handlers PathUnescape the param before parsing the id; + // mirror that here so we assert on the recovered id. + cap.itemID, _ = url.PathUnescape(raw) + cap.original = r.Context().Value(middleware.OriginalPathContextKey) + w.WriteHeader(http.StatusOK) + } + } + + m := chi.NewMux() + // Mirror Graph.ServeHTTP: RawPath drives chi's tree walk, which is what + // makes RoutePath carry the percent-encoded wire form. + m.Use(func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + r.URL.RawPath = r.URL.EscapedPath() + next.ServeHTTP(w, r) + }) + }) + // Leaf routes mirror production exactly (service.go), per version, so the + // tests assert reachability that production actually reproduces: a rewrite + // to a suffix that doesn't exist for that version must 404 here too. + m.Route("/graph", func(r chi.Router) { + r.Route("/v1beta1/drives/{driveID}", func(r chi.Router) { + r.Use(middleware.ResolveGraphPath(selector, log.NopLogger())) + r.Route("/items/{itemID}", func(r chi.Router) { + r.Get("/", leaf("item")) + r.Post("/createLink", leaf("createLink")) + r.Route("/permissions", func(r chi.Router) { + r.Get("/", leaf("permissions")) + r.Post("/{permissionID}/setPassword", leaf("setPassword")) + }) + }) + }) + r.Route("/v1.0/drives/{driveID}", func(r chi.Router) { + r.Use(middleware.ResolveGraphPath(selector, log.NopLogger())) + r.Route("/items/{driveItemID}", func(r chi.Router) { + r.Get("/", leaf("item")) + r.Get("/children", leaf("children")) + }) + }) + }) + return m, cap +} + +func TestResolveGraphPath(t *testing.T) { + tests := []struct { + name string + method string // defaults to GET when empty + urlPath string + statCode cs3rpc.Code + expectStatCalled bool + expectStatus int + expectHit string // leaf the request must land on ("" = none reached) + expectItemID string // resolved item id the leaf must see (after PathUnescape) + }{ + { + name: "non-colon URL routes normally without a Stat", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/items/" + testItemID + "/children", + expectStatCalled: false, + expectStatus: http.StatusOK, + expectHit: "children", + expectItemID: testItemID, + }, + { + name: "colon URL not matching the lookup pattern passes through (404, no Stat)", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/items/" + testItemID + "/foo:bar", + expectStatCalled: false, + expectStatus: http.StatusNotFound, + expectHit: "", + }, + { + // Anchoring: the colon-syntax shape appearing under a junk prefix + // (i.e. NOT at the configured HTTP root) must not trigger a rewrite + // or a CS3 Stat. Because the middleware is mounted under the + // /graph root, chi never routes such a path into it (404 first) - + // no /foo/.../v1.0/drives/...:/... can over-match. + name: "colon-syntax under a junk prefix does not match (anchored on HTTP root)", + urlPath: "/foo/graph/v1.0/drives/" + testDriveID + "/root:/Documents:/children", + expectStatCalled: false, + expectStatus: http.StatusNotFound, + expectHit: "", + }, + { + name: "v1.0 root-anchored with /children rewrites and routes", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/Documents:/children", + statCode: cs3rpc.Code_CODE_OK, + expectStatCalled: true, + expectStatus: http.StatusOK, + expectHit: "children", + expectItemID: testItemID, + }, + { + // Mirrors acceptance scenario 5: v1beta1 root-anchored with a + // /permissions suffix (a real v1beta1-only route, GET). + name: "v1beta1 root-anchored with /permissions rewrites and routes", + urlPath: "/graph/v1beta1/drives/" + testDriveID + "/root:/folder1:/permissions", + statCode: cs3rpc.Code_CODE_OK, + expectStatCalled: true, + expectStatus: http.StatusOK, + expectHit: "permissions", + expectItemID: testItemID, + }, + { + // Non-GET colon request: createLink is POST on v1beta1. + name: "v1beta1 root-anchored with /createLink (POST) rewrites and routes", + method: http.MethodPost, + urlPath: "/graph/v1beta1/drives/" + testDriveID + "/root:/folder1:/createLink", + statCode: cs3rpc.Code_CODE_OK, + expectStatCalled: true, + expectStatus: http.StatusOK, + expectHit: "createLink", + expectItemID: testItemID, + }, + { + // Multi-segment suffix: /permissions/{permissionID}/setPassword on + // v1beta1 (POST). Pins that parseColonPath carries a suffix with + // embedded slashes through the rewrite into a real nested route. + name: "v1beta1 root-anchored with multi-segment suffix rewrites and routes", + method: http.MethodPost, + urlPath: "/graph/v1beta1/drives/" + testDriveID + "/root:/folder1:/permissions/perm-1/setPassword", + statCode: cs3rpc.Code_CODE_OK, + expectStatCalled: true, + expectStatus: http.StatusOK, + expectHit: "setPassword", + expectItemID: testItemID, + }, + { + // Item-anchored colon syntax on v1beta1 (POST createLink). + name: "v1beta1 item-anchored with /createLink (POST) rewrites and routes", + method: http.MethodPost, + urlPath: "/graph/v1beta1/drives/" + testDriveID + "/items/" + testItemID + ":/notes.txt:/createLink", + statCode: cs3rpc.Code_CODE_OK, + expectStatCalled: true, + expectStatus: http.StatusOK, + expectHit: "createLink", + expectItemID: testItemID, + }, + { + name: "trailing colon (no suffix) rewrites to bare item URL", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/Documents:", + statCode: cs3rpc.Code_CODE_OK, + expectStatCalled: true, + expectStatus: http.StatusOK, + expectHit: "item", + expectItemID: testItemID, + }, + { + name: "no trailing colon, no suffix rewrites to bare item URL", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/Documents", + statCode: cs3rpc.Code_CODE_OK, + expectStatCalled: true, + expectStatus: http.StatusOK, + expectHit: "item", + expectItemID: testItemID, + }, + { + name: "deep path rewrites correctly", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/Documents/Reports:/children", + statCode: cs3rpc.Code_CODE_OK, + expectStatCalled: true, + expectStatus: http.StatusOK, + expectHit: "children", + expectItemID: testItemID, + }, + { + // A trailing segment that looks like a suffix ("children") is only a + // suffix when a SECOND colon introduces it. Without it, the whole + // thing is the path, so this must resolve to the bare item (GET "/"), + // NOT to /items/{id}/children. If the parser wrongly split it, the + // request would land on the "children" leaf and this fails. + name: "trailing suffix-looking segment without a second colon stays in the path", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/Documents/children", + statCode: cs3rpc.Code_CODE_OK, + expectStatCalled: true, + expectStatus: http.StatusOK, + expectHit: "item", + expectItemID: testItemID, + }, + { + name: "item-anchored colon syntax rewrites", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/items/" + testItemID + ":/notes.txt:/children", + statCode: cs3rpc.Code_CODE_OK, + expectStatCalled: true, + expectStatus: http.StatusOK, + expectHit: "children", + expectItemID: testItemID, + }, + { + name: "Stat NOT_FOUND returns 404 without reaching a leaf", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/Missing:", + statCode: cs3rpc.Code_CODE_NOT_FOUND, + expectStatCalled: true, + expectStatus: http.StatusNotFound, + expectHit: "", + }, + { + // CRITICAL security test: PERMISSION_DENIED must not leak existence. + // We collapse it to 404, identical to NOT_FOUND, so an unauthorized + // caller can't distinguish "doesn't exist" from "exists but hidden". + name: "Stat PERMISSION_DENIED returns 404 (not 403) - don't disclose existence", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/Restricted:", + statCode: cs3rpc.Code_CODE_PERMISSION_DENIED, + expectStatCalled: true, + expectStatus: http.StatusNotFound, + expectHit: "", + }, + { + // Operational/unexpected CS3 statuses must NOT collapse to 404 - + // that would mask outages. Surface as 500 like other graph handlers. + name: "Stat unexpected status returns 500 (not 404 - don't mask outages)", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/Anything:", + statCode: cs3rpc.Code_CODE_INTERNAL, + expectStatCalled: true, + expectStatus: http.StatusInternalServerError, + expectHit: "", + }, + { + // UNAUTHENTICATED is its own distinct outcome - must surface as 401, + // not 500, so clients can detect "your token is bad" vs "server error". + name: "Stat UNAUTHENTICATED returns 401", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/Documents:", + statCode: cs3rpc.Code_CODE_UNAUTHENTICATED, + expectStatCalled: true, + expectStatus: http.StatusUnauthorized, + expectHit: "", + }, + { + // Item-anchored form with a driveID that doesn't match the itemID's + // storage/space - the request is malformed; short-circuit to 400 + // instead of doing a Stat that would only fail downstream. + name: "drive id and item id storage/space mismatch returns 400", + urlPath: "/graph/v1.0/drives/storage-users-2$other-space-id/items/" + testItemID + ":/notes.txt:/children", + expectStatCalled: false, + expectStatus: http.StatusBadRequest, + expectHit: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gw := &cs3mocks.GatewayAPIClient{} + if tt.expectStatCalled { + gw.On("Stat", mock.Anything, mock.Anything). + Return(statResponse(tt.statCode, tt.statCode == cs3rpc.Code_CODE_OK), nil) + } + + method := tt.method + if method == "" { + method = http.MethodGet + } + router, cap := newGraphTestRouter(t, gw) + req := httptest.NewRequest(method, "http://localhost"+tt.urlPath, nil) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + assert.Equal(t, tt.expectStatus, rr.Code, "status code") + assert.Equal(t, tt.expectHit, cap.hit, "leaf handler reached") + + if tt.expectHit != "" { + assert.Equal(t, testDriveID, cap.driveID, "driveID param") + assert.Equal(t, tt.expectItemID, cap.itemID, "resolved item id seen by leaf") + // r.URL.Path must stay the original; only chi's RoutePath is rewritten. + assert.Equal(t, tt.urlPath, cap.urlPath, "r.URL.Path must remain the original request path") + } + + if tt.expectStatCalled { + gw.AssertCalled(t, "Stat", mock.Anything, mock.Anything) + } else { + gw.AssertNotCalled(t, "Stat", mock.Anything, mock.Anything) + } + }) + } +} + +// TestResolveGraphPath_DecodesEncodedPath pins the decoding contract: chi's +// RoutePath carries the percent-encoded wire form (because Graph.ServeHTTP sets +// RawPath), and the middleware must hand CS3 Stat the decoded path - exactly +// what a normal handler reading r.URL.Path would get. Driving this through real +// chi routing means a chi change to RoutePath encoding would break this test. +func TestResolveGraphPath_DecodesEncodedPath(t *testing.T) { + tests := []struct { + name string + urlPath string + expectedStat string + }{ + { + name: "space encoded as %20 is decoded for Stat", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/Documents/My%20File:/children", + expectedStat: "./Documents/My File", + }, + { + // A crafted double-encoding must be decoded exactly once (to the + // literal "%2F"), never twice into a path separator. + name: "double-encoded %252F decodes once, not twice", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/a%252Fb:/children", + expectedStat: "./a%2Fb", + }, + { + // Without a second colon the whole remainder is the path: Stat must + // receive "/Documents/children", not "/Documents" (which would mean + // "/children" was wrongly treated as a suffix). + name: "suffix-looking segment without a second colon is part of the Stat path", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/Documents/children", + expectedStat: "./Documents/children", + }, + { + // Clients percent-encode ':' as "%3A" to put it in a name. "%3A" is + // never seen as the ":/" delimiter and decodes back to a literal ':' + // - here a directory named "weird:". + name: "percent-encoded colon (%3A) reaches Stat as a literal colon", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/weird%3A/file.txt", + expectedStat: "./weird:/file.txt", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var statPath string + gw := &cs3mocks.GatewayAPIClient{} + gw.On("Stat", mock.Anything, mock.Anything). + Run(func(args mock.Arguments) { + req := args.Get(1).(*storageprovider.StatRequest) + statPath = req.GetRef().GetPath() + }). + Return(statResponse(cs3rpc.Code_CODE_OK, true), nil) + + router, _ := newGraphTestRouter(t, gw) + req := httptest.NewRequest(http.MethodGet, "http://localhost"+tt.urlPath, nil) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusOK, rr.Code) + assert.Equal(t, tt.expectedStat, statPath, "path passed to CS3 Stat must be decoded exactly once") + }) + } +} + +// TestResolveGraphPath_ItemIDRoundTrip guards the sub-delimiter round-trip: the +// resolved id contains `$` and `!`, and after the RoutePath rewrite chi must +// bind it to the {itemID} param such that the downstream PathUnescape recovers +// the original id. This exercises chi's param binding indirectly; a regression +// in how chi stores matched segments would surface here. +func TestResolveGraphPath_ItemIDRoundTrip(t *testing.T) { + gw := &cs3mocks.GatewayAPIClient{} + gw.On("Stat", mock.Anything, mock.Anything). + Return(statResponse(cs3rpc.Code_CODE_OK, true), nil) + + router, cap := newGraphTestRouter(t, gw) + req := httptest.NewRequest( + http.MethodGet, + "http://localhost/graph/v1.0/drives/"+testDriveID+"/root:/Documents:/children", + nil, + ) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusOK, rr.Code, "chi must route the rewritten path to the leaf") + assert.Equal(t, "children", cap.hit) + assert.Equal(t, testItemID, cap.itemID, + "PathUnescape(chi.URLParam(itemID)) must recover the original id (with `$` and `!`)") +} + +// TestResolveGraphPath_OriginalPathContext verifies the original URL is +// preserved in request context for downstream tracing/logging, and that +// r.URL.Path itself is left untouched by the rewrite. +func TestResolveGraphPath_OriginalPathContext(t *testing.T) { + gw := &cs3mocks.GatewayAPIClient{} + gw.On("Stat", mock.Anything, mock.Anything). + Return(statResponse(cs3rpc.Code_CODE_OK, true), nil) + + original := "/graph/v1.0/drives/" + testDriveID + "/root:/Documents:/children" + router, cap := newGraphTestRouter(t, gw) + req := httptest.NewRequest(http.MethodGet, "http://localhost"+original, nil) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusOK, rr.Code) + assert.Equal(t, original, cap.original, "original URL must be available via OriginalPathContextKey") + assert.Equal(t, original, cap.urlPath, "r.URL.Path must remain the original request path") +} diff --git a/services/graph/pkg/service/v0/service.go b/services/graph/pkg/service/v0/service.go index ecce7f69e1..fb5e490851 100644 --- a/services/graph/pkg/service/v0/service.go +++ b/services/graph/pkg/service/v0/service.go @@ -265,6 +265,12 @@ func NewService(opts ...Option) (Graph, error) { //nolint:maintidx r.Route("/drives", func(r chi.Router) { r.Get("/", svc.GetAllDrives(APIVersion_1_Beta_1)) r.Route("/{driveID}", func(r chi.Router) { + // Rewrites MS Graph colon-syntax lookups (root:/path, + // items/{id}:/path) to /items/{resolvedID}... before chi + // matches the leaf route. Must sit here, on the + // /drives/{driveID} sub-router, so it can rewrite the + // remaining RoutePath. See graphm.ResolveGraphPath. + r.Use(graphm.ResolveGraphPath(options.GatewaySelector, options.Logger)) r.Route("/root", func(r chi.Router) { r.Post("/children", drivesDriveItemApi.CreateDriveItem) r.Post("/invite", driveItemPermissionsApi.SpaceRootInvite) @@ -367,6 +373,12 @@ func NewService(opts ...Option) (Graph, error) { //nolint:maintidx r.Get("/", svc.GetAllDrives(APIVersion_1)) r.Post("/", svc.CreateDrive) r.Route("/{driveID}", func(r chi.Router) { + // Rewrites MS Graph colon-syntax lookups (root:/path, + // items/{id}:/path) to /items/{resolvedID}... before chi + // matches the leaf route. Must sit here, on the + // /drives/{driveID} sub-router, so it can rewrite the + // remaining RoutePath. See graphm.ResolveGraphPath. + r.Use(graphm.ResolveGraphPath(options.GatewaySelector, options.Logger)) r.Patch("/", svc.UpdateDrive) r.Get("/", svc.GetSingleDrive) r.Delete("/", svc.DeleteDrive) diff --git a/tests/acceptance/bootstrap/GraphContext.php b/tests/acceptance/bootstrap/GraphContext.php index cf350be6be..70254354c0 100644 --- a/tests/acceptance/bootstrap/GraphContext.php +++ b/tests/acceptance/bootstrap/GraphContext.php @@ -3334,4 +3334,169 @@ class GraphContext implements Context { $response = $this->unmarkFavorite($user, $itemId); $this->featureContext->theHTTPStatusCodeShouldBe(204, '', $response); } + + /** + * Encode a colon-syntax path segment so slashes survive but the + * structural ":" delimiters of the Graph URL remain literal. + * + * @param string $path + * + * @return string + */ + private function encodeColonPathSegment(string $path): string { + $path = \ltrim($path, '/'); + $parts = \explode('/', $path); + $encoded = \array_map('rawurlencode', $parts); + return \implode('/', $encoded); + } + + /** + * Send a Graph API request and capture the response on FeatureContext. + * + * @param string $user + * @param string $method + * @param string $relativeUrl + * + * @return void + */ + private function sendGraphRequestAndCaptureResponse( + string $user, + string $method, + string $relativeUrl + ): void { + $response = $this->featureContext->sendingToWithDirectUrl($user, $method, $relativeUrl); + $this->featureContext->setResponse($response); + } + + /** + * @When /^user "([^"]*)" gets the drive item with colon path "([^"]*)" of space "([^"]*)" using the Graph API version "(v1\.0|v1beta1)"$/ + * + * Hits /graph/{version}/drives/{driveID}/root:/{path}, exercising the + * colon-syntax path lookup middleware (root-anchored, no suffix). + * + * @param string $user + * @param string $path + * @param string $spaceName + * @param string $apiVersion + * + * @return void + */ + public function userGetsDriveItemWithColonPathOfSpace( + string $user, + string $path, + string $spaceName, + string $apiVersion + ): void { + $driveId = $this->spacesContext->getSpaceIdByName($user, $spaceName); + $encoded = $this->encodeColonPathSegment($path); + $url = "/graph/$apiVersion/drives/$driveId/root:/$encoded"; + $this->sendGraphRequestAndCaptureResponse($user, "GET", $url); + } + + /** + * @When /^user "([^"]*)" gets the drive item with colon path "([^"]*)" of space "([^"]*)" with trailing colon using the Graph API version "(v1\.0|v1beta1)"$/ + * + * Hits /graph/{version}/drives/{driveID}/root:/{path}: with the optional + * trailing ":" — verifies the middleware accepts both forms. + * + * @param string $user + * @param string $path + * @param string $spaceName + * @param string $apiVersion + * + * @return void + */ + public function userGetsDriveItemWithColonPathOfSpaceWithTrailingColon( + string $user, + string $path, + string $spaceName, + string $apiVersion + ): void { + $driveId = $this->spacesContext->getSpaceIdByName($user, $spaceName); + $encoded = $this->encodeColonPathSegment($path); + $url = "/graph/$apiVersion/drives/$driveId/root:/$encoded:"; + $this->sendGraphRequestAndCaptureResponse($user, "GET", $url); + } + + /** + * @When /^user "([^"]*)" gets the drive item with colon path "([^"]*)" relative to folder "([^"]*)" of space "([^"]*)" using the Graph API version "(v1\.0|v1beta1)"$/ + * + * Hits /graph/{version}/drives/{driveID}/items/{itemID}:/{relPath}, the + * item-anchored colon-syntax form. + * + * @param string $user + * @param string $relPath + * @param string $folderName + * @param string $spaceName + * @param string $apiVersion + * + * @return void + */ + public function userGetsDriveItemWithColonPathRelativeToFolderOfSpace( + string $user, + string $relPath, + string $folderName, + string $spaceName, + string $apiVersion + ): void { + $driveId = $this->spacesContext->getSpaceIdByName($user, $spaceName); + $folderId = $this->spacesContext->getResourceId($user, $spaceName, $folderName); + $encoded = $this->encodeColonPathSegment($relPath); + $url = "/graph/$apiVersion/drives/$driveId/items/$folderId:/$encoded"; + $this->sendGraphRequestAndCaptureResponse($user, "GET", $url); + } + + /** + * @When /^user "([^"]*)" lists permissions of the drive item with colon path "([^"]*)" of space "([^"]*)" using the Graph API version "(v1\.0|v1beta1)"$/ + * + * Hits /graph/{version}/drives/{driveID}/root:/{path}:/permissions, the + * "colon path with suffix" form (root-anchored colon path + canonical + * sub-route). + * + * @param string $user + * @param string $path + * @param string $spaceName + * @param string $apiVersion + * + * @return void + */ + public function userListsPermissionsOfDriveItemWithColonPathOfSpace( + string $user, + string $path, + string $spaceName, + string $apiVersion + ): void { + $driveId = $this->spacesContext->getSpaceIdByName($user, $spaceName); + $encoded = $this->encodeColonPathSegment($path); + $url = "/graph/$apiVersion/drives/$driveId/root:/$encoded:/permissions"; + $this->sendGraphRequestAndCaptureResponse($user, "GET", $url); + } + + /** + * @When /^user "([^"]*)" gets the drive item with colon path "([^"]*)" of the personal space of "([^"]*)" using the Graph API version "(v1\.0|v1beta1)"$/ + * + * Same as userGetsDriveItemWithColonPathOfSpace, but the request is + * issued by :user against the personal space drive ID of :owner. Used + * for security tests where one user attempts to reach another user's + * resource via colon syntax — should be indistinguishable from a + * not-found result. + * + * @param string $user + * @param string $path + * @param string $owner + * @param string $apiVersion + * + * @return void + */ + public function userGetsDriveItemWithColonPathOfPersonalSpaceOf( + string $user, + string $path, + string $owner, + string $apiVersion + ): void { + $driveId = $this->spacesContext->getSpaceIdByName($owner, "Personal"); + $encoded = $this->encodeColonPathSegment($path); + $url = "/graph/$apiVersion/drives/$driveId/root:/$encoded"; + $this->sendGraphRequestAndCaptureResponse($user, "GET", $url); + } } diff --git a/tests/acceptance/features/apiGraph/colonSyntaxPathLookup.feature b/tests/acceptance/features/apiGraph/colonSyntaxPathLookup.feature new file mode 100644 index 0000000000..6cd45ab8b1 --- /dev/null +++ b/tests/acceptance/features/apiGraph/colonSyntaxPathLookup.feature @@ -0,0 +1,120 @@ +Feature: colon-syntax path lookup on the Graph API + As a client + I want to address drive items by path using colon-syntax URLs on the Graph API + So that I do not have to walk the path with successive lookups before issuing a request + + The colon-syntax shapes recognised by the path-lookup middleware are: + + /graph/{version}/drives/{driveID}/root:/[:/][:] + /graph/{version}/drives/{driveID}/items/{itemID}:/[:/][:] + + Both /v1.0 and /v1beta1 are supported. NOT_FOUND and PERMISSION_DENIED + collapse to 404 so the middleware never discloses the existence of + resources the caller is not allowed to see. + + Clients must percent-encode each path segment, exactly as MS Graph requires. + The only OpenCloud-specific point: ':' must be encoded as "%3A" (the split is + on the literal ":/", so an encoded ':' is never a delimiter). OpenCloud allows + ':' in names whereas MS Graph/OneDrive forbid it, so "%3A" is the one extra + character to encode. + + Background: + Given user "Alice" has been created with default attributes + And user "Alice" has created folder "folder1" + And user "Alice" has created folder "folder1/sub" + And user "Alice" has uploaded file with content "hello" to "folder1/file.txt" + And user "Alice" has uploaded file with content "deep" to "folder1/sub/deep.txt" + + + Scenario: get a drive item by root-anchored colon path + When user "Alice" gets the drive item with colon path "folder1/file.txt" of space "Personal" using the Graph API version "v1.0" + Then the HTTP status code should be "200" + And the JSON data of the response should match + """ + { + "type": "object", + "required": ["id", "name", "parentReference"], + "properties": { + "id": { + "type": "string", + "pattern": "^%file_id_pattern%$" + }, + "name": { + "const": "file.txt" + } + } + } + """ + + + Scenario: get a drive item by root-anchored colon path with a deep path + When user "Alice" gets the drive item with colon path "folder1/sub/deep.txt" of space "Personal" using the Graph API version "v1.0" + Then the HTTP status code should be "200" + And the JSON data of the response should match + """ + { + "type": "object", + "required": ["id", "name"], + "properties": { + "name": { + "const": "deep.txt" + } + } + } + """ + + + Scenario: get a drive item by root-anchored colon path with a trailing colon + When user "Alice" gets the drive item with colon path "folder1/file.txt" of space "Personal" with trailing colon using the Graph API version "v1.0" + Then the HTTP status code should be "200" + And the JSON data of the response should match + """ + { + "type": "object", + "required": ["id", "name"], + "properties": { + "name": { + "const": "file.txt" + } + } + } + """ + + + Scenario: get a drive item by item-anchored colon path + When user "Alice" gets the drive item with colon path "file.txt" relative to folder "folder1" of space "Personal" using the Graph API version "v1.0" + Then the HTTP status code should be "200" + And the JSON data of the response should match + """ + { + "type": "object", + "required": ["id", "name"], + "properties": { + "name": { + "const": "file.txt" + } + } + } + """ + + + Scenario: list permissions of a drive item via colon path with a sub-route suffix on v1beta1 + # Exercises the "/:/" rewrite shape and, at the same time, + # the v1beta1 mount of the middleware. The /permissions sub-route is only + # registered at /v1beta1/, and the canonical /v1beta1 GetDriveItem + # handler is share-jail-only, so this is the cleanest way to assert that + # the v1beta1 colon-syntax path actually reaches a working handler for + # regular drive items. + When user "Alice" lists permissions of the drive item with colon path "folder1" of space "Personal" using the Graph API version "v1beta1" + Then the HTTP status code should be "200" + + + Scenario: non-existent colon path returns 404 + When user "Alice" gets the drive item with colon path "folder1/does-not-exist.txt" of space "Personal" using the Graph API version "v1.0" + Then the HTTP status code should be "404" + + + Scenario: another user cannot disclose existence of a resource via colon path + Given user "Brian" has been created with default attributes + When user "Brian" gets the drive item with colon path "folder1/file.txt" of the personal space of "Alice" using the Graph API version "v1.0" + Then the HTTP status code should be "404"