Implement token exchange from OCM#57234
Open
enriquepablo wants to merge 85 commits into
Open
Conversation
enriquepablo
requested review from
come-nc,
icewind1991,
nfebe and
provokateurin
and removed request for
a team
This was referenced Jan 2, 2026
Contributor
|
Hello there, 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.) |
enriquepablo
force-pushed
the
master
branch
2 times, most recently
from
869318e to
fa85a7c
Compare
Author
|
Disclaimer: Some of the code in this PR (mainly regarding the tests) was generated by Claude Opus 4.5. I have provided it with very specific prompts, and thoroughly reviewed the output. |
enriquepablo
force-pushed
the
master
branch
2 times, most recently
from
af0f4f2 to
736b3ff
Compare
mickenordin
force-pushed
the
master
branch
2 times, most recently
from
6e14936 to
84713bc
Compare
Signed-off-by: Enrique Pérez Arnaud <enrique@cazalla.net>
Signed-off-by: Enrique Pérez Arnaud <enrique@cazalla.net>
Signed-off-by: Enrique Pérez Arnaud <enrique@cazalla.net>
Signed-off-by: Enrique Pérez Arnaud <enrique@cazalla.net>
Signed-off-by: Enrique Pérez Arnaud <enrique@cazalla.net>
- Align 3rdparty submodule with master - Add baseline entry for the extra getFederationIdFromSharedSecret call introduced for refresh-token flow in RequestHandlerController Signed-off-by: Micke Nordin <kano@sunet.se>
exchangeRefreshToken hardcoded https:// for OCM discovery, breaking federated shares against http remotes (e.g. integration test setup). Use $this->secure to pick the scheme. Signed-off-by: Micke Nordin <kano@sunet.se>
Master's TEMPORARY_TOKEN-via-Bearer rejection (725f5be) blocked the PR's own OCM access tokens, which are intentionally TEMPORARY_TOKEN delivered via Bearer. Mark the OCM tokens with a well-known name and exempt them from the rejection. Signed-off-by: Micke Nordin <kano@sunet.se>
Signed-off-by: Micke Nordin <kano@sunet.se>
Re-run build/autoloaderchecker.sh so the new Version1037Date20260306120000 migration is picked up and the duplicate entries for Version1038/Version1039 are removed. Signed-off-by: Micke Nordin <kano@sunet.se>
By using a JWT that the receiver can verify against a public key, we make it easy for third party services to validate the token independently. This is relevant for example in the context of webapp shares that may defer to a separate service to deliver the actual webapp. NOTE: This requires nextcloud/3rdparty#2413 to be merged before this. Signed-off-by: Micke Nordin <kano@sunet.se>
Signed-off-by: Enrique Pérez Arnaud <enrique@cazalla.net>
…ation_api app Signed-off-by: Enrique Pérez Arnaud <enrique@cazalla.net>
Signed-off-by: Enrique Pérez Arnaud <enrique@cazalla.net>
… signature Signed-off-by: Enrique Pérez Arnaud <enrique@cazalla.net>
…sh token Signed-off-by: Enrique Pérez Arnaud <enrique@cazalla.net>
Signed-off-by: Enrique Pérez Arnaud <enrique@cazalla.net>
Signed-off-by: Enrique Pérez Arnaud <enrique@cazalla.net>
…lities Signed-off-by: Enrique Pérez Arnaud <enrique@cazalla.net>
Signed-off-by: Enrique Pérez Arnaud <enrique@cazalla.net>
Signed-off-by: Enrique Pérez Arnaud <enrique@cazalla.net>
Signed-off-by: Enrique Pérez Arnaud <enrique@cazalla.net>
…s access tokens Signed-off-by: Enrique Pérez Arnaud <enrique@cazalla.net>
Signed-off-by: Enrique Pérez Arnaud <enrique@cazalla.net>
…table was too small Signed-off-by: Enrique Pérez Arnaud <enrique@cazalla.net>
The outbound POST to the remote /api/v1/access-token endpoint was signed using bare OCMSignatoryManager, which falls through to the draft-cavage path because no rfc9421.format option is set. Wrap the manager in Rfc9421SignatoryManager so the exchange request is signed with the Ed25519 key and RFC 9421 signature base, matching what OCMDiscoveryService::prepareOcmPayload already does for OCM endpoints when the remote advertises the http-sig capability. Receiving TokenController auto-detects RFC 9421 vs draft-cavage via the Signature-Input header, so no inbound change is needed. Signed-off-by: Micke Nordin <kano@sunet.se>
…factor The AddressHandler and ISignatureManager usages in RequestHandlerController were superseded on master by the http-sig (RFC 9421) work, which performs incoming-signature verification via IOCMDiscoveryService. After rebasing on top of it, these two imports are no longer referenced. Signed-off-by: Micke Nordin <kano@sunet.se>
… path When getShareByToken() throws DoesNotExistException, $share is never assigned, so logging $share->getId() in the catch block fatals with "Call to a member function getId() on null". Log the exception instead. Signed-off-by: Micke Nordin <kano@sunet.se>
TokenController gained a 10th constructor argument (IShareManager); the test only passed 9, triggering ArgumentCountError on every case. Signed-off-by: Micke Nordin <kano@sunet.se>
getCapabilities() now returns an OCMCapabilities object rather than an array; call ->toArray() before array_diff(). Signed-off-by: Micke Nordin <kano@sunet.se>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See #57152 #57166
Summary
This PR implements the token exchange flow from OCM, allowing Nextcloud to use bearer auth with short lived tokens. This opens up interoperability with OpenCloud/ownCloud OCIS and the possibility to implement webapp shares in the future, which requires this.
Discussion
This is a preliminary PR for discussion. If this goes through there are some things missing with which we'll be grateful to get some guidance:
There is also the question that for access tokens, we are keeping a reference to the corresponding refresh token in the uid field of the access token db record.
Prerequisite
This needs nextcloud/3rdparty#2413 before merging