Allow U2F 2FA without TOTP (#11573)

This change enables the usage of U2F without being forced to enroll an TOTP authenticator.
The `/user/auth/u2f` has been changed to hide the "use TOTP instead" bar if TOTP is not enrolled.

Fixes #5410
Fixes #17495

diff --git a/models/fixtures/u2f_registration.yml b/models/fixtures/u2f_registration.yml
index 4a9d1d9624a3b40623eabe1849b911f71f471a1c..60555c43f1f9f13265e1113e552a248040103c89 100644 (file)
--- 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 c49fe1b656a22e1aef67be3bd567f795ac102aca..cf07542eed16cc1ed9f13e8ce36dc225db2f1297 100644 (file)
--- 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 1c4d2734fca02e0014e2d5f11d703ec7c063c285..acb5e1b2d50ec80334d5153753807003ac53c348 100644 (file)
--- 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 05d39cc05ec1d99216717bb406f4041acb64bed2..8cea98463f115f572b2149b881d418b97c94f093 100644 (file)
--- 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 32505b62a67dcfb74220f693191f70cd46ab6cac..8f5cea61508a5cf4a7254f1c5506c69e6d763380 100644 (file)
--- 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 2dcca20346b6188c36f07221026b63ca99dae95d..3f3536dafaa90045f318a8c19a3e900a553d08d4 100644 (file)
--- 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 4a9e3c3894d7ed5b76d4aeaf660e500b3620f484..2632531e2fb5441b191bc447b302e11178919185 100644 (file)
--- 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 <a rel="noreferrer" href="https://fidoalliance.org/">FIDO U2F</a> 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 21d48e98341a8dfe1a1b1e67d86d81808a1a9f8f..55f304a7cb6258b2f0e8a94e2806e2e41bf6b668 100644 (file)
--- 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 53f672282d1a2f140b3d30de10d6747358d7531c..65e9790d47725b1cbc872de738b481b4662480fb 100644 (file)
--- 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 2013d149377d92a6ddc5bb747527c0d48f6c2e2a..8b04866bbcae7ad1ea5a080f4b7d235aaf9525ce 100644 (file)
--- a/templates/user/auth/u2f.tmpl
+++ b/templates/user/auth/u2f.tmpl
@@ -12,9 +12,11 @@
                                <p>{{.i18n.Tr "u2f_sign_in"}}</p>
                        </div>
                        <div id="wait-for-key" class="ui attached segment"><div class="ui active indeterminate inline loader"></div> {{.i18n.Tr "u2f_press_button"}} </div>
-                       <div class="ui attached segment">
-                               <a href="{{AppSubUrl}}/user/two_factor">{{.i18n.Tr "u2f_use_twofa"}}</a>
-                       </div>
+                       {{if .TOTPEnrolled}}
+                               <div class="ui attached segment">
+                                       <a href="{{AppSubUrl}}/user/two_factor">{{.i18n.Tr "u2f_use_twofa"}}</a>
+                               </div>
+                       {{end}}
                </div>
        </div>
 </div>

diff --git a/templates/user/settings/security_twofa.tmpl b/templates/user/settings/security_twofa.tmpl
index f48b2f4cb22700058f2f2268151efab2689e4014..3d6804d9c67da63cf9197011350f7e3d5181777b 100644 (file)
--- a/templates/user/settings/security_twofa.tmpl
+++ b/templates/user/settings/security_twofa.tmpl
@@ -3,7 +3,7 @@
 </h4>
 <div class="ui attached segment">
        <p>{{.i18n.Tr "settings.twofa_desc"}}</p>
-       {{if .TwofaEnrolled}}
+       {{if .TOTPEnrolled}}
        <p>{{$.i18n.Tr "settings.twofa_is_enrolled" | Str2html }}</p>
        <form class="ui form" action="{{AppSubUrl}}/user/settings/security/two_factor/regenerate_scratch" method="post" enctype="multipart/form-data">
                {{.CsrfTokenHtml}}

diff --git a/templates/user/settings/security_u2f.tmpl b/templates/user/settings/security_u2f.tmpl
index 8fe01d8c70156e9b1093648850851c01b239241f..a2ff6c2212dd8f264f831541b90d66ad06eee33f 100644 (file)
--- a/templates/user/settings/security_u2f.tmpl
+++ b/templates/user/settings/security_u2f.tmpl
@@ -3,32 +3,28 @@
 </h4>
 <div class="ui attached segment">
        <p>{{.i18n.Tr "settings.u2f_desc" | Str2html}}</p>
-       {{if .TwofaEnrolled}}
-               <div class="ui key list">
-                       {{range .U2FRegistrations}}
-                               <div class="item">
-                                       <div class="right floated content">
-                                               <button class="ui red tiny button delete-button" data-modal-id="delete-registration" data-url="{{$.Link}}/u2f/delete" data-id="{{.ID}}">
-                                               {{$.i18n.Tr "settings.delete_key"}}
-                                               </button>
-                                       </div>
-                                       <div class="content">
-                                               <strong>{{.Name}}</strong>
-                                       </div>
+       <div class="ui key list">
+               {{range .U2FRegistrations}}
+                       <div class="item">
+                               <div class="right floated content">
+                                       <button class="ui red tiny button delete-button" id="delete-registration" data-url="{{$.Link}}/u2f/delete" data-id="{{.ID}}">
+                                       {{$.i18n.Tr "settings.delete_key"}}
+                                       </button>
+                               </div>
+                               <div class="content">
+                                       <strong>{{.Name}}</strong>
                                </div>
-                       {{end}}
-               </div>
-               <div class="ui form">
-                       {{.CsrfTokenHtml}}
-                       <div class="required field">
-                               <label for="nickname">{{.i18n.Tr "settings.u2f_nickname"}}</label>
-                               <input id="nickname" name="nickname" type="text" required>
                        </div>
-                       <button id="register-security-key" class="ui green button">{{svg "octicon-key"}} {{.i18n.Tr "settings.u2f_register_key"}}</button>
+               {{end}}
+       </div>
+       <div class="ui form">
+               {{.CsrfTokenHtml}}
+               <div class="required field">
+                       <label for="nickname">{{.i18n.Tr "settings.u2f_nickname"}}</label>
+                       <input id="nickname" name="nickname" type="text" required>
                </div>
-       {{else}}
-               <b>{{.i18n.Tr "settings.u2f_require_twofa"}}</b>
-       {{end}}
+               <button id="register-security-key" class="ui green button">{{svg "octicon-key"}} {{.i18n.Tr "settings.u2f_register_key"}}</button>
+       </div>
 </div>
 
 <div class="ui small modal" id="register-device">