Skip to content

A compromise force-revoke warns the revoked user instead of the owners of shared secrets #802

Description

@rjzondervan

Code reading on 28 September 2026 at 124849b (development). Unverified: not checked live. Found in the development → beta security review. This is new in the delta.

What happens

When an admin force-revokes a suite with markCompromised=true, SuiteCompromiseOnRevokeListener should warn the owners of every secret that suite could read. For a shared copy, that is the owner of the source secret, who has to rotate the credential. The warning goes to the revoked user instead.

The cause is listener order. SuiteLifecycleEventRegistrar (about lines 94–110) registers EncryptionSuiteRevokedListener before SuiteCompromiseOnRevokeListener on the same event, both at priority 0. Nextcloud hands registrations to the dispatcher first-in, first-out (RegistrationContext::delegateEventListenerRegistrations, array_shift), and equal-priority listeners run in the order they were added. So:

  1. EncryptionSuiteRevokedListener runs deleteByTargetUser() and removes every ShareTarget where the revoked user was the recipient.
  2. SuiteCompromiseOnRevokeListener::resolveSourceOwner() then looks those ShareTargets up with findByRecipientSecret(). The lookup always misses, so it falls back to the copy's own owner: the revoked user.

The source owners are never notified, and their source secrets are never stamped possibly_compromised_at or flagged for rotation. The registrar's docblock says the order "is not significant", which is wrong for this pair.

Proposed fix

Make SuiteCompromiseOnRevokeListener run first by giving it a higher priority, and correct the docblock. Add a test that pins the order, because unit tests that mock findByRecipientSecret() cannot see this.

Activity

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

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingencryption-suitesEncryptionSuite lifecyclesecurityA user can read or write what they must nottriageAwaiting triage

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions