feat: auto-redirect to OIDC IdP (#991) - #1029
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1029 +/- ##
==========================================
+ Coverage 74.78% 75.26% +0.48%
==========================================
Files 66 66
Lines 3541 3570 +29
==========================================
+ Hits 2648 2687 +39
+ Misses 688 674 -14
- Partials 205 209 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Add GOTIFY_OIDC_AUTO_REDIRECT to skip the login page and redirect straight to the configured OIDC provider. Only takes effect when local auth is disabled, since local login would otherwise be unreachable. Add GOTIFY_OIDC_AUTO_REDIRECT_REQUIRE_REAUTH (default false) to send prompt=login on that redirect, so logging out of Gotify doesn't silently log the user back in via an existing IdP session. Does not end that IdP session, so other apps using it are unaffected. Wire both flags through gotifyinfo/injected UI config and the WebUI login page, which now redirects instead of showing the OIDC button when enabled.
69d7eb8 to
d764fd9
Compare
jmattheis
left a comment
There was a problem hiding this comment.
Thanks for the contribution.
| # GOTIFY_OIDC_AUTO_REDIRECT is also in effect. | ||
| # | ||
| # Type: boolean | ||
| # GOTIFY_OIDC_AUTO_REDIRECT_REQUIRE_REAUTH=false |
There was a problem hiding this comment.
Can you rename this to GOTIFY_OIDC_PROMPT with type text and default login. This should be used for both login and elevate. And be independent of the auto redirect feature.
There was a problem hiding this comment.
Makes sense, but if we do that, since we then pass whatever is the value of that env var to the IdP as a url param, I would suggest we also check the value provided against the allowed values according to the oidc spec
I really dont want to pass any unvalidated data to a login system
There was a problem hiding this comment.
I think it's okay to do this unvalidated as this setting set by administrators. If the spec changes we'd have to adjust the validation, so I don't think there is much benefit in validating it.
There was a problem hiding this comment.
Well, yes, if a protocol you use changes, you have might have to adjust the code.
If i remember correctly, the OIDC spec follows semnatic versioning, that means future minor version bumps shouldnt remove existing params or values, only add to it.
I already added validation as func parseOIDCPrompt to config/parse.go
func parseOIDCPrompt(target *string, env string) error {
raw, ok, err := lookupEnv(env)
if err != nil {
return err
}
if !ok {
return nil
}
values := strings.Fields(raw)
hasNone := false
for _, value := range values {
if !validOIDCPromptValues[value] {
return fmt.Errorf(
"invalid value for %s (%q): must be a space-delimited combination of none, login, consent, select_account",
env, raw,
)
}
hasNone = hasNone || value == "none"
}
if hasNone && len(values) > 1 {
return fmt.Errorf("invalid value for %s (%q): none must not be combined with other values", env, raw)
}
*target = raw
return nil
}I think its worth doing, it protects gotify admins from faulty configs and protects OIDC IdP system from some injection vectors.
*GOTIFY_OIDC_AUTO_REDIRECT now takes effect reagardless of wether GOTIFY_LOCALAUTH_ENABLED is true or not * replaced GOTIFY_OIDC_AUTO_REDIRECT_REQUIRE_REAUTH with GOTIFY_OIDC_PROMPT: type string that can take in any combination of prompt params according to the oidc spec * added url param redirect to /login ui route, when set to false, auto redirect to idp is skipped and the login form is displayed, allowing local users to login with username and password
|
Im pretty confident the prompt params are correct and to spec, but I'm having trouble testing value combinations, it seems I have discovered a bug in authelia 🤷♂️ I will test mutliple prompt values against one of my dev keycloak instances later, but configuring keycloak is always such a pain in the a**, I doint have time for that right now |
This PR attempts to implement the missing part of #991
Add GOTIFY_OIDC_AUTO_REDIRECT (with default
false) to skip the login page and redirect straight to the configured OIDC provider. Only takes effect when local auth is disabled, since local login would otherwise be unreachable.Add GOTIFY_OIDC_AUTO_REDIRECT_REQUIRE_REAUTH (default false) to send prompt=login on that redirect, so logging out of Gotify doesn't silently log the user back in via an existing IdP session. Does not end that IdP session, so other apps using it are unaffected.
Wire both flags through gotifyinfo/injected UI config and the WebUI login page, which now redirects instead of showing the OIDC button when enabled.
Added helperfunction to emit warnigns via FutureLog in config builder/parser