From d764fd9d2af764bb386a31b3a88238c093376a10 Mon Sep 17 00:00:00 2001 From: Bernd Konrad Date: Sat, 15 Aug 2026 12:05:12 +0200 Subject: [PATCH 1/3] feat: auto-redirect to OIDC IdP on logout (#991) 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. --- api/oidc.go | 8 ++- api/oidc_test.go | 59 ++++++++++++++++++++ config/config.go | 18 +++++++ config/config_test.go | 111 ++++++++++++++++++++++++++++++++++++++ config/error.go | 7 +++ config/keys.go | 2 + docs/spec.json | 9 +++- gotify-server.env.example | 17 ++++++ model/gotifyinfo.go | 7 +++ router/router.go | 13 ++--- router/router_test.go | 53 +++++++++++++++++- ui/serve.go | 23 ++++---- ui/src/config.ts | 2 + ui/src/user/Login.tsx | 22 +++++--- 14 files changed, 326 insertions(+), 25 deletions(-) diff --git a/api/oidc.go b/api/oidc.go index 172ec7054..79f1d1640 100644 --- a/api/oidc.go +++ b/api/oidc.go @@ -66,6 +66,7 @@ func NewOIDC(conf *config.Configuration, db *database.GormDatabase, userChangeNo SecureCookie: conf.Server.SecureCookie, AutoRegister: conf.OIDC.AutoRegister, LinkByUsername: conf.OIDC.LinkByUsername, + PromptLogin: conf.OIDC.RequireReauth, pendingSessions: decaymap.NewDecayMap[string, *pendingOIDCSession](time.Now(), pendingSessionMaxAge), } } @@ -94,6 +95,7 @@ type OIDCAPI struct { SecureCookie bool AutoRegister bool LinkByUsername bool + PromptLogin bool pendingSessions *decaymap.DecayMap[string, *pendingOIDCSession] } @@ -131,7 +133,11 @@ func (a *OIDCAPI) LoginHandler() gin.HandlerFunc { return } a.pendingSessions.Set(time.Now(), state, &pendingOIDCSession{ClientName: clientName, CreatedAt: time.Now()}) - rp.AuthURLHandler(func() string { return state }, a.Provider)(w, r) + var urlParams []rp.URLParamOpt + if a.PromptLogin { + urlParams = append(urlParams, rp.WithPromptURLParam("login")) + } + rp.AuthURLHandler(func() string { return state }, a.Provider, urlParams...)(w, r) }) } diff --git a/api/oidc_test.go b/api/oidc_test.go index b0d5ee43a..ac0a3d9ef 100644 --- a/api/oidc_test.go +++ b/api/oidc_test.go @@ -1,7 +1,11 @@ package api import ( + "context" + "encoding/json" + "net/http" "net/http/httptest" + "net/url" "strings" "testing" "time" @@ -14,6 +18,7 @@ import ( "github.com/gotify/server/v2/test/testdb" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" + "github.com/zitadel/oidc/v3/pkg/client/rp" "github.com/zitadel/oidc/v3/pkg/oidc" ) @@ -62,6 +67,60 @@ func (s *OIDCSuite) Test_GenerateState_Unique() { assert.NotEqual(s.T(), s1, s2) } +// --- LoginHandler --- + +func newDiscoveryServer(t *testing.T) *httptest.Server { + t.Helper() + mux := http.NewServeMux() + server := httptest.NewServer(mux) + t.Cleanup(server.Close) + mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "issuer": server.URL, + "authorization_endpoint": server.URL + "/authorize", + "token_endpoint": server.URL + "/token", + "userinfo_endpoint": server.URL + "/userinfo", + "jwks_uri": server.URL + "/keys", + }) + }) + return server +} + +func (s *OIDCSuite) Test_LoginHandler_PromptLogin() { + issuer := newDiscoveryServer(s.T()) + + provider, err := rp.NewRelyingPartyOIDC( + context.Background(), issuer.URL, "client", "secret", "https://gotify.example/callback", []string{"openid"}, + ) + assert.NoError(s.T(), err) + s.a.Provider = provider + + tests := []struct { + name string + promptLogin bool + wantPrompt string + }{ + {name: "auto redirect enabled", promptLogin: true, wantPrompt: "login"}, + {name: "auto redirect disabled", promptLogin: false, wantPrompt: ""}, + } + + for _, tc := range tests { + s.Run(tc.name, func() { + s.a.PromptLogin = tc.promptLogin + recorder := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(recorder) + ctx.Request = httptest.NewRequest("GET", "/auth/oidc/login?name=testclient", nil) + + s.a.LoginHandler()(ctx) + + location, err := url.Parse(recorder.Header().Get("Location")) + assert.NoError(s.T(), err) + assert.Equal(s.T(), tc.wantPrompt, location.Query().Get("prompt")) + }) + } +} + func (s *OIDCSuite) Test_ResolveUser_ReturningUser_MatchedByOIDCID() { oidcID := testIssuer + "#sub-1" s.db.CreateUser(&model.User{ID: 1, Name: "alice", OIDCID: &oidcID}) diff --git a/config/config.go b/config/config.go index 85ece6bba..c3282b9c2 100644 --- a/config/config.go +++ b/config/config.go @@ -69,6 +69,8 @@ type OIDC struct { LinkByUsername bool Scopes []string IDPName string + AutoRedirect bool + RequireReauth bool } type Configuration struct { @@ -183,6 +185,8 @@ func Get() (*Configuration, []FutureLog) { add(parseBool(&c.OIDC.LinkByUsername, EnvOIDCLinkByUsername)) add(parseList(&c.OIDC.Scopes, EnvOIDCScopes)) add(parseString(&c.OIDC.IDPName, EnvOIDCIDPName)) + add(parseBool(&c.OIDC.AutoRedirect, EnvOIDCAutoRedirect)) + add(parseBool(&c.OIDC.RequireReauth, EnvOIDCAutoRedirectRequireReauth)) add(parseString(&c.NoColor, EnvNoColor)) @@ -194,6 +198,20 @@ func Get() (*Configuration, []FutureLog) { if c.Registration && !c.LocalAuthEnabled { logs = append(logs, futureFatal("registration requires local authentication to be enabled")) } + if c.OIDC.AutoRedirect && c.LocalAuthEnabled { + logs = append(logs, futureWarn( + "GOTIFY_OIDC_AUTO_REDIRECT has no effect while local authentication is enabled", + )) + } + c.OIDC.AutoRedirect = c.OIDC.AutoRedirect && !c.LocalAuthEnabled + + if c.OIDC.RequireReauth && !c.OIDC.AutoRedirect { + logs = append(logs, futureWarn( + "GOTIFY_OIDC_AUTO_REDIRECT_REQUIRE_REAUTH has no effect unless GOTIFY_OIDC_AUTO_REDIRECT is also in effect", + )) + } + c.OIDC.RequireReauth = c.OIDC.RequireReauth && c.OIDC.AutoRedirect + return c, logs } diff --git a/config/config_test.go b/config/config_test.go index 548157fe9..04f1febef 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -84,6 +84,117 @@ func TestLocalAuthDisabled(t *testing.T) { } } +func TestOIDCAutoRedirect(t *testing.T) { + tests := []struct { + name string + localAuthEnabled string + want bool + warns []FutureLog + }{ + { + name: "local auth disabled", + localAuthEnabled: "false", + want: true, + }, + { + name: "local auth enabled", + localAuthEnabled: "true", + want: false, + warns: []FutureLog{futureWarn( + "GOTIFY_OIDC_AUTO_REDIRECT has no effect while local authentication is enabled", + )}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + mode.Set(mode.TestDev) + t.Setenv(EnvOIDCAutoRedirect, "true") + t.Setenv(EnvLocalAuthEnabled, tc.localAuthEnabled) + t.Setenv(EnvOIDCEnabled, "true") + + conf, logs := Get() + assert.Equal(t, tc.want, conf.OIDC.AutoRedirect) + + var warns []FutureLog + for _, entry := range logs { + if entry.Level == zerolog.WarnLevel { + warns = append(warns, entry) + } + } + assert.Equal(t, tc.warns, warns) + }) + } +} + +func TestOIDCPromptLogin(t *testing.T) { + tests := []struct { + name string + autoRedirect string + localAuthEnabled string + requireReauth string + want bool + warns []FutureLog + }{ + { + name: "auto redirect and require reauth enabled", + autoRedirect: "true", + localAuthEnabled: "false", + requireReauth: "true", + want: true, + }, + { + name: "auto redirect enabled, require reauth left at default", + autoRedirect: "true", + localAuthEnabled: "false", + requireReauth: "false", + want: false, + }, + { + name: "require reauth enabled but auto redirect disabled", + autoRedirect: "false", + localAuthEnabled: "false", + requireReauth: "true", + want: false, + warns: []FutureLog{futureWarn( + "GOTIFY_OIDC_AUTO_REDIRECT_REQUIRE_REAUTH has no effect unless GOTIFY_OIDC_AUTO_REDIRECT is also in effect", + )}, + }, + { + name: "require reauth enabled but local auth enabled", + autoRedirect: "true", + localAuthEnabled: "true", + requireReauth: "true", + want: false, + warns: []FutureLog{ + futureWarn("GOTIFY_OIDC_AUTO_REDIRECT has no effect while local authentication is enabled"), + futureWarn("GOTIFY_OIDC_AUTO_REDIRECT_REQUIRE_REAUTH has no effect unless GOTIFY_OIDC_AUTO_REDIRECT is also in effect"), + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + mode.Set(mode.TestDev) + t.Setenv(EnvOIDCEnabled, "true") + t.Setenv(EnvOIDCAutoRedirect, tc.autoRedirect) + t.Setenv(EnvLocalAuthEnabled, tc.localAuthEnabled) + t.Setenv(EnvOIDCAutoRedirectRequireReauth, tc.requireReauth) + + conf, logs := Get() + assert.Equal(t, tc.want, conf.OIDC.RequireReauth) + + var warns []FutureLog + for _, entry := range logs { + if entry.Level == zerolog.WarnLevel { + warns = append(warns, entry) + } + } + assert.Equal(t, tc.warns, warns) + }) + } +} + func TestFile(t *testing.T) { mode.Set(mode.TestDev) dir := t.TempDir() diff --git a/config/error.go b/config/error.go index 7cdd65fa1..4ed2926c3 100644 --- a/config/error.go +++ b/config/error.go @@ -15,3 +15,10 @@ func futureFatal(msg string) FutureLog { Msg: msg, } } + +func futureWarn(msg string) FutureLog { + return FutureLog{ + Level: zerolog.WarnLevel, + Msg: msg, + } +} diff --git a/config/keys.go b/config/keys.go index c6bf7167f..c9463e9cd 100644 --- a/config/keys.go +++ b/config/keys.go @@ -43,5 +43,7 @@ const ( EnvLocalAuthEnabled = "GOTIFY_LOCALAUTH_ENABLED" EnvOIDCScopes = "GOTIFY_OIDC_SCOPES" EnvOIDCIDPName = "GOTIFY_OIDC_IDP_NAME" + EnvOIDCAutoRedirect = "GOTIFY_OIDC_AUTO_REDIRECT" + EnvOIDCAutoRedirectRequireReauth = "GOTIFY_OIDC_AUTO_REDIRECT_REQUIRE_REAUTH" EnvNoColor = "NOCOLOR" ) diff --git a/docs/spec.json b/docs/spec.json index 5692a37ba..9d535efb1 100644 --- a/docs/spec.json +++ b/docs/spec.json @@ -2948,7 +2948,8 @@ "register", "localAuth", "oidc", - "oidcIdpName" + "oidcIdpName", + "oidcAutoRedirect" ], "properties": { "localAuth": { @@ -2963,6 +2964,12 @@ "x-go-name": "Oidc", "example": true }, + "oidcAutoRedirect": { + "description": "If the WebUI should automatically redirect to the OIDC identity\nprovider instead of showing the login page. Always false while local\nauthentication is enabled.", + "type": "boolean", + "x-go-name": "OIDCAutoRedirect", + "example": false + }, "oidcIdpName": { "description": "Name of the OIDC identity provider.", "type": "string", diff --git a/gotify-server.env.example b/gotify-server.env.example index e8e734460..0048b921f 100644 --- a/gotify-server.env.example +++ b/gotify-server.env.example @@ -224,6 +224,23 @@ # Type: text-list # GOTIFY_OIDC_SCOPES=openid,profile,email +# Automatically redirect to the OIDC identity provider instead of showing the +# login page. Only takes effect if GOTIFY_OIDC_ENABLED is true and +# GOTIFY_LOCALAUTH_ENABLED is false, since local login would otherwise be +# unreachable. +# +# Type: boolean +# GOTIFY_OIDC_AUTO_REDIRECT=false + +# Send prompt=login to the OIDC provider on auto-redirect, so that logging +# out of Gotify does not silently and invisibly log the user back in via an +# existing IdP session. This does not end that IdP session, so other +# applications using it are unaffected. Only takes effect if +# GOTIFY_OIDC_AUTO_REDIRECT is also in effect. +# +# Type: boolean +# GOTIFY_OIDC_AUTO_REDIRECT_REQUIRE_REAUTH=false + # Enable authentication via username and password. # Type: boolean # GOTIFY_LOCALAUTH_ENABLED=true diff --git a/model/gotifyinfo.go b/model/gotifyinfo.go index 526276efd..547094dd3 100644 --- a/model/gotifyinfo.go +++ b/model/gotifyinfo.go @@ -29,4 +29,11 @@ type GotifyInfo struct { // required: true // example: OIDC OIDCIDPName string `json:"oidcIdpName"` + // If the WebUI should automatically redirect to the OIDC identity + // provider instead of showing the login page. Always false while local + // authentication is enabled. + // + // required: true + // example: false + OIDCAutoRedirect bool `json:"oidcAutoRedirect"` } diff --git a/router/router.go b/router/router.go index e65842c0d..a1e92293c 100644 --- a/router/router.go +++ b/router/router.go @@ -120,7 +120,7 @@ func Create(db *database.GormDatabase, vInfo *model.VersionInfo, conf *config.Co userChangeNotifier.OnUserDeleted(pluginManager.RemoveUser) userChangeNotifier.OnUserAdded(pluginManager.InitializeForUserID) - ui.Register(g, *vInfo, conf.Registration, conf.LocalAuthEnabled, conf.OIDC.Enabled, conf.OIDC.IDPName) + ui.Register(g, *vInfo, conf.Registration, conf.LocalAuthEnabled, conf.OIDC.Enabled, conf.OIDC.IDPName, conf.OIDC.AutoRedirect) if conf.OIDC.Enabled { oidcHandler := api.NewOIDC(conf, db, userChangeNotifier) @@ -192,11 +192,12 @@ func Create(db *database.GormDatabase, vInfo *model.VersionInfo, conf *config.Co // $ref: "#/definitions/GotifyInfo" g.GET("gotifyinfo", func(ctx *gin.Context) { ctx.JSON(200, &model.GotifyInfo{ - Version: vInfo.Version, - Oidc: conf.OIDC.Enabled, - Register: conf.Registration, - LocalAuth: conf.LocalAuthEnabled, - OIDCIDPName: conf.OIDC.IDPName, + Version: vInfo.Version, + Oidc: conf.OIDC.Enabled, + Register: conf.Registration, + LocalAuth: conf.LocalAuthEnabled, + OIDCIDPName: conf.OIDC.IDPName, + OIDCAutoRedirect: conf.OIDC.AutoRedirect, }) }) diff --git a/router/router_test.go b/router/router_test.go index cd6f7864a..fc33f47ef 100644 --- a/router/router_test.go +++ b/router/router_test.go @@ -66,8 +66,59 @@ func (s *IntegrationSuite) TestVersionInfo() { func (s *IntegrationSuite) TestGotifyInfo() { req := s.newRequest("GET", "gotifyinfo", "") + doRequestAndExpect(s.T(), req, 200, `{"version":"1.0.0", "oidc":false, "register":false, "localAuth":true, "oidcIdpName":"Company XYZ SSO", "oidcAutoRedirect":false}`) +} - doRequestAndExpect(s.T(), req, 200, `{"version":"1.0.0", "oidc":false, "register":false, "localAuth":true, "oidcIdpName":"Company XYZ SSO"}`) +func TestGotifyInfo_OIDCAutoRedirect(t *testing.T) { + tests := []struct { + name string + autoRedirect bool + want string + }{ + { + name: "auto redirect enabled", + autoRedirect: true, + want: `{"version":"1.0.0", "oidc":false, "register":false, "localAuth":true, "oidcAutoRedirect":true, "oidcIdpName":""}`, + }, + { + name: "auto redirect disabled", + autoRedirect: false, + want: `{"version":"1.0.0", "oidc":false, "register":false, "localAuth":true, "oidcAutoRedirect":false, "oidcIdpName":""}`, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + mode.Set(mode.TestDev) + db := testdb.NewDBWithDefaultUser(t) + defer db.Close() + + g, closable := Create( + db.GormDatabase, + &model.VersionInfo{Version: "1.0.0", BuildDate: "2018-02-20-17:30:47", Commit: "asdasds"}, + &config.Configuration{ + PassStrength: 5, + LocalAuthEnabled: true, + OIDC: config.OIDC{AutoRedirect: tc.autoRedirect}, + }, + ) + server := httptest.NewServer(g) + + defer func() { + closable() + server.Close() + }() + + req, err := http.NewRequest("GET", fmt.Sprintf("%s/%s", server.URL, "gotifyinfo"), nil) + assert.Nil(t, err) + + res, err := client.Do(req) + assert.Nil(t, err) + buf := new(bytes.Buffer) + buf.ReadFrom(res.Body) + assert.JSONEq(t, tc.want, buf.String()) + }) + } } func (s *IntegrationSuite) TestHeaderInProd() { diff --git a/ui/serve.go b/ui/serve.go index afe2e10d4..635ce77bf 100644 --- a/ui/serve.go +++ b/ui/serve.go @@ -16,11 +16,12 @@ import ( var box embed.FS type uiConfig struct { - Register bool `json:"register"` - Version model.VersionInfo `json:"version"` - LocalAuth bool `json:"localAuth"` - OIDC bool `json:"oidc"` - OIDCIDPName string `json:"oidcIdpName"` + Register bool `json:"register"` + Version model.VersionInfo `json:"version"` + LocalAuth bool `json:"localAuth"` + OIDC bool `json:"oidc"` + OIDCIDPName string `json:"oidcIdpName"` + OIDCAutoRedirect bool `json:"oidcAutoRedirect"` } // Register registers the ui on the root path. @@ -31,13 +32,15 @@ func Register( localAuthEnabled bool, oidcEnabled bool, oidcIDPName string, + oidcAutoRedirect bool, ) { uiConfigBytes, err := json.Marshal(uiConfig{ - Version: version, - Register: register, - LocalAuth: localAuthEnabled, - OIDC: oidcEnabled, - OIDCIDPName: oidcIDPName, + Version: version, + Register: register, + LocalAuth: localAuthEnabled, + OIDC: oidcEnabled, + OIDCIDPName: oidcIDPName, + OIDCAutoRedirect: oidcAutoRedirect, }) if err != nil { panic(err) diff --git a/ui/src/config.ts b/ui/src/config.ts index be57bd213..b88baefdc 100644 --- a/ui/src/config.ts +++ b/ui/src/config.ts @@ -5,6 +5,7 @@ export interface IConfig { register: boolean; version: IVersion; oidc: boolean; + oidcAutoRedirect: boolean; localAuth: boolean; oidcIdpName: string; } @@ -20,6 +21,7 @@ const config: IConfig = { register: false, version: {commit: 'unknown', buildDate: 'unknown', version: 'unknown'}, oidc: false, + oidcAutoRedirect: false, localAuth: true, oidcIdpName: 'OIDC', ...window.config, diff --git a/ui/src/user/Login.tsx b/ui/src/user/Login.tsx index 8e6572a20..28b1dd2fc 100644 --- a/ui/src/user/Login.tsx +++ b/ui/src/user/Login.tsx @@ -5,6 +5,7 @@ import TextField from '@mui/material/TextField'; import React from 'react'; import Container from '../common/Container'; import DefaultPage from '../common/DefaultPage'; +import LoadingSpinner from '../common/LoadingSpinner'; import * as config from '../config'; import RegistrationDialog from './Register'; import {useStores} from '../stores'; @@ -21,11 +22,24 @@ const Login = observer(() => { const oidcEnabled = config.get('oidc'); const oidcIdpName = config.get('oidcIdpName'); + const oidcAutoRedirect = config.get('oidcAutoRedirect'); + const oidcLoginUrl = + config.get('url') + + 'auth/oidc/login?name=' + + encodeURIComponent(currentUser.createClientName()); React.useEffect(() => { if (currentUser.loggedIn) { navigate('/'); + return; } - }, [currentUser.loggedIn]); + if (!currentUser.authenticating && oidcAutoRedirect) { + window.location.href = oidcLoginUrl; + } + }, [currentUser.loggedIn, currentUser.authenticating]); + + if (oidcAutoRedirect && !currentUser.loggedIn) { + return ; + } const registerButton = () => { if (localAuthEnabled && config.get('register')) return ( @@ -96,11 +110,7 @@ const Login = observer(() => {