one-time passwords (e.g. for public file shares)

[theCalcaholic]

• Requires: Refactor: Centralize logic for checking if a share is password protected #61946
• Partially solves: Extendible anonymous sharing authentication options #38906
• Related: Allow temporary email passwords to be one-time use only #32353
• Related: External verification of Share (via Link) #44406

Summary

This PR adds one-time password management to Nextcloud server and integrates them with the files_sharing app.

(the actual form in the screenshot is part of #61733)

Notes

• This PR only changes <60 files. The huge amount of changes comes from rebuilding the frontend (make build-js-production).
• There is a second otp provider available as a Nextcloud app over at otp_provider_debug, which basically just logs OTPs to the Nextcloud logs. This could be helpful for testing/debugging and I would suggest moving it to the nextcloud namespace once this is merged (unless you don't consider it necessary). The main advantage is that you can test OTPs without setting up SMTP. However, I will not keep maintaining it, because that would require tracking changes to the OTP API and testing every new Nextcloud version.
• This PR should be merged after the refactoring of IShare->isPasswordProtected() (Refactor: Centralize logic for checking if a share is password protected #61946)

TODO

• (core) Implement OTP core functionality
• (core) Implement OTP integration into shares
• (files_sharing) Implement creation of OTP protected shares via OCS
• (files_sharing) Implement retrieval of public OTP protected shares via OCS
• Harden bruteforce settings for endpoints
• Tests
• Linting
• Regenerate openapi definitions for files_sharing
• Contribute documentation for the feature

Architecture and Rationale

General concepts

OTPs (one-time password) are short-lived, single use credentials sent to users via an (according to a given threat model) trusted channel (e.g. a specific email address).

OTP Providers define a method of sending OTPs to users.

OTP Recipients are valid address definitions within the scope of an OTP provider that can be sent OTPs.

Core/Server Changes

Generic

One-time passwords are implemented with generic interfaces so that they can be used by other parts of Nextcloud than sharing.

The core functionality for one-time passwords is implemented within the \OCP and \OC namespaces. OTPs are stored within a new database table one_time_password and have a providerID, a recipient string, an expiration date and a password. The idea here is, that the OTP configuration (i.e. provider+recipient) can be long lived, while the credentials (password+expiration date) are (re-)generated per use.
Management of OTPs is implemented in \OC\OneTimePassword\Manager (implementing the injectable interface at \OCP\OneTimePassword\IManager).

\OCP\Security\PasswordContext has been extended by an OTP case to allow the creation of password policies specifically for OTPs.

Events are used to allow apps to register OTP providers. They need to hook into the GetOneTimePasswordProviders and the SendOneTimePassword events to provider their functionality. Providers also need to implement the interface \OCP\OneTimePassword\IOneTimePasswordProvider, which defines methods that allow the Manager to select providers and provide information about them.

Sharing specific

Shares (see \OCP\Share\IShare) have been extended with an one_time_password field.

The \OC\Share20\Manager has been adjusted to prioritize OTPs when checking the authentication for a share.

The template publicshareauth.php has been adjusted to receive and display OTP related information (used in #61733 to show the OTP specific password form).

files_sharing Changes

The ShareAPIController has been extended to allow creating and updating OTP protected shares and returning the otp configuration when fetching shares. OTPs and passwords are mutually exclusive and an error will be returned when attempting to create a share with both.

The ShareController has been extended to supply template responses for public shares with otp related information.

A new ShareOTPController has been implemented that allows users to request OTPs for a share.

OTP Providers

An OTP provider, which allows sending OTPs via email has been implemented as (core) app: otp_provider_email. It can be disabled or restricted by administrators to disallow this functionality.

UI changes

The authentication page for public shares has been updated to accommodate OTPs (see screenshot).
When an OTP is required for a share, the authentication form will change to present a button for requesting an OTP in addition to the normal password field. It will also display a different title above the password field.

Password authentication page for OTP protected shares

The email template for sending OTPs to users is located in apps/otp_provider_email/lib/listener/SendOneTimePasswordEventListener.php and uses Nextclouds usual Mailer interface for templating emails.

Example OTP email sent to users

The logo is not displayed, because I'm testing with a local Nextcloud instance at a non-public URL.

The custom permissions in the sidebar sharing tab (files->sidebar->sharing->custom permissions) has been adjusted to allow choosing between no password protection, setting a normal password and configuring an OTP. The descriptions of the OTP are defined and localized by the OTP provider.

New UI for selecting an authentication method during share creation

Previous UI for selecting an auth method for comparison

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)
• Labels added where applicable (ex: bug/enhancement, 3. to review, feature component)
• Milestone added for target branch/version (ex: 32.x for stable32)

AI (if applicable)

• The content of this PR was partly or fully generated using AI

[theCalcaholic]

From my side, this PR is ready for review. :)

The only thing missing, is documentation, but I would want to wait for feedback before working on that.

Let me know if there is anything you require from me.

Here are curl commands to create an OTP protected share on a Nextcloud instance running at http://nextcloud.local with user=admin and password=admin (requires the otp_provider_debug and otp_provider_email apps to be enabled):

For email:

curl http://nextcloud.local/ocs/v2.php/apps/files_sharing/api/v1/shares -u 'admin:admin' -H 'Content-Type: application/json' -H "Accept: application/json" -H "OCS-APIRequest: true" --data '{"path": "/Nextcloud_Server_Administration_Manual.pdf", "attributes": "[]", "shareType": 3, "otpProvider": "email", "otpRecipient": "your@email.address"}'

For debug (OTPs will be written to Nextcloud logs, requires https://github.com/theCalcaholic/otp_provider_debug):

curl http://nextcloud.local/ocs/v2.php/apps/files_sharing/api/v1/shares -u 'admin:admin' -H 'Content-Type: application/json' -H "Accept: application/json" -H "OCS-APIRequest: true" --data '{"path": "/Nextcloud_Server_Administration_Manual.pdf", "attributes": "[]", "shareType": 3, "otpProvider": "debug", "otpRecipient": "foobar"}'

If the file /Nextcloud_Server_Administration_Manual.pdf doesn't exist for your user, you will need to use a different path.

[theCalcaholic]

@susnux I hope it's ok to tag you (I wasn't sure if the update messages above would reach you otherwise).

[theCalcaholic]

Other than that:

• I have still no idea, why the CI build results in different files than my local build. 🤷

• The failed SQLite integration are probably unrelated to this PR (but I'll double check)
  Update: The filesdrop_features test succeeds for me locally - so it was probably a fluke. Should we just try rerunning it?

• The failing PHPUnit sharding - MySQL 9.4 test seems to be unrelated to this PR.

• The cypress tests for file_sharing/public_share fail inconsistently but quite often with a HTTP 405 error during directory creation via webdav (MKCOL) - long before testing anything sharing related. While debugging, I found that the actual error is this:

  Sabre\DAV\Exception\MethodNotAllowed: The resource you tried to create already exists

  I'm pretty sure this issue is unrelated to this PR and instead linked to that specific test setup.

[theCalcaholic]

@miaulalala I have now checked all failed jobs and don't think they are related to issues in this PR, apart from the translation issue (see my notes above).
Feel free to have a look at it at any time.

[github-actions]

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

[theCalcaholic]

Just did another rebase on master, since the upstream PR is merged now. :)

[theCalcaholic]

Previously, I seem to have made a mistake while rebasing, resulting in a duplicate function and breaking the playwright tests. This should be fixed now

[theCalcaholic]

Nice, now CI finally looks as expected.

[miaulalala]

Security-related

• Uncaught OTPProviderNotFoundException in ShareOTPController::request() — the catch block only handles OTPSendException, not OTPProviderNotFoundException. If a share's OTP provider gets disabled/uninstalled between share creation and an OTP request, this will 500 instead of failing gracefully. apps/files_sharing/lib/Controller/ShareOTPController.php

• requestotp is a state-changing GET route with no CSRF protection. It sends a real email and mutates the OTP row, but as a GET it can be triggered by link prefetching, crawlers, or an <img src> — no user interaction required. Recommend POST + explicit CSRF handling. apps/files_sharing/appinfo/routes.php

• IManager::createOTP() accepts a raw plaintext $password with no hashing guarantee at the public interface boundary — only the internal sendOTP() codepath hashes before persisting. Any third-party app (or future core code) calling createOTP() directly could store OTPs in plaintext. Suggest either dropping that parameter from the public API or hashing centrally inside createOTP(). lib/public/OneTimePassword/IManager.php

• Unescaped HTML interpolation in the OTP email body — SendOneTimePasswordEventListener::sendEmail() concatenates $recipientMsg directly into HTML. No attacker-controlled input reaches it today, but it's fragile: any future caller passing user-controlled text would introduce stored/reflected XSS. Recommend building it through the template's normal parameter substitution instead. apps/otp_provider_email/lib/Listener/SendOneTimePasswordEventListener.php

Regular issues

• IShare public interface gains two new abstract methods (getOneTimePassword/setOneTimePassword) — a BC break for any third-party code implementing IShare directly rather than extending OC\Share20\Share. Appropriately gated behind @since 35.0.0, but worth calling out explicitly in the changelog/upgrade notes. lib/public/Share/IShare.php

• l10n placeholder bug — createShare() uses %s-style interpolation ('No OTP provider found for id %s'), but Nextcloud's IL10N::t() needs {}-style named placeholders, as the sibling updateShare() correctly does with {provider}. The error message won't interpolate for end users as written. apps/files_sharing/lib/Controller/ShareAPIController.php:130 (compare to line 192)

• DI inconsistency — createShare() instantiates new Randomizer() directly instead of using the injected $this->randomizer, while updateShare() uses the injected instance correctly. Harmless functionally but hurts testability/mockability. apps/files_sharing/lib/Controller/ShareAPIController.php:136

• JS operator-precedence bug — if (!response.status === 200) evaluates !response.status first, so it's always comparing a boolean to 200. Currently harmless since axios throws on non-2xx, but it's dead/incorrect logic. apps/files_sharing/src/mixins/ShareRequests.js:614

• Test gaps — the single-use OTP invalidation in ShareController::authSucceeded() (clearing the OTP after successful auth) has no dedicated test, nor does the OTPProviderNotFoundException path in ShareOTPController::request() (see above) or concurrent/overlapping OTP requests.

[theCalcaholic]

@miaulalala Thanks so much for the review!

Most of it makes perfect sense to me and I will start implementing the requested changes shortly, but I have some questions:

Regarding:

requestotp is a state-changing GET route with no CSRF protection.

I was under the assumption that routes without the #[NoCSRFRequired] annotation were, in fact, CSRF protected. Is that incorrect? Nonetheless, I'll update the route to use POST, but I'm unsure how to enforce CSRF protection if that's not already the case.

Regarding:

IManager::createOTP() accepts a raw plaintext $password with no hashing guarantee at the public interface boundary

That (admittedly poorly named) argument expects actually not a plaintext password but a password hash. Setting a plaintext password here will cause the password validation to fail because that expects a hash. Because of this, this is not really a security issue in my opinion, but still not great. I'll drop the argument entirely (alongside the expirationTime argument), since OTPs shouldn't have a password set outside of the sendOTP flow.

Regarding:

l10n placeholder bug — createShare() uses %s-style interpolation ('No OTP provider found for id %s'), but Nextcloud's IL10N::t() needs {}-style named placeholders, as the sibling updateShare() correctly does with {provider}. The error message won't interpolate for end users as written. apps/files_sharing/lib/Controller/ShareAPIController.php:130 (compare to line 192)

That's interesting. I remember trying {...} interpolation at some point but it didn't work (I don't remember the details though). Do I understand correctly, that a different method (so, not IL10N::t()) uses %s interpolation, while IL10N::t() uses named {...} interpolation? If so, I was probably using that method at the time. 🤔
EDIT: I just realized that I copied this syntax from one of the many other places within the same class (and even method) where it is used this way (example). Are you sure this is incorrect?

[theCalcaholic]

And I just noticed something else:

Since requesting the OTP is implemented in the template publicshareauth.php which resides in core, wouldn't it make sense to add the requestOTP function to lib/public/AppFramework/AuthPublicShareController.php?

The base implementation should return an error (401 or 404) and the actual logic for selecting an OTP should lie with the implementing child class, but since the pubblicshareauth template is outside of the control of the application, I think it makes sense to indicate the existence and the interface of the endpoint by having a method to overwrite.

Let me know whether you agree.
(I don't insist on this, btw. It was just a thought that occurred to me.)

[theCalcaholic]

@miaulalala I have now implemented the requested changes apart from those that aren't yet fully clear to me:

Security-related

• Uncaught OTPProviderNotFoundException in ShareOTPController::request() — the catch block only handles OTPSendException, not OTPProviderNotFoundException. If a share's OTP provider gets disabled/uninstalled between share creation and an OTP request, this will 500 instead of failing gracefully. apps/files_sharing/lib/Controller/ShareOTPController.php

  -> fixed

• [-] requestotp is a state-changing GET route with no CSRF protection. It sends a real email and mutates the OTP row, but as a GET it can be triggered by link prefetching, crawlers, or an <img src> — no user interaction required. Recommend POST + explicit CSRF handling. apps/files_sharing/appinfo/routes.php

  -> requestotp is now a POST route - still unsure about the CSRF configuration (see question above)

• IManager::createOTP() accepts a raw plaintext $password with no hashing guarantee at the public interface boundary — only the internal sendOTP() codepath hashes before persisting. Any third-party app (or future core code) calling createOTP() directly could store OTPs in plaintext. Suggest either dropping that parameter from the public API or hashing centrally inside createOTP(). lib/public/OneTimePassword/IManager.php

  -> fixed by removing the password and expirationTime arguments

• Unescaped HTML interpolation in the OTP email body — SendOneTimePasswordEventListener::sendEmail() concatenates $recipientMsg directly into HTML. No attacker-controlled input reaches it today, but it's fragile: any future caller passing user-controlled text would introduce stored/reflected XSS. Recommend building it through the template's normal parameter substitution instead. apps/otp_provider_email/lib/Listener/SendOneTimePasswordEventListener.php

  -> fixed by wrapping $recipientMsg in htmlspecialchars() - I'm not sure if that's what you meant though, but it is how this is solved other places throughout the codebase where email templates were being used. If you would prefer another solution, please let me know. :)

Regular issues

• IShare public interface gains two new abstract methods (getOneTimePassword/setOneTimePassword) — a BC break for any third-party code implementing IShare directly rather than extending OC\Share20\Share. Appropriately gated behind @since 35.0.0, but worth calling out explicitly in the changelog/upgrade notes. lib/public/Share/IShare.php

  -> I'm happy to do that but unsure how where to add changelog/upgrade notes. Could you give me a pointer?

• l10n placeholder bug — createShare() uses %s-style interpolation ('No OTP provider found for id %s'), but Nextcloud's IL10N::t() needs {}-style named placeholders, as the sibling updateShare() correctly does with {provider}. The error message won't interpolate for end users as written. apps/files_sharing/lib/Controller/ShareAPIController.php:130 (compare to line 192)

  -> I'm awaiting your response to my previous question first, to be sure that this is what we want. :)

• DI inconsistency — createShare() instantiates new Randomizer() directly instead of using the injected $this->randomizer, while updateShare() uses the injected instance correctly. Harmless functionally but hurts testability/mockability. apps/files_sharing/lib/Controller/ShareAPIController.php:136

  -> fixed

• JS operator-precedence bug — if (!response.status === 200) evaluates !response.status first, so it's always comparing a boolean to 200. Currently harmless since axios throws on non-2xx, but it's dead/incorrect logic. apps/files_sharing/src/mixins/ShareRequests.js:614

  -> fixed

• Test gaps — the single-use OTP invalidation in ShareController::authSucceeded() (clearing the OTP after successful auth) has no dedicated test, nor does the OTPProviderNotFoundException path in ShareOTPController::request() (see above) or concurrent/overlapping OTP requests.

  -> fixed in apps/files_sharing/tests/Controller/{ShareControllerTest.php,ShareOTPControllerTest.php}

[theCalcaholic]

Another day, another rebase (significantly reducing the changed files in dist/, this time) 🙃

[theCalcaholic]

Sorry for the new pipeline failures. Apparently I introduced them during merge.

I rebased again and fixed the issues (hopefully, couldn't test all of them reliably).