From 87bb5ed0bcaeb52ccb97ccdd2fadc92663b6f41e Mon Sep 17 00:00:00 2001 From: hiifong Date: Wed, 27 Nov 2024 00:04:17 +0800 Subject: [PATCH] Fix: passkey login not working anymore (#32623) Quick fix #32595, use authenticator auth flags to login --------- Co-authored-by: wxiaoguang --- models/auth/webauthn.go | 22 ++++++++- models/db/engine.go | 3 ++ models/migrations/base/tests.go | 8 ++-- modules/auth/webauthn/webauthn.go | 45 ++++++++++--------- routers/web/auth/webauthn.go | 21 ++++++--- routers/web/user/setting/security/webauthn.go | 6 ++- web_src/js/features/user-auth-webauthn.ts | 22 ++++----- web_src/js/modules/fetch.ts | 4 +- web_src/js/types.ts | 2 +- 9 files changed, 86 insertions(+), 47 deletions(-) diff --git a/models/auth/webauthn.go b/models/auth/webauthn.go index 553130ee2e..6d8b542957 100644 --- a/models/auth/webauthn.go +++ b/models/auth/webauthn.go @@ -12,6 +12,7 @@ import ( "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" + "github.com/go-webauthn/webauthn/protocol" "github.com/go-webauthn/webauthn/webauthn" ) @@ -89,14 +90,33 @@ func (cred *WebAuthnCredential) AfterLoad() { // WebAuthnCredentialList is a list of *WebAuthnCredential type WebAuthnCredentialList []*WebAuthnCredential +// newCredentialFlagsFromAuthenticatorFlags is copied from https://github.com/go-webauthn/webauthn/pull/337 +// to convert protocol.AuthenticatorFlags to webauthn.CredentialFlags +func newCredentialFlagsFromAuthenticatorFlags(flags protocol.AuthenticatorFlags) webauthn.CredentialFlags { + return webauthn.CredentialFlags{ + UserPresent: flags.HasUserPresent(), + UserVerified: flags.HasUserVerified(), + BackupEligible: flags.HasBackupEligible(), + BackupState: flags.HasBackupState(), + } +} + // ToCredentials will convert all WebAuthnCredentials to webauthn.Credentials -func (list WebAuthnCredentialList) ToCredentials() []webauthn.Credential { +func (list WebAuthnCredentialList) ToCredentials(defaultAuthFlags ...protocol.AuthenticatorFlags) []webauthn.Credential { + // TODO: at the moment, Gitea doesn't store or check the flags + // so we need to use the default flags from the authenticator to make the login validation pass + // In the future, we should: + // 1. store the flags when registering the credential + // 2. provide the stored flags when converting the credentials (for login) + // 3. for old users, still use this fallback to the default flags + defAuthFlags := util.OptionalArg(defaultAuthFlags) creds := make([]webauthn.Credential, 0, len(list)) for _, cred := range list { creds = append(creds, webauthn.Credential{ ID: cred.CredentialID, PublicKey: cred.PublicKey, AttestationType: cred.AttestationType, + Flags: newCredentialFlagsFromAuthenticatorFlags(defAuthFlags), Authenticator: webauthn.Authenticator{ AAGUID: cred.AAGUID, SignCount: cred.SignCount, diff --git a/models/db/engine.go b/models/db/engine.go index e50a8580bf..b17188945a 100755 --- a/models/db/engine.go +++ b/models/db/engine.go @@ -134,6 +134,9 @@ func SyncAllTables() error { func InitEngine(ctx context.Context) error { xormEngine, err := newXORMEngine() if err != nil { + if strings.Contains(err.Error(), "SQLite3 support") { + return fmt.Errorf(`sqlite3 requires: -tags sqlite,sqlite_unlock_notify%s%w`, "\n", err) + } return fmt.Errorf("failed to connect to database: %w", err) } diff --git a/models/migrations/base/tests.go b/models/migrations/base/tests.go index ddf9a544da..c2134f702a 100644 --- a/models/migrations/base/tests.go +++ b/models/migrations/base/tests.go @@ -18,7 +18,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/testlogger" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "xorm.io/xorm" ) @@ -33,15 +33,15 @@ func PrepareTestEnv(t *testing.T, skip int, syncModels ...any) (*xorm.Engine, fu ourSkip := 2 ourSkip += skip deferFn := testlogger.PrintCurrentTest(t, ourSkip) - assert.NoError(t, unittest.SyncDirs(filepath.Join(filepath.Dir(setting.AppPath), "tests/gitea-repositories-meta"), setting.RepoRootPath)) + require.NoError(t, unittest.SyncDirs(filepath.Join(filepath.Dir(setting.AppPath), "tests/gitea-repositories-meta"), setting.RepoRootPath)) if err := deleteDB(); err != nil { - t.Errorf("unable to reset database: %v", err) + t.Fatalf("unable to reset database: %v", err) return nil, deferFn } x, err := newXORMEngine() - assert.NoError(t, err) + require.NoError(t, err) if x != nil { oldDefer := deferFn deferFn = func() { diff --git a/modules/auth/webauthn/webauthn.go b/modules/auth/webauthn/webauthn.go index 790006ee56..cbf5279c65 100644 --- a/modules/auth/webauthn/webauthn.go +++ b/modules/auth/webauthn/webauthn.go @@ -4,13 +4,14 @@ package webauthn import ( + "context" "encoding/binary" "encoding/gob" "code.gitea.io/gitea/models/auth" - "code.gitea.io/gitea/models/db" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "github.com/go-webauthn/webauthn/protocol" "github.com/go-webauthn/webauthn/webauthn" @@ -38,40 +39,42 @@ func Init() { } } -// User represents an implementation of webauthn.User based on User model -type User user_model.User +// user represents an implementation of webauthn.User based on User model +type user struct { + ctx context.Context + User *user_model.User + + defaultAuthFlags protocol.AuthenticatorFlags +} + +var _ webauthn.User = (*user)(nil) + +func NewWebAuthnUser(ctx context.Context, u *user_model.User, defaultAuthFlags ...protocol.AuthenticatorFlags) webauthn.User { + return &user{ctx: ctx, User: u, defaultAuthFlags: util.OptionalArg(defaultAuthFlags)} +} // WebAuthnID implements the webauthn.User interface -func (u *User) WebAuthnID() []byte { +func (u *user) WebAuthnID() []byte { id := make([]byte, 8) - binary.PutVarint(id, u.ID) + binary.PutVarint(id, u.User.ID) return id } // WebAuthnName implements the webauthn.User interface -func (u *User) WebAuthnName() string { - if u.LoginName == "" { - return u.Name - } - return u.LoginName +func (u *user) WebAuthnName() string { + return util.IfZero(u.User.LoginName, u.User.Name) } // WebAuthnDisplayName implements the webauthn.User interface -func (u *User) WebAuthnDisplayName() string { - return (*user_model.User)(u).DisplayName() -} - -// WebAuthnIcon implements the webauthn.User interface -func (u *User) WebAuthnIcon() string { - return (*user_model.User)(u).AvatarLink(db.DefaultContext) +func (u *user) WebAuthnDisplayName() string { + return u.User.DisplayName() } // WebAuthnCredentials implements the webauthn.User interface -func (u *User) WebAuthnCredentials() []webauthn.Credential { - dbCreds, err := auth.GetWebAuthnCredentialsByUID(db.DefaultContext, u.ID) +func (u *user) WebAuthnCredentials() []webauthn.Credential { + dbCreds, err := auth.GetWebAuthnCredentialsByUID(u.ctx, u.User.ID) if err != nil { return nil } - - return dbCreds.ToCredentials() + return dbCreds.ToCredentials(u.defaultAuthFlags) } diff --git a/routers/web/auth/webauthn.go b/routers/web/auth/webauthn.go index 3160c5e23f..ba25d45070 100644 --- a/routers/web/auth/webauthn.go +++ b/routers/web/auth/webauthn.go @@ -76,8 +76,17 @@ func WebAuthnPasskeyLogin(ctx *context.Context) { }() // Validate the parsed response. + + // ParseCredentialRequestResponse+ValidateDiscoverableLogin equals to FinishDiscoverableLogin, but we need to ParseCredentialRequestResponse first to get flags var user *user_model.User - cred, err := wa.WebAuthn.FinishDiscoverableLogin(func(rawID, userHandle []byte) (webauthn.User, error) { + parsedResponse, err := protocol.ParseCredentialRequestResponse(ctx.Req) + if err != nil { + // Failed authentication attempt. + log.Info("Failed authentication attempt for %s from %s: %v", user.Name, ctx.RemoteAddr(), err) + ctx.Status(http.StatusForbidden) + return + } + cred, err := wa.WebAuthn.ValidateDiscoverableLogin(func(rawID, userHandle []byte) (webauthn.User, error) { userID, n := binary.Varint(userHandle) if n <= 0 { return nil, errors.New("invalid rawID") @@ -89,8 +98,8 @@ func WebAuthnPasskeyLogin(ctx *context.Context) { return nil, err } - return (*wa.User)(user), nil - }, *sessionData, ctx.Req) + return wa.NewWebAuthnUser(ctx, user, parsedResponse.Response.AuthenticatorData.Flags), nil + }, *sessionData, parsedResponse) if err != nil { // Failed authentication attempt. log.Info("Failed authentication attempt for passkey from %s: %v", ctx.RemoteAddr(), err) @@ -171,7 +180,8 @@ func WebAuthnLoginAssertion(ctx *context.Context) { return } - assertion, sessionData, err := wa.WebAuthn.BeginLogin((*wa.User)(user)) + webAuthnUser := wa.NewWebAuthnUser(ctx, user) + assertion, sessionData, err := wa.WebAuthn.BeginLogin(webAuthnUser) if err != nil { ctx.ServerError("webauthn.BeginLogin", err) return @@ -216,7 +226,8 @@ func WebAuthnLoginAssertionPost(ctx *context.Context) { } // Validate the parsed response. - cred, err := wa.WebAuthn.ValidateLogin((*wa.User)(user), *sessionData, parsedResponse) + webAuthnUser := wa.NewWebAuthnUser(ctx, user, parsedResponse.Response.AuthenticatorData.Flags) + cred, err := wa.WebAuthn.ValidateLogin(webAuthnUser, *sessionData, parsedResponse) if err != nil { // Failed authentication attempt. log.Info("Failed authentication attempt for %s from %s: %v", user.Name, ctx.RemoteAddr(), err) diff --git a/routers/web/user/setting/security/webauthn.go b/routers/web/user/setting/security/webauthn.go index aafc2f2a64..70bfaac6e0 100644 --- a/routers/web/user/setting/security/webauthn.go +++ b/routers/web/user/setting/security/webauthn.go @@ -51,7 +51,8 @@ func WebAuthnRegister(ctx *context.Context) { return } - credentialOptions, sessionData, err := wa.WebAuthn.BeginRegistration((*wa.User)(ctx.Doer), webauthn.WithAuthenticatorSelection(protocol.AuthenticatorSelection{ + webAuthnUser := wa.NewWebAuthnUser(ctx, ctx.Doer) + credentialOptions, sessionData, err := wa.WebAuthn.BeginRegistration(webAuthnUser, webauthn.WithAuthenticatorSelection(protocol.AuthenticatorSelection{ ResidentKey: protocol.ResidentKeyRequirementRequired, })) if err != nil { @@ -92,7 +93,8 @@ func WebauthnRegisterPost(ctx *context.Context) { }() // Verify that the challenge succeeded - cred, err := wa.WebAuthn.FinishRegistration((*wa.User)(ctx.Doer), *sessionData, ctx.Req) + webAuthnUser := wa.NewWebAuthnUser(ctx, ctx.Doer) + cred, err := wa.WebAuthn.FinishRegistration(webAuthnUser, *sessionData, ctx.Req) if err != nil { if pErr, ok := err.(*protocol.Error); ok { log.Error("Unable to finish registration due to error: %v\nDevInfo: %s", pErr, pErr.DevInfo) diff --git a/web_src/js/features/user-auth-webauthn.ts b/web_src/js/features/user-auth-webauthn.ts index 610b559833..70516c280d 100644 --- a/web_src/js/features/user-auth-webauthn.ts +++ b/web_src/js/features/user-auth-webauthn.ts @@ -40,14 +40,15 @@ async function loginPasskey() { try { const credential = await navigator.credentials.get({ publicKey: options.publicKey, - }); + }) as PublicKeyCredential; + const credResp = credential.response as AuthenticatorAssertionResponse; // Move data into Arrays in case it is super long - const authData = new Uint8Array(credential.response.authenticatorData); - const clientDataJSON = new Uint8Array(credential.response.clientDataJSON); + const authData = new Uint8Array(credResp.authenticatorData); + const clientDataJSON = new Uint8Array(credResp.clientDataJSON); const rawId = new Uint8Array(credential.rawId); - const sig = new Uint8Array(credential.response.signature); - const userHandle = new Uint8Array(credential.response.userHandle); + const sig = new Uint8Array(credResp.signature); + const userHandle = new Uint8Array(credResp.userHandle); const res = await POST(`${appSubUrl}/user/webauthn/passkey/login`, { data: { @@ -175,7 +176,7 @@ async function webauthnRegistered(newCredential) { window.location.reload(); } -function webAuthnError(errorType, message) { +function webAuthnError(errorType: string, message:string = '') { const elErrorMsg = document.querySelector(`#webauthn-error-msg`); if (errorType === 'general') { @@ -207,10 +208,9 @@ function detectWebAuthnSupport() { } export function initUserAuthWebAuthnRegister() { - const elRegister = document.querySelector('#register-webauthn'); - if (!elRegister) { - return; - } + const elRegister = document.querySelector('#register-webauthn'); + if (!elRegister) return; + if (!detectWebAuthnSupport()) { elRegister.disabled = true; return; @@ -222,7 +222,7 @@ export function initUserAuthWebAuthnRegister() { } async function webAuthnRegisterRequest() { - const elNickname = document.querySelector('#nickname'); + const elNickname = document.querySelector('#nickname'); const formData = new FormData(); formData.append('name', elNickname.value); diff --git a/web_src/js/modules/fetch.ts b/web_src/js/modules/fetch.ts index de3ef1de7e..0c078bfe78 100644 --- a/web_src/js/modules/fetch.ts +++ b/web_src/js/modules/fetch.ts @@ -1,5 +1,5 @@ import {isObject} from '../utils.ts'; -import type {RequestData, RequestOpts} from '../types.ts'; +import type {RequestOpts} from '../types.ts'; const {csrfToken} = window.config; @@ -10,7 +10,7 @@ const safeMethods = new Set(['GET', 'HEAD', 'OPTIONS', 'TRACE']); // which will automatically set an appropriate headers. For json content, only object // and array types are currently supported. export function request(url: string, {method = 'GET', data, headers = {}, ...other}: RequestOpts = {}): Promise { - let body: RequestData; + let body: string | FormData | URLSearchParams; let contentType: string; if (data instanceof FormData || data instanceof URLSearchParams) { body = data; diff --git a/web_src/js/types.ts b/web_src/js/types.ts index 317719fad5..29279bbded 100644 --- a/web_src/js/types.ts +++ b/web_src/js/types.ts @@ -24,7 +24,7 @@ export type Config = { export type Intent = 'error' | 'warning' | 'info'; -export type RequestData = string | FormData | URLSearchParams; +export type RequestData = string | FormData | URLSearchParams | Record; export type RequestOpts = { data?: RequestData,