diff --git a/api/oidc.go b/api/oidc.go index 172ec7054..5ac15528f 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, + Prompt: conf.OIDC.Prompt, pendingSessions: decaymap.NewDecayMap[string, *pendingOIDCSession](time.Now(), pendingSessionMaxAge), } } @@ -94,6 +95,7 @@ type OIDCAPI struct { SecureCookie bool AutoRegister bool LinkByUsername bool + Prompt string pendingSessions *decaymap.DecayMap[string, *pendingOIDCSession] } @@ -131,7 +133,7 @@ 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) + rp.AuthURLHandler(func() string { return state }, a.Provider, a.promptURLParams()...)(w, r) }) } @@ -174,7 +176,16 @@ func (a *OIDCAPI) ElevateHandler(ctx *gin.Context) { return } a.pendingSessions.Set(time.Now(), state, &pendingOIDCSession{CreatedAt: time.Now(), Elevate: &elevate}) - rp.AuthURLHandler(func() string { return state }, a.Provider)(ctx.Writer, ctx.Request) + rp.AuthURLHandler(func() string { return state }, a.Provider, a.promptURLParams()...)(ctx.Writer, ctx.Request) +} + +// promptURLParams returns the `prompt` URL param option to send to the OIDC +// provider, if GOTIFY_OIDC_PROMPT is set to a non-empty value. +func (a *OIDCAPI) promptURLParams() []rp.URLParamOpt { + if a.Prompt == "" { + return nil + } + return []rp.URLParamOpt{rp.WithPromptURLParam(a.Prompt)} } // swagger:operation GET /auth/oidc/callback oidc oidcCallback diff --git a/api/oidc_test.go b/api/oidc_test.go index b0d5ee43a..6eed59918 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,82 @@ 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_Prompt() { + 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 + prompt string + wantPrompt string + }{ + {name: "default prompt", prompt: "login", wantPrompt: "login"}, + {name: "custom prompt", prompt: "consent", wantPrompt: "consent"}, + {name: "empty prompt disables the parameter", prompt: "", wantPrompt: ""}, + } + + for _, tc := range tests { + s.Run(tc.name, func() { + s.a.Prompt = tc.prompt + 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_ElevateHandler_Prompt() { + 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 + s.a.Prompt = "login" + + recorder := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(recorder) + ctx.Request = httptest.NewRequest("GET", "/auth/oidc/elevate?id=1&durationSeconds=60", nil) + + s.a.ElevateHandler(ctx) + + location, err := url.Parse(recorder.Header().Get("Location")) + assert.NoError(s.T(), err) + assert.Equal(s.T(), "login", 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..2a4c54fd1 100644 --- a/config/config.go +++ b/config/config.go @@ -69,6 +69,8 @@ type OIDC struct { LinkByUsername bool Scopes []string IDPName string + AutoRedirect bool + Prompt string } type Configuration struct { @@ -119,6 +121,7 @@ func Get() (*Configuration, []FutureLog) { AutoRegister: true, Scopes: []string{"openid", "profile", "email"}, IDPName: "OIDC", + Prompt: "login", }, } @@ -183,6 +186,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(parseOIDCPrompt(&c.OIDC.Prompt, EnvOIDCPrompt)) add(parseString(&c.NoColor, EnvNoColor)) diff --git a/config/config_test.go b/config/config_test.go index 548157fe9..71e405fa9 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -84,6 +84,126 @@ func TestLocalAuthDisabled(t *testing.T) { } } +func TestOIDCAutoRedirect(t *testing.T) { + tests := []struct { + name string + localAuthEnabled string + }{ + {name: "local auth disabled", localAuthEnabled: "false"}, + {name: "local auth enabled", localAuthEnabled: "true"}, + } + + 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() + // GOTIFY_OIDC_AUTO_REDIRECT always takes effect, regardless of + // GOTIFY_LOCALAUTH_ENABLED. Local admins opt out per-request via + // the WebUI's ?redirect=false login URL param. + assert.True(t, conf.OIDC.AutoRedirect) + + var warns []FutureLog + for _, entry := range logs { + if entry.Level == zerolog.WarnLevel { + warns = append(warns, entry) + } + } + assert.Empty(t, warns) + }) + } +} + +func TestOIDCPrompt(t *testing.T) { + tests := []struct { + name string + autoRedirect string + localAuthEnabled string + setPrompt bool + prompt string + want string + }{ + { + name: "defaults to login regardless of auto redirect", + autoRedirect: "false", + localAuthEnabled: "true", + want: "login", + }, + { + name: "custom prompt independent of auto redirect", + autoRedirect: "false", + localAuthEnabled: "true", + setPrompt: true, + prompt: "consent", + want: "consent", + }, + { + name: "empty prompt disables the parameter", + autoRedirect: "true", + localAuthEnabled: "false", + setPrompt: true, + prompt: "", + want: "", + }, + { + name: "space-delimited combination of valid values", + autoRedirect: "false", + localAuthEnabled: "true", + setPrompt: true, + prompt: "login consent", + want: "login consent", + }, + } + + 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) + if tc.setPrompt { + t.Setenv(EnvOIDCPrompt, tc.prompt) + } + + conf, _ := Get() + assert.Equal(t, tc.want, conf.OIDC.Prompt) + }) + } +} + +func TestOIDCPromptInvalid(t *testing.T) { + tests := []struct { + name string + prompt string + }{ + {name: "unknown value", prompt: "bogus"}, + {name: "unknown value combined with valid ones", prompt: "login bogus"}, + {name: "none combined with other values", prompt: "none login"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + mode.Set(mode.TestDev) + t.Setenv(EnvOIDCPrompt, tc.prompt) + + conf, logs := Get() + // The invalid value is rejected and the default is kept. + assert.Equal(t, "login", conf.OIDC.Prompt) + + var fatals []FutureLog + for _, entry := range logs { + if entry.Level == zerolog.FatalLevel { + fatals = append(fatals, entry) + } + } + assert.Len(t, fatals, 1) + }) + } +} + func TestFile(t *testing.T) { mode.Set(mode.TestDev) dir := t.TempDir() diff --git a/config/keys.go b/config/keys.go index c6bf7167f..8be8ae40f 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" + EnvOIDCPrompt = "GOTIFY_OIDC_PROMPT" EnvNoColor = "NOCOLOR" ) diff --git a/config/parse.go b/config/parse.go index 42558b479..1b8a23c88 100644 --- a/config/parse.go +++ b/config/parse.go @@ -112,3 +112,42 @@ func parseLogLevel(target *LogLevel, env string) error { } return target.Decode(raw) } + +// validOIDCPromptValues are the `prompt` values defined by the OIDC spec +// (https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). +var validOIDCPromptValues = map[string]bool{ + "none": true, + "login": true, + "consent": true, + "select_account": true, +} + +// parseOIDCPrompt parses GOTIFY_OIDC_PROMPT. Per the OIDC spec, it must be +// empty (to omit the prompt parameter) or a space-delimited combination of +// none, login, consent, select_account, where none must not be combined +// with the other values. +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 +} diff --git a/docs/spec.json b/docs/spec.json index 5692a37ba..8bfd7018c 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.", + "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..c7db71807 100644 --- a/gotify-server.env.example +++ b/gotify-server.env.example @@ -224,6 +224,26 @@ # 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. Users can +# still reach the login form by visiting the WebUI login route with +# ?redirect=false, e.g. https://push.example.com/#/login?redirect=false +# +# Type: boolean +# GOTIFY_OIDC_AUTO_REDIRECT=false + +# Value of the prompt parameter sent to the OIDC provider on login and +# session elevation. The default of login forces reauthentication, so that +# logging out of Gotify (or, with GOTIFY_OIDC_AUTO_REDIRECT, an existing IdP +# session) does not silently and invisibly log the user back in. This does +# not end that IdP session, so other applications using it are unaffected. +# Must be empty (to not send a prompt parameter) or a space-delimited +# combination of none, login, consent, select_account, where none must not +# be combined with the other values. See the OIDC spec for details. +# +# Type: text +# GOTIFY_OIDC_PROMPT=login + # Enable authentication via username and password. # Type: boolean # GOTIFY_LOCALAUTH_ENABLED=true diff --git a/model/gotifyinfo.go b/model/gotifyinfo.go index 526276efd..4d5499fd3 100644 --- a/model/gotifyinfo.go +++ b/model/gotifyinfo.go @@ -29,4 +29,10 @@ 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. + // + // 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..25a6d607d 100644 --- a/ui/src/user/Login.tsx +++ b/ui/src/user/Login.tsx @@ -5,11 +5,12 @@ 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'; import {observer} from 'mobx-react-lite'; -import {useNavigate} from 'react-router'; +import {useNavigate, useSearchParams} from 'react-router'; const Login = observer(() => { const [username, setUsername] = React.useState(''); @@ -17,15 +18,33 @@ const Login = observer(() => { const [registerDialog, setRegisterDialog] = React.useState(false); const {currentUser} = useStores(); const navigate = useNavigate(); + const [searchParams] = useSearchParams(); const localAuthEnabled = config.get('localAuth'); const oidcEnabled = config.get('oidc'); const oidcIdpName = config.get('oidcIdpName'); + // ?redirect=false lets users reach the login form even when + // GOTIFY_OIDC_AUTO_REDIRECT is enabled, e.g. so local admins can still + // sign in with a username and password. + const oidcAutoRedirect = + config.get('oidcAutoRedirect') && searchParams.get('redirect') !== 'false'; + 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, oidcAutoRedirect]); + + if (oidcAutoRedirect && !currentUser.loggedIn) { + return ; + } const registerButton = () => { if (localAuthEnabled && config.get('register')) return ( @@ -96,11 +115,7 @@ const Login = observer(() => {