Skip to content

Commit 965a5db

Browse files
authored
Merge branch 'master' into feature/improve-responsive-ui-shell
2 parents f0c163b + aceb5ff commit 965a5db

25 files changed

Lines changed: 421 additions & 157 deletions

api/session.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,10 @@ type SessionDatabase interface {
2020

2121
// SessionAPI provides handlers for cookie-based session authentication.
2222
type SessionAPI struct {
23-
DB SessionDatabase
24-
NotifyDeleted func(uint, string)
25-
SecureCookie bool
23+
DB SessionDatabase
24+
NotifyDeleted func(uint, string)
25+
SecureCookie bool
26+
LocalAuthEnabled bool
2627
}
2728

2829
// swagger:operation POST /auth/local/login auth localLogin
@@ -53,7 +54,16 @@ type SessionAPI struct {
5354
// description: Unauthorized
5455
// schema:
5556
// $ref: "#/definitions/Error"
57+
// 403:
58+
// description: Forbidden
59+
// schema:
60+
// $ref: "#/definitions/Error"
5661
func (a *SessionAPI) Login(ctx *gin.Context) {
62+
if !a.LocalAuthEnabled {
63+
ctx.AbortWithError(403, errors.New("local authentication is disabled"))
64+
return
65+
}
66+
5767
name, pass, ok := ctx.Request.BasicAuth()
5868
if !ok {
5969
ctx.AbortWithError(401, errors.New("basic auth required"))

api/session_test.go

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/gotify/server/v2/model"
1515
"github.com/gotify/server/v2/test/testdb"
1616
"github.com/stretchr/testify/assert"
17+
"github.com/stretchr/testify/require"
1718
"github.com/stretchr/testify/suite"
1819
)
1920

@@ -37,11 +38,14 @@ func (s *SessionSuite) BeforeTest(suiteName, testName string) {
3738
s.ctx, _ = gin.CreateTestContext(s.recorder)
3839
withURL(s.ctx, "http", "example.com")
3940
s.notified = false
40-
s.a = &SessionAPI{DB: s.db, NotifyDeleted: s.notify}
41+
s.a = &SessionAPI{DB: s.db, NotifyDeleted: s.notify, LocalAuthEnabled: true}
42+
43+
pw, err := password.CreatePassword("testpass", 5)
44+
require.NoError(s.T(), err)
4145

4246
s.db.CreateUser(&model.User{
4347
Name: "testuser",
44-
Pass: password.CreatePassword("testpass", 5),
48+
Pass: pw,
4549
})
4650
}
4751

@@ -89,6 +93,21 @@ func (s *SessionSuite) Test_Login_Success() {
8993
assert.Equal(s.T(), uint(auth.CookieMaxAge), clients[0].ExpiresAfterInactivitySeconds)
9094
}
9195

96+
func (s *SessionSuite) Test_Login_LocalAuthDisabled() {
97+
s.a.LocalAuthEnabled = false
98+
s.ctx.Request = httptest.NewRequest("POST", "/auth/local/login", strings.NewReader("name=test-browser"))
99+
s.ctx.Request.Header.Set("Content-Type", "application/x-www-form-urlencoded")
100+
s.ctx.Request.Header.Set("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte("testuser:testpass")))
101+
102+
s.a.Login(s.ctx)
103+
104+
assert.Equal(s.T(), 403, s.recorder.Code)
105+
106+
for _, c := range s.recorder.Result().Cookies() {
107+
assert.NotEqual(s.T(), auth.CookieName, c.Name)
108+
}
109+
}
110+
92111
func (s *SessionSuite) Test_Login_WrongPassword() {
93112
s.ctx.Request = httptest.NewRequest("POST", "/auth/local/login", strings.NewReader("name=test-browser"))
94113
s.ctx.Request.Header.Set("Content-Type", "application/x-www-form-urlencoded")

api/user.go

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,10 +188,19 @@ func (a *UserAPI) GetCurrentUser(ctx *gin.Context) {
188188
func (a *UserAPI) CreateUser(ctx *gin.Context) {
189189
user := model.CreateUserExternal{}
190190
if err := ctx.Bind(&user); err == nil {
191+
if err := password.ValidateNewPassword(user.Pass); err != nil {
192+
ctx.AbortWithError(http.StatusBadRequest, err)
193+
return
194+
}
195+
pw, err := password.CreatePassword(user.Pass, a.PasswordStrength)
196+
if err != nil {
197+
ctx.AbortWithError(http.StatusInternalServerError, fmt.Errorf("failed to prepare password: %s", err))
198+
return
199+
}
191200
internal := &model.User{
192201
Name: user.Name,
193202
Admin: user.Admin,
194-
Pass: password.CreatePassword(user.Pass, a.PasswordStrength),
203+
Pass: pw,
195204
}
196205
existingUser, err := a.DB.GetUserByName(internal.Name)
197206
if success := successOrAbort(ctx, 500, err); !success {
@@ -389,11 +398,20 @@ func (a *UserAPI) DeleteUserByID(ctx *gin.Context) {
389398
func (a *UserAPI) ChangePassword(ctx *gin.Context) {
390399
pw := model.UserExternalPass{}
391400
if err := ctx.Bind(&pw); err == nil {
401+
if err := password.ValidateNewPassword(pw.Pass); err != nil {
402+
ctx.AbortWithError(http.StatusBadRequest, err)
403+
return
404+
}
392405
user, err := a.DB.GetUserByID(auth.GetUserID(ctx))
393406
if success := successOrAbort(ctx, 500, err); !success {
394407
return
395408
}
396-
user.Pass = password.CreatePassword(pw.Pass, a.PasswordStrength)
409+
pw, err := password.CreatePassword(pw.Pass, a.PasswordStrength)
410+
if err != nil {
411+
ctx.AbortWithError(http.StatusInternalServerError, fmt.Errorf("failed to prepare password: %s", err))
412+
return
413+
}
414+
user.Pass = pw
397415
successOrAbort(ctx, 500, a.DB.UpdateUser(user))
398416
}
399417
}
@@ -465,7 +483,16 @@ func (a *UserAPI) UpdateUserByID(ctx *gin.Context) {
465483
dbUser.Admin = updatedUser.Admin
466484

467485
if updatedUser.Pass != "" {
468-
dbUser.Pass = password.CreatePassword(updatedUser.Pass, a.PasswordStrength)
486+
if err := password.ValidateNewPassword(updatedUser.Pass); err != nil {
487+
ctx.AbortWithError(http.StatusBadRequest, err)
488+
return
489+
}
490+
pw, err := password.CreatePassword(updatedUser.Pass, a.PasswordStrength)
491+
if err != nil {
492+
ctx.AbortWithError(http.StatusInternalServerError, fmt.Errorf("failed to prepare password: %s", err))
493+
return
494+
}
495+
dbUser.Pass = pw
469496
}
470497
if success := successOrAbort(ctx, 500, a.DB.UpdateUser(dbUser)); !success {
471498
return

api/user_test.go

Lines changed: 65 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/gotify/server/v2/test"
1515
"github.com/gotify/server/v2/test/testdb"
1616
"github.com/stretchr/testify/assert"
17+
"github.com/stretchr/testify/require"
1718
"github.com/stretchr/testify/suite"
1819
)
1920

@@ -320,6 +321,24 @@ func (s *UserSuite) Test_CreateUser_NameAlreadyExists() {
320321
assert.Equal(s.T(), 400, s.recorder.Code)
321322
}
322323

324+
func (s *UserSuite) Test_CreateUser_EmptyPassword_Expect400() {
325+
s.loginAdmin()
326+
327+
s.ctx.Request = httptest.NewRequest("POST", "/user", strings.NewReader(`{"name": "admin", "pass": "", "admin": false}`))
328+
s.ctx.Request.Header.Set("Content-Type", "application/json")
329+
s.a.CreateUser(s.ctx)
330+
assert.Equal(s.T(), 400, s.recorder.Code)
331+
}
332+
333+
func (s *UserSuite) Test_CreateUser_TooLongPassword_Expect400() {
334+
s.loginAdmin()
335+
336+
s.ctx.Request = httptest.NewRequest("POST", "/user", strings.NewReader(`{"name": "admin", "pass": "`+strings.Repeat("a", 100)+`", "admin": false}`))
337+
s.ctx.Request.Header.Set("Content-Type", "application/json")
338+
s.a.CreateUser(s.ctx)
339+
assert.Equal(s.T(), 400, s.recorder.Code)
340+
}
341+
323342
func (s *UserSuite) Test_UpdateUserByID_InvalidID() {
324343
s.ctx.Params = gin.Params{{Key: "id", Value: "abc"}}
325344

@@ -331,6 +350,28 @@ func (s *UserSuite) Test_UpdateUserByID_InvalidID() {
331350
assert.Equal(s.T(), 400, s.recorder.Code)
332351
}
333352

353+
func (s *UserSuite) Test_UpdateUserByID_EmptyPassword_Expect400() {
354+
s.loginAdmin()
355+
356+
s.ctx.Params = gin.Params{{Key: "id", Value: "1"}}
357+
358+
s.ctx.Request = httptest.NewRequest("POST", "/user/1", strings.NewReader(`{"name": "admin", "pass": "", "admin": false}`))
359+
s.ctx.Request.Header.Set("Content-Type", "application/json")
360+
s.a.UpdateUserByID(s.ctx)
361+
assert.Equal(s.T(), 400, s.recorder.Code)
362+
}
363+
364+
func (s *UserSuite) Test_UpdateUserByID_TooLongPassword_Expect400() {
365+
s.loginAdmin()
366+
367+
s.ctx.Params = gin.Params{{Key: "id", Value: "1"}}
368+
369+
s.ctx.Request = httptest.NewRequest("POST", "/user/1", strings.NewReader(`{"name": "admin", "pass": "`+strings.Repeat("a", 100)+`", "admin": false}`))
370+
s.ctx.Request.Header.Set("Content-Type", "application/json")
371+
s.a.UpdateUserByID(s.ctx)
372+
assert.Equal(s.T(), 400, s.recorder.Code)
373+
}
374+
334375
func (s *UserSuite) Test_UpdateUserByID_LastAdmin_Expect400() {
335376
s.db.CreateUser(&model.User{
336377
ID: 7,
@@ -359,7 +400,9 @@ func (s *UserSuite) Test_UpdateUserByID_UnknownUser() {
359400
}
360401

361402
func (s *UserSuite) Test_UpdateUserByID_UpdateNotPassword() {
362-
s.db.CreateUser(&model.User{ID: 2, Name: "nico", Pass: password.CreatePassword("old", 5)})
403+
pw, err := password.CreatePassword("old", 5)
404+
require.NoError(s.T(), err)
405+
s.db.CreateUser(&model.User{ID: 2, Name: "nico", Pass: pw})
363406

364407
s.ctx.Params = gin.Params{{Key: "id", Value: "2"}}
365408

@@ -376,7 +419,9 @@ func (s *UserSuite) Test_UpdateUserByID_UpdateNotPassword() {
376419
}
377420

378421
func (s *UserSuite) Test_UpdateUserByID_UpdatePassword() {
379-
s.db.CreateUser(&model.User{ID: 2, Name: "tom", Pass: password.CreatePassword("old", 5)})
422+
pw, err := password.CreatePassword("old", 5)
423+
require.NoError(s.T(), err)
424+
s.db.CreateUser(&model.User{ID: 2, Name: "tom", Pass: pw})
380425

381426
s.ctx.Params = gin.Params{{Key: "id", Value: "2"}}
382427

@@ -413,7 +458,9 @@ func (s *UserSuite) Test_UpdateUserByID_PreservesOIDCID() {
413458
}
414459

415460
func (s *UserSuite) Test_UpdatePassword() {
416-
s.db.CreateUser(&model.User{ID: 1, Name: "jmattheis", Pass: password.CreatePassword("old", 5)})
461+
pw, err := password.CreatePassword("old", 5)
462+
require.NoError(s.T(), err)
463+
s.db.CreateUser(&model.User{ID: 1, Name: "jmattheis", Pass: pw})
417464

418465
test.WithUser(s.ctx, 1)
419466
s.ctx.Request = httptest.NewRequest("POST", "/user/current/password", strings.NewReader(`{"pass": "new"}`))
@@ -429,7 +476,9 @@ func (s *UserSuite) Test_UpdatePassword() {
429476
}
430477

431478
func (s *UserSuite) Test_UpdatePassword_EmptyPassword() {
432-
s.db.CreateUser(&model.User{ID: 1, Name: "jmattheis", Pass: password.CreatePassword("old", 5)})
479+
pw, err := password.CreatePassword("old", 5)
480+
require.NoError(s.T(), err)
481+
s.db.CreateUser(&model.User{ID: 1, Name: "jmattheis", Pass: pw})
433482

434483
test.WithUser(s.ctx, 1)
435484
s.ctx.Request = httptest.NewRequest("POST", "/user/current/password", strings.NewReader(`{"pass":""}`))
@@ -444,6 +493,18 @@ func (s *UserSuite) Test_UpdatePassword_EmptyPassword() {
444493
assert.True(s.T(), password.ComparePassword(user.Pass, []byte("old")))
445494
}
446495

496+
func (s *UserSuite) Test_UpdatePassword_TooLongPassword_Expect400() {
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+
501+
test.WithUser(s.ctx, 1)
502+
s.ctx.Request = httptest.NewRequest("POST", "/user/current/password", strings.NewReader(`{"pass": "`+strings.Repeat("a", 100)+`"}`))
503+
s.ctx.Request.Header.Set("Content-Type", "application/json")
504+
s.a.ChangePassword(s.ctx)
505+
assert.Equal(s.T(), 400, s.recorder.Code)
506+
}
507+
447508
func (s *UserSuite) loginAdmin() {
448509
s.db.CreateUser(&model.User{ID: 1, Name: "admin", Admin: true})
449510
auth.RegisterUser(s.ctx, &model.User{ID: 1, Admin: true})

app.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ func serve(vInfo *model.VersionInfo) int {
106106
return 1
107107
}
108108

109-
db, err := database.New(conf.Database.Dialect, conf.Database.Connection, conf.DefaultUser.Name, conf.DefaultUser.Pass, conf.PassStrength, true, time.Now)
109+
db, err := database.New(conf.Database.Dialect, conf.Database.Connection, conf.DefaultUser.Name, conf.DefaultUser.Pass, conf.PassStrength, conf.LocalAuthEnabled, time.Now)
110110
if err != nil {
111111
log.Error().Err(err).Msg("Cannot initialize database")
112112
return 1

auth/authentication.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ const (
1818
authStateForbidden
1919
authStateNotElevated
2020
authStateOk
21+
authStateLocalAuthDisabled
2122
)
2223

2324
const (
@@ -39,9 +40,10 @@ type Database interface {
3940

4041
// Auth is the provider for authentication middleware.
4142
type Auth struct {
42-
DB Database
43-
SecureCookie bool
44-
CrossOrigin *http.CrossOriginProtection
43+
DB Database
44+
SecureCookie bool
45+
LocalAuthEnabled bool
46+
CrossOrigin *http.CrossOriginProtection
4547
}
4648

4749
// RequireAdmin requires an elevated client token or basic auth, the user must be an admin.
@@ -109,6 +111,9 @@ func (a *Auth) evaluate(ctx *gin.Context, funcs ...func(ctx *gin.Context) (authS
109111
case authStateNotElevated:
110112
ctx.AbortWithError(403, errors.New("session not elevated, use basic auth or call /client:elevate"))
111113
return true
114+
case authStateLocalAuthDisabled:
115+
ctx.AbortWithError(403, errors.New("local authentication is disabled"))
116+
return true
112117
case authStateOk:
113118
ctx.Next()
114119
return true
@@ -147,6 +152,9 @@ func (a *Auth) rejectForeignOrigin(ctx *gin.Context) bool {
147152
func (a *Auth) handleUser(checks ...func(*model.User) (authState, error)) func(ctx *gin.Context) (authState, error) {
148153
return func(ctx *gin.Context) (authState, error) {
149154
if name, pass, ok := ctx.Request.BasicAuth(); ok {
155+
if !a.LocalAuthEnabled {
156+
return authStateLocalAuthDisabled, nil
157+
}
150158
if user, err := a.DB.GetUserByName(name); err != nil {
151159
return authStateSkip, err
152160
} else if user != nil && password.ComparePassword(user.Pass, []byte(pass)) {

auth/authentication_test.go

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/gotify/server/v2/model"
1414
"github.com/gotify/server/v2/test/testdb"
1515
"github.com/stretchr/testify/assert"
16+
"github.com/stretchr/testify/require"
1617
"github.com/stretchr/testify/suite"
1718
)
1819

@@ -29,17 +30,20 @@ type AuthenticationSuite struct {
2930
func (s *AuthenticationSuite) SetupSuite() {
3031
mode.Set(mode.TestDev)
3132
s.DB = testdb.NewDB(s.T())
32-
s.auth = &Auth{DB: s.DB, CrossOrigin: http.NewCrossOriginProtection()}
33+
s.auth = &Auth{DB: s.DB, LocalAuthEnabled: true, CrossOrigin: http.NewCrossOriginProtection()}
3334

3435
now := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC)
3536
timeNow = func() time.Time { return now }
3637

3738
elevated := now.Add(time.Hour)
3839
expired := now.Add(-time.Hour)
3940

41+
pw, err := password.CreatePassword("pw", 5)
42+
require.NoError(s.T(), err)
43+
4044
s.DB.CreateUser(&model.User{
4145
Name: "existing",
42-
Pass: password.CreatePassword("pw", 5),
46+
Pass: pw,
4347
Admin: false,
4448
Applications: []model.Application{{Token: "apptoken", Name: "backup server1", Description: "irrelevant"}},
4549
Clients: []model.Client{
@@ -51,7 +55,7 @@ func (s *AuthenticationSuite) SetupSuite() {
5155

5256
s.DB.CreateUser(&model.User{
5357
Name: "admin",
54-
Pass: password.CreatePassword("pw", 5),
58+
Pass: pw,
5559
Admin: true,
5660
Applications: []model.Application{{Token: "apptoken_admin", Name: "backup server2", Description: "irrelevant"}},
5761
Clients: []model.Client{
@@ -270,6 +274,16 @@ func (s *AuthenticationSuite) TestBasicAuth() {
270274
s.assertHeaderRequest("Authorization", "Basic bm90ZXhpc3Rpbmc6cHc=", s.auth.RequireElevatedClient, 401)
271275
}
272276

277+
func (s *AuthenticationSuite) TestBasicAuthDisabled() {
278+
s.auth.LocalAuthEnabled = false
279+
defer func() { s.auth.LocalAuthEnabled = true }()
280+
281+
s.assertHeaderRequest("Authorization", "Basic YWRtaW46cHc=", s.auth.RequireApplicationToken, 403)
282+
s.assertHeaderRequest("Authorization", "Basic YWRtaW46cHc=", s.auth.RequireClient, 403)
283+
s.assertHeaderRequest("Authorization", "Basic YWRtaW46cHc=", s.auth.RequireAdmin, 403)
284+
s.assertHeaderRequest("Authorization", "Basic YWRtaW46cHc=", s.auth.RequireElevatedClient, 403)
285+
}
286+
273287
func (s *AuthenticationSuite) TestOptionalAuth() {
274288
// various invalid users
275289
ctx := s.assertQueryRequest("token", "ergerogerg", s.auth.Optional, 200)

0 commit comments

Comments
 (0)