From 835cf4c397a00a2b1041b1f591f868d10dce8cdd Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Thu, 28 Jul 2022 22:13:27 +0100 Subject: [PATCH 01/12] WebAuthn CredentialID field needs to be increased in size WebAuthn have updated their specification to set the maximum size of the CredentialID to 1023 bytes. This is somewhat larger than our current size and therefore we need to migrate. Fix #20457 Signed-off-by: Andrew Thornton --- models/auth/webauthn.go | 2 +- models/migrations/migrations.go | 2 + models/migrations/v210.go | 8 ++-- models/migrations/v210_test.go | 4 +- models/migrations/v221.go | 75 +++++++++++++++++++++++++++++++++ 5 files changed, 84 insertions(+), 7 deletions(-) create mode 100644 models/migrations/v221.go diff --git a/models/auth/webauthn.go b/models/auth/webauthn.go index 2dc304378010..fc4dc6b76324 100644 --- a/models/auth/webauthn.go +++ b/models/auth/webauthn.go @@ -43,7 +43,7 @@ type WebAuthnCredential struct { Name string LowerName string `xorm:"unique(s)"` UserID int64 `xorm:"INDEX unique(s)"` - CredentialID string `xorm:"INDEX VARCHAR(410)"` + CredentialID string `xorm:"INDEX VARCHAR(1640)"` PublicKey []byte AttestationType string AAGUID []byte diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index beeba866dc70..876ea7f29df1 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -400,6 +400,8 @@ var migrations = []Migration{ NewMigration("Add sync_on_commit column to push_mirror table", addSyncOnCommitColForPushMirror), // v220 -> v221 NewMigration("Add container repository property", addContainerRepositoryProperty), + // v221 -> v222 + NewMigration("Increase WebAuthentication CredentialID size to 1640", increaseCredentialIDTo1640), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v210.go b/models/migrations/v210.go index f32fae77bacc..92932372f63a 100644 --- a/models/migrations/v210.go +++ b/models/migrations/v210.go @@ -25,7 +25,7 @@ func remigrateU2FCredentials(x *xorm.Engine) error { Name string LowerName string `xorm:"unique(s)"` UserID int64 `xorm:"INDEX unique(s)"` - CredentialID string `xorm:"INDEX VARCHAR(410)"` // CredentalID in U2F is at most 255bytes / 5 * 8 = 408 - add a few extra characters for safety + CredentialID string `xorm:"INDEX VARCHAR(1640)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 -> 1640 base32 encoding PublicKey []byte AttestationType string AAGUID []byte @@ -40,12 +40,12 @@ func remigrateU2FCredentials(x *xorm.Engine) error { switch x.Dialect().URI().DBType { case schemas.MYSQL: - _, err := x.Exec("ALTER TABLE webauthn_credential MODIFY COLUMN credential_id VARCHAR(410)") + _, err := x.Exec("ALTER TABLE webauthn_credential MODIFY COLUMN credential_id VARCHAR(1640)") if err != nil { return err } case schemas.ORACLE: - _, err := x.Exec("ALTER TABLE webauthn_credential MODIFY credential_id VARCHAR(410)") + _, err := x.Exec("ALTER TABLE webauthn_credential MODIFY credential_id VARCHAR(1640)") if err != nil { return err } @@ -70,7 +70,7 @@ func remigrateU2FCredentials(x *xorm.Engine) error { return err } case schemas.POSTGRES: - _, err := x.Exec("ALTER TABLE webauthn_credential ALTER COLUMN credential_id TYPE VARCHAR(410)") + _, err := x.Exec("ALTER TABLE webauthn_credential ALTER COLUMN credential_id TYPE VARCHAR(1640)") if err != nil { return err } diff --git a/models/migrations/v210_test.go b/models/migrations/v210_test.go index 70dbe61b06eb..d621a39bc29a 100644 --- a/models/migrations/v210_test.go +++ b/models/migrations/v210_test.go @@ -20,7 +20,7 @@ func Test_remigrateU2FCredentials(t *testing.T) { Name string LowerName string `xorm:"unique(s)"` UserID int64 `xorm:"INDEX unique(s)"` - CredentialID string `xorm:"INDEX VARCHAR(410)"` // CredentalID in U2F is at most 255bytes / 5 * 8 = 408 - add a few extra characters for safety + CredentialID string `xorm:"INDEX VARCHAR(1640)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 -> 1640 base32 encoding PublicKey []byte AttestationType string SignCount uint32 `xorm:"BIGINT"` @@ -40,7 +40,7 @@ func Test_remigrateU2FCredentials(t *testing.T) { type ExpectedWebauthnCredential struct { ID int64 `xorm:"pk autoincr"` - CredentialID string `xorm:"INDEX VARCHAR(410)"` // CredentalID in U2F is at most 255bytes / 5 * 8 = 408 - add a few extra characters for safety + CredentialID string `xorm:"INDEX VARCHAR(1640)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 -> 1640 base32 encoding } // Prepare and load the testing database diff --git a/models/migrations/v221.go b/models/migrations/v221.go new file mode 100644 index 000000000000..5c610fd3ff35 --- /dev/null +++ b/models/migrations/v221.go @@ -0,0 +1,75 @@ +// Copyright 2022 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 ( + "code.gitea.io/gitea/modules/timeutil" + + "xorm.io/xorm" + "xorm.io/xorm/schemas" +) + +func increaseCredentialIDTo1640(x *xorm.Engine) error { + // Create webauthnCredential table + type webauthnCredential struct { + ID int64 `xorm:"pk autoincr"` + Name string + LowerName string `xorm:"unique(s)"` + UserID int64 `xorm:"INDEX unique(s)"` + CredentialID string `xorm:"INDEX VARCHAR(1640)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 -> 1640 base32 encoding + PublicKey []byte + AttestationType string + AAGUID []byte + SignCount uint32 `xorm:"BIGINT"` + CloneWarning bool + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` + } + if err := x.Sync2(&webauthnCredential{}); err != nil { + return err + } + + switch x.Dialect().URI().DBType { + case schemas.MYSQL: + _, err := x.Exec("ALTER TABLE webauthn_credential MODIFY COLUMN credential_id VARCHAR(1640)") + if err != nil { + return err + } + case schemas.ORACLE: + _, err := x.Exec("ALTER TABLE webauthn_credential MODIFY credential_id VARCHAR(1640)") + if err != nil { + return err + } + case schemas.MSSQL: + // This column has an index on it. I could write all of the code to attempt to change the index OR + // I could just use recreate table. + sess := x.NewSession() + if err := sess.Begin(); err != nil { + _ = sess.Close() + return err + } + + if err := recreateTable(sess, new(webauthnCredential)); err != nil { + _ = sess.Close() + return err + } + if err := sess.Commit(); err != nil { + _ = sess.Close() + return err + } + if err := sess.Close(); err != nil { + return err + } + case schemas.POSTGRES: + _, err := x.Exec("ALTER TABLE webauthn_credential ALTER COLUMN credential_id TYPE VARCHAR(1640)") + if err != nil { + return err + } + default: + // SQLite doesn't support ALTER COLUMN, and it already makes String _TEXT_ by default so no migration needed + // nor is there any need to re-migrate + } + return nil +} From 46fced84fb86d09c90aaa1e8bd78981c83f5ec49 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 29 Jul 2022 20:00:03 +0100 Subject: [PATCH 02/12] Move to use webauthnCredential CredentialID as bytes Signed-off-by: Andrew Thornton --- models/auth/webauthn.go | 44 ++++----- .../expected_webauthn_credential.yml | 9 ++ .../webauthn_credential.yml | 31 ++++++ models/migrations/migrations.go | 2 +- models/migrations/v210.go | 8 +- models/migrations/v210_test.go | 4 +- models/migrations/v221.go | 99 ++++++++++--------- models/migrations/v221_test.go | 61 ++++++++++++ 8 files changed, 182 insertions(+), 76 deletions(-) create mode 100644 models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/expected_webauthn_credential.yml create mode 100644 models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/webauthn_credential.yml create mode 100644 models/migrations/v221_test.go diff --git a/models/auth/webauthn.go b/models/auth/webauthn.go index fc4dc6b76324..af7086c24de0 100644 --- a/models/auth/webauthn.go +++ b/models/auth/webauthn.go @@ -6,7 +6,6 @@ package auth import ( "context" - "encoding/base32" "fmt" "strings" @@ -39,18 +38,18 @@ func IsErrWebAuthnCredentialNotExist(err error) bool { // WebAuthnCredential represents the WebAuthn credential data for a public-key // credential conformant to WebAuthn Level 1 type WebAuthnCredential struct { - ID int64 `xorm:"pk autoincr"` - Name string - LowerName string `xorm:"unique(s)"` - UserID int64 `xorm:"INDEX unique(s)"` - CredentialID string `xorm:"INDEX VARCHAR(1640)"` - PublicKey []byte - AttestationType string - AAGUID []byte - SignCount uint32 `xorm:"BIGINT"` - CloneWarning bool - CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` - UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` + ID int64 `xorm:"pk autoincr"` + Name string + LowerName string `xorm:"unique(s)"` + UserID int64 `xorm:"INDEX unique(s)"` + CredentialIDBytes []byte `xorm:"INDEX VARBINARY(1024)"` + PublicKey []byte + AttestationType string + AAGUID []byte + SignCount uint32 `xorm:"BIGINT"` + CloneWarning bool + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` } func init() { @@ -94,9 +93,8 @@ type WebAuthnCredentialList []*WebAuthnCredential func (list WebAuthnCredentialList) ToCredentials() []webauthn.Credential { creds := make([]webauthn.Credential, 0, len(list)) for _, cred := range list { - credID, _ := base32.HexEncoding.DecodeString(cred.CredentialID) creds = append(creds, webauthn.Credential{ - ID: credID, + ID: cred.CredentialIDBytes, PublicKey: cred.PublicKey, AttestationType: cred.AttestationType, Authenticator: webauthn.Authenticator{ @@ -185,14 +183,14 @@ func CreateCredential(userID int64, name string, cred *webauthn.Credential) (*We func createCredential(ctx context.Context, userID int64, name string, cred *webauthn.Credential) (*WebAuthnCredential, error) { c := &WebAuthnCredential{ - UserID: userID, - Name: name, - CredentialID: base32.HexEncoding.EncodeToString(cred.ID), - PublicKey: cred.PublicKey, - AttestationType: cred.AttestationType, - AAGUID: cred.Authenticator.AAGUID, - SignCount: cred.Authenticator.SignCount, - CloneWarning: false, + UserID: userID, + Name: name, + CredentialIDBytes: cred.ID, + PublicKey: cred.PublicKey, + AttestationType: cred.AttestationType, + AAGUID: cred.Authenticator.AAGUID, + SignCount: cred.Authenticator.SignCount, + CloneWarning: false, } if err := db.Insert(ctx, c); err != nil { diff --git a/models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/expected_webauthn_credential.yml b/models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/expected_webauthn_credential.yml new file mode 100644 index 000000000000..f68b3cd24c38 --- /dev/null +++ b/models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/expected_webauthn_credential.yml @@ -0,0 +1,9 @@ +- + id: 1 + credential_id_bytes: "TVHE44TOH7DF7V48SEAIT3EMMJ7TGBOQ289E5AQB34S98LFCUFJ7U2NAVI8RJG6K2F4TC8AQ8KBNO7AGEOQOL9NE43GR63HTEHJSLOG=" +- + id: 2 + credential_id_bytes: "051CLMMKB62S6M9M2A4H54K7MMCQALFJ36G4TGB2S9A47APLTILU6C6744CEBG4EKCGV357N21BSLH8JD33GQMFAR6DQ70S76P34J6FR=" +- + id: 4 + credential_id_bytes: "APU4B1NDTEVTEM60V4T0FRL7SRJMO9KIE2AKFQ8JDGTQ7VHFI41FDEFTDLBVQEAE4ER49QV2GTGVFDNBO31BPOA3OQN6879OT6MTU3G=" diff --git a/models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/webauthn_credential.yml b/models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/webauthn_credential.yml new file mode 100644 index 000000000000..c02a76e3742a --- /dev/null +++ b/models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/webauthn_credential.yml @@ -0,0 +1,31 @@ +- + id: 1 + lower_name: "u2fkey-correctly-migrated" + name: "u2fkey-correctly-migrated" + user_id: 1 + credential_id: "TVHE44TOH7DF7V48SEAIT3EMMJ7TGBOQ289E5AQB34S98LFCUFJ7U2NAVI8RJG6K2F4TC8AQ8KBNO7AGEOQOL9NE43GR63HTEHJSLOG=" + public_key: 0x040d0967a2cad045011631187576492a0beb5b377954b4f694c5afc8bdf25270f87f09a9ab6ce9c282f447ba71b2f2bae2105b32b847e0704f310f48644e3eddf2 + attestation_type: 'fido-u2f' + sign_count: 1 + clone_warning: false +- + id: 2 + lower_name: "non-u2f-key" + name: "non-u2f-key" + user_id: 1 + credential_id: "051CLMMKB62S6M9M2A4H54K7MMCQALFJ36G4TGB2S9A47APLTILU6C6744CEBG4EKCGV357N21BSLH8JD33GQMFAR6DQ70S76P34J6FR" + public_key: 0x040d0967a2cad045011631187576492a0beb5b377954b4f694c5afc8bdf25270f87f09a9ab6ce9c282f447ba71b2f2bae2105b32b847e0704f310f48644e3eddf2 + attestation_type: 'none' + sign_count: 1 + clone_warning: false +- + id: 4 + lower_name: "packed-key" + name: "packed-key" + user_id: 1 + credential_id: "APU4B1NDTEVTEM60V4T0FRL7SRJMO9KIE2AKFQ8JDGTQ7VHFI41FDEFTDLBVQEAE4ER49QV2GTGVFDNBO31BPOA3OQN6879OT6MTU3G=" + public_key: 0x040d0967a2cad045011631187576492a0beb5b377954b4f694c5afc8bdf25270f87f09a9ab6ce9c282f447ba71b2f2bae2105b32b847e0704f310f48644e3eddf2 + attestation_type: 'fido-u2f' + sign_count: 1 + clone_warning: false + diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 876ea7f29df1..1673c718b3e9 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -401,7 +401,7 @@ var migrations = []Migration{ // v220 -> v221 NewMigration("Add container repository property", addContainerRepositoryProperty), // v221 -> v222 - NewMigration("Increase WebAuthentication CredentialID size to 1640", increaseCredentialIDTo1640), + NewMigration("Store WebAuthentication CredentialID as bytes and increase size to at least 1024", storeWebauthnCredentialIDAsBytes), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v210.go b/models/migrations/v210.go index 92932372f63a..f32fae77bacc 100644 --- a/models/migrations/v210.go +++ b/models/migrations/v210.go @@ -25,7 +25,7 @@ func remigrateU2FCredentials(x *xorm.Engine) error { Name string LowerName string `xorm:"unique(s)"` UserID int64 `xorm:"INDEX unique(s)"` - CredentialID string `xorm:"INDEX VARCHAR(1640)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 -> 1640 base32 encoding + CredentialID string `xorm:"INDEX VARCHAR(410)"` // CredentalID in U2F is at most 255bytes / 5 * 8 = 408 - add a few extra characters for safety PublicKey []byte AttestationType string AAGUID []byte @@ -40,12 +40,12 @@ func remigrateU2FCredentials(x *xorm.Engine) error { switch x.Dialect().URI().DBType { case schemas.MYSQL: - _, err := x.Exec("ALTER TABLE webauthn_credential MODIFY COLUMN credential_id VARCHAR(1640)") + _, err := x.Exec("ALTER TABLE webauthn_credential MODIFY COLUMN credential_id VARCHAR(410)") if err != nil { return err } case schemas.ORACLE: - _, err := x.Exec("ALTER TABLE webauthn_credential MODIFY credential_id VARCHAR(1640)") + _, err := x.Exec("ALTER TABLE webauthn_credential MODIFY credential_id VARCHAR(410)") if err != nil { return err } @@ -70,7 +70,7 @@ func remigrateU2FCredentials(x *xorm.Engine) error { return err } case schemas.POSTGRES: - _, err := x.Exec("ALTER TABLE webauthn_credential ALTER COLUMN credential_id TYPE VARCHAR(1640)") + _, err := x.Exec("ALTER TABLE webauthn_credential ALTER COLUMN credential_id TYPE VARCHAR(410)") if err != nil { return err } diff --git a/models/migrations/v210_test.go b/models/migrations/v210_test.go index d621a39bc29a..70dbe61b06eb 100644 --- a/models/migrations/v210_test.go +++ b/models/migrations/v210_test.go @@ -20,7 +20,7 @@ func Test_remigrateU2FCredentials(t *testing.T) { Name string LowerName string `xorm:"unique(s)"` UserID int64 `xorm:"INDEX unique(s)"` - CredentialID string `xorm:"INDEX VARCHAR(1640)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 -> 1640 base32 encoding + CredentialID string `xorm:"INDEX VARCHAR(410)"` // CredentalID in U2F is at most 255bytes / 5 * 8 = 408 - add a few extra characters for safety PublicKey []byte AttestationType string SignCount uint32 `xorm:"BIGINT"` @@ -40,7 +40,7 @@ func Test_remigrateU2FCredentials(t *testing.T) { type ExpectedWebauthnCredential struct { ID int64 `xorm:"pk autoincr"` - CredentialID string `xorm:"INDEX VARCHAR(1640)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 -> 1640 base32 encoding + CredentialID string `xorm:"INDEX VARCHAR(410)"` // CredentalID in U2F is at most 255bytes / 5 * 8 = 408 - add a few extra characters for safety } // Prepare and load the testing database diff --git a/models/migrations/v221.go b/models/migrations/v221.go index 5c610fd3ff35..3fdad3e78bdc 100644 --- a/models/migrations/v221.go +++ b/models/migrations/v221.go @@ -5,71 +5,78 @@ package migrations import ( + "encoding/base32" + "fmt" + "code.gitea.io/gitea/modules/timeutil" "xorm.io/xorm" - "xorm.io/xorm/schemas" ) -func increaseCredentialIDTo1640(x *xorm.Engine) error { +func storeWebauthnCredentialIDAsBytes(x *xorm.Engine) error { // Create webauthnCredential table type webauthnCredential struct { - ID int64 `xorm:"pk autoincr"` - Name string - LowerName string `xorm:"unique(s)"` - UserID int64 `xorm:"INDEX unique(s)"` - CredentialID string `xorm:"INDEX VARCHAR(1640)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 -> 1640 base32 encoding - PublicKey []byte - AttestationType string - AAGUID []byte - SignCount uint32 `xorm:"BIGINT"` - CloneWarning bool - CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` - UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` + ID int64 `xorm:"pk autoincr"` + Name string + LowerName string `xorm:"unique(s)"` + UserID int64 `xorm:"INDEX unique(s)"` + CredentialID string `xorm:"INDEX VARCHAR(410)"` + CredentialIDBytes []byte `xorm:"INDEX VARBINARY(1024)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 + PublicKey []byte + AttestationType string + AAGUID []byte + SignCount uint32 `xorm:"BIGINT"` + CloneWarning bool + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` } if err := x.Sync2(&webauthnCredential{}); err != nil { return err } - switch x.Dialect().URI().DBType { - case schemas.MYSQL: - _, err := x.Exec("ALTER TABLE webauthn_credential MODIFY COLUMN credential_id VARCHAR(1640)") + var start int + creds := make([]*webauthnCredential, 0, 50) + for { + err := x.Select("id, credential_id").OrderBy("id").Limit(50, start).Find(&creds) if err != nil { return err } - case schemas.ORACLE: - _, err := x.Exec("ALTER TABLE webauthn_credential MODIFY credential_id VARCHAR(1640)") + + err = func() error { + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + return fmt.Errorf("unable to allow start session. Error: %w", err) + } + for _, cred := range creds { + cred.CredentialIDBytes, err = base32.HexEncoding.DecodeString(cred.CredentialID) + if err != nil { + return fmt.Errorf("unable to parse credential id %s for credential[%d]: %w", cred.CredentialID, cred.ID, err) + } + count, err := sess.ID(cred.ID).Cols("credential_id_bytes").Update(cred) + if count != 1 || err != nil { + return fmt.Errorf("unable to update credential id bytes for credential[%d]: %d,%w", cred.ID, count, err) + } + } + return sess.Commit() + }() if err != nil { return err } - case schemas.MSSQL: - // This column has an index on it. I could write all of the code to attempt to change the index OR - // I could just use recreate table. - sess := x.NewSession() - if err := sess.Begin(); err != nil { - _ = sess.Close() - return err - } - if err := recreateTable(sess, new(webauthnCredential)); err != nil { - _ = sess.Close() - return err - } - if err := sess.Commit(); err != nil { - _ = sess.Close() - return err + if len(creds) < 50 { + break } - if err := sess.Close(); err != nil { - return err - } - case schemas.POSTGRES: - _, err := x.Exec("ALTER TABLE webauthn_credential ALTER COLUMN credential_id TYPE VARCHAR(1640)") - if err != nil { - return err - } - default: - // SQLite doesn't support ALTER COLUMN, and it already makes String _TEXT_ by default so no migration needed - // nor is there any need to re-migrate + start += 50 + creds = creds[:0] + } + + // Drop the old credential ID + sess := x.NewSession() + defer sess.Close() + + if err := dropTableColumns(sess, "webauthn_credential", "credential_id"); err != nil { + return fmt.Errorf("unable to drop old credentialID column: %w", err) } - return nil + return sess.Commit() } diff --git a/models/migrations/v221_test.go b/models/migrations/v221_test.go new file mode 100644 index 000000000000..b0a3250550a2 --- /dev/null +++ b/models/migrations/v221_test.go @@ -0,0 +1,61 @@ +// Copyright 2022 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" + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_storeWebauthnCredentialIDAsBytes(t *testing.T) { + // Create webauthnCredential table + type WebauthnCredential struct { + ID int64 `xorm:"pk autoincr"` + Name string + LowerName string `xorm:"unique(s)"` + UserID int64 `xorm:"INDEX unique(s)"` + CredentialID string `xorm:"INDEX VARCHAR(410)"` + PublicKey []byte + AttestationType string + AAGUID []byte + SignCount uint32 `xorm:"BIGINT"` + CloneWarning bool + } + + type ExpectedWebauthnCredential struct { + ID int64 `xorm:"pk autoincr"` + CredentialIDBytes []byte `xorm:"INDEX VARBINARY(1024)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 + } + + // Prepare and load the testing database + x, deferable := prepareTestEnv(t, 0, new(WebauthnCredential), new(ExpectedWebauthnCredential)) + if x == nil || t.Failed() { + defer deferable() + return + } + defer deferable() + + if err := storeWebauthnCredentialIDAsBytes(x); err != nil { + assert.NoError(t, err) + return + } + + expected := []ExpectedWebauthnCredential{} + if err := x.Table("expected_webauthn_credential").Asc("id").Find(&expected); !assert.NoError(t, err) { + return + } + + got := []ExpectedWebauthnCredential{} + if err := x.Table("webauthn_credential").Select("id, credential_id_bytes").Asc("id").Find(&got); !assert.NoError(t, err) { + return + } + + for i, e := range expected { + credIDBytes, _ := base32.HexEncoding.DecodeString(string(e.CredentialIDBytes)) + assert.Equal(t, credIDBytes, got[i].CredentialIDBytes) + } +} From 919d5172530537c5bdecc80a036791060a0f911f Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 29 Jul 2022 20:10:26 +0100 Subject: [PATCH 03/12] fix test Signed-off-by: Andrew Thornton --- models/auth/webauthn_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/models/auth/webauthn_test.go b/models/auth/webauthn_test.go index 216bf110806e..59ea3e67db99 100644 --- a/models/auth/webauthn_test.go +++ b/models/auth/webauthn_test.go @@ -5,7 +5,6 @@ package auth import ( - "encoding/base32" "testing" "code.gitea.io/gitea/models/unittest" @@ -61,9 +60,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 := base32.HexEncoding.DecodeString(res.CredentialID) - assert.NoError(t, err) - assert.Equal(t, []byte("Test"), bs) + assert.Equal(t, []byte("Test"), []byte(res.CredentialIDBytes)) unittest.AssertExistsIf(t, true, &WebAuthnCredential{Name: "WebAuthn Created Credential", UserID: 1}) } From 248b5ce24ade8600acee1dd217a42ae9f9616018 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 29 Jul 2022 20:18:31 +0100 Subject: [PATCH 04/12] i hate our linter Signed-off-by: Andrew Thornton --- models/auth/webauthn_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/auth/webauthn_test.go b/models/auth/webauthn_test.go index 59ea3e67db99..293ba52eec1f 100644 --- a/models/auth/webauthn_test.go +++ b/models/auth/webauthn_test.go @@ -60,7 +60,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) - assert.Equal(t, []byte("Test"), []byte(res.CredentialIDBytes)) + assert.Equal(t, []byte("Test"), res.CredentialIDBytes) unittest.AssertExistsIf(t, true, &WebAuthnCredential{Name: "WebAuthn Created Credential", UserID: 1}) } From 72cd759bf43a7572e5a4e0cb14e2920e17c57202 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 29 Jul 2022 20:40:51 +0100 Subject: [PATCH 05/12] of course MSSQL has to be difficult Signed-off-by: Andrew Thornton --- .../expected_webauthn_credential.yml | 6 +++--- models/migrations/v221_test.go | 9 +++++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/expected_webauthn_credential.yml b/models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/expected_webauthn_credential.yml index f68b3cd24c38..55a237a0d633 100644 --- a/models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/expected_webauthn_credential.yml +++ b/models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/expected_webauthn_credential.yml @@ -1,9 +1,9 @@ - id: 1 - credential_id_bytes: "TVHE44TOH7DF7V48SEAIT3EMMJ7TGBOQ289E5AQB34S98LFCUFJ7U2NAVI8RJG6K2F4TC8AQ8KBNO7AGEOQOL9NE43GR63HTEHJSLOG=" + credential_id: "TVHE44TOH7DF7V48SEAIT3EMMJ7TGBOQ289E5AQB34S98LFCUFJ7U2NAVI8RJG6K2F4TC8AQ8KBNO7AGEOQOL9NE43GR63HTEHJSLOG=" - id: 2 - credential_id_bytes: "051CLMMKB62S6M9M2A4H54K7MMCQALFJ36G4TGB2S9A47APLTILU6C6744CEBG4EKCGV357N21BSLH8JD33GQMFAR6DQ70S76P34J6FR=" + credential_id: "051CLMMKB62S6M9M2A4H54K7MMCQALFJ36G4TGB2S9A47APLTILU6C6744CEBG4EKCGV357N21BSLH8JD33GQMFAR6DQ70S76P34J6FR=" - id: 4 - credential_id_bytes: "APU4B1NDTEVTEM60V4T0FRL7SRJMO9KIE2AKFQ8JDGTQ7VHFI41FDEFTDLBVQEAE4ER49QV2GTGVFDNBO31BPOA3OQN6879OT6MTU3G=" + credential_id: "APU4B1NDTEVTEM60V4T0FRL7SRJMO9KIE2AKFQ8JDGTQ7VHFI41FDEFTDLBVQEAE4ER49QV2GTGVFDNBO31BPOA3OQN6879OT6MTU3G=" diff --git a/models/migrations/v221_test.go b/models/migrations/v221_test.go index b0a3250550a2..0f53ba8a4922 100644 --- a/models/migrations/v221_test.go +++ b/models/migrations/v221_test.go @@ -27,6 +27,11 @@ func Test_storeWebauthnCredentialIDAsBytes(t *testing.T) { } type ExpectedWebauthnCredential struct { + ID int64 `xorm:"pk autoincr"` + CredentialID string // CredentialID is at most 1023 bytes as per spec released 20 July 2022 + } + + type ConvertedWebauthnCredential struct { ID int64 `xorm:"pk autoincr"` CredentialIDBytes []byte `xorm:"INDEX VARBINARY(1024)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 } @@ -49,13 +54,13 @@ func Test_storeWebauthnCredentialIDAsBytes(t *testing.T) { return } - got := []ExpectedWebauthnCredential{} + got := []ConvertedWebauthnCredential{} if err := x.Table("webauthn_credential").Select("id, credential_id_bytes").Asc("id").Find(&got); !assert.NoError(t, err) { return } for i, e := range expected { - credIDBytes, _ := base32.HexEncoding.DecodeString(string(e.CredentialIDBytes)) + credIDBytes, _ := base32.HexEncoding.DecodeString(string(e.CredentialID)) assert.Equal(t, credIDBytes, got[i].CredentialIDBytes) } } From 25388aadcf33a2f1e40734a92367213c7765e9a6 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 29 Jul 2022 20:54:03 +0100 Subject: [PATCH 06/12] i hate our linter Signed-off-by: Andrew Thornton --- models/migrations/v221_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/v221_test.go b/models/migrations/v221_test.go index 0f53ba8a4922..ddd840f06e9e 100644 --- a/models/migrations/v221_test.go +++ b/models/migrations/v221_test.go @@ -60,7 +60,7 @@ func Test_storeWebauthnCredentialIDAsBytes(t *testing.T) { } for i, e := range expected { - credIDBytes, _ := base32.HexEncoding.DecodeString(string(e.CredentialID)) + credIDBytes, _ := base32.HexEncoding.DecodeString(e.CredentialID) assert.Equal(t, credIDBytes, got[i].CredentialIDBytes) } } From 3eb1d13e7303cae948bdc1fb18faffb878e03ad6 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 29 Jul 2022 21:19:42 +0100 Subject: [PATCH 07/12] and change the lookup function Signed-off-by: Andrew Thornton --- models/auth/webauthn.go | 11 ++++++----- routers/web/auth/webauthn.go | 3 +-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/models/auth/webauthn.go b/models/auth/webauthn.go index af7086c24de0..40206653b49e 100644 --- a/models/auth/webauthn.go +++ b/models/auth/webauthn.go @@ -6,6 +6,7 @@ package auth import ( "context" + "encoding/base32" "fmt" "strings" @@ -19,14 +20,14 @@ import ( // ErrWebAuthnCredentialNotExist represents a "ErrWebAuthnCRedentialNotExist" kind of error. type ErrWebAuthnCredentialNotExist struct { ID int64 - CredentialID string + CredentialID []byte } func (err ErrWebAuthnCredentialNotExist) Error() string { - if err.CredentialID == "" { + if len(err.CredentialID) == 0 { return fmt.Sprintf("WebAuthn credential does not exist [id: %d]", err.ID) } - return fmt.Sprintf("WebAuthn credential does not exist [credential_id: %s]", err.CredentialID) + return fmt.Sprintf("WebAuthn credential does not exist [credential_id: %s]", base32.HexEncoding.EncodeToString(err.CredentialID)) } // IsErrWebAuthnCredentialNotExist checks if an error is a ErrWebAuthnCredentialNotExist. @@ -162,11 +163,11 @@ func HasWebAuthnRegistrationsByUID(uid int64) (bool, error) { } // GetWebAuthnCredentialByCredID returns WebAuthn credential by credential ID -func GetWebAuthnCredentialByCredID(userID int64, credID string) (*WebAuthnCredential, error) { +func GetWebAuthnCredentialByCredID(userID int64, credID []byte) (*WebAuthnCredential, error) { return getWebAuthnCredentialByCredID(db.DefaultContext, userID, credID) } -func getWebAuthnCredentialByCredID(ctx context.Context, userID int64, credID string) (*WebAuthnCredential, error) { +func getWebAuthnCredentialByCredID(ctx context.Context, userID int64, credID []byte) (*WebAuthnCredential, error) { cred := new(WebAuthnCredential) if found, err := db.GetEngine(ctx).Where("user_id = ? AND credential_id = ?", userID, credID).Get(cred); err != nil { return nil, err diff --git a/routers/web/auth/webauthn.go b/routers/web/auth/webauthn.go index 4778c9a9a369..917cbdd57bf6 100644 --- a/routers/web/auth/webauthn.go +++ b/routers/web/auth/webauthn.go @@ -5,7 +5,6 @@ package auth import ( - "encoding/base32" "errors" "net/http" @@ -129,7 +128,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(user.ID, base32.HexEncoding.EncodeToString(cred.ID)) + dbCred, err := auth.GetWebAuthnCredentialByCredID(user.ID, cred.ID) if err != nil { ctx.ServerError("GetWebAuthnCredentialByCredID", err) return From da04731be758166ba9d01a21e24bd18bc330aa2f Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 29 Jul 2022 21:21:14 +0100 Subject: [PATCH 08/12] and change the lookup function Signed-off-by: Andrew Thornton --- models/auth/webauthn.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/auth/webauthn.go b/models/auth/webauthn.go index 40206653b49e..7383ca462064 100644 --- a/models/auth/webauthn.go +++ b/models/auth/webauthn.go @@ -169,7 +169,7 @@ func GetWebAuthnCredentialByCredID(userID int64, credID []byte) (*WebAuthnCreden func getWebAuthnCredentialByCredID(ctx context.Context, userID int64, credID []byte) (*WebAuthnCredential, error) { cred := new(WebAuthnCredential) - if found, err := db.GetEngine(ctx).Where("user_id = ? AND credential_id = ?", userID, credID).Get(cred); err != nil { + if found, err := db.GetEngine(ctx).Where("user_id = ? AND credential_id_bytes = ?", userID, credID).Get(cred); err != nil { return nil, err } else if !found { return nil, ErrWebAuthnCredentialNotExist{CredentialID: credID} From 9f15fedc13f4f6507364f8c8c3e759ed8b1bba6d Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 29 Jul 2022 21:52:10 +0100 Subject: [PATCH 09/12] separate the out the droptablecolumns into another migration Signed-off-by: Andrew Thornton --- models/auth/webauthn.go | 44 +++++++------- models/auth/webauthn_test.go | 2 +- models/migrations/migrations.go | 4 ++ models/migrations/v221.go | 23 +++---- models/migrations/v221_test.go | 2 +- models/migrations/v222.go | 64 ++++++++++++++++++++ models/migrations/v223.go | 103 ++++++++++++++++++++++++++++++++ 7 files changed, 203 insertions(+), 39 deletions(-) create mode 100644 models/migrations/v222.go create mode 100644 models/migrations/v223.go diff --git a/models/auth/webauthn.go b/models/auth/webauthn.go index 7383ca462064..d2617481212b 100644 --- a/models/auth/webauthn.go +++ b/models/auth/webauthn.go @@ -39,18 +39,18 @@ func IsErrWebAuthnCredentialNotExist(err error) bool { // WebAuthnCredential represents the WebAuthn credential data for a public-key // credential conformant to WebAuthn Level 1 type WebAuthnCredential struct { - ID int64 `xorm:"pk autoincr"` - Name string - LowerName string `xorm:"unique(s)"` - UserID int64 `xorm:"INDEX unique(s)"` - CredentialIDBytes []byte `xorm:"INDEX VARBINARY(1024)"` - PublicKey []byte - AttestationType string - AAGUID []byte - SignCount uint32 `xorm:"BIGINT"` - CloneWarning bool - CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` - UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` + ID int64 `xorm:"pk autoincr"` + Name string + LowerName string `xorm:"unique(s)"` + UserID int64 `xorm:"INDEX unique(s)"` + CredentialID []byte `xorm:"INDEX VARBINARY(1024)"` + PublicKey []byte + AttestationType string + AAGUID []byte + SignCount uint32 `xorm:"BIGINT"` + CloneWarning bool + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` } func init() { @@ -95,7 +95,7 @@ func (list WebAuthnCredentialList) ToCredentials() []webauthn.Credential { creds := make([]webauthn.Credential, 0, len(list)) for _, cred := range list { creds = append(creds, webauthn.Credential{ - ID: cred.CredentialIDBytes, + ID: cred.CredentialID, PublicKey: cred.PublicKey, AttestationType: cred.AttestationType, Authenticator: webauthn.Authenticator{ @@ -169,7 +169,7 @@ func GetWebAuthnCredentialByCredID(userID int64, credID []byte) (*WebAuthnCreden func getWebAuthnCredentialByCredID(ctx context.Context, userID int64, credID []byte) (*WebAuthnCredential, error) { cred := new(WebAuthnCredential) - if found, err := db.GetEngine(ctx).Where("user_id = ? AND credential_id_bytes = ?", userID, 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} @@ -184,14 +184,14 @@ func CreateCredential(userID int64, name string, cred *webauthn.Credential) (*We func createCredential(ctx context.Context, userID int64, name string, cred *webauthn.Credential) (*WebAuthnCredential, error) { c := &WebAuthnCredential{ - UserID: userID, - Name: name, - CredentialIDBytes: cred.ID, - PublicKey: cred.PublicKey, - AttestationType: cred.AttestationType, - AAGUID: cred.Authenticator.AAGUID, - SignCount: cred.Authenticator.SignCount, - CloneWarning: false, + UserID: userID, + Name: name, + CredentialID: cred.ID, + PublicKey: cred.PublicKey, + AttestationType: cred.AttestationType, + AAGUID: cred.Authenticator.AAGUID, + SignCount: cred.Authenticator.SignCount, + CloneWarning: false, } if err := db.Insert(ctx, c); err != nil { diff --git a/models/auth/webauthn_test.go b/models/auth/webauthn_test.go index 293ba52eec1f..cc39691ce2d3 100644 --- a/models/auth/webauthn_test.go +++ b/models/auth/webauthn_test.go @@ -60,7 +60,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) - assert.Equal(t, []byte("Test"), res.CredentialIDBytes) + assert.Equal(t, []byte("Test"), res.CredentialID) unittest.AssertExistsIf(t, true, &WebAuthnCredential{Name: "WebAuthn Created Credential", UserID: 1}) } diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 1673c718b3e9..2719f45efbbb 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -402,6 +402,10 @@ var migrations = []Migration{ NewMigration("Add container repository property", addContainerRepositoryProperty), // v221 -> v222 NewMigration("Store WebAuthentication CredentialID as bytes and increase size to at least 1024", storeWebauthnCredentialIDAsBytes), + // v222 -> v223 + NewMigration("Drop old CredentialID column", dropOldCredentialIDColumn), + // v223 -> v224 + NewMigration("Rename CredentialIDBytes column to CredentialID", renameCredentialIDBytes), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v221.go b/models/migrations/v221.go index 3fdad3e78bdc..f3bcfcdf1de2 100644 --- a/models/migrations/v221.go +++ b/models/migrations/v221.go @@ -16,12 +16,13 @@ import ( func storeWebauthnCredentialIDAsBytes(x *xorm.Engine) error { // Create webauthnCredential table type webauthnCredential struct { - ID int64 `xorm:"pk autoincr"` - Name string - LowerName string `xorm:"unique(s)"` - UserID int64 `xorm:"INDEX unique(s)"` - CredentialID string `xorm:"INDEX VARCHAR(410)"` - CredentialIDBytes []byte `xorm:"INDEX VARBINARY(1024)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 + ID int64 `xorm:"pk autoincr"` + Name string + LowerName string `xorm:"unique(s)"` + UserID int64 `xorm:"INDEX unique(s)"` + CredentialID string `xorm:"INDEX VARCHAR(410)"` + // Note the lack of INDEX here - these will be created once the column is renamed in v223.go + CredentialIDBytes []byte `xorm:"VARBINARY(1024)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 PublicKey []byte AttestationType string AAGUID []byte @@ -70,13 +71,5 @@ func storeWebauthnCredentialIDAsBytes(x *xorm.Engine) error { start += 50 creds = creds[:0] } - - // Drop the old credential ID - sess := x.NewSession() - defer sess.Close() - - if err := dropTableColumns(sess, "webauthn_credential", "credential_id"); err != nil { - return fmt.Errorf("unable to drop old credentialID column: %w", err) - } - return sess.Commit() + return nil } diff --git a/models/migrations/v221_test.go b/models/migrations/v221_test.go index ddd840f06e9e..cd20bb0fde92 100644 --- a/models/migrations/v221_test.go +++ b/models/migrations/v221_test.go @@ -33,7 +33,7 @@ func Test_storeWebauthnCredentialIDAsBytes(t *testing.T) { type ConvertedWebauthnCredential struct { ID int64 `xorm:"pk autoincr"` - CredentialIDBytes []byte `xorm:"INDEX VARBINARY(1024)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 + CredentialIDBytes []byte `xorm:"VARBINARY(1024)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 } // Prepare and load the testing database diff --git a/models/migrations/v222.go b/models/migrations/v222.go new file mode 100644 index 000000000000..99acdfd20608 --- /dev/null +++ b/models/migrations/v222.go @@ -0,0 +1,64 @@ +// Copyright 2022 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 ( + "context" + "fmt" + + "code.gitea.io/gitea/modules/timeutil" + + "xorm.io/xorm" +) + +func dropOldCredentialIDColumn(x *xorm.Engine) error { + // This migration maybe rerun so that we should check if it has been run + credentialIDExist, err := x.Dialect().IsColumnExist(x.DB(), context.Background(), "webauthn_credential", "credential_id") + if err != nil { + return err + } + if !credentialIDExist { + // Column is already non-extant + return nil + } + credentialIDBytesExists, err := x.Dialect().IsColumnExist(x.DB(), context.Background(), "webauthn_credential", "credential_id_bytes") + if err != nil { + return err + } + if !credentialIDBytesExists { + // looks like 221 hasn't properly run + return fmt.Errorf("webauthn_credential does not have a credential_id_bytes column... it is not safe to run this migration") + } + + // Create webauthnCredential table + type webauthnCredential struct { + ID int64 `xorm:"pk autoincr"` + Name string + LowerName string `xorm:"unique(s)"` + UserID int64 `xorm:"INDEX unique(s)"` + CredentialID string `xorm:"INDEX VARCHAR(410)"` + // Note the lack of the INDEX on CredentialIDBytes - we will add this in v223.go + CredentialIDBytes []byte `xorm:"VARBINARY(1024)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 + PublicKey []byte + AttestationType string + AAGUID []byte + SignCount uint32 `xorm:"BIGINT"` + CloneWarning bool + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` + } + if err := x.Sync2(&webauthnCredential{}); err != nil { + return err + } + + // Drop the old credential ID + sess := x.NewSession() + defer sess.Close() + + if err := dropTableColumns(sess, "webauthn_credential", "credential_id"); err != nil { + return fmt.Errorf("unable to drop old credentialID column: %w", err) + } + return sess.Commit() +} diff --git a/models/migrations/v223.go b/models/migrations/v223.go new file mode 100644 index 000000000000..430dd7aecea0 --- /dev/null +++ b/models/migrations/v223.go @@ -0,0 +1,103 @@ +// Copyright 2022 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 ( + "context" + "fmt" + + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/timeutil" + + "xorm.io/xorm" +) + +func renameCredentialIDBytes(x *xorm.Engine) error { + // This migration maybe rerun so that we should check if it has been run + credentialIDExist, err := x.Dialect().IsColumnExist(x.DB(), context.Background(), "webauthn_credential", "credential_id") + if err != nil { + return err + } + if !credentialIDExist { + credentialIDBytesExists, err := x.Dialect().IsColumnExist(x.DB(), context.Background(), "webauthn_credential", "credential_id_bytes") + if err != nil { + return err + } + if !credentialIDBytesExists { + return nil + } + } + + err = func() error { + // webauthnCredential table + type webauthnCredential struct { + ID int64 `xorm:"pk autoincr"` + Name string + LowerName string `xorm:"unique(s)"` + UserID int64 `xorm:"INDEX unique(s)"` + // Note the lack of INDEX here + CredentialIDBytes []byte `xorm:"VARBINARY(1024)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 + PublicKey []byte + AttestationType string + AAGUID []byte + SignCount uint32 `xorm:"BIGINT"` + CloneWarning bool + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` + } + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + return err + } + + if err := sess.Sync2(new(webauthnCredential)); err != nil { + return fmt.Errorf("error on Sync2: %v", err) + } + + if credentialIDExist { + // if both errors and message exist, drop message at first + if err := dropTableColumns(sess, "webauthn_credential", "credential_id"); err != nil { + return err + } + } + + switch { + case setting.Database.UseMySQL: + if _, err := sess.Exec("ALTER TABLE `webauthn_credential` CHANGE credential_id_bytes credential_id VARBINARY(1024)"); err != nil { + return err + } + case setting.Database.UseMSSQL: + if _, err := sess.Exec("sp_rename 'webauthn_credential.credential_id_bytes', 'credential_id', 'COLUMN'"); err != nil { + return err + } + default: + if _, err := sess.Exec("ALTER TABLE `webauthn_credential` RENAME COLUMN credential_id_bytes TO credential_id"); err != nil { + return err + } + } + return sess.Commit() + }() + if err != nil { + return err + } + + // Create webauthnCredential table + type webauthnCredential struct { + ID int64 `xorm:"pk autoincr"` + Name string + LowerName string `xorm:"unique(s)"` + UserID int64 `xorm:"INDEX unique(s)"` + CredentialID []byte `xorm:"INDEX VARBINARY(1024)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 + PublicKey []byte + AttestationType string + AAGUID []byte + SignCount uint32 `xorm:"BIGINT"` + CloneWarning bool + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` + } + return x.Sync2(&webauthnCredential{}) +} From 0e675c92d8c66bbf9ef3bae3ef5d30f596de3cba Mon Sep 17 00:00:00 2001 From: zeripath Date: Fri, 29 Jul 2022 21:57:10 +0100 Subject: [PATCH 10/12] Update models/migrations/v223.go --- models/migrations/v223.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/v223.go b/models/migrations/v223.go index 430dd7aecea0..d7ee4812b826 100644 --- a/models/migrations/v223.go +++ b/models/migrations/v223.go @@ -20,7 +20,7 @@ func renameCredentialIDBytes(x *xorm.Engine) error { if err != nil { return err } - if !credentialIDExist { + if credentialIDExist { credentialIDBytesExists, err := x.Dialect().IsColumnExist(x.DB(), context.Background(), "webauthn_credential", "credential_id_bytes") if err != nil { return err From 7926dacc3687c0a11576c6b57d152b168fbf1a31 Mon Sep 17 00:00:00 2001 From: zeripath Date: Fri, 29 Jul 2022 22:41:15 +0100 Subject: [PATCH 11/12] Update webauthn.go --- models/auth/webauthn.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/models/auth/webauthn.go b/models/auth/webauthn.go index d2617481212b..d3062342f545 100644 --- a/models/auth/webauthn.go +++ b/models/auth/webauthn.go @@ -6,7 +6,6 @@ package auth import ( "context" - "encoding/base32" "fmt" "strings" @@ -27,7 +26,7 @@ func (err ErrWebAuthnCredentialNotExist) Error() string { if len(err.CredentialID) == 0 { return fmt.Sprintf("WebAuthn credential does not exist [id: %d]", err.ID) } - return fmt.Sprintf("WebAuthn credential does not exist [credential_id: %s]", base32.HexEncoding.EncodeToString(err.CredentialID)) + return fmt.Sprintf("WebAuthn credential does not exist [credential_id: %x]", err.CredentialID) } // IsErrWebAuthnCredentialNotExist checks if an error is a ErrWebAuthnCredentialNotExist. From 197c197400bd7db6379fcb6dc32522d23f20aac4 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 30 Jul 2022 09:44:55 +0200 Subject: [PATCH 12/12] nit --- models/migrations/v221_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/models/migrations/v221_test.go b/models/migrations/v221_test.go index cd20bb0fde92..c50ca5c87329 100644 --- a/models/migrations/v221_test.go +++ b/models/migrations/v221_test.go @@ -38,11 +38,10 @@ func Test_storeWebauthnCredentialIDAsBytes(t *testing.T) { // Prepare and load the testing database x, deferable := prepareTestEnv(t, 0, new(WebauthnCredential), new(ExpectedWebauthnCredential)) + defer deferable() if x == nil || t.Failed() { - defer deferable() return } - defer deferable() if err := storeWebauthnCredentialIDAsBytes(x); err != nil { assert.NoError(t, err)