Put oauth_allow_insecure_email_lookup in the section Grafana actually reads#677
Merged
Conversation
The flag sat under grafana.ini's auth.generic_oauth map, so the grafana
subchart rendered it into [auth.generic_oauth] (charts/grafana
templates/configmap.yaml emits each top-level grafana.ini key as its own
literal ini section header). Grafana reads the flag with a hardcoded
section literal -- pkg/services/authn/clients/oauth.go:207 calls
settingsProviderSvc.KeyValue("auth", "oauth_allow_insecure_email_lookup")
-- and go-ini child sections inherit from the parent, never the reverse,
so [auth] never saw it. The read fell back to conf/defaults.ini:708,
where the key is declared under [auth] and defaults to false. The flag
has therefore never been active.
The consequence is a lockout on upgrade. oauth.go:206-210 starts with an
empty UserLookupParams and only populates Email, and only when the flag
is true; Login is never set on this path. With the flag inert the lookup
params are empty, lookupByOneOf matches nothing, user sync falls through
to createUser, and the insert collides with the existing v5 row on
"email=? OR login=?" -- the reported "user already exists" at the OAuth
callback. Every user carried over from v5 auth.proxy hits this.
grafana/grafana#103974 (the flag in [auth] appearing not to take effect
on v11.4.0) was checked and does not apply. It is closed as completed,
and the reporter's own closing comment attributes it to a stale row
written to the Enterprise `setting` table via PUT /api/admin/settings
shadowing the file, not to a defect reading grafana.ini. We deploy OSS
grafana/grafana, where setting.OSSImpl.Section reads o.Cfg.Raw (the
parsed ini) and nothing else, and OSSImpl.Update hard-errors, so no DB
row can ever shadow the file. No env-var or admin-API workaround is
needed; verified by helm template that the key now renders under [auth].
Also correct the break-glass procedure in the release notes and the
values.yaml comment. Both claimed the local admin stays reachable at
/login?disableLoginForm=false. There is no such query parameter: the
login page reads disableLoginForm from the server-provided frontend
config (LoginCtrl.tsx:127) and never inspects the query string, and
pkg/services/authn/authnimpl/registration.go:83-84 does not register the
form client at all when disable_login_form is set, so no interactive or
API password login exists. The real recovery path is a values override
setting auth.disable_login_form to false plus a pod restart.
AlexGodbehere
added a commit
that referenced
this pull request
Jul 17, 2026
…#678) ## The bug Upgrading any ACS v5.1.0 installation to v6 leaves Grafana in CrashLoopBackOff: ``` Error: ✗ unable to start dualwrite service due to migration error: unified storage data migration failed: migration failed (id = playlists migration): migration failed for org 1 (default): SQL logic error: no such column: p.created_at (1) ``` Reproduced on amrc-fpd-jahx upgrading v5.1.0 -> v6.0.0-rc.4. ## Root cause `playlist.created_at`/`updated_at` are added by a sqlstore migration that existed **only in the 11.x/12.x line**. Grafana 13 **deleted it** — `pkg/services/sqlstore/migrations/playlist_mig.go` is gone as of v13.0.0 and `migrations.go` no longer calls `addPlaylistMigrations` — so the 13.1.0 binary has no code path that can add those columns to an existing database. It nevertheless still ships a unified-storage playlist data migration that is enabled by default (`MigratedUnifiedResources[PlaylistResource] = true`, `pkg/setting/setting_unified_storage.go`) whose SQL unconditionally selects `p.created_at` (`pkg/registry/apps/playlist/migrator/query_playlists.sql:7`). It fails at SQLite statement-prepare, so having zero playlists does not avoid it, and `RunMigrations` aborts on first failure so Grafana never finishes booting. The upgrade paths: | path | outcome | |---|---| | 10.x -> 13.x direct | **broken** — columns never added | | 10.x -> 11/12 -> 13 | fine — 11/12 add the columns | | fresh 13 install | fine — v13 seeds them from its schema snapshot | ACS v5.1.0 ships Grafana 10.0.1 and v6 jumps straight to 13.1.0 (#670), so **every upgraded site hits this and no fresh install does** — which is exactly why CI stayed green through rc.4. ## The fix Set `enableMigration: false` for the playlists resource. The flag is read at registration time (`pkg/storage/unified/migrations/resources.go:23-33`), so the broken SQL is never assembled — the same mechanism that already disables the datasource/stars/preferences/querycacheconfigs migrations by default. It touches no data: the legacy `playlist` table is left as-is and unified storage simply has no playlists. ACS does not use playlists. Dashboards, folders, users, datasources and alerts are unaffected. ## Verified on a real cluster With the migration skipped, on amrc-fpd-jahx: ``` logger=unifiedstorage-migrator msg="migrations completed" performed=0 skipped=2 logger=storage.unified.migrations msg="Unified storage migrations completed successfully" ``` The folder and dashboard migrations — which were masked behind the playlist failure and carry the real ACS data — then completed, and dashboards survived. `helm template` renders `[unified_storage.playlists.playlist.grafana.app]` / `enableMigration = false`, with the `[auth]` block from #677 intact. ## How to test 1. Install v5.1.0, create a dashboard, upgrade to this build with the Grafana PVC kept. 2. Grafana starts; `kubectl logs deploy/acs-grafana -c grafana | grep -i migration` shows the playlist migration skipped and unified storage migrations completing. 3. The dashboard is still there. ## Notes - This is an upstream Grafana bug: v13 removed a schema migration that databases in the wild still need. Worth reporting. - One log line is a red herring — `"Unified storage config has no effect for fully migrated resources"` is emitted *before* the code that reads and honours the key. - Untested: a fresh v13 install has no `playlist` table at all, so forcing the migration off might leave the Playlists UI empty in Legacy mode. Does not affect upgrades; worth a check against an empty namespace before v6.0.0 final. ## Release notes Fixes Grafana failing to start after upgrading from v5, with `no such column: p.created_at`. Grafana 13 removed the schema migration that adds the playlist timestamp columns while still running a migration that requires them, so databases created by the Grafana 10.0.1 shipped in v5.1.0 could not start under v6.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
deploy/values.yamlnestedoauth_allow_insecure_email_lookupunder theauth.generic_oauthmap. The grafana subchart renders every top-levelgrafana.inikey as its own ini section, so it landed in[auth.generic_oauth]— and Grafana reads it with a hardcoded section literal:go-ini child sections inherit from the parent, never the reverse, so the value was silently discarded and the read fell back to
conf/defaults.ini:708(false, declared inside[auth]).Why that locks everyone out
oauth.go:206-210starts with an emptylookupParamsand only populatesEmailwhen the flag is true;Loginis never set on it. With the flag ineffective,lookupByOneOfmatches nothing, sync falls through tocreateUser, and the insert collides with the existing row's login/email uniqueness — every user who existed under v5's auth.proxy is refused OAuth login with "user already exists", admin included. Only brand-new principals can sign in.The fix
Move the key into the
authmap so it renders into[auth]. Verified by render, not by reading —helm templatenow emits:with the
[auth.generic_oauth]block otherwise unchanged.Also corrects a false claim in
docs/reference/release-notes.mdand in the values.yaml comment: the v6.0.0 notes said the seeded admin stays reachable at/login?disableLoginForm=false. No such query parameter exists. Withdisable_login_form: true,registration.godoes not register the form client at all, so no interactive or API password login exists. Replaced with the real break-glass procedure: overridedisable_login_formto false and restart the pod.On grafana/grafana#103974
An earlier review flagged this issue as evidence that the
[auth]file setting might be ignored anyway. It is not: the reporter closed it himself explaining he had enabled the flag viaPUT /api/admin/settings, and the resulting row in thesettingtable was shadowing his file config. That override path is Enterprise-only — OSS registers onlyGET /admin/settings(pkg/api/api.go:116) andOSSImpl.Updatehard-returns an error, so the failure mode cannot occur here.OSSImpl.KeyValuereadsCfg.Raw— the parsed ini file — and nothing else.Three independent investigations (v13.1.0 source, issue history, chart render) agreed unanimously.
How to test
kubectl get cm acs-grafana -o yamlshows the key under[auth].Release notes
Fixes Grafana SSO refusing every pre-existing user with "user already exists" after upgrading to v6. Corrects the documented Grafana emergency-access procedure, which described a query parameter that does not exist.