Skip to content

Commit 8c142c3

Browse files
committed
Attempt to fix the webauthn migration again - part 3
MariaDB now asserts that 408 characters is too long for a VARCHAR(410) I have no words. See #18756 Signed-off-by: Andrew Thornton <[email protected]>
1 parent 5348e19 commit 8c142c3

File tree

10 files changed

+157
-138
lines changed

10 files changed

+157
-138
lines changed

models/auth/webauthn.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ type WebAuthnCredential struct {
4343
Name string
4444
LowerName string `xorm:"unique(s)"`
4545
UserID int64 `xorm:"INDEX unique(s)"`
46-
CredentialID string `xorm:"INDEX"`
46+
CredentialID string `xorm:"INDEX VARCHAR(500)"`
4747
PublicKey []byte
4848
AttestationType string
4949
AAGUID []byte

models/migrations/migrations.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,9 @@ var migrations = []Migration{
371371
// v208 -> v209
372372
NewMigration("Use base32.HexEncoding instead of base64 encoding for cred ID as it is case insensitive", useBase32HexForCredIDInWebAuthnCredential),
373373
// v209 -> v210
374-
NewMigration("Increase WebAuthentication CredentialID size to 410", increaseCredentialIDTo410),
374+
NewMigration("Increase WebAuthentication CredentialID size to 410 - NOOPED", increaseCredentialIDTo410),
375+
// v210 -> v211
376+
NewMigration("Increase WebAuthentication CredentialID size to 500", increaseCredentialIDTo500),
375377
}
376378

377379
// GetCurrentDBVersion returns the current db version

models/migrations/v207.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func addWebAuthnCred(x *xorm.Engine) error {
2222
Name string
2323
LowerName string `xorm:"unique(s)"`
2424
UserID int64 `xorm:"INDEX unique(s)"`
25-
CredentialID string `xorm:"INDEX VARCHAR(410)"` // CredentalID in U2F is at most 255bytes / 5 * 8 = 408 - add a few extra characters for safety
25+
CredentialID string `xorm:"INDEX VARCHAR(500)"` // CredentalID in U2F is at most 255bytes / 5 * 8 = 408 - add a few extra characters for safety
2626
PublicKey []byte
2727
AttestationType string
2828
AAGUID []byte

models/migrations/v208.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ func useBase32HexForCredIDInWebAuthnCredential(x *xorm.Engine) error {
1515
// Create webauthnCredential table
1616
type webauthnCredential struct {
1717
ID int64 `xorm:"pk autoincr"`
18-
CredentialID string `xorm:"INDEX VARCHAR(410)"`
18+
CredentialID string `xorm:"INDEX VARCHAR(500)"`
1919
}
2020
if err := x.Sync2(&webauthnCredential{}); err != nil {
2121
return err

models/migrations/v209.go

Lines changed: 3 additions & 130 deletions
Original file line numberDiff line numberDiff line change
@@ -5,140 +5,13 @@
55
package migrations
66

77
import (
8-
"encoding/base32"
9-
"fmt"
10-
"strings"
11-
12-
"code.gitea.io/gitea/modules/timeutil"
13-
14-
"github.com/tstranex/u2f"
158
"xorm.io/xorm"
16-
"xorm.io/xorm/schemas"
179
)
1810

1911
func increaseCredentialIDTo410(x *xorm.Engine) error {
20-
// Create webauthnCredential table
21-
type webauthnCredential struct {
22-
ID int64 `xorm:"pk autoincr"`
23-
Name string
24-
LowerName string `xorm:"unique(s)"`
25-
UserID int64 `xorm:"INDEX unique(s)"`
26-
CredentialID string `xorm:"INDEX VARCHAR(410)"` // CredentalID in U2F is at most 255bytes / 5 * 8 = 408 - add a few extra characters for safety
27-
PublicKey []byte
28-
AttestationType string
29-
AAGUID []byte
30-
SignCount uint32 `xorm:"BIGINT"`
31-
CloneWarning bool
32-
CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
33-
UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
34-
}
35-
if err := x.Sync2(&webauthnCredential{}); err != nil {
36-
return err
37-
}
38-
39-
switch x.Dialect().URI().DBType {
40-
case schemas.MYSQL:
41-
_, err := x.Exec("ALTER TABLE webauthn_credential MODIFY COLUMN credential_id VARCHAR(410)")
42-
if err != nil {
43-
return err
44-
}
45-
case schemas.ORACLE:
46-
_, err := x.Exec("ALTER TABLE webauthn_credential MODIFY credential_id VARCHAR(410)")
47-
if err != nil {
48-
return err
49-
}
50-
case schemas.MSSQL:
51-
// This column has an index on it. I could write all of the code to attempt to change the index OR
52-
// I could just use recreate table.
53-
sess := x.NewSession()
54-
if err := sess.Begin(); err != nil {
55-
_ = sess.Close()
56-
return err
57-
}
58-
59-
if err := recreateTable(sess, new(webauthnCredential)); err != nil {
60-
_ = sess.Close()
61-
return err
62-
}
63-
if err := sess.Commit(); err != nil {
64-
_ = sess.Close()
65-
return err
66-
}
67-
if err := sess.Close(); err != nil {
68-
return err
69-
}
70-
case schemas.POSTGRES:
71-
_, err := x.Exec("ALTER TABLE webauthn_credential ALTER COLUMN credential_id TYPE VARCHAR(410)")
72-
if err != nil {
73-
return err
74-
}
75-
default:
76-
// SQLite doesn't support ALTER COLUMN, and it already makes String _TEXT_ by default so no migration needed
77-
// nor is there any need to re-migrate
78-
return nil
79-
}
80-
81-
exist, err := x.IsTableExist("u2f_registration")
82-
if err != nil {
83-
return err
84-
}
85-
if !exist {
86-
return nil
87-
}
88-
89-
// Now migrate the old u2f registrations to the new format
90-
type u2fRegistration struct {
91-
ID int64 `xorm:"pk autoincr"`
92-
Name string
93-
UserID int64 `xorm:"INDEX"`
94-
Raw []byte
95-
Counter uint32 `xorm:"BIGINT"`
96-
CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
97-
UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
98-
}
99-
100-
var start int
101-
regs := make([]*u2fRegistration, 0, 50)
102-
for {
103-
err := x.OrderBy("id").Limit(50, start).Find(&regs)
104-
if err != nil {
105-
return err
106-
}
107-
108-
for _, reg := range regs {
109-
parsed := new(u2f.Registration)
110-
err = parsed.UnmarshalBinary(reg.Raw)
111-
if err != nil {
112-
continue
113-
}
114-
115-
cred := &webauthnCredential{}
116-
has, err := x.ID(reg.ID).Where("id = ? AND user_id = ?", reg.ID, reg.UserID).Get(cred)
117-
if err != nil {
118-
return fmt.Errorf("unable to get webauthn_credential[%d]. Error: %v", reg.ID, err)
119-
}
120-
if !has {
121-
continue
122-
}
123-
remigratedCredID := base32.HexEncoding.EncodeToString(parsed.KeyHandle)
124-
if cred.CredentialID == remigratedCredID || (!strings.HasPrefix(remigratedCredID, cred.CredentialID) && cred.CredentialID != "") {
125-
continue
126-
}
127-
128-
cred.CredentialID = remigratedCredID
129-
130-
_, err = x.ID(cred.ID).Update(cred)
131-
if err != nil {
132-
return err
133-
}
134-
}
135-
136-
if len(regs) < 50 {
137-
break
138-
}
139-
start += 50
140-
regs = regs[:0]
141-
}
12+
// no-op
13+
// MariaDB asserts that 408 characters is too long for a VARCHAR(410) so this migration is clearly wrong.
14+
// So now we have to no-op again.
14215

14316
return nil
14417
}

models/migrations/v210.go

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
// Copyright 2022 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package migrations
6+
7+
import (
8+
"encoding/base32"
9+
"fmt"
10+
"strings"
11+
12+
"code.gitea.io/gitea/modules/timeutil"
13+
"github.com/tstranex/u2f"
14+
15+
"xorm.io/xorm"
16+
"xorm.io/xorm/schemas"
17+
)
18+
19+
// MariaDB asserts that 408 is too long for a VARCHAR(410) so now we need 500
20+
func increaseCredentialIDTo500(x *xorm.Engine) error {
21+
// Create webauthnCredential table
22+
type webauthnCredential struct {
23+
ID int64 `xorm:"pk autoincr"`
24+
Name string
25+
LowerName string `xorm:"unique(s)"`
26+
UserID int64 `xorm:"INDEX unique(s)"`
27+
CredentialID string `xorm:"INDEX VARCHAR(500)"` // CredentalID in U2F is at most 255bytes / 5 * 8 = 408 - add a few extra characters for safety
28+
PublicKey []byte
29+
AttestationType string
30+
AAGUID []byte
31+
SignCount uint32 `xorm:"BIGINT"`
32+
CloneWarning bool
33+
CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
34+
UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
35+
}
36+
if err := x.Sync2(&webauthnCredential{}); err != nil {
37+
return err
38+
}
39+
40+
switch x.Dialect().URI().DBType {
41+
case schemas.MYSQL:
42+
_, err := x.Exec("ALTER TABLE webauthn_credential MODIFY COLUMN credential_id VARCHAR(500)")
43+
if err != nil {
44+
return err
45+
}
46+
case schemas.ORACLE:
47+
_, err := x.Exec("ALTER TABLE webauthn_credential MODIFY credential_id VARCHAR(500)")
48+
if err != nil {
49+
return err
50+
}
51+
case schemas.MSSQL:
52+
// This column has an index on it. I could write all of the code to attempt to change the index OR
53+
// I could just use recreate table.
54+
sess := x.NewSession()
55+
if err := sess.Begin(); err != nil {
56+
_ = sess.Close()
57+
return err
58+
}
59+
60+
if err := recreateTable(sess, new(webauthnCredential)); err != nil {
61+
_ = sess.Close()
62+
return err
63+
}
64+
if err := sess.Commit(); err != nil {
65+
_ = sess.Close()
66+
return err
67+
}
68+
if err := sess.Close(); err != nil {
69+
return err
70+
}
71+
case schemas.POSTGRES:
72+
_, err := x.Exec("ALTER TABLE webauthn_credential ALTER COLUMN credential_id TYPE VARCHAR(500)")
73+
if err != nil {
74+
return err
75+
}
76+
default:
77+
// SQLite doesn't support ALTER COLUMN, and it already makes String _TEXT_ by default so no migration needed
78+
// nor is there any need to re-migrate
79+
}
80+
81+
exist, err := x.IsTableExist("u2f_registration")
82+
if err != nil {
83+
return err
84+
}
85+
if !exist {
86+
return nil
87+
}
88+
89+
// Now migrate the old u2f registrations to the new format
90+
type u2fRegistration struct {
91+
ID int64 `xorm:"pk autoincr"`
92+
Name string
93+
UserID int64 `xorm:"INDEX"`
94+
Raw []byte
95+
Counter uint32 `xorm:"BIGINT"`
96+
CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
97+
UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
98+
}
99+
100+
var start int
101+
regs := make([]*u2fRegistration, 0, 50)
102+
for {
103+
err := x.OrderBy("id").Limit(50, start).Find(&regs)
104+
if err != nil {
105+
return err
106+
}
107+
108+
for _, reg := range regs {
109+
parsed := new(u2f.Registration)
110+
err = parsed.UnmarshalBinary(reg.Raw)
111+
if err != nil {
112+
continue
113+
}
114+
115+
cred := &webauthnCredential{}
116+
has, err := x.ID(reg.ID).Where("id = ? AND user_id = ?", reg.ID, reg.UserID).Get(cred)
117+
if err != nil {
118+
return fmt.Errorf("unable to get webauthn_credential[%d]. Error: %v", reg.ID, err)
119+
}
120+
if !has {
121+
continue
122+
}
123+
remigratedCredID := base32.HexEncoding.EncodeToString(parsed.KeyHandle)
124+
if cred.CredentialID == remigratedCredID || (!strings.HasPrefix(remigratedCredID, cred.CredentialID) && cred.CredentialID != "") {
125+
continue
126+
}
127+
128+
cred.CredentialID = remigratedCredID
129+
130+
_, err = x.ID(cred.ID).Update(cred)
131+
if err != nil {
132+
return err
133+
}
134+
}
135+
136+
if len(regs) < 50 {
137+
break
138+
}
139+
start += 50
140+
regs = regs[:0]
141+
}
142+
143+
return nil
144+
}

models/migrations/v209_test.go renamed to models/migrations/v210_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,14 @@ import (
1212
"xorm.io/xorm/schemas"
1313
)
1414

15-
func Test_increaseCredentialIDTo410(t *testing.T) {
15+
func Test_increaseCredentialIDTo500(t *testing.T) {
1616
// Create webauthnCredential table
1717
type WebauthnCredential struct {
1818
ID int64 `xorm:"pk autoincr"`
1919
Name string
2020
LowerName string `xorm:"unique(s)"`
2121
UserID int64 `xorm:"INDEX unique(s)"`
22-
CredentialID string `xorm:"INDEX VARCHAR(410)"` // CredentalID in U2F is at most 255bytes / 5 * 8 = 408 - add a few extra characters for safety
22+
CredentialID string `xorm:"INDEX VARCHAR(500)"` // CredentalID in U2F is at most 255bytes / 5 * 8 = 408 - add a few extra characters for safety
2323
PublicKey []byte
2424
AttestationType string
2525
SignCount uint32 `xorm:"BIGINT"`
@@ -39,7 +39,7 @@ func Test_increaseCredentialIDTo410(t *testing.T) {
3939

4040
type ExpectedWebauthnCredential struct {
4141
ID int64 `xorm:"pk autoincr"`
42-
CredentialID string `xorm:"INDEX VARCHAR(410)"` // CredentalID in U2F is at most 255bytes / 5 * 8 = 408 - add a few extra characters for safety
42+
CredentialID string `xorm:"INDEX VARCHAR(500)"` // CredentalID in U2F is at most 255bytes / 5 * 8 = 408 - add a few extra characters for safety
4343
}
4444

4545
// Prepare and load the testing database
@@ -55,7 +55,7 @@ func Test_increaseCredentialIDTo410(t *testing.T) {
5555
}
5656

5757
// Run the migration
58-
if err := increaseCredentialIDTo410(x); err != nil {
58+
if err := increaseCredentialIDTo500(x); err != nil {
5959
assert.NoError(t, err)
6060
return
6161
}

0 commit comments

Comments
 (0)