Skip to content

fix(ocdav): send basic auth challenge for password protected links - #904

Merged
butonic merged 1 commit into
mainfrom
fix/public-files-basic-auth-challenge
Oct 23, 2026
Merged

butonic merged 1 commit into
mainfrom
fix/public-files-basic-auth-challenge

Conversation

@kulmann

@kulmann kulmann commented Oct 23, 2026 •

Copy link
Copy Markdown
Member

3rd party clients like Dolphin or macOS Finder send an unauthenticated OPTIONS request when trying to connect to a public link via WebDAV. When it doesn't carry the WWW-Authenticate header in the response those clients simply fail to connect when the public link is protected by a password.

At the same time we don't want a WWW-Authenticate header on an XHR, because otherwise our web ui would show a native browser auth popup, so we don't send it on an XHR.

@kulmann
kulmann requested review from aduffeck, butonic and maki5 October 23, 2026 08:08
@kulmann kulmann self-assigned this Oct 23, 2026

@dschmidt dschmidt left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice and minimal. Verified the flow: auth in the public-files case runs before the dispatch to the folder/file handlers, so an unauthenticated OPTIONS does reach the 401 branch, and the XHR exemption matches what web sends (dav client, SDK client and tus all set X-Requested-With). Also the same check/format the proxy uses in writeSupportedAuthenticateHeader, so behaviour is consistent across both layers.

A few non-blocking nits:

  • The tests use PROPFIND, the PR description talks about OPTIONS. The branch is method agnostic so it is equivalent, but one OPTIONS entry in the table would exercise the actual scenario.
  • The mock returns UNAUTHENTICATED, while the real backend path for a protected link (wrong or missing password) ends up as PERMISSION_DENIED. The fallthrough covers both, but one PermissionDenied entry would test the production path.
  • Optional: the header name and value could be constants next to WwwAuthenticate, and the challenge format string now exists twice in this function (Basic here, Bearer a few lines below). A tiny helper would remove the duplication. Fine to leave as is.

One question: the challenge is now sent on every 401 in this branch, including an expired or invalid signed URL. Is there any case where a browser follows a signed public-files URL as a plain navigation (no XHR)? If so, that would now show a native login popup. If web only uses Basic auth over XHR for protected links, this is a non-issue.

@butonic
butonic merged commit 12937d9 into main Oct 23, 2026
19 of 20 checks passed
@butonic
butonic deleted the fix/public-files-basic-auth-challenge branch October 23, 2026 14:52
@openclouders openclouders mentioned this pull request Oct 23, 2026
1 task
@kulmann

kulmann commented Oct 26, 2026

Copy link
Copy Markdown
Member Author

One question: the challenge is now sent on every 401 in this branch, including an expired or invalid signed URL. Is there any case where a browser follows a signed public-files URL as a plain navigation (no XHR)? If so, that would now show a native login popup. If web only uses Basic auth over XHR for protected links, this is a non-issue.

Urgh, yes. The Preview app supports basic video playback and that uses signed urls. When they expire, we had exactly that issue in the past - native browser popups when you pause a video for longer than 30 minutes or so and then want to resume it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants