-
Notifications
You must be signed in to change notification settings - Fork 18
add phasetwo realm for testing #83
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.
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughUpdated VCS ignore and example configs: added 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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 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 |
|
|
||
| # ======= Keycloak ================= | ||
| # Phase Two (https://dash.phasetwo.io) | ||
| # VITE_OIDC_ISSUER_URI=https://phasetwo.us-west-2.aws.auth.ac/auth/realms/oidc-spa |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # 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-spafalls 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:
- DNS-based routing:
https://auth.my-company.com - 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-spais listed as an adapter option alongsidekeycloak-js. - On a shorter timeline, I will propose a PR to Keycloak to replace
keycloak-jswithoidc-spafor 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.
|
@garronej No problem, I can get the cluster updated on the two points:
|
|
Great! |
There was a problem hiding this 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/authpath request.Same concern as
examples/tanstack-router-file-based/.env.local.sample: the/authpath 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/authpath request.Same concern as other
.env.local.samplefiles: the/authpath 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/authpath request.The Phase Two configuration still includes
/authin the path, but garronej's previous feedback onexamples/react-router-framework/.env.local.samplesuggested 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-spawithout/auth, creating an inconsistency.examples/react-router-framework/.env.local.sample (1)
4-9: Consider addressing the/authpath request.This is the file where garronej originally requested removing
/authfrom 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.tsline 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
📒 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', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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.
There was a problem hiding this 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 silencedotenv-linter.The latest lint run complains that
VITE_OIDC_CLIENT_IDshould precedeVITE_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-spaexamples/react-router/.env.local.sample (1)
5-6: Match dotenv key order here as well.For consistency with
dotenv-linter, placeVITE_OIDC_CLIENT_IDbeforeVITE_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-spaexamples/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_IDcomes first, mirroring the guidance fromdotenv-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-spaexamples/react-router-framework/.env.local.sample (1)
5-6: Apply the same key ordering tweak here.Moving
VITE_OIDC_CLIENT_IDaboveVITE_OIDC_ISSUER_URIkeeps 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
📒 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)
|
@garronej updates to the domain and removal of We will want to align your |
Summary by CodeRabbit