feat(graph): public links - #3472
Draft
dschmidt wants to merge 26 commits into
Draft
Conversation
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Security | 2 critical |
🟢 Metrics 27 complexity
Metric Results Complexity 27
🟢 Coverage 22.73% diff coverage · -0.02% coverage variation
Metric Results Coverage variation ✅ -0.02% coverage variation (-1.00%) Diff coverage ✅ 22.73% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (391705b) 88344 20828 23.58% Head commit (359dd2f) 88460 (+116) 20835 (+7) 23.55% (-0.02%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#3472) 132 30 22.73% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
7 tasks
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.
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.
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.
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.
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.
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.
The relaxed item addressing is a read feature; writing through it is untested and not part of this surface.
…drive" This reverts commit 1f51d78.
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.
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.
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.
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.
… 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.
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.
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.
dschmidt
force-pushed
the
feat/graph-public-link-listing
branch
from
September 8, 2026 09:21
fc32ff5 to
359dd2f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
List a public link over the Graph API instead of WebDAV:
{pub}is7993447f-...$7993447f-...!{token}, the public mountpoint space; the client builds it from the token. A link password goes in as basic authpublic:{password}.What the change does:
/graph/requests that carry apublic-token, the same way it already covers the archiver.VerifyScopeis given the url path; before it got an*http.Request, which no scope handler can match, so every public token was rejected.publicshare.FilterResourceInfo) and graph applies it too: paths relative to the share root, permissions capped at the link role.drive.Owneris filled from the context user (id and display name). This is the same value ocdav already returns asoc:owner-display-namefor a public link, so graph and webdav agree; it also matches MS Graph'ssharedDriveItem.owner. A deliberate disclosure of the sharer's name to the anonymous visitor.reva changes sit in vendor/ until we agree on the shape and I open a reva PR: the scope path check, the shared
publicshare.Filter*(which also adds theDenyGrantfield the old private copy dropped), and whether the reduction belongs in a gateway middleware (reva's own FIXME) rather than in each caller.Upload sessions follow the link role (edit 200, view 404). The upload itself does not complete for anyone yet, public or logged in: the session hands back the internal data server url. That is existing behaviour and belongs to the graph upload work.
Tests:
apiGraph/publicLinkDriveItemListing.feature, 21 scenarios. They cover the listing, both colon-path forms, expand, passwords, the sharer showing up as the drive owner, and the boundaries: no drive enumeration, no reading a space by id, no foreign drive, no write, no leak of paths above the share, share types or the owner's favorite flag.