Skip to content

CloudFederationApi: access-token lifecycle fixes - #61889

Open
mickenordin wants to merge 4 commits into
masterfrom
kano-fix-ocm-duplicate-sharedSecret
Open

CloudFederationApi: access-token lifecycle fixes#61889
mickenordin wants to merge 4 commits into
masterfrom
kano-fix-ocm-duplicate-sharedSecret

Conversation

@mickenordin

@mickenordin mickenordin commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes three lifecycle bugs in the OCM access-token exchange

  1. A refresh token (the OCM sharedSecret) can back several concurrent access tokens.

  2. CleanupExpiredOcmTokensJob only deleted ocm_token_map not the access tokens in oc_authtoken

  3. A new ShareDeletedEvent listener revokes all access tokens exchanged from the removed share's secret

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Tests (unit, integration, api and/or acceptance) are included
  • Screenshots N/A because only a backend change
  • Documentation is not required
  • Backports not neccessary
  • Labels added
  • Milestone added

AI (if applicable)

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

@mickenordin mickenordin added this to the Nextcloud 35 milestone Jul 7, 2026
@mickenordin
mickenordin requested a review from enriquepablo July 7, 2026 17:35
@mickenordin mickenordin added bug 3. to review Waiting for reviews labels Jul 7, 2026
@mickenordin
mickenordin force-pushed the kano-fix-ocm-duplicate-sharedSecret branch 2 times, most recently from 83513fa to 60f962d Compare July 8, 2026 06:45
@mickenordin

Copy link
Copy Markdown
Contributor Author

I was able to verify this in my testrig:

kubectl --context ocm-test -n nextcloud exec alice-db-1 -- \
  psql -U postgres -d alice -c \
  "SELECT count(*) AS maps_for_secret
     FROM oc_ocm_token_map
    WHERE refresh_token = '<verysecret>';
   SELECT count(*) AS total_ocm_access_tokens
     FROM oc_authtoken WHERE name = 'OCM Access Token';"
Defaulted container "postgres" out of: postgres, bootstrap-controller (init)
 maps_for_secret 
-----------------
               3
(1 row)

 total_ocm_access_tokens 
-------------------------
                       3
(1 row)

Then after unshare:

kubectl --context ocm-test -n nextcloud exec alice-db-1 -- \
  psql -U postgres -d alice -c \
  "SELECT count(*) AS maps_for_secret
     FROM oc_ocm_token_map
    WHERE refresh_token = '<verysecret>';
   SELECT count(*) AS total_ocm_access_tokens
     FROM oc_authtoken WHERE name = 'OCM Access Token';"
Defaulted container "postgres" out of: postgres, bootstrap-controller (init)
 maps_for_secret 
-----------------
               0
(1 row)

 total_ocm_access_tokens 
-------------------------
                       0
(1 row)

@mickenordin
mickenordin marked this pull request as ready for review July 9, 2026 15:59
@mickenordin
mickenordin requested a review from a team as a code owner July 9, 2026 15:59
@mickenordin
mickenordin requested review from Altahrim, ArtificialOwl, icewind1991 and salmart-dev and removed request for a team July 9, 2026 15:59
@susnux susnux added the community pull requests from community label Jul 14, 2026

@enriquepablo enriquepablo 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.

lgtm

@mickenordin
mickenordin force-pushed the kano-fix-ocm-duplicate-sharedSecret branch 3 times, most recently from 0d5d5f2 to 455f467 Compare July 17, 2026 09:01
@mickenordin
mickenordin enabled auto-merge July 17, 2026 09:22
@mickenordin
mickenordin force-pushed the kano-fix-ocm-duplicate-sharedSecret branch from 455f467 to aa41eb1 Compare July 17, 2026 12:31
The cleanup job only deleted ocm_token_map rows, orphaning the
temporary access tokens in oc_authtoken so they accumulated
indefinitely. Revoke each expired access token before dropping its
mapping.

Assisted-by: ClaudeCode:claude-fable-5

Signed-off-by: Micke Nordin <kano@sunet.se>
…s removed

Extract the revoke-then-delete logic into OcmTokenService and add a
ShareDeletedEvent listener that revokes the access tokens exchanged from
a removed share's secret, drops their ocm_token_map rows, and invalidates
the refresh token — instead of leaving them until the expiry job, which
can no longer find them once the share is gone.

Assisted-by: ClaudeCode:claude-fable-5

Signed-off-by: Micke Nordin <kano@sunet.se>
…oken

A refresh token may back several concurrent access tokens. E.g. in a multi
protocol share for a webapp the webdav mount and the webapp launcher exchange
the same secret, so the exchange must not assume a single mapping.

Drop the revoke-previous step and the findByRefreshToken (findEntity) lookup, which threw
MultipleObjectsReturnedException once more than one access token existed.
Expiry and unshare cleanup revoke the tokens instead.

Assisted-by: ClaudeCode:claude-fable-5

Signed-off-by: Micke Nordin <kano@sunet.se>
Also update stale comment that mentioned a function
that no longer exists.

Signed-off-by: Micke Nordin <kano@sunet.se>
@mickenordin
mickenordin force-pushed the kano-fix-ocm-duplicate-sharedSecret branch from aa41eb1 to 58cf6c7 Compare July 17, 2026 13:08
@github-actions

Copy link
Copy Markdown
Contributor

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

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 AI assisted bug community pull requests from community feedback-requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants