Task/db backed OIDC - #39
Conversation
There was a problem hiding this comment.
Reviewed the whole change. The architecture is sound — the two-phase candidate/activation model, the write-only secret contract, AES-GCM with per-provider AAD, and the restricted egress transport are all the right shapes for this problem, and the test coverage on the crypto and normalization helpers is genuinely good.
That said, I found several things I'd want resolved before this ships, plus a number of smaller ones. Inline comments have the detail; summarizing the ones I'd treat as blockers:
1. EnableOIDCProvider silently no-ops while logging every user out. oidcProviderFromCandidate doesn't copy ID, so when a candidate revision is staged the provider passed into setOIDCProviderEnabled has ID == 0. The UPDATE ... WHERE id = 0 matches zero rows, but RevokeProviderSessions still revokes every session for that provider and the handler returns 200 with enabled: true. UpdateColumns doesn't surface rows-affected, so nothing catches it.
2. Retiring a provider permanently burns its key and issuer. RetireOIDCProvider sets retired_at but the unique indexes on provider_key and issuer_url cover retired rows too. Re-creating a provider with the same key/issuer afterwards hits a constraint violation and surfaces as a 500 with no path forward short of manual SQL.
3. Saving a draft provider mutates live Grafana. In SaveOIDCProvider, the pre-activation path calls syncGrafana(..., enabled: false, ...), which PUTs the new client into Grafana's generic_oauth settings and disables it. If the customer already had Grafana Generic OAuth configured (via ini/env or a previous provider), that config is overwritten and OAuth login to Grafana breaks — before the admin has activated anything, and with no restore path if they never do.
4. A transient DB error during RefreshOIDCProvider locks everyone out until restart. The error branch calls replaceProviderState(databaseOIDCUnavailableConfig(...)), which installs an empty provider map. Fail-closed is the right instinct, but there's no retry and no self-heal, so a momentary DB blip during an enable/activate turns into an indefinite login outage.
Smaller items (SSRF edge cases, swallowed Grafana client construction error, unreachable backend routes, a UI refresh gap that hides the retry button) are inline.
Edited after review discussion. I originally listed a fifth blocker — gorm:"type:blob" being invalid Postgres DDL. @muzamilkm pushed back that this deployment is MySQL, and that's substantially right: the fork already uses MySQL-only type:longtext in the notion/hubspot/salesforce models, so this is an established pattern rather than an oversight. I've downgraded that comment to non-blocking; the suggestion there is now just a zero-cost hedge (dropping the tag changes nothing on MySQL). Leaving the correction visible rather than silently editing it away.
One process note: the PR description says the local run "covered 24 lifecycle/security scenarios and completed successfully." Blockers 1, 2, and 3 above are all lifecycle scenarios, so it'd be worth reconciling what those 24 actually exercised — I suspect the candidate-revision paths weren't among them. Related: see my separate comment on CI coverage, which is why none of this was caught automatically.
Summary
Moves native DevLake OIDC provider configuration from deployment environment variables into the existing MySQL-backed access-management feature.
Customer administrators can now validate, save and activate an OIDC provider from Config UI without changing deployment configuration or restarting DevLake. The client secret is encrypted before persistence and the database becomes the authoritative provider source after activation.
The same validated provider is also synchronized to Grafana Generic OAuth through Grafana's supported SSO Settings API, so customers configure their IdP once in DevLake while DevLake and Grafana continue to own their own users and sessions.
Included
PUT /api/v1/sso-settings/generic_oauthusing a dedicated deployment-owned Grafana server-admin machine identity.Authentication and Grafana Boundary
DevLake remains the customer-facing source of truth for OIDC configuration.
The customer registers one confidential OIDC client with both application callbacks:
DevLake performs discovery and uses the resulting provider configuration for its existing native OIDC flow. The same validated configuration is mapped into Grafana Generic OAuth through Grafana's SSO Settings API.
Grafana continues to own its own user database and session. Existing pre-created Grafana users are still matched through the existing Generic OAuth flow and clicking Dashboards continues to reuse the browser's IdP session for a seamless second login.
Grafana OSS does not allow an ordinary service-account token to obtain the instance-level
settings:read/settings:writepermissions required by the SSO Settings API. The integration therefore uses a dedicated local Grafana server-admin machine account supplied as a backend-only deployment secret. This account is separate from the customer's human Grafana administrator.Scope and Upstream Coupling
Most of the feature is contained within the existing fork-owned access/authentication boundaries:
backend/server/api/accessbackend/server/api/authconfig-ui/src/routes/accessUpstream-facing changes are limited to the required integration points:
The implementation does not modify Grafana source, Grafana database internals, unrelated plugins or shared Config UI infrastructure.
Verification
The implementation has been validated locally across the full provider lifecycle, including:
Focused checks include:
The local end-to-end test run covered 24 lifecycle/security scenarios and completed successfully, including live DevLake Google login and live Grafana Generic OAuth synchronization.
Future Work
(issuer, subject)identity directly on an access user, so this PR deliberately supports one active database provider at a time.auth_access_identities, plus explicit identity linking, session revocation across identities and migration of existing users.Staging and Production Follow-up
Application support does not provision the deployment-owned secrets or Grafana machine identity itself.
Before rollout, deployment automation/DevOps must provide:
devlake-systemserver-admin machine user and random passwordThe Grafana machine identity can be provisioned using Grafana's documented server-admin APIs and is independent from the customer-facing Grafana administrator.
Staging should first migrate the existing Google provider from environment configuration into the database, verify DevLake and Grafana login after activation and then remove the old provider credentials from the runtime environment.