Fix invalid shipped OIDC authentication.toml example#4123
Open
lunkwill42 wants to merge 5 commits into
Open
Conversation
The Pydantic rewrite (Uninett#4025) dropped OIDC's entry-level `scope`, which `OIDCProviderEntry` then rejected via `extra="forbid"`. Restore it as a real field and route it into the generated per-APP allauth settings, where the openid_connect provider reads it via `app.settings["scope"]`. Entry-level scope takes precedence over any value smuggled in through the `[...settings]` passthrough. This mirrors how `SocialProviderEntry` already exposes `scope`.
The bundled authentication.toml documented `scope` at the IdP entry level (now valid again) but also mis-spelled the settings section as `[oidc.dataporten-oidc.settings]`, dropping the `.idps.` segment. Copying the example verbatim produced a startup ValidationError. Correct the section path and apply the same fix to the stale docstring in the legacy OIDCConfigParser.
Align the reference guide with the shipped example: document `scope` at the IdP entry level rather than under `[...settings]`, and drop the `# scope = [],` line whose trailing comma is invalid TOML if uncommented.
Cover entry-level scope routing and its precedence over settings-level scope, plus a regression test that enables the examples in the shipped authentication.toml and asserts the whole file validates.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 5.19.x #4123 +/- ##
==========================================
- Coverage 65.39% 65.36% -0.03%
==========================================
Files 629 629
Lines 47191 47194 +3
==========================================
- Hits 30861 30850 -11
- Misses 16330 16344 +14 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
5 tasks
lunkwill42
commented
Jul 9, 2026
| } | ||
| # Pass through any extra provider-specific settings | ||
| settings.update(entry.settings.model_extra) | ||
| # Entry-level scope (the documented spelling) wins over any scope |
Contributor
There was a problem hiding this comment.
What I meant here is that I would suggest rather (the documented level)
johannaengland
self-requested a review
July 9, 2026 09:21
johannaengland
approved these changes
Jul 9, 2026
johannaengland
left a comment
johannaengland
left a comment
Contributor
There was a problem hiding this comment.
Just the nitpick about the comment
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
2 participants
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.
Scope and purpose
Fixes #4121.
The OIDC example shipped in
webfront/authentication.tomlno longer validated against the Pydantic config models introduced in #4025, so a user who copied it hit aValidationErrorand NAV refused to start. Two things were wrong:scopewas documented at the IdP entry level, whichOIDCProviderEntryrejected (extra="forbid"), and the settings section was mis-spelled[oidc.dataporten-oidc.settings], missing the.idps.segment.Rather than force
scopeinto the[...settings]sub-table, this restores it as a first-class entry-level field on OIDC providers — matching how social providers already exposescope— and routes it into allauth's per-APPsettings, where the openid_connect provider reads it. Entry-level scope takes precedence over any value passed through the settings passthrough. The shipped example and the reference docs are corrected to match, and a regression test now enables the bundled example and asserts the whole file validates.A reviewer can observe the effect by uncommenting the OIDC example block in
python/nav/etc/webfront/authentication.tomland starting NAV: before this change it raises aValidationErrorat startup, after it loads cleanly.Contributor Checklist
<major>.<minor>.x). For a new feature or other additions, it should be based onmaster.If applicable: Created new issues if this PR does not fix the issue completely/there is further work to be doneIf this results in changes in the UI: Added screenshots of the before and afterIf this adds a new Python source code file: Added the boilerplate header to that file