From c92fe83c4060f74942350510f7df3f6ac3846556 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Tue, 21 Jan 2025 17:30:08 +0000 Subject: [PATCH] fix: teach the doctor about orphaned two_factor rows (#6639) 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. Improve the error message to suggest using the doctor to remove it. Fixes: https://codeberg.org/forgejo/forgejo/issues/6637 ## Testing - make TAGS='sqlite sqlite_unlock_notify' watch - make TAGS='sqlite sqlite_unlock_notify' forgejo - sqlite3 data/gitea.db 'INSERT INTO two_factor VALUES( 0, 500, "", "", "", "", 0, 0)' - ./forgejo doctor check --run check-db-consistency ``` [1] Check consistency of database - [W] Found 1 Orphaned TwoFactor without existing User OK All done (checks: 1). ``` - ./forgejo doctor check --run check-db-consistency --fix ``` [1] Check consistency of database - [I] Deleted 1 Orphaned TwoFactor without existing User OK All done (checks: 1). ``` ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] I do not want this change to show in the release notes. - [x] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/.md` to be be used for the release notes instead of the title. ## Release notes - Bug fixes - [PR](https://codeberg.org/forgejo/forgejo/pulls/6639): Teach the doctor to remove orphaned two_factor with `forgejo doctor check --run check-db-consistency --fix`. Such rows may contain invalid data and [block the migration to v10](https://codeberg.org/forgejo/forgejo/issues/6637) with a message such as `failed: AesDecrypt invalid decrypted base64 string: illegal base64 data at input byte 0`. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6639 Reviewed-by: Mathieu Fenniak Co-authored-by: Earl Warren Co-committed-by: Earl Warren --- modules/secret/secret.go | 2 +- release-notes/6639.md | 1 + services/doctor/dbconsistency.go | 3 +++ 3 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 release-notes/6639.md diff --git a/modules/secret/secret.go b/modules/secret/secret.go index e70ae1839c..e3557b91b9 100644 --- a/modules/secret/secret.go +++ b/modules/secret/secret.go @@ -47,7 +47,7 @@ func AesDecrypt(key, text []byte) ([]byte, error) { cfb.XORKeyStream(text, text) data, err := base64.StdEncoding.DecodeString(string(text)) if err != nil { - return nil, fmt.Errorf("AesDecrypt invalid decrypted base64 string: %w", err) + return nil, fmt.Errorf("AesDecrypt invalid decrypted base64 string: %w - it can be caused by a change of the [security].SECRET_KEY setting or a database corruption - `forgejo doctor check --run check-db-consistency --fix` will get rid of orphaned rows found in the `two_factor` table and may fix this problem if they are the one with the invalid content", err) } return data, nil } diff --git a/release-notes/6639.md b/release-notes/6639.md new file mode 100644 index 0000000000..1bc01c12a3 --- /dev/null +++ b/release-notes/6639.md @@ -0,0 +1 @@ +Teach the doctor to remove orphaned two_factor with `forgejo doctor check --run check-db-consistency --fix`. Such rows may contain invalid data and [block the migration to v10](https://codeberg.org/forgejo/forgejo/issues/6637) with a message such as `failed: AesDecrypt invalid decrypted base64 string: illegal base64 data at input byte 0`. diff --git a/services/doctor/dbconsistency.go b/services/doctor/dbconsistency.go index 80f538d670..9e2fcb645f 100644 --- a/services/doctor/dbconsistency.go +++ b/services/doctor/dbconsistency.go @@ -246,6 +246,9 @@ func checkDBConsistency(ctx context.Context, logger log.Logger, autofix bool) er // find authorization tokens without existing user genericOrphanCheck("Authorization token without existing User", "forgejo_auth_token", "user", "forgejo_auth_token.uid=`user`.id"), + // find two_factor without existing user + genericOrphanCheck("Orphaned TwoFactor without existing User", + "two_factor", "user", "`two_factor`.uid=`user`.id"), ) for _, c := range consistencyChecks {