diff --git a/services/graph/pkg/middleware/path_lookup.go b/services/graph/pkg/middleware/path_lookup.go new file mode 100644 index 0000000000..ff634ca81a --- /dev/null +++ b/services/graph/pkg/middleware/path_lookup.go @@ -0,0 +1,197 @@ +package middleware + +import ( + "context" + "net/http" + "net/url" + "regexp" + "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/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" +) + +// {HTTP.Root}/{version}/drives/{driveID}/root:/[:/][:] +// where {version} is "v1.0" or "v1beta1" — preserves the requested version +// so it lands on a registered route. +// group 1: prefix up to and including the driveID +// group 2: driveID (URL-encoded) +// group 3: path (with leading slash) +// group 4: suffix (with leading slash) — empty for direct item lookup +var rootColonRe = regexp.MustCompile(`^(.*?/v1(?:\.0|beta1)/drives/([^/]+))/root:(/.+?)(?::(/[^:]+))?:?$`) + +// {HTTP.Root}/{version}/drives/{driveID}/items/{itemID}:/[:/][:] +// group 1: prefix up to and including the driveID +// group 2: driveID (URL-encoded) +// group 3: anchor itemID (URL-encoded) +// group 4: path (with leading slash) +// group 5: suffix (with leading slash) +var itemColonRe = regexp.MustCompile(`^(.*?/v1(?:\.0|beta1)/drives/([^/]+))/items/([^/]+):(/.+?)(?::(/[^:]+))?:?$`) + +type contextKey string + +// OriginalPathContextKey holds the pre-rewrite request path for downstream +// tracing/logging consumers. +const OriginalPathContextKey contextKey = "graph.original_path" + +// ResolveGraphPath returns middleware that detects MS Graph colon-syntax +// path lookup URLs and rewrites them to the canonical +// /v1beta1/drives/{driveID}/items/{resolvedItemID}/{suffix} form before +// chi performs route matching. +// +// Two URL shapes are recognized: +// +// /v1beta1/drives/{driveID}/root:/[:/][:] +// /v1beta1/drives/{driveID}/items/{itemID}:/[:/][:] +// +// 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 the middleware with a +// single substring check. +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) { + // Fast-path: skip URLs that can't be colon-syntax + if !strings.Contains(r.URL.Path, ":") { + next.ServeHTTP(w, r) + return + } + + original := r.URL.Path + rewritten, matched := rewriteColonPath(r.Context(), gws, l, original) + if !matched { + next.ServeHTTP(w, r) + return + } + if rewritten == "" { + // Resolution failed: not found, permission denied, or invalid input. + // Collapse to 404 to avoid disclosing existence. + l.Debug().Str("original", original).Msg("colon-path resolution failed; returning 404") + errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, "item not found") + return + } + + l.Debug(). + Str("original", original). + Str("rewritten", rewritten). + Msg("rewrote MS Graph colon-syntax path") + + // Stash original URL for downstream logging/tracing. + ctx := context.WithValue(r.Context(), OriginalPathContextKey, original) + r = r.WithContext(ctx) + r.URL.Path = rewritten + r.URL.RawPath = "" + next.ServeHTTP(w, r) + }) + } +} + +// rewriteColonPath returns: +// - rewritten URL + true when the URL matched a colon-syntax pattern and resolution succeeded +// - "" + true when the URL matched a colon-syntax pattern but resolution failed (caller should 404) +// - "" + false when the URL did not match any colon-syntax pattern +func rewriteColonPath(ctx context.Context, gws pool.Selectable[gateway.GatewayAPIClient], logger zerolog.Logger, urlPath string) (string, bool) { + if m := rootColonRe.FindStringSubmatch(urlPath); m != nil { + prefix, driveIDStr, relPath, suffix := m[1], m[2], m[3], m[4] + driveID, err := decodeAndParseID(driveIDStr) + if err != nil { + logger.Debug().Err(err).Str("driveID", driveIDStr).Msg("invalid driveID in colon path") + return "", true + } + itemID, ok := resolvePath(ctx, gws, logger, &driveID, relPath) + if !ok { + return "", true + } + return buildCanonicalPath(prefix, itemID, suffix), true + } + + if m := itemColonRe.FindStringSubmatch(urlPath); m != nil { + prefix, _, anchorIDStr, relPath, suffix := m[1], m[2], m[3], m[4], m[5] + anchorID, err := decodeAndParseID(anchorIDStr) + if err != nil { + logger.Debug().Err(err).Str("itemID", anchorIDStr).Msg("invalid item anchor in colon path") + return "", true + } + itemID, ok := resolvePath(ctx, gws, logger, &anchorID, relPath) + if !ok { + return "", true + } + return buildCanonicalPath(prefix, itemID, suffix), true + } + + return "", false +} + +func resolvePath(ctx context.Context, gws pool.Selectable[gateway.GatewayAPIClient], logger zerolog.Logger, anchor *storageprovider.ResourceId, rawPath string) (string, bool) { + gw, err := gws.Next() + if err != nil { + logger.Error().Err(err).Msg("could not select gateway client") + return "", false + } + + decoded, err := url.PathUnescape(rawPath) + if err != nil { + logger.Debug().Err(err).Str("path", rawPath).Msg("failed to URL-decode path segment") + return "", false + } + + statRes, err := gw.Stat(ctx, &storageprovider.StatRequest{ + Ref: &storageprovider.Reference{ + ResourceId: anchor, + Path: utils.MakeRelativePath(decoded), + }, + }) + if err != nil { + logger.Error().Err(err).Msg("Stat call failed during colon-path resolution") + return "", false + } + + switch statRes.GetStatus().GetCode() { + case cs3rpc.Code_CODE_OK: + // fall through + case cs3rpc.Code_CODE_NOT_FOUND, cs3rpc.Code_CODE_PERMISSION_DENIED: + // Both collapse to "not found" — never tell unauthorized callers + // that the resource exists. + return "", false + default: + logger.Debug(). + Str("code", statRes.GetStatus().GetCode().String()). + Str("message", statRes.GetStatus().GetMessage()). + Msg("unexpected Stat status during colon-path resolution") + return "", false + } + + id := statRes.GetInfo().GetId() + if id == nil { + return "", false + } + return storagespace.FormatResourceID(id), true +} + +func decodeAndParseID(s string) (storageprovider.ResourceId, error) { + decoded, err := url.PathUnescape(s) + if err != nil { + return storageprovider.ResourceId{}, err + } + return storagespace.ParseID(decoded) +} + +func buildCanonicalPath(prefix, itemID, suffix string) string { + // r.URL.Path is the decoded form per Go's net/http convention; chi tree + // matching reads it directly. Insert the raw itemID without escaping so + // chi's {itemID} param captures the same string the handler will see + // after parseIDParam unescapes (no-op for already-decoded chars). + return prefix + "/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..11ffe87801 --- /dev/null +++ b/services/graph/pkg/middleware/path_lookup_test.go @@ -0,0 +1,219 @@ +package middleware_test + +import ( + "net/http" + "net/http/httptest" + "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/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. + svcName := "TestGatewaySelector" + key := "test.gateway." + t.Name() + 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 +} + +// recordingHandler captures the request URL path it receives so tests can assert +// the middleware rewrote (or passed through) correctly. +func recordingHandler() (http.Handler, *string) { + var seen string + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + seen = r.URL.Path + w.WriteHeader(http.StatusOK) + }), &seen +} + +func TestResolveGraphPath(t *testing.T) { + tests := []struct { + name string + urlPath string + statCode cs3rpc.Code + statErr error + expectStatCalled bool + expectStatus int + expectURLPath string // empty = handler not invoked + }{ + { + name: "non-colon URL passes through unchanged", + urlPath: "/graph/v1.0/me/drives", + expectStatCalled: false, + expectStatus: http.StatusOK, + expectURLPath: "/graph/v1.0/me/drives", + }, + { + name: "URL with colon but not matching pattern passes through", + urlPath: "/graph/v1.0/some/other/route:/foo", + expectStatCalled: false, + expectStatus: http.StatusOK, + expectURLPath: "/graph/v1.0/some/other/route:/foo", + }, + { + name: "v1.0 root-anchored with /children rewrites", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/Documents:/children", + statCode: cs3rpc.Code_CODE_OK, + expectStatCalled: true, + expectStatus: http.StatusOK, + expectURLPath: "/graph/v1.0/drives/" + testDriveID + "/items/storage-users-1$f503f6fe-2656-4b0f-8289-fb3184962dfd!f0e20017-9cba-498a-87e5-3467b976604d/children", + }, + { + name: "v1beta1 root-anchored with /createLink rewrites", + urlPath: "/graph/v1beta1/drives/" + testDriveID + "/root:/Documents:/createLink", + statCode: cs3rpc.Code_CODE_OK, + expectStatCalled: true, + expectStatus: http.StatusOK, + expectURLPath: "/graph/v1beta1/drives/" + testDriveID + "/items/storage-users-1$f503f6fe-2656-4b0f-8289-fb3184962dfd!f0e20017-9cba-498a-87e5-3467b976604d/createLink", + }, + { + 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, + expectURLPath: "/graph/v1.0/drives/" + testDriveID + "/items/storage-users-1$f503f6fe-2656-4b0f-8289-fb3184962dfd!f0e20017-9cba-498a-87e5-3467b976604d", + }, + { + 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, + expectURLPath: "/graph/v1.0/drives/" + testDriveID + "/items/storage-users-1$f503f6fe-2656-4b0f-8289-fb3184962dfd!f0e20017-9cba-498a-87e5-3467b976604d", + }, + { + 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, + expectURLPath: "/graph/v1.0/drives/" + testDriveID + "/items/storage-users-1$f503f6fe-2656-4b0f-8289-fb3184962dfd!f0e20017-9cba-498a-87e5-3467b976604d/children", + }, + { + 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, + expectURLPath: "/graph/v1.0/drives/" + testDriveID + "/items/storage-users-1$f503f6fe-2656-4b0f-8289-fb3184962dfd!f0e20017-9cba-498a-87e5-3467b976604d/children", + }, + { + name: "Stat NOT_FOUND returns 404 without invoking handler", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/Missing:", + statCode: cs3rpc.Code_CODE_NOT_FOUND, + expectStatCalled: true, + expectStatus: http.StatusNotFound, + expectURLPath: "", // handler must NOT be called + }, + { + // 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 you can't see it". + 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, + expectURLPath: "", + }, + { + name: "Stat unexpected status returns 404", + urlPath: "/graph/v1.0/drives/" + testDriveID + "/root:/Anything:", + statCode: cs3rpc.Code_CODE_INTERNAL, + expectStatCalled: true, + expectStatus: http.StatusNotFound, + expectURLPath: "", + }, + } + + 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), tt.statErr) + } + + selector := newTestSelector(t, gw) + handler, seen := recordingHandler() + mw := middleware.ResolveGraphPath(selector, log.NewLogger()) + + req := httptest.NewRequest(http.MethodGet, "http://localhost"+tt.urlPath, nil) + rr := httptest.NewRecorder() + + mw(handler).ServeHTTP(rr, req) + + assert.Equal(t, tt.expectStatus, rr.Code, "status code") + assert.Equal(t, tt.expectURLPath, *seen, "URL seen by next handler") + + if tt.expectStatCalled { + gw.AssertCalled(t, "Stat", mock.Anything, mock.Anything) + } else { + gw.AssertNotCalled(t, "Stat", mock.Anything, mock.Anything) + } + }) + } +} + +// TestResolveGraphPath_OriginalPathContext verifies the rewrite preserves the +// original URL in request context for downstream tracing/logging. +func TestResolveGraphPath_OriginalPathContext(t *testing.T) { + gw := &cs3mocks.GatewayAPIClient{} + gw.On("Stat", mock.Anything, mock.Anything). + Return(statResponse(cs3rpc.Code_CODE_OK, true), nil) + + selector := newTestSelector(t, gw) + original := "/graph/v1.0/drives/" + testDriveID + "/root:/Documents:/children" + + var capturedOriginal interface{} + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + capturedOriginal = r.Context().Value(middleware.OriginalPathContextKey) + w.WriteHeader(http.StatusOK) + }) + + mw := middleware.ResolveGraphPath(selector, log.NewLogger()) + req := httptest.NewRequest(http.MethodGet, "http://localhost"+original, nil) + rr := httptest.NewRecorder() + + mw(handler).ServeHTTP(rr, req) + + assert.Equal(t, http.StatusOK, rr.Code) + assert.Equal(t, original, capturedOriginal, "original URL must be available via OriginalPathContextKey") +} diff --git a/services/graph/pkg/service/v0/service.go b/services/graph/pkg/service/v0/service.go index ecce7f69e1..16c811c97a 100644 --- a/services/graph/pkg/service/v0/service.go +++ b/services/graph/pkg/service/v0/service.go @@ -132,6 +132,12 @@ func NewService(opts ...Option) (Graph, error) { //nolint:maintidx m := chi.NewMux() m.Use(options.Middleware...) + // Must be top-level mux.Use, not r.Use inside a Route block: chi + // sub-router middleware runs after the prefix has already been matched, + // so URL rewriting at that point can't redirect to a different leaf + // route. Top-level middleware runs before any matching, so the rewritten + // URL is what chi walks the radix tree against. + m.Use(graphm.ResolveGraphPath(options.GatewaySelector, options.Logger)) m.Use( otelchi.Middleware( "graph",