diff --git a/api/oidc.go b/api/oidc.go index a686f78a..91e2acf1 100644 --- a/api/oidc.go +++ b/api/oidc.go @@ -8,6 +8,8 @@ import ( "fmt" "io" "net/http" + "net/url" + "strings" "time" "github.com/gin-gonic/gin" @@ -56,6 +58,7 @@ func NewOIDC(conf *config.Configuration, db *database.GormDatabase, userChangeNo PasswordStrength: conf.PassStrength, SecureCookie: conf.Server.SecureCookie, AutoRegister: conf.OIDC.AutoRegister, + LinkByUsername: conf.OIDC.LinkByUsername, pendingSessions: decaymap.NewDecayMap[string, *pendingOIDCSession](time.Now(), pendingSessionMaxAge), } } @@ -83,6 +86,7 @@ type OIDCAPI struct { PasswordStrength int SecureCookie bool AutoRegister bool + LinkByUsername bool pendingSessions *decaymap.DecayMap[string, *pendingOIDCSession] } @@ -196,7 +200,7 @@ func (a *OIDCAPI) ElevateHandler(ctx *gin.Context) { // $ref: "#/definitions/Error" func (a *OIDCAPI) CallbackHandler() gin.HandlerFunc { callback := func(w http.ResponseWriter, r *http.Request, tokens *oidc.Tokens[*oidc.IDTokenClaims], state string, provider rp.RelyingParty, info *oidc.UserInfo) { - user, status, err := a.resolveUser(info) + user, status, err := a.resolveUser(tokens.IDTokenClaims.GetIssuer(), info) if err != nil { http.Error(w, err.Error(), status) return @@ -362,7 +366,7 @@ func (a *OIDCAPI) ExternalTokenHandler(ctx *gin.Context) { ctx.AbortWithError(http.StatusInternalServerError, fmt.Errorf("failed to get user info: %w", err)) return } - user, status, resolveErr := a.resolveUser(info) + user, status, resolveErr := a.resolveUser(tokens.IDTokenClaims.GetIssuer(), info) if resolveErr != nil { ctx.AbortWithError(status, resolveErr) return @@ -386,32 +390,83 @@ func (a *OIDCAPI) generateState() (string, error) { return hex.EncodeToString(nonce), nil } -// resolveUser looks up or creates a user from OIDC userinfo claims. -func (a *OIDCAPI) resolveUser(info *oidc.UserInfo) (*model.User, int, error) { +// resolveUser looks up, links, or creates the user bound to an OIDC identity. +// +// 1. Look up the user by OIDC id (#). If found, use it. +// 2. Otherwise look up a user by the username claim. If one exists, link it to +// this OIDC identity, which requires GOTIFY_OIDC_LINK_BY_USERNAME and +// that the user is not already bound to a different identity. +// 3. Otherwise auto-register a new user, which requires GOTIFY_OIDC_AUTOREGISTER. +func (a *OIDCAPI) resolveUser(issuer string, info *oidc.UserInfo) (*model.User, int, error) { + if issuer == "" { + return nil, http.StatusInternalServerError, errors.New("issuer claim was empty") + } + if _, err := url.Parse(issuer); err != nil { + return nil, http.StatusInternalServerError, fmt.Errorf("issuer url %q is not a valid url: %w", issuer, err) + } + if strings.Contains(issuer, "#") { + return nil, http.StatusInternalServerError, fmt.Errorf("issuer url %q may not contain a fragment", issuer) + } + subject := info.GetSubject() + if subject == "" { + return nil, http.StatusInternalServerError, errors.New("subject claim was empty") + } + oidcID := issuer + "#" + subject + + user, err := a.DB.GetUserByOIDC(oidcID) + if err != nil { + return nil, http.StatusInternalServerError, fmt.Errorf("database error: %w", err) + } + if user != nil { + return user, 0, nil + } + usernameRaw, ok := info.Claims[a.UsernameClaim] if !ok { return nil, http.StatusInternalServerError, fmt.Errorf("username claim %q is missing", a.UsernameClaim) } username := fmt.Sprint(usernameRaw) if username == "" || usernameRaw == nil { - return nil, http.StatusInternalServerError, fmt.Errorf("username claim was empty") + return nil, http.StatusInternalServerError, errors.New("username claim was empty") } - user, err := a.DB.GetUserByName(username) + byUsername, err := a.DB.GetUserByName(username) if err != nil { return nil, http.StatusInternalServerError, fmt.Errorf("database error: %w", err) } - if user == nil { - if !a.AutoRegister { - return nil, http.StatusForbidden, fmt.Errorf("user does not exist and auto-registration is disabled") - } - user = &model.User{Name: username, Admin: false, Pass: nil} - if err := a.DB.CreateUser(user); err != nil { - return nil, http.StatusInternalServerError, fmt.Errorf("failed to create user: %w", err) - } - if err := a.UserChangeNotifier.fireUserAdded(user.ID); err != nil { - log.Error().Err(err).Uint("user_id", user.ID).Msg("Could not notify user change") - } + if byUsername != nil { + return a.linkExistingUser(byUsername, oidcID) + } + return a.registerUser(username, oidcID) +} + +func (a *OIDCAPI) linkExistingUser(user *model.User, oidcID string) (*model.User, int, error) { + if !a.LinkByUsername { + log.Warn().Str("oidc_id", oidcID).Str("username", user.Name).Msgf("OIDC login rejected: a local user with the username already exists and %s is disabled", config.EnvOIDCLinkByUsername) + return nil, http.StatusForbidden, fmt.Errorf("a local user with the username %s already exists and linking by username is disabled", user.Name) + } + if user.OIDCID != nil { + log.Warn().Str("oidc_id", oidcID).Str("bound_oidc_id", *user.OIDCID).Str("username", user.Name).Msg("OIDC login rejected: the username is already bound to a different OIDC identity") + return nil, http.StatusForbidden, fmt.Errorf("the user %s is already bound to a different OIDC identity", user.Name) + } + user.OIDCID = &oidcID + if err := a.DB.UpdateUser(user); err != nil { + return nil, http.StatusInternalServerError, fmt.Errorf("failed to bind user to OIDC identity: %w", err) + } + return user, 0, nil +} + +func (a *OIDCAPI) registerUser(username, oidcID string) (*model.User, int, error) { + if !a.AutoRegister { + return nil, http.StatusForbidden, errors.New("user does not exist and auto-registration is disabled") + } + user := &model.User{Name: username, Admin: false, Pass: nil, OIDCID: &oidcID} + if err := a.DB.CreateUser(user); err != nil { + return nil, http.StatusInternalServerError, fmt.Errorf("failed to create user: %w", err) + } + log.Info().Str("oidc_id", oidcID).Str("username", user.Name).Msg("OIDC auto registration") + if err := a.UserChangeNotifier.fireUserAdded(user.ID); err != nil { + log.Error().Err(err).Uint("user_id", user.ID).Msg("Could not notify user change") } return user, 0, nil } diff --git a/api/oidc_test.go b/api/oidc_test.go index 473e57f6..0bd8424b 100644 --- a/api/oidc_test.go +++ b/api/oidc_test.go @@ -10,6 +10,7 @@ import ( "github.com/gotify/server/v2/auth" "github.com/gotify/server/v2/decaymap" "github.com/gotify/server/v2/mode" + "github.com/gotify/server/v2/model" "github.com/gotify/server/v2/test" "github.com/gotify/server/v2/test/testdb" "github.com/stretchr/testify/assert" @@ -19,6 +20,8 @@ import ( var origGenClientToken = generateClientToken +const testIssuer = "https://idp.example.com" + func TestOIDCSuite(t *testing.T) { suite.Run(t, new(OIDCSuite)) } @@ -62,83 +65,172 @@ func (s *OIDCSuite) Test_GenerateState_Unique() { assert.NotEqual(s.T(), s1, s2) } -func (s *OIDCSuite) Test_ResolveUser_ExistingUser() { - s.db.NewUserWithName(1, "alice") +func (s *OIDCSuite) Test_ResolveUser_ReturningUser_MatchedByOIDCID() { + oidcID := testIssuer + "#sub-1" + s.db.CreateUser(&model.User{ID: 1, Name: "alice", OIDCID: &oidcID}) - info := &oidc.UserInfo{Claims: map[string]any{"preferred_username": "alice"}} - user, status, err := s.a.resolveUser(info) + // The username claim differs from the stored name; the binding still matches. + info := &oidc.UserInfo{Subject: "sub-1", Claims: map[string]any{"preferred_username": "renamed"}} + user, status, err := s.a.resolveUser(testIssuer, info) assert.NoError(s.T(), err) assert.Equal(s.T(), 0, status) + assert.Equal(s.T(), uint(1), user.ID) assert.Equal(s.T(), "alice", user.Name) + assert.False(s.T(), s.notified) +} + +func (s *OIDCSuite) Test_ResolveUser_LinkByUsername_BindsExistingUser() { + s.a.LinkByUsername = true + s.db.NewUserWithName(1, "alice") + + info := &oidc.UserInfo{Subject: "sub-1", Claims: map[string]any{"preferred_username": "alice"}} + user, _, err := s.a.resolveUser(testIssuer, info) + + assert.NoError(s.T(), err) assert.Equal(s.T(), uint(1), user.ID) + assert.NotNil(s.T(), user.OIDCID) + assert.Equal(s.T(), testIssuer+"#sub-1", *user.OIDCID) + // Binding an existing user is not a registration, so no notification. assert.False(s.T(), s.notified) + + bound, err := s.db.GetUserByOIDC(testIssuer + "#sub-1") + assert.NoError(s.T(), err) + assert.NotNil(s.T(), bound) + assert.Equal(s.T(), uint(1), bound.ID) +} + +func (s *OIDCSuite) Test_ResolveUser_InvalidIssuer() { + s.db.NewUserWithName(1, "alice") + + info := &oidc.UserInfo{Subject: "sub-1", Claims: map[string]any{"preferred_username": "alice"}} + _, status, err := s.a.resolveUser("://example.org", info) + + assert.EqualError(s.T(), err, `issuer url "://example.org" is not a valid url: parse "://example.org": missing protocol scheme`) + assert.Equal(s.T(), 500, status) +} + +func (s *OIDCSuite) Test_ResolveUser_InvalidIssuer_containsFragment() { + s.db.NewUserWithName(1, "alice") + + info := &oidc.UserInfo{Subject: "sub-1", Claims: map[string]any{"preferred_username": "alice"}} + _, status, err := s.a.resolveUser(testIssuer+"#", info) + + assert.EqualError(s.T(), err, `issuer url "https://idp.example.com#" may not contain a fragment`) + assert.Equal(s.T(), 500, status) +} + +func (s *OIDCSuite) Test_ResolveUser_LinkDisabled_RejectsExistingUsername() { + s.db.NewUserWithName(1, "alice") + + info := &oidc.UserInfo{Subject: "sub-1", Claims: map[string]any{"preferred_username": "alice"}} + _, status, err := s.a.resolveUser(testIssuer, info) + + assert.EqualError(s.T(), err, "a local user with the username alice already exists and linking by username is disabled") + assert.Equal(s.T(), 403, status) + + // The existing user must not have been bound. + user, _ := s.db.GetUserByName("alice") + assert.Nil(s.T(), user.OIDCID) +} + +func (s *OIDCSuite) Test_ResolveUser_LinkByUsername_RejectsDifferentIdentity() { + s.a.LinkByUsername = true + otherID := testIssuer + "#other-sub" + s.db.CreateUser(&model.User{ID: 1, Name: "alice", OIDCID: &otherID}) + + info := &oidc.UserInfo{Subject: "sub-1", Claims: map[string]any{"preferred_username": "alice"}} + _, status, err := s.a.resolveUser(testIssuer, info) + + assert.EqualError(s.T(), err, "the user alice is already bound to a different OIDC identity") + assert.Equal(s.T(), 403, status) } func (s *OIDCSuite) Test_ResolveUser_AutoRegister() { - info := &oidc.UserInfo{Claims: map[string]any{"preferred_username": "newuser"}} - user, status, err := s.a.resolveUser(info) + info := &oidc.UserInfo{Subject: "sub-1", Claims: map[string]any{"preferred_username": "newuser"}} + user, status, err := s.a.resolveUser(testIssuer, info) assert.NoError(s.T(), err) assert.Equal(s.T(), 0, status) assert.Equal(s.T(), "newuser", user.Name) assert.False(s.T(), user.Admin) + assert.NotNil(s.T(), user.OIDCID) + assert.Equal(s.T(), testIssuer+"#sub-1", *user.OIDCID) assert.True(s.T(), s.notified) - // verify persisted - dbUser, err := s.db.GetUserByName("newuser") + // Verify persisted and bound. + dbUser, err := s.db.GetUserByOIDC(testIssuer + "#sub-1") assert.NoError(s.T(), err) assert.NotNil(s.T(), dbUser) + assert.Equal(s.T(), "newuser", dbUser.Name) } func (s *OIDCSuite) Test_ResolveUser_AutoRegisterDisabled() { s.a.AutoRegister = false - info := &oidc.UserInfo{Claims: map[string]any{"preferred_username": "newuser"}} + info := &oidc.UserInfo{Subject: "sub-1", Claims: map[string]any{"preferred_username": "newuser"}} - _, status, err := s.a.resolveUser(info) + _, status, err := s.a.resolveUser(testIssuer, info) - assert.Error(s.T(), err) + assert.EqualError(s.T(), err, "user does not exist and auto-registration is disabled") assert.Equal(s.T(), 403, status) s.db.AssertUsernameNotExist("newuser") } +func (s *OIDCSuite) Test_ResolveUser_MissingIssuer() { + info := &oidc.UserInfo{Subject: "sub-1", Claims: map[string]any{"preferred_username": "newuser"}} + + _, status, err := s.a.resolveUser("", info) + + assert.EqualError(s.T(), err, "issuer claim was empty") + assert.Equal(s.T(), 500, status) +} + +func (s *OIDCSuite) Test_ResolveUser_MissingSubject() { + info := &oidc.UserInfo{Claims: map[string]any{"preferred_username": "newuser"}} + + _, status, err := s.a.resolveUser(testIssuer, info) + + assert.EqualError(s.T(), err, "subject claim was empty") + assert.Equal(s.T(), 500, status) +} + func (s *OIDCSuite) Test_ResolveUser_MissingClaim() { - info := &oidc.UserInfo{Claims: map[string]any{}} + info := &oidc.UserInfo{Subject: "sub-1", Claims: map[string]any{}} - _, status, err := s.a.resolveUser(info) + _, status, err := s.a.resolveUser(testIssuer, info) - assert.Error(s.T(), err) + assert.EqualError(s.T(), err, `username claim "preferred_username" is missing`) assert.Equal(s.T(), 500, status) - assert.Contains(s.T(), err.Error(), "preferred_username") } func (s *OIDCSuite) Test_ResolveUser_EmptyClaim() { - info := &oidc.UserInfo{Claims: map[string]any{"preferred_username": ""}} + info := &oidc.UserInfo{Subject: "sub-1", Claims: map[string]any{"preferred_username": ""}} - _, status, err := s.a.resolveUser(info) + _, status, err := s.a.resolveUser(testIssuer, info) - assert.Error(s.T(), err) + assert.EqualError(s.T(), err, "username claim was empty") assert.Equal(s.T(), 500, status) } func (s *OIDCSuite) Test_ResolveUser_NilClaim() { - info := &oidc.UserInfo{Claims: map[string]any{"preferred_username": nil}} + info := &oidc.UserInfo{Subject: "sub-1", Claims: map[string]any{"preferred_username": nil}} - _, status, err := s.a.resolveUser(info) + _, status, err := s.a.resolveUser(testIssuer, info) - assert.Error(s.T(), err) + assert.EqualError(s.T(), err, "username claim was empty") assert.Equal(s.T(), 500, status) } func (s *OIDCSuite) Test_ResolveUser_CustomClaim() { s.a.UsernameClaim = "email" - s.db.NewUserWithName(1, "alice@example.com") - info := &oidc.UserInfo{Claims: map[string]any{"email": "alice@example.com"}} - user, _, err := s.a.resolveUser(info) + info := &oidc.UserInfo{Subject: "sub-1", Claims: map[string]any{"email": "new@example.com"}} + user, status, err := s.a.resolveUser(testIssuer, info) assert.NoError(s.T(), err) - assert.Equal(s.T(), "alice@example.com", user.Name) + assert.Equal(s.T(), 0, status) + assert.Equal(s.T(), "new@example.com", user.Name) + assert.NotNil(s.T(), user.OIDCID) } // --- createClient --- @@ -170,6 +262,7 @@ func (s *OIDCSuite) Test_ExternalAuthorizeHandler_MissingFields() { s.a.ExternalAuthorizeHandler(s.ctx) assert.Equal(s.T(), 400, s.recorder.Code) + assert.Contains(s.T(), s.ctx.Errors.Last().Error(), "'CodeChallenge' failed on the 'required' tag") } // --- ExternalTokenHandler --- @@ -181,6 +274,7 @@ func (s *OIDCSuite) Test_ExternalTokenHandler_InvalidJSON() { s.a.ExternalTokenHandler(s.ctx) assert.Equal(s.T(), 400, s.recorder.Code) + assert.Contains(s.T(), s.ctx.Errors.Last().Error(), "invalid character") } func (s *OIDCSuite) Test_ExternalTokenHandler_UnknownState() { @@ -192,4 +286,5 @@ func (s *OIDCSuite) Test_ExternalTokenHandler_UnknownState() { s.a.ExternalTokenHandler(s.ctx) assert.Equal(s.T(), 400, s.recorder.Code) + assert.EqualError(s.T(), s.ctx.Errors.Last(), "unknown or expired state") } diff --git a/config/config.go b/config/config.go index bc3f6f65..71a9963c 100644 --- a/config/config.go +++ b/config/config.go @@ -59,14 +59,15 @@ type DefaultUser struct { } type OIDC struct { - Enabled bool - Issuer string - ClientID string - ClientSecret string - UsernameClaim string - RedirectURL string - AutoRegister bool - Scopes []string + Enabled bool + Issuer string + ClientID string + ClientSecret string + UsernameClaim string + RedirectURL string + AutoRegister bool + LinkByUsername bool + Scopes []string } type Configuration struct { @@ -174,6 +175,7 @@ func Get() (*Configuration, []FutureLog) { add(parseString(&c.OIDC.UsernameClaim, EnvOIDCUsernameClaim)) add(parseString(&c.OIDC.RedirectURL, EnvOIDCRedirectURL)) add(parseBool(&c.OIDC.AutoRegister, EnvOIDCAutoRegister)) + add(parseBool(&c.OIDC.LinkByUsername, EnvOIDCLinkByUsername)) add(parseList(&c.OIDC.Scopes, EnvOIDCScopes)) add(parseString(&c.NoColor, EnvNoColor)) diff --git a/config/keys.go b/config/keys.go index d01bcdcb..6578fd13 100644 --- a/config/keys.go +++ b/config/keys.go @@ -39,6 +39,7 @@ const ( EnvOIDCUsernameClaim = "GOTIFY_OIDC_USERNAMECLAIM" EnvOIDCRedirectURL = "GOTIFY_OIDC_REDIRECTURL" EnvOIDCAutoRegister = "GOTIFY_OIDC_AUTOREGISTER" + EnvOIDCLinkByUsername = "GOTIFY_OIDC_LINK_BY_USERNAME" EnvOIDCScopes = "GOTIFY_OIDC_SCOPES" EnvNoColor = "NOCOLOR" ) diff --git a/database/user.go b/database/user.go index 8e6bab35..2f3b435b 100644 --- a/database/user.go +++ b/database/user.go @@ -18,6 +18,19 @@ func (d *GormDatabase) GetUserByName(name string) (*model.User, error) { return nil, err } +// GetUserByOIDC returns the user bound to the given oidc id or nil. +func (d *GormDatabase) GetUserByOIDC(oidcID string) (*model.User, error) { + user := new(model.User) + err := d.DB.Where("oidc_id = ?", oidcID).Find(user).Error + if err == gorm.ErrRecordNotFound { + err = nil + } + if user.OIDCID != nil && *user.OIDCID == oidcID { + return user, err + } + return nil, err +} + // GetUserByID returns the user by the given id or nil. func (d *GormDatabase) GetUserByID(id uint) (*model.User, error) { user := new(model.User) diff --git a/gotify-server.env.example b/gotify-server.env.example index e59f9a42..c0b23d55 100644 --- a/gotify-server.env.example +++ b/gotify-server.env.example @@ -205,6 +205,14 @@ # Type: boolean # GOTIFY_OIDC_AUTOREGISTER=true +# Bind an OIDC identity to a pre-existing local user with a matching username +# on first login. When disabled (default), existing local users are never +# claimed by an OIDC login and an identity whose username is already taken is +# rejected +# +# Type: boolean +# GOTIFY_OIDC_LINK_BY_USERNAME=false + # OIDC ID-token claim used as the local username. Common values are # preferred_username or email. # diff --git a/model/user.go b/model/user.go index 2e70582f..6490fd28 100644 --- a/model/user.go +++ b/model/user.go @@ -12,6 +12,8 @@ type User struct { Applications []Application Clients []Client Plugins []PluginConf + // Format: OIDC claims combined as "#". + OIDCID *string `gorm:"column:oidc_id;type:text;uniqueIndex:uix_users_oidc_id,length:512"` } // UserExternal Model diff --git a/ui/src/tests/dex.ts b/ui/src/tests/dex.ts new file mode 100644 index 00000000..21858b38 --- /dev/null +++ b/ui/src/tests/dex.ts @@ -0,0 +1,121 @@ +import {spawn, execFileSync} from 'child_process'; +import getPort from 'get-port'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import {rimrafSync} from 'rimraf'; +import {stringify} from 'yaml'; +// @ts-expect-error no types +import wait from 'wait-on'; + +// All dex test users share this password. The hash is bcrypt("password"). +export const DEX_PASSWORD = 'password'; +const DEX_PASSWORD_HASH = '$2a$10$2b2cU8CPhOTaGrs1HRQuAueS7JTT5ZHsHSzYiFPm1leZck7Mc8T4W'; + +const DEX_IMAGE = 'ghcr.io/dexidp/dex:v2.45.1-alpine'; + +export interface DexUser { + email: string; + username: string; + userID: string; +} + +export interface DexInstance { + issuer: string; + close: () => void; +} + +const dexConfig = (issuerPort: number, redirectURL: string, users: DexUser[]): string => + stringify({ + issuer: `http://127.0.0.1:${issuerPort}/dex`, + storage: {type: 'memory'}, + web: {http: '0.0.0.0:5556'}, + oauth2: {skipApprovalScreen: true}, + staticClients: [ + { + id: 'gotify', + name: 'Gotify', + secret: 'secret', + redirectURIs: [redirectURL], + }, + ], + enablePasswordDB: true, + staticPasswords: users.map((u) => ({ + email: u.email, + hash: DEX_PASSWORD_HASH, + username: u.username, + preferredUsername: u.username, + userID: u.userID, + })), + }); + +const waitForDex = (port: number): Promise => + new Promise((resolve, reject) => { + wait( + { + resources: [`http-get://127.0.0.1:${port}/dex/.well-known/openid-configuration`], + timeout: 60000, + }, + (error: string) => (error ? reject(error) : resolve()) + ); + }); + +export const startDex = async (redirectURL: string, users: DexUser[]): Promise => { + const port = await getPort(); + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gotify-dex-')); + fs.writeFileSync(path.join(configDir, 'dex.conf'), dexConfig(port, redirectURL, users)); + + const userArgs = + process.getuid && process.getgid + ? ['--user', `${process.getuid()}:${process.getgid()}`] + : []; + + const containerName = `gotify-dex-test-${port}`; + process.stdout.write(`### Starting dex ${containerName}\n`); + const dex = spawn('docker', [ + 'run', + '--rm', + ...userArgs, + '--name', + containerName, + '-p', + `${port}:5556`, + '-v', + `${configDir}:/config`, + DEX_IMAGE, + 'dex', + 'serve', + '/config/dex.conf', + ]); + dex.stdout.pipe(process.stdout); + dex.stderr.pipe(process.stderr); + + const crashed = new Promise((_, reject) => { + const abort = (reason: string) => reject(new Error(`dex ${containerName} ${reason}`)); + dex.on('exit', (code, signal) => + abort(`exited unexpectedly (code=${code}, signal=${signal})`) + ); + dex.on('error', (err) => abort(`failed to start: ${err.message}`)); + }); + + const cleanup = () => { + try { + execFileSync('docker', ['rm', '-f', containerName], {stdio: 'ignore'}); + } catch { + // container may already be gone (e.g. it crashed with --rm) + } + rimrafSync(configDir); + }; + + try { + await Promise.race([waitForDex(port), crashed]); + } catch (err) { + cleanup(); + throw err; + } + + return { + issuer: `http://127.0.0.1:${port}/dex`, + close: cleanup, + }; +}; diff --git a/ui/src/tests/oidc.test.ts b/ui/src/tests/oidc.test.ts new file mode 100644 index 00000000..527b25c4 --- /dev/null +++ b/ui/src/tests/oidc.test.ts @@ -0,0 +1,114 @@ +import axios from 'axios'; +import {Browser, Page} from 'puppeteer'; +import {afterAll, beforeAll, describe, expect, it} from 'vitest'; +import {newTest, GotifyTest} from './setup'; +import {DEX_PASSWORD, DexUser} from './dex'; +import {waitForExists} from './utils'; +import * as selector from './selector'; +import * as auth from './authentication'; + +const linkUser: DexUser = {email: 'link@gotify.net', username: 'linkuser', userID: 'id-link'}; +const dupUser1: DexUser = {email: 'dup1@gotify.net', username: 'dupuser', userID: 'id-dup-1'}; +const dupUser2: DexUser = {email: 'dup2@gotify.net', username: 'dupuser', userID: 'id-dup-2'}; + +const createLocalUser = async (url: string, name: string, pass: string): Promise => + axios.post( + `${url}/user`, + {name, pass, admin: false}, + {auth: {username: 'admin', password: 'admin'}} + ); + +const loginWithOIDC = async (page: Page, user: DexUser): Promise => { + await waitForExists(page, selector.heading(), 'Login'); + const href = await page.$eval('#oidc-login', (a) => (a as HTMLAnchorElement).href); + await page.goto(href); + + await page.waitForSelector('#login'); + await page.type('#login', user.email); + await page.type('#password', DEX_PASSWORD); + await page.click('#submit-login'); +}; + +const expectLoggedIn = async (page: Page): Promise => { + await waitForExists(page, selector.heading(), 'All Messages'); + await page.waitForSelector('#logout'); +}; + +const oidcError = async (page: Page): Promise => { + await page.waitForFunction(() => document.location.pathname.includes('/auth/oidc/callback')); + return page.evaluate(() => document.body.innerText); +}; + +const clearSession = async (browser: Browser): Promise => { + await browser.deleteCookie(...(await browser.cookies())); +}; + +describe('OIDC login of an existing local user without link-by-username', () => { + let gotify: GotifyTest; + let page: Page; + beforeAll(async () => { + gotify = await newTest('', { + oidc: {autoRegister: true, linkByUsername: false, users: [linkUser]}, + }); + page = gotify.page; + await createLocalUser(gotify.url, linkUser.username, 'localpass'); + }); + afterAll(async () => await gotify.close()); + + it('rejects the oidc login because linking is disabled', async () => { + await loginWithOIDC(page, linkUser); + expect(await oidcError(page)).toContain( + `a local user with the username ${linkUser.username} already exists and linking by username is disabled` + ); + }); + + it('still allows the local user to log in with a password', async () => { + await page.goto(gotify.url); + await auth.login(page, 'linkuser', 'localpass'); + await auth.logout(page); + }); +}); + +describe('OIDC login of an existing local user with link-by-username', () => { + let gotify: GotifyTest; + let page: Page; + beforeAll(async () => { + gotify = await newTest('', { + oidc: {autoRegister: true, linkByUsername: true, users: [linkUser]}, + }); + page = gotify.page; + await createLocalUser(gotify.url, linkUser.username, 'localpass'); + }); + afterAll(async () => await gotify.close()); + + it('links the existing local user and logs in', async () => { + await loginWithOIDC(page, linkUser); + await expectLoggedIn(page); + }); +}); + +describe('OIDC login with two identities sharing the same username', () => { + let gotify: GotifyTest; + let page: Page; + beforeAll(async () => { + gotify = await newTest('', { + oidc: {autoRegister: true, linkByUsername: true, users: [dupUser1, dupUser2]}, + }); + page = gotify.page; + }); + afterAll(async () => await gotify.close()); + + it('auto-registers the first identity', async () => { + await loginWithOIDC(page, dupUser1); + await expectLoggedIn(page); + }); + + it('clears session', () => clearSession(gotify.browser)); + it('rejects the second identity with same username', async () => { + await page.goto(gotify.url); + await loginWithOIDC(page, dupUser2); + expect(await oidcError(page)).toContain( + `the user ${dupUser2.username} is already bound to a different OIDC identity` + ); + }); +}); diff --git a/ui/src/tests/setup.ts b/ui/src/tests/setup.ts index 171ee03e..005d0e99 100644 --- a/ui/src/tests/setup.ts +++ b/ui/src/tests/setup.ts @@ -7,6 +7,7 @@ import fs from 'fs'; // @ts-expect-error no types import wait from 'wait-on'; import kill from 'tree-kill'; +import {startDex, DexUser, DexInstance} from './dex'; export interface GotifyTest { url: string; @@ -15,6 +16,12 @@ export interface GotifyTest { page: Page; } +export interface OIDCOptions { + autoRegister?: boolean; + linkByUsername?: boolean; + users: DexUser[]; +} + const windowsPrefix = process.platform === 'win32' ? '.exe' : ''; const appDotGo = path.join(__dirname, '..', '..', '..', 'app.go'); const testBuildPath = path.join(__dirname, 'build'); @@ -27,14 +34,39 @@ export const newPluginDir = async (plugins: string[]): Promise => { return dir; }; -export const newTest = async (pluginsDir = ''): Promise => { +export interface NewTestOptions { + env?: Record; + oidc?: OIDCOptions; +} + +export const newTest = async ( + pluginsDir = '', + options: NewTestOptions = {} +): Promise => { const port = await getPort(); + let dex: DexInstance | undefined; + let env = options.env ?? {}; + if (options.oidc) { + const redirectURL = `http://localhost:${port}/auth/oidc/callback`; + dex = await startDex(redirectURL, options.oidc.users); + env = { + ...env, + GOTIFY_OIDC_ENABLED: 'true', + GOTIFY_OIDC_ISSUER: dex.issuer, + GOTIFY_OIDC_CLIENTID: 'gotify', + GOTIFY_OIDC_CLIENTSECRET: 'secret', + GOTIFY_OIDC_REDIRECTURL: redirectURL, + GOTIFY_OIDC_AUTOREGISTER: String(options.oidc.autoRegister ?? false), + GOTIFY_OIDC_LINK_BY_USERNAME: String(options.oidc.linkByUsername ?? false), + }; + } + const gotifyFile = testFilePath(); await buildGoExecutable(gotifyFile); - const gotifyInstance = startGotify(gotifyFile, port, pluginsDir); + const gotifyInstance = startGotify(gotifyFile, port, pluginsDir, env); const gotifyURL = 'http://localhost:' + port; await waitForGotify('http-get://localhost:' + port); @@ -55,6 +87,7 @@ export const newTest = async (pluginsDir = ''): Promise => { ), ]); rimrafSync(gotifyFile, {maxRetries: 8}); + dex?.close(); }, url: gotifyURL, browser, @@ -122,7 +155,12 @@ const buildGoExecutable = (filename: string): Promise => { } }; -const startGotify = (filename: string, port: number, pluginDir: string): ChildProcess => { +const startGotify = ( + filename: string, + port: number, + pluginDir: string, + extraEnv: Record = {} +): ChildProcess => { const gotify = spawn(filename, ['serve'], { env: { GOTIFY_SERVER_PORT: '' + port, @@ -130,6 +168,7 @@ const startGotify = (filename: string, port: number, pluginDir: string): ChildPr GOTIFY_PLUGINSDIR: pluginDir, NODE_ENV: process.env.NODE_ENV, PUBLIC_URL: process.env.PUBLIC_URL, + ...extraEnv, }, }); gotify.stdout.pipe(process.stdout); diff --git a/ui/src/user/Login.tsx b/ui/src/user/Login.tsx index e6e572a3..77f93750 100644 --- a/ui/src/user/Login.tsx +++ b/ui/src/user/Login.tsx @@ -85,6 +85,7 @@ const Login = observer(() => { <> or