oidc: stop persisting bare openid scopes from empty setup input#389
Open
jflecool2 wants to merge 2 commits into
Open
oidc: stop persisting bare openid scopes from empty setup input#389jflecool2 wants to merge 2 commits into
jflecool2 wants to merge 2 commits into
Conversation
The setup wizard has no oidc_scopes field, so Initialize received an empty
scopes string. ParseScopes("") always injects openid and returns only that
value, which was then written to the DB as oidc_scopes. On subsequent boots
the DB value overrode the code default (openid email profile), so authorize
requests asked only for openid and IdPs that gate email/profile claims on
those scopes returned tokens without email.
- When OIDC is enabled and scopes are empty, persist DefaultOIDCScopes
before ParseScopes in setup and settings update.
- At config resolve, treat a DB value of exactly "openid" as the legacy
bug and heal it to the full default (explicit env OIDC_SCOPES=openid
is left alone).
- Export DefaultOIDCScopes for the write paths; add unit tests for heal
and env override behaviour.
Add unit tests for the empty-scopes default and legacy-openid heal: - Setup Initialize with empty scopes persists openid email profile - Setup Initialize with env OIDC_SCOPES normalizes and persists them - Settings update empty scopes defaults; explicit openid email preserved - Resolve keeps non-legacy DB scopes (openid email) without healing
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.
AI Disclamer
As I was unable to make my OIDC work, I asked my AI for help. It kept hallucinating again and again on why i was getting "email validation" errors. Then it dawned on me that cloudflare access OIDC didn't give any email ... because notifuse didn't ask any!! If the default include all 3, why was mine only asking openid? turns out the setup add "" in db, and "openid" + "" = "openid". And when theres a value in db, theres no need to take the default string for it. Anyways, the fix is made by AI. It opted to self heal bad database settings value. Anyways, this took too much time of my life so I hope this is useful.
Problem
Enabling OIDC via the setup wizard left instances requesting only the openid scope on the authorize URL.
The wizard has no oidc_scopes field, so setup always received an empty scopes string. The write path then did:
oidcScopes = strings.Join(ParseScopes(oidcScopes), " ")
ParseScopes("") always injects openid and returns only that value, which was persisted as oidc_scopes=openid. On later boots, the DB value overrode the code default (openid email profile), so IdPs that put email/profile claims behind those scopes returned tokens without email (e.g. Cloudflare Access).
Fix
No migration required; existing broken rows are healed at boot. No API or frontend changes required.
Testing
make test-unit (race) passes on this branch.