diff options
author | Earl Warren <contact@earl-warren.org> | 2025-01-21 18:30:08 +0100 |
---|---|---|
committer | Earl Warren <earl-warren@noreply.codeberg.org> | 2025-01-21 18:30:08 +0100 |
commit | c92fe83c4060f74942350510f7df3f6ac3846556 (patch) | |
tree | 9aefad9923bdd26ba700223dc5de377f5b4af15f /modules/secret | |
parent | Update dependency go to v1.23.5 (forgejo) (#6590) (diff) | |
download | forgejo-c92fe83c4060f74942350510f7df3f6ac3846556.tar.xz forgejo-c92fe83c4060f74942350510f7df3f6ac3846556.zip |
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/<pull request number>.md` to be be used for the release notes instead of the title.
<!--start release-notes-assistant-->
## Release notes
<!--URL:https://codeberg.org/forgejo/forgejo-->
- Bug fixes
- [PR](https://codeberg.org/forgejo/forgejo/pulls/6639): <!--number 6639 --><!--line 0 --><!--description VGVhY2ggdGhlIGRvY3RvciB0byByZW1vdmUgb3JwaGFuZWQgdHdvX2ZhY3RvciB3aXRoIGBmb3JnZWpvIGRvY3RvciBjaGVjayAtLXJ1biBjaGVjay1kYi1jb25zaXN0ZW5jeSAtLWZpeGAuIFN1Y2ggcm93cyBtYXkgY29udGFpbiBpbnZhbGlkIGRhdGEgYW5kIFtibG9jayB0aGUgbWlncmF0aW9uIHRvIHYxMF0oaHR0cHM6Ly9jb2RlYmVyZy5vcmcvZm9yZ2Vqby9mb3JnZWpvL2lzc3Vlcy82NjM3KSB3aXRoIGEgbWVzc2FnZSBzdWNoIGFzIGBmYWlsZWQ6IEFlc0RlY3J5cHQgaW52YWxpZCBkZWNyeXB0ZWQgYmFzZTY0IHN0cmluZzogaWxsZWdhbCBiYXNlNjQgZGF0YSBhdCBpbnB1dCBieXRlIDBgLg==-->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`.<!--description-->
<!--end release-notes-assistant-->
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6639
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>
Diffstat (limited to 'modules/secret')
-rw-r--r-- | modules/secret/secret.go | 2 |
1 files changed, 1 insertions, 1 deletions
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 } |