Skip to content

Add the Martyrology Frontend OIDC login app - #19

Merged
JohnRDOrazio merged 10 commits into
mainfrom
feat/martyrology-frontend-oidc-client
Aug 3, 2026
Merged

Add the Martyrology Frontend OIDC login app#19
JohnRDOrazio merged 10 commits into
mainfrom
feat/martyrology-frontend-oidc-client

Conversation

@JohnRDOrazio

@JohnRDOrazio JohnRDOrazio commented Aug 3, 2026

Copy link
Copy Markdown
Member

Closes the provisioning half of CatholicOS/martyrology-api#26No OIDC client exists for human login.

Why

The MartyrologyAPI project contains exactly one app, an API-type validator the API uses to call /oauth/v2/introspect. It has no grant types and no redirect URIs, so no human can obtain a token. Zitadel issues PATs to machine users only, so there is no fallback. v0.2.0's role gate and licensing gate both work, but the licensing gate is currently reachable only in its deny direction.

What's here

do_provision_martyrology_frontend and its --provision-martyrology-frontend action, creating one confidential OIDC Web app — Martyrology Frontend, https://romanmartyrology.com, devMode=false, client_secret_post. Plus a design doc and implementation plan under docs/superpowers/.

The app goes in the existing MartyrologyAPI project rather than a new one: same-project membership is what puts the roles claim in the token without requesting an :aud scope. The function resolves the project read-only via find_project_id and exits 16 if it is absent — it never creates one. create_oidc_web_app is unchanged.

Why a confidential client rather than PKCE

martyrology-frontend already runs a backend-for-frontend: the browser calls same-origin /api/mr/... and the Next.js server proxies to the API. Attaching a bearer token there is an addition to that architecture. A browser-held token would instead require adding CORS to martyrology-api — which currently has none configured anywhere — and reworking the proxy, in exchange for putting a token that unlocks licensed 2004-edition text into browser-reachable storage.

Reversed mid-review: no localhost client

This branch originally provisioned a second devMode=true app for http://localhost:3000, following CDCF Website. That was the weaker of the two available precedents and has been reverted.

LITCAL_FRONTEND_URLS (auth/setup-zitadel.sh:309-312) registers production and staging only. LiturgicalCalendar does local development against an entirely separate Zitadel — Postgres, Zitadel, OpenFGA and the API in LiturgicalCalendarFrontend/docker-compose.yml, provisioned by its own scripts/setup-zitadel.sh. Martyrology now follows LitCal: production Zitadel carries production URIs only.

Accepted consequence: there is no local sign-in for martyrology-frontend until a Martyrology local stack exists. That is its own design cycle. _emit_martyrology_frontend_app keeps its dev_mode parameter so that design can reuse it against a local Zitadel.

Not yet done

This provisions the client; it does not wire up sign-in. The frontend work is Tasks 3-7 of the plan, which are flagged stale in the plan itself — their code and unit tests are unaffected, but four live verification steps assumed a localhost client and need revising once the local-stack design lands.

The verification gate has not run yet. It checks the design's one unverified assumption: that martyrology-api, which introspects using the API app's credentials, accepts a token minted by a sibling app in the same project. Merging this PR is what puts the new action on the VPS — sync-to-vps.yml pulls --ff-only from main, so a feature branch never reaches /opt/cdcf-auth. The action is inert until invoked, so merging ahead of the gate carries no risk. The gate does not need a dev client: the manual flow uses the production redirect URI, which 404s while nothing is deployed there.

Review notes

Code reviewed clean twice — once as originally written, once after the reversal, both times against the live file rather than the diff alone. The second review confirmed no trace of the dev app remains in the code path and that the documentation's claims about LITCAL_FRONTEND_URLS and LitCal's docker stack are actually true. It also found stale two-app language in the docs, since fixed in 7525b51.

Deferred minors: the top-of-file # Actions: comment block doesn't list the new action (usage() does), and the === Martyrology Frontend shared values === output header reads oddly now that only one block follows it.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added production authentication setup for the Martyrology website, enabling secure sign-in and sign-out through the existing identity service.
    • Added session handling, token refresh, and authenticated requests while preserving anonymous access behavior.
    • Added visible authentication status and account controls to the website.
  • Documentation
    • Added implementation and design documentation covering authentication, deployment configuration, verification, and secret management.

JohnRDOrazio and others added 4 commits August 3, 2026 17:52
martyrology-api issue #26: the MartyrologyAPI project holds only an API-type
validator app, so no human can obtain a token. Zitadel issues PATs to machine
users only, leaving no fallback.

Design: two confidential OIDC Web apps (production + dev) in the existing
MartyrologyAPI project, with Auth.js v5 sign-in wired into martyrology-frontend's
existing server-side proxy. Chosen over a PKCE public client because the frontend
already runs a backend-for-frontend, so the token never reaches the browser and
martyrology-api needs no CORS configuration — it needs no changes at all.

Sequencing puts the one unverified assumption first: that the API accepts a
token minted by a sibling app in the same project.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Seven tasks across cdcf-infra and martyrology-frontend. Task 2 is a gate: it
retires the design's one unverified assumption — that the API accepts a token
minted by a sibling app in the same project — before any frontend code is
written.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two confidential Web apps (production + localhost dev) in the existing
MartyrologyAPI project, so the roles claim needs no :aud scope. Mirrors
_emit_cdcf_app, which solves the same prod/non-prod secret-separation
problem for the CDCF website.
Step 1 was written with a local repo path, but setup-zitadel.sh only works on
the VPS: it reaches Zitadel over loopback, reads the PAT from
/opt/cdcf-auth/runtime, and sources .env.production, which is mode 0600 and
never leaves the box. Steps 2-8 touch only public endpoints and a local
browser.

Also records the prerequisite the sync workflow imposes: it pulls --ff-only
from main, so a feature branch never reaches /opt/cdcf-auth.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@JohnRDOrazio, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 48 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 875324e3-7241-4e26-827c-5961d5bcec1a

📥 Commits

Reviewing files that changed from the base of the PR and between c3e6bde and 5dd6304.

📒 Files selected for processing (2)
  • docs/superpowers/plans/2026-08-03-martyrology-oidc-login-client.md
  • docs/superpowers/specs/2026-08-03-martyrology-oidc-login-client-design.md
📝 Walkthrough

Walkthrough

The PR adds production-only Martyrology frontend OIDC provisioning to auth/setup-zitadel.sh. It also adds design and implementation plans for Auth.js authentication, token refresh, authenticated proxying, deployment, testing, and operational handoff.

Changes

Martyrology OIDC frontend

Layer / File(s) Summary
OIDC architecture and scope
docs/superpowers/specs/..., docs/superpowers/plans/...
The design defines one confidential production client, server-side Auth.js sessions, token refresh, bearer forwarding, anonymous access, and excluded API changes.
Production frontend client provisioning
auth/setup-zitadel.sh, docs/superpowers/plans/...
The script validates the Martyrology organization and project, creates or updates the production Web OIDC app, registers callback and logout URIs, emits credentials, and wires CLI and --all support.
Authentication and proxy validation
docs/superpowers/plans/..., docs/superpowers/specs/...
The plan defines token refresh, session handling, authenticated proxy requests, sign-in states, anonymous behavior, and validation flows.
Deployment and operational documentation
docs/superpowers/plans/..., docs/superpowers/specs/...
The documents define secret handling, issuer configuration, provider smoke checks, deployment acceptance checks, stale local-client tasks, handoff, and self-review tracking.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related issues

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: adding the Martyrology Frontend OIDC login app.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/martyrology-frontend-oidc-client

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 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 `@docs/superpowers/plans/2026-08-03-martyrology-oidc-login-client.md`:
- Around line 184-187: Update the fenced block containing
--provision-martyrology-frontend with a text language identifier on its opening
fence to satisfy the MD040 lint requirement.
- Around line 1276-1289: The providers endpoint check in
docs/superpowers/plans/2026-08-03-martyrology-oidc-login-client.md lines
1276-1289 must be described as provider-registration verification only; add a
post-deploy authorization-code callback check or deployment-side Plesk
environment validation that verifies the client secret. In
docs/superpowers/specs/2026-08-03-martyrology-oidc-login-client-design.md lines
215-219, remove the claim that provider discovery verifies manually configured
secrets.
- Line 637: Revise the client-authentication checkpoint to make the
`Zitadel({...})` override conditional: if Task 2 confirms HTTP Basic succeeds,
reconcile the Zitadel client configuration and do not add `client: {
token_endpoint_auth_method: "client_secret_post" }`; add that override only when
form-field authentication succeeds but Auth.js defaults to HTTP Basic, and
verify the result at Step 9.
- Around line 627-631: Update the session callback around `session({ session,
token })` to stop assigning `current.access_token` to `session.accessToken`.
Return only non-sensitive session fields, while keeping the access token
available exclusively through the server-side JWT callback.
🪄 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 Plus

Run ID: 5d8f2d52-ea7c-4a29-9081-053654b69cc3

📥 Commits

Reviewing files that changed from the base of the PR and between ff969b3 and d259569.

📒 Files selected for processing (3)
  • auth/setup-zitadel.sh
  • docs/superpowers/plans/2026-08-03-martyrology-oidc-login-client.md
  • docs/superpowers/specs/2026-08-03-martyrology-oidc-login-client-design.md

Comment thread docs/superpowers/plans/2026-08-03-martyrology-oidc-login-client.md Outdated
Comment thread docs/superpowers/plans/2026-08-03-martyrology-oidc-login-client.md
});
```

**Checkpoint — client authentication method.** The built-in Zitadel provider must agree with the app's `OIDC_AUTH_METHOD_TYPE_POST`. Task 2 Step 4 established which form the live app accepts. If Task 2 found that HTTP Basic was required rather than form fields, add `client: { token_endpoint_auth_method: "client_secret_post" }` to the `Zitadel({...})` call to force the form-field variant, since `_POST` is what we provisioned. Verify at Step 9 rather than assuming.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

For next-auth@5.0.0-beta.31 and the built-in Zitadel provider, how is token_endpoint_auth_method: "client_secret_post" configured, and what authentication method does the provider use by default?

💡 Result:

In next-auth@5.0.0-beta.31 (part of the Auth.js v5 ecosystem), you configure the token endpoint authentication method by adding the client property to the provider configuration [1][2][3]. To use client_secret_post, you should structure your Zitadel provider as follows: import Zitadel from "next-auth/providers/zitadel"; export const { handlers, auth, signIn, signOut } = NextAuth({ providers: [ Zitadel({ clientId: process.env.AUTH_ZITADEL_ID, clientSecret: process.env.AUTH_ZITADEL_SECRET, client: { token_endpoint_auth_method: "client_secret_post", }, }), ], }); Regarding the default behavior, the built-in Zitadel provider in Auth.js inherits its client configuration from the underlying OpenID Connect (OIDC) implementation [4]. By default, Auth.js providers that follow the OIDC specification typically default to client_secret_basic [2][5]. While Zitadel supports both client_secret_basic and client_secret_post as authentication methods [5], it expects the method configured in your application to match the method registered in your ZITADEL console for that specific client [5]. If you do not explicitly override this in the provider options, the library will default to client_secret_basic, which requires the client credentials to be sent in the Authorization header [2][6].

Citations:


🌐 Web query:

Auth.js provider token_endpoint_auth_method default client_secret_basic next-auth@5 beta

💡 Result:

In Auth.js (NextAuth.js) v5, the default token_endpoint_auth_method for OAuth providers is typically client_secret_basic [1][2]. If your specific OAuth provider does not support this method and requires client_secret_post (or another method) instead, you must explicitly override this configuration in your provider definition [3][2]. You can override the authentication method by adding a client object to your provider configuration [4][3][2]: typescript providers: [ { id: "my-provider", name: "My Provider", type: "oauth", // ... other config (authorization, token, userinfo, etc.) clientId: process.env.AUTH_CLIENT_ID, clientSecret: process.env.AUTH_CLIENT_SECRET, client: { token_endpoint_auth_method: "client_secret_post", }, }, ], This configuration tells the underlying OAuth library used by Auth.js to adjust how it authenticates with the token endpoint [4][3]. If you encounter an invalid_client error stating that client_secret_basic was requested but the provider requires client_secret_post, applying this override is the standard solution [1][2].

Citations:


Correct the client-authentication checkpoint.

Authentication must match what Zitadel accepts. If Task 2 shows that HTTP Basic succeeds, do not add client: { token_endpoint_auth_method: "client_secret_post" }; reconcile the Zitadel client configuration instead. Add the client_secret_post override only if the form request succeeds but Auth.js sends HTTP Basic by default.

🤖 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 `@docs/superpowers/plans/2026-08-03-martyrology-oidc-login-client.md` at line
637, Revise the client-authentication checkpoint to make the `Zitadel({...})`
override conditional: if Task 2 confirms HTTP Basic succeeds, reconcile the
Zitadel client configuration and do not add `client: {
token_endpoint_auth_method: "client_secret_post" }`; add that override only when
form-field authentication succeeds but Auth.js defaults to HTTP Basic, and
verify the result at Step 9.

Comment thread docs/superpowers/plans/2026-08-03-martyrology-oidc-login-client.md Outdated
JohnRDOrazio and others added 4 commits August 3, 2026 20:15
LITCAL_FRONTEND_URLS registers production and staging only; LitCal does
local development against a separate local Zitadel in its own compose
stack. Follow that pattern rather than CDCF Website's, which registers
http://localhost:3000 as a devMode=true client in production Zitadel.

A local stack for Martyrology is a separate design.
LITCAL_FRONTEND_URLS registers production and staging only. LiturgicalCalendar
develops locally against a separate Zitadel in its own compose stack, rather
than registering http://localhost:3000 in the production instance the way CDCF
Website does. The original D3 decision picked the weaker precedent.

Consequence recorded honestly: there is no local sign-in until a Martyrology
local stack exists, which is now its own design cycle. Tasks 3-7 of the plan
are flagged stale, since their live verification steps assumed a localhost
client; their code and unit tests are unaffected.

The gate does not need a dev client — the manual flow can use the production
redirect URI, which 404s while nothing is deployed there.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Only the production app is created now.
Review of the amendment found the spec and plan still asserting two apps in
sections the reversal had otherwise rewritten: D2's heading, the order-of-work
step that provisioned "the dev app only", both Interfaces blocks, and the
self-review coverage row.

Also extends the Tasks 3-7 staleness banner to name Task 7 Steps 1 and 5 —
the handoff table and the issue-close comment both still describe two apps,
and the latter would have been posted publicly.

Corrects the LITCAL_FRONTEND_URLS citation to 309-312.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@JohnRDOrazio JohnRDOrazio changed the title Add the Martyrology Frontend OIDC login apps Add the Martyrology Frontend OIDC login app Aug 3, 2026
…session

Four findings, all verified valid against the current files and the Auth.js
source before fixing.

The serious one: the plan put the access token on the Session in the session
callback. Auth.js sets that callback's return value as the response body of
GET /api/auth/session (packages/core/src/lib/actions/session.ts,
`response.body = newSession`), so the token would have been readable by the
browser — defeating the BFF arrangement that is the whole justification for
choosing a confidential client over PKCE. The proxy now reads the JWT
server-side via getToken; the Session carries only `error`. Note this is the
pattern Auth.js's own third-party-backend guide recommends, which is correct
for clients meant to call a backend directly and wrong here.

Also:
- The client-auth checkpoint was inverted — it told the implementer to force
  client_secret_post when Basic was what worked, which would break the
  exchange rather than fix it. Rewritten as two cases, only one of which
  calls for an override.
- The /api/auth/providers smoke check was described as proving both secrets
  landed. It proves provider registration only; a wrong client secret is
  invisible to it and surfaces as invalid_client on a real sign-in. Corrected
  in the spec, the workflow comment, and the failure message.
- MD040: the usage-text fence lacked a language identifier.

Records what is not covered: a permanent unit guard against the session leak
needs auth.ts to export its callbacks, which is left out of scope and flagged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
auth/setup-zitadel.sh (1)

107-109: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Enforce the production-only target for this action.

MARTYROLOGY_FRONTEND_URLS always contains the production origin, but the action remains available with --target local, and --all queues it without a target check. A local run can create or update this app in the local Zitadel instance with a production redirect URI. Reject non-production targets before queued actions run, or skip this action explicitly for local --all.

Suggested guard
 do_provision_martyrology_frontend() {
+    if [[ "$TARGET" != "production" ]]; then
+        err "--provision-martyrology-frontend requires --target production"
+        exit 64
+    fi
     log "Provisioning Martyrology Frontend OIDC app"

Also gate the --all queue before any local actions execute.

Also applies to: 751-760, 874-896

🤖 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 `@auth/setup-zitadel.sh` around lines 107 - 109, Enforce a production-only
guard for the provision-martyrology-frontend action: reject or skip it whenever
the selected target is not production, including when queued through --all,
before any queued actions execute. Update the argument handling and
action-dispatch logic around the provision-martyrology-frontend and --all
entries, preserving other local actions and ensuring the production URL is never
applied to a local Zitadel instance.
docs/superpowers/specs/2026-08-03-martyrology-oidc-login-client-design.md (2)

285-293: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Run the token-acceptance gate after deployment and isolate authorization.

The current steps run the flow while the callback endpoint is undeployed and use the 404 URL to read the code. This conflicts with the PR requirement to run the gate after deployment from main, and it does not validate the deployed Auth.js callback or proxy. Line 47 also states that restricted text requires an identity with OpenFGA can_read_texts; specify that test user and tuple before interpreting a redacted response. Assert introspection activity and roles separately. One unredacted response does not prove the complete design end to end.

🤖 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 `@docs/superpowers/specs/2026-08-03-martyrology-oidc-login-client-design.md`
around lines 285 - 293, Revise the token-acceptance milestone to run only after
deploying main with the production callback endpoint and proxy available, using
the deployed Auth.js flow rather than extracting a code from a 404 URL. Identify
the test user and OpenFGA can_read_texts tuple required for restricted text, and
define separate assertions for token introspection activity, roles, and
unredacted API output. Do not describe one successful unredacted response as
proof of the complete design.

223-234: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Add the AUTH_ZITADEL_ID to the deployment contract.

AUTH_ZITADEL_SECRET proves only the client secret is present; Auth.js still needs the Martyrology Frontend OIDC client_id produced by --provision-martyrology-frontend. Add AUTH_ZITADEL_ID to the handoff, GitHub variables, and .env target list.

🤖 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 `@docs/superpowers/specs/2026-08-03-martyrology-oidc-login-client-design.md`
around lines 223 - 234, Add AUTH_ZITADEL_ID to the OIDC deployment contract
alongside AUTH_ZITADEL_SECRET, including the deployment handoff, GitHub
variables, and existing .env target list. Preserve the current
secret/configuration distinction and ensure the value is sourced from the
Martyrology Frontend client provisioned by --provision-martyrology-frontend.
🧹 Nitpick comments (1)
docs/superpowers/specs/2026-08-03-martyrology-oidc-login-client-design.md (1)

236-245: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Narrow the smoke-test assertion to what /api/auth/providers actually checks.

Use startup validation or discovery logic that actually verifies AUTH_ZITADEL_ISSUER; otherwise, a missing issuer can remain hidden until users try authorization.

🤖 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 `@docs/superpowers/specs/2026-08-03-martyrology-oidc-login-client-design.md`
around lines 236 - 245, Revise the deploy workflow smoke-test design around GET
$SITE_URL/api/auth/providers so it does not claim to validate
AUTH_ZITADEL_ISSUER unless the endpoint’s startup or discovery behavior actually
verifies that setting. Add or reuse an explicit startup validation or
issuer-discovery check that fails when AUTH_ZITADEL_ISSUER is missing, while
keeping the provider-list assertion limited to confirming Auth.js booted and
registered zitadel.
🤖 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.

Outside diff comments:
In `@auth/setup-zitadel.sh`:
- Around line 107-109: Enforce a production-only guard for the
provision-martyrology-frontend action: reject or skip it whenever the selected
target is not production, including when queued through --all, before any queued
actions execute. Update the argument handling and action-dispatch logic around
the provision-martyrology-frontend and --all entries, preserving other local
actions and ensuring the production URL is never applied to a local Zitadel
instance.

In `@docs/superpowers/specs/2026-08-03-martyrology-oidc-login-client-design.md`:
- Around line 285-293: Revise the token-acceptance milestone to run only after
deploying main with the production callback endpoint and proxy available, using
the deployed Auth.js flow rather than extracting a code from a 404 URL. Identify
the test user and OpenFGA can_read_texts tuple required for restricted text, and
define separate assertions for token introspection activity, roles, and
unredacted API output. Do not describe one successful unredacted response as
proof of the complete design.
- Around line 223-234: Add AUTH_ZITADEL_ID to the OIDC deployment contract
alongside AUTH_ZITADEL_SECRET, including the deployment handoff, GitHub
variables, and existing .env target list. Preserve the current
secret/configuration distinction and ensure the value is sourced from the
Martyrology Frontend client provisioned by --provision-martyrology-frontend.

---

Nitpick comments:
In `@docs/superpowers/specs/2026-08-03-martyrology-oidc-login-client-design.md`:
- Around line 236-245: Revise the deploy workflow smoke-test design around GET
$SITE_URL/api/auth/providers so it does not claim to validate
AUTH_ZITADEL_ISSUER unless the endpoint’s startup or discovery behavior actually
verifies that setting. Add or reuse an explicit startup validation or
issuer-discovery check that fails when AUTH_ZITADEL_ISSUER is missing, while
keeping the provider-list assertion limited to confirming Auth.js booted and
registered zitadel.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 33cf19c7-d80f-43aa-827a-0314f99c84ab

📥 Commits

Reviewing files that changed from the base of the PR and between d259569 and c3e6bde.

📒 Files selected for processing (3)
  • auth/setup-zitadel.sh
  • docs/superpowers/plans/2026-08-03-martyrology-oidc-login-client.md
  • docs/superpowers/specs/2026-08-03-martyrology-oidc-login-client-design.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/superpowers/plans/2026-08-03-martyrology-oidc-login-client.md

Guard (approved separately): auth.ts now exports its `callbacks` object so the
session callback can be called directly, and lib/__tests__/auth-callbacks.test.ts
asserts the access and refresh tokens never appear in the serialized session.
Asserting on the serialized form rather than named properties catches the token
arriving under any future key. Exported for testability only.

CodeRabbit findings, verified before fixing:

- AUTH_ZITADEL_ID was consumed by auth.ts and zitadel-token.ts but appeared
  nowhere in the deployment contract — no GitHub variable, no preflight, not in
  the shipped .env. The deployed app would have had no client id. Added as a
  variable (a client id is public; the split stays by sensitivity), with a
  preflight assertion and a line in the .env write.
- The claim that the providers smoke check catches a missing AUTH_ZITADEL_ISSUER
  was wrong, including in the text added one commit ago: the endpoint reports
  configuration and performs no discovery. Narrowed to AUTH_SECRET and pointed
  at the preflight, which is what actually guards the issuer and the id.
- The gate milestone stated three assertions as one and claimed a single
  unredacted response proved the design end to end. Split into introspection,
  roles claim, and unredacted output; records that the third depends on the
  caller holding can_read_texts, since signing in as anyone else yields
  redaction that looks exactly like failure.

Not fixed — a production-only guard on --provision-martyrology-frontend.
provision-litcal-frontend and provision-cdcf-website hardcode production URLs
and sit in --all on the same terms, so guarding only the new action would be
inconsistent. Worth doing across all three or not at all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@JohnRDOrazio
JohnRDOrazio merged commit 0b3ccc9 into main Aug 3, 2026
1 check passed
@JohnRDOrazio
JohnRDOrazio deleted the feat/martyrology-frontend-oidc-client branch August 3, 2026 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant