feat(auth): Phases 1b + 2 + 3 of #2 — bearer validator (multi-aud per #173), Auth.js v5, my-team-member REST#172
Merged
Conversation
Phase 1b of cdcf-website issue #2 (team-member bio self-edit). Hooks determine_current_user at priority 20 to accept Authorization: Bearer <access_token> headers issued by the umbrella Zitadel instance at https://auth.catholicdigitalcommons.org. Validates each token against the /oidc/v1/userinfo endpoint and resolves the WP user by the email_verified claim. Behaviour: - Falls through (returns the prior filter's $user_id unchanged) on any unhappy path — non-200 userinfo, malformed JSON, unverified email, no matching WP user, network error. Application Passwords, cookies, and other auth methods keep working untouched. - Caches accepted (sha256(token) → user_id) pairs in a 60-second transient. The cache key is the SHA-256 hash so raw tokens never land in the WP options table. - No auto-provisioning: a Zitadel user whose email doesn't match an existing WP user falls through. A Zitadel admin manually creates the WP user (already supported via /cdcf/v1/create-user) and the author_team_member link (already supported via PR #160's endpoint) before the user can log in to WP REST. The wp_remote_post → /oidc/v1/userinfo round-trip is the trust anchor for now. A future PR could switch to local JWKS-cached signature verification (the pattern LitCal uses per ZitadelService.php) to save the round-trip — out of scope for issue #2's hot path, which is a human clicking Save on a bio edit form. 13 PHPUnit tests cover: early-exit (already authenticated, no header, non-Bearer scheme), cache hit, valid token + verified email + existing WP user, email_verified=false, missing email claim, no WP user match, userinfo 401, malformed JSON, network WP_Error, sha256 cache-key non-leakage of raw token, REDIRECT_HTTP_AUTHORIZATION fallback. Full theme suite still green (406 tests, 950 assertions). Hook wiring lives in functions.php (alongside the require_once) rather than in the include itself — that's the convention every other handler follows, and it keeps the include test-loadable without needing the Brain Monkey wp-hook shim. AGENTS.md (and CLAUDE.md via the symlink) documents the new auth mechanism in a "Zitadel bearer authentication" subsection of the existing cdcf/v1 endpoints chapter. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughAdds Auth.js v5 NextAuth setup with JWT refresh and session helpers, a client AuthButton integrated into the header, WordPress Zitadel bearer-token authenticator with audience allow-list and caching, and GET/PATCH REST endpoints for users to view/edit their team-member bio across Polylang translations, with tests and docs. ChangesZitadel OIDC Authentication and Team Member Self-Edit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
🚥 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 |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 173 |
| Duplication | 0 |
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.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…property replay Address HIGH-severity security review finding on PR #172: the bearer validator was accepting any Zitadel-issued token whose userinfo claim matched a WP user's email — even tokens minted for sibling umbrella properties (LiturgicalCalendar, OntoKit, BibleGet). Attack chain blocked by this commit: 1. Attacker authenticates against LitCal (same Zitadel instance), receives an access token with aud=<litcal-client-id>. 2. Attacker replays the token to cdcf-website: Authorization: Bearer <litcal-token> 3. Pre-fix: /oidc/v1/userinfo returns 200 with email_verified=true and the attacker's email. The umbrella uses UserEmailAsUsername=true so login names are globally unique emails. If a WP user exists with that email, the attacker is authenticated as them. 4. Post-fix: audience check at the top of the validator rejects the token before the userinfo round-trip because aud ≠ CDCF_ZITADEL_EXPECTED_AUD. Implementation: - New constant CDCF_ZITADEL_EXPECTED_AUD (defaults to '', defined in zitadel-bearer.php). Must be set in wp-config.php to the CDCF Website Web app's client ID emitted by cdcf-infra's setup-zitadel.sh --provision-cdcf-website handoff. Empty value fails every audience check closed — no Bearer auth works at all until the constant is configured. - New helper cdcf_zitadel_bearer_decode_jwt_payload() decodes the unverified JWT payload (signature verification stays delegated to the userinfo round-trip — a tampered payload would have failed the upstream signature check, so the unverified payload's claims are integrity-safe). - New helper cdcf_zitadel_bearer_audience_ok() does the actual match, accepting either a string or string[] aud claim per RFC 7519, using hash_equals for constant-time comparison. - Audience check runs BEFORE the cache lookup and userinfo POST — invalid-audience tokens never even consume a userinfo call. - Opaque (non-JWT) access tokens are now also rejected, since the audience check requires a parseable payload. cdcf-infra is already configured for OIDC_TOKEN_TYPE_JWT so this is the expected case. Tests: - New buildJwt helper in ZitadelBearerTest mints realistic test JWTs (header.payload.signature shape, payload base64url-encoded JSON). - All existing happy-path tests updated to use minted JWTs with matching aud so they continue to exercise the full pipeline. - 6 new tests: - wrong audience → fall through without userinfo call - missing aud claim → fall through - aud as string[] containing match → accepted (RFC 7519 array form) - opaque non-JWT token → fall through - audience helper rejects with empty expected (misconfigured deploy) - audience helper does exact-string match (no prefix/substring) - Test bootstrap pins CDCF_ZITADEL_EXPECTED_AUD before requiring the validator so the in-file defined()|define default no-ops. The helper-direct test covers the empty-expected branch since PHP constants can't be redefined at runtime. AGENTS.md (mirrored to CLAUDE.md via the symlink) documents the audience-verification step + the wp-config.php constant requirement prominently as the first bullet of the auth section. Theme suite: 412 tests, 959 assertions (up from 406 / 950). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
Follow-up commit The original validator only checked the userinfo response — which accepts any valid token issued by the umbrella Zitadel instance, including tokens minted for sibling properties (LiturgicalCalendar / OntoKit / BibleGet). Combined with Fix: Audience verification of the JWT's
Tests: 6 new tests covering wrong-aud, missing-aud, Suite: 412 tests, 959 assertions — was 406 / 950 in the original commit. Follow-up needed on cdcf-infra side: PR #11's |
Implements Phase 2 of cdcf-bio-edit-zitadel plan. The Next.js frontend
can now initiate the Zitadel OIDC sign-in flow, holds an access/refresh
token pair in a JWT session, exposes it to server components via auth(),
and surfaces sign-in/out controls in the header.
No user-facing protected routes yet — those land in Phase 4 with the
/my-bio page. Phase 2's UI contribution is just the header button
flipping between "Sign in" (unauthenticated) and an email/sign-out
dropdown (authenticated).
Files added
-----------
- lib/auth.ts — NextAuth({...}) config: Zitadel provider with
offline_access + the project:roles scope, JWT
session strategy, refresh-token rotation against
/oauth/v2/token, roles + locale claim extraction.
Session type augmentation inline so this file is
the single source of truth for the exposed shape.
- lib/auth-utils.ts — Server-only helpers: requireSession() (redirects
to /api/auth/signin), requireRole(role) (throws
403-shaped Error), getAccessToken(session)
(returns undefined when session has refresh
error).
- lib/auth-utils.test.ts — 10 Vitest tests covering all three helpers.
- app/api/auth/[...nextauth]/route.ts — `export { GET, POST } from handlers`.
- components/AuthButton.tsx — Client component salvaged from the deferred
feature/zitadel-integration branch with
CodeRabbit-fix-equivalent cleanups already
applied: stable closeTimeout cleanup, ARIA on
the dropdown, keyboard nav, no /profile link
(the bio editor route lands in Phase 4).
Files modified
--------------
- app/[lang]/layout.tsx — auth() called server-side in parallel with
getMessages(), session passed into
<SessionProvider> wrapping the existing
NextIntlClientProvider.
- components/Header.tsx — <AuthButton /> rendered between
<LanguageSwitcher /> and the donate CTA.
- messages/{en,it,es,fr,pt,de}.json — New top-level "Auth" block:
signIn / signOut / loading.
- .env.local.example — AUTH_ZITADEL_ID/SECRET/ISSUER + AUTH_SECRET
with provenance + generation hints.
- AGENTS.md (→ CLAUDE.md symlink) — Environment-variables section now lists the
new AUTH_* vars and cross-links the bearer-
validator section's CDCF_ZITADEL_EXPECTED_AUD
constant (same client_id value, different
file: wp-config.php vs Next.js env).
- package.json / package-lock.json — next-auth@5.0.0-beta.31 added.
Design choices and what was deliberately NOT done
-------------------------------------------------
- Dropped the deferred branch's `events.signIn` WP user sync — that hit
a non-existent /cdcf/v1/sync-user endpoint and used Application
Password auth. Per locked decision #1 of the plan, a Zitadel admin
manually creates WP users on email drift; no auto-provisioning.
- No teamMemberId on the session yet. The plan called for a
cdcfWpProfile JWT callback that hits /cdcf/v1/my-team-member to
cache the linked post id — but that endpoint doesn't exist yet
(Phase 3). Phase 4's edit page will resolve it server-side per
request. The session carries accessToken + roles + locale, which is
all that's needed for the sign-in flow.
- AuthButton only shows "Sign in" / email-dropdown / sign-out. The
"Edit my bio" entry lands in Phase 4 once we have a route to point
it at.
Verification
------------
- npm run build: clean, /api/auth/[...nextauth] route registered.
- npm run lint: clean.
- npm test: 134 pass (was 124 — +10 auth-utils tests).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JohnRDOrazio
changed the title
Member
Author
|
Phase 2 added in commit
Phase 2 surface area:
Deliberately not done in Phase 2 (per plan, deferred to later phases):
Verification: PR title updated to reflect the wider scope. |
6 tasks
…s + interactive-element a11y Two findings raised by Codacy on PR #172: [MEDIUM, noExplicitAny] lib/auth-utils.test.ts Two `as any` casts on session-shaped object literals inside the getAccessToken tests. Replaced with `Session`-typed declarations using the actual exported type from next-auth (now imported as a type-only import). The literal now includes the required `expires` field so it's a real Session, not a partially-typed cast. eslint- disable lines for no-explicit-any removed; they're no longer needed. [HIGH, jsx-a11y/no-static-element-interactions] components/AuthButton.tsx The wrapping <div className="relative"> carried onMouseEnter + onMouseLeave handlers — a static <div> with interactive behaviour fails the a11y rule. Moved both handlers onto the <button> (already interactive) and added a paired set to the <div role="menu"> (the menu role makes that element interactive too). Also added onFocus + onBlur to the button so keyboard users get the same open-on-focus / close-on-blur behaviour as mouse users. Functional behaviour is preserved on hover: cursor over button opens the menu, cursor over menu keeps it open (because the menu's own mouseEnter cancels the scheduled-close timer), cursor leaving either schedules close after 150ms. Verification: npm run build / lint / test all clean. 134 vitest tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
Codacy findings addressed in
Functional behaviour is preserved: hovering the trigger opens the menu, hovering the menu keeps it open (the menu's own
|
cdcf-infra PR #11 ultimately split the CDCF Website OIDC client into TWO confidential apps under the CDCF Org: - production: aud=<prod_client_id>, origin catholicdigitalcommons.org, devMode=false - non-prod: aud=<nonprod_client_id>, origins staging.* + http://localhost:3000, devMode=true The shared WP backend serves both frontends, so the single-value audience pin from PR #172 would have 401'd whichever client_id wasn't configured. Switch CDCF_ZITADEL_EXPECTED_AUD to a comma-separated allow-list and accept any token whose aud claim intersects with it. Validator changes ----------------- - New cdcf_zitadel_bearer_parse_allowed_auds($raw) parses the constant into a string[] — comma-split, whitespace-trimmed, empty entries dropped. Empty / whitespace-only / all-comma input returns []. - cdcf_zitadel_bearer_audience_ok() signature changed from (claims, string $expected) to (claims, array $allowed). Inner matcher still uses hash_equals for constant-time compare per entry; outer logic now does intersect-non-empty. - Filter callback parses the constant on every call and short-circuits on empty allow-list with the existing error_log line ("... not configured ... rejecting all bearer tokens"). Misconfigured- deploy behaviour unchanged: fail closed. - Header docblock on the constant explains the prod + non-prod split and gives the exact wp-config.php line shape. Tests ----- - bootstrap.php now defines the constant as the comma-separated form `'<prod>, <nonprod>'` so multi-aud integration tests work. - All existing audience_ok tests now pass the allow-list as an array. - New tests: * match against second allow-list entry (nonprod) * reject when token aud matches NONE of the allow-list * RFC 7519 aud-as-array + allow-list-as-array intersection * empty allow-list fails closed * parser: comma split, whitespace trim, empty/double-comma drop * parser: '', ' ', ',,,' all return [] * integration: nonprod aud token authenticates through full filter - Bearer-test count: 19 → 27. Full theme suite: 412 → 420. AGENTS.md (+CLAUDE.md symlink) updates the Zitadel bearer auth section to document the multi-aud allow-list semantics and the exact wp-config.php line shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ints
GET /cdcf/v1/my-team-member
PATCH /cdcf/v1/my-team-member/{lang}
The authenticated user's bio self-edit surface. Discovery resolves the
caller's author_team_member ACF link, pulls the full Polylang group,
and reports which language versions are editable. Edit accepts a
partial PATCH against any language in that group, persists to the
target post, and fans out re-translation to the OTHER 5 langs from
the just-saved source — reusing cdcf_enqueue_post_translation() so
all the existing redis-queue + Polylang-link + advisory-lock infra
works as-is.
Authorization model
-------------------
- Bearer-validated session via the Phase 1b authenticator (or cookie
/ Application Password for non-frontend callers).
- author_team_member ACF user-field is the canonical ownership signal.
It's admin-managed (set via /cdcf/v1/author-team-member, PR #160),
so end users can never widen their own access from inside this
endpoint.
- Permission gate: logged-in AND has any link → admits.
- Per-language gate inside the handler: the link must point at SOME
post in the {lang} target's Polylang group. Covers the case where
the admin set the link at the EN post but the user edits the DE
sibling.
Translation strategy (issue #2 locked decision #3)
--------------------------------------------------
- Edit in any language — no privileged "primary language". Whichever
language gets saved becomes the new source-of-truth for that cycle.
- Re-translation fan-out covers the other 5 langs in the group, NOT
including the just-saved one. Each enqueue is independent; a single
enqueue failure is recorded in `errors` without aborting siblings
(the just-saved source-language post is already persisted at that
point — we don't roll it back for a downstream queue blip).
- The OpenAI prompt reads source-lang from the source post (PR #171
fix), so editing in any locale produces correct translations.
URL host allowlist
------------------
- member_linkedin_url: linkedin.com (or any subdomain) or empty.
- member_github_url: github.com (or any subdomain) or empty.
- esc_url_raw sanitization runs first via the args block; the
hostname constraint can't be expressed there so it's enforced in
the handler body, returning 400 on violation.
- Suffix match is dot-boundary anchored — linkedin.com.evil.org
does NOT pass.
Partial PATCH semantics
-----------------------
- A field is updated only when the request actually carried a string
value for it (caught via `is_string($request->get_param(…))`). A
PATCH that only changes member_title does NOT clobber post_content.
- Empty string IS a valid value and clears the field (used by URL
fields and member_title alike).
Tests (27 new in MyTeamMemberHandlerTest)
-----------------------------------------
- resolve_link: scalar id / array of ids / WP_Post / false / anonymous
- collect_group: normal / fallback to pll_get_post_language / invalid
- url_host_ok: empty / exact / subdomain / unrelated / suffix-
lookalike / malformed
- permission: 401 anonymous / 403 no link / true linked
- GET happy: full group with two languages
- GET filter: polluted group with non-team_member post stripped
- PATCH happy: DE edit fans out to en/it/es/fr/pt, all three ACF
fields written to the DE post, wp_update_post
carries the right ID + content
- PATCH partial: member_title-only PATCH leaves wp_update_post
uncalled
- PATCH 404: lang not in Polylang group
- PATCH 400: LinkedIn URL pointing at evil.example.org
- PATCH 400: GitHub URL pointing at gitlab.com
- PATCH 500: wp_update_post WP_Error bubbles up
- PATCH partial-fail: one enqueue WP_Error recorded, siblings still
queued, no exception
- PATCH ownership: link points outside the resolved Polylang group →
403
Test bootstrap also gains a minimal WP_Post shim (4 fields) and the
new handler include is required-up-front like the others.
Theme suite: 420 → 447 tests (+27), 970 → 1033 assertions (+63).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JohnRDOrazio
changed the title
Member
Author
|
Phase 3 + issue #173 added. PR now covers:
Issue #173 fix (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
AGENTS.md (1)
196-204: 💤 Low valueFix list formatting.
Add a blank line before the parameter list to satisfy Prettier and Codacy markdown linting rules. This will be auto-fixed by running
prettier --write AGENTS.md(as indicated by the pipeline failure).🤖 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 `@AGENTS.md` around lines 196 - 204, The markdown in AGENTS.md is failing lint because the parameter list (the bullet list of `content`, `member_title`, `member_linkedin_url`, `member_github_url`, etc.) lacks a blank line before it; open AGENTS.md and insert a single blank line above the parameters bullet list so Prettier/Codacy markdown rules are satisfied, then run `prettier --write AGENTS.md` (or stage the file) to apply formatting and clear the pipeline failure.package.json (1)
25-25: ⚡ Quick winPrefer exact pin for
next-authbeta to avoid accidental upgrade drift.Using
^5.0.0-beta.31can pull newer prerelease/stable versions unexpectedly. Pinning the exact beta helps keep auth behavior deterministic until you intentionally upgrade.Suggested change
- "next-auth": "^5.0.0-beta.31", + "next-auth": "5.0.0-beta.31",🤖 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 `@package.json` at line 25, The dependency entry for next-auth uses a caret range ("next-auth": "^5.0.0-beta.31") which allows accidental prerelease/stable upgrades; update the package.json entry for next-auth to an exact version string ("next-auth": "5.0.0-beta.31") and then regenerate the lockfile (run npm install or yarn install) so the lockfile reflects the pinned beta; locate the dependency in package.json to make this change.
🤖 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 @.env.local.example:
- Around line 52-65: Update the .env comment near
AUTH_ZITADEL_ID/AUTH_ZITADEL_SECRET to add a short cross-reference that
wp-config.php must set the CDCF_ZITADEL_EXPECTED_AUD constant to a
comma-separated allow-list of client IDs (multi-audience) rather than a single
AUTH_ZITADEL_ID; mention wp-config.php and reference AGENTS.md (around line 212)
for details so deployers know to include prod+nonprod client IDs.
In `@AGENTS.md`:
- Around line 298-299: The doc incorrectly instructs setting
CDCF_ZITADEL_EXPECTED_AUD to a single AUTH_ZITADEL_ID; instead update the
guidance so CDCF_ZITADEL_EXPECTED_AUD is documented as a comma-separated
allow-list of all client IDs (e.g., prod and nonprod) that the shared WordPress
backend should accept; mention that each frontend still uses its own
AUTH_ZITADEL_ID in its .env.* files and that wp-config.php must include the
combined CDCF_ZITADEL_EXPECTED_AUD value to avoid rejecting valid bearer tokens
from other environments.
In `@components/AuthButton.tsx`:
- Around line 89-90: The blur handler currently calls scheduleClose which starts
a timer and never gets cleared when focus moves into the dropdown, so keyboard
users can lose the menu before activating items; add a corresponding focus
handler (e.g., onFocus or onMouseEnter) that cancels the scheduled close
(implement/rename functions like scheduleClose and cancelScheduledClose or
clearCloseTimer) and attach it to both the trigger and the dropdown/menu
container (including the menu item list used around handleKeyDown) so any focus
transition into the menu clears the timer and prevents premature closing; ensure
cancelScheduledClose is invoked on focus and that scheduleClose is still used on
blur to preserve delayed close behavior.
In `@lib/auth.ts`:
- Around line 46-55: The refresh-token HTTP request in lib/auth.ts (the fetch
call that posts to `${issuer}/oauth/v2/token` using token.refreshToken) needs an
explicit timeout: create an AbortController, start a timer (e.g., setTimeout)
that calls controller.abort() after the desired timeout, pass controller.signal
into the fetch options, and clear the timer after fetch completes; ensure fetch
aborts propagate so the existing error handling falls back to
RefreshAccessTokenError. Update the fetch invocation and surrounding logic (the
block that builds headers/body and awaits the response) to use this
AbortController pattern.
In `@wordpress/themes/cdcf-headless/functions.php`:
- Around line 701-713: Add a 'lang' entry to the args array passed to
register_rest_route for the 'cdcf/v1' PATCH '/my-team-member/(?P<lang>[a-z]{2})'
route and supply a sanitize_callback (e.g., sanitize_text_field or a stricter
two-letter validator) so the path param is validated like other request fields;
update the args block alongside 'content', 'member_title', etc., ensuring
cdcf_rest_update_my_team_member continues to read $request['lang'] but now
receives a sanitized value via the route declaration.
In `@wordpress/themes/cdcf-headless/includes/auth/zitadel-bearer.php`:
- Around line 216-218: The transient cache check uses is_int($cached) so numeric
strings (common from DB-backed transients) miss hits; update the logic around
get_transient and $cached to treat numeric-string hits as valid by checking
is_numeric($cached) (or ctype_digit if you prefer) and then cast to int before
returning — e.g., replace the is_int($cached) && $cached > 0 guard with a
numeric check and return (int)$cached to restore the intended 60s caching
behavior in the get_transient/$cached branch.
In `@wordpress/themes/cdcf-headless/includes/handlers/my-team-member.php`:
- Around line 197-217: The Polylang group entry at $group[$requested_lang] must
be validated before trusting it for updates: fetch the post with
get_post($group[$requested_lang]) (or get_post($target_post_id)) and verify it
exists and its post_type === 'team_member' (and optionally
current_user_can('edit_post', $target_post_id)) before proceeding to call
wp_update_post(); if the post is missing or has the wrong post_type, return a
WP_Error (similar to the existing rest_no_translation_for_lang/rest_forbidden
responses) to reject polluted/stale translation entries; keep the existing
ownership in_array($linked_id, $group, true) check as an additional guard.
- Around line 239-283: This handler currently enqueues re-translation jobs even
when the PATCH supplies no mutable fields; detect whether any of the target
fields were actually provided before persisting or queuing: check
request->get_param('content') and each of request->get_param for 'member_title',
'member_linkedin_url', 'member_github_url' and set a boolean like $did_update
(or $has_changes) if any is a string and an update will be performed (or
succeeds); after applying updates with wp_update_post and update_field
(functions referenced in this diff), if $did_update is false then skip the
fan-out loop that calls cdcf_enqueue_post_translation and simply return early
(or return success without queuing), preventing no-op PATCHes from enqueueing
translation jobs for other entries in $group for $other_lang !==
$requested_lang.
---
Nitpick comments:
In `@AGENTS.md`:
- Around line 196-204: The markdown in AGENTS.md is failing lint because the
parameter list (the bullet list of `content`, `member_title`,
`member_linkedin_url`, `member_github_url`, etc.) lacks a blank line before it;
open AGENTS.md and insert a single blank line above the parameters bullet list
so Prettier/Codacy markdown rules are satisfied, then run `prettier --write
AGENTS.md` (or stage the file) to apply formatting and clear the pipeline
failure.
In `@package.json`:
- Line 25: The dependency entry for next-auth uses a caret range ("next-auth":
"^5.0.0-beta.31") which allows accidental prerelease/stable upgrades; update the
package.json entry for next-auth to an exact version string ("next-auth":
"5.0.0-beta.31") and then regenerate the lockfile (run npm install or yarn
install) so the lockfile reflects the pinned beta; locate the dependency in
package.json to make this change.
🪄 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: 1f5e460e-75ed-41f1-97fb-f7c9a185bf1b
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (22)
.env.local.exampleAGENTS.mdapp/[lang]/layout.tsxapp/api/auth/[...nextauth]/route.tscomponents/AuthButton.tsxcomponents/Header.tsxlib/auth-utils.test.tslib/auth-utils.tslib/auth.tsmessages/de.jsonmessages/en.jsonmessages/es.jsonmessages/fr.jsonmessages/it.jsonmessages/pt.jsonpackage.jsonwordpress/themes/cdcf-headless/functions.phpwordpress/themes/cdcf-headless/includes/auth/zitadel-bearer.phpwordpress/themes/cdcf-headless/includes/handlers/my-team-member.phpwordpress/themes/cdcf-headless/tests/MyTeamMemberHandlerTest.phpwordpress/themes/cdcf-headless/tests/ZitadelBearerTest.phpwordpress/themes/cdcf-headless/tests/bootstrap.php
Eight findings resolved, all verified against current code; one was already an addressed-elsewhere placeholder so the instruction note below records what's unchanged. [lint-markdown] AGENTS.md MD032 (blanks-around-lists) on the Phase 3 Parameters list. Inserted the required blank line and ran prettier --write to converge the Method/Route column widths the new PATCH row had widened. [multi-aud docs miss] AGENTS.md L298 The Environment Variables section still said "the same AUTH_ZITADEL_ID value also needs to be set as CDCF_ZITADEL_EXPECTED_AUD" — predates the multi-aud commit 99ebefe. Now correctly documents the comma- separated allow-list and that each frontend pins its OWN AUTH_ZITADEL_ID while wp-config.php's CDCF_ZITADEL_EXPECTED_AUD carries BOTH client IDs. Same fix applied to .env.local.example with a back-pointer to AGENTS.md. [a11y] components/AuthButton.tsx onBlur on the trigger button scheduled a 150ms close timer, but the menu <div role="menu"> only had mouse handlers — keyboard tabbing from button into a menu item let the timer fire before any onFocus could cancel it. Added onFocus/onBlur to the menu div; React's onFocus bubbles so item focus reaches the wrapper and clears the timer. Mouse paths unchanged. [auth resilience] lib/auth.ts Refresh-token POST to /oauth/v2/token had no client-side timeout — an unreachable Zitadel could stall a full page render at every expiry. Wrapped in an AbortController with a 5s cap; the existing catch path covers timeout + network errors uniformly via RefreshAccessTokenError. clearTimeout in finally so the timer doesn't keep the event loop alive after success. [sanitization convention] functions.php The PATCH route's {lang} URL param was being validated only by the inline regex in the path pattern. Declared it in the args block per AGENTS.md's "Every cdcf/v1 route declares its sanitize_callback per field" convention: sanitize_callback => sanitize_key, validate_callback => /^[a-z]{2}$/ regex. Defense in depth — the URL pattern already filters, but the args block is the canonical point of validation declaration. [cache regression] includes/auth/zitadel-bearer.php is_int($cached) on the get_transient hit missed numeric-string shapes the default DB-backed transients can return for int values (object-cache deployments preserve the int type; the default doesn't). Replaced with is_numeric($cached) && (int) $cached > 0 → return (int) $cached, restoring the 60s cache in the no-object-cache case. [stale-group safety] includes/handlers/my-team-member.php (PATCH) $group[$requested_lang] was trusted as a valid team_member id. A polluted Polylang group (post deleted, or wrong CPT linked into the same translation term) would route an edit to a stale post id. Added a get_post + post_type check after the in_array ownership invariant; mismatch returns rest_no_translation_for_lang/404. Mirrors the GET endpoint's existing filter. [no-op fan-out] includes/handlers/my-team-member.php (PATCH) A PATCH with no mutable field supplied was still enqueueing five re-translation jobs that would produce identical OpenAI output. Track $did_update across the content + ACF-field writes; when no field was actually supplied, return the same envelope shape (with empty queued + errors) and skip the fan-out loop entirely. [stable pin] package.json next-auth caret range "^5.0.0-beta.31" was admitting accidental prerelease/stable upgrades — risky on a pinned beta. Switched to the exact "5.0.0-beta.31" and regenerated package-lock.json. Tests added (3 new in MyTeamMemberHandlerTest, 450 total) - patch_rejects_when_target_post_missing_or_wrong_type - patch_rejects_when_target_post_is_wrong_cpt - patch_no_op_skips_fan_out stubCommon() now stubs get_post with a fake team_member default so the existing PATCH tests (which don't care about the validity branch) keep passing without per-test stub additions. Verification - Theme PHPUnit: 450/450, 1041 assertions - Vitest: 134/134 - ESLint: clean - markdownlint: 0 errors - prettier check: clean - next build: clean Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
CodeRabbit findings + markdown-lint addressed in Verified each finding against current code; all eight were still valid. One follow-on doc fix (AGENTS.md L298 was outdated post-multi-aud) caught in the same pass.
3 new tests in Verification: |
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
Phase 1b of issue #2. Adds a
determine_current_userfilter that acceptsAuthorization: Bearer <access_token>headers issued by the umbrella Zitadel athttps://auth.catholicdigitalcommons.org, validates them against the userinfo endpoint, and resolves the WP user by the verified-email claim.Pure infrastructure — no new REST endpoints, no new UI, no behavior change for any existing caller. The bearer validator only fires when a
Bearertoken is present and no earlier filter (cookie auth, Application Password) has already populated$user_id. Application Passwords stay the canonical auth method for the Python CLI and other server-side callers.Auth flow
PATCH /cdcf/v1/my-team-member/{lang}once Phase 3 lands — sends it asAuthorization: Bearer <token>to WPauth.catholicdigitalcommons.org/oidc/v1/userinfowith a 5s timeout200+email_verified=true+ matching WP user →$user_idset, REST request proceeds under that user's caps(sha256(token) → user_id)in a 60s transient — raw tokens never enter the WP options tableFail-closed semantics
Every unhappy path returns
$user_idunchanged:Authorizationheader, or non-Bearer scheme → no-op (Application Passwords still work)WP_Errorfromwp_remote_post→ fall throughemail_verified=falseoremailclaim missing → fall throughFiles
wordpress/themes/cdcf-headless/includes/auth/zitadel-bearer.php(new)CDCF_ZITADEL_USERINFO_URL,CDCF_ZITADEL_BEARER_CACHE_TTL=60,CDCF_ZITADEL_USERINFO_TIMEOUT=5), header extraction helper coveringHTTP_AUTHORIZATION+REDIRECT_HTTP_AUTHORIZATION+getallheaders()fallbackwordpress/themes/cdcf-headless/tests/ZitadelBearerTest.php(new)wordpress/themes/cdcf-headless/functions.phprequire_once+add_filter(..., 20)registration. Priority 20 keeps this AFTER core's cookie and Application Password resolvers.wordpress/themes/cdcf-headless/tests/bootstrap.phpAGENTS.mdcdcf/v1chapter (mirrored toCLAUDE.mdvia the symlink)Why hook registration is in functions.php, not the include itself
Convention every other handler file follows: includes only define functions;
add_filter/add_action/register_rest_routecalls live infunctions.php. Keeps the include test-loadable without stubbing Brain Monkey's WP hook shim in bootstrap.Tests
13 new PHPUnit tests in
ZitadelBearerTest.php:Authorizationheader (no userinfo call);Basicscheme (no userinfo call)email_verified=falsefalls through; missing email claim falls through; no matching WP user falls through; userinfo 401 falls through; malformed JSON falls through;WP_Errorfromwp_remote_postfalls throughREDIRECT_HTTP_AUTHORIZATIONwhenHTTP_AUTHORIZATIONis empty (Apache+FCGI case)Full theme suite locally:
(Was 393 tests before Phase 1b; +13 new = 406.)
Future-proofing not done here
LiturgicalCalendarAPI/src/Services/ZitadelService.phpdoes JWKS validation). For issue frontend/backend login via Zitadel provider #2's hot path (human clicks Save on a bio edit form, not a high-RPS API), the round-trip + 60s cache is fine. Worth revisiting if/when the bearer path serves higher-frequency reads.Dependencies
falsefor every token until PR Please remove atomicmail.io from disposable list (legitimate permanent email service) #11 lands and a user is actually issued an access token, which is the correct fail-closed behaviour.Deploy
Theme change — needs
gh workflow run deploy.yml -f environment=productionto ship. Astaging-target run skips theme/plugin steps.Test plan
cdcf/v1endpoint withAuthorization: Bearer junk— confirm 401 (token validation fails, no other auth attempted)Authorization: Basic <appwd>— confirm Application Password auth still works as before (bearer validator no-ops on non-Bearer schemes)Summary by CodeRabbit
New Features
Documentation