Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
46a7dbf
feat(proxy): authenticate graph requests carrying a public link token
dschmidt Sep 6, 2026
40f7aaa
fix(graph): verify token scopes against the url path
dschmidt Sep 6, 2026
1d65ce7
feat(graph): address drive items below the public share drive
dschmidt Sep 6, 2026
aa03c6c
test(acceptance): list public links via the graph api
dschmidt Sep 6, 2026
d18c1a1
fix(graph): advertise only what the public link grants
dschmidt Sep 6, 2026
b045b2b
fix(vendor): filterPermissions misses DenyGrant
dschmidt Sep 6, 2026
af99934
refactor: the public link reduction lives once, in reva
dschmidt Sep 6, 2026
adb7327
docs: trim comments
dschmidt Sep 6, 2026
6eb235a
chore: drop a local compose override that slipped in
dschmidt Sep 6, 2026
da421b6
refactor(graph): fail when the public link does not resolve
dschmidt Sep 6, 2026
1e29eb9
refactor: keep the provider's filter naming
dschmidt Sep 6, 2026
f640858
test(acceptance): pin the item anchored colon path in a public link
dschmidt Sep 6, 2026
a9822a7
style: satisfy the gherkin and php linters
dschmidt Sep 6, 2026
8733428
fix(graph): keep CreateUploadSession strict below the public drive
dschmidt Sep 6, 2026
84f9621
Revert "fix(graph): keep CreateUploadSession strict below the public …
dschmidt Sep 6, 2026
5b26087
test(acceptance): pin the upload session authorization on public links
dschmidt Sep 6, 2026
bcfccc4
chore: drop the local compose override again
dschmidt Sep 6, 2026
56bee0a
docs: shorter comments
dschmidt Sep 6, 2026
f70d2c4
docs: say why the prefix check does not apply below the public drive
dschmidt Sep 6, 2026
a0e1d7c
fix(scope): pin the public share scope to the link's own graph drive
dschmidt Sep 6, 2026
6d09a83
fix(graph): keep owner data out of public link responses
dschmidt Sep 6, 2026
6727cec
test(acceptance): pin the closed surface around a public link
dschmidt Sep 6, 2026
d9613a0
fix(proxy): log a token hint, not the token
dschmidt Sep 6, 2026
1e6d2ec
test(acceptance): pin that writes through the public link surface are…
dschmidt Sep 6, 2026
b392f43
feat(graph): tell password-required from wrong-password on public links
dschmidt Sep 7, 2026
359dd2f
feat(graph): expose the sharer as the owner of a public link drive
dschmidt Sep 7, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions pkg/middleware/publiclink.go
Original file line number Diff line number Diff line change
@@ -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"

Check failure on line 18 in pkg/middleware/publiclink.go

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

pkg/middleware/publiclink.go#L18

A gitleaks hashicorp-tf-password was detected which attempts to identify hard-coded credentials.
// PublicLinkInvalidPassword means a password was provided but rejected.
PublicLinkInvalidPassword = "invalid-password"

Check failure on line 20 in pkg/middleware/publiclink.go

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

pkg/middleware/publiclink.go#L20

A gitleaks hashicorp-tf-password was detected which attempts to identify hard-coded credentials.
)

// 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) != ""
}
8 changes: 8 additions & 0 deletions services/graph/pkg/errorcode/errorcode.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -99,6 +103,8 @@ var errorCodes = [...]string{
"unauthenticated",
"preconditionFailed",
"itemIsLocked",
"publicLinkPasswordRequired",
"publicLinkPasswordInvalid",
}

// New constructs a new errorcode.Error
Expand Down Expand Up @@ -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:
Expand Down
20 changes: 19 additions & 1 deletion services/graph/pkg/middleware/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -67,7 +84,8 @@ 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 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")
return
Expand Down
4 changes: 3 additions & 1 deletion services/graph/pkg/middleware/path_lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,9 @@ 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; 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().
Str("driveID", driveID).
Str("itemID", anchorIDStr).
Expand Down
97 changes: 90 additions & 7 deletions services/graph/pkg/service/v0/driveitems.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,76 @@ func odataListContains(r *http.Request, parameter, value string) bool {
return false
}

// driveItemInDrive reports whether an item id may be addressed below a drive.
// 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
}
return driveID.GetStorageId() == driveItemID.GetStorageId() && driveID.GetSpaceId() == driveItemID.GetSpaceId()
}

// publicDriveRequest reports whether the request addresses the public share drive.
func publicDriveRequest(r *http.Request) bool {
driveID, err := parseIDParam(r, "driveID")
return err == nil &&
driveID.GetStorageId() == utils.PublicStorageProviderID &&
driveID.GetSpaceId() == utils.PublicStorageSpaceID
}

// sanitizePublicDriveInfos applies the publicstorageprovider's reduction to
// 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 {
return err
}
for _, info := range infos {
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
}

// 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 {
return nil, nil, err
}
gatewayClient, err := g.gatewaySelector.Next()
if err != nil {
return nil, nil, err
}
shareResp, err := gatewayClient.GetPublicShare(ctx, &link.GetPublicShareRequest{
Ref: &link.PublicShareReference{
Spec: &link.PublicShareReference_Token{Token: driveID.GetOpaqueId()},
},
})
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 := 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(), nil
}

// 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)
Expand Down Expand Up @@ -98,7 +168,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
}
Expand Down Expand Up @@ -287,7 +357,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
}
Expand All @@ -312,7 +382,12 @@ 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) {
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())
return
Expand Down Expand Up @@ -345,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))
}
Expand All @@ -372,7 +447,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
}
Expand Down Expand Up @@ -405,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
}

Expand All @@ -430,13 +505,21 @@ func (g Graph) listDriveItemChildren(w http.ResponseWriter, r *http.Request, dri
return nil, false
}

if publicDriveRequest(r) {
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())
if err != nil {
errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error())
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())
}

Expand Down
13 changes: 13 additions & 0 deletions services/graph/pkg/service/v0/drives.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion services/proxy/pkg/middleware/basic_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
47 changes: 42 additions & 5 deletions services/proxy/pkg/middleware/public_share_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -14,7 +16,7 @@ import (

const (
headerRevaAccessToken = revactx.TokenHeader
headerShareToken = "public-token"
headerShareToken = ocmw.PublicLinkTokenName
basicAuthPasswordPrefix = "password|"
authenticationType = "publicshares"

Expand Down Expand Up @@ -55,12 +57,30 @@ 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)
}

// 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)
}

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) {
if !isPublicPath(r.URL.Path) && !isPublicShareArchive(r) && !isPublicShareAppOpen(r) && !isPublicShareGraphRequest(r) {
return nil, false
}

Expand Down Expand Up @@ -99,7 +119,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
Expand All @@ -115,12 +135,29 @@ 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
}

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"))
Expand Down
Loading