Skip to content

Commit aceb5ff

Browse files
authored
Merge pull request #1020 from DerDummePunkt/github_1007_option_to_disable_local_login_when_oidc_enabled
feat: #1007 - Option to disable local login when OIDC is enabled
2 parents 491a17a + 795d9d1 commit aceb5ff

17 files changed

Lines changed: 248 additions & 122 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: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func (s *SessionSuite) BeforeTest(suiteName, testName string) {
3838
s.ctx, _ = gin.CreateTestContext(s.recorder)
3939
withURL(s.ctx, "http", "example.com")
4040
s.notified = false
41-
s.a = &SessionAPI{DB: s.db, NotifyDeleted: s.notify}
41+
s.a = &SessionAPI{DB: s.db, NotifyDeleted: s.notify, LocalAuthEnabled: true}
4242

4343
pw, err := password.CreatePassword("testpass", 5)
4444
require.NoError(s.T(), err)
@@ -93,6 +93,21 @@ func (s *SessionSuite) Test_Login_Success() {
9393
assert.Equal(s.T(), uint(auth.CookieMaxAge), clients[0].ExpiresAfterInactivitySeconds)
9494
}
9595

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+
96111
func (s *SessionSuite) Test_Login_WrongPassword() {
97112
s.ctx.Request = httptest.NewRequest("POST", "/auth/local/login", strings.NewReader("name=test-browser"))
98113
s.ctx.Request.Header.Set("Content-Type", "application/x-www-form-urlencoded")

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: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ type AuthenticationSuite struct {
3030
func (s *AuthenticationSuite) SetupSuite() {
3131
mode.Set(mode.TestDev)
3232
s.DB = testdb.NewDB(s.T())
33-
s.auth = &Auth{DB: s.DB, CrossOrigin: http.NewCrossOriginProtection()}
33+
s.auth = &Auth{DB: s.DB, LocalAuthEnabled: true, CrossOrigin: http.NewCrossOriginProtection()}
3434

3535
now := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC)
3636
timeNow = func() time.Time { return now }
@@ -274,6 +274,16 @@ func (s *AuthenticationSuite) TestBasicAuth() {
274274
s.assertHeaderRequest("Authorization", "Basic bm90ZXhpc3Rpbmc6cHc=", s.auth.RequireElevatedClient, 401)
275275
}
276276

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+
277287
func (s *AuthenticationSuite) TestOptionalAuth() {
278288
// various invalid users
279289
ctx := s.assertQueryRequest("token", "ergerogerg", s.auth.Optional, 200)

config/config.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ type Configuration struct {
7979
UploadedImagesDir string
8080
PluginsDir string
8181
Registration bool
82+
LocalAuthEnabled bool
8283
OIDC OIDC
8384
NoColor string
8485
}
@@ -111,6 +112,7 @@ func Get() (*Configuration, []FutureLog) {
111112
PassStrength: 10,
112113
UploadedImagesDir: "data/images",
113114
PluginsDir: "data/plugins",
115+
LocalAuthEnabled: true,
114116
OIDC: OIDC{
115117
UsernameClaim: "preferred_username",
116118
AutoRegister: true,
@@ -167,6 +169,7 @@ func Get() (*Configuration, []FutureLog) {
167169
add(parseString(&c.UploadedImagesDir, EnvUploadedImagesDir))
168170
add(parseString(&c.PluginsDir, EnvPluginsDir))
169171
add(parseBool(&c.Registration, EnvRegistration))
172+
add(parseBool(&c.LocalAuthEnabled, EnvLocalAuthEnabled))
170173

171174
add(parseBool(&c.OIDC.Enabled, EnvOIDCEnabled))
172175
add(parseString(&c.OIDC.Issuer, EnvOIDCIssuer))
@@ -182,6 +185,12 @@ func Get() (*Configuration, []FutureLog) {
182185

183186
addTrailingSlashToPaths(c)
184187

188+
if !c.LocalAuthEnabled && !c.OIDC.Enabled {
189+
logs = append(logs, futureFatal("either local authentication or OIDC must be enabled"))
190+
}
191+
if c.Registration && !c.LocalAuthEnabled {
192+
logs = append(logs, futureFatal("registration requires local authentication to be enabled"))
193+
}
185194
return c, logs
186195
}
187196

config/config_test.go

Lines changed: 61 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -6,31 +6,22 @@ import (
66
"testing"
77

88
"github.com/gotify/server/v2/mode"
9+
"github.com/rs/zerolog"
910
"github.com/stretchr/testify/assert"
1011
)
1112

1213
func TestConfigEnv(t *testing.T) {
1314
mode.Set(mode.TestDev)
14-
os.Setenv("GOTIFY_DEFAULTUSER_NAME", "jmattheis")
15-
os.Setenv("GOTIFY_SERVER_SSL_LETSENCRYPT_HOSTS", "push.example.tld,push.other.tld")
16-
os.Setenv(
15+
t.Setenv("GOTIFY_DEFAULTUSER_NAME", "jmattheis")
16+
t.Setenv("GOTIFY_SERVER_SSL_LETSENCRYPT_HOSTS", "push.example.tld,push.other.tld")
17+
t.Setenv(
1718
"GOTIFY_SERVER_RESPONSEHEADERS",
1819
`{"Access-Control-Allow-Origin":"*","Access-Control-Allow-Methods":"GET,POST"}`,
1920
)
20-
os.Setenv("GOTIFY_SERVER_CORS_ALLOWORIGINS", ".+.example.com,otherdomain.com")
21-
os.Setenv("GOTIFY_SERVER_CORS_ALLOWMETHODS", "GET,POST")
22-
os.Setenv("GOTIFY_SERVER_CORS_ALLOWHEADERS", "Authorization,content-type")
23-
os.Setenv("GOTIFY_SERVER_STREAM_ALLOWEDORIGINS", ".+.example.com,otherdomain.com")
24-
25-
defer func() {
26-
os.Unsetenv("GOTIFY_DEFAULTUSER_NAME")
27-
os.Unsetenv("GOTIFY_SERVER_SSL_LETSENCRYPT_HOSTS")
28-
os.Unsetenv("GOTIFY_SERVER_RESPONSEHEADERS")
29-
os.Unsetenv("GOTIFY_SERVER_CORS_ALLOWORIGINS")
30-
os.Unsetenv("GOTIFY_SERVER_CORS_ALLOWMETHODS")
31-
os.Unsetenv("GOTIFY_SERVER_CORS_ALLOWHEADERS")
32-
os.Unsetenv("GOTIFY_SERVER_STREAM_ALLOWEDORIGINS")
33-
}()
21+
t.Setenv("GOTIFY_SERVER_CORS_ALLOWORIGINS", ".+.example.com,otherdomain.com")
22+
t.Setenv("GOTIFY_SERVER_CORS_ALLOWMETHODS", "GET,POST")
23+
t.Setenv("GOTIFY_SERVER_CORS_ALLOWHEADERS", "Authorization,content-type")
24+
t.Setenv("GOTIFY_SERVER_STREAM_ALLOWEDORIGINS", ".+.example.com,otherdomain.com")
3425

3526
conf, _ := Get()
3627
assert.Equal(t, 80, conf.Server.Port, "should use defaults")
@@ -44,6 +35,53 @@ func TestConfigEnv(t *testing.T) {
4435
assert.Equal(t, []string{".+.example.com", "otherdomain.com"}, conf.Server.Stream.AllowedOrigins)
4536
}
4637

38+
func TestLocalAuthDisabled(t *testing.T) {
39+
tests := []struct {
40+
name string
41+
env map[string]string
42+
fatals []FutureLog
43+
}{
44+
{
45+
name: "with oidc",
46+
env: map[string]string{EnvLocalAuthEnabled: "false", EnvOIDCEnabled: "true"},
47+
},
48+
{
49+
name: "without oidc",
50+
env: map[string]string{EnvLocalAuthEnabled: "false"},
51+
fatals: []FutureLog{futureFatal("either local authentication or OIDC must be enabled")},
52+
},
53+
{
54+
name: "with registration",
55+
env: map[string]string{
56+
EnvLocalAuthEnabled: "false",
57+
EnvOIDCEnabled: "true",
58+
EnvRegistration: "true",
59+
},
60+
fatals: []FutureLog{futureFatal("registration requires local authentication to be enabled")},
61+
},
62+
}
63+
64+
for _, tc := range tests {
65+
t.Run(tc.name, func(t *testing.T) {
66+
mode.Set(mode.TestDev)
67+
for key, value := range tc.env {
68+
t.Setenv(key, value)
69+
}
70+
71+
conf, logs := Get()
72+
assert.False(t, conf.LocalAuthEnabled)
73+
74+
var fatals []FutureLog
75+
for _, entry := range logs {
76+
if entry.Level == zerolog.FatalLevel {
77+
fatals = append(fatals, entry)
78+
}
79+
}
80+
assert.Equal(t, tc.fatals, fatals)
81+
})
82+
}
83+
}
84+
4785
func TestFile(t *testing.T) {
4886
mode.Set(mode.TestDev)
4987
dir := t.TempDir()
@@ -52,10 +90,8 @@ func TestFile(t *testing.T) {
5290
assert.Nil(t, os.WriteFile(passPath, []byte("filesecret\n"), 0o600))
5391
assert.Nil(t, os.WriteFile(hostsPath, []byte("a.example.com,b.example.com"), 0o600))
5492

55-
os.Setenv("GOTIFY_DEFAULTUSER_PASS_FILE", passPath)
56-
os.Setenv("GOTIFY_SERVER_SSL_LETSENCRYPT_HOSTS_FILE", hostsPath)
57-
defer os.Unsetenv("GOTIFY_DEFAULTUSER_PASS_FILE")
58-
defer os.Unsetenv("GOTIFY_SERVER_SSL_LETSENCRYPT_HOSTS_FILE")
93+
t.Setenv("GOTIFY_DEFAULTUSER_PASS_FILE", passPath)
94+
t.Setenv("GOTIFY_SERVER_SSL_LETSENCRYPT_HOSTS_FILE", hostsPath)
5995

6096
conf, _ := Get()
6197
assert.Equal(t, "filesecret", conf.DefaultUser.Pass)
@@ -68,27 +104,24 @@ func TestGotifyConfigFile(t *testing.T) {
68104
configPath := filepath.Join(dir, "custom.env")
69105
assert.Nil(t, os.WriteFile(configPath, []byte("GOTIFY_DEFAULTUSER_NAME=fromfile\n"), 0o600))
70106

71-
os.Setenv("GOTIFY_CONFIG_FILE", configPath)
72-
defer os.Unsetenv("GOTIFY_CONFIG_FILE")
107+
t.Setenv("GOTIFY_CONFIG_FILE", configPath)
73108

74109
conf, _ := Get()
75110
assert.Equal(t, "fromfile", conf.DefaultUser.Name)
76111
}
77112

78113
func TestAddSlash(t *testing.T) {
79114
mode.Set(mode.TestDev)
80-
os.Setenv("GOTIFY_UPLOADEDIMAGESDIR", "../data/images")
115+
t.Setenv("GOTIFY_UPLOADEDIMAGESDIR", "../data/images")
81116
conf, _ := Get()
82117
assert.Equal(t, "../data/images"+string(filepath.Separator), conf.UploadedImagesDir)
83-
os.Unsetenv("GOTIFY_UPLOADEDIMAGESDIR")
84118
}
85119

86120
func TestNotAddSlash(t *testing.T) {
87121
mode.Set(mode.TestDev)
88-
os.Setenv("GOTIFY_UPLOADEDIMAGESDIR", "../data/")
122+
t.Setenv("GOTIFY_UPLOADEDIMAGESDIR", "../data/")
89123
conf, _ := Get()
90124
assert.Equal(t, "../data/", conf.UploadedImagesDir)
91-
os.Unsetenv("GOTIFY_UPLOADEDIMAGESDIR")
92125
}
93126

94127
func TestParseList(t *testing.T) {
@@ -106,8 +139,7 @@ func TestParseList(t *testing.T) {
106139

107140
for _, tc := range tests {
108141
t.Run(tc.name, func(t *testing.T) {
109-
os.Setenv(env, tc.raw)
110-
defer os.Unsetenv(env)
142+
t.Setenv(env, tc.raw)
111143

112144
var got []string
113145
assert.Nil(t, parseList(&got, env))

config/keys.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ const (
4040
EnvOIDCRedirectURL = "GOTIFY_OIDC_REDIRECTURL"
4141
EnvOIDCAutoRegister = "GOTIFY_OIDC_AUTOREGISTER"
4242
EnvOIDCLinkByUsername = "GOTIFY_OIDC_LINK_BY_USERNAME"
43+
EnvLocalAuthEnabled = "GOTIFY_LOCALAUTH_ENABLED"
4344
EnvOIDCScopes = "GOTIFY_OIDC_SCOPES"
4445
EnvNoColor = "NOCOLOR"
4546
)

docs/spec.json

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -720,6 +720,12 @@
720720
"schema": {
721721
"$ref": "#/definitions/Error"
722722
}
723+
},
724+
"403": {
725+
"description": "Forbidden",
726+
"schema": {
727+
"$ref": "#/definitions/Error"
728+
}
723729
}
724730
}
725731
}
@@ -2940,9 +2946,16 @@
29402946
"required": [
29412947
"version",
29422948
"register",
2949+
"localAuth",
29432950
"oidc"
29442951
],
29452952
"properties": {
2953+
"localAuth": {
2954+
"description": "If local authentication is enabled.",
2955+
"type": "boolean",
2956+
"x-go-name": "LocalAuth",
2957+
"example": true
2958+
},
29462959
"oidc": {
29472960
"description": "If oidc is enabled.",
29482961
"type": "boolean",

gotify-server.env.example

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,10 @@
224224
# Type: text-list
225225
# GOTIFY_OIDC_SCOPES=openid,profile,email
226226

227+
# Enable authentication via username and password.
228+
# Type: boolean
229+
# GOTIFY_LOCALAUTH_ENABLED=true
230+
227231
# Database driver to use. For mysql and postgres the target database must
228232
# already exist and the configured user must have sufficient permissions.
229233
#

0 commit comments

Comments
 (0)