feat(theme): bearer validator auto-provisions WP Subscriber on first sign-in (Phase 5 Track B)#176
Merged
Merged
Conversation
…sign-in (Phase 5 Track B) Supersedes locked decision #1 of cdcf-bio-edit-zitadel ("no auto- provisioning") for the Subscriber path only. Elevated roles still flow through the Phase 6 admin-approval workflow (separate, not in this PR). Why --- After deploying bio-edit Phase 4 to staging, the operator signed up via Zitadel (open sign-up + email verification per umbrella policy) but no corresponding WP user existed yet — so /api/my-bio/check correctly returned {linked:false} and the bearer validator fell through. Bridging that gap manually for every new sign-up is operationally untenable. Phase 5 closes it: any email-verified Zitadel sign-up gets a WP Subscriber automatically on first request. Subscriber has zero edit capabilities in WP core, so the blast radius is bounded. Cross-system identity primary key: Zitadel sub claim ---------------------------------------------------- The Zitadel sub claim is the immutable cross-system identity primary key. Stored as WP user meta `cdcf_zitadel_sub` at provisioning / first sign-in. Email is mutable in Zitadel, so using email-only would silently create a second WP user every time the operator changed their Zitadel email — the never-a-second-WP-user invariant is the load-bearing requirement. User resolution flow (cdcf_zitadel_bearer_resolve_user) ------------------------------------------------------- 1. By sub via user_meta cdcf_zitadel_sub → if claim email differs from wp_users.user_email, wp_update_user to match (email-drift sync). Never creates a second WP user. 2. By email (one-time migration for users created before sub binding existed) → write cdcf_zitadel_sub meta so subsequent requests take path 1. 3. Auto-provision a Subscriber via wp_insert_user with: - user_login = full email (immutable per WP, matches the umbrella Zitadel UserEmailAsUsername=true invariant at creation time) - user_email = email - user_pass = wp_generate_password(64, true, true) — never surfaced, sign-in must always flow through Zitadel - role = 'subscriber' - display_name = Zitadel `name` claim, falling back to email Then bind cdcf_zitadel_sub user-meta. Concurrency-safe: if two parallel sign-ins for the same sub race into the auto-provision branch, the second wp_insert_user fails with user_email_exists, and the validator recovers by re-querying for the sub-bound user (the winner's row) — same id, no duplicate. Validator fail-safes preserved ------------------------------ - Audience verification (PR #173 multi-aud allow-list) runs FIRST, before any new code. - email_verified=false still falls through (no provision). - Missing sub claim still falls through (no email-only resolution, no provision) — required for safety. - Transient cache is keyed by sha256(token) and stores the resolved WP id, same as before. Files ----- - includes/auth/zitadel-bearer.php: cdcf_zitadel_bearer_authenticate now delegates user resolution to cdcf_zitadel_bearer_resolve_user. Two new helpers: cdcf_zitadel_bearer_user_by_sub() and cdcf_zitadel_bearer_auto_provision_subscriber(). - tests/ZitadelBearerTest.php: 7 new tests covering sub-primary-key hit, email-drift sync, sub-miss/email-match migration, missing-sub fallthrough, Zitadel `name` → display_name + email fallback, and the auto-provision race recovery. - tests/bootstrap.php: WP_User stub gains user_email, user_login, display_name (typed string properties) to silence PHP 8.2+ dynamic- property deprecations from the new tests. - AGENTS.md (+CLAUDE.md symlink): replaces the "no auto-provisioning" paragraph in the Zitadel bearer auth section with the new three- step resolution flow. Theme suite: 450 → 457 tests, 1033 → 1056 assertions, all green. Track A dependency ------------------ cdcf-infra PR #13 updates the CDCF Website project's role catalog to mirror WP (subscriber, contributor, author, editor, administrator). This PR uses 'subscriber' as the WP role assigned on provision; that role exists in WP core so the WP write succeeds regardless of cdcf- infra ordering. Order them as A → B in the prod rollout so the elevation flow (Phase 6) has a clean canonical role catalog to write userGrants against later. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Worried about impact? Review this PR in Change Stack to explore blast radius before you approve or request changes. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughBearer authentication for WordPress REST API now resolves user identity via Zitadel ChangesZitadel Sub-Driven User Resolution
Sequence DiagramsequenceDiagram
participant Client
participant BearerHandler as Bearer Handler
participant Resolver as Resolve User
participant WordPress as WordPress
participant Cache as Transient Cache
Client->>BearerHandler: HTTP request + Bearer token
BearerHandler->>BearerHandler: Decode JWT & verify audience
BearerHandler->>BearerHandler: Fetch userinfo, verify email_verified
BearerHandler->>BearerHandler: Require sub claim
BearerHandler->>Resolver: cdcf_zitadel_bearer_resolve_user(sub, email, name)
rect rgba(200, 220, 255, 0.5)
Note over Resolver,WordPress: Sub-based lookup
Resolver->>WordPress: get_users(meta: cdcf_zitadel_sub)
WordPress-->>Resolver: WP_User (if found)
alt Found by sub
Resolver->>Resolver: Check email drift
alt Email drifted
Resolver->>WordPress: wp_update_user(new email)
end
end
end
rect rgba(220, 200, 255, 0.5)
Note over Resolver,WordPress: Email fallback (migration)
alt Not found by sub
Resolver->>WordPress: get_users(user_email)
WordPress-->>Resolver: WP_User (if found)
alt Found by email
Resolver->>WordPress: update_user_meta(cdcf_zitadel_sub)
end
end
end
rect rgba(255, 220, 200, 0.5)
Note over Resolver,WordPress: Auto-provision
alt User not found
Resolver->>WordPress: wp_insert_user(subscriber)
WordPress-->>Resolver: user_id or error
alt Insert conflict
Resolver->>WordPress: get_users(meta: cdcf_zitadel_sub)
WordPress-->>Resolver: WP_User
end
Resolver->>WordPress: update_user_meta(cdcf_zitadel_sub)
end
end
Resolver-->>BearerHandler: user_id
BearerHandler->>Cache: Store user_id
BearerHandler-->>Client: Authenticated
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 25 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
wordpress/themes/cdcf-headless/includes/auth/zitadel-bearer.php (1)
5-20: ⚡ Quick winUpdate the file header docblock to match current behavior.
The top docblock still says the resolver is email-based and does not auto-provision, which no longer matches this implementation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wordpress/themes/cdcf-headless/includes/auth/zitadel-bearer.php` around lines 5 - 20, Update the file header docblock to reflect the current implementation: describe that the code hooks into the determine_current_user filter and accepts an Authorization: Bearer <token> header, validates the token against Zitadel's /oidc/v1/userinfo endpoint, caches accepted token-hash → user_id pairs using sha256(token) in a transient (TTL 60s default), and resolves the WP user by the current claim used in the code (replace the outdated "email-based" wording with the actual claim the implementation uses, e.g., sub or preferred_username). Also update the provisioning statement to reflect current behavior (if the code now auto-provisions users or still falls through, state that accurately) and remove any references that contradict the implemented logic such as "does not auto-provision" or "email-based resolver" and keep mention of author_team_member linkage if still relevant.wordpress/themes/cdcf-headless/tests/ZitadelBearerTest.php (1)
453-457: ⚡ Quick winTighten
get_users()mocks to assert sub-lookup query args.These aliases return a user without validating
meta_key/meta_value, so a regression in sub-query wiring could still pass. Assertcdcf_zitadel_suband expectedsubin these mocks.Also applies to: 585-593
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wordpress/themes/cdcf-headless/tests/ZitadelBearerTest.php` around lines 453 - 457, The get_users() test aliases should inspect incoming query args and assert the sub-lookup keys instead of always returning a user; update the Functions\when('get_users')->alias callbacks (the one creating a WP_User) to accept the query args, assert that args['meta_key'] === 'cdcf_zitadel_sub' and args['meta_value'] === the expected sub value, then return the WP_User with the desired email; apply the same change to the other get_users() mock block (the second alias) so both mocks validate meta_key/meta_value before returning a user.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Around line 215-220: The "Fails closed on every unhappy path" paragraph still
lists the stale example "no WP user match" which contradicts the new
auto-provisioning behavior described in the earlier bullets (the auto-provision
Subscriber flow and user-resolution steps that write cdcf_zitadel_sub); update
that paragraph by removing the "no WP user match" example so the fail-closed
list only contains true unhappy paths (opaque non-JWT token, wrong audience,
non-200 userinfo, malformed JSON, unverified email, etc.) and ensure the text
references the auto-provisioning/user-resolution behavior (cdcf_zitadel_sub,
user_login=user email, display_name seeded from name claim) to avoid future
contradiction.
In `@wordpress/themes/cdcf-headless/includes/auth/zitadel-bearer.php`:
- Around line 251-255: The current guard uses empty($userinfo['email_verified'])
which allows non-boolean truthy values; update the check in the
authentication/resolver block that inspects $userinfo (the code around
$userinfo['email_verified'] and $userinfo['email'] that currently returns
$user_id) to require strict boolean true for email verification (e.g. verify
$userinfo['email_verified'] === true) while still ensuring $userinfo['email'] is
present; replace the empty() test with an explicit boolean check so untrusted
truthy values are rejected.
In `@wordpress/themes/cdcf-headless/tests/ZitadelBearerTest.php`:
- Around line 72-80: The docstring for the helper that "Build[s] a fake
wp_remote_post response" incorrectly says omitting the 'sub' key will bypass the
default; update the comment to state that the helper injects a default 'sub'
when the key is omitted and that tests that want a missing 'sub' must explicitly
pass 'sub' => null (not simply omit the key). Also update the other similar doc
block (the second paragraph noted) to the same wording so both places reflect
the actual behavior.
- Around line 243-250: Update the stale test comment in ZitadelBearerTest (the
block containing "PRE-PHASE-5" and the phrase "fall through, no
auto-provisioning") to reflect current behavior: remove or replace "fall
through, no auto-provisioning" and any pre-Phase-5 wording and instead state
that this test now validates successful auto-provisioning for the Subscriber
path (while keeping notes about other fallthrough cases like
email-verified-false and sub-missing as applicable).
---
Nitpick comments:
In `@wordpress/themes/cdcf-headless/includes/auth/zitadel-bearer.php`:
- Around line 5-20: Update the file header docblock to reflect the current
implementation: describe that the code hooks into the determine_current_user
filter and accepts an Authorization: Bearer <token> header, validates the token
against Zitadel's /oidc/v1/userinfo endpoint, caches accepted token-hash →
user_id pairs using sha256(token) in a transient (TTL 60s default), and resolves
the WP user by the current claim used in the code (replace the outdated
"email-based" wording with the actual claim the implementation uses, e.g., sub
or preferred_username). Also update the provisioning statement to reflect
current behavior (if the code now auto-provisions users or still falls through,
state that accurately) and remove any references that contradict the implemented
logic such as "does not auto-provision" or "email-based resolver" and keep
mention of author_team_member linkage if still relevant.
In `@wordpress/themes/cdcf-headless/tests/ZitadelBearerTest.php`:
- Around line 453-457: The get_users() test aliases should inspect incoming
query args and assert the sub-lookup keys instead of always returning a user;
update the Functions\when('get_users')->alias callbacks (the one creating a
WP_User) to accept the query args, assert that args['meta_key'] ===
'cdcf_zitadel_sub' and args['meta_value'] === the expected sub value, then
return the WP_User with the desired email; apply the same change to the other
get_users() mock block (the second alias) so both mocks validate
meta_key/meta_value before returning a user.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 12b97f24-7ce3-4b43-9cbf-346a343c2328
📒 Files selected for processing (4)
AGENTS.mdwordpress/themes/cdcf-headless/includes/auth/zitadel-bearer.phpwordpress/themes/cdcf-headless/tests/ZitadelBearerTest.phpwordpress/themes/cdcf-headless/tests/bootstrap.php
- zitadel-bearer.php header docblock now reflects Phase 5 sub-based resolution + auto-provisioning (was stale: email-only, no provisioning). - email_verified gate tightened from empty() to === true so a spec-violating IdP can't trick it with non-boolean truthy payloads. - AGENTS.md fail-closed list: "no WP user match" removed (it's an auto-provision case now); added missing-sub + auto-provision-failure. - ZitadelBearerTest helper docstring: clarify that omitting 'sub' triggers default injection (tests for the missing-sub branch must override explicitly, e.g. by bypassing buildUserinfoResponse). - ZitadelBearerTest test_no_matching_wp_user_auto_provisions_subscriber lead comment no longer says "fall through, no auto-provisioning". Theme suite: 457/457, 1056 assertions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
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
2 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.
Summary
Track B of Phase 5 of the cdcf-role-mirroring plan (planner-side). Supersedes locked decision #1 of
~/.claude/plans/cdcf-bio-edit-zitadel.md("no auto-provisioning") for the Subscriber path only. Elevated roles still flow through Phase 6's admin-approval workflow (separate, not in this PR).Why
After deploying bio-edit Phase 4 to staging, the operator signed up via Zitadel (open sign-up + email verification per umbrella policy) but no corresponding WP user existed yet — so
/api/my-bio/checkcorrectly returned{linked: false}and the bearer validator fell through. Bridging that gap manually for every new sign-up is operationally untenable. Phase 5 closes it: any email-verified Zitadel sign-up gets a WP Subscriber automatically on first request. Subscriber has zero edit capabilities in WP core, so the blast radius is bounded.Cross-system identity primary key: Zitadel
subThe Zitadel
subclaim is the immutable cross-system identity primary key. Stored as WP user metacdcf_zitadel_subat provisioning / first sign-in. Email is mutable in Zitadel, so using email-only would silently create a second WP user every time the operator changed their Zitadel email — the never-a-second-WP-user invariant is the load-bearing requirement.User-resolution flow (
cdcf_zitadel_bearer_resolve_user)subvia user_metacdcf_zitadel_sub. If found and the claim's email differs fromwp_users.user_email,wp_update_userto match — the email-drift sync. Never creates a second WP user.cdcf_zitadel_subuser-meta on match so subsequent requests take path 1.wp_insert_user:user_login= full email (immutable per WP, matches the umbrella ZitadelUserEmailAsUsername=trueinvariant at creation time)user_email= emailuser_pass=wp_generate_password(64, true, true)— never surfaced, sign-in must always flow through Zitadelrole=subscriberdisplay_name= Zitadelnameclaim → fallback emailThen bind
cdcf_zitadel_subuser-meta.Concurrency safety
Two parallel sign-ins for the same
subboth reach the auto-provision branch. The firstwp_insert_userwins; the second getsWP_Error(existing_user_email). The validator recovers by re-querying for the sub-bound user (the winner's row) — same id, no duplicate.Fail-safes preserved
email_verified=falsestill falls through (no provision).subclaim still falls through (no email-only resolution, no provision) — required for safety.sha256(token)and stores the resolved WP id, same as before.Cross-track ordering
cdcf-infra PR #13 (Track A) updates the CDCF Website project's Zitadel role catalog to mirror WP (
subscriber,contributor,author,editor,administrator). This PR uses'subscriber'as the WP role assigned on provision; that role exists in WP core so the WP write succeeds regardless of cdcf-infra ordering.Order them as A → B in the prod rollout so Phase 6's elevation flow has a clean canonical role catalog to write
userGrantsagainst later.This PR is also independent of cdcf-website PR #174 (bio editor UI — orthogonal scope, mid-review).
Tests
ZitadelBearerTest.php:test_sub_primary_key_lookup_hits_no_email_drifttest_sub_match_with_drifted_email_updates_wp_user_emailtest_sub_miss_email_match_binds_sub_meta_for_migrationtest_sub_claim_missing_falls_through_without_provisioningtest_auto_provision_uses_zitadel_name_for_display_nametest_auto_provision_falls_back_to_email_for_display_name_when_name_missingtest_auto_provision_race_recovers_via_sub_relookuptest_no_matching_wp_user_falls_through→test_no_matching_wp_user_auto_provisions_subscriber. The old behavior is documented inline as a marker of the policy reversal.tests/bootstrap.phpWP_Userstub gainsuser_email/user_login/display_nametyped string properties so PHP 8.2+ doesn't deprecate the dynamic-property writes.Theme suite locally: 457 / 457 (was 450 → +7), 1056 assertions, no deprecations.
Deploy
WP theme change. Needs
gh workflow run deploy.yml -f environment=productionafter merge — astaging-target run skips theme/plugin steps and the new resolver won't reach live WP.Test plan
wp user list --role=subscribershows the new row withuser_emailmatching the Zitadel claim andcdcf_zitadel_submeta set.user_email(no second user, sub binding still set).submatches an existing WP user butaudis wrong → audience check rejects before any user resolution runs.Summary by CodeRabbit
New Features
Improvements
Documentation
Tests