mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2024-11-25 14:35:40 +00:00
[FEAT] Add support for webauthn credential level 3
- For WebAuthn Credential level 3, the `backup_eligible` and `backup_state` flags are checked if they are consistent with the values given on login. Forgejo never stored this data, so add a database migration that makes all webauthn credentials 'legacy' and on the next first use capture the values of `backup_eligible` and `backup_state`. As suggested in https://github.com/go-webauthn/webauthn/discussions/219#discussioncomment-10429662 - Adds unit tests. - Add E2E test.
This commit is contained in:
parent
28c3f1e254
commit
63736e8301
|
@ -40,7 +40,7 @@ func IsErrWebAuthnCredentialNotExist(err error) bool {
|
||||||
}
|
}
|
||||||
|
|
||||||
// WebAuthnCredential represents the WebAuthn credential data for a public-key
|
// 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 {
|
type WebAuthnCredential struct {
|
||||||
ID int64 `xorm:"pk autoincr"`
|
ID int64 `xorm:"pk autoincr"`
|
||||||
Name string
|
Name string
|
||||||
|
@ -52,6 +52,10 @@ type WebAuthnCredential struct {
|
||||||
AAGUID []byte
|
AAGUID []byte
|
||||||
SignCount uint32 `xorm:"BIGINT"`
|
SignCount uint32 `xorm:"BIGINT"`
|
||||||
CloneWarning bool
|
CloneWarning bool
|
||||||
|
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"`
|
CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
|
||||||
UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
|
UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
|
||||||
}
|
}
|
||||||
|
@ -71,6 +75,12 @@ func (cred *WebAuthnCredential) UpdateSignCount(ctx context.Context) error {
|
||||||
return err
|
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
|
// BeforeInsert will be invoked by XORM before updating a record
|
||||||
func (cred *WebAuthnCredential) BeforeInsert() {
|
func (cred *WebAuthnCredential) BeforeInsert() {
|
||||||
cred.LowerName = strings.ToLower(cred.Name)
|
cred.LowerName = strings.ToLower(cred.Name)
|
||||||
|
@ -97,6 +107,10 @@ func (list WebAuthnCredentialList) ToCredentials() []webauthn.Credential {
|
||||||
ID: cred.CredentialID,
|
ID: cred.CredentialID,
|
||||||
PublicKey: cred.PublicKey,
|
PublicKey: cred.PublicKey,
|
||||||
AttestationType: cred.AttestationType,
|
AttestationType: cred.AttestationType,
|
||||||
|
Flags: webauthn.CredentialFlags{
|
||||||
|
BackupEligible: cred.BackupEligible,
|
||||||
|
BackupState: cred.BackupState,
|
||||||
|
},
|
||||||
Authenticator: webauthn.Authenticator{
|
Authenticator: webauthn.Authenticator{
|
||||||
AAGUID: cred.AAGUID,
|
AAGUID: cred.AAGUID,
|
||||||
SignCount: cred.SignCount,
|
SignCount: cred.SignCount,
|
||||||
|
@ -167,6 +181,9 @@ func CreateCredential(ctx context.Context, userID int64, name string, cred *weba
|
||||||
AAGUID: cred.Authenticator.AAGUID,
|
AAGUID: cred.Authenticator.AAGUID,
|
||||||
SignCount: cred.Authenticator.SignCount,
|
SignCount: cred.Authenticator.SignCount,
|
||||||
CloneWarning: false,
|
CloneWarning: false,
|
||||||
|
BackupEligible: cred.Flags.BackupEligible,
|
||||||
|
BackupState: cred.Flags.BackupState,
|
||||||
|
Legacy: false,
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := db.Insert(ctx, c); err != nil {
|
if err := db.Insert(ctx, c); err != nil {
|
||||||
|
|
|
@ -56,13 +56,23 @@ func TestWebAuthnCredential_UpdateLargeCounter(t *testing.T) {
|
||||||
unittest.AssertExistsIf(t, true, &auth_model.WebAuthnCredential{ID: 1, SignCount: 0xffffffff})
|
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) {
|
func TestCreateCredential(t *testing.T) {
|
||||||
require.NoError(t, unittest.PrepareTestDatabase())
|
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)
|
require.NoError(t, err)
|
||||||
assert.Equal(t, "WebAuthn Created Credential", res.Name)
|
assert.Equal(t, "WebAuthn Created Credential", res.Name)
|
||||||
assert.Equal(t, []byte("Test"), res.CredentialID)
|
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")
|
||||||
}
|
}
|
||||||
|
|
|
@ -5,5 +5,6 @@
|
||||||
attestation_type: none
|
attestation_type: none
|
||||||
sign_count: 0
|
sign_count: 0
|
||||||
clone_warning: false
|
clone_warning: false
|
||||||
|
legacy: true
|
||||||
created_unix: 946684800
|
created_unix: 946684800
|
||||||
updated_unix: 946684800
|
updated_unix: 946684800
|
||||||
|
|
|
@ -80,6 +80,8 @@ var migrations = []*Migration{
|
||||||
NewMigration("Creating Quota-related tables", CreateQuotaTables),
|
NewMigration("Creating Quota-related tables", CreateQuotaTables),
|
||||||
// v21 -> v22
|
// v21 -> v22
|
||||||
NewMigration("Add SSH keypair to `pull_mirror` table", AddSSHKeypairToPushMirror),
|
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.
|
// GetCurrentDBVersion returns the current Forgejo database version.
|
||||||
|
|
17
models/forgejo_migrations/v22.go
Normal file
17
models/forgejo_migrations/v22.go
Normal file
|
@ -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{})
|
||||||
|
}
|
|
@ -116,6 +116,25 @@ func WebAuthnLoginAssertionPost(ctx *context.Context) {
|
||||||
return
|
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.
|
// Validate the parsed response.
|
||||||
cred, err := wa.WebAuthn.ValidateLogin((*wa.User)(user), *sessionData, parsedResponse)
|
cred, err := wa.WebAuthn.ValidateLogin((*wa.User)(user), *sessionData, parsedResponse)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
@ -133,13 +152,6 @@ func WebAuthnLoginAssertionPost(ctx *context.Context) {
|
||||||
return
|
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
|
dbCred.SignCount = cred.Authenticator.SignCount
|
||||||
if err := dbCred.UpdateSignCount(ctx); err != nil {
|
if err := dbCred.UpdateSignCount(ctx); err != nil {
|
||||||
ctx.ServerError("UpdateSignCount", err)
|
ctx.ServerError("UpdateSignCount", err)
|
||||||
|
|
60
tests/e2e/webauthn.test.e2e.js
Normal file
60
tests/e2e/webauthn.test.e2e.js
Normal file
|
@ -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`);
|
||||||
|
});
|
Loading…
Reference in a new issue