Skip to content

Commit d7c2a29

Browse files
zeripath6543wxiaoguang
authored
Webauthn nits (#18284)
This contains some additional fixes and small nits related to #17957 Signed-off-by: Andrew Thornton <[email protected]> Co-authored-by: 6543 <[email protected]> Co-authored-by: wxiaoguang <[email protected]>
1 parent e239d35 commit d7c2a29

File tree

10 files changed

+87
-40
lines changed

10 files changed

+87
-40
lines changed

‎models/auth/webauthn.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ package auth
66

77
import (
88
"context"
9-
"encoding/base64"
9+
"encoding/base32"
1010
"fmt"
1111
"strings"
1212

@@ -94,7 +94,7 @@ type WebAuthnCredentialList []*WebAuthnCredential
9494
func (list WebAuthnCredentialList) ToCredentials() []webauthn.Credential {
9595
creds := make([]webauthn.Credential, 0, len(list))
9696
for _, cred := range list {
97-
credID, _ := base64.RawStdEncoding.DecodeString(cred.CredentialID)
97+
credID, _ := base32.HexEncoding.DecodeString(cred.CredentialID)
9898
creds = append(creds, webauthn.Credential{
9999
ID: credID,
100100
PublicKey: cred.PublicKey,
@@ -164,13 +164,13 @@ func HasWebAuthnRegistrationsByUID(uid int64) (bool, error) {
164164
}
165165

166166
// GetWebAuthnCredentialByCredID returns WebAuthn credential by credential ID
167-
func GetWebAuthnCredentialByCredID(credID string) (*WebAuthnCredential, error) {
168-
return getWebAuthnCredentialByCredID(db.DefaultContext, credID)
167+
func GetWebAuthnCredentialByCredID(userID int64, credID string) (*WebAuthnCredential, error) {
168+
return getWebAuthnCredentialByCredID(db.DefaultContext, userID, credID)
169169
}
170170

171-
func getWebAuthnCredentialByCredID(ctx context.Context, credID string) (*WebAuthnCredential, error) {
171+
func getWebAuthnCredentialByCredID(ctx context.Context, userID int64, credID string) (*WebAuthnCredential, error) {
172172
cred := new(WebAuthnCredential)
173-
if found, err := db.GetEngine(ctx).Where("credential_id = ?", credID).Get(cred); err != nil {
173+
if found, err := db.GetEngine(ctx).Where("user_id = ? AND credential_id = ?", userID, credID).Get(cred); err != nil {
174174
return nil, err
175175
} else if !found {
176176
return nil, ErrWebAuthnCredentialNotExist{CredentialID: credID}
@@ -187,7 +187,7 @@ func createCredential(ctx context.Context, userID int64, name string, cred *weba
187187
c := &WebAuthnCredential{
188188
UserID: userID,
189189
Name: name,
190-
CredentialID: base64.RawStdEncoding.EncodeToString(cred.ID),
190+
CredentialID: base32.HexEncoding.EncodeToString(cred.ID),
191191
PublicKey: cred.PublicKey,
192192
AttestationType: cred.AttestationType,
193193
AAGUID: cred.Authenticator.AAGUID,

‎models/auth/webauthn_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
package auth
66

77
import (
8-
"encoding/base64"
8+
"encoding/base32"
99
"testing"
1010

1111
"code.gitea.io/gitea/models/unittest"
@@ -61,7 +61,7 @@ func TestCreateCredential(t *testing.T) {
6161
res, err := CreateCredential(1, "WebAuthn Created Credential", &webauthn.Credential{ID: []byte("Test")})
6262
assert.NoError(t, err)
6363
assert.Equal(t, "WebAuthn Created Credential", res.Name)
64-
bs, err := base64.RawStdEncoding.DecodeString(res.CredentialID)
64+
bs, err := base32.HexEncoding.DecodeString(res.CredentialID)
6565
assert.NoError(t, err)
6666
assert.Equal(t, []byte("Test"), bs)
6767

‎models/migrations/migrations.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,8 @@ var migrations = []Migration{
368368
NewMigration("Add authorize column to team_unit table", addAuthorizeColForTeamUnit),
369369
// v207 -> v208
370370
NewMigration("Add webauthn table and migrate u2f data to webauthn", addWebAuthnCred),
371+
// v208 -> v209
372+
NewMigration("Use base32.HexEncoding instead of base64 encoding for cred ID as it is case insensitive", useBase32HexForCredIDInWebAuthnCredential),
371373
}
372374

373375
// GetCurrentDBVersion returns the current db version

‎models/migrations/v208.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Copyright 2021 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package migrations
6+
7+
import (
8+
"encoding/base32"
9+
"encoding/base64"
10+
11+
"xorm.io/xorm"
12+
)
13+
14+
func useBase32HexForCredIDInWebAuthnCredential(x *xorm.Engine) error {
15+
16+
// Create webauthnCredential table
17+
type webauthnCredential struct {
18+
ID int64 `xorm:"pk autoincr"`
19+
CredentialID string `xorm:"INDEX"`
20+
}
21+
if err := x.Sync2(&webauthnCredential{}); err != nil {
22+
return err
23+
}
24+
25+
var start int
26+
regs := make([]*webauthnCredential, 0, 50)
27+
for {
28+
err := x.OrderBy("id").Limit(50, start).Find(&regs)
29+
if err != nil {
30+
return err
31+
}
32+
33+
for _, reg := range regs {
34+
credID, _ := base64.RawStdEncoding.DecodeString(reg.CredentialID)
35+
reg.CredentialID = base32.HexEncoding.EncodeToString(credID)
36+
37+
_, err := x.Update(reg)
38+
if err != nil {
39+
return err
40+
}
41+
}
42+
43+
if len(regs) < 50 {
44+
break
45+
}
46+
start += 50
47+
regs = regs[:0]
48+
}
49+
50+
return nil
51+
}

‎options/locale/locale_en-US.ini

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -748,10 +748,9 @@ passcode_invalid = The passcode is incorrect. Try again.
748748
twofa_enrolled = Your account has been enrolled into two-factor authentication. Store your scratch token (%s) in a safe place as it is only shown once!
749749
twofa_failed_get_secret = Failed to get secret.
750750

751-
webauthn_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://w3c.github.io/webauthn/#webauthn-authenticator">WebAuthn Authenticator</a> standard.
751+
webauthn_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" target="_blank" href="https://w3c.github.io/webauthn/#webauthn-authenticator">WebAuthn Authenticator</a> standard.
752752
webauthn_register_key = Add Security Key
753753
webauthn_nickname = Nickname
754-
webauthn_press_button = Press the button on your security key to register it.
755754
webauthn_delete_key = Remove Security Key
756755
webauthn_delete_key_desc = If you remove a security key you can no longer sign in with it. Continue?
757756

‎routers/web/auth/webauthn.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
package auth
66

77
import (
8-
"encoding/base64"
8+
"encoding/base32"
99
"errors"
1010
"net/http"
1111

@@ -131,7 +131,7 @@ func WebAuthnLoginAssertionPost(ctx *context.Context) {
131131
}
132132

133133
// Success! Get the credential and update the sign count with the new value we received.
134-
dbCred, err := auth.GetWebAuthnCredentialByCredID(base64.RawStdEncoding.EncodeToString(cred.ID))
134+
dbCred, err := auth.GetWebAuthnCredentialByCredID(user.ID, base32.HexEncoding.EncodeToString(cred.ID))
135135
if err != nil {
136136
ctx.ServerError("GetWebAuthnCredentialByCredID", err)
137137
return

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

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@ func WebAuthnRegister(ctx *context.Context) {
3838
return
3939
}
4040

41-
_ = ctx.Session.Delete("registration")
42-
if err := ctx.Session.Set("WebauthnName", form.Name); err != nil {
43-
ctx.ServerError("Unable to set session key for WebauthnName", err)
41+
_ = ctx.Session.Delete("webauthnRegistration")
42+
if err := ctx.Session.Set("webauthnName", form.Name); err != nil {
43+
ctx.ServerError("Unable to set session key for webauthnName", err)
4444
return
4545
}
4646

@@ -51,7 +51,7 @@ func WebAuthnRegister(ctx *context.Context) {
5151
}
5252

5353
// Save the session data as marshaled JSON
54-
if err = ctx.Session.Set("registration", sessionData); err != nil {
54+
if err = ctx.Session.Set("webauthnRegistration", sessionData); err != nil {
5555
ctx.ServerError("Unable to set session", err)
5656
return
5757
}
@@ -61,20 +61,20 @@ func WebAuthnRegister(ctx *context.Context) {
6161

6262
// WebauthnRegisterPost receives the response of the security key
6363
func WebauthnRegisterPost(ctx *context.Context) {
64-
name, ok := ctx.Session.Get("WebauthnName").(string)
64+
name, ok := ctx.Session.Get("webauthnName").(string)
6565
if !ok || name == "" {
66-
ctx.ServerError("Get WebauthnName", errors.New("no WebauthnName"))
66+
ctx.ServerError("Get webauthnName", errors.New("no webauthnName"))
6767
return
6868
}
6969

7070
// Load the session data
71-
sessionData, ok := ctx.Session.Get("registration").(*webauthn.SessionData)
71+
sessionData, ok := ctx.Session.Get("webauthnRegistration").(*webauthn.SessionData)
7272
if !ok || sessionData == nil {
7373
ctx.ServerError("Get registration", errors.New("no registration"))
7474
return
7575
}
7676
defer func() {
77-
_ = ctx.Session.Delete("registration")
77+
_ = ctx.Session.Delete("webauthnRegistration")
7878
}()
7979

8080
// Verify that the challenge succeeded
@@ -103,6 +103,8 @@ func WebauthnRegisterPost(ctx *context.Context) {
103103
ctx.ServerError("CreateCredential", err)
104104
return
105105
}
106+
_ = ctx.Session.Delete("webauthnName")
107+
106108
ctx.JSON(http.StatusCreated, cred)
107109
}
108110

‎templates/user/auth/webauthn_error.tmpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
<div class="hide" data-webauthn-error-msg="duplicated"><p>{{.i18n.Tr "webauthn_error_duplicated"}}</div>
1313
<div class="hide" data-webauthn-error-msg="empty"><p>{{.i18n.Tr "webauthn_error_empty"}}</div>
1414
<div class="hide" data-webauthn-error-msg="timeout"><p>{{.i18n.Tr "webauthn_error_timeout"}}</div>
15-
<div class="hide" data-webauthn-error-msg="0"></div>
15+
<div class="hide" data-webauthn-error-msg="general"></div>
1616
</div>
1717
</div>
1818
<div class="actions">

‎templates/user/settings/security/webauthn.tmpl

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,6 @@
2828
</div>
2929
</div>
3030

31-
<div class="ui small modal" id="register-device">
32-
<div class="header">{{.i18n.Tr "settings.webauthn_register_key"}}</div>
33-
<div class="content">
34-
<i class="notched spinner loading icon"></i> {{.i18n.Tr "settings.webauthn_press_button"}}
35-
</div>
36-
<div class="actions">
37-
<div class="ui cancel button">{{.i18n.Tr "cancel"}}</div>
38-
</div>
39-
</div>
40-
4131
{{template "user/auth/webauthn_error" .}}
4232

4333
<div class="ui small basic delete modal" id="delete-registration">

‎web_src/js/features/user-auth-webauthn.js

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export function initUserAuthWebAuthn() {
2424
.then((credential) => {
2525
verifyAssertion(credential);
2626
}).catch((err) => {
27-
webAuthnError(0, err.message);
27+
webAuthnError('general', err.message);
2828
});
2929
}).fail(() => {
3030
webAuthnError('unknown');
@@ -113,11 +113,16 @@ function webauthnRegistered(newCredential) {
113113

114114
function webAuthnError(errorType, message) {
115115
$('#webauthn-error [data-webauthn-error-msg]').hide();
116-
if (errorType === 0 && message && message.length > 1) {
117-
$(`#webauthn-error [data-webauthn-error-msg=0]`).text(message);
118-
$(`#webauthn-error [data-webauthn-error-msg=0]`).show();
116+
const $errorGeneral = $(`#webauthn-error [data-webauthn-error-msg=general]`);
117+
if (errorType === 'general') {
118+
$errorGeneral.show().text(message || 'unknown error');
119119
} else {
120-
$(`#webauthn-error [data-webauthn-error-msg=${errorType}]`).show();
120+
const $errorTyped = $(`#webauthn-error [data-webauthn-error-msg=${errorType}]`);
121+
if ($errorTyped.length) {
122+
$errorTyped.show();
123+
} else {
124+
$errorGeneral.show().text(`unknown error type: ${errorType}`);
125+
}
121126
}
122127
$('#webauthn-error').modal('show');
123128
}
@@ -149,7 +154,6 @@ export function initUserAuthWebAuthnRegister() {
149154
return;
150155
}
151156

152-
$('#register-device').modal({allowMultiple: false});
153157
$('#webauthn-error').modal({allowMultiple: false});
154158
$('#register-webauthn').on('click', (e) => {
155159
e.preventDefault();
@@ -167,7 +171,6 @@ function webAuthnRegisterRequest() {
167171
name: $('#nickname').val(),
168172
}).done((makeCredentialOptions) => {
169173
$('#nickname').closest('div.field').removeClass('error');
170-
$('#register-device').modal('show');
171174

172175
makeCredentialOptions.publicKey.challenge = decode(makeCredentialOptions.publicKey.challenge);
173176
makeCredentialOptions.publicKey.user.id = decode(makeCredentialOptions.publicKey.user.id);
@@ -185,7 +188,7 @@ function webAuthnRegisterRequest() {
185188
webAuthnError('unknown');
186189
return;
187190
}
188-
webAuthnError(0, err);
191+
webAuthnError('general', err.message);
189192
});
190193
}).fail((xhr) => {
191194
if (xhr.status === 409) {

0 commit comments

Comments
 (0)