From 46a7dbfb4798107990a39bf8cacfec7e48ccb984 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Sun, 6 Sep 2026 22:55:51 +0200 Subject: [PATCH 01/26] feat(proxy): authenticate graph requests carrying a public link token --- services/proxy/pkg/middleware/basic_auth.go | 2 +- .../proxy/pkg/middleware/public_share_auth.go | 15 +++++++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/services/proxy/pkg/middleware/basic_auth.go b/services/proxy/pkg/middleware/basic_auth.go index bc9e1222f4..cb488587c5 100644 --- a/services/proxy/pkg/middleware/basic_auth.go +++ b/services/proxy/pkg/middleware/basic_auth.go @@ -18,7 +18,7 @@ type BasicAuthenticator struct { // Authenticate implements the authenticator interface to authenticate requests via basic auth. func (m BasicAuthenticator) Authenticate(r *http.Request) (*http.Request, bool) { - if isPublicPath(r.URL.Path) && isPublicWithShareToken(r) { + if (isPublicPath(r.URL.Path) && isPublicWithShareToken(r)) || isPublicShareGraphRequest(r) { // The authentication of public path requests is handled by another authenticator. // Since we can't guarantee the order of execution of the authenticators, we better // implement an early return here for paths we can't authenticate in this authenticator. diff --git a/services/proxy/pkg/middleware/public_share_auth.go b/services/proxy/pkg/middleware/public_share_auth.go index 047a332527..40b34abb31 100644 --- a/services/proxy/pkg/middleware/public_share_auth.go +++ b/services/proxy/pkg/middleware/public_share_auth.go @@ -55,12 +55,23 @@ func isPublicShareAppOpen(r *http.Request) bool { // the BasicAuthenticator needs to ignore the request when the headerShareToken exist. func isPublicWithShareToken(r *http.Request) bool { return (strings.HasPrefix(r.URL.Path, "/dav/public-files") || strings.HasPrefix(r.URL.Path, "/remote.php/dav/public-files")) && - (r.URL.Query().Get(headerShareToken) != "" || r.Header.Get(headerShareToken) != "") + hasShareToken(r) +} + +// Graph requests are authenticated requests, except when they carry a share +// token: then they run in the public share context, like public-files. A graph +// request without a token stays with the other authenticators. +func isPublicShareGraphRequest(r *http.Request) bool { + return strings.HasPrefix(r.URL.Path, "/graph/") && hasShareToken(r) +} + +func hasShareToken(r *http.Request) bool { + return r.URL.Query().Get(headerShareToken) != "" || r.Header.Get(headerShareToken) != "" } // Authenticate implements the authenticator interface to authenticate requests via public share auth. func (a PublicShareAuthenticator) Authenticate(r *http.Request) (*http.Request, bool) { - if !isPublicPath(r.URL.Path) && !isPublicShareArchive(r) && !isPublicShareAppOpen(r) { + if !isPublicPath(r.URL.Path) && !isPublicShareArchive(r) && !isPublicShareAppOpen(r) && !isPublicShareGraphRequest(r) { return nil, false } From 40f7aaa327b9dfd85b120d625f432d762a82e6f2 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Sun, 6 Sep 2026 22:55:52 +0200 Subject: [PATCH 02/26] fix(graph): verify token scopes against the url path The scope handlers know CS3 request types and url paths; handing them the *http.Request made every restricted scope fail with a type assertion error. Pass the path, like reva's own http auth interceptor, and allow the graph drives surface in the public share scope. The vendored reva change (checkGraphDrivesPath) needs a reva PR before this can go anywhere. --- services/graph/pkg/middleware/auth.go | 5 ++++- .../opencloud-eu/reva/v2/pkg/auth/scope/publicshare.go | 9 ++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/services/graph/pkg/middleware/auth.go b/services/graph/pkg/middleware/auth.go index aba4d44b3d..b24e6c5f29 100644 --- a/services/graph/pkg/middleware/auth.go +++ b/services/graph/pkg/middleware/auth.go @@ -67,7 +67,10 @@ func Auth(opts ...account.Option) func(http.Handler) http.Handler { errorcode.InvalidAuthenticationToken.Render(w, r, http.StatusUnauthorized, "invalid token") return } - if ok, err := scope.VerifyScope(ctx, tokenScope, r); err != nil || !ok { + // scope handlers know CS3 request types and URL paths; a restricted + // scope (a public share token, say) cannot judge an *http.Request. + // Pass the path, exactly like reva's own http auth interceptor. + if ok, err := scope.VerifyScope(ctx, tokenScope, r.URL.Path); err != nil || !ok { opt.Logger.Error().Str(log.RequestIDString, r.Header.Get("X-Request-ID")).Err(err).Msg("verifying scope failed") errorcode.InvalidAuthenticationToken.Render(w, r, http.StatusUnauthorized, "verifying scope failed") return diff --git a/vendor/github.com/opencloud-eu/reva/v2/pkg/auth/scope/publicshare.go b/vendor/github.com/opencloud-eu/reva/v2/pkg/auth/scope/publicshare.go index ce1a9e29e2..0dc4bb763f 100644 --- a/vendor/github.com/opencloud-eu/reva/v2/pkg/auth/scope/publicshare.go +++ b/vendor/github.com/opencloud-eu/reva/v2/pkg/auth/scope/publicshare.go @@ -143,7 +143,7 @@ func publicshareScope(ctx context.Context, scope *authpb.Scope, resource interfa // public links must not leak info about collaborative shares return false, nil case string: - return checkResourcePath(v), nil + return checkResourcePath(v) || checkGraphDrivesPath(v), nil } msg := "public resource type assertion failed" @@ -151,6 +151,13 @@ func publicshareScope(ctx context.Context, scope *authpb.Scope, resource interfa return false, errtypes.InternalError(msg) } +// checkGraphDrivesPath allows the graph drive item surface. Which items a +// public token may actually read is enforced per CS3 request (checkStorageRef); +// this only opens the HTTP route, like /archiver or /app/open in checkResourcePath. +func checkGraphDrivesPath(path string) bool { + return strings.HasPrefix(path, "/graph/v1.0/drives/") || strings.HasPrefix(path, "/graph/v1beta1/drives/") +} + func checkStorageRef(ctx context.Context, s *link.PublicShare, r *provider.Reference) bool { // r: path:$path > > if utils.ResourceIDEqual(s.ResourceId, r.GetResourceId()) { From 1d65ce705229191568c6a7c903dace78c936e49d Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Sun, 6 Sep 2026 22:55:52 +0200 Subject: [PATCH 03/26] feat(graph): address drive items below the public share drive Items inside a public link keep their real ids, so the storage/space equality check cannot hold below the public drive; the token scope guards access instead. Paths in responses are cut to their base name there: they are anchored at the owner's space root, and everything above the share root is the owner's directory structure. --- services/graph/pkg/middleware/path_lookup.go | 5 +- services/graph/pkg/service/v0/driveitems.go | 48 ++++++++++++++++++-- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/services/graph/pkg/middleware/path_lookup.go b/services/graph/pkg/middleware/path_lookup.go index 1fc89bcd6c..59f4c9cc20 100644 --- a/services/graph/pkg/middleware/path_lookup.go +++ b/services/graph/pkg/middleware/path_lookup.go @@ -203,7 +203,10 @@ func rewriteColonPath( 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() { + // items below the public share drive keep their real ids, so the + // storage/space prefix cannot match there; the token scope guards access + isPublicDrive := drive.GetStorageId() == utils.PublicStorageProviderID && drive.GetSpaceId() == utils.PublicStorageSpaceID + if !isPublicDrive && (drive.GetStorageId() != anchor.GetStorageId() || drive.GetSpaceId() != anchor.GetSpaceId()) { logger.Debug(). Str("driveID", driveID). Str("itemID", anchorIDStr). diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 167f8b1b62..8162dd962e 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -61,6 +61,40 @@ func odataListContains(r *http.Request, parameter, value string) bool { return false } +// driveItemInDrive reports whether an item id may be addressed below a drive. +// Normally the item must carry the drive's storage and space id. The public +// share drive is the exception: its root is the virtual public space, but the +// items inside keep their real ids (the publicstorageprovider does not rewrite +// them), so any item id is acceptable there. Access is enforced by the public +// share scope on the token, not by this routing check. +func driveItemInDrive(driveID, driveItemID *storageprovider.ResourceId) bool { + if driveID.GetStorageId() == utils.PublicStorageProviderID && driveID.GetSpaceId() == utils.PublicStorageSpaceID { + return true + } + return driveID.GetStorageId() == driveItemID.GetStorageId() && driveID.GetSpaceId() == driveItemID.GetSpaceId() +} + +// publicDriveRequest reports whether the request addresses the public share +// drive, i.e. runs in a public link context. +func publicDriveRequest(r *http.Request) bool { + driveID, err := parseIDParam(r, "driveID") + return err == nil && + driveID.GetStorageId() == utils.PublicStorageProviderID && + driveID.GetSpaceId() == utils.PublicStorageSpaceID +} + +// stripSpacePaths cuts resource paths down to their base name. A public link +// visitor gets real resource infos when navigating by id, and their paths are +// anchored at the owner's space root: everything above the share root is the +// owner's directory structure and must not leak. +func stripSpacePaths(infos ...*storageprovider.ResourceInfo) { + for _, info := range infos { + if info != nil && info.Path != "" { + info.Path = path.Base(info.Path) + } + } +} + // driveItemPropertySelected reports whether the given opt-in property was requested via $select func driveItemPropertySelected(r *http.Request, property string) bool { return odataListContains(r, "$select", property) @@ -98,7 +132,7 @@ func (g Graph) CreateUploadSession(w http.ResponseWriter, r *http.Request) { errorcode.RenderError(w, r, err) return } - if driveID.GetStorageId() != driveItemID.GetStorageId() || driveID.GetSpaceId() != driveItemID.GetSpaceId() { + if !driveItemInDrive(&driveID, &driveItemID) { errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, "Item does not exist") return } @@ -287,7 +321,7 @@ func (g Graph) GetDriveItem(w http.ResponseWriter, r *http.Request) { errorcode.RenderError(w, r, err) return } - if driveID.GetStorageId() != driveItemID.GetStorageId() || driveID.GetSpaceId() != driveItemID.GetSpaceId() { + if !driveItemInDrive(&driveID, &driveItemID) { errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, "Item does not exist") return } @@ -312,7 +346,9 @@ func (g Graph) GetDriveItem(w http.ResponseWriter, r *http.Request) { errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) return case res.GetStatus().GetCode() == cs3rpc.Code_CODE_OK: - // ok + if publicDriveRequest(r) { + stripSpacePaths(res.GetInfo()) + } case res.GetStatus().GetCode() == cs3rpc.Code_CODE_NOT_FOUND: errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, res.GetStatus().GetMessage()) return @@ -372,7 +408,7 @@ func (g Graph) GetDriveItemChildren(w http.ResponseWriter, r *http.Request) { errorcode.RenderError(w, r, err) return } - if driveID.GetStorageId() != driveItemID.GetStorageId() || driveID.GetSpaceId() != driveItemID.GetSpaceId() { + if !driveItemInDrive(&driveID, &driveItemID) { errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, "Item does not exist") return } @@ -430,6 +466,10 @@ func (g Graph) listDriveItemChildren(w http.ResponseWriter, r *http.Request, dri return nil, false } + if publicDriveRequest(r) { + stripSpacePaths(res.GetInfos()...) + } + files, err := formatDriveItems(g.logger, g.publicBaseURL, res.GetInfos()) if err != nil { errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) From aa03c6c742542325c43a9b02dcd1fa20a815fab5 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Sun, 6 Sep 2026 22:55:52 +0200 Subject: [PATCH 04/26] test(acceptance): list public links via the graph api --- tests/acceptance/bootstrap/GraphContext.php | 101 ++++++++++++ .../publicLinkDriveItemListing.feature | 155 ++++++++++++++++++ 2 files changed, 256 insertions(+) create mode 100644 tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature diff --git a/tests/acceptance/bootstrap/GraphContext.php b/tests/acceptance/bootstrap/GraphContext.php index 8129bb35f0..09ed172b29 100644 --- a/tests/acceptance/bootstrap/GraphContext.php +++ b/tests/acceptance/bootstrap/GraphContext.php @@ -3490,4 +3490,105 @@ public function userGetsDriveItemWithColonPathOfPersonalSpaceOf( $url = "/graph/$apiVersion/drives/$driveId/root:/$encoded"; $this->sendGraphRequestAndCaptureResponse($user, "GET", $url); } + + /** + * The virtual drive a public link is addressed under: the public storage + * provider/space id pair with the link token as opaque id. + * + * @param string $token + * + * @return string + */ + private function publicLinkDriveId(string $token): string { + $publicStorageId = "7993447f-687f-490d-875c-ac95e89a62a4"; + return "$publicStorageId\$$publicStorageId!$token"; + } + + /** + * Send an anonymous Graph request in the context of the last created + * public link: token via the public-token query parameter, an optional + * password as basic auth for user "public". + * + * @param string $urlSuffix part below /graph/v1.0/drives/{publicDriveId} + * @param string|null $password + * + * @return void + */ + private function publicSendsGraphDriveRequest(string $urlSuffix, ?string $password = null): void { + $token = $this->featureContext->shareNgGetLastCreatedLinkShareToken(); + $driveId = $this->publicLinkDriveId($token); + $url = $this->featureContext->getBaseUrl() + . "/graph/v1.0/drives/$driveId$urlSuffix" + . (\str_contains($urlSuffix, "?") ? "&" : "?") . "public-token=$token"; + $response = HttpRequestHelper::get( + $url, + $this->featureContext->getStepLineRef(), + $password === null ? null : "public", + $this->featureContext->getActualPassword($password) + ); + $this->featureContext->setResponse($response); + } + + /** + * @param string|null $password + * + * @return void + */ + #[When('the public lists the children of the last created public link using the Graph API')] + #[When('the public lists the children of the last created public link with password :password using the Graph API')] + public function thePublicListsTheChildrenOfTheLastCreatedPublicLink(?string $password = null): void { + $token = $this->featureContext->shareNgGetLastCreatedLinkShareToken(); + $rootId = $this->publicLinkDriveId($token); + $this->publicSendsGraphDriveRequest("/items/$rootId/children", $password); + } + + /** + * @param string $path + * @param string|null $password + * + * @return void + */ + #[When('the public lists the children of path :path of the last created public link using the Graph API')] + #[When('the public lists the children of path :path of the last created public link with password :password using the Graph API')] + public function thePublicListsTheChildrenOfPathOfTheLastCreatedPublicLink( + string $path, + ?string $password = null + ): void { + $encoded = $this->encodeColonPathSegment($path); + $this->publicSendsGraphDriveRequest("/root:/$encoded:/children", $password); + } + + /** + * @param string|null $password + * + * @return void + */ + #[When('the public gets the root of the last created public link expanding its children using the Graph API')] + #[When('the public gets the root of the last created public link expanding its children with password :password using the Graph API')] + public function thePublicGetsTheRootOfTheLastCreatedPublicLinkExpandingItsChildren(?string $password = null): void { + $token = $this->featureContext->shareNgGetLastCreatedLinkShareToken(); + $rootId = $this->publicLinkDriveId($token); + $this->publicSendsGraphDriveRequest("/items/$rootId?\$expand=children", $password); + } + + /** + * The security probe: an id of a resource that is NOT inside the public + * link must not be readable through the link's token. + * + * @param string $path + * @param string $user + * + * @return void + */ + #[When('the public tries to get the resource :path of user :user through the last created public link using the Graph API')] + #[When('the public tries to get the resource :path of user :user through the last created public link with password :password using the Graph API')] + public function thePublicTriesToGetTheResourceOfUserThroughTheLastCreatedPublicLink( + string $path, + string $user, + ?string $password = null + ): void { + $user = $this->featureContext->getActualUsername($user); + $resourceId = $this->featureContext->getFileIdForPath($user, $path); + $this->publicSendsGraphDriveRequest("/items/$resourceId", $password); + } } diff --git a/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature b/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature new file mode 100644 index 0000000000..1ba6199473 --- /dev/null +++ b/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature @@ -0,0 +1,155 @@ +Feature: listing the content of a public link via the Graph API + As an anonymous visitor of a public link + I want to list the shared folder through the Graph API + So that clients can browse public links without WebDAV + + Background: + Given user "Alice" has been created with default attributes + And user "Alice" has created folder "publicfolder" + And user "Alice" has created folder "publicfolder/sub" + And user "Alice" has uploaded file with content "hello public" to "publicfolder/a.txt" + And user "Alice" has uploaded file with content "nested" to "publicfolder/sub/b.txt" + And user "Alice" has uploaded file with content "not shared" to "private.txt" + And user "Alice" has created the following resource link share: + | resource | publicfolder | + | space | Personal | + | permissionsRole | view | + | password | %public% | + + Scenario: the public lists the children of a public link + When the public lists the children of the last created public link with password "%public%" using the Graph API + Then the HTTP status code should be "200" + And the JSON data of the response should match + """ + { + "type": "object", + "required": ["value"], + "properties": { + "value": { + "type": "array", + "minItems": 2, + "maxItems": 2, + "uniqueItems": true, + "items": { + "oneOf": [ + { + "type": "object", + "required": ["name", "folder"], + "properties": { + "name": { "const": "sub" } + } + }, + { + "type": "object", + "required": ["name", "file", "size"], + "properties": { + "name": { "const": "a.txt" }, + "size": { "const": 12 } + } + } + ] + } + } + } + } + """ + + Scenario: the public lists a subfolder of a public link by path + When the public lists the children of path "sub" of the last created public link with password "%public%" using the Graph API + Then the HTTP status code should be "200" + And the JSON data of the response should match + """ + { + "type": "object", + "required": ["value"], + "properties": { + "value": { + "type": "array", + "minItems": 1, + "maxItems": 1, + "items": { + "type": "object", + "required": ["name", "file"], + "properties": { + "name": { "const": "b.txt" } + } + } + } + } + } + """ + + Scenario: the public expands the children of the public link root + When the public gets the root of the last created public link expanding its children with password "%public%" using the Graph API + Then the HTTP status code should be "200" + And the JSON data of the response should match + """ + { + "type": "object", + "required": ["folder", "children"], + "properties": { + "children": { + "type": "array", + "minItems": 2, + "maxItems": 2, + "uniqueItems": true, + "items": { + "type": "object", + "required": ["name"] + } + } + } + } + """ + + Scenario: the public must not see the owner's paths above the share + Given user "Alice" has created folder "deep" + And user "Alice" has created folder "deep/shared" + And user "Alice" has uploaded file with content "x" to "deep/shared/c.txt" + And user "Alice" has created the following resource link share: + | resource | deep/shared | + | space | Personal | + | permissionsRole | view | + | password | %public% | + When the public lists the children of the last created public link with password "%public%" using the Graph API + Then the HTTP status code should be "200" + And the JSON data of the response should match + """ + { + "type": "object", + "required": ["value"], + "properties": { + "value": { + "type": "array", + "minItems": 1, + "maxItems": 1, + "uniqueItems": true, + "items": { + "type": "object", + "properties": { + "parentReference": { + "type": "object", + "properties": { + "path": { + "not": { "pattern": "deep" } + } + } + } + } + } + } + } + } + """ + + Scenario: listing a password protected public link without the password fails + When the public lists the children of the last created public link using the Graph API + Then the HTTP status code should be "401" + + Scenario: listing a password protected public link with a wrong password fails + When the public lists the children of the last created public link with password "wrong" using the Graph API + Then the HTTP status code should be "401" + + Scenario: a resource outside the public link is not readable through its token + When the public tries to get the resource "private.txt" of user "Alice" through the last created public link with password "%public%" using the Graph API + Then the HTTP status code should be "404" From d18c1a1edb1549b97539cf961feb798b2fb7dbef Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Sun, 6 Sep 2026 23:08:37 +0200 Subject: [PATCH 05/26] fix(graph): advertise only what the public link grants Navigating by id bypasses the publicstorageprovider, so the permission sets are the owner's: a view-only link advertised delete and upload on items below the root. Intersect with the link's permissions, the same reduction the publicstorageprovider applies on its own responses; enforcement was always intact through the token scope. --- services/graph/pkg/service/v0/driveitems.go | 73 +++++++++++++++++-- tests/acceptance/bootstrap/GraphContext.php | 16 ++++ .../publicLinkDriveItemListing.feature | 23 ++++++ 3 files changed, 104 insertions(+), 8 deletions(-) diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 8162dd962e..7b5fc2b0d6 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -83,18 +83,75 @@ func publicDriveRequest(r *http.Request) bool { driveID.GetSpaceId() == utils.PublicStorageSpaceID } -// stripSpacePaths cuts resource paths down to their base name. A public link -// visitor gets real resource infos when navigating by id, and their paths are -// anchored at the owner's space root: everything above the share root is the -// owner's directory structure and must not leak. -func stripSpacePaths(infos ...*storageprovider.ResourceInfo) { +// sanitizePublicDriveInfos prepares resource infos for a public link response. +// Navigating by id bypasses the publicstorageprovider, so the infos are the +// owner's: paths are anchored at the owner's space root (everything above the +// share root must not leak) and the permission sets are the owner's, not the +// link's. Paths are cut to their base name and permissions intersected with +// what the link grants; when the link cannot be resolved nothing is advertised. +func (g Graph) sanitizePublicDriveInfos(ctx context.Context, r *http.Request, infos ...*storageprovider.ResourceInfo) { + linkPermissions := g.publicLinkPermissions(ctx, r) for _, info := range infos { - if info != nil && info.Path != "" { + if info == nil { + continue + } + if info.Path != "" { info.Path = path.Base(info.Path) } + if info.PermissionSet != nil { + intersectPermissions(info.PermissionSet, linkPermissions) + } } } +// publicLinkPermissions resolves the permissions the public link grants. The +// link token is the opaque id of the public drive; the token scope permits +// reading exactly this one share. +func (g Graph) publicLinkPermissions(ctx context.Context, r *http.Request) *storageprovider.ResourcePermissions { + driveID, err := parseIDParam(r, "driveID") + if err != nil { + return nil + } + gatewayClient, err := g.gatewaySelector.Next() + if err != nil { + return nil + } + resp, err := gatewayClient.GetPublicShare(ctx, &link.GetPublicShareRequest{ + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Token{Token: driveID.GetOpaqueId()}, + }, + }) + if err != nil || resp.GetStatus().GetCode() != cs3rpc.Code_CODE_OK { + g.logger.Error().Err(err).Str("status", resp.GetStatus().GetCode().String()).Msg("could not resolve the public link of the request") + return nil + } + return resp.GetShare().GetPermissions().GetPermissions() +} + +// intersectPermissions reduces l to what r also grants, the same reduction the +// publicstorageprovider applies on its own responses. A nil r clears l. +func intersectPermissions(l, r *storageprovider.ResourcePermissions) { + l.AddGrant = l.AddGrant && r.GetAddGrant() + l.CreateContainer = l.CreateContainer && r.GetCreateContainer() + l.Delete = l.Delete && r.GetDelete() + l.GetPath = l.GetPath && r.GetGetPath() + l.GetQuota = l.GetQuota && r.GetGetQuota() + l.InitiateFileDownload = l.InitiateFileDownload && r.GetInitiateFileDownload() + l.InitiateFileUpload = l.InitiateFileUpload && r.GetInitiateFileUpload() + l.ListContainer = l.ListContainer && r.GetListContainer() + l.ListFileVersions = l.ListFileVersions && r.GetListFileVersions() + l.ListGrants = l.ListGrants && r.GetListGrants() + l.ListRecycle = l.ListRecycle && r.GetListRecycle() + l.Move = l.Move && r.GetMove() + l.PurgeRecycle = l.PurgeRecycle && r.GetPurgeRecycle() + l.RemoveGrant = l.RemoveGrant && r.GetRemoveGrant() + l.RestoreFileVersion = l.RestoreFileVersion && r.GetRestoreFileVersion() + l.RestoreRecycleItem = l.RestoreRecycleItem && r.GetRestoreRecycleItem() + l.Stat = l.Stat && r.GetStat() + l.UpdateGrant = l.UpdateGrant && r.GetUpdateGrant() + l.DenyGrant = l.DenyGrant && r.GetDenyGrant() +} + // driveItemPropertySelected reports whether the given opt-in property was requested via $select func driveItemPropertySelected(r *http.Request, property string) bool { return odataListContains(r, "$select", property) @@ -347,7 +404,7 @@ func (g Graph) GetDriveItem(w http.ResponseWriter, r *http.Request) { return case res.GetStatus().GetCode() == cs3rpc.Code_CODE_OK: if publicDriveRequest(r) { - stripSpacePaths(res.GetInfo()) + g.sanitizePublicDriveInfos(ctx, r, res.GetInfo()) } case res.GetStatus().GetCode() == cs3rpc.Code_CODE_NOT_FOUND: errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, res.GetStatus().GetMessage()) @@ -467,7 +524,7 @@ func (g Graph) listDriveItemChildren(w http.ResponseWriter, r *http.Request, dri } if publicDriveRequest(r) { - stripSpacePaths(res.GetInfos()...) + g.sanitizePublicDriveInfos(r.Context(), r, res.GetInfos()...) } files, err := formatDriveItems(g.logger, g.publicBaseURL, res.GetInfos()) diff --git a/tests/acceptance/bootstrap/GraphContext.php b/tests/acceptance/bootstrap/GraphContext.php index 09ed172b29..ba9ebe5a60 100644 --- a/tests/acceptance/bootstrap/GraphContext.php +++ b/tests/acceptance/bootstrap/GraphContext.php @@ -3571,6 +3571,22 @@ public function thePublicGetsTheRootOfTheLastCreatedPublicLinkExpandingItsChildr $this->publicSendsGraphDriveRequest("/items/$rootId?\$expand=children", $password); } + /** + * @param string $path + * @param string|null $password + * + * @return void + */ + #[When('the public gets the drive item of path :path of the last created public link selecting the allowed actions with password :password using the Graph API')] + public function thePublicGetsTheDriveItemOfPathSelectingTheAllowedActions( + string $path, + ?string $password = null + ): void { + $encoded = $this->encodeColonPathSegment($path); + $select = "%24select=%40libre.graph.permissions.actions.allowedValues"; + $this->publicSendsGraphDriveRequest("/root:/$encoded?$select", $password); + } + /** * The security probe: an id of a resource that is NOT inside the public * link must not be readable through the link's token. diff --git a/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature b/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature index 1ba6199473..fcf10644ef 100644 --- a/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature +++ b/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature @@ -142,6 +142,29 @@ Feature: listing the content of a public link via the Graph API } """ + Scenario: advertised permissions inside a public link stay within the link role + When the public gets the drive item of path "sub" of the last created public link selecting the allowed actions with password "%public%" using the Graph API + Then the HTTP status code should be "200" + And the JSON data of the response should match + """ + { + "type": "object", + "required": ["@libre.graph.permissions.actions.allowedValues"], + "properties": { + "@libre.graph.permissions.actions.allowedValues": { + "type": "array", + "minItems": 6, + "maxItems": 6, + "uniqueItems": true, + "items": { + "type": "string", + "not": { "pattern": "/(delete|create|update|deny)$" } + } + } + } + } + """ + Scenario: listing a password protected public link without the password fails When the public lists the children of the last created public link using the Graph API Then the HTTP status code should be "401" From b045b2be864732ddad96ab2be39abc12c9f1816d Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Sun, 6 Sep 2026 23:13:47 +0200 Subject: [PATCH 06/26] fix(vendor): filterPermissions misses DenyGrant The publicstorageprovider reduces child permissions to what the link grants, field by field, but DenyGrant is not in the list: a child of a view-only link could advertise deny. Same vendored-reva caveat as checkGraphDrivesPath, goes into the reva PR later. --- .../grpc/services/publicstorageprovider/publicstorageprovider.go | 1 + 1 file changed, 1 insertion(+) diff --git a/vendor/github.com/opencloud-eu/reva/v2/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/vendor/github.com/opencloud-eu/reva/v2/internal/grpc/services/publicstorageprovider/publicstorageprovider.go index e2643547ba..1a1ce89f94 100644 --- a/vendor/github.com/opencloud-eu/reva/v2/internal/grpc/services/publicstorageprovider/publicstorageprovider.go +++ b/vendor/github.com/opencloud-eu/reva/v2/internal/grpc/services/publicstorageprovider/publicstorageprovider.go @@ -871,6 +871,7 @@ func filterPermissions(l *provider.ResourcePermissions, r *provider.ResourcePerm l.RestoreRecycleItem = l.RestoreRecycleItem && r.RestoreRecycleItem l.Stat = l.Stat && r.Stat l.UpdateGrant = l.UpdateGrant && r.UpdateGrant + l.DenyGrant = l.DenyGrant && r.DenyGrant } func (s *service) ListFileVersions(ctx context.Context, req *provider.ListFileVersionsRequest) (*provider.ListFileVersionsResponse, error) { From af999345a669ccf5427eeafd828eac05b3bbf28e Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Sun, 6 Sep 2026 23:29:38 +0200 Subject: [PATCH 07/26] refactor: the public link reduction lives once, in reva publicshare.ReduceResourceInfo/ReducePermissions replace both copies: the publicstorageprovider's private filterPermissions plus its augment path rewrite, and graph's reimplementation. Graph resolves the link of the request (share root and grant, one GetPublicShare and one Stat) and applies the same reduction the provider applies to its own responses. Paths below the public drive are share-root relative now, on every route. Vendored reva change, goes into the reva PR later. --- services/graph/pkg/service/v0/driveitems.go | 76 +++++++------------ tests/acceptance/docker/src/publink-local.yml | 4 + .../publicstorageprovider.go | 36 +-------- .../reva/v2/pkg/publicshare/reduce.go | 73 ++++++++++++++++++ 4 files changed, 109 insertions(+), 80 deletions(-) create mode 100644 tests/acceptance/docker/src/publink-local.yml create mode 100644 vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/reduce.go diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 7b5fc2b0d6..6c06be893d 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -83,73 +83,55 @@ func publicDriveRequest(r *http.Request) bool { driveID.GetSpaceId() == utils.PublicStorageSpaceID } -// sanitizePublicDriveInfos prepares resource infos for a public link response. -// Navigating by id bypasses the publicstorageprovider, so the infos are the -// owner's: paths are anchored at the owner's space root (everything above the -// share root must not leak) and the permission sets are the owner's, not the -// link's. Paths are cut to their base name and permissions intersected with -// what the link grants; when the link cannot be resolved nothing is advertised. +// sanitizePublicDriveInfos prepares resource infos for a public link +// response. Navigating by id bypasses the publicstorageprovider, so the infos +// are the owner's; publicshare.ReduceResourceInfo is the provider's own +// reduction: paths relative to the share root, permissions cut to the link +// grant. When the link cannot be resolved nothing is advertised. func (g Graph) sanitizePublicDriveInfos(ctx context.Context, r *http.Request, infos ...*storageprovider.ResourceInfo) { - linkPermissions := g.publicLinkPermissions(ctx, r) + shareRoot, grant := g.publicLinkOfRequest(ctx, r) for _, info := range infos { if info == nil { continue } - if info.Path != "" { - info.Path = path.Base(info.Path) - } - if info.PermissionSet != nil { - intersectPermissions(info.PermissionSet, linkPermissions) + if shareRoot == nil { + info.Path = path.Base(info.GetPath()) + info.PermissionSet = &storageprovider.ResourcePermissions{} + continue } + publicshare.ReduceResourceInfo(info, shareRoot, grant) } } -// publicLinkPermissions resolves the permissions the public link grants. The -// link token is the opaque id of the public drive; the token scope permits -// reading exactly this one share. -func (g Graph) publicLinkPermissions(ctx context.Context, r *http.Request) *storageprovider.ResourcePermissions { +// publicLinkOfRequest resolves the public link the request runs in: the share +// root info and the granted permissions. The link token is the opaque id of +// the public drive; the token scope permits reading exactly this one share. +func (g Graph) publicLinkOfRequest(ctx context.Context, r *http.Request) (*storageprovider.ResourceInfo, *storageprovider.ResourcePermissions) { driveID, err := parseIDParam(r, "driveID") if err != nil { - return nil + return nil, nil } gatewayClient, err := g.gatewaySelector.Next() if err != nil { - return nil + return nil, nil } - resp, err := gatewayClient.GetPublicShare(ctx, &link.GetPublicShareRequest{ + shareResp, err := gatewayClient.GetPublicShare(ctx, &link.GetPublicShareRequest{ Ref: &link.PublicShareReference{ Spec: &link.PublicShareReference_Token{Token: driveID.GetOpaqueId()}, }, }) - if err != nil || resp.GetStatus().GetCode() != cs3rpc.Code_CODE_OK { - g.logger.Error().Err(err).Str("status", resp.GetStatus().GetCode().String()).Msg("could not resolve the public link of the request") - return nil + if err != nil || shareResp.GetStatus().GetCode() != cs3rpc.Code_CODE_OK { + g.logger.Error().Err(err).Str("status", shareResp.GetStatus().GetCode().String()).Msg("could not resolve the public link of the request") + return nil, nil } - return resp.GetShare().GetPermissions().GetPermissions() -} - -// intersectPermissions reduces l to what r also grants, the same reduction the -// publicstorageprovider applies on its own responses. A nil r clears l. -func intersectPermissions(l, r *storageprovider.ResourcePermissions) { - l.AddGrant = l.AddGrant && r.GetAddGrant() - l.CreateContainer = l.CreateContainer && r.GetCreateContainer() - l.Delete = l.Delete && r.GetDelete() - l.GetPath = l.GetPath && r.GetGetPath() - l.GetQuota = l.GetQuota && r.GetGetQuota() - l.InitiateFileDownload = l.InitiateFileDownload && r.GetInitiateFileDownload() - l.InitiateFileUpload = l.InitiateFileUpload && r.GetInitiateFileUpload() - l.ListContainer = l.ListContainer && r.GetListContainer() - l.ListFileVersions = l.ListFileVersions && r.GetListFileVersions() - l.ListGrants = l.ListGrants && r.GetListGrants() - l.ListRecycle = l.ListRecycle && r.GetListRecycle() - l.Move = l.Move && r.GetMove() - l.PurgeRecycle = l.PurgeRecycle && r.GetPurgeRecycle() - l.RemoveGrant = l.RemoveGrant && r.GetRemoveGrant() - l.RestoreFileVersion = l.RestoreFileVersion && r.GetRestoreFileVersion() - l.RestoreRecycleItem = l.RestoreRecycleItem && r.GetRestoreRecycleItem() - l.Stat = l.Stat && r.GetStat() - l.UpdateGrant = l.UpdateGrant && r.GetUpdateGrant() - l.DenyGrant = l.DenyGrant && r.GetDenyGrant() + statResp, err := gatewayClient.Stat(ctx, &storageprovider.StatRequest{ + Ref: &storageprovider.Reference{ResourceId: shareResp.GetShare().GetResourceId()}, + }) + if err != nil || statResp.GetStatus().GetCode() != cs3rpc.Code_CODE_OK { + g.logger.Error().Err(err).Str("status", statResp.GetStatus().GetCode().String()).Msg("could not stat the public link root") + return nil, nil + } + return statResp.GetInfo(), shareResp.GetShare().GetPermissions().GetPermissions() } // driveItemPropertySelected reports whether the given opt-in property was requested via $select diff --git a/tests/acceptance/docker/src/publink-local.yml b/tests/acceptance/docker/src/publink-local.yml new file mode 100644 index 0000000000..d380d09b3c --- /dev/null +++ b/tests/acceptance/docker/src/publink-local.yml @@ -0,0 +1,4 @@ +services: + opencloud-server: + ports: !override + - "9402:9200" diff --git a/vendor/github.com/opencloud-eu/reva/v2/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/vendor/github.com/opencloud-eu/reva/v2/internal/grpc/services/publicstorageprovider/publicstorageprovider.go index 1a1ce89f94..f854fff2f3 100644 --- a/vendor/github.com/opencloud-eu/reva/v2/internal/grpc/services/publicstorageprovider/publicstorageprovider.go +++ b/vendor/github.com/opencloud-eu/reva/v2/internal/grpc/services/publicstorageprovider/publicstorageprovider.go @@ -23,7 +23,6 @@ package publicstorageprovider import ( "context" "encoding/json" - "path" "strings" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" @@ -37,6 +36,7 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/appctx" ctxpkg "github.com/opencloud-eu/reva/v2/pkg/ctx" "github.com/opencloud-eu/reva/v2/pkg/errtypes" + "github.com/opencloud-eu/reva/v2/pkg/publicshare" "github.com/opencloud-eu/reva/v2/pkg/rgrpc" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/status" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" @@ -769,15 +769,7 @@ func (s *service) augmentStatResponse(ctx context.Context, statInfo *provider.Re appctx.GetLogger(ctx).Error().Err(err).Interface("share", share).Interface("info", statInfo).Msg("error when adding share") } - var sharePath string - if shareInfo.Type == provider.ResourceType_RESOURCE_TYPE_FILE { - sharePath = path.Base(shareInfo.Path) - } else { - sharePath = strings.TrimPrefix(statInfo.Path, shareInfo.Path) - } - - statInfo.Path = path.Join("/", sharePath) - filterPermissions(statInfo.PermissionSet, shareInfo.PermissionSet) + publicshare.ReduceResourceInfo(statInfo, shareInfo, shareInfo.GetPermissionSet()) } } @@ -843,7 +835,7 @@ func (s *service) ListContainer(ctx context.Context, req *provider.ListContainer for i := range listContainerR.Infos { // FIXME how do we reduce permissions to what is granted by the public link? // only a problem for id based access -> middleware - filterPermissions(listContainerR.Infos[i].PermissionSet, info.PermissionSet) + publicshare.ReducePermissions(listContainerR.Infos[i].PermissionSet, info.PermissionSet) if err := addShare(listContainerR.Infos[i], share); err != nil { appctx.GetLogger(ctx).Error().Err(err).Interface("share", share).Interface("info", listContainerR.Infos[i]).Msg("error when adding share") } @@ -852,28 +844,6 @@ func (s *service) ListContainer(ctx context.Context, req *provider.ListContainer return listContainerR, nil } -func filterPermissions(l *provider.ResourcePermissions, r *provider.ResourcePermissions) { - l.AddGrant = l.AddGrant && r.AddGrant - l.CreateContainer = l.CreateContainer && r.CreateContainer - l.Delete = l.Delete && r.Delete - l.GetPath = l.GetPath && r.GetPath - l.GetQuota = l.GetQuota && r.GetQuota - l.InitiateFileDownload = l.InitiateFileDownload && r.InitiateFileDownload - l.InitiateFileUpload = l.InitiateFileUpload && r.InitiateFileUpload - l.ListContainer = l.ListContainer && r.ListContainer - l.ListFileVersions = l.ListFileVersions && r.ListFileVersions - l.ListGrants = l.ListGrants && r.ListGrants - l.ListRecycle = l.ListRecycle && r.ListRecycle - l.Move = l.Move && r.Move - l.PurgeRecycle = l.PurgeRecycle && r.PurgeRecycle - l.RemoveGrant = l.RemoveGrant && r.RemoveGrant - l.RestoreFileVersion = l.RestoreFileVersion && r.RestoreFileVersion - l.RestoreRecycleItem = l.RestoreRecycleItem && r.RestoreRecycleItem - l.Stat = l.Stat && r.Stat - l.UpdateGrant = l.UpdateGrant && r.UpdateGrant - l.DenyGrant = l.DenyGrant && r.DenyGrant -} - func (s *service) ListFileVersions(ctx context.Context, req *provider.ListFileVersionsRequest) (*provider.ListFileVersionsResponse, error) { return nil, gstatus.Errorf(codes.Unimplemented, "method not implemented") } diff --git a/vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/reduce.go b/vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/reduce.go new file mode 100644 index 0000000000..7e0e471af4 --- /dev/null +++ b/vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/reduce.go @@ -0,0 +1,73 @@ +// Copyright 2018-2026 CERN +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// In applying this license, CERN does not waive the privileges and immunities +// granted to it by virtue of its status as an Intergovernmental Organization +// or submit itself to any jurisdiction. + +package publicshare + +import ( + "path" + "strings" + + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" +) + +// ReduceResourceInfo rewrites a resource info for a public link consumer. The +// path becomes relative to the share root, so the owner's directory structure +// above the share does not leak, and the permissions are cut to what the link +// grants. The publicstorageprovider applies it to everything it serves; +// consumers that reach resources inside a link by id bypass that provider and +// apply it themselves. +func ReduceResourceInfo(info, shareRoot *provider.ResourceInfo, grant *provider.ResourcePermissions) { + if info == nil { + return + } + + var sharePath string + if shareRoot.GetType() == provider.ResourceType_RESOURCE_TYPE_FILE { + sharePath = path.Base(shareRoot.GetPath()) + } else { + sharePath = strings.TrimPrefix(info.GetPath(), shareRoot.GetPath()) + } + info.Path = path.Join("/", sharePath) + + if info.PermissionSet != nil { + ReducePermissions(info.PermissionSet, grant) + } +} + +// ReducePermissions reduces l to what r also grants. A nil r clears l. +func ReducePermissions(l, r *provider.ResourcePermissions) { + l.AddGrant = l.AddGrant && r.GetAddGrant() + l.CreateContainer = l.CreateContainer && r.GetCreateContainer() + l.Delete = l.Delete && r.GetDelete() + l.DenyGrant = l.DenyGrant && r.GetDenyGrant() + l.GetPath = l.GetPath && r.GetGetPath() + l.GetQuota = l.GetQuota && r.GetGetQuota() + l.InitiateFileDownload = l.InitiateFileDownload && r.GetInitiateFileDownload() + l.InitiateFileUpload = l.InitiateFileUpload && r.GetInitiateFileUpload() + l.ListContainer = l.ListContainer && r.GetListContainer() + l.ListFileVersions = l.ListFileVersions && r.GetListFileVersions() + l.ListGrants = l.ListGrants && r.GetListGrants() + l.ListRecycle = l.ListRecycle && r.GetListRecycle() + l.Move = l.Move && r.GetMove() + l.PurgeRecycle = l.PurgeRecycle && r.GetPurgeRecycle() + l.RemoveGrant = l.RemoveGrant && r.GetRemoveGrant() + l.RestoreFileVersion = l.RestoreFileVersion && r.GetRestoreFileVersion() + l.RestoreRecycleItem = l.RestoreRecycleItem && r.GetRestoreRecycleItem() + l.Stat = l.Stat && r.GetStat() + l.UpdateGrant = l.UpdateGrant && r.GetUpdateGrant() +} From adb732777951d5e1ba84eb375a57f9b7875495fb Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Sun, 6 Sep 2026 23:32:27 +0200 Subject: [PATCH 08/26] docs: trim comments --- services/graph/pkg/middleware/auth.go | 5 ++-- services/graph/pkg/middleware/path_lookup.go | 3 +-- services/graph/pkg/service/v0/driveitems.go | 23 +++++++------------ .../proxy/pkg/middleware/public_share_auth.go | 5 ++-- .../reva/v2/pkg/auth/scope/publicshare.go | 5 ++-- .../reva/v2/pkg/publicshare/reduce.go | 10 ++++---- 6 files changed, 19 insertions(+), 32 deletions(-) diff --git a/services/graph/pkg/middleware/auth.go b/services/graph/pkg/middleware/auth.go index b24e6c5f29..c8a73d150e 100644 --- a/services/graph/pkg/middleware/auth.go +++ b/services/graph/pkg/middleware/auth.go @@ -67,9 +67,8 @@ func Auth(opts ...account.Option) func(http.Handler) http.Handler { errorcode.InvalidAuthenticationToken.Render(w, r, http.StatusUnauthorized, "invalid token") return } - // scope handlers know CS3 request types and URL paths; a restricted - // scope (a public share token, say) cannot judge an *http.Request. - // Pass the path, exactly like reva's own http auth interceptor. + // scope handlers judge CS3 request types and URL paths, never an + // *http.Request; pass the path like reva's own http interceptor if ok, err := scope.VerifyScope(ctx, tokenScope, r.URL.Path); err != nil || !ok { opt.Logger.Error().Str(log.RequestIDString, r.Header.Get("X-Request-ID")).Err(err).Msg("verifying scope failed") errorcode.InvalidAuthenticationToken.Render(w, r, http.StatusUnauthorized, "verifying scope failed") diff --git a/services/graph/pkg/middleware/path_lookup.go b/services/graph/pkg/middleware/path_lookup.go index 59f4c9cc20..32c9cda810 100644 --- a/services/graph/pkg/middleware/path_lookup.go +++ b/services/graph/pkg/middleware/path_lookup.go @@ -203,8 +203,7 @@ func rewriteColonPath( logger.Debug().Err(err).Str("driveID", driveID).Msg("invalid drive id in colon path") return "", errInvalidRequest } - // items below the public share drive keep their real ids, so the - // storage/space prefix cannot match there; the token scope guards access + // items below the public share drive keep their real ids; the scope guards access isPublicDrive := drive.GetStorageId() == utils.PublicStorageProviderID && drive.GetSpaceId() == utils.PublicStorageSpaceID if !isPublicDrive && (drive.GetStorageId() != anchor.GetStorageId() || drive.GetSpaceId() != anchor.GetSpaceId()) { logger.Debug(). diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 6c06be893d..4bee8452e4 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -62,11 +62,8 @@ func odataListContains(r *http.Request, parameter, value string) bool { } // driveItemInDrive reports whether an item id may be addressed below a drive. -// Normally the item must carry the drive's storage and space id. The public -// share drive is the exception: its root is the virtual public space, but the -// items inside keep their real ids (the publicstorageprovider does not rewrite -// them), so any item id is acceptable there. Access is enforced by the public -// share scope on the token, not by this routing check. +// Items below the public share drive keep their real ids, so any id is +// acceptable there; the token scope enforces access, not this routing check. func driveItemInDrive(driveID, driveItemID *storageprovider.ResourceId) bool { if driveID.GetStorageId() == utils.PublicStorageProviderID && driveID.GetSpaceId() == utils.PublicStorageSpaceID { return true @@ -74,8 +71,7 @@ func driveItemInDrive(driveID, driveItemID *storageprovider.ResourceId) bool { return driveID.GetStorageId() == driveItemID.GetStorageId() && driveID.GetSpaceId() == driveItemID.GetSpaceId() } -// publicDriveRequest reports whether the request addresses the public share -// drive, i.e. runs in a public link context. +// publicDriveRequest reports whether the request addresses the public share drive. func publicDriveRequest(r *http.Request) bool { driveID, err := parseIDParam(r, "driveID") return err == nil && @@ -83,11 +79,9 @@ func publicDriveRequest(r *http.Request) bool { driveID.GetSpaceId() == utils.PublicStorageSpaceID } -// sanitizePublicDriveInfos prepares resource infos for a public link -// response. Navigating by id bypasses the publicstorageprovider, so the infos -// are the owner's; publicshare.ReduceResourceInfo is the provider's own -// reduction: paths relative to the share root, permissions cut to the link -// grant. When the link cannot be resolved nothing is advertised. +// sanitizePublicDriveInfos applies the publicstorageprovider's reduction to +// infos that bypassed it (navigation by id): paths relative to the share root, +// permissions cut to the link grant. An unresolvable link advertises nothing. func (g Graph) sanitizePublicDriveInfos(ctx context.Context, r *http.Request, infos ...*storageprovider.ResourceInfo) { shareRoot, grant := g.publicLinkOfRequest(ctx, r) for _, info := range infos { @@ -103,9 +97,8 @@ func (g Graph) sanitizePublicDriveInfos(ctx context.Context, r *http.Request, in } } -// publicLinkOfRequest resolves the public link the request runs in: the share -// root info and the granted permissions. The link token is the opaque id of -// the public drive; the token scope permits reading exactly this one share. +// publicLinkOfRequest resolves the request's public link into the share root +// info and the granted permissions; the token is the public drive's opaque id. func (g Graph) publicLinkOfRequest(ctx context.Context, r *http.Request) (*storageprovider.ResourceInfo, *storageprovider.ResourcePermissions) { driveID, err := parseIDParam(r, "driveID") if err != nil { diff --git a/services/proxy/pkg/middleware/public_share_auth.go b/services/proxy/pkg/middleware/public_share_auth.go index 40b34abb31..5b1e7c0739 100644 --- a/services/proxy/pkg/middleware/public_share_auth.go +++ b/services/proxy/pkg/middleware/public_share_auth.go @@ -58,9 +58,8 @@ func isPublicWithShareToken(r *http.Request) bool { hasShareToken(r) } -// Graph requests are authenticated requests, except when they carry a share -// token: then they run in the public share context, like public-files. A graph -// request without a token stays with the other authenticators. +// Graph requests carrying a share token run in the public share context, like +// public-files; without a token they stay with the other authenticators. func isPublicShareGraphRequest(r *http.Request) bool { return strings.HasPrefix(r.URL.Path, "/graph/") && hasShareToken(r) } diff --git a/vendor/github.com/opencloud-eu/reva/v2/pkg/auth/scope/publicshare.go b/vendor/github.com/opencloud-eu/reva/v2/pkg/auth/scope/publicshare.go index 0dc4bb763f..ed210f89dc 100644 --- a/vendor/github.com/opencloud-eu/reva/v2/pkg/auth/scope/publicshare.go +++ b/vendor/github.com/opencloud-eu/reva/v2/pkg/auth/scope/publicshare.go @@ -151,9 +151,8 @@ func publicshareScope(ctx context.Context, scope *authpb.Scope, resource interfa return false, errtypes.InternalError(msg) } -// checkGraphDrivesPath allows the graph drive item surface. Which items a -// public token may actually read is enforced per CS3 request (checkStorageRef); -// this only opens the HTTP route, like /archiver or /app/open in checkResourcePath. +// checkGraphDrivesPath opens the graph drive item routes, like /archiver in +// checkResourcePath; what a token may read is enforced per CS3 request. func checkGraphDrivesPath(path string) bool { return strings.HasPrefix(path, "/graph/v1.0/drives/") || strings.HasPrefix(path, "/graph/v1beta1/drives/") } diff --git a/vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/reduce.go b/vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/reduce.go index 7e0e471af4..9f01c0290d 100644 --- a/vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/reduce.go +++ b/vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/reduce.go @@ -25,12 +25,10 @@ import ( provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" ) -// ReduceResourceInfo rewrites a resource info for a public link consumer. The -// path becomes relative to the share root, so the owner's directory structure -// above the share does not leak, and the permissions are cut to what the link -// grants. The publicstorageprovider applies it to everything it serves; -// consumers that reach resources inside a link by id bypass that provider and -// apply it themselves. +// ReduceResourceInfo rewrites a resource info for a public link consumer: the +// path becomes share-root relative (the structure above must not leak), the +// permissions are cut to the link grant. Consumers that reach link content by +// id bypass the publicstorageprovider and apply it themselves. func ReduceResourceInfo(info, shareRoot *provider.ResourceInfo, grant *provider.ResourcePermissions) { if info == nil { return From 6eb235ab1c990d5878b35138eec2df44090296d3 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Sun, 6 Sep 2026 23:40:07 +0200 Subject: [PATCH 09/26] chore: drop a local compose override that slipped in --- tests/acceptance/docker/src/publink-local.yml | 4 ---- 1 file changed, 4 deletions(-) delete mode 100644 tests/acceptance/docker/src/publink-local.yml diff --git a/tests/acceptance/docker/src/publink-local.yml b/tests/acceptance/docker/src/publink-local.yml deleted file mode 100644 index d380d09b3c..0000000000 --- a/tests/acceptance/docker/src/publink-local.yml +++ /dev/null @@ -1,4 +0,0 @@ -services: - opencloud-server: - ports: !override - - "9402:9200" From da421b6747bd1cf9d2dfdb161f8c1ffb72da132f Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Sun, 6 Sep 2026 23:40:07 +0200 Subject: [PATCH 10/26] refactor(graph): fail when the public link does not resolve The fallback reduced by hand, base name paths and a blank permission set, a second copy of the reduction in degraded form. If the link of the request cannot be resolved there is nothing to serve. --- services/graph/pkg/service/v0/driveitems.go | 50 +++++++++++---------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 4bee8452e4..8aa301d6cb 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -81,50 +81,48 @@ func publicDriveRequest(r *http.Request) bool { // sanitizePublicDriveInfos applies the publicstorageprovider's reduction to // infos that bypassed it (navigation by id): paths relative to the share root, -// permissions cut to the link grant. An unresolvable link advertises nothing. -func (g Graph) sanitizePublicDriveInfos(ctx context.Context, r *http.Request, infos ...*storageprovider.ResourceInfo) { - shareRoot, grant := g.publicLinkOfRequest(ctx, r) +// permissions cut to the link grant. +func (g Graph) sanitizePublicDriveInfos(ctx context.Context, r *http.Request, infos ...*storageprovider.ResourceInfo) error { + shareRoot, grant, err := g.publicLinkOfRequest(ctx, r) + if err != nil { + return err + } for _, info := range infos { - if info == nil { - continue - } - if shareRoot == nil { - info.Path = path.Base(info.GetPath()) - info.PermissionSet = &storageprovider.ResourcePermissions{} - continue + if info != nil { + publicshare.ReduceResourceInfo(info, shareRoot, grant) } - publicshare.ReduceResourceInfo(info, shareRoot, grant) } + return nil } // publicLinkOfRequest resolves the request's public link into the share root // info and the granted permissions; the token is the public drive's opaque id. -func (g Graph) publicLinkOfRequest(ctx context.Context, r *http.Request) (*storageprovider.ResourceInfo, *storageprovider.ResourcePermissions) { +func (g Graph) publicLinkOfRequest(ctx context.Context, r *http.Request) (*storageprovider.ResourceInfo, *storageprovider.ResourcePermissions, error) { driveID, err := parseIDParam(r, "driveID") if err != nil { - return nil, nil + return nil, nil, err } gatewayClient, err := g.gatewaySelector.Next() if err != nil { - return nil, nil + return nil, nil, err } shareResp, err := gatewayClient.GetPublicShare(ctx, &link.GetPublicShareRequest{ Ref: &link.PublicShareReference{ Spec: &link.PublicShareReference_Token{Token: driveID.GetOpaqueId()}, }, }) - if err != nil || shareResp.GetStatus().GetCode() != cs3rpc.Code_CODE_OK { - g.logger.Error().Err(err).Str("status", shareResp.GetStatus().GetCode().String()).Msg("could not resolve the public link of the request") - return nil, nil + if err := errorcode.FromCS3Status(shareResp.GetStatus(), err); err != nil { + g.logger.Error().Err(err).Msg("could not resolve the public link of the request") + return nil, nil, err } statResp, err := gatewayClient.Stat(ctx, &storageprovider.StatRequest{ Ref: &storageprovider.Reference{ResourceId: shareResp.GetShare().GetResourceId()}, }) - if err != nil || statResp.GetStatus().GetCode() != cs3rpc.Code_CODE_OK { - g.logger.Error().Err(err).Str("status", statResp.GetStatus().GetCode().String()).Msg("could not stat the public link root") - return nil, nil + if err := errorcode.FromCS3Status(statResp.GetStatus(), err); err != nil { + g.logger.Error().Err(err).Msg("could not stat the public link root") + return nil, nil, err } - return statResp.GetInfo(), shareResp.GetShare().GetPermissions().GetPermissions() + return statResp.GetInfo(), shareResp.GetShare().GetPermissions().GetPermissions(), nil } // driveItemPropertySelected reports whether the given opt-in property was requested via $select @@ -379,7 +377,10 @@ func (g Graph) GetDriveItem(w http.ResponseWriter, r *http.Request) { return case res.GetStatus().GetCode() == cs3rpc.Code_CODE_OK: if publicDriveRequest(r) { - g.sanitizePublicDriveInfos(ctx, r, res.GetInfo()) + if err := g.sanitizePublicDriveInfos(ctx, r, res.GetInfo()); err != nil { + errorcode.RenderError(w, r, err) + return + } } case res.GetStatus().GetCode() == cs3rpc.Code_CODE_NOT_FOUND: errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, res.GetStatus().GetMessage()) @@ -499,7 +500,10 @@ func (g Graph) listDriveItemChildren(w http.ResponseWriter, r *http.Request, dri } if publicDriveRequest(r) { - g.sanitizePublicDriveInfos(r.Context(), r, res.GetInfos()...) + if err := g.sanitizePublicDriveInfos(r.Context(), r, res.GetInfos()...); err != nil { + errorcode.RenderError(w, r, err) + return nil, false + } } files, err := formatDriveItems(g.logger, g.publicBaseURL, res.GetInfos()) From 1e29eb9504b5c4a932bbe78b1a1f9486adf045d5 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Sun, 6 Sep 2026 23:41:45 +0200 Subject: [PATCH 11/26] refactor: keep the provider's filter naming --- services/graph/pkg/service/v0/driveitems.go | 2 +- tests/acceptance/docker/src/publink-local.yml | 4 ++++ .../publicstorageprovider/publicstorageprovider.go | 4 ++-- .../reva/v2/pkg/publicshare/{reduce.go => filter.go} | 10 +++++----- 4 files changed, 12 insertions(+), 8 deletions(-) create mode 100644 tests/acceptance/docker/src/publink-local.yml rename vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/{reduce.go => filter.go} (89%) diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 8aa301d6cb..4c58be5ffd 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -89,7 +89,7 @@ func (g Graph) sanitizePublicDriveInfos(ctx context.Context, r *http.Request, in } for _, info := range infos { if info != nil { - publicshare.ReduceResourceInfo(info, shareRoot, grant) + publicshare.FilterResourceInfo(info, shareRoot, grant) } } return nil diff --git a/tests/acceptance/docker/src/publink-local.yml b/tests/acceptance/docker/src/publink-local.yml new file mode 100644 index 0000000000..d380d09b3c --- /dev/null +++ b/tests/acceptance/docker/src/publink-local.yml @@ -0,0 +1,4 @@ +services: + opencloud-server: + ports: !override + - "9402:9200" diff --git a/vendor/github.com/opencloud-eu/reva/v2/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/vendor/github.com/opencloud-eu/reva/v2/internal/grpc/services/publicstorageprovider/publicstorageprovider.go index f854fff2f3..7b569bb63f 100644 --- a/vendor/github.com/opencloud-eu/reva/v2/internal/grpc/services/publicstorageprovider/publicstorageprovider.go +++ b/vendor/github.com/opencloud-eu/reva/v2/internal/grpc/services/publicstorageprovider/publicstorageprovider.go @@ -769,7 +769,7 @@ func (s *service) augmentStatResponse(ctx context.Context, statInfo *provider.Re appctx.GetLogger(ctx).Error().Err(err).Interface("share", share).Interface("info", statInfo).Msg("error when adding share") } - publicshare.ReduceResourceInfo(statInfo, shareInfo, shareInfo.GetPermissionSet()) + publicshare.FilterResourceInfo(statInfo, shareInfo, shareInfo.GetPermissionSet()) } } @@ -835,7 +835,7 @@ func (s *service) ListContainer(ctx context.Context, req *provider.ListContainer for i := range listContainerR.Infos { // FIXME how do we reduce permissions to what is granted by the public link? // only a problem for id based access -> middleware - publicshare.ReducePermissions(listContainerR.Infos[i].PermissionSet, info.PermissionSet) + publicshare.FilterPermissions(listContainerR.Infos[i].PermissionSet, info.PermissionSet) if err := addShare(listContainerR.Infos[i], share); err != nil { appctx.GetLogger(ctx).Error().Err(err).Interface("share", share).Interface("info", listContainerR.Infos[i]).Msg("error when adding share") } diff --git a/vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/reduce.go b/vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/filter.go similarity index 89% rename from vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/reduce.go rename to vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/filter.go index 9f01c0290d..ff2b3afee4 100644 --- a/vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/reduce.go +++ b/vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/filter.go @@ -25,11 +25,11 @@ import ( provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" ) -// ReduceResourceInfo rewrites a resource info for a public link consumer: the +// FilterResourceInfo rewrites a resource info for a public link consumer: the // path becomes share-root relative (the structure above must not leak), the // permissions are cut to the link grant. Consumers that reach link content by // id bypass the publicstorageprovider and apply it themselves. -func ReduceResourceInfo(info, shareRoot *provider.ResourceInfo, grant *provider.ResourcePermissions) { +func FilterResourceInfo(info, shareRoot *provider.ResourceInfo, grant *provider.ResourcePermissions) { if info == nil { return } @@ -43,12 +43,12 @@ func ReduceResourceInfo(info, shareRoot *provider.ResourceInfo, grant *provider. info.Path = path.Join("/", sharePath) if info.PermissionSet != nil { - ReducePermissions(info.PermissionSet, grant) + FilterPermissions(info.PermissionSet, grant) } } -// ReducePermissions reduces l to what r also grants. A nil r clears l. -func ReducePermissions(l, r *provider.ResourcePermissions) { +// FilterPermissions reduces l to what r also grants. A nil r clears l. +func FilterPermissions(l, r *provider.ResourcePermissions) { l.AddGrant = l.AddGrant && r.GetAddGrant() l.CreateContainer = l.CreateContainer && r.GetCreateContainer() l.Delete = l.Delete && r.GetDelete() From f6408584276dd5e5f6b66bb55735c003f391e227 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Sun, 6 Sep 2026 23:46:01 +0200 Subject: [PATCH 12/26] test(acceptance): pin the item anchored colon path in a public link --- tests/acceptance/bootstrap/GraphContext.php | 38 +++++++++++++++++++ .../publicLinkDriveItemListing.feature | 21 ++++++++++ 2 files changed, 59 insertions(+) diff --git a/tests/acceptance/bootstrap/GraphContext.php b/tests/acceptance/bootstrap/GraphContext.php index ba9ebe5a60..938b36afac 100644 --- a/tests/acceptance/bootstrap/GraphContext.php +++ b/tests/acceptance/bootstrap/GraphContext.php @@ -3587,6 +3587,44 @@ public function thePublicGetsTheDriveItemOfPathSelectingTheAllowedActions( $this->publicSendsGraphDriveRequest("/root:/$encoded?$select", $password); } + /** + * Item anchored colon path: the anchor id is resolved through the public + * children listing, so the step stays within the public API. + * + * @param string $path + * @param string $child + * @param string|null $password + * + * @return void + */ + #[When('the public gets the drive item :path below the child :child of the last created public link with password :password using the Graph API')] + public function thePublicGetsTheDriveItemBelowTheChildOfTheLastCreatedPublicLink( + string $path, + string $child, + ?string $password = null + ): void { + $token = $this->featureContext->shareNgGetLastCreatedLinkShareToken(); + $rootId = $this->publicLinkDriveId($token); + $url = $this->featureContext->getBaseUrl() + . "/graph/v1.0/drives/$rootId/items/$rootId/children?public-token=$token"; + $response = HttpRequestHelper::get( + $url, + $this->featureContext->getStepLineRef(), + $password === null ? null : "public", + $this->featureContext->getActualPassword($password) + ); + $children = \json_decode($response->getBody()->getContents(), true)["value"] ?? []; + $childId = null; + foreach ($children as $entry) { + if ($entry["name"] === $child) { + $childId = $entry["id"]; + } + } + Assert::assertNotNull($childId, "child '$child' not found in the public link listing"); + $encoded = $this->encodeColonPathSegment($path); + $this->publicSendsGraphDriveRequest("/items/$childId:/$encoded", $password); + } + /** * The security probe: an id of a resource that is NOT inside the public * link must not be readable through the link's token. diff --git a/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature b/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature index fcf10644ef..5164a188af 100644 --- a/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature +++ b/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature @@ -102,6 +102,27 @@ Feature: listing the content of a public link via the Graph API } """ + Scenario: the public gets a file through an item anchored colon path + When the public gets the drive item "b.txt" below the child "sub" of the last created public link with password "%public%" using the Graph API + Then the HTTP status code should be "200" + And the JSON data of the response should match + """ + { + "type": "object", + "required": ["name", "file", "parentReference"], + "properties": { + "name": { "const": "b.txt" }, + "parentReference": { + "type": "object", + "required": ["path"], + "properties": { + "path": { "const": "/sub" } + } + } + } + } + """ + Scenario: the public must not see the owner's paths above the share Given user "Alice" has created folder "deep" And user "Alice" has created folder "deep/shared" From a9822a7e24d5b7c2e825a47a129f8c732b4728f6 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Sun, 6 Sep 2026 23:50:16 +0200 Subject: [PATCH 13/26] style: satisfy the gherkin and php linters --- tests/acceptance/bootstrap/GraphContext.php | 1 + .../features/apiGraph/publicLinkDriveItemListing.feature | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/tests/acceptance/bootstrap/GraphContext.php b/tests/acceptance/bootstrap/GraphContext.php index 938b36afac..6717311481 100644 --- a/tests/acceptance/bootstrap/GraphContext.php +++ b/tests/acceptance/bootstrap/GraphContext.php @@ -3631,6 +3631,7 @@ public function thePublicGetsTheDriveItemBelowTheChildOfTheLastCreatedPublicLink * * @param string $path * @param string $user + * @param string|null $password * * @return void */ diff --git a/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature b/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature index 5164a188af..82c191ccea 100644 --- a/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature +++ b/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature @@ -16,6 +16,7 @@ Feature: listing the content of a public link via the Graph API | permissionsRole | view | | password | %public% | + Scenario: the public lists the children of a public link When the public lists the children of the last created public link with password "%public%" using the Graph API Then the HTTP status code should be "200" @@ -54,6 +55,7 @@ Feature: listing the content of a public link via the Graph API } """ + Scenario: the public lists a subfolder of a public link by path When the public lists the children of path "sub" of the last created public link with password "%public%" using the Graph API Then the HTTP status code should be "200" @@ -79,6 +81,7 @@ Feature: listing the content of a public link via the Graph API } """ + Scenario: the public expands the children of the public link root When the public gets the root of the last created public link expanding its children with password "%public%" using the Graph API Then the HTTP status code should be "200" @@ -102,6 +105,7 @@ Feature: listing the content of a public link via the Graph API } """ + Scenario: the public gets a file through an item anchored colon path When the public gets the drive item "b.txt" below the child "sub" of the last created public link with password "%public%" using the Graph API Then the HTTP status code should be "200" @@ -123,6 +127,7 @@ Feature: listing the content of a public link via the Graph API } """ + Scenario: the public must not see the owner's paths above the share Given user "Alice" has created folder "deep" And user "Alice" has created folder "deep/shared" @@ -163,6 +168,7 @@ Feature: listing the content of a public link via the Graph API } """ + Scenario: advertised permissions inside a public link stay within the link role When the public gets the drive item of path "sub" of the last created public link selecting the allowed actions with password "%public%" using the Graph API Then the HTTP status code should be "200" @@ -186,14 +192,17 @@ Feature: listing the content of a public link via the Graph API } """ + Scenario: listing a password protected public link without the password fails When the public lists the children of the last created public link using the Graph API Then the HTTP status code should be "401" + Scenario: listing a password protected public link with a wrong password fails When the public lists the children of the last created public link with password "wrong" using the Graph API Then the HTTP status code should be "401" + Scenario: a resource outside the public link is not readable through its token When the public tries to get the resource "private.txt" of user "Alice" through the last created public link with password "%public%" using the Graph API Then the HTTP status code should be "404" From 8733428aedc0db57e28371c7b9decd662415bcfb Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Sun, 6 Sep 2026 23:52:27 +0200 Subject: [PATCH 14/26] fix(graph): keep CreateUploadSession strict below the public drive The relaxed item addressing is a read feature; writing through it is untested and not part of this surface. --- services/graph/pkg/service/v0/driveitems.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 4c58be5ffd..6453c79fb5 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -162,7 +162,9 @@ func (g Graph) CreateUploadSession(w http.ResponseWriter, r *http.Request) { errorcode.RenderError(w, r, err) return } - if !driveItemInDrive(&driveID, &driveItemID) { + // strict on purpose: uploading through the public drive addressing is not + // part of the listing surface and stays untested for now + if driveID.GetStorageId() != driveItemID.GetStorageId() || driveID.GetSpaceId() != driveItemID.GetSpaceId() { errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, "Item does not exist") return } From 84f9621659e51bceaff29b2c03dba054aac020d8 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Sun, 6 Sep 2026 23:58:06 +0200 Subject: [PATCH 15/26] Revert "fix(graph): keep CreateUploadSession strict below the public drive" This reverts commit 1f51d786d80480df68275dc5fcaf8141556e4c07. --- services/graph/pkg/service/v0/driveitems.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 6453c79fb5..4c58be5ffd 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -162,9 +162,7 @@ func (g Graph) CreateUploadSession(w http.ResponseWriter, r *http.Request) { errorcode.RenderError(w, r, err) return } - // strict on purpose: uploading through the public drive addressing is not - // part of the listing surface and stays untested for now - if driveID.GetStorageId() != driveItemID.GetStorageId() || driveID.GetSpaceId() != driveItemID.GetSpaceId() { + if !driveItemInDrive(&driveID, &driveItemID) { errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, "Item does not exist") return } From 5b260877dc869cd68dfd8991f0a11731abad82ad Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 00:02:47 +0200 Subject: [PATCH 16/26] test(acceptance): pin the upload session authorization on public links An editable link grants an upload session, a view only link answers 404. The returned endpoint is not reachable from outside: CreateUploadSession hands out the internal data server url, for authenticated callers just the same, so the byte transfer stays with the graph upload work. --- tests/acceptance/bootstrap/GraphContext.php | 27 ++++++++++++++++++ .../publicLinkDriveItemListing.feature | 28 +++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/tests/acceptance/bootstrap/GraphContext.php b/tests/acceptance/bootstrap/GraphContext.php index 6717311481..c99371488c 100644 --- a/tests/acceptance/bootstrap/GraphContext.php +++ b/tests/acceptance/bootstrap/GraphContext.php @@ -3625,6 +3625,33 @@ public function thePublicGetsTheDriveItemBelowTheChildOfTheLastCreatedPublicLink $this->publicSendsGraphDriveRequest("/items/$childId:/$encoded", $password); } + /** + * @param string $name + * @param string|null $password + * + * @return void + */ + #[When('the public creates an upload session for :name in the last created public link using the Graph API')] + #[When('the public creates an upload session for :name in the last created public link with password :password using the Graph API')] + public function thePublicCreatesAnUploadSessionForInTheLastCreatedPublicLink( + string $name, + ?string $password = null + ): void { + $token = $this->featureContext->shareNgGetLastCreatedLinkShareToken(); + $rootId = $this->publicLinkDriveId($token); + $url = $this->featureContext->getBaseUrl() + . "/graph/v1.0/drives/$rootId/items/$rootId/createUploadSession?public-token=$token"; + $response = HttpRequestHelper::post( + $url, + $this->featureContext->getStepLineRef(), + $password === null ? null : "public", + $this->featureContext->getActualPassword($password), + ["Content-Type" => "application/json"], + \json_encode(["item" => ["name" => $name, "fileSize" => 6]]) + ); + $this->featureContext->setResponse($response); + } + /** * The security probe: an id of a resource that is NOT inside the public * link must not be readable through the link's token. diff --git a/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature b/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature index 82c191ccea..7c451ff597 100644 --- a/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature +++ b/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature @@ -203,6 +203,34 @@ Feature: listing the content of a public link via the Graph API Then the HTTP status code should be "401" + Scenario: an editable public link grants an upload session + Given user "Alice" has created the following resource link share: + | resource | publicfolder | + | space | Personal | + | permissionsRole | edit | + | password | %public% | + When the public creates an upload session for "up.txt" in the last created public link with password "%public%" using the Graph API + Then the HTTP status code should be "200" + And the JSON data of the response should match + """ + { + "type": "object", + "required": ["UploadURL"], + "properties": { + "UploadURL": { + "type": "string", + "pattern": "/data/" + } + } + } + """ + + + Scenario: a view only public link does not grant an upload session + When the public creates an upload session for "up.txt" in the last created public link with password "%public%" using the Graph API + Then the HTTP status code should be "404" + + Scenario: a resource outside the public link is not readable through its token When the public tries to get the resource "private.txt" of user "Alice" through the last created public link with password "%public%" using the Graph API Then the HTTP status code should be "404" From bcfccc4dd292d562eb135f8a8a5a4da62d524382 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 00:05:58 +0200 Subject: [PATCH 17/26] chore: drop the local compose override again --- tests/acceptance/docker/src/publink-local.yml | 4 ---- 1 file changed, 4 deletions(-) delete mode 100644 tests/acceptance/docker/src/publink-local.yml diff --git a/tests/acceptance/docker/src/publink-local.yml b/tests/acceptance/docker/src/publink-local.yml deleted file mode 100644 index d380d09b3c..0000000000 --- a/tests/acceptance/docker/src/publink-local.yml +++ /dev/null @@ -1,4 +0,0 @@ -services: - opencloud-server: - ports: !override - - "9402:9200" From 56bee0a339a47352d95eb1ba42646f7b279160b4 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 00:07:10 +0200 Subject: [PATCH 18/26] docs: shorter comments --- services/graph/pkg/middleware/auth.go | 3 +-- services/graph/pkg/service/v0/driveitems.go | 10 ++++------ services/proxy/pkg/middleware/public_share_auth.go | 4 ++-- .../opencloud-eu/reva/v2/pkg/auth/scope/publicshare.go | 4 ++-- .../opencloud-eu/reva/v2/pkg/publicshare/filter.go | 4 +--- 5 files changed, 10 insertions(+), 15 deletions(-) diff --git a/services/graph/pkg/middleware/auth.go b/services/graph/pkg/middleware/auth.go index c8a73d150e..193c94e517 100644 --- a/services/graph/pkg/middleware/auth.go +++ b/services/graph/pkg/middleware/auth.go @@ -67,8 +67,7 @@ func Auth(opts ...account.Option) func(http.Handler) http.Handler { errorcode.InvalidAuthenticationToken.Render(w, r, http.StatusUnauthorized, "invalid token") return } - // scope handlers judge CS3 request types and URL paths, never an - // *http.Request; pass the path like reva's own http interceptor + // scope handlers judge CS3 requests and url paths, not *http.Request if ok, err := scope.VerifyScope(ctx, tokenScope, r.URL.Path); err != nil || !ok { opt.Logger.Error().Str(log.RequestIDString, r.Header.Get("X-Request-ID")).Err(err).Msg("verifying scope failed") errorcode.InvalidAuthenticationToken.Render(w, r, http.StatusUnauthorized, "verifying scope failed") diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 4c58be5ffd..df9e82ca52 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -62,8 +62,7 @@ func odataListContains(r *http.Request, parameter, value string) bool { } // driveItemInDrive reports whether an item id may be addressed below a drive. -// Items below the public share drive keep their real ids, so any id is -// acceptable there; the token scope enforces access, not this routing check. +// The public share drive accepts any id; the token scope enforces access. func driveItemInDrive(driveID, driveItemID *storageprovider.ResourceId) bool { if driveID.GetStorageId() == utils.PublicStorageProviderID && driveID.GetSpaceId() == utils.PublicStorageSpaceID { return true @@ -80,8 +79,7 @@ func publicDriveRequest(r *http.Request) bool { } // sanitizePublicDriveInfos applies the publicstorageprovider's reduction to -// infos that bypassed it (navigation by id): paths relative to the share root, -// permissions cut to the link grant. +// infos that bypassed it (navigation by id). func (g Graph) sanitizePublicDriveInfos(ctx context.Context, r *http.Request, infos ...*storageprovider.ResourceInfo) error { shareRoot, grant, err := g.publicLinkOfRequest(ctx, r) if err != nil { @@ -95,8 +93,8 @@ func (g Graph) sanitizePublicDriveInfos(ctx context.Context, r *http.Request, in return nil } -// publicLinkOfRequest resolves the request's public link into the share root -// info and the granted permissions; the token is the public drive's opaque id. +// publicLinkOfRequest resolves the link the request runs in; the token is the +// public drive's opaque id. func (g Graph) publicLinkOfRequest(ctx context.Context, r *http.Request) (*storageprovider.ResourceInfo, *storageprovider.ResourcePermissions, error) { driveID, err := parseIDParam(r, "driveID") if err != nil { diff --git a/services/proxy/pkg/middleware/public_share_auth.go b/services/proxy/pkg/middleware/public_share_auth.go index 5b1e7c0739..3bd7d7965b 100644 --- a/services/proxy/pkg/middleware/public_share_auth.go +++ b/services/proxy/pkg/middleware/public_share_auth.go @@ -58,8 +58,8 @@ func isPublicWithShareToken(r *http.Request) bool { hasShareToken(r) } -// Graph requests carrying a share token run in the public share context, like -// public-files; without a token they stay with the other authenticators. +// A graph request carrying a share token runs in the public share context, +// like public-files. func isPublicShareGraphRequest(r *http.Request) bool { return strings.HasPrefix(r.URL.Path, "/graph/") && hasShareToken(r) } diff --git a/vendor/github.com/opencloud-eu/reva/v2/pkg/auth/scope/publicshare.go b/vendor/github.com/opencloud-eu/reva/v2/pkg/auth/scope/publicshare.go index ed210f89dc..a0b79a4eb7 100644 --- a/vendor/github.com/opencloud-eu/reva/v2/pkg/auth/scope/publicshare.go +++ b/vendor/github.com/opencloud-eu/reva/v2/pkg/auth/scope/publicshare.go @@ -151,8 +151,8 @@ func publicshareScope(ctx context.Context, scope *authpb.Scope, resource interfa return false, errtypes.InternalError(msg) } -// checkGraphDrivesPath opens the graph drive item routes, like /archiver in -// checkResourcePath; what a token may read is enforced per CS3 request. +// checkGraphDrivesPath opens the graph drive routes; what a token may read is +// enforced per CS3 request. func checkGraphDrivesPath(path string) bool { return strings.HasPrefix(path, "/graph/v1.0/drives/") || strings.HasPrefix(path, "/graph/v1beta1/drives/") } diff --git a/vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/filter.go b/vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/filter.go index ff2b3afee4..e46c0bbc89 100644 --- a/vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/filter.go +++ b/vendor/github.com/opencloud-eu/reva/v2/pkg/publicshare/filter.go @@ -26,9 +26,7 @@ import ( ) // FilterResourceInfo rewrites a resource info for a public link consumer: the -// path becomes share-root relative (the structure above must not leak), the -// permissions are cut to the link grant. Consumers that reach link content by -// id bypass the publicstorageprovider and apply it themselves. +// path becomes share-root relative, the permissions are cut to the grant. func FilterResourceInfo(info, shareRoot *provider.ResourceInfo, grant *provider.ResourcePermissions) { if info == nil { return From f70d2c45b5af7e594156383a08e48f5c262ca013 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 00:08:46 +0200 Subject: [PATCH 19/26] docs: say why the prefix check does not apply below the public drive --- services/graph/pkg/service/v0/driveitems.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index df9e82ca52..2a140e55fa 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -62,7 +62,8 @@ func odataListContains(r *http.Request, parameter, value string) bool { } // driveItemInDrive reports whether an item id may be addressed below a drive. -// The public share drive accepts any id; the token scope enforces access. +// Below the public share drive items keep their real storage ids, so the +// prefix can never match; the token scope checks containment instead. func driveItemInDrive(driveID, driveItemID *storageprovider.ResourceId) bool { if driveID.GetStorageId() == utils.PublicStorageProviderID && driveID.GetSpaceId() == utils.PublicStorageSpaceID { return true From a0e1d7cbabb7c3edf9b5fc4726051637ade2c21b Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 00:52:35 +0200 Subject: [PATCH 20/26] fix(scope): pin the public share scope to the link's own graph drive The prefix opened the whole /drives subtree, and the scope allows ListStorageSpacesRequest unconditionally: any link token could enumerate the owner's spaces via GET /drives/, read any space by id, and address in-share items through their real drive id, skipping the response reduction. The path is cleaned and must now name the token's public drive; found by review. --- .../reva/v2/pkg/auth/scope/publicshare.go | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/vendor/github.com/opencloud-eu/reva/v2/pkg/auth/scope/publicshare.go b/vendor/github.com/opencloud-eu/reva/v2/pkg/auth/scope/publicshare.go index a0b79a4eb7..da9e272e42 100644 --- a/vendor/github.com/opencloud-eu/reva/v2/pkg/auth/scope/publicshare.go +++ b/vendor/github.com/opencloud-eu/reva/v2/pkg/auth/scope/publicshare.go @@ -20,6 +20,7 @@ package scope import ( "context" + "path" "strings" appprovider "github.com/cs3org/go-cs3apis/cs3/app/provider/v1beta1" @@ -143,7 +144,7 @@ func publicshareScope(ctx context.Context, scope *authpb.Scope, resource interfa // public links must not leak info about collaborative shares return false, nil case string: - return checkResourcePath(v) || checkGraphDrivesPath(v), nil + return checkResourcePath(v) || checkGraphDrivesPath(v, share.Token), nil } msg := "public resource type assertion failed" @@ -151,10 +152,21 @@ func publicshareScope(ctx context.Context, scope *authpb.Scope, resource interfa return false, errtypes.InternalError(msg) } -// checkGraphDrivesPath opens the graph drive routes; what a token may read is -// enforced per CS3 request. -func checkGraphDrivesPath(path string) bool { - return strings.HasPrefix(path, "/graph/v1.0/drives/") || strings.HasPrefix(path, "/graph/v1beta1/drives/") +// checkGraphDrivesPath opens the graph drive routes of exactly the link's own +// public drive; every other drive stays closed, notably the drives collection +// and real space ids. Per CS3 request checks enforce what may be read below it. +func checkGraphDrivesPath(p, token string) bool { + if token == "" { + return false + } + p = path.Clean(p) + drive := PublicStorageProviderID + "$" + PublicStorageProviderID + "!" + token + for _, prefix := range []string{"/graph/v1.0/drives/", "/graph/v1beta1/drives/"} { + if p == prefix+drive || strings.HasPrefix(p, prefix+drive+"/") { + return true + } + } + return false } func checkStorageRef(ctx context.Context, s *link.PublicShare, r *provider.Reference) bool { From 6d09a83adca18a4df1eb9b79e6699a15bfacd4f4 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 00:52:35 +0200 Subject: [PATCH 21/26] fix(graph): keep owner data out of public link responses The owner's favorite flag travelled as @libre.graph.meFollowing, the share root exposed its out-of-share parent id, and $select=@libre.graph.shareTypes disclosed collaborative grants; found by review. --- services/graph/pkg/service/v0/driveitems.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 2a140e55fa..fdf74964f9 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -87,8 +87,15 @@ func (g Graph) sanitizePublicDriveInfos(ctx context.Context, r *http.Request, in return err } for _, info := range infos { - if info != nil { - publicshare.FilterResourceInfo(info, shareRoot, grant) + if info == nil { + continue + } + publicshare.FilterResourceInfo(info, shareRoot, grant) + // the favorite flag is the owner's, not the visitor's + delete(info.GetArbitraryMetadata().GetMetadata(), _favoriteMetadataKey) + // the share root's parent lies outside the share + if utils.ResourceIDEqual(info.GetId(), shareRoot.GetId()) { + info.ParentId = nil } } return nil @@ -413,7 +420,7 @@ func (g Graph) GetDriveItem(w http.ResponseWriter, r *http.Request) { driveItem.Children = children } - if driveItemPropertySelected(r, _selectShareTypes) { + if driveItemPropertySelected(r, _selectShareTypes) && !publicDriveRequest(r) { infos := []*storageprovider.ResourceInfo{res.GetInfo()} driveItem.LibreGraphShareTypes = shareTypesOf(res.GetInfo(), g.listLinkShares(ctx, infos)) } @@ -473,7 +480,7 @@ func (g Graph) listDriveItemChildren(w http.ResponseWriter, r *http.Request, dri childrenRequest := &storageprovider.ListContainerRequest{ Ref: &storageprovider.Reference{ResourceId: driveItemID}, } - if driveItemPropertySelected(r, _selectShareTypes) { + if driveItemPropertySelected(r, _selectShareTypes) && !publicDriveRequest(r) { childrenRequest.FieldMask = shareTypesFieldMask } @@ -511,7 +518,8 @@ func (g Graph) listDriveItemChildren(w http.ResponseWriter, r *http.Request, dri return nil, false } - if driveItemPropertySelected(r, _selectShareTypes) { + // collaborative grants are not for public link visitors + if driveItemPropertySelected(r, _selectShareTypes) && !publicDriveRequest(r) { g.addShareTypes(r.Context(), files, res.GetInfos()) } From 6727cec5268e3d18bc8c612663b06845c7008996 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 00:52:35 +0200 Subject: [PATCH 22/26] test(acceptance): pin the closed surface around a public link Drives collection, the owner's personal drive, real drive id addressing, a foreign link's drive and share type disclosure all answer 401 or omit the data. --- tests/acceptance/bootstrap/GraphContext.php | 99 +++++++++++++++++++ .../publicLinkDriveItemListing.feature | 51 ++++++++++ 2 files changed, 150 insertions(+) diff --git a/tests/acceptance/bootstrap/GraphContext.php b/tests/acceptance/bootstrap/GraphContext.php index c99371488c..d787a5d346 100644 --- a/tests/acceptance/bootstrap/GraphContext.php +++ b/tests/acceptance/bootstrap/GraphContext.php @@ -3587,6 +3587,105 @@ public function thePublicGetsTheDriveItemOfPathSelectingTheAllowedActions( $this->publicSendsGraphDriveRequest("/root:/$encoded?$select", $password); } + /** + * @param string $urlSuffix raw suffix below /graph/v1.0, may contain :spaceOfUser + * @param string|null $password + * + * @return void + */ + private function publicSendsRawGraphRequest(string $urlSuffix, ?string $password = null): void { + $token = $this->featureContext->shareNgGetLastCreatedLinkShareToken(); + $url = $this->featureContext->getBaseUrl() . "/graph/v1.0" . $urlSuffix + . (\str_contains($urlSuffix, "?") ? "&" : "?") . "public-token=$token"; + $response = HttpRequestHelper::get( + $url, + $this->featureContext->getStepLineRef(), + $password === null ? null : "public", + $this->featureContext->getActualPassword($password) + ); + $this->featureContext->setResponse($response); + } + + /** + * @param string|null $password + * + * @return void + */ + #[When('the public lists the children of the last created public link selecting the share types with password :password using the Graph API')] + public function thePublicListsTheChildrenSelectingTheShareTypes(?string $password = null): void { + $token = $this->featureContext->shareNgGetLastCreatedLinkShareToken(); + $rootId = $this->publicLinkDriveId($token); + $select = "%24select=%40libre.graph.shareTypes"; + $this->publicSendsGraphDriveRequest("/items/$rootId/children?$select", $password); + } + + /** + * @param string|null $password + * + * @return void + */ + #[When('the public tries to list the drives using the token of the last created public link with password :password using the Graph API')] + public function thePublicTriesToListTheDrives(?string $password = null): void { + $this->publicSendsRawGraphRequest("/drives/", $password); + } + + /** + * @param string $user + * @param string|null $password + * + * @return void + */ + #[When('the public tries to get the personal drive of user :user through the last created public link with password :password using the Graph API')] + public function thePublicTriesToGetThePersonalDriveOfUser(string $user, ?string $password = null): void { + $user = $this->featureContext->getActualUsername($user); + $driveId = $this->spacesContext->getSpaceIdByName($user, "Personal"); + $this->publicSendsRawGraphRequest("/drives/$driveId", $password); + } + + /** + * Addressing an in-share item through its REAL drive id must be rejected: + * only the token's public drive is authorized. + * + * @param string $child + * @param string|null $password + * + * @return void + */ + #[When('the public tries to get the child :child of the last created public link through its real drive id with password :password using the Graph API')] + public function thePublicTriesToGetTheChildThroughItsRealDriveId(string $child, ?string $password = null): void { + $token = $this->featureContext->shareNgGetLastCreatedLinkShareToken(); + $rootId = $this->publicLinkDriveId($token); + $url = $this->featureContext->getBaseUrl() + . "/graph/v1.0/drives/$rootId/items/$rootId/children?public-token=$token"; + $listing = HttpRequestHelper::get( + $url, + $this->featureContext->getStepLineRef(), + $password === null ? null : "public", + $this->featureContext->getActualPassword($password) + ); + $children = \json_decode($listing->getBody()->getContents(), true)["value"] ?? []; + $childId = null; + foreach ($children as $entry) { + if ($entry["name"] === $child) { + $childId = $entry["id"]; + } + } + Assert::assertNotNull($childId, "child '$child' not found in the public link listing"); + $realDriveId = \explode("!", $childId)[0]; + $this->publicSendsRawGraphRequest("/drives/$realDriveId/items/$childId", $password); + } + + /** + * @param string|null $password + * + * @return void + */ + #[When('the public tries to list the children of a foreign public link drive using the last created token with password :password using the Graph API')] + public function thePublicTriesToListAForeignPublicLinkDrive(?string $password = null): void { + $foreign = $this->publicLinkDriveId("notthetokenofthislink"); + $this->publicSendsRawGraphRequest("/drives/$foreign/items/$foreign/children", $password); + } + /** * Item anchored colon path: the anchor id is resolved through the public * children listing, so the step stays within the public API. diff --git a/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature b/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature index 7c451ff597..dab0525d3a 100644 --- a/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature +++ b/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature @@ -234,3 +234,54 @@ Feature: listing the content of a public link via the Graph API Scenario: a resource outside the public link is not readable through its token When the public tries to get the resource "private.txt" of user "Alice" through the last created public link with password "%public%" using the Graph API Then the HTTP status code should be "404" + + + Scenario: a public token cannot list drives + When the public tries to list the drives using the token of the last created public link with password "%public%" using the Graph API + Then the HTTP status code should be "401" + + + Scenario: a public token cannot read the owner's personal drive + When the public tries to get the personal drive of user "Alice" through the last created public link with password "%public%" using the Graph API + Then the HTTP status code should be "401" + + + Scenario: an in-share item is not addressable through its real drive id + When the public tries to get the child "sub" of the last created public link through its real drive id with password "%public%" using the Graph API + Then the HTTP status code should be "401" + + + Scenario: a token does not open another link's drive + When the public tries to list the children of a foreign public link drive using the last created token with password "%public%" using the Graph API + Then the HTTP status code should be "401" + + + Scenario: collaborative share types are not disclosed to the public + Given user "Brian" has been created with default attributes + And user "Alice" has sent the following resource share invitation: + | resource | publicfolder/sub | + | space | Personal | + | sharee | Brian | + | shareType | user | + | permissionsRole | Viewer | + When the public lists the children of the last created public link selecting the share types with password "%public%" using the Graph API + Then the HTTP status code should be "200" + And the JSON data of the response should match + """ + { + "type": "object", + "required": ["value"], + "properties": { + "value": { + "type": "array", + "minItems": 2, + "maxItems": 2, + "uniqueItems": true, + "items": { + "type": "object", + "not": { "required": ["@libre.graph.shareTypes"] } + } + } + } + } + """ From d9613a068fe45eae5f11d265b372284e1526add0 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 01:12:22 +0200 Subject: [PATCH 23/26] fix(proxy): log a token hint, not the token --- services/proxy/pkg/middleware/public_share_auth.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/services/proxy/pkg/middleware/public_share_auth.go b/services/proxy/pkg/middleware/public_share_auth.go index 3bd7d7965b..f2be7d6706 100644 --- a/services/proxy/pkg/middleware/public_share_auth.go +++ b/services/proxy/pkg/middleware/public_share_auth.go @@ -68,6 +68,14 @@ func hasShareToken(r *http.Request) bool { return r.URL.Query().Get(headerShareToken) != "" || r.Header.Get(headerShareToken) != "" } +// shareTokenHint identifies a token in logs without spelling it out. +func shareTokenHint(token string) string { + if len(token) <= 4 { + return token + } + return token[:4] + "..." +} + // Authenticate implements the authenticator interface to authenticate requests via public share auth. func (a PublicShareAuthenticator) Authenticate(r *http.Request) (*http.Request, bool) { if !isPublicPath(r.URL.Path) && !isPublicShareArchive(r) && !isPublicShareAppOpen(r) && !isPublicShareGraphRequest(r) { @@ -109,7 +117,7 @@ func (a PublicShareAuthenticator) Authenticate(r *http.Request) (*http.Request, a.Logger.Error(). Err(err). Str("authenticator", "public_share"). - Str("public_share_token", shareToken). + Str("public_share_token", shareTokenHint(shareToken)). Str("path", r.URL.Path). Msg("could not select next gateway client") return nil, false @@ -125,7 +133,7 @@ func (a PublicShareAuthenticator) Authenticate(r *http.Request) (*http.Request, a.Logger.Error(). Err(err). Str("authenticator", "public_share"). - Str("public_share_token", shareToken). + Str("public_share_token", shareTokenHint(shareToken)). Str("path", r.URL.Path). Msg("failed to authenticate request") return nil, false From 1e6d2ec168790874380a5facc61f15b9ef0dc006 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 01:12:22 +0200 Subject: [PATCH 24/26] test(acceptance): pin that writes through the public link surface are rejected createLink, delete, rename and listing permissions on the beta routes all fail even for an edit link, and the file survives; measured, no 500s. --- tests/acceptance/bootstrap/GraphContext.php | 72 +++++++++++++++++++ .../publicLinkDriveItemListing.feature | 18 +++++ 2 files changed, 90 insertions(+) diff --git a/tests/acceptance/bootstrap/GraphContext.php b/tests/acceptance/bootstrap/GraphContext.php index d787a5d346..64569d397e 100644 --- a/tests/acceptance/bootstrap/GraphContext.php +++ b/tests/acceptance/bootstrap/GraphContext.php @@ -3686,6 +3686,78 @@ public function thePublicTriesToListAForeignPublicLinkDrive(?string $password = $this->publicSendsRawGraphRequest("/drives/$foreign/items/$foreign/children", $password); } + /** + * Mutation probes: the public link surface is read only, every write on the + * beta routes has to be rejected. + * + * @param string $action + * @param string $child + * @param string|null $password + * + * @return void + */ + #[When('/^the public tries to (create a link for|delete|rename|list the permissions of) the child "([^"]*)" of the last created public link with password "([^"]*)" using the Graph API$/')] + public function thePublicTriesToMutateTheChildOfTheLastCreatedPublicLink( + string $action, + string $child, + ?string $password = null + ): void { + $token = $this->featureContext->shareNgGetLastCreatedLinkShareToken(); + $rootId = $this->publicLinkDriveId($token); + $listing = HttpRequestHelper::get( + $this->featureContext->getBaseUrl() + . "/graph/v1.0/drives/$rootId/items/$rootId/children?public-token=$token", + $this->featureContext->getStepLineRef(), + "public", + $this->featureContext->getActualPassword($password) + ); + $children = \json_decode($listing->getBody()->getContents(), true)["value"] ?? []; + $childId = null; + foreach ($children as $entry) { + if ($entry["name"] === $child) { + $childId = $entry["id"]; + } + } + Assert::assertNotNull($childId, "child '$child' not found in the public link listing"); + + $base = "/graph/v1beta1/drives/$rootId/items/$childId"; + switch ($action) { + case "create a link for": + $method = "POST"; + $url = "$base/createLink"; + $body = \json_encode(["type" => "view", "password" => "Sup3rS3cret!x"]); + break; + case "delete": + $method = "DELETE"; + $url = $base; + $body = null; + break; + case "rename": + $method = "PATCH"; + $url = $base; + $body = \json_encode(["name" => "renamed.txt"]); + break; + case "list the permissions of": + $method = "GET"; + $url = "$base/permissions"; + $body = null; + break; + default: + throw new \Exception("unknown mutation action '$action'"); + } + $response = HttpRequestHelper::sendRequest( + $this->featureContext->getBaseUrl() . $url + . (\str_contains($url, "?") ? "&" : "?") . "public-token=$token", + $this->featureContext->getStepLineRef(), + $method, + "public", + $this->featureContext->getActualPassword($password), + ["Content-Type" => "application/json"], + $body + ); + $this->featureContext->setResponse($response); + } + /** * Item anchored colon path: the anchor id is resolved through the public * children listing, so the step stays within the public API. diff --git a/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature b/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature index dab0525d3a..e77ed6635d 100644 --- a/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature +++ b/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature @@ -285,3 +285,21 @@ Feature: listing the content of a public link via the Graph API } } """ + + + Scenario Outline: writes through the public link surface are rejected + Given user "Alice" has created the following resource link share: + | resource | publicfolder | + | space | Personal | + | permissionsRole | edit | + | password | %public% | + When the public tries to the child "a.txt" of the last created public link with password "%public%" using the Graph API + Then the HTTP status code should be "" + And as "Alice" file "publicfolder/a.txt" should exist + + Examples: + | action | code | + | create a link for | 404 | + | delete | 400 | + | rename | 400 | + | list the permissions of | 404 | From b392f43662fd1058a1f57e0fb06ec827a1dd3d06 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 10:07:33 +0200 Subject: [PATCH 25/26] feat(graph): tell password-required from wrong-password on public links A public link stat that fails for the password came back as a generic 401 "Access token is empty", so a client could not tell "show the password field" from "the password was wrong". The proxy now marks the two cases (it holds the auth result) and the graph service renders them as distinct odata codes, publicLinkPasswordRequired and publicLinkPasswordInvalid, the way webdav distinguishes ERR_MISSING_BASIC_AUTH from ERR_INVALID_CREDENTIALS. The distinction rides in the body, never a WWW-Authenticate: Basic header, which would pop the browser's native auth dialog instead of the app's password field. The shared header/token contract lives in pkg/middleware. --- pkg/middleware/publiclink.go | 27 ++++++++++++++ services/graph/pkg/errorcode/errorcode.go | 8 +++++ services/graph/pkg/middleware/auth.go | 17 +++++++++ .../proxy/pkg/middleware/public_share_auth.go | 21 ++++++++++- .../publicLinkDriveItemListing.feature | 36 +++++++++++++++++-- 5 files changed, 106 insertions(+), 3 deletions(-) create mode 100644 pkg/middleware/publiclink.go diff --git a/pkg/middleware/publiclink.go b/pkg/middleware/publiclink.go new file mode 100644 index 0000000000..9ea942198b --- /dev/null +++ b/pkg/middleware/publiclink.go @@ -0,0 +1,27 @@ +package middleware + +import "net/http" + +const ( + // PublicLinkTokenName is the query parameter and header carrying a public + // link token on a request. + PublicLinkTokenName = "public-token" + + // PublicLinkAuthHeader marks the outcome of a failed public link + // authentication so a downstream service can tell "password required" from + // "wrong password" when it renders the 401. The proxy sets it, the graph + // service reads it. + PublicLinkAuthHeader = "X-Public-Link-Auth" + + // PublicLinkPasswordRequired means the link is password protected and no + // password was provided. + PublicLinkPasswordRequired = "password-required" + // PublicLinkInvalidPassword means a password was provided but rejected. + PublicLinkInvalidPassword = "invalid-password" +) + +// HasPublicLinkToken reports whether a public link token rides on the request, +// as a query parameter or header. +func HasPublicLinkToken(r *http.Request) bool { + return r.URL.Query().Get(PublicLinkTokenName) != "" || r.Header.Get(PublicLinkTokenName) != "" +} diff --git a/services/graph/pkg/errorcode/errorcode.go b/services/graph/pkg/errorcode/errorcode.go index 0347d83dbb..79c9bca432 100644 --- a/services/graph/pkg/errorcode/errorcode.go +++ b/services/graph/pkg/errorcode/errorcode.go @@ -76,6 +76,10 @@ const ( PreconditionFailed // ItemIsLocked The item is locked by another process. Try again later. ItemIsLocked + // PublicLinkPasswordRequired the public link is password protected and no password was provided. + PublicLinkPasswordRequired + // PublicLinkPasswordInvalid a password was provided for the public link but it was rejected. + PublicLinkPasswordInvalid ) var errorCodes = [...]string{ @@ -99,6 +103,8 @@ var errorCodes = [...]string{ "unauthenticated", "preconditionFailed", "itemIsLocked", + "publicLinkPasswordRequired", + "publicLinkPasswordInvalid", } // New constructs a new errorcode.Error @@ -151,6 +157,8 @@ func (e Error) Render(w http.ResponseWriter, r *http.Request) { status = http.StatusMethodNotAllowed case ItemIsLocked: status = http.StatusLocked + case PublicLinkPasswordRequired, PublicLinkPasswordInvalid: + status = http.StatusUnauthorized case PreconditionFailed: status = http.StatusPreconditionFailed default: diff --git a/services/graph/pkg/middleware/auth.go b/services/graph/pkg/middleware/auth.go index 193c94e517..e19b79ecbc 100644 --- a/services/graph/pkg/middleware/auth.go +++ b/services/graph/pkg/middleware/auth.go @@ -44,6 +44,23 @@ func Auth(opts ...account.Option) func(http.Handler) http.Handler { ctx := r.Context() t := r.Header.Get(revactx.TokenHeader) if t == "" { + // a public link request that failed the share auth carries a + // hint (set by the proxy) so we can tell the two cases apart; + // only trust it when a share token is actually on the request + if hint := r.Header.Get(opkgm.PublicLinkAuthHeader); hint != "" && opkgm.HasPublicLinkToken(r) { + switch hint { + // distinguish via the body only, never WWW-Authenticate: a + // Basic challenge would pop the browser's native auth dialog + // instead of the app's password field (the proxy strips it + // on public paths for the same reason) + case opkgm.PublicLinkPasswordRequired: + errorcode.PublicLinkPasswordRequired.Render(w, r, http.StatusUnauthorized, "This public link is password protected.") + return + case opkgm.PublicLinkInvalidPassword: + errorcode.PublicLinkPasswordInvalid.Render(w, r, http.StatusUnauthorized, "The password is incorrect.") + return + } + } errorcode.InvalidAuthenticationToken.Render(w, r, http.StatusUnauthorized, "Access token is empty.") /* msgraph error for GET https://graph.microsoft.com/v1.0/me { diff --git a/services/proxy/pkg/middleware/public_share_auth.go b/services/proxy/pkg/middleware/public_share_auth.go index f2be7d6706..cc58a4a113 100644 --- a/services/proxy/pkg/middleware/public_share_auth.go +++ b/services/proxy/pkg/middleware/public_share_auth.go @@ -5,7 +5,9 @@ import ( "strings" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" "github.com/opencloud-eu/opencloud/pkg/log" + ocmw "github.com/opencloud-eu/opencloud/pkg/middleware" revactx "github.com/opencloud-eu/reva/v2/pkg/ctx" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" "go.opentelemetry.io/otel/attribute" @@ -14,7 +16,7 @@ import ( const ( headerRevaAccessToken = revactx.TokenHeader - headerShareToken = "public-token" + headerShareToken = ocmw.PublicLinkTokenName basicAuthPasswordPrefix = "password|" authenticationType = "publicshares" @@ -139,6 +141,23 @@ func (a PublicShareAuthenticator) Authenticate(r *http.Request) (*http.Request, return nil, false } + if authResp.GetStatus().GetCode() != rpc.Code_CODE_OK { + // A graph request cannot render its own 401 from here (no writer), and + // the generic one cannot tell the two password cases apart. Mark the + // outcome and let the graph auth middleware render it. Other surfaces + // (webdav) are handled by their own backend, so they just fail here. + if isPublicShareGraphRequest(r) { + _, password, ok := r.BasicAuth() + if ok && password != "" { + r.Header.Set(ocmw.PublicLinkAuthHeader, ocmw.PublicLinkInvalidPassword) + } else { + r.Header.Set(ocmw.PublicLinkAuthHeader, ocmw.PublicLinkPasswordRequired) + } + return r, true + } + return nil, false + } + r.Header.Add(headerRevaAccessToken, authResp.Token) trace.SpanFromContext(r.Context()).SetAttributes(attribute.String("enduser.id", "public")) diff --git a/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature b/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature index e77ed6635d..4ee12b868d 100644 --- a/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature +++ b/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature @@ -193,14 +193,46 @@ Feature: listing the content of a public link via the Graph API """ - Scenario: listing a password protected public link without the password fails + Scenario: listing a password protected public link without the password reports that a password is required When the public lists the children of the last created public link using the Graph API Then the HTTP status code should be "401" + And the JSON data of the response should match + """ + { + "type": "object", + "required": ["error"], + "properties": { + "error": { + "type": "object", + "required": ["code"], + "properties": { + "code": { "const": "publicLinkPasswordRequired" } + } + } + } + } + """ - Scenario: listing a password protected public link with a wrong password fails + Scenario: listing a password protected public link with a wrong password reports an invalid password When the public lists the children of the last created public link with password "wrong" using the Graph API Then the HTTP status code should be "401" + And the JSON data of the response should match + """ + { + "type": "object", + "required": ["error"], + "properties": { + "error": { + "type": "object", + "required": ["code"], + "properties": { + "code": { "const": "publicLinkPasswordInvalid" } + } + } + } + } + """ Scenario: an editable public link grants an upload session From 359dd2f2674bc310a527afd64cc5aa8175dc3be7 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 10:35:52 +0200 Subject: [PATCH 26/26] feat(graph): expose the sharer as the owner of a public link drive The public share mountpoint carries no space owner, so GET /drives/{pub} came back without one and the drop page had no name to show. The request runs as the share creator (publicshares auth), so the context user is who shared the link; fill drive.Owner from it, id and display name, the same source webdav fills oc:owner-display-name from. Deliberate disclosure to the anonymous visitor, matching webdav and MS Graph's sharedDriveItem.owner. --- services/graph/pkg/service/v0/drives.go | 13 +++++++++ tests/acceptance/bootstrap/GraphContext.php | 22 +++++++++++++++ .../publicLinkDriveItemListing.feature | 28 +++++++++++++++++++ 3 files changed, 63 insertions(+) diff --git a/services/graph/pkg/service/v0/drives.go b/services/graph/pkg/service/v0/drives.go index 954cd0791d..114f59d3e0 100644 --- a/services/graph/pkg/service/v0/drives.go +++ b/services/graph/pkg/service/v0/drives.go @@ -881,6 +881,19 @@ func (g Graph) cs3StorageSpaceToDrive(ctx context.Context, baseURL *url.URL, spa // DisplayName: , TODO read and cache from users provider }, } + } else if space.GetRoot().GetStorageId() == utils.PublicStorageProviderID { + // a public share mountpoint carries no space owner; the request runs as + // the share creator (publicshares auth), so the context user is who + // shared it, the same source webdav fills oc:owner-display-name from. + if u, ok := revactx.ContextGetUser(ctx); ok && u.GetId().GetOpaqueId() != "" { + id := u.GetId().GetOpaqueId() + drive.Owner = &libregraph.IdentitySet{ + User: &libregraph.Identity{ + Id: &id, + DisplayName: u.GetDisplayName(), + }, + } + } } if space.Mtime != nil { lastModified := cs3TimestampToTime(space.Mtime) diff --git a/tests/acceptance/bootstrap/GraphContext.php b/tests/acceptance/bootstrap/GraphContext.php index 64569d397e..ec5cd33e11 100644 --- a/tests/acceptance/bootstrap/GraphContext.php +++ b/tests/acceptance/bootstrap/GraphContext.php @@ -3758,6 +3758,28 @@ public function thePublicTriesToMutateTheChildOfTheLastCreatedPublicLink( $this->featureContext->setResponse($response); } + /** + * The public link drive endpoint rejects the token as a query parameter, + * so it rides in the header here. + * + * @param string|null $password + * + * @return void + */ + #[When('the public gets the drive of the last created public link with password :password using the Graph API')] + public function thePublicGetsTheDriveOfTheLastCreatedPublicLink(?string $password = null): void { + $token = $this->featureContext->shareNgGetLastCreatedLinkShareToken(); + $driveId = $this->publicLinkDriveId($token); + $response = HttpRequestHelper::get( + $this->featureContext->getBaseUrl() . "/graph/v1.0/drives/$driveId", + $this->featureContext->getStepLineRef(), + $password === null ? null : "public", + $this->featureContext->getActualPassword($password), + ["public-token" => $token] + ); + $this->featureContext->setResponse($response); + } + /** * Item anchored colon path: the anchor id is resolved through the public * children listing, so the step stays within the public API. diff --git a/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature b/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature index 4ee12b868d..db605dbd50 100644 --- a/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature +++ b/tests/acceptance/features/apiGraph/publicLinkDriveItemListing.feature @@ -335,3 +335,31 @@ Feature: listing the content of a public link via the Graph API | delete | 400 | | rename | 400 | | list the permissions of | 404 | + + + Scenario: the public sees who shared the link on the drive + When the public gets the drive of the last created public link with password "%public%" using the Graph API + Then the HTTP status code should be "200" + And the JSON data of the response should match + """ + { + "type": "object", + "required": ["driveType", "owner"], + "properties": { + "driveType": { "const": "mountpoint" }, + "owner": { + "type": "object", + "required": ["user"], + "properties": { + "user": { + "type": "object", + "required": ["id", "displayName"], + "properties": { + "displayName": { "const": "Alice Hansen" } + } + } + } + } + } + } + """