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 <art27@cantab.net>

diff --git a/cmd/admin_auth_ldap.go b/cmd/admin_auth_ldap.go
index 4314930a3e0834a1e9d8439bd855856f99bf5e8d..feeaf17661f7ce4ad3a89f8d524af8977d37fa67 100644 (file)
--- a/cmd/admin_auth_ldap.go
+++ b/cmd/admin_auth_ldap.go
@@ -89,6 +89,10 @@ var (
                        Name:  "public-ssh-key-attribute",
                        Usage: "The attribute of the user’s LDAP record containing the user’s public ssh key.",
                },
+               cli.BoolFlag{
+                       Name:  "skip-local-2fa",
+                       Usage: "Set to true to skip local 2fa for users authenticated by this source",
+               },
        }
 
        ldapBindDnCLIFlags = append(commonLdapCLIFlags,
@@ -245,6 +249,10 @@ func parseLdapConfig(c *cli.Context, config *ldap.Source) error {
        if c.IsSet("allow-deactivate-all") {
                config.AllowDeactivateAll = c.Bool("allow-deactivate-all")
        }
+       if c.IsSet("skip-local-2fa") {
+               config.SkipLocalTwoFA = c.Bool("skip-local-2fa")
+       }
+
        return nil
 }
 

diff --git a/modules/context/api.go b/modules/context/api.go
index 47ea8acfe05f3dd653bbbfbb9be406eea5d8e82e..e80e63cd96233e72607fcec17c2b7b3f8da2248a 100644 (file)
--- a/modules/context/api.go
+++ b/modules/context/api.go
@@ -214,6 +214,10 @@ func (ctx *APIContext) RequireCSRF() {
 
 // CheckForOTP validates OTP
 func (ctx *APIContext) CheckForOTP() {
+       if skip, ok := ctx.Data["SkipLocalTwoFA"]; ok && skip.(bool) {
+               return // Skip 2FA
+       }
+
        otpHeader := ctx.Req.Header.Get("X-Gitea-OTP")
        twofa, err := models.GetTwoFactorByUID(ctx.Context.User.ID)
        if err != nil {

diff --git a/modules/context/auth.go b/modules/context/auth.go
index ed220d542032bfe2cb436999b7cb27e36e21b015..0a62b2741e4a76589a16af892ec1fdec8f028437 100644 (file)
--- a/modules/context/auth.go
+++ b/modules/context/auth.go
@@ -151,6 +151,9 @@ func ToggleAPI(options *ToggleOptions) func(ctx *APIContext) {
                                return
                        }
                        if ctx.IsSigned && ctx.IsBasicAuth {
+                               if skip, ok := ctx.Data["SkipLocalTwoFA"]; ok && skip.(bool) {
+                                       return // Skip 2FA
+                               }
                                twofa, err := models.GetTwoFactorByUID(ctx.User.ID)
                                if err != nil {
                                        if models.IsErrTwoFactorNotEnrolled(err) {

diff --git a/routers/web/admin/auths.go b/routers/web/admin/auths.go
index b2879d7c4f66c38a100fe30913618fb4776290ad..2937190a1f5010d4498e3a9a3518ac59ab973a0e 100644 (file)
--- a/routers/web/admin/auths.go
+++ b/routers/web/admin/auths.go
@@ -145,6 +145,7 @@ func parseLDAPConfig(form forms.AuthenticationForm) *ldap.Source {
                RestrictedFilter:      form.RestrictedFilter,
                AllowDeactivateAll:    form.AllowDeactivateAll,
                Enabled:               true,
+               SkipLocalTwoFA:        form.SkipLocalTwoFA,
        }
 }
 

diff --git a/routers/web/user/auth.go b/routers/web/user/auth.go
index 38e0d989b8d56cd5a9b206ca8ae2932548388168..a5c0a14d17f3efd63446a6923191bac2ee6bbf9e 100644 (file)
--- a/routers/web/user/auth.go
+++ b/routers/web/user/auth.go
@@ -175,7 +175,7 @@ func SignInPost(ctx *context.Context) {
        }
 
        form := web.GetForm(ctx).(*forms.SignInForm)
-       u, err := auth.UserSignIn(form.UserName, form.Password)
+       u, source, err := auth.UserSignIn(form.UserName, form.Password)
        if err != nil {
                if models.IsErrUserNotExist(err) {
                        ctx.RenderWithErr(ctx.Tr("form.username_password_incorrect"), tplSignIn, &form)
@@ -201,6 +201,15 @@ func SignInPost(ctx *context.Context) {
                }
                return
        }
+
+       // Now handle 2FA:
+
+       // First of all if the source can skip local two fa we're done
+       if skipper, ok := source.Cfg.(auth.LocalTwoFASkipper); ok && skipper.IsSkipLocalTwoFA() {
+               handleSignIn(ctx, u, form.Remember)
+               return
+       }
+
        // If this user is enrolled in 2FA, we can't sign the user in just yet.
        // Instead, redirect them to the 2FA authentication page.
        _, err = models.GetTwoFactorByUID(u.ID)
@@ -905,7 +914,7 @@ func LinkAccountPostSignIn(ctx *context.Context) {
                return
        }
 
-       u, err := auth.UserSignIn(signInForm.UserName, signInForm.Password)
+       u, _, err := auth.UserSignIn(signInForm.UserName, signInForm.Password)
        if err != nil {
                if models.IsErrUserNotExist(err) {
                        ctx.Data["user_exists"] = true
@@ -924,6 +933,7 @@ func linkAccount(ctx *context.Context, u *models.User, gothUser goth.User, remem
 
        // If this user is enrolled in 2FA, we can't sign the user in just yet.
        // Instead, redirect them to the 2FA authentication page.
+       // We deliberately ignore the skip local 2fa setting here because we are linking to a previous user here
        _, err := models.GetTwoFactorByUID(u.ID)
        if err != nil {
                if !models.IsErrTwoFactorNotEnrolled(err) {

diff --git a/routers/web/user/auth_openid.go b/routers/web/user/auth_openid.go
index fc419a7f6ea633df88a83c62e4557b26b1188351..e6ad6fef4c413d8d49fca2cf4a22979cc864f760 100644 (file)
--- a/routers/web/user/auth_openid.go
+++ b/routers/web/user/auth_openid.go
@@ -291,7 +291,7 @@ func ConnectOpenIDPost(ctx *context.Context) {
        ctx.Data["EnableOpenIDSignUp"] = setting.Service.EnableOpenIDSignUp
        ctx.Data["OpenID"] = oid
 
-       u, err := auth.UserSignIn(form.UserName, form.Password)
+       u, _, err := auth.UserSignIn(form.UserName, form.Password)
        if err != nil {
                if models.IsErrUserNotExist(err) {
                        ctx.RenderWithErr(ctx.Tr("form.username_password_incorrect"), tplConnectOID, &form)

diff --git a/routers/web/user/setting/account.go b/routers/web/user/setting/account.go
index 6201078954412554a7297eaf52c1ea572724e00f..249793578a1a9ed61f9cb9b6c700b3a3090e52bd 100644 (file)
--- a/routers/web/user/setting/account.go
+++ b/routers/web/user/setting/account.go
@@ -229,7 +229,7 @@ func DeleteAccount(ctx *context.Context) {
        ctx.Data["Title"] = ctx.Tr("settings")
        ctx.Data["PageIsSettingsAccount"] = true
 
-       if _, err := auth.UserSignIn(ctx.User.Name, ctx.FormString("password")); err != nil {
+       if _, _, err := auth.UserSignIn(ctx.User.Name, ctx.FormString("password")); err != nil {
                if models.IsErrUserNotExist(err) {
                        loadAccountData(ctx)
 

diff --git a/services/auth/basic.go b/services/auth/basic.go
index 244f63d2f733ad5dc18749a52b94bbd4394027d4..0c400b6b5329ade7623d95bf9a2ff0d96a2b8d48 100644 (file)
--- a/services/auth/basic.go
+++ b/services/auth/basic.go
@@ -107,7 +107,7 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore
        }
 
        log.Trace("Basic Authorization: Attempting SignIn for %s", uname)
-       u, err := UserSignIn(uname, passwd)
+       u, source, err := UserSignIn(uname, passwd)
        if err != nil {
                if !models.IsErrUserNotExist(err) {
                        log.Error("UserSignIn: %v", err)
@@ -115,6 +115,10 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore
                return nil
        }
 
+       if skipper, ok := source.Cfg.(LocalTwoFASkipper); ok && skipper.IsSkipLocalTwoFA() {
+               store.GetData()["SkipLocalTwoFA"] = true
+       }
+
        log.Trace("Basic Authorization: Logged in user %-v", u)
 
        return u

diff --git a/services/auth/interface.go b/services/auth/interface.go
index 51c7043370bb18648fde419a70fdd6bc6c4e7e54..a198fbe5b847999f866e70ce47d811991a267a1b 100644 (file)
--- a/services/auth/interface.go
+++ b/services/auth/interface.go
@@ -54,6 +54,11 @@ type PasswordAuthenticator interface {
        Authenticate(user *models.User, login, password string) (*models.User, error)
 }
 
+// LocalTwoFASkipper represents a source of authentication that can skip local 2fa
+type LocalTwoFASkipper interface {
+       IsSkipLocalTwoFA() bool
+}
+
 // SynchronizableSource represents a source that can synchronize users
 type SynchronizableSource interface {
        Sync(ctx context.Context, updateExisting bool) error

diff --git a/services/auth/signin.go b/services/auth/signin.go
index 2c4bf9b35b7e3b7ce1fc3a9098ef3d149ec603e3..0ac2634c80a32d79c9507a604bd4c6afbde29ef7 100644 (file)
--- a/services/auth/signin.go
+++ b/services/auth/signin.go
@@ -20,24 +20,24 @@ import (
 )
 
 // UserSignIn validates user name and password.
-func UserSignIn(username, password string) (*models.User, error) {
+func UserSignIn(username, password string) (*models.User, *models.LoginSource, error) {
        var user *models.User
        if strings.Contains(username, "@") {
                user = &models.User{Email: strings.ToLower(strings.TrimSpace(username))}
                // check same email
                cnt, err := models.Count(user)
                if err != nil {
-                       return nil, err
+                       return nil, nil, err
                }
                if cnt > 1 {
-                       return nil, models.ErrEmailAlreadyUsed{
+                       return nil, nil, models.ErrEmailAlreadyUsed{
                                Email: user.Email,
                        }
                }
        } else {
                trimmedUsername := strings.TrimSpace(username)
                if len(trimmedUsername) == 0 {
-                       return nil, models.ErrUserNotExist{Name: username}
+                       return nil, nil, models.ErrUserNotExist{Name: username}
                }
 
                user = &models.User{LowerName: strings.ToLower(trimmedUsername)}
@@ -45,41 +45,41 @@ func UserSignIn(username, password string) (*models.User, error) {
 
        hasUser, err := models.GetUser(user)
        if err != nil {
-               return nil, err
+               return nil, nil, err
        }
 
        if hasUser {
                source, err := models.GetLoginSourceByID(user.LoginSource)
                if err != nil {
-                       return nil, err
+                       return nil, nil, err
                }
 
                if !source.IsActive {
-                       return nil, models.ErrLoginSourceNotActived
+                       return nil, nil, models.ErrLoginSourceNotActived
                }
 
                authenticator, ok := source.Cfg.(PasswordAuthenticator)
                if !ok {
-                       return nil, models.ErrUnsupportedLoginType
+                       return nil, nil, models.ErrUnsupportedLoginType
                }
 
                user, err := authenticator.Authenticate(user, username, password)
                if err != nil {
-                       return nil, err
+                       return nil, nil, err
                }
 
                // WARN: DON'T check user.IsActive, that will be checked on reqSign so that
                // user could be hint to resend confirm email.
                if user.ProhibitLogin {
-                       return nil, models.ErrUserProhibitLogin{UID: user.ID, Name: user.Name}
+                       return nil, nil, models.ErrUserProhibitLogin{UID: user.ID, Name: user.Name}
                }
 
-               return user, nil
+               return user, source, nil
        }
 
        sources, err := models.AllActiveLoginSources()
        if err != nil {
-               return nil, err
+               return nil, nil, err
        }
 
        for _, source := range sources {
@@ -97,7 +97,7 @@ func UserSignIn(username, password string) (*models.User, error) {
 
                if err == nil {
                        if !authUser.ProhibitLogin {
-                               return authUser, nil
+                               return authUser, source, nil
                        }
                        err = models.ErrUserProhibitLogin{UID: authUser.ID, Name: authUser.Name}
                }
@@ -109,5 +109,5 @@ func UserSignIn(username, password string) (*models.User, error) {
                }
        }
 
-       return nil, models.ErrUserNotExist{Name: username}
+       return nil, nil, models.ErrUserNotExist{Name: username}
 }

diff --git a/services/auth/source/ldap/assert_interface_test.go b/services/auth/source/ldap/assert_interface_test.go
index 4cf3eafe76358f84392403c9225da99164ca78d3..a0425d2f763c1089b442794af7cb84c40eac60aa 100644 (file)
--- a/services/auth/source/ldap/assert_interface_test.go
+++ b/services/auth/source/ldap/assert_interface_test.go
@@ -16,6 +16,7 @@ import (
 type sourceInterface interface {
        auth.PasswordAuthenticator
        auth.SynchronizableSource
+       auth.LocalTwoFASkipper
        models.SSHKeyProvider
        models.LoginConfig
        models.SkipVerifiable

diff --git a/services/auth/source/ldap/source.go b/services/auth/source/ldap/source.go
index 79f118f784654c96dd5eb584782f7a78162c78c2..d1228d41aeb1160a9ee9a57517fcfa0538046671 100644 (file)
--- a/services/auth/source/ldap/source.go
+++ b/services/auth/source/ldap/source.go
@@ -52,6 +52,7 @@ type Source struct {
        GroupFilter           string // Group Name Filter
        GroupMemberUID        string // Group Attribute containing array of UserUID
        UserUID               string // User Attribute listed in Group
+       SkipLocalTwoFA        bool   // Skip Local 2fa for users authenticated with this source
 
        // reference to the loginSource
        loginSource *models.LoginSource

diff --git a/services/auth/source/ldap/source_authenticate.go b/services/auth/source/ldap/source_authenticate.go
index ecc95fbd56efff5f2951b7927111b8ad3f039e94..46478e60296edacc920feb0ae804fee9e81c2e9e 100644 (file)
--- a/services/auth/source/ldap/source_authenticate.go
+++ b/services/auth/source/ldap/source_authenticate.go
@@ -97,3 +97,8 @@ func (source *Source) Authenticate(user *models.User, login, password string) (*
 
        return user, err
 }
+
+// IsSkipLocalTwoFA returns if this source should skip local 2fa for password authentication
+func (source *Source) IsSkipLocalTwoFA() bool {
+       return source.SkipLocalTwoFA
+}

diff --git a/services/auth/source/oauth2/source_authenticate.go b/services/auth/source/oauth2/source_authenticate.go
index 2e39f245dff074bfcafe5e4d31ac196d397ddb47..be2ff05356cd4028974273906c8d92fa29f1329c 100644 (file)
--- a/services/auth/source/oauth2/source_authenticate.go
+++ b/services/auth/source/oauth2/source_authenticate.go
@@ -13,3 +13,6 @@ import (
 func (source *Source) Authenticate(user *models.User, login, password string) (*models.User, error) {
        return db.Authenticate(user, login, password)
 }
+
+// NB: Oauth2 does not implement LocalTwoFASkipper for password authentication
+// as its password authentication drops to db authentication

diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl
index 3e21710353e3547d6d1f8f38e01acead03dbe442..9ff80663846b0191228c196b907c08116a2b873d 100644 (file)
--- a/templates/admin/auth/edit.tmpl
+++ b/templates/admin/auth/edit.tmpl
@@ -147,6 +147,13 @@
                                                        </div>
                                                </div>
                                        {{end}}
+                                       <div class="optional field">
+                                               <div class="ui checkbox">
+                                                       <label for="skip_local_two_fa"><strong>{{.i18n.Tr "admin.auths.skip_local_two_fa"}}</strong></label>
+                                                       <input id="skip_local_two_fa" name="skip_local_two_fa" type="checkbox" {{if $cfg.SkipLocalTwoFA}}checked{{end}}>
+                                                       <p class="help">{{.i18n.Tr "admin.auths.skip_local_two_fa_helper"}}</p>
+                                               </div>
+                                       </div>
                                        <div class="inline field">
                                                <div class="ui checkbox">
                                                        <label for="allow_deactivate_all"><strong>{{.i18n.Tr "admin.auths.allow_deactivate_all"}}</strong></label>

diff --git a/templates/admin/auth/source/ldap.tmpl b/templates/admin/auth/source/ldap.tmpl
index 295e001cf4a365d37daee578e50672a521004bf3..b8018f5b548ccefe671da8fb958d9625a509b073 100644 (file)
--- a/templates/admin/auth/source/ldap.tmpl
+++ b/templates/admin/auth/source/ldap.tmpl
@@ -111,4 +111,17 @@
                <label for="search_page_size">{{.i18n.Tr "admin.auths.search_page_size"}}</label>
                <input id="search_page_size" name="search_page_size" value="{{.search_page_size}}">
        </div>
+       <div class="optional field">
+               <div class="ui checkbox">
+                       <label for="skip_local_two_fa"><strong>{{.i18n.Tr "admin.auths.skip_local_two_fa"}}</strong></label>
+                       <input id="skip_local_two_fa" name="skip_local_two_fa" type="checkbox" {{if .skip_local_two_fa}}checked{{end}}>
+                       <p class="help">{{.i18n.Tr "admin.auths.skip_local_two_fa_helper"}}</p>
+               </div>
+       </div>
+       <div class="inline field">
+               <div class="ui checkbox">
+                       <label for="allow_deactivate_all"><strong>{{.i18n.Tr "admin.auths.allow_deactivate_all"}}</strong></label>
+                       <input id="allow_deactivate_all" name="allow_deactivate_all" type="checkbox" {{if .allow_deactivate_all}}checked{{end}}>
+               </div>
+       </div>
 </div>