fix: ignore orphaned two_factor failing upgrade to v10 with invalid decrypted base64

If a row in the two_factor table references a non existent user, it
may contain a secret that has an invalid format. Such an orphaned row
is never used and should be removed.

Instead of blocking an upgrade when a database migration fails to
convert the secret because it does not contain a base64 string, it is
ignored.

A warning is displayed to suggest the row is removed and clarify that
it does not require action. It is safe to ignore.

models/forgejo_migrations/v25.go

@@ -10,6 +10,8 @@ import (
 
 	"code.gitea.io/gitea/models/auth"
 	"code.gitea.io/gitea/models/db"
+	"code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/secret"
 	"code.gitea.io/gitea/modules/setting"
 
@@ -58,6 +60,14 @@ func MigrateTwoFactorToKeying(x *xorm.Engine) error {
 	oldEncryptionKey := md5.Sum([]byte(setting.SecretKey))
 
 	return db.Iterate(context.Background(), nil, func(ctx context.Context, bean *auth.TwoFactor) error {
+		if _, err := user.GetUserByID(context.Background(), bean.UID); err != nil {
+			if user.IsErrUserNotExist(err) {
+				log.Warn("two_factor.id = %d references non existent user id %d. It is harmless and was ignored but should be removed", bean.ID, bean.UID)
+				return nil
+			}
+			return err
+		}
+
 		decodedStoredSecret, err := base64.StdEncoding.DecodeString(string(bean.Secret))
 		if err != nil {
 			return err

models/forgejo_migrations/v25_test.go

@@ -8,6 +8,7 @@ import (
 
 	"code.gitea.io/gitea/models/auth"
 	migration_tests "code.gitea.io/gitea/models/migrations/test"
+	"code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/keying"
 	"code.gitea.io/gitea/modules/timeutil"
 
@@ -28,7 +29,7 @@ func Test_MigrateTwoFactorToKeying(t *testing.T) {
 	}
 
 	// Prepare and load the testing database
-	x, deferable := migration_tests.PrepareTestEnv(t, 0, new(TwoFactor))
+	x, deferable := migration_tests.PrepareTestEnv(t, 0, new(TwoFactor), new(user.User))
 	defer deferable()
 	if x == nil || t.Failed() {
 		return
@@ -36,7 +37,7 @@ func Test_MigrateTwoFactorToKeying(t *testing.T) {
 
 	cnt, err := x.Table("two_factor").Count()
 	require.NoError(t, err)
-	assert.EqualValues(t, 1, cnt)
+	assert.EqualValues(t, 2, cnt)
 
 	require.NoError(t, MigrateTwoFactorToKeying(x))
 
@@ -47,4 +48,9 @@ func Test_MigrateTwoFactorToKeying(t *testing.T) {
 	secretBytes, err := keying.DeriveKey(keying.ContextTOTP).Decrypt(twofactor.Secret, keying.ColumnAndID("secret", twofactor.ID))
 	require.NoError(t, err)
 	assert.Equal(t, []byte("AVDYS32OPIAYSNBG2NKYV4AHBVEMKKKIGBQ46OXTLMJO664G4TIECOGEANMSNBLS"), secretBytes)
+
+	var twofactorOrphaned auth.TwoFactor
+	_, err = x.Table("two_factor").ID(2).Get(&twofactorOrphaned)
+	require.NoError(t, err)
+	assert.Equal(t, []byte("CORRUPTED_SECRET"), twofactorOrphaned.Secret)
 }

models/migrations/fixtures/Test_MigrateTwoFactorToKeying/two_factor.yml

@@ -7,3 +7,8 @@
   last_used_passcode:
   created_unix: 1564253724
   updated_unix: 1564253724
+
+-
+  id: 2
+  uid: 2000
+  secret: CORRUPTED_SECRET