Repository navigation
radar-hub: turn on OIDC refresh tokens by default (1.10.0-rc.1) - #57
Conversation
PR Summary by QodoEnable OIDC refresh tokens by default in the radar-hub chart
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
Code Review by Qodo
1.
|
Hub 1.10.0 ends a self-hosted OIDC session that carries IdP groups and has no refresh token when its ID token expires, instead of keeping it and silently dropping the groups. With the default groupsClaim and refresh off, that signs users out every ID token lifetime: 5 minutes on a default Keycloak, an hour on Okta or Entra. useRefreshToken now defaults to true, so sessions last session.maxLifetime when the IdP issues a refresh token (Keycloak does by default; Okta and Entra need offline_access). An IdP that returns none behaves as before. NOTES says after every install or upgrade whether refresh tokens are on and what the IdP client needs. Split out of #51 so it can ship with Hub 1.10.0 ahead of clusterAccess. Staged as 1.10.0-rc.1 with appVersion unchanged; the Hub v1.10.0 tag promotes it to 1.10.0.
9aecfba to
87a38b5
Compare
Why
Hub 1.10.0 includes skyhook-dev/radar-hub#272 (
4a8169f). It ends a self-hosted OIDC session at ID-token expiry when the session carries IdP groups and has no refresh token. Before, the hub kept the session and quietly stopped forwarding the groups. The chart today defaultsgroupsClaim: "groups"withuseRefreshToken: false. Upgrading the hub without this change would therefore send every user who is in an IdP group to the sign-in page each time their ID token expires: every 5 minutes on a default Keycloak, every hour on Okta or Entra.These are the refresh-token parts of #51, split out so they can ship with Hub 1.10.0.
auth.oidc.clusterAccessstays in #51 until skyhook-dev/radar-hub-web#326 is ready. Without #326 the value has no effect, and #51's NOTES text would tell admins that new clusters follow the IdP.What
values.yaml:auth.oidc.useRefreshTokendefaults totrue. The comment explains what happens without a refresh token, by Hub version.NOTES.txt: after every install or upgrade, says whether refresh tokens are on and what the IdP client needs (Okta and Entra:offline_accessinscopes).tests/deployment_hub_sessions_test.yaml: the flag is emitted by default and omitted when set tofalse.Chart.yaml:1.10.0-rc.1(a default changes behaviour on upgrade, so a minor).appVersionstays1.9.1. The Hubv1.10.0tag promotes the chart to1.10.0and setsappVersion: 1.10.0.scripts/next-chart-version.shreturns1.10.0against the current tags.What upgrades see:
false.offline_access.helm upgrade --reuse-valueskeeps the old chart defaults, so such installs stay onfalse. NOTES prints "Refresh tokens: off" for them.--reset-then-reuse-valuespicks up the new default.Test plan
helm unittest charts/radar-hub: 20/20charts/radar-hub/tests/render-matrix.sh: all checks passedhelm lintandct lint --target-branch main.github/ci/agent-test.sh: 58 assertions passeduseRefreshTokentrue and falseFollow-up for #51: rebase onto this and drop its
useRefreshTokenand NOTES refresh-token hunks.Note
Medium Risk
Changing the default alters OIDC session and re-auth behavior on upgrade paths that adopt new values, though IdPs without refresh tokens behave as before and the flag can be set to false.
Overview
Aligns the Helm chart with Hub 1.10.0 OIDC session behavior by defaulting
auth.oidc.useRefreshTokento true, so OIDC installs keep IdP groups for the fullsession.maxLifetimeinstead of forcing re-login when the ID token expires (a breaking change on Hub 1.10+ when groups are forwarded and refresh is off).The chart version is bumped to 1.10.0-rc.1 (
appVersionunchanged at 1.9.1). values.yaml comments now document token-expiry behavior by Hub version and IdP quirks (e.g.offline_accessfor Okta/Entra). NOTES.txt prints whether refresh tokens are on and what to configure on the IdP client. Helm unit tests expectRADAR_HUB_OIDC_USE_REFRESH_TOKEN=trueby default and omission when explicitly set tofalse.Upgrades with
--reuse-valueskeep the previousfalsedefault; fresh installs or--reset-then-reuse-valuespick uptrue.Reviewed by Cursor Bugbot for commit 87a38b5. Bugbot is set up for automated code reviews on this repo. Configure here.