Saving the anonymous user (uid 0) revokes access tokens for any consumer with an unset/anonymous default user

I took a look at this one. The uid 0 guard fixes the symptom but might miss the deeper root cause where collectForAccount() can revoke other users non-refresh tokens for any Consumer default user, not only for uid 0.

Follow-up to the above. @benstallings I've updated your existing MR with a few changes based on my testing:

- removed the early return in 'simple_oauth_user_update()' while keeping the
uid 0 guard in the default handler;
- fixed 'collectForAccount()`' so saving one account no longer revokes another
user's token just because the Consumer defaults to the saved account;
- added coverage for uid 0, positive default users, refresh tokens, and the
real OAuth flows.

There's a PHPstan error CI reports but that's an unchanged deprecated call that this code doesn't touch.

AI disclosure. AI was used to triage, understand, and resolve this issue. Review and testing was done manually by me

Assigning myself for testing/review this week.

CI is red

Review flagged: the deleted-subject path hydrated every candidate and parsed rows positionally (fragile under hook_query_alter); multi-value field reordering counted as credential changes; inconsistent accessCheck(); gating lived outside the swappable-handler contract; the anonymous docblock was wrong. Smaller nits: '007'=='7' coercion, per-token (not per-user) uid IN list, missing name/client_id, transient properties compared, EntityInterface hint without hasField().

Fix (substantive work):

1. 25d4c7b — Deleted subjects resolved via one aggregate query (groupBy('id') + MAX(auth_user_id.target_id)), deduped uid IN list, rows keyed on 'id' and read by the by-reference alias behind isset(), skipping reshaped rows rather than guessing. Unit tests use aggregate-first rows to catch positional-regression.
2. fadd9a7 — Gating in the handler: isCredentialChange() on a canonical schema-columns-only signature (strict !==); USER_CREDENTIAL_FIELDS/CONSUMER_CREDENTIAL_FIELDS gained name/client_id; all collection queries accessCheck(FALSE); uid 0 docblock corrected; hint → FieldableEntityInterface.
3. da915e0 — Coverage: reordering is not a change, '007'→'7' renames are, client_id rotation revokes refresh, testUserUpdateRevokesRefreshToken (headline behavior), and anonymous re-saves preserve unrelated tokens.

Separate test maintenance (unrelated to behavior; pre-existing issues surfaced by CI):

1. f0c4494 — Stabilized the ScopeGranularityFormTest flake (file created 2024-12-30 in #3426090): wait for the AJAX-rebuilt subform (waitForField('Permission')) before saving.
2. 3c6b980 — Oauth2ScopePluginManagerLegacyTest: four stale deprecation expectations updated to #3526770's wording.
3. b776361 — EntityCollectorTest: cspell:ignore for the SQLite-stripped aggregate alias (idtarget).
4. 65904ea — AuthCodeFunctionalTest: user_pass_reset_url() → OneTimeAuthentication::generateOneTimeLoginUrl() (deprecated in 11.4, removed in 13.0).

On #3531263: collectForAccount() now includes refresh tokens — reversing that issue's literal expectation, but the intent holds via the gate: routine user saves never reach the collector, so unrelated edits never remove refresh tokens; only a real credential change does.

Assumptions: routine user saves revoke nothing; routine consumer saves keep the historical access-only revocation (deliberate BC); renames are credential changes (conservative, pre-6.2.0-consistent); the ($entity->original ?? NULL) shim is kept for ^10.3 || ^11.

Status: 104/1606 green (+18/131 in simple_oauth_static_scope); phpcs, phpstan, cspell clean.

AI disclosure. AI ran six review rounds; its findings drove most of the fixes in 25d4c7b/fadd9a7, and it helped draft this comment. Review was done manually.

I did a review and test on my local. Everything looks good and works as expected.

RTBC once the test fallback and the issue summary are done

Assigning myself to work on this in sprint 18.

Noting that this is blocked by #3620861: Fix user_pass_reset_url() deprecation compatibility in AuthCodeFunctionalTest and #3620858: Fix deprecation message strings in Oauth2ScopePluginManagerLegacyTest

I updated the issue summary with an API changes section. It covers the change to user token revocation and refresh token handling. It also covers the cron access check and a new constructor argument on the expired token collector.

The one-time login fallback is no longer needed here. #3620861: Fix user_pass_reset_url() deprecation compatibility in AuthCodeFunctionalTest and #3620858: Fix deprecation message strings in Oauth2ScopePluginManagerLegacyTest landed, and this branch is rebased on them.

This issue overlaps with #3436728: Check the status of a consumer. Unpublishing a consumer now revokes its tokens. That issue adds a status check at authentication time, which covers consumers that are already unpublished. Whichever lands second should check for a duplicate status check.

Drafted with the help of an LLM.

Runtime retest against tokens issued through the real authorization_code flow: the user-side contract holds, but the consumer-side refresh revocation cannot reach real refresh tokens because RefreshTokenEntityNormalizer stores them with client = NULL, so unpublishing a consumer or rotating its secret leaves the refresh grant working. Details, evidence, and a fix direction are on the MR: https://git.drupalcode.org/project/simple_oauth/-/merge_requests/229#not...

AI was used to assist with this review.

I added the latest changes that Alex uncovered and added the missing test that @benstallings mentioned. Additional details in the MR.

Thanks @pfrilling, I tested and the token revocation and upgrade cleanup look good. The only remaining thing I see would be moving the new cleanup to a post-update hook so the entity loading&deletion runs after all schema changes are complete.

Good call @scott falconer. I moved the update from .install to .post_update.php.

Thanks @pfrilling, all looks good to me! Marking this as rtbc.