diff --git a/models/auth/webauthn.go b/models/auth/webauthn.go
index a65d2e1e34..aa13cf6cb1 100644
--- a/models/auth/webauthn.go
+++ b/models/auth/webauthn.go
@@ -40,7 +40,7 @@ func IsErrWebAuthnCredentialNotExist(err error) bool {
}
// WebAuthnCredential represents the WebAuthn credential data for a public-key
-// credential conformant to WebAuthn Level 1
+// credential conformant to WebAuthn Level 3
type WebAuthnCredential struct {
ID int64 `xorm:"pk autoincr"`
Name string
@@ -52,8 +52,12 @@ type WebAuthnCredential struct {
AAGUID []byte
SignCount uint32 `xorm:"BIGINT"`
CloneWarning bool
- CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
- UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
+ BackupEligible bool `XORM:"NOT NULL DEFAULT false"`
+ BackupState bool `XORM:"NOT NULL DEFAULT false"`
+ // If legacy is set to true, backup_eligible and backup_state isn't set.
+ Legacy bool `XORM:"NOT NULL DEFAULT true"`
+ CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
+ UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
}
func init() {
@@ -71,6 +75,12 @@ func (cred *WebAuthnCredential) UpdateSignCount(ctx context.Context) error {
return err
}
+// UpdateFromLegacy update the values that aren't present on legacy credentials.
+func (cred *WebAuthnCredential) UpdateFromLegacy(ctx context.Context) error {
+ _, err := db.GetEngine(ctx).ID(cred.ID).Cols("legacy", "backup_eligible", "backup_state").Update(cred)
+ return err
+}
+
// BeforeInsert will be invoked by XORM before updating a record
func (cred *WebAuthnCredential) BeforeInsert() {
cred.LowerName = strings.ToLower(cred.Name)
@@ -97,6 +107,10 @@ func (list WebAuthnCredentialList) ToCredentials() []webauthn.Credential {
ID: cred.CredentialID,
PublicKey: cred.PublicKey,
AttestationType: cred.AttestationType,
+ Flags: webauthn.CredentialFlags{
+ BackupEligible: cred.BackupEligible,
+ BackupState: cred.BackupState,
+ },
Authenticator: webauthn.Authenticator{
AAGUID: cred.AAGUID,
SignCount: cred.SignCount,
@@ -167,6 +181,9 @@ func CreateCredential(ctx context.Context, userID int64, name string, cred *weba
AAGUID: cred.Authenticator.AAGUID,
SignCount: cred.Authenticator.SignCount,
CloneWarning: false,
+ BackupEligible: cred.Flags.BackupEligible,
+ BackupState: cred.Flags.BackupState,
+ Legacy: false,
}
if err := db.Insert(ctx, c); err != nil {
diff --git a/models/auth/webauthn_test.go b/models/auth/webauthn_test.go
index cae590c790..e1cd652009 100644
--- a/models/auth/webauthn_test.go
+++ b/models/auth/webauthn_test.go
@@ -56,13 +56,23 @@ func TestWebAuthnCredential_UpdateLargeCounter(t *testing.T) {
unittest.AssertExistsIf(t, true, &auth_model.WebAuthnCredential{ID: 1, SignCount: 0xffffffff})
}
+func TestWebAuthenCredential_UpdateFromLegacy(t *testing.T) {
+ require.NoError(t, unittest.PrepareTestDatabase())
+ cred := unittest.AssertExistsAndLoadBean(t, &auth_model.WebAuthnCredential{ID: 1, Legacy: true})
+ cred.Legacy = false
+ cred.BackupEligible = true
+ cred.BackupState = true
+ require.NoError(t, cred.UpdateFromLegacy(db.DefaultContext))
+ unittest.AssertExistsIf(t, true, &auth_model.WebAuthnCredential{ID: 1, BackupEligible: true, BackupState: true}, "legacy = false")
+}
+
func TestCreateCredential(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
- res, err := auth_model.CreateCredential(db.DefaultContext, 1, "WebAuthn Created Credential", &webauthn.Credential{ID: []byte("Test")})
+ res, err := auth_model.CreateCredential(db.DefaultContext, 1, "WebAuthn Created Credential", &webauthn.Credential{ID: []byte("Test"), Flags: webauthn.CredentialFlags{BackupEligible: true, BackupState: true}})
require.NoError(t, err)
assert.Equal(t, "WebAuthn Created Credential", res.Name)
assert.Equal(t, []byte("Test"), res.CredentialID)
- unittest.AssertExistsIf(t, true, &auth_model.WebAuthnCredential{Name: "WebAuthn Created Credential", UserID: 1})
+ unittest.AssertExistsIf(t, true, &auth_model.WebAuthnCredential{Name: "WebAuthn Created Credential", UserID: 1, BackupEligible: true, BackupState: true}, "legacy = false")
}
diff --git a/models/fixtures/webauthn_credential.yml b/models/fixtures/webauthn_credential.yml
index bc43127fcd..edf9935ebf 100644
--- a/models/fixtures/webauthn_credential.yml
+++ b/models/fixtures/webauthn_credential.yml
@@ -5,5 +5,6 @@
attestation_type: none
sign_count: 0
clone_warning: false
+ legacy: true
created_unix: 946684800
updated_unix: 946684800
diff --git a/models/forgejo_migrations/migrate.go b/models/forgejo_migrations/migrate.go
index 598ec8bbaa..cca83d6b4d 100644
--- a/models/forgejo_migrations/migrate.go
+++ b/models/forgejo_migrations/migrate.go
@@ -80,6 +80,8 @@ var migrations = []*Migration{
NewMigration("Creating Quota-related tables", CreateQuotaTables),
// v21 -> v22
NewMigration("Add SSH keypair to `pull_mirror` table", AddSSHKeypairToPushMirror),
+ // v22 -> v23
+ NewMigration("Add `legacy` to `web_authn_credential` table", AddLegacyToWebAuthnCredential),
}
// GetCurrentDBVersion returns the current Forgejo database version.
diff --git a/models/forgejo_migrations/v22.go b/models/forgejo_migrations/v22.go
new file mode 100644
index 0000000000..eeb738799c
--- /dev/null
+++ b/models/forgejo_migrations/v22.go
@@ -0,0 +1,17 @@
+// Copyright 2024 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package forgejo_migrations //nolint:revive
+
+import "xorm.io/xorm"
+
+func AddLegacyToWebAuthnCredential(x *xorm.Engine) error {
+ type WebauthnCredential struct {
+ ID int64 `xorm:"pk autoincr"`
+ BackupEligible bool `xorm:"NOT NULL DEFAULT false"`
+ BackupState bool `xorm:"NOT NULL DEFAULT false"`
+ Legacy bool `xorm:"NOT NULL DEFAULT true"`
+ }
+
+ return x.Sync(&WebauthnCredential{})
+}
diff --git a/routers/web/auth/webauthn.go b/routers/web/auth/webauthn.go
index 1079f44a08..5c93c1410e 100644
--- a/routers/web/auth/webauthn.go
+++ b/routers/web/auth/webauthn.go
@@ -116,6 +116,25 @@ func WebAuthnLoginAssertionPost(ctx *context.Context) {
return
}
+ dbCred, err := auth.GetWebAuthnCredentialByCredID(ctx, user.ID, parsedResponse.RawID)
+ if err != nil {
+ ctx.ServerError("GetWebAuthnCredentialByCredID", err)
+ return
+ }
+
+ // If the credential is legacy, assume the values are correct. The
+ // specification mandates these flags don't change.
+ if dbCred.Legacy {
+ dbCred.BackupEligible = parsedResponse.Response.AuthenticatorData.Flags.HasBackupEligible()
+ dbCred.BackupState = parsedResponse.Response.AuthenticatorData.Flags.HasBackupState()
+ dbCred.Legacy = false
+
+ if err := dbCred.UpdateFromLegacy(ctx); err != nil {
+ ctx.ServerError("UpdateFromLegacy", err)
+ return
+ }
+ }
+
// Validate the parsed response.
cred, err := wa.WebAuthn.ValidateLogin((*wa.User)(user), *sessionData, parsedResponse)
if err != nil {
@@ -133,13 +152,6 @@ func WebAuthnLoginAssertionPost(ctx *context.Context) {
return
}
- // Success! Get the credential and update the sign count with the new value we received.
- dbCred, err := auth.GetWebAuthnCredentialByCredID(ctx, user.ID, cred.ID)
- if err != nil {
- ctx.ServerError("GetWebAuthnCredentialByCredID", err)
- return
- }
-
dbCred.SignCount = cred.Authenticator.SignCount
if err := dbCred.UpdateSignCount(ctx); err != nil {
ctx.ServerError("UpdateSignCount", err)
diff --git a/tests/e2e/webauthn.test.e2e.js b/tests/e2e/webauthn.test.e2e.js
new file mode 100644
index 0000000000..30c92c48f0
--- /dev/null
+++ b/tests/e2e/webauthn.test.e2e.js
@@ -0,0 +1,60 @@
+// Copyright 2024 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+// @ts-check
+
+import {expect} from '@playwright/test';
+import {test, login_user, load_logged_in_context} from './utils_e2e.js';
+
+test.beforeAll(async ({browser}, workerInfo) => {
+ await login_user(browser, workerInfo, 'user2');
+});
+
+test('WebAuthn register & login flow', async ({browser}, workerInfo) => {
+ test.skip(workerInfo.project.name !== 'chromium', 'Uses Chrome protocol');
+ const context = await load_logged_in_context(browser, workerInfo, 'user2');
+ const page = await context.newPage();
+
+ // Register a security key.
+ let response = await page.goto('/user/settings/security');
+ await expect(response?.status()).toBe(200);
+
+ // https://github.com/microsoft/playwright/issues/7276#issuecomment-1516768428
+ const cdpSession = await page.context().newCDPSession(page);
+ await cdpSession.send('WebAuthn.enable');
+ await cdpSession.send('WebAuthn.addVirtualAuthenticator', {
+ options: {
+ protocol: 'ctap2',
+ ctap2Version: 'ctap2_1',
+ hasUserVerification: true,
+ transport: 'usb',
+ automaticPresenceSimulation: true,
+ isUserVerified: true,
+ backupEligibility: true,
+ },
+ });
+
+ await page.locator('input#nickname').fill('Testing Security Key');
+ await page.getByText('Add security key').click();
+
+ // Logout.
+ await page.locator('div[aria-label="Profile and settings…"]').click();
+ await page.getByText('Sign Out').click();
+ await page.waitForURL(`${workerInfo.project.use.baseURL}/`);
+
+ // Login.
+ response = await page.goto('/user/login');
+ await expect(response?.status()).toBe(200);
+
+ await page.getByLabel('Username or email address').fill('user2');
+ await page.getByLabel('Password').fill('password');
+ await page.getByRole('button', {name: 'Sign in'}).click();
+ await page.waitForURL(`${workerInfo.project.use.baseURL}/user/webauthn`);
+ await page.waitForURL(`${workerInfo.project.use.baseURL}/`);
+
+ // Cleanup.
+ response = await page.goto('/user/settings/security');
+ await expect(response?.status()).toBe(200);
+ await page.getByRole('button', {name: 'Remove'}).click();
+ await page.getByRole('button', {name: 'Yes'}).click();
+ await page.waitForURL(`${workerInfo.project.use.baseURL}/user/settings/security`);
+});