Skip to content

Commit 27b351a

Browse files
zeripathzeripath
authored
Make LDAP be able to skip local 2FA (#16954)
This PR extends #16594 to allow LDAP to be able to be set to skip local 2FA too. The technique used here would be extensible to PAM and SMTP sources. Signed-off-by: Andrew Thornton <[email protected]>
1 parent f96d0d3 commit 27b351a

File tree

16 files changed

+84
-19
lines changed

16 files changed

+84
-19
lines changed

‎cmd/admin_auth_ldap.go

+8
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ var (
8989
Name: "public-ssh-key-attribute",
9090
Usage: "The attribute of the user’s LDAP record containing the user’s public ssh key.",
9191
},
92+
cli.BoolFlag{
93+
Name: "skip-local-2fa",
94+
Usage: "Set to true to skip local 2fa for users authenticated by this source",
95+
},
9296
}
9397

9498
ldapBindDnCLIFlags = append(commonLdapCLIFlags,
@@ -245,6 +249,10 @@ func parseLdapConfig(c *cli.Context, config *ldap.Source) error {
245249
if c.IsSet("allow-deactivate-all") {
246250
config.AllowDeactivateAll = c.Bool("allow-deactivate-all")
247251
}
252+
if c.IsSet("skip-local-2fa") {
253+
config.SkipLocalTwoFA = c.Bool("skip-local-2fa")
254+
}
255+
248256
return nil
249257
}
250258

‎modules/context/api.go

+4
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,10 @@ func (ctx *APIContext) RequireCSRF() {
214214

215215
// CheckForOTP validates OTP
216216
func (ctx *APIContext) CheckForOTP() {
217+
if skip, ok := ctx.Data["SkipLocalTwoFA"]; ok && skip.(bool) {
218+
return // Skip 2FA
219+
}
220+
217221
otpHeader := ctx.Req.Header.Get("X-Gitea-OTP")
218222
twofa, err := models.GetTwoFactorByUID(ctx.Context.User.ID)
219223
if err != nil {

‎modules/context/auth.go

+3
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,9 @@ func ToggleAPI(options *ToggleOptions) func(ctx *APIContext) {
151151
return
152152
}
153153
if ctx.IsSigned && ctx.IsBasicAuth {
154+
if skip, ok := ctx.Data["SkipLocalTwoFA"]; ok && skip.(bool) {
155+
return // Skip 2FA
156+
}
154157
twofa, err := models.GetTwoFactorByUID(ctx.User.ID)
155158
if err != nil {
156159
if models.IsErrTwoFactorNotEnrolled(err) {

‎routers/web/admin/auths.go

+1
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ func parseLDAPConfig(form forms.AuthenticationForm) *ldap.Source {
145145
RestrictedFilter: form.RestrictedFilter,
146146
AllowDeactivateAll: form.AllowDeactivateAll,
147147
Enabled: true,
148+
SkipLocalTwoFA: form.SkipLocalTwoFA,
148149
}
149150
}
150151

‎routers/web/user/auth.go

+12-2
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ func SignInPost(ctx *context.Context) {
175175
}
176176

177177
form := web.GetForm(ctx).(*forms.SignInForm)
178-
u, err := auth.UserSignIn(form.UserName, form.Password)
178+
u, source, err := auth.UserSignIn(form.UserName, form.Password)
179179
if err != nil {
180180
if models.IsErrUserNotExist(err) {
181181
ctx.RenderWithErr(ctx.Tr("form.username_password_incorrect"), tplSignIn, &form)
@@ -201,6 +201,15 @@ func SignInPost(ctx *context.Context) {
201201
}
202202
return
203203
}
204+
205+
// Now handle 2FA:
206+
207+
// First of all if the source can skip local two fa we're done
208+
if skipper, ok := source.Cfg.(auth.LocalTwoFASkipper); ok && skipper.IsSkipLocalTwoFA() {
209+
handleSignIn(ctx, u, form.Remember)
210+
return
211+
}
212+
204213
// If this user is enrolled in 2FA, we can't sign the user in just yet.
205214
// Instead, redirect them to the 2FA authentication page.
206215
_, err = models.GetTwoFactorByUID(u.ID)
@@ -905,7 +914,7 @@ func LinkAccountPostSignIn(ctx *context.Context) {
905914
return
906915
}
907916

908-
u, err := auth.UserSignIn(signInForm.UserName, signInForm.Password)
917+
u, _, err := auth.UserSignIn(signInForm.UserName, signInForm.Password)
909918
if err != nil {
910919
if models.IsErrUserNotExist(err) {
911920
ctx.Data["user_exists"] = true
@@ -924,6 +933,7 @@ func linkAccount(ctx *context.Context, u *models.User, gothUser goth.User, remem
924933

925934
// If this user is enrolled in 2FA, we can't sign the user in just yet.
926935
// Instead, redirect them to the 2FA authentication page.
936+
// We deliberately ignore the skip local 2fa setting here because we are linking to a previous user here
927937
_, err := models.GetTwoFactorByUID(u.ID)
928938
if err != nil {
929939
if !models.IsErrTwoFactorNotEnrolled(err) {

‎routers/web/user/auth_openid.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ func ConnectOpenIDPost(ctx *context.Context) {
291291
ctx.Data["EnableOpenIDSignUp"] = setting.Service.EnableOpenIDSignUp
292292
ctx.Data["OpenID"] = oid
293293

294-
u, err := auth.UserSignIn(form.UserName, form.Password)
294+
u, _, err := auth.UserSignIn(form.UserName, form.Password)
295295
if err != nil {
296296
if models.IsErrUserNotExist(err) {
297297
ctx.RenderWithErr(ctx.Tr("form.username_password_incorrect"), tplConnectOID, &form)

‎routers/web/user/setting/account.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ func DeleteAccount(ctx *context.Context) {
229229
ctx.Data["Title"] = ctx.Tr("settings")
230230
ctx.Data["PageIsSettingsAccount"] = true
231231

232-
if _, err := auth.UserSignIn(ctx.User.Name, ctx.FormString("password")); err != nil {
232+
if _, _, err := auth.UserSignIn(ctx.User.Name, ctx.FormString("password")); err != nil {
233233
if models.IsErrUserNotExist(err) {
234234
loadAccountData(ctx)
235235

‎services/auth/basic.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -107,14 +107,18 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore
107107
}
108108

109109
log.Trace("Basic Authorization: Attempting SignIn for %s", uname)
110-
u, err := UserSignIn(uname, passwd)
110+
u, source, err := UserSignIn(uname, passwd)
111111
if err != nil {
112112
if !models.IsErrUserNotExist(err) {
113113
log.Error("UserSignIn: %v", err)
114114
}
115115
return nil
116116
}
117117

118+
if skipper, ok := source.Cfg.(LocalTwoFASkipper); ok && skipper.IsSkipLocalTwoFA() {
119+
store.GetData()["SkipLocalTwoFA"] = true
120+
}
121+
118122
log.Trace("Basic Authorization: Logged in user %-v", u)
119123

120124
return u

‎services/auth/interface.go

+5
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,11 @@ type PasswordAuthenticator interface {
5454
Authenticate(user *models.User, login, password string) (*models.User, error)
5555
}
5656

57+
// LocalTwoFASkipper represents a source of authentication that can skip local 2fa
58+
type LocalTwoFASkipper interface {
59+
IsSkipLocalTwoFA() bool
60+
}
61+
5762
// SynchronizableSource represents a source that can synchronize users
5863
type SynchronizableSource interface {
5964
Sync(ctx context.Context, updateExisting bool) error

‎services/auth/signin.go

+14-14
Original file line numberDiff line numberDiff line change
@@ -20,66 +20,66 @@ import (
2020
)
2121

2222
// UserSignIn validates user name and password.
23-
func UserSignIn(username, password string) (*models.User, error) {
23+
func UserSignIn(username, password string) (*models.User, *models.LoginSource, error) {
2424
var user *models.User
2525
if strings.Contains(username, "@") {
2626
user = &models.User{Email: strings.ToLower(strings.TrimSpace(username))}
2727
// check same email
2828
cnt, err := models.Count(user)
2929
if err != nil {
30-
return nil, err
30+
return nil, nil, err
3131
}
3232
if cnt > 1 {
33-
return nil, models.ErrEmailAlreadyUsed{
33+
return nil, nil, models.ErrEmailAlreadyUsed{
3434
Email: user.Email,
3535
}
3636
}
3737
} else {
3838
trimmedUsername := strings.TrimSpace(username)
3939
if len(trimmedUsername) == 0 {
40-
return nil, models.ErrUserNotExist{Name: username}
40+
return nil, nil, models.ErrUserNotExist{Name: username}
4141
}
4242

4343
user = &models.User{LowerName: strings.ToLower(trimmedUsername)}
4444
}
4545

4646
hasUser, err := models.GetUser(user)
4747
if err != nil {
48-
return nil, err
48+
return nil, nil, err
4949
}
5050

5151
if hasUser {
5252
source, err := models.GetLoginSourceByID(user.LoginSource)
5353
if err != nil {
54-
return nil, err
54+
return nil, nil, err
5555
}
5656

5757
if !source.IsActive {
58-
return nil, models.ErrLoginSourceNotActived
58+
return nil, nil, models.ErrLoginSourceNotActived
5959
}
6060

6161
authenticator, ok := source.Cfg.(PasswordAuthenticator)
6262
if !ok {
63-
return nil, models.ErrUnsupportedLoginType
63+
return nil, nil, models.ErrUnsupportedLoginType
6464
}
6565

6666
user, err := authenticator.Authenticate(user, username, password)
6767
if err != nil {
68-
return nil, err
68+
return nil, nil, err
6969
}
7070

7171
// WARN: DON'T check user.IsActive, that will be checked on reqSign so that
7272
// user could be hint to resend confirm email.
7373
if user.ProhibitLogin {
74-
return nil, models.ErrUserProhibitLogin{UID: user.ID, Name: user.Name}
74+
return nil, nil, models.ErrUserProhibitLogin{UID: user.ID, Name: user.Name}
7575
}
7676

77-
return user, nil
77+
return user, source, nil
7878
}
7979

8080
sources, err := models.AllActiveLoginSources()
8181
if err != nil {
82-
return nil, err
82+
return nil, nil, err
8383
}
8484

8585
for _, source := range sources {
@@ -97,7 +97,7 @@ func UserSignIn(username, password string) (*models.User, error) {
9797

9898
if err == nil {
9999
if !authUser.ProhibitLogin {
100-
return authUser, nil
100+
return authUser, source, nil
101101
}
102102
err = models.ErrUserProhibitLogin{UID: authUser.ID, Name: authUser.Name}
103103
}
@@ -109,5 +109,5 @@ func UserSignIn(username, password string) (*models.User, error) {
109109
}
110110
}
111111

112-
return nil, models.ErrUserNotExist{Name: username}
112+
return nil, nil, models.ErrUserNotExist{Name: username}
113113
}

‎services/auth/source/ldap/assert_interface_test.go

+1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
type sourceInterface interface {
1717
auth.PasswordAuthenticator
1818
auth.SynchronizableSource
19+
auth.LocalTwoFASkipper
1920
models.SSHKeyProvider
2021
models.LoginConfig
2122
models.SkipVerifiable

‎services/auth/source/ldap/source.go

+1
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ type Source struct {
5252
GroupFilter string // Group Name Filter
5353
GroupMemberUID string // Group Attribute containing array of UserUID
5454
UserUID string // User Attribute listed in Group
55+
SkipLocalTwoFA bool // Skip Local 2fa for users authenticated with this source
5556

5657
// reference to the loginSource
5758
loginSource *models.LoginSource

‎services/auth/source/ldap/source_authenticate.go

+5
Original file line numberDiff line numberDiff line change
@@ -97,3 +97,8 @@ func (source *Source) Authenticate(user *models.User, login, password string) (*
9797

9898
return user, err
9999
}
100+
101+
// IsSkipLocalTwoFA returns if this source should skip local 2fa for password authentication
102+
func (source *Source) IsSkipLocalTwoFA() bool {
103+
return source.SkipLocalTwoFA
104+
}

‎services/auth/source/oauth2/source_authenticate.go

+3
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,6 @@ import (
1313
func (source *Source) Authenticate(user *models.User, login, password string) (*models.User, error) {
1414
return db.Authenticate(user, login, password)
1515
}
16+
17+
// NB: Oauth2 does not implement LocalTwoFASkipper for password authentication
18+
// as its password authentication drops to db authentication

‎templates/admin/auth/edit.tmpl

+7
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,13 @@
147147
</div>
148148
</div>
149149
{{end}}
150+
<div class="optional field">
151+
<div class="ui checkbox">
152+
<label for="skip_local_two_fa"><strong>{{.i18n.Tr "admin.auths.skip_local_two_fa"}}</strong></label>
153+
<input id="skip_local_two_fa" name="skip_local_two_fa" type="checkbox" {{if $cfg.SkipLocalTwoFA}}checked{{end}}>
154+
<p class="help">{{.i18n.Tr "admin.auths.skip_local_two_fa_helper"}}</p>
155+
</div>
156+
</div>
150157
<div class="inline field">
151158
<div class="ui checkbox">
152159
<label for="allow_deactivate_all"><strong>{{.i18n.Tr "admin.auths.allow_deactivate_all"}}</strong></label>

‎templates/admin/auth/source/ldap.tmpl

+13
Original file line numberDiff line numberDiff line change
@@ -111,4 +111,17 @@
111111
<label for="search_page_size">{{.i18n.Tr "admin.auths.search_page_size"}}</label>
112112
<input id="search_page_size" name="search_page_size" value="{{.search_page_size}}">
113113
</div>
114+
<div class="optional field">
115+
<div class="ui checkbox">
116+
<label for="skip_local_two_fa"><strong>{{.i18n.Tr "admin.auths.skip_local_two_fa"}}</strong></label>
117+
<input id="skip_local_two_fa" name="skip_local_two_fa" type="checkbox" {{if .skip_local_two_fa}}checked{{end}}>
118+
<p class="help">{{.i18n.Tr "admin.auths.skip_local_two_fa_helper"}}</p>
119+
</div>
120+
</div>
121+
<div class="inline field">
122+
<div class="ui checkbox">
123+
<label for="allow_deactivate_all"><strong>{{.i18n.Tr "admin.auths.allow_deactivate_all"}}</strong></label>
124+
<input id="allow_deactivate_all" name="allow_deactivate_all" type="checkbox" {{if .allow_deactivate_all}}checked{{end}}>
125+
</div>
126+
</div>
114127
</div>

0 commit comments

Comments
 (0)