Webauthn nits (#18284)

This contains some additional fixes and small nits related to #17957 

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

models/auth/webauthn.go

@@ -6,7 +6,7 @@ package auth
 
 import (
 	"context"
-	"encoding/base64"
+	"encoding/base32"
 	"fmt"
 	"strings"
 
@@ -94,7 +94,7 @@ type WebAuthnCredentialList []*WebAuthnCredential
 func (list WebAuthnCredentialList) ToCredentials() []webauthn.Credential {
 	creds := make([]webauthn.Credential, 0, len(list))
 	for _, cred := range list {
-		credID, _ := base64.RawStdEncoding.DecodeString(cred.CredentialID)
+		credID, _ := base32.HexEncoding.DecodeString(cred.CredentialID)
 		creds = append(creds, webauthn.Credential{
 			ID:              credID,
 			PublicKey:       cred.PublicKey,
@@ -164,13 +164,13 @@ func HasWebAuthnRegistrationsByUID(uid int64) (bool, error) {
 }
 
 // GetWebAuthnCredentialByCredID returns WebAuthn credential by credential ID
-func GetWebAuthnCredentialByCredID(credID string) (*WebAuthnCredential, error) {
+func GetWebAuthnCredentialByCredID(userID int64, credID string) (*WebAuthnCredential, error) {
-	return getWebAuthnCredentialByCredID(db.DefaultContext, credID)
+	return getWebAuthnCredentialByCredID(db.DefaultContext, userID, credID)
 }
 
-func getWebAuthnCredentialByCredID(ctx context.Context, credID string) (*WebAuthnCredential, error) {
+func getWebAuthnCredentialByCredID(ctx context.Context, userID int64, credID string) (*WebAuthnCredential, error) {
 	cred := new(WebAuthnCredential)
-	if found, err := db.GetEngine(ctx).Where("credential_id = ?", credID).Get(cred); err != nil {
+	if found, err := db.GetEngine(ctx).Where("user_id = ? AND credential_id = ?", userID, credID).Get(cred); err != nil {
 		return nil, err
 	} else if !found {
 		return nil, ErrWebAuthnCredentialNotExist{CredentialID: credID}
@@ -187,7 +187,7 @@ func createCredential(ctx context.Context, userID int64, name string, cred *weba
 	c := &WebAuthnCredential{
 		UserID:          userID,
 		Name:            name,
-		CredentialID:    base64.RawStdEncoding.EncodeToString(cred.ID),
+		CredentialID:    base32.HexEncoding.EncodeToString(cred.ID),
 		PublicKey:       cred.PublicKey,
 		AttestationType: cred.AttestationType,
 		AAGUID:          cred.Authenticator.AAGUID,

models/auth/webauthn_test.go

@@ -5,7 +5,7 @@
 package auth
 
 import (
-	"encoding/base64"
+	"encoding/base32"
 	"testing"
 
 	"code.gitea.io/gitea/models/unittest"
@@ -61,7 +61,7 @@ func TestCreateCredential(t *testing.T) {
 	res, err := CreateCredential(1, "WebAuthn Created Credential", &webauthn.Credential{ID: []byte("Test")})
 	assert.NoError(t, err)
 	assert.Equal(t, "WebAuthn Created Credential", res.Name)
-	bs, err := base64.RawStdEncoding.DecodeString(res.CredentialID)
+	bs, err := base32.HexEncoding.DecodeString(res.CredentialID)
 	assert.NoError(t, err)
 	assert.Equal(t, []byte("Test"), bs)
 

models/migrations/migrations.go

@@ -368,6 +368,8 @@ var migrations = []Migration{
 	NewMigration("Add authorize column to team_unit table", addAuthorizeColForTeamUnit),
 	// v207 -> v208
 	NewMigration("Add webauthn table and migrate u2f data to webauthn", addWebAuthnCred),
+	// v208 -> v209
+	NewMigration("Use base32.HexEncoding instead of base64 encoding for cred ID as it is case insensitive", useBase32HexForCredIDInWebAuthnCredential),
 }
 
 // GetCurrentDBVersion returns the current db version

models/migrations/v208.go

@@ -0,0 +1,51 @@
+// Copyright 2021 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package migrations
+
+import (
+	"encoding/base32"
+	"encoding/base64"
+
+	"xorm.io/xorm"
+)
+
+func useBase32HexForCredIDInWebAuthnCredential(x *xorm.Engine) error {
+
+	// Create webauthnCredential table
+	type webauthnCredential struct {
+		ID           int64  `xorm:"pk autoincr"`
+		CredentialID string `xorm:"INDEX"`
+	}
+	if err := x.Sync2(&webauthnCredential{}); err != nil {
+		return err
+	}
+
+	var start int
+	regs := make([]*webauthnCredential, 0, 50)
+	for {
+		err := x.OrderBy("id").Limit(50, start).Find(&regs)
+		if err != nil {
+			return err
+		}
+
+		for _, reg := range regs {
+			credID, _ := base64.RawStdEncoding.DecodeString(reg.CredentialID)
+			reg.CredentialID = base32.HexEncoding.EncodeToString(credID)
+
+			_, err := x.Update(reg)
+			if err != nil {
+				return err
+			}
+		}
+
+		if len(regs) < 50 {
+			break
+		}
+		start += 50
+		regs = regs[:0]
+	}
+
+	return nil
+}

options/locale/locale_en-US.ini

@@ -748,10 +748,9 @@ passcode_invalid = The passcode is incorrect. Try again.
 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!
 twofa_failed_get_secret = Failed to get secret.
 
-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.
+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.
 webauthn_register_key = Add Security Key
 webauthn_nickname = Nickname
-webauthn_press_button = Press the button on your security key to register it.
 webauthn_delete_key = Remove Security Key
 webauthn_delete_key_desc = If you remove a security key you can no longer sign in with it. Continue?
 

routers/web/auth/webauthn.go

@@ -5,7 +5,7 @@
 package auth
 
 import (
-	"encoding/base64"
+	"encoding/base32"
 	"errors"
 	"net/http"
 
@@ -131,7 +131,7 @@ func WebAuthnLoginAssertionPost(ctx *context.Context) {
 	}
 
 	// Success! Get the credential and update the sign count with the new value we received.
-	dbCred, err := auth.GetWebAuthnCredentialByCredID(base64.RawStdEncoding.EncodeToString(cred.ID))
+	dbCred, err := auth.GetWebAuthnCredentialByCredID(user.ID, base32.HexEncoding.EncodeToString(cred.ID))
 	if err != nil {
 		ctx.ServerError("GetWebAuthnCredentialByCredID", err)
 		return

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

@@ -38,9 +38,9 @@ func WebAuthnRegister(ctx *context.Context) {
 		return
 	}
 
-	_ = ctx.Session.Delete("registration")
+	_ = ctx.Session.Delete("webauthnRegistration")
-	if err := ctx.Session.Set("WebauthnName", form.Name); err != nil {
+	if err := ctx.Session.Set("webauthnName", form.Name); err != nil {
-		ctx.ServerError("Unable to set session key for WebauthnName", err)
+		ctx.ServerError("Unable to set session key for webauthnName", err)
 		return
 	}
 
@@ -51,7 +51,7 @@ func WebAuthnRegister(ctx *context.Context) {
 	}
 
 	// Save the session data as marshaled JSON
-	if err = ctx.Session.Set("registration", sessionData); err != nil {
+	if err = ctx.Session.Set("webauthnRegistration", sessionData); err != nil {
 		ctx.ServerError("Unable to set session", err)
 		return
 	}
@@ -61,20 +61,20 @@ func WebAuthnRegister(ctx *context.Context) {
 
 // WebauthnRegisterPost receives the response of the security key
 func WebauthnRegisterPost(ctx *context.Context) {
-	name, ok := ctx.Session.Get("WebauthnName").(string)
+	name, ok := ctx.Session.Get("webauthnName").(string)
 	if !ok || name == "" {
-		ctx.ServerError("Get WebauthnName", errors.New("no WebauthnName"))
+		ctx.ServerError("Get webauthnName", errors.New("no webauthnName"))
 		return
 	}
 
 	// Load the session data
-	sessionData, ok := ctx.Session.Get("registration").(*webauthn.SessionData)
+	sessionData, ok := ctx.Session.Get("webauthnRegistration").(*webauthn.SessionData)
 	if !ok || sessionData == nil {
 		ctx.ServerError("Get registration", errors.New("no registration"))
 		return
 	}
 	defer func() {
-		_ = ctx.Session.Delete("registration")
+		_ = ctx.Session.Delete("webauthnRegistration")
 	}()
 
 	// Verify that the challenge succeeded
@@ -103,6 +103,8 @@ func WebauthnRegisterPost(ctx *context.Context) {
 		ctx.ServerError("CreateCredential", err)
 		return
 	}
+	_ = ctx.Session.Delete("webauthnName")
+
 	ctx.JSON(http.StatusCreated, cred)
 }
 

templates/user/auth/webauthn_error.tmpl

@@ -12,7 +12,7 @@
 			<div class="hide" data-webauthn-error-msg="duplicated"><p>{{.i18n.Tr "webauthn_error_duplicated"}}</div>
 			<div class="hide" data-webauthn-error-msg="empty"><p>{{.i18n.Tr "webauthn_error_empty"}}</div>
 			<div class="hide" data-webauthn-error-msg="timeout"><p>{{.i18n.Tr "webauthn_error_timeout"}}</div>
-			<div class="hide" data-webauthn-error-msg="0"></div>
+			<div class="hide" data-webauthn-error-msg="general"></div>
 		</div>
 	</div>
 	<div class="actions">

templates/user/settings/security/webauthn.tmpl

@@ -28,16 +28,6 @@
 	</div>
 </div>
 
-<div class="ui small modal" id="register-device">
-	<div class="header">{{.i18n.Tr "settings.webauthn_register_key"}}</div>
-	<div class="content">
-		<i class="notched spinner loading icon"></i> {{.i18n.Tr "settings.webauthn_press_button"}}
-	</div>
-	<div class="actions">
-		<div class="ui cancel button">{{.i18n.Tr "cancel"}}</div>
-	</div>
-</div>
-
 {{template "user/auth/webauthn_error" .}}
 
 <div class="ui small basic delete modal" id="delete-registration">

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

@@ -24,7 +24,7 @@ export function initUserAuthWebAuthn() {
         .then((credential) => {
           verifyAssertion(credential);
         }).catch((err) => {
-          webAuthnError(0, err.message);
+          webAuthnError('general', err.message);
         });
     }).fail(() => {
       webAuthnError('unknown');
@@ -113,11 +113,16 @@ function webauthnRegistered(newCredential) {
 
 function webAuthnError(errorType, message) {
   $('#webauthn-error [data-webauthn-error-msg]').hide();
-  if (errorType === 0 && message && message.length > 1) {
+  const $errorGeneral = $(`#webauthn-error [data-webauthn-error-msg=general]`);
-    $(`#webauthn-error [data-webauthn-error-msg=0]`).text(message);
+  if (errorType === 'general') {
-    $(`#webauthn-error [data-webauthn-error-msg=0]`).show();
+    $errorGeneral.show().text(message || 'unknown error');
   } else {
-    $(`#webauthn-error [data-webauthn-error-msg=${errorType}]`).show();
+    const $errorTyped = $(`#webauthn-error [data-webauthn-error-msg=${errorType}]`);
+    if ($errorTyped.length) {
+      $errorTyped.show();
+    } else {
+      $errorGeneral.show().text(`unknown error type: ${errorType}`);
+    }
   }
   $('#webauthn-error').modal('show');
 }
@@ -149,7 +154,6 @@ export function initUserAuthWebAuthnRegister() {
     return;
   }
 
-  $('#register-device').modal({allowMultiple: false});
   $('#webauthn-error').modal({allowMultiple: false});
   $('#register-webauthn').on('click', (e) => {
     e.preventDefault();
@@ -167,7 +171,6 @@ function webAuthnRegisterRequest() {
     name: $('#nickname').val(),
   }).done((makeCredentialOptions) => {
     $('#nickname').closest('div.field').removeClass('error');
-    $('#register-device').modal('show');
 
     makeCredentialOptions.publicKey.challenge = decode(makeCredentialOptions.publicKey.challenge);
     makeCredentialOptions.publicKey.user.id = decode(makeCredentialOptions.publicKey.user.id);
@@ -185,7 +188,7 @@ function webAuthnRegisterRequest() {
           webAuthnError('unknown');
           return;
         }
-        webAuthnError(0, err);
+        webAuthnError('general', err.message);
       });
   }).fail((xhr) => {
     if (xhr.status === 409) {