Skip to content

fix(theme): pass Zitadel given_name + family_name to auto-provisioned WP Subscriber #178

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JohnRDOrazio wants to merge 1 commit into
base: main
Choose a base branch
from
+79 −7
Open

fix(theme): pass Zitadel given_name + family_name to auto-provisioned WP Subscriber #178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 37 additions & 7 deletions wordpress/themes/cdcf-headless/includes/auth/zitadel-bearer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,10 +276,18 @@ function cdcf_zitadel_bearer_authenticate($user_id) {
return $user_id;
}

// Pull the OIDC standard profile claims that map to WP user fields
// on auto-provision. Each falls back to '' when absent — the
// downstream resolver handles missing pieces (e.g. empty display_name
// falls back to email so the WP user row is never literally blank).
$resolved_id = cdcf_zitadel_bearer_resolve_user(
(string) $userinfo['sub'],
sanitize_email((string) $userinfo['email']),
is_string($userinfo['name'] ?? null) ? $userinfo['name'] : ''
[
'display_name' => is_string($userinfo['name'] ?? null) ? $userinfo['name'] : '',
'first_name' => is_string($userinfo['given_name'] ?? null) ? $userinfo['given_name'] : '',
'last_name' => is_string($userinfo['family_name'] ?? null) ? $userinfo['family_name'] : '',
Comment on lines +286 to +289
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Normalize the OIDC name claims before the empty-string checks.

These values come through as raw strings, so whitespace-only claims still pass the !== '' guards here. A token with name = ' ' or given_name = ' ' will write a blank display_name/first_name into wp_insert_user() instead of falling back to the email or omitting the key, which breaks the "don't blank WP defaults" behavior this PR is aiming for.

Proposed fix 
         [
-            'display_name' => is_string($userinfo['name'] ?? null) ? $userinfo['name'] : '',
-            'first_name'   => is_string($userinfo['given_name'] ?? null) ? $userinfo['given_name'] : '',
-            'last_name'    => is_string($userinfo['family_name'] ?? null) ? $userinfo['family_name'] : '',
+            'display_name' => is_string($userinfo['name'] ?? null) ? sanitize_text_field($userinfo['name']) : '',
+            'first_name'   => is_string($userinfo['given_name'] ?? null) ? sanitize_text_field($userinfo['given_name']) : '',
+            'last_name'    => is_string($userinfo['family_name'] ?? null) ? sanitize_text_field($userinfo['family_name']) : '',
         ]

Also applies to: 403-410

🤖 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
286 - 289, Normalize OIDC name claims by trimming whitespace before checking for
emptiness: for the array keys 'display_name', 'first_name', and 'last_name' in
zitadel-bearer.php (the block building the user data passed to
wp_insert_user()), call trim() on the incoming claims (e.g., $userinfo['name'],
$userinfo['given_name'], $userinfo['family_name']) and only use the value if the
trimmed string is non-empty; otherwise omit the key or fall back to the
email/default. Apply the same trim-and-empty check logic to the other similar
block around the 403-410 region so whitespace-only claims won't overwrite WP
defaults.

]
);
if ($resolved_id <= 0) {
return $user_id;
Expand Down Expand Up @@ -310,9 +318,17 @@ function cdcf_zitadel_bearer_authenticate($user_id) {
* password is generated and never surfaced — sign-in must always
* flow through Zitadel.
*
* The $profile array carries the OIDC profile claims used on
* auto-provision (display_name from `name`, first_name from
* `given_name`, last_name from `family_name`). Each key is a string,
* may be empty, and is ignored on the sub-hit / email-hit paths
* (those return an existing WP user we don't want to overwrite).
*
* See Phase 5 of ~/.claude/plans/cdcf-role-mirroring.md for the design.
*
* @param array{display_name:string,first_name:string,last_name:string} $profile
*/
function cdcf_zitadel_bearer_resolve_user(string $sub, string $email, string $display_name): int {
function cdcf_zitadel_bearer_resolve_user(string $sub, string $email, array $profile): int {
// 1. sub primary key.
$user = cdcf_zitadel_bearer_user_by_sub($sub);
if ($user instanceof WP_User) {
Expand All @@ -336,7 +352,7 @@ function cdcf_zitadel_bearer_resolve_user(string $sub, string $email, string $di
}

// 3. Auto-provision a Subscriber.
return cdcf_zitadel_bearer_auto_provision_subscriber($sub, $email, $display_name);
return cdcf_zitadel_bearer_auto_provision_subscriber($sub, $email, $profile);
}

/**
Expand Down Expand Up @@ -370,15 +386,29 @@ function cdcf_zitadel_bearer_user_by_sub(string $sub): ?WP_User {
* a duplicate.
*
* Returns the WP user id on success, 0 on hard failure.
*
* @param array{display_name:string,first_name:string,last_name:string} $profile
*/
function cdcf_zitadel_bearer_auto_provision_subscriber(string $sub, string $email, string $display_name): int {
$result = wp_insert_user([
function cdcf_zitadel_bearer_auto_provision_subscriber(string $sub, string $email, array $profile): int {
// Empty first/last/display are passed through unset rather than as
// '' so wp_insert_user uses its own defaults (and a missing
// display_name falls back to the email) instead of overwriting with
// blanks — keeps a partial-profile sign-up from producing a
// visibly-empty WP user row.
$args = [
'user_login' => $email,
'user_email' => $email,
'role' => 'subscriber',
'user_pass' => wp_generate_password(64, true, true),
'display_name' => $display_name !== '' ? $display_name : $email,
]);
'display_name' => $profile['display_name'] !== '' ? $profile['display_name'] : $email,
];
if ($profile['first_name'] !== '') {
$args['first_name'] = $profile['first_name'];
}
if ($profile['last_name'] !== '') {
$args['last_name'] = $profile['last_name'];
}
$result = wp_insert_user($args);

if (is_wp_error($result)) {
// Race with a parallel auto-provision: try the sub primary
Expand Down
42 changes: 42 additions & 0 deletions wordpress/themes/cdcf-headless/tests/ZitadelBearerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,8 @@ public function test_auto_provision_uses_zitadel_name_for_display_name(): void
'email' => 'newauthor@example.org',
'email_verified' => true,
'name' => 'Jane Doe',
'given_name' => 'Jane',
'family_name' => 'Doe',
]));
Functions\when('get_user_by')->justReturn(false);
$captured = null;
Expand All @@ -540,6 +542,8 @@ function (array $args) use (&$captured): int {
cdcf_zitadel_bearer_authenticate(false);

$this->assertSame('Jane Doe', $captured['display_name']);
$this->assertSame('Jane', $captured['first_name']);
$this->assertSame('Doe', $captured['last_name']);
$this->assertSame('subscriber', $captured['role']);
}

Expand All @@ -565,6 +569,44 @@ function (array $args) use (&$captured): int {
cdcf_zitadel_bearer_authenticate(false);

$this->assertSame('nameless@example.org', $captured['display_name']);
// first_name / last_name omitted (not present as keys) when the
// Zitadel claims are absent — wp_insert_user keeps its defaults
// rather than writing literal empty strings to the WP user row.
$this->assertArrayNotHasKey('first_name', $captured);
$this->assertArrayNotHasKey('last_name', $captured);
}

public function test_auto_provision_passes_given_and_family_name_to_wp_insert_user(): void
{
// Given a sign-up where the Zitadel claims include given_name +
// family_name (but no aggregate `name`), wp_insert_user receives
// first_name + last_name so the WP user row isn't visibly empty.
// display_name still falls back to email when `name` is absent —
// composing it from given_name + family_name is a UI concern.
$_SERVER['HTTP_AUTHORIZATION'] = 'Bearer ' . $this->mintJwt();
$this->stubWp();
Functions\when('wp_remote_post')->justReturn($this->buildUserinfoResponse(200, [
'sub' => 'zitadel-sub-named',
'email' => 'named@example.org',
'email_verified' => true,
'given_name' => 'Sam',
'family_name' => 'Park',
]));
Functions\when('get_user_by')->justReturn(false);
$captured = null;
Functions\when('wp_insert_user')->alias(
function (array $args) use (&$captured): int {
$captured = $args;
return 52;
}
);
Functions\when('wp_generate_password')->justReturn('random');

cdcf_zitadel_bearer_authenticate(false);

$this->assertSame('Sam', $captured['first_name']);
$this->assertSame('Park', $captured['last_name']);
$this->assertSame('named@example.org', $captured['display_name']);
}

public function test_auto_provision_race_recovers_via_sub_relookup(): void
Expand Down