From 27b351aba564804f65e5574919a88d6194c75256 Mon Sep 17 00:00:00 2001 From: zeripath Date: Fri, 17 Sep 2021 12:43:47 +0100 Subject: [PATCH] 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 --- cmd/admin_auth_ldap.go | 8 ++++++ modules/context/api.go | 4 +++ modules/context/auth.go | 3 ++ routers/web/admin/auths.go | 1 + routers/web/user/auth.go | 14 ++++++++-- routers/web/user/auth_openid.go | 2 +- routers/web/user/setting/account.go | 2 +- services/auth/basic.go | 6 +++- services/auth/interface.go | 5 ++++ services/auth/signin.go | 28 +++++++++---------- .../auth/source/ldap/assert_interface_test.go | 1 + services/auth/source/ldap/source.go | 1 + .../auth/source/ldap/source_authenticate.go | 5 ++++ .../auth/source/oauth2/source_authenticate.go | 3 ++ templates/admin/auth/edit.tmpl | 7 +++++ templates/admin/auth/source/ldap.tmpl | 13 +++++++++ 16 files changed, 84 insertions(+), 19 deletions(-) diff --git a/cmd/admin_auth_ldap.go b/cmd/admin_auth_ldap.go index 4314930a3e..feeaf17661 100644 --- 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 47ea8acfe0..e80e63cd96 100644 --- 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 ed220d5420..0a62b2741e 100644 --- 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 b2879d7c4f..2937190a1f 100644 --- 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 38e0d989b8..a5c0a14d17 100644 --- 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 fc419a7f6e..e6ad6fef4c 100644 --- 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 6201078954..249793578a 100644 --- 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 244f63d2f7..0c400b6b53 100644 --- 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 51c7043370..a198fbe5b8 100644 --- 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 2c4bf9b35b..0ac2634c80 100644 --- 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 4cf3eafe76..a0425d2f76 100644 --- 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 79f118f784..d1228d41ae 100644 --- 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 ecc95fbd56..46478e6029 100644 --- 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 2e39f245df..be2ff05356 100644 --- 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 3e21710353..9ff8066384 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -147,6 +147,13 @@ {{end}} +
+
+ + +

{{.i18n.Tr "admin.auths.skip_local_two_fa_helper"}}

+
+
diff --git a/templates/admin/auth/source/ldap.tmpl b/templates/admin/auth/source/ldap.tmpl index 295e001cf4..b8018f5b54 100644 --- a/templates/admin/auth/source/ldap.tmpl +++ b/templates/admin/auth/source/ldap.tmpl @@ -111,4 +111,17 @@
+
+
+ + +

{{.i18n.Tr "admin.auths.skip_local_two_fa_helper"}}

+
+
+
+
+ + +
+