Skip to content

Commit 021df29

Browse files
kdomanskikdomanski
authored
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
1 parent a3f9e92 commit 021df29

File tree

12 files changed

+100
-57
lines changed

12 files changed

+100
-57
lines changed

‎models/fixtures/u2f_registration.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
-
22
id: 1
33
name: "U2F Key"
4-
user_id: 1
4+
user_id: 32
55
counter: 0
66
created_unix: 946684800
77
updated_unix: 946684800

‎models/fixtures/user.yml

+16
Original file line numberDiff line numberDiff line change
@@ -542,3 +542,19 @@
542542
avatar_email: [email protected]
543543
num_repos: 0
544544
is_active: true
545+
546+
-
547+
id: 32
548+
lower_name: user32
549+
name: user32
550+
full_name: User 32 (U2F test)
551+
email: [email protected]
552+
passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password
553+
type: 0 # individual
554+
salt: ZogKvWdyEx
555+
is_admin: false
556+
is_restricted: false
557+
avatar: avatar32
558+
avatar_email: [email protected]
559+
num_repos: 0
560+
is_active: true

‎models/login/twofactor.go

+6
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,12 @@ func GetTwoFactorByUID(uid int64) (*TwoFactor, error) {
136136
return twofa, nil
137137
}
138138

139+
// HasTwoFactorByUID returns the two-factor authentication token associated with
140+
// the user, if any.
141+
func HasTwoFactorByUID(uid int64) (bool, error) {
142+
return db.GetEngine(db.DefaultContext).Where("uid=?", uid).Exist(&TwoFactor{})
143+
}
144+
139145
// DeleteTwoFactorByID deletes two-factor authentication token by given ID.
140146
func DeleteTwoFactorByID(id, userID int64) error {
141147
cnt, err := db.GetEngine(db.DefaultContext).ID(id).Delete(&TwoFactor{

‎models/login/u2f.go

+5
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,11 @@ func GetU2FRegistrationsByUID(uid int64) (U2FRegistrationList, error) {
115115
return getU2FRegistrationsByUID(db.GetEngine(db.DefaultContext), uid)
116116
}
117117

118+
// HasU2FRegistrationsByUID returns whether a given user has U2F registrations
119+
func HasU2FRegistrationsByUID(uid int64) (bool, error) {
120+
return db.GetEngine(db.DefaultContext).Where("user_id = ?", uid).Exist(&U2FRegistration{})
121+
}
122+
118123
func createRegistration(e db.Engine, userID int64, name string, reg *u2f.Registration) (*U2FRegistration, error) {
119124
raw, err := reg.MarshalBinary()
120125
if err != nil {

‎models/login/u2f_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ func TestGetU2FRegistrationByID(t *testing.T) {
2929
func TestGetU2FRegistrationsByUID(t *testing.T) {
3030
assert.NoError(t, db.PrepareTestDatabase())
3131

32-
res, err := GetU2FRegistrationsByUID(1)
32+
res, err := GetU2FRegistrationsByUID(32)
3333

3434
assert.NoError(t, err)
3535
assert.Len(t, res, 1)

‎models/user_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -147,13 +147,13 @@ func TestSearchUsers(t *testing.T) {
147147
}
148148

149149
testUserSuccess(&SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}},
150-
[]int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30})
150+
[]int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32})
151151

152152
testUserSuccess(&SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsActive: util.OptionalBoolFalse},
153153
[]int64{9})
154154

155155
testUserSuccess(&SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: util.OptionalBoolTrue},
156-
[]int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 28, 29, 30})
156+
[]int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 28, 29, 30, 32})
157157

158158
testUserSuccess(&SearchUserOptions{Keyword: "user1", OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: util.OptionalBoolTrue},
159159
[]int64{1, 10, 11, 12, 13, 14, 15, 16, 18})

‎options/locale/locale_en-US.ini

-1
Original file line numberDiff line numberDiff line change
@@ -714,7 +714,6 @@ twofa_enrolled = Your account has been enrolled into two-factor authentication.
714714
twofa_failed_get_secret = Failed to get secret.
715715

716716
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.
717-
u2f_require_twofa = Your account must be enrolled in two-factor authentication to use security keys.
718717
u2f_register_key = Add Security Key
719718
u2f_nickname = Nickname
720719
u2f_press_button = Press the button on your security key to register it.

‎routers/web/user/auth.go

+35-10
Original file line numberDiff line numberDiff line change
@@ -211,38 +211,58 @@ func SignInPost(ctx *context.Context) {
211211
return
212212
}
213213

214-
// If this user is enrolled in 2FA, we can't sign the user in just yet.
214+
// If this user is enrolled in 2FA TOTP, we can't sign the user in just yet.
215215
// Instead, redirect them to the 2FA authentication page.
216-
_, err = login.GetTwoFactorByUID(u.ID)
216+
hasTOTPtwofa, err := login.HasTwoFactorByUID(u.ID)
217217
if err != nil {
218-
if login.IsErrTwoFactorNotEnrolled(err) {
219-
handleSignIn(ctx, u, form.Remember)
220-
} else {
221-
ctx.ServerError("UserSignIn", err)
222-
}
218+
ctx.ServerError("UserSignIn", err)
223219
return
224220
}
225221

226-
// User needs to use 2FA, save data and redirect to 2FA page.
222+
// Check if the user has u2f registration
223+
hasU2Ftwofa, err := login.HasU2FRegistrationsByUID(u.ID)
224+
if err != nil {
225+
ctx.ServerError("UserSignIn", err)
226+
return
227+
}
228+
229+
if !hasTOTPtwofa && !hasU2Ftwofa {
230+
// No two factor auth configured we can sign in the user
231+
handleSignIn(ctx, u, form.Remember)
232+
return
233+
}
234+
235+
// User will need to use 2FA TOTP or U2F, save data
227236
if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
228237
ctx.ServerError("UserSignIn: Unable to set twofaUid in session", err)
229238
return
230239
}
240+
231241
if err := ctx.Session.Set("twofaRemember", form.Remember); err != nil {
232242
ctx.ServerError("UserSignIn: Unable to set twofaRemember in session", err)
233243
return
234244
}
245+
246+
if hasTOTPtwofa {
247+
// User will need to use U2F, save data
248+
if err := ctx.Session.Set("totpEnrolled", u.ID); err != nil {
249+
ctx.ServerError("UserSignIn: Unable to set u2fEnrolled in session", err)
250+
return
251+
}
252+
}
253+
235254
if err := ctx.Session.Release(); err != nil {
236255
ctx.ServerError("UserSignIn: Unable to save session", err)
237256
return
238257
}
239258

240-
regs, err := login.GetU2FRegistrationsByUID(u.ID)
241-
if err == nil && len(regs) > 0 {
259+
// If we have U2F redirect there first
260+
if hasU2Ftwofa {
242261
ctx.Redirect(setting.AppSubURL + "/user/u2f")
243262
return
244263
}
245264

265+
// Fallback to 2FA
246266
ctx.Redirect(setting.AppSubURL + "/user/two_factor")
247267
}
248268

@@ -406,6 +426,11 @@ func U2F(ctx *context.Context) {
406426
return
407427
}
408428

429+
// See whether TOTP is also available.
430+
if ctx.Session.Get("totpEnrolled") != nil {
431+
ctx.Data["TOTPEnrolled"] = true
432+
}
433+
409434
ctx.HTML(http.StatusOK, tplU2F)
410435
}
411436

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

+9-15
Original file line numberDiff line numberDiff line change
@@ -55,23 +55,17 @@ func DeleteAccountLink(ctx *context.Context) {
5555
}
5656

5757
func loadSecurityData(ctx *context.Context) {
58-
enrolled := true
59-
_, err := login.GetTwoFactorByUID(ctx.User.ID)
58+
enrolled, err := login.HasTwoFactorByUID(ctx.User.ID)
6059
if err != nil {
61-
if login.IsErrTwoFactorNotEnrolled(err) {
62-
enrolled = false
63-
} else {
64-
ctx.ServerError("SettingsTwoFactor", err)
65-
return
66-
}
60+
ctx.ServerError("SettingsTwoFactor", err)
61+
return
6762
}
68-
ctx.Data["TwofaEnrolled"] = enrolled
69-
if enrolled {
70-
ctx.Data["U2FRegistrations"], err = login.GetU2FRegistrationsByUID(ctx.User.ID)
71-
if err != nil {
72-
ctx.ServerError("GetU2FRegistrationsByUID", err)
73-
return
74-
}
63+
ctx.Data["TOTPEnrolled"] = enrolled
64+
65+
ctx.Data["U2FRegistrations"], err = login.GetU2FRegistrationsByUID(ctx.User.ID)
66+
if err != nil {
67+
ctx.ServerError("GetU2FRegistrationsByUID", err)
68+
return
7569
}
7670

7771
tokens, err := models.ListAccessTokens(models.ListAccessTokensOptions{UserID: ctx.User.ID})

‎templates/user/auth/u2f.tmpl

+5-3
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,11 @@
1212
<p>{{.i18n.Tr "u2f_sign_in"}}</p>
1313
</div>
1414
<div id="wait-for-key" class="ui attached segment"><div class="ui active indeterminate inline loader"></div> {{.i18n.Tr "u2f_press_button"}} </div>
15-
<div class="ui attached segment">
16-
<a href="{{AppSubUrl}}/user/two_factor">{{.i18n.Tr "u2f_use_twofa"}}</a>
17-
</div>
15+
{{if .TOTPEnrolled}}
16+
<div class="ui attached segment">
17+
<a href="{{AppSubUrl}}/user/two_factor">{{.i18n.Tr "u2f_use_twofa"}}</a>
18+
</div>
19+
{{end}}
1820
</div>
1921
</div>
2022
</div>

‎templates/user/settings/security_twofa.tmpl

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
</h4>
44
<div class="ui attached segment">
55
<p>{{.i18n.Tr "settings.twofa_desc"}}</p>
6-
{{if .TwofaEnrolled}}
6+
{{if .TOTPEnrolled}}
77
<p>{{$.i18n.Tr "settings.twofa_is_enrolled" | Str2html }}</p>
88
<form class="ui form" action="{{AppSubUrl}}/user/settings/security/two_factor/regenerate_scratch" method="post" enctype="multipart/form-data">
99
{{.CsrfTokenHtml}}

‎templates/user/settings/security_u2f.tmpl

+19-23
Original file line numberDiff line numberDiff line change
@@ -3,32 +3,28 @@
33
</h4>
44
<div class="ui attached segment">
55
<p>{{.i18n.Tr "settings.u2f_desc" | Str2html}}</p>
6-
{{if .TwofaEnrolled}}
7-
<div class="ui key list">
8-
{{range .U2FRegistrations}}
9-
<div class="item">
10-
<div class="right floated content">
11-
<button class="ui red tiny button delete-button" data-modal-id="delete-registration" data-url="{{$.Link}}/u2f/delete" data-id="{{.ID}}">
12-
{{$.i18n.Tr "settings.delete_key"}}
13-
</button>
14-
</div>
15-
<div class="content">
16-
<strong>{{.Name}}</strong>
17-
</div>
6+
<div class="ui key list">
7+
{{range .U2FRegistrations}}
8+
<div class="item">
9+
<div class="right floated content">
10+
<button class="ui red tiny button delete-button" id="delete-registration" data-url="{{$.Link}}/u2f/delete" data-id="{{.ID}}">
11+
{{$.i18n.Tr "settings.delete_key"}}
12+
</button>
13+
</div>
14+
<div class="content">
15+
<strong>{{.Name}}</strong>
1816
</div>
19-
{{end}}
20-
</div>
21-
<div class="ui form">
22-
{{.CsrfTokenHtml}}
23-
<div class="required field">
24-
<label for="nickname">{{.i18n.Tr "settings.u2f_nickname"}}</label>
25-
<input id="nickname" name="nickname" type="text" required>
2617
</div>
27-
<button id="register-security-key" class="ui green button">{{svg "octicon-key"}} {{.i18n.Tr "settings.u2f_register_key"}}</button>
18+
{{end}}
19+
</div>
20+
<div class="ui form">
21+
{{.CsrfTokenHtml}}
22+
<div class="required field">
23+
<label for="nickname">{{.i18n.Tr "settings.u2f_nickname"}}</label>
24+
<input id="nickname" name="nickname" type="text" required>
2825
</div>
29-
{{else}}
30-
<b>{{.i18n.Tr "settings.u2f_require_twofa"}}</b>
31-
{{end}}
26+
<button id="register-security-key" class="ui green button">{{svg "octicon-key"}} {{.i18n.Tr "settings.u2f_register_key"}}</button>
27+
</div>
3228
</div>
3329

3430
<div class="ui small modal" id="register-device">

0 commit comments

Comments
 (0)