From b392f43662fd1058a1f57e0fb06ec827a1dd3d06 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Mon, 7 Sep 2026 10:07:33 +0200 Subject: [PATCH] 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