fix(oidc): report provider initialization status to prevent auto-redirect failure#1475
fix(oidc): report provider initialization status to prevent auto-redirect failure#1475wucm667 wants to merge 3 commits into
Conversation
WalkthroughAdds an Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Frontend as Frontend (Browser)
participant Backend as Backend (API)
participant OIDC as OIDC Provider
User->>Frontend: Open page
Frontend->>Backend: GET /v1/status
Backend->>Backend: check ctrl.oidcProvider != nil
Backend-->>Frontend: respond with status.oidc { enabled, autoRedirect, initialized, ... }
alt initialized && enabled && autoRedirect && no oidcError
Frontend->>User: perform OIDC redirect to provider
User->>OIDC: OIDC auth flow
else not initialized or other block
Frontend->>User: show local UI (hide/disable OIDC button)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Security Recommendations
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…rect failure When the OIDC issuer (e.g. Authentik) is not reachable at startup, the provider initialization fails silently and ctrl.oidcProvider remains nil. However, the status endpoint still reports oidc.enabled=true, causing the frontend to auto-redirect to a broken /api/v1/users/login/oidc endpoint which returns HTTP 500 "OIDC provider not available". Add an "initialized" field to OIDCStatus that reflects whether the provider was actually created successfully. The frontend now checks this field before auto-redirecting or showing the OIDC login button. Fixes sysadminsmedia#1471 Signed-off-by: wucm667 <stevenwucongmin@gmail.com>
wucm667
force-pushed
the
fix/oidc-provider-not-available
branch
from
a012b1d to
d540412
Compare
|
Fixed a prettier formatting warning on line 76. Lint now passes on our modified files. Note: The remaining CI warnings (unused vars in location pages, tailwind issues) are pre-existing on main and not introduced by this PR. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/lib/api/types/data-contracts.ts (1)
1216-1222:OIDCStatus.initializedaddition looks correct and well-scoped.This contract update matches the backend status payload change and enables safer frontend gating of OIDC redirect/login UI.
Security recommendation: keep backend availability checks on/users/login/oidcas the enforcement point; UI checks should stay defense-in-depth only.As per coding guidelines, "Never edit generated types in
lib/api/types/- runtask generateafter backend API changes".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/lib/api/types/data-contracts.ts` around lines 1216 - 1222, You manually edited the generated OIDCStatus interface (symbol OIDCStatus) in frontend/lib/api/types/data-contracts.ts; revert this edit and instead regenerate the types so the change comes from the API spec: undo the manual change to OIDCStatus, run the repository codegen task (run "task generate" or the project's equivalent) to produce updated types, verify the generated OIDCStatus includes initialized, and commit the regenerated file rather than editing lib/api/types by hand.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/lib/api/types/data-contracts.ts`:
- Around line 1216-1222: You manually edited the generated OIDCStatus interface
(symbol OIDCStatus) in frontend/lib/api/types/data-contracts.ts; revert this
edit and instead regenerate the types so the change comes from the API spec:
undo the manual change to OIDCStatus, run the repository codegen task (run "task
generate" or the project's equivalent) to produce updated types, verify the
generated OIDCStatus includes initialized, and commit the regenerated file
rather than editing lib/api/types by hand.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 91d372d2-bb19-481d-8efa-34f11a9cf47f
📒 Files selected for processing (3)
backend/app/api/handlers/v1/controller.gofrontend/lib/api/types/data-contracts.tsfrontend/pages/index.vue
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/app/api/handlers/v1/controller.go
- frontend/pages/index.vue
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/partial-frontend.yaml (1)
22-24: Pin Node to a stable major version (e.g.,24) instead oflts/*for better reproducibility and security.The
lts/*specifier rolls to a new major version annually, risking unexpected CI failures and dependency-resolution drift. Pinning to a stable major—such as Node 24, already used incopilot-setup-steps.yml—ensures consistent runtime behavior across builds and strengthens the supply chain.Other workflows in this repository (
upgrade-test.yaml,e2e-partial.yaml) use the samelts/*pattern and would also benefit from pinning.Suggested change
- - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 - with: - node-version: lts/* + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 + with: + node-version: "24"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/partial-frontend.yaml around lines 22 - 24, Replace the floating node-version specifier with a pinned major version: change the actions/setup-node step (actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020) to use node-version: "24" (or "24.x") instead of "lts/*" to ensure reproducible builds; apply the same change to other workflows that use lts/* (e.g., upgrade-test.yaml, e2e-partial.yaml) so all CI jobs target the same Node 24 major runtime.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/partial-frontend.yaml:
- Around line 22-24: Replace the floating node-version specifier with a pinned
major version: change the actions/setup-node step
(actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020) to use
node-version: "24" (or "24.x") instead of "lts/*" to ensure reproducible builds;
apply the same change to other workflows that use lts/* (e.g.,
upgrade-test.yaml, e2e-partial.yaml) so all CI jobs target the same Node 24
major runtime.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6c55c758-1616-445b-8928-7a157fc7eded
📒 Files selected for processing (1)
.github/workflows/partial-frontend.yaml
|
The Lint CI is now running with Node.js 22 LTS (the setup-node fix worked), but it's still failing due to 26 pre-existing lint issues on main:
None of these are from this PR's changes. The modified files ( The |
1 participant
What type of PR is this?
What this PR does / why we need it:
When the OIDC issuer (e.g. Authentik) is not reachable at startup, the provider initialization fails silently and
ctrl.oidcProviderremainsnil. However, the status endpoint still reportsoidc.enabled=true, causing the frontend to auto-redirect to a broken/api/v1/users/login/oidcendpoint which returns HTTP 500 "OIDC provider not available".Changes:
Initializedfield toOIDCStatusstruct that reflects whether the provider was actually created (ctrl.oidcProvider != nil)status?.oidc?.initializedbefore redirectingWhich issue(s) this PR fixes:
Fixes #1471
Special notes for your reviewer:
The fix is minimal and targeted — only 3 files changed with 5 insertions and 2 deletions. No behavioral change when OIDC initializes successfully.
Testing
Verified that:
initializedistrueand auto-redirect works as beforeinitializedisfalse, preventing auto-redirect to a broken endpointSummary by CodeRabbit
New Features
Chores