diff --git a/models/fixtures/u2f_registration.yml b/models/fixtures/u2f_registration.yml index 4a9d1d9624..60555c43f1 100644 --- a/models/fixtures/u2f_registration.yml +++ b/models/fixtures/u2f_registration.yml @@ -1,7 +1,7 @@ - id: 1 name: "U2F Key" - user_id: 1 + user_id: 32 counter: 0 created_unix: 946684800 updated_unix: 946684800 diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index c49fe1b656..cf07542eed 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -542,3 +542,19 @@ avatar_email: user31@example.com num_repos: 0 is_active: true + +- + id: 32 + lower_name: user32 + name: user32 + full_name: User 32 (U2F test) + email: user32@example.com + passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password + type: 0 # individual + salt: ZogKvWdyEx + is_admin: false + is_restricted: false + avatar: avatar32 + avatar_email: user30@example.com + num_repos: 0 + is_active: true diff --git a/models/login/twofactor.go b/models/login/twofactor.go index 1c4d2734fc..acb5e1b2d5 100644 --- a/models/login/twofactor.go +++ b/models/login/twofactor.go @@ -136,6 +136,12 @@ func GetTwoFactorByUID(uid int64) (*TwoFactor, error) { return twofa, nil } +// HasTwoFactorByUID returns the two-factor authentication token associated with +// the user, if any. +func HasTwoFactorByUID(uid int64) (bool, error) { + return db.GetEngine(db.DefaultContext).Where("uid=?", uid).Exist(&TwoFactor{}) +} + // DeleteTwoFactorByID deletes two-factor authentication token by given ID. func DeleteTwoFactorByID(id, userID int64) error { cnt, err := db.GetEngine(db.DefaultContext).ID(id).Delete(&TwoFactor{ diff --git a/models/login/u2f.go b/models/login/u2f.go index 05d39cc05e..8cea98463f 100644 --- a/models/login/u2f.go +++ b/models/login/u2f.go @@ -115,6 +115,11 @@ func GetU2FRegistrationsByUID(uid int64) (U2FRegistrationList, error) { return getU2FRegistrationsByUID(db.GetEngine(db.DefaultContext), uid) } +// HasU2FRegistrationsByUID returns whether a given user has U2F registrations +func HasU2FRegistrationsByUID(uid int64) (bool, error) { + return db.GetEngine(db.DefaultContext).Where("user_id = ?", uid).Exist(&U2FRegistration{}) +} + func createRegistration(e db.Engine, userID int64, name string, reg *u2f.Registration) (*U2FRegistration, error) { raw, err := reg.MarshalBinary() if err != nil { diff --git a/models/login/u2f_test.go b/models/login/u2f_test.go index 32505b62a6..8f5cea6150 100644 --- a/models/login/u2f_test.go +++ b/models/login/u2f_test.go @@ -29,7 +29,7 @@ func TestGetU2FRegistrationByID(t *testing.T) { func TestGetU2FRegistrationsByUID(t *testing.T) { assert.NoError(t, db.PrepareTestDatabase()) - res, err := GetU2FRegistrationsByUID(1) + res, err := GetU2FRegistrationsByUID(32) assert.NoError(t, err) assert.Len(t, res, 1) diff --git a/models/user_test.go b/models/user_test.go index 2dcca20346..3f3536dafa 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -147,13 +147,13 @@ func TestSearchUsers(t *testing.T) { } testUserSuccess(&SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}}, - []int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30}) + []int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32}) testUserSuccess(&SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsActive: util.OptionalBoolFalse}, []int64{9}) testUserSuccess(&SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: util.OptionalBoolTrue}, - []int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 28, 29, 30}) + []int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 28, 29, 30, 32}) testUserSuccess(&SearchUserOptions{Keyword: "user1", OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: util.OptionalBoolTrue}, []int64{1, 10, 11, 12, 13, 14, 15, 16, 18}) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 4a9e3c3894..2632531e2f 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -714,7 +714,6 @@ twofa_enrolled = Your account has been enrolled into two-factor authentication. twofa_failed_get_secret = Failed to get secret. u2f_desc = Security keys are hardware devices containing cryptographic keys. They can be used for two-factor authentication. Security keys must support the FIDO U2F standard. -u2f_require_twofa = Your account must be enrolled in two-factor authentication to use security keys. u2f_register_key = Add Security Key u2f_nickname = Nickname u2f_press_button = Press the button on your security key to register it. diff --git a/routers/web/user/auth.go b/routers/web/user/auth.go index 21d48e9834..55f304a7cb 100644 --- a/routers/web/user/auth.go +++ b/routers/web/user/auth.go @@ -211,38 +211,58 @@ func SignInPost(ctx *context.Context) { return } - // If this user is enrolled in 2FA, we can't sign the user in just yet. + // If this user is enrolled in 2FA TOTP, we can't sign the user in just yet. // Instead, redirect them to the 2FA authentication page. - _, err = login.GetTwoFactorByUID(u.ID) + hasTOTPtwofa, err := login.HasTwoFactorByUID(u.ID) if err != nil { - if login.IsErrTwoFactorNotEnrolled(err) { - handleSignIn(ctx, u, form.Remember) - } else { - ctx.ServerError("UserSignIn", err) - } + ctx.ServerError("UserSignIn", err) return } - // User needs to use 2FA, save data and redirect to 2FA page. + // Check if the user has u2f registration + hasU2Ftwofa, err := login.HasU2FRegistrationsByUID(u.ID) + if err != nil { + ctx.ServerError("UserSignIn", err) + return + } + + if !hasTOTPtwofa && !hasU2Ftwofa { + // No two factor auth configured we can sign in the user + handleSignIn(ctx, u, form.Remember) + return + } + + // User will need to use 2FA TOTP or U2F, save data if err := ctx.Session.Set("twofaUid", u.ID); err != nil { ctx.ServerError("UserSignIn: Unable to set twofaUid in session", err) return } + if err := ctx.Session.Set("twofaRemember", form.Remember); err != nil { ctx.ServerError("UserSignIn: Unable to set twofaRemember in session", err) return } + + if hasTOTPtwofa { + // User will need to use U2F, save data + if err := ctx.Session.Set("totpEnrolled", u.ID); err != nil { + ctx.ServerError("UserSignIn: Unable to set u2fEnrolled in session", err) + return + } + } + if err := ctx.Session.Release(); err != nil { ctx.ServerError("UserSignIn: Unable to save session", err) return } - regs, err := login.GetU2FRegistrationsByUID(u.ID) - if err == nil && len(regs) > 0 { + // If we have U2F redirect there first + if hasU2Ftwofa { ctx.Redirect(setting.AppSubURL + "/user/u2f") return } + // Fallback to 2FA ctx.Redirect(setting.AppSubURL + "/user/two_factor") } @@ -406,6 +426,11 @@ func U2F(ctx *context.Context) { return } + // See whether TOTP is also available. + if ctx.Session.Get("totpEnrolled") != nil { + ctx.Data["TOTPEnrolled"] = true + } + ctx.HTML(http.StatusOK, tplU2F) } diff --git a/routers/web/user/setting/security.go b/routers/web/user/setting/security.go index 53f672282d..65e9790d47 100644 --- a/routers/web/user/setting/security.go +++ b/routers/web/user/setting/security.go @@ -55,23 +55,17 @@ func DeleteAccountLink(ctx *context.Context) { } func loadSecurityData(ctx *context.Context) { - enrolled := true - _, err := login.GetTwoFactorByUID(ctx.User.ID) + enrolled, err := login.HasTwoFactorByUID(ctx.User.ID) if err != nil { - if login.IsErrTwoFactorNotEnrolled(err) { - enrolled = false - } else { - ctx.ServerError("SettingsTwoFactor", err) - return - } + ctx.ServerError("SettingsTwoFactor", err) + return } - ctx.Data["TwofaEnrolled"] = enrolled - if enrolled { - ctx.Data["U2FRegistrations"], err = login.GetU2FRegistrationsByUID(ctx.User.ID) - if err != nil { - ctx.ServerError("GetU2FRegistrationsByUID", err) - return - } + ctx.Data["TOTPEnrolled"] = enrolled + + ctx.Data["U2FRegistrations"], err = login.GetU2FRegistrationsByUID(ctx.User.ID) + if err != nil { + ctx.ServerError("GetU2FRegistrationsByUID", err) + return } tokens, err := models.ListAccessTokens(models.ListAccessTokensOptions{UserID: ctx.User.ID}) diff --git a/templates/user/auth/u2f.tmpl b/templates/user/auth/u2f.tmpl index 2013d14937..8b04866bbc 100644 --- a/templates/user/auth/u2f.tmpl +++ b/templates/user/auth/u2f.tmpl @@ -12,9 +12,11 @@

{{.i18n.Tr "u2f_sign_in"}}

{{.i18n.Tr "u2f_press_button"}}
-
- {{.i18n.Tr "u2f_use_twofa"}} -
+ {{if .TOTPEnrolled}} +
+ {{.i18n.Tr "u2f_use_twofa"}} +
+ {{end}} diff --git a/templates/user/settings/security_twofa.tmpl b/templates/user/settings/security_twofa.tmpl index f48b2f4cb2..3d6804d9c6 100644 --- a/templates/user/settings/security_twofa.tmpl +++ b/templates/user/settings/security_twofa.tmpl @@ -3,7 +3,7 @@

{{.i18n.Tr "settings.twofa_desc"}}

- {{if .TwofaEnrolled}} + {{if .TOTPEnrolled}}

{{$.i18n.Tr "settings.twofa_is_enrolled" | Str2html }}

{{.CsrfTokenHtml}} diff --git a/templates/user/settings/security_u2f.tmpl b/templates/user/settings/security_u2f.tmpl index 8fe01d8c70..a2ff6c2212 100644 --- a/templates/user/settings/security_u2f.tmpl +++ b/templates/user/settings/security_u2f.tmpl @@ -3,32 +3,28 @@

{{.i18n.Tr "settings.u2f_desc" | Str2html}}

- {{if .TwofaEnrolled}} -
- {{range .U2FRegistrations}} -
-
- -
-
- {{.Name}} -
+
+ {{range .U2FRegistrations}} +
+
+ +
+
+ {{.Name}}
- {{end}} -
-
- {{.CsrfTokenHtml}} -
- -
- + {{end}} +
+
+ {{.CsrfTokenHtml}} +
+ +
- {{else}} - {{.i18n.Tr "settings.u2f_require_twofa"}} - {{end}} + +