Skip to content

Conversation

@pnzrr
Copy link

@pnzrr pnzrr commented Sep 10, 2025

Summary by CodeRabbit

  • Documentation
    • Updated example app sample configs to highlight Phase Two issuer settings and add Phase Two guidance alongside existing Cloud IAM references; several examples now reference the Phase Two issuer in their samples.
  • Chores
    • Added the pnpm lockfile to the repository ignore list.

@coderabbitai
Copy link

coderabbitai bot commented Sep 10, 2025

Walkthrough

Updated VCS ignore and example configs: added pnpm-lock.yaml to .gitignore and replaced or annotated Keycloak issuer URIs to reference Phase Two in multiple example env samples; updated Angular app config to use the Phase Two issuer URI. No API signatures changed.

Changes

Cohort / File(s) Summary of Changes
VCS config
.gitignore
Added pnpm-lock.yaml to the ignore list.
Example env samples (Keycloak -> Phase Two)
examples/react-router-framework/.env.local.sample, examples/react-router/.env.local.sample, examples/tanstack-router-file-based/.env.local.sample, examples/tanstack-router/.env.local.sample
Updated Keycloak sections to add a "Phase Two" header and reference Phase Two issuer URIs (phasetwo.oidc-spa.dev/...). Some files make Phase Two the active value; others add Phase Two examples and comment Cloud-IAM fallback lines. VITE_OIDC_CLIENT_ID values unchanged.
Angular app config (runtime issuer change)
examples/angular/src/app/app.config.ts
Replaced the issuerUri passed to Oidc.provide from https://cloud-iam.oidc-spa.dev/realms/oidc-spa to https://phasetwo.oidc-spa.dev/realms/oidc-spa. No signature or other logic changes.

Sequence Diagram(s)

sequenceDiagram
  participant App as Application
  participant Config as OIDC Config
  participant Issuer as Identity Provider (Issuer)

  rect rgb(235,245,255)
    Note over App,Config: App loads OIDC configuration
    App->>Config: request issuerUri
    Config-->>App: returns issuerUri (phasetwo.oidc-spa.dev)
  end

  rect rgb(245,255,235)
    Note over App,Issuer: Auth flow targets Phase Two issuer
    App->>Issuer: OIDC discovery / auth requests (phasetwo.oidc-spa.dev)
    Issuer-->>App: discovery document / tokens
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I thump my paws on docs anew,
Phase Two hops in with a jaunty cue.
Lockfile hidden, samples show the way,
Clients unchanged, the config will stay.
A carrot nod — ready for the day. 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “add phasetwo realm for testing” succinctly captures the principal change of the pull request by indicating the addition of the Phase Two realm for testing, aligning directly with the updates made to configuration files and sample environments.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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 and usage tips.


# ======= Keycloak =================
# Phase Two (https://dash.phasetwo.io)
# VITE_OIDC_ISSUER_URI=https://phasetwo.us-west-2.aws.auth.ac/auth/realms/oidc-spa
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# VITE_OIDC_ISSUER_URI=https://phasetwo.us-west-2.aws.auth.ac/auth/realms/oidc-spa
# VITE_OIDC_ISSUER_URI=https://phasetwo.oidc-spa.dev/realms/oidc-spa

This should ideally point to the demo app’s domain.

oidc-spa detects whether the auth server shares a parent subdomain with the running app.

  • If yes: it can use an iframe to silently restore the session.
  • If no: the browser treats the auth server as a third party and does not send the required cookies. In this case, oidc-spa falls back to full-page redirects.

Thanks to the library’s early-init strategy, redirects are barely noticeable. However, without iframe support, three full-page reloads occur on every refresh.

Comparison iframe vs. no iframe (when I was in Tokyo connecting to a keycloak in france):
https://www.youtube.com/watch?v=55sZ7XSWh4Q

More details: https://docs.oidc-spa.dev/resources/why-no-client-secret

I’d like to demonstrate oidc-spa in its best-case scenario.


It would also be best if <KC_RELATIVE_PATH> could be "" instead of "/auth".

  • It shortens URLs, making it more parsable, important for didactic purpose.
  • It matches modern Keycloak’s default configuration.

In practice, if your app is hosted at my-company.com, there are two common patterns:

  1. DNS-based routing: https://auth.my-company.com
  2. Reverse-proxy routing: https://my-company.com/auth

Mixing both (custom subdomain and custom subpath) is not something I’d want to promote as best practice.


The most desirable setup would be:

VITE_OIDC_ISSUER_URI=https://phasetwo.oidc-spa.dev/realms/demo

This would achieve the following:

  • Clearly highlight PhaseTwo as a Keycloak hosting solution.
  • Provide a clean, easily parsable URL aligned with best practices.
  • More accurately reflect what prospects will experience when using your service—not just a realm on a shared cluster, but a dedicated Keycloak instance.

I realize this may be a bigger ask since it would require a separate Keycloak instance and associated costs.

That said, I believe the promotional value could make it worthwhile:

  • While the project is still niche, I plan to support TanStack Start and submit a PR to get it featured in the TanStack Start documentation, ideally under a "Keycloak / provider agnostic" section. If that lands, adoption could grow significantly, and having PhaseTwo showcased there as the go-to Keycloak hosting solution would be a strong promotional win.
  • I also intend to submit a PR to the official Keycloak documentation so that oidc-spa is listed as an adapter option alongside keycloak-js.
  • On a shorter timeline, I will propose a PR to Keycloak to replace keycloak-js with oidc-spa for the Account and Admin console. This was actually hinted at by Marek, one of the team leads, so I have good reason to believe it can land. If it does, it would be a strong authority signal that could further accelerate adoption.

Let me know what you think.

@pnzrr
Copy link
Author

pnzrr commented Sep 11, 2025

@garronej No problem, I can get the cluster updated on the two points:

  1. custom domain
  2. remove /auth

@garronej
Copy link
Collaborator

Great!

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (2)
examples/react-router/.env.local.sample (1)

4-9: Consider addressing the /auth path request.

Same concern as examples/tanstack-router-file-based/.env.local.sample: the /auth path segment remains despite previous feedback to remove it, and this creates inconsistency with the Angular example.

examples/tanstack-router/.env.local.sample (1)

4-9: Consider addressing the /auth path request.

Same concern as other .env.local.sample files: the /auth path segment remains, creating inconsistency with the Angular example and not addressing garronej's previous feedback.

🧹 Nitpick comments (2)
examples/tanstack-router-file-based/.env.local.sample (1)

4-9: Consider addressing the /auth path request.

The Phase Two configuration still includes /auth in the path, but garronej's previous feedback on examples/react-router-framework/.env.local.sample suggested removing it to match modern Keycloak defaults and simplify URLs. The PR comments mention this can be addressed, but it hasn't been applied here yet.

Additionally, the Angular example (examples/angular/src/app/app.config.ts) uses /realms/oidc-spa without /auth, creating an inconsistency.

examples/react-router-framework/.env.local.sample (1)

4-9: Consider addressing the /auth path request.

This is the file where garronej originally requested removing /auth from the path. The Phase Two configuration still includes /auth/realms/oidc-spa, and this creates inconsistency with the Angular example (examples/angular/src/app/app.config.ts line 19) which uses /realms/oidc-spa.

While the PR comments indicate this can be addressed, applying it consistently across all examples would improve clarity.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 52c9239 and 7f72413.

📒 Files selected for processing (5)
  • examples/angular/src/app/app.config.ts (1 hunks)
  • examples/react-router-framework/.env.local.sample (1 hunks)
  • examples/react-router/.env.local.sample (1 hunks)
  • examples/tanstack-router-file-based/.env.local.sample (1 hunks)
  • examples/tanstack-router/.env.local.sample (1 hunks)
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
examples/react-router-framework/.env.local.sample

[warning] 6-6: [UnorderedKey] The VITE_OIDC_CLIENT_ID key should go before the VITE_OIDC_ISSUER_URI key

(UnorderedKey)

examples/react-router/.env.local.sample

[warning] 6-6: [UnorderedKey] The VITE_OIDC_CLIENT_ID key should go before the VITE_OIDC_ISSUER_URI key

(UnorderedKey)

examples/tanstack-router-file-based/.env.local.sample

[warning] 6-6: [UnorderedKey] The VITE_OIDC_CLIENT_ID key should go before the VITE_OIDC_ISSUER_URI key

(UnorderedKey)

examples/tanstack-router/.env.local.sample

[warning] 6-6: [UnorderedKey] The VITE_OIDC_CLIENT_ID key should go before the VITE_OIDC_ISSUER_URI key

(UnorderedKey)

provideRouter(routes),
Oidc.provide({
issuerUri: 'https://cloud-iam.oidc-spa.dev/realms/oidc-spa',
issuerUri: 'https://phasetwo.oidc-spa.dev/realms/oidc-spa',
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify path consistency across examples.

The Phase Two URL here uses /realms/oidc-spa without /auth, while the .env.local.sample files in other examples (react-router, tanstack-router, etc.) include /auth/realms/oidc-spa. This inconsistency may confuse developers.

Run the following script to verify the URL patterns across all example configs:


🏁 Script executed:

#!/bin/bash
# Description: Check for Phase Two URL patterns across example configs

rg -n "phasetwo\.oidc-spa\.dev" examples/

Length of output: 107


🏁 Script executed:

#!/bin/bash
# Check for Phase Two URL in environment sample files
rg -n "phasetwo\\.oidc-spa\\.dev" examples/ -g "*.env*"
# Check for the `/auth/realms/oidc-spa` pattern across all examples
rg -n "auth/realms/oidc-spa" examples/

Length of output: 482


Use consistent Phase Two issuerUri
Replace https://phasetwo.oidc-spa.dev/realms/oidc-spa with https://phasetwo.oidc-spa.dev/auth/realms/oidc-spa to match other examples.

🤖 Prompt for AI Agents
In examples/angular/src/app/app.config.ts around line 19, the issuerUri value
uses the path /realms/oidc-spa but other examples use /auth/realms/oidc-spa;
update the issuerUri string to use
https://phasetwo.oidc-spa.dev/auth/realms/oidc-spa so it matches the project's
consistent Phase Two issuerUri format.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
examples/tanstack-router-file-based/.env.local.sample (1)

5-6: Reorder env keys to silence dotenv-linter.

The latest lint run complains that VITE_OIDC_CLIENT_ID should precede VITE_OIDC_ISSUER_URI. Swapping the two lines keeps the sample clean without changing behavior.

-VITE_OIDC_ISSUER_URI=https://phasetwo.oidc-spa.dev/realms/oidc-spa
-VITE_OIDC_CLIENT_ID=example-tanstack-router
+VITE_OIDC_CLIENT_ID=example-tanstack-router
+VITE_OIDC_ISSUER_URI=https://phasetwo.oidc-spa.dev/realms/oidc-spa
examples/react-router/.env.local.sample (1)

5-6: Match dotenv key order here as well.

For consistency with dotenv-linter, place VITE_OIDC_CLIENT_ID before VITE_OIDC_ISSUER_URI.

-VITE_OIDC_ISSUER_URI=https://phasetwo.oidc-spa.dev/realms/oidc-spa
-VITE_OIDC_CLIENT_ID=example-react-router
+VITE_OIDC_CLIENT_ID=example-react-router
+VITE_OIDC_ISSUER_URI=https://phasetwo.oidc-spa.dev/realms/oidc-spa
examples/tanstack-router/.env.local.sample (1)

5-6: Keep env keys ordered to avoid lint noise.

Swap the issuer and client lines so VITE_OIDC_CLIENT_ID comes first, mirroring the guidance from dotenv-linter.

-VITE_OIDC_ISSUER_URI=https://phasetwo.oidc-spa.dev/realms/oidc-spa
-VITE_OIDC_CLIENT_ID=example-tanstack-router
+VITE_OIDC_CLIENT_ID=example-tanstack-router
+VITE_OIDC_ISSUER_URI=https://phasetwo.oidc-spa.dev/realms/oidc-spa
examples/react-router-framework/.env.local.sample (1)

5-6: Apply the same key ordering tweak here.

Moving VITE_OIDC_CLIENT_ID above VITE_OIDC_ISSUER_URI keeps this sample aligned with the linter expectations and the other files.

-VITE_OIDC_ISSUER_URI=https://phasetwo.oidc-spa.dev/realms/oidc-spa
-VITE_OIDC_CLIENT_ID=example-react-router
+VITE_OIDC_CLIENT_ID=example-react-router
+VITE_OIDC_ISSUER_URI=https://phasetwo.oidc-spa.dev/realms/oidc-spa
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7f72413 and d91f0dd.

📒 Files selected for processing (4)
  • examples/react-router-framework/.env.local.sample (1 hunks)
  • examples/react-router/.env.local.sample (1 hunks)
  • examples/tanstack-router-file-based/.env.local.sample (1 hunks)
  • examples/tanstack-router/.env.local.sample (1 hunks)
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
examples/react-router-framework/.env.local.sample

[warning] 6-6: [UnorderedKey] The VITE_OIDC_CLIENT_ID key should go before the VITE_OIDC_ISSUER_URI key

(UnorderedKey)

examples/react-router/.env.local.sample

[warning] 6-6: [UnorderedKey] The VITE_OIDC_CLIENT_ID key should go before the VITE_OIDC_ISSUER_URI key

(UnorderedKey)

examples/tanstack-router-file-based/.env.local.sample

[warning] 6-6: [UnorderedKey] The VITE_OIDC_CLIENT_ID key should go before the VITE_OIDC_ISSUER_URI key

(UnorderedKey)

examples/tanstack-router/.env.local.sample

[warning] 6-6: [UnorderedKey] The VITE_OIDC_CLIENT_ID key should go before the VITE_OIDC_ISSUER_URI key

(UnorderedKey)

@pnzrr
Copy link
Author

pnzrr commented Oct 13, 2025

@garronej updates to the domain and removal of /auth

We will want to align your zod token verification with what the realm is configured to work accept during a registration. For instance, current zod wants a name, so do we need the user profile to have first + last name as part of the login (or remove first + last name in registration and the name check in the zod verification

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.

2 participants