Skip to content

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

Open
theCalcaholic wants to merge 8 commits into
nextcloud:masterfrom
theCalcaholic:feature/nextcloud-share-otp
Open

one-time passwords (e.g. for public file shares)#61722
theCalcaholic wants to merge 8 commits into
nextcloud:masterfrom
theCalcaholic:feature/nextcloud-share-otp

Conversation

@theCalcaholic

@theCalcaholic theCalcaholic commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

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

image

(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 image

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 image

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 image image image
Previous UI for selecting an auth method for comparison image

Checklist

AI (if applicable)

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

@theCalcaholic
theCalcaholic requested review from a team and provokateurin as code owners July 2, 2026 11:40
@theCalcaholic
theCalcaholic requested review from Altahrim, ArtificialOwl, leftybournes, nfebe, sorbaugh and susnux and removed request for a team July 2, 2026 11:40
@theCalcaholic
theCalcaholic force-pushed the feature/nextcloud-share-otp branch 2 times, most recently from adbd854 to ae1bd6a Compare July 2, 2026 12:14
@provokateurin
provokateurin marked this pull request as draft July 2, 2026 13:35
@theCalcaholic
theCalcaholic force-pushed the feature/nextcloud-share-otp branch from ae1bd6a to 5035daf Compare July 2, 2026 13:56
@susnux susnux added enhancement 2. developing Work in progress labels Jul 2, 2026
@theCalcaholic
theCalcaholic force-pushed the feature/nextcloud-share-otp branch from 5035daf to 99f7621 Compare July 2, 2026 15:36
@theCalcaholic theCalcaholic changed the title [WIP] Support one-time passwords (e.g. for public file shares) [backend] Support one-time passwords (e.g. for public file shares) Jul 2, 2026
@theCalcaholic
theCalcaholic force-pushed the feature/nextcloud-share-otp branch 3 times, most recently from edb4db5 to 27730a7 Compare July 2, 2026 23:28
@susnux susnux added the community pull requests from community label Jul 3, 2026
@theCalcaholic
theCalcaholic force-pushed the feature/nextcloud-share-otp branch from bc04d3a to 9a9f4d3 Compare July 3, 2026 13:26
@theCalcaholic
theCalcaholic marked this pull request as ready for review July 3, 2026 14:00
@theCalcaholic

theCalcaholic commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

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
theCalcaholic force-pushed the feature/nextcloud-share-otp branch from f79b8ba to 702cee9 Compare July 3, 2026 17:36
@theCalcaholic

Copy link
Copy Markdown
Contributor Author

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

@susnux susnux removed the 2. developing Work in progress label Jul 6, 2026
@theCalcaholic
theCalcaholic force-pushed the feature/nextcloud-share-otp branch from 1c2d696 to 271c553 Compare July 22, 2026 11:10
@theCalcaholic

theCalcaholic commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@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 bodySendOneTimePasswordEventListener::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 bugcreateShare() 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 inconsistencycreateShare() 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 bugif (!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
theCalcaholic force-pushed the feature/nextcloud-share-otp branch 2 times, most recently from f583d80 to e1b1aa5 Compare July 24, 2026 07:17
@theCalcaholic

theCalcaholic commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

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

@theCalcaholic
theCalcaholic force-pushed the feature/nextcloud-share-otp branch 2 times, most recently from 049e0bf to faf1609 Compare July 28, 2026 06:15
@theCalcaholic

Copy link
Copy Markdown
Contributor Author

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).

@theCalcaholic

theCalcaholic commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

@miaulalala Hi, I don't want to create any pressure, however, I wanted to ask if there's anything you need from me right now?

Other than that, please let me know, when you want me to rebase again (I'd rather not do that every day, since that means more work and always creates the risk of introducing errors).

@miaulalala

Copy link
Copy Markdown
Contributor

/compile amend

@miaulalala

Copy link
Copy Markdown
Contributor

@theCalcaholic can you rebase your PR and squash? Then I'll retrigger CI 🙏

@theCalcaholic

Copy link
Copy Markdown
Contributor Author

@miaulalala Sure, will do!

…ords

Signed-off-by: Tobias Knöppler <tobias@knoeppler.org>
Signed-off-by: Tobias Knöppler <tobias@knoeppler.org>
…ackend/API)

Signed-off-by: Tobias Knöppler <tobias@knoeppler.org>
…ge (if available)

Signed-off-by: Tobias Knöppler <tobias@knoeppler.org>
Signed-off-by: Tobias Knöppler <tobias@knoeppler.org>
Signed-off-by: Tobias Knöppler <tobias@knoeppler.org>
Signed-off-by: Tobias Knöppler <tobias@knoeppler.org>
@theCalcaholic
theCalcaholic force-pushed the feature/nextcloud-share-otp branch from faf1609 to 7136cc7 Compare August 13, 2026 15:25
@theCalcaholic

Copy link
Copy Markdown
Contributor Author

@miaulalala Done. I hope I didn't miss anything this time :)

Signed-off-by: Tobias Knöppler <tobias@knoeppler.org>
@theCalcaholic
theCalcaholic force-pushed the feature/nextcloud-share-otp branch from 7136cc7 to 41321ee Compare August 13, 2026 15:27
@theCalcaholic

theCalcaholic commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

@miaulalala 🙈 And it's outdated again. But at least there are no conflicts, so I think, this is still fine?
If I rebase and test everything again, we will likely be in the same situation ...

@susnux

susnux commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

@theCalcaholic you do not need to rebase if only dist/* conflicts. We can do so after this is reviewed and ready to be merged :)

* @psalm-import-type Files_SharingOTPSendError from ResponseDefinitions
* @psalm-import-type Files_SharingOTPProvider from ResponseDefinitions
*/
class ShareOTPController extends ApiController {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this should be an OCS controller instead - OCS would be the standard for all our "officially" provided APIs

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Alright, I remember, that it was a deliberate decision to not implement this as OCS controller, but I don't remember the reasons anymore (it's been some time :D).
I'll try to change it and if it comes up again I'll discuss it with you.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I dont think we need an empty routes file?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Originally I had no routes.php, but it broke the CI checks. Not quite sure anymore, but it might have been the OpenAPI generator that complains if we remove it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I do not really get why this interface provides the setters?

Isnt the workflow:

  • app requests OTP from manager
  • manager generates an OTP
  • app checks OTP later?

In this case the interface never needs setters?

@theCalcaholic theCalcaholic Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OTPs are created during share creation without password and expiration time (but providing OTP configuration, i.e. provider, recipient) while passwords are set at a later point, when requested by the recipient (see sequence diagram below). Also, the setters are used by the OTP manager when loading and creating OTPs (see here).

@susnux susnux left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How to get the available providers?
Would you need to know them / hardcode them?

Why not use the common registration approach? Meaning add something like
addOtpProvider to the BootstrapContext so that an provider app would enable itself.

This way the otp manager can:

  1. check there is at least one provider enabled
  2. check if a provider id is valid
  3. get a list of available providers

In general I am not sure I correctly understood the workflow that should be done here.
From how I understand the feature, there are following entities:

  1. Manager - handling OTP from within OCP API
  2. (multiple) Providers - handling delivery of OTPs to the user
  3. OTP - The password object
  4. App - Requests a new OTP and checks user input against OTP

I would expect the workflow to be something like

sequenceDiagram
    App->>+Manager: Request OTP for Subject and Receiver
    Manager->>+App: Responds with OTP
    App-->>+App: Stores that OTP is used
    Manager->>+Receiver: Delivers OTP
    Receiver->>+App: Requests access to Subject with OTP
    App->>+Manager: Checks validity of OTP or Subject and Receiver
    Manager->>+App: Confirms access
    Manager-->>+Manager: Deletes OTP
Loading

Meaning

  • The manager handles generation and storage of OTPs (secret, subject, receiver).
  • Providers add support for different receivers.
  • Manager provides list of available providers so apps can offer OTP for specific receivers
  • App only stores that an OTP is used -> checks manager if OTP is correct

@theCalcaholic

theCalcaholic commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

@susnux Thanks so much for the review!

I'll try to answer your questions below.

How to get the available providers? Would you need to know them / hardcode them?

Nothing is hardcoded. Instead, the OTPManager creates a GetOneTimePasswordProvidersEvent to collect a list of providers. Providers, in turn, need to implement and register listeners to the event.

Why not use the common registration approach? Meaning add something like addOtpProvider to the BootstrapContext so that an provider app would enable itself.

This way the otp manager can:

1. check there is at least one provider enabled

2. check if a provider id is valid

3. get a list of available providers

All of this is already possible (and working this way).
I was following the pattern I have found in the implementation of the password policies here and here, but if there's a better way to implement provider registration, I'm open to it. :)
However, if I understand this right, registering OTP providers would mean that they would register during every single request, right? At the moment, a provider only registers an event listener (actually two, but one for this purpose). I'm not sure if it would be more or less expensive to perform OTP provider registration instead of event listener registration and also I'm not sure what types of bootstrap actions can be cached/executed during app installation by Nextcloud.

In general I am not sure I correctly understood the workflow that should be done here. From how I understand the feature, there are following entities:
1. Manager - handling OTP from within OCP API
Correct, the manager takes care of interfacing with the database and implementing the general OTP logic.

2. (multiple) Providers - handling delivery of OTPs to the user

Yes, handling delivery + validation of recipients. They are using events to communicate both back to the OTP manager (or whoever needs to access them).

3. OTP - The password object

OTP is an object containing both the configuration for the OTP (provider + allowed recipient + (optional) expiry date) as well as the password. Initially, the password is empty until requested by a client.

4. App - Requests a new OTP and checks user input against OTP

The actual validation logic is implemented in the OTP Manager, but it is called from the app (or in the implemented use case - i.e. public shares - from the Share20 Manager).

I would expect the workflow to be something like

The workflow for public shares is slightly different, because it is not the sharee who decides whether OTPs are used, but the sharer. So, the actual process looks as follows (but mind, that the implementation is designed to be flexible enough to allow other use cases to implement support in different ways). I have split it into share creation and retrieval for better readability.

Share Creation

sequenceDiagram
  actor Sharer
  Sharer->>+FilesSharingApp: Request dialog for public share creation
  FilesSharingApp-)+EventBus: Request list of OTP providers with UI metadata to provide options for Sharer
  EventBus-)+ProviderApp: Receive list-otp-providers request
  ProviderApp--)+EventBus: Respond with providers + metadata
  EventBus--)+FilesSharingApp: Receive providers + metadata
  FilesSharingApp-->>+Sharer: Offer list of OTP providers for public share creation
  Sharer->>+FilesSharingApp: Request OTP protected share for given provider + recipient
  FilesSharingApp->>+Manager: Create OTP configuration for the share (with empty password)
Loading

Share Retrieval

sequenceDiagram
  actor Sharee
  Sharee->>+FilesSharingApp: Request OTP for configured recipient
  FilesSharingApp->>+Manager: Request OTP to be generated (sets password + expiration time)
  Manager-)+EventBus: Request OTP provider to be sent to recipient
  EventBus-)+ProviderApp: Receive send request with matching provider id
  ProviderApp--)+Sharee: Sends OTP
  ProviderApp--)+EventBus: Responds with send success/failure status
  EventBus--)+Manager: Returns send success/failure status
  Sharee->>+FilesSharingApp: Asks for access with (OTP) password
  FilesSharingApp->>+Manager: Checks OTP validity
  Manager->>+Manager: Deletes OTP
  Manager-->>+FilesSharingApp: Confirms access
  FilesSharingApp-->>+Sharee: Grants access to share
Loading

Note: I have merged the responsibilities of the files_sharing app and the Share20/Manager here a bit, but I hope, the picture still becomes clear.

Meaning

* The manager handles generation and storage of OTPs (secret, subject, receiver).

correct

* Providers add support for different receivers.

correct

* Manager provides list of available providers so apps can offer OTP for specific receivers

yes and no, the providers themselves add themselves to the list for anyone asking (via event bus, see here). This doesn't require the manager, but it provides a method for this purpose, for convenience.

* App only stores that an OTP is used -> checks manager if OTP is correct

The app stores the OTP id (as foreign key), which is required to retrieve recipient and provider ids.

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

Labels

3. to review Waiting for reviews community pull requests from community enhancement feedback-requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants