Skip to content

Commit d08317a

Browse files
committed
fix: reject password changes when local auth is disabled
The WebUI offered a "Change Password" prompt even with GOTIFY_LOCALAUTH_ENABLED=false, and the endpoint behind it accepted the change: ChangePassword never consulted the setting, so a user on an OIDC-only server could still set a local password that the login form no longer accepts. Guard the handler the same way SessionAPI.Login already does, and hide the header entry that opens the dialog when local auth is off. Closes #1040
1 parent 14bfc25 commit d08317a

4 files changed

Lines changed: 37 additions & 9 deletions

File tree

api/user.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ type UserAPI struct {
6363
PasswordStrength int
6464
UserChangeNotifier *UserChangeNotifier
6565
Registration bool
66+
LocalAuthEnabled bool
6667
}
6768

6869
// GetUsers returns all the users
@@ -396,6 +397,11 @@ func (a *UserAPI) DeleteUserByID(ctx *gin.Context) {
396397
// schema:
397398
// $ref: "#/definitions/Error"
398399
func (a *UserAPI) ChangePassword(ctx *gin.Context) {
400+
if !a.LocalAuthEnabled {
401+
ctx.AbortWithError(403, errors.New("local authentication is disabled"))
402+
return
403+
}
404+
399405
pw := model.UserExternalPass{}
400406
if err := ctx.Bind(&pw); err == nil {
401407
if err := password.ValidateNewPassword(pw.Pass); err != nil {

api/user_test.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func (s *UserSuite) BeforeTest(suiteName, testName string) {
4949
s.notifiedAdd = true
5050
return nil
5151
})
52-
s.a = &UserAPI{DB: s.db, UserChangeNotifier: s.notifier}
52+
s.a = &UserAPI{DB: s.db, UserChangeNotifier: s.notifier, LocalAuthEnabled: true}
5353
}
5454

5555
func (s *UserSuite) AfterTest(suiteName, testName string) {
@@ -493,6 +493,25 @@ func (s *UserSuite) Test_UpdatePassword_EmptyPassword() {
493493
assert.True(s.T(), password.ComparePassword(user.Pass, []byte("old")))
494494
}
495495

496+
func (s *UserSuite) Test_UpdatePassword_LocalAuthDisabled_Expect403() {
497+
pw, err := password.CreatePassword("old", 5)
498+
require.NoError(s.T(), err)
499+
s.db.CreateUser(&model.User{ID: 1, Name: "jmattheis", Pass: pw})
500+
s.a.LocalAuthEnabled = false
501+
502+
test.WithUser(s.ctx, 1)
503+
s.ctx.Request = httptest.NewRequest("POST", "/user/current/password", strings.NewReader(`{"pass": "new"}`))
504+
s.ctx.Request.Header.Set("Content-Type", "application/json")
505+
506+
s.a.ChangePassword(s.ctx)
507+
508+
assert.Equal(s.T(), 403, s.recorder.Code)
509+
user, err := s.db.GetUserByID(1)
510+
assert.NoError(s.T(), err)
511+
assert.NotNil(s.T(), user)
512+
assert.True(s.T(), password.ComparePassword(user.Pass, []byte("old")))
513+
}
514+
496515
func (s *UserSuite) Test_UpdatePassword_TooLongPassword_Expect400() {
497516
pw, err := password.CreatePassword("old", 5)
498517
require.NoError(s.T(), err)

router/router.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ func Create(db *database.GormDatabase, vInfo *model.VersionInfo, conf *config.Co
104104
}
105105
sessionHandler := api.SessionAPI{DB: db, NotifyDeleted: streamHandler.NotifyDeletedClient, SecureCookie: conf.Server.SecureCookie, LocalAuthEnabled: conf.LocalAuthEnabled}
106106
userChangeNotifier := new(api.UserChangeNotifier)
107-
userHandler := api.UserAPI{DB: db, PasswordStrength: conf.PassStrength, UserChangeNotifier: userChangeNotifier, Registration: conf.Registration}
107+
userHandler := api.UserAPI{DB: db, PasswordStrength: conf.PassStrength, UserChangeNotifier: userChangeNotifier, Registration: conf.Registration, LocalAuthEnabled: conf.LocalAuthEnabled}
108108

109109
pluginManager, err := plugin.NewManager(db, conf.PluginsDir, g.Group("/plugin/:id/custom/"), streamHandler)
110110
if err != nil {

ui/src/layout/Header.tsx

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import React, {CSSProperties} from 'react';
2020
import {Link} from 'react-router';
2121
import {useMediaQuery} from '@mui/material';
2222
import {ThemeKey} from './theme';
23+
import * as config from '../config';
2324

2425
const themeIcons: Record<ThemeKey, React.ReactElement> = {
2526
dark: <Brightness4 />,
@@ -195,13 +196,15 @@ const Buttons = ({
195196
<Link className={classes.link} to="/plugins" id="navigate-plugins">
196197
<ResponsiveButton icon={<Apps />} label="plugins" color="inherit" />
197198
</Link>
198-
<ResponsiveButton
199-
icon={<AccountCircle />}
200-
label={name}
201-
onClick={showSettings}
202-
id="changepw"
203-
color="inherit"
204-
/>
199+
{config.get('localAuth') && (
200+
<ResponsiveButton
201+
icon={<AccountCircle />}
202+
label={name}
203+
onClick={showSettings}
204+
id="changepw"
205+
color="inherit"
206+
/>
207+
)}
205208
<ResponsiveButton
206209
icon={<ExitToApp />}
207210
label="Logout"

0 commit comments

Comments
 (0)