### Problem
Big instances can have huge authorized_keys files when using OpenSSH instead of the internal ssh server. Forgejo always re-generates the contents of that file when a user is deleted, even if he does not even have a public key uploaded. In case of codeberg.org, a 15MB file gets rewritten. If we batch delete 100 Spam users without ssh keys, we rewrite 1.5GB, this takes time and wears the SSD. In addition, there is a high chance of hitting a race contidion bug, when deleting users in parallel.
### Solution / Mitigation
This patch prevents rewriting authorized_keys files, when not necessary. It greatly speeds up deleting malicious users, saves IO bandwidth and SSD wear. It also greatly reduces the chance of hitting a race condition bug. Fixing the race condition is not the scope of this patch though.
## 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...
- [x] in their respective `*_test.go` for unit tests.
- [ ] 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
- [x] I do not want this change to show in the release notes.
- [ ] 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.
Co-authored-by: Gusted <postmaster@gusted.xyz>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6097
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Andreas Shimokawa <shimokawa@fsfe.org>
Co-committed-by: Andreas Shimokawa <shimokawa@fsfe.org>
Add log for the problem #31480
(cherry picked from commit a264c46fb04112c5ec2c1b2acd523a2e4450da40)
Conflicts:
- services/user/user.go
Resolved by manually adding the log line.
- `user_model.DeleteInactiveEmailAddresses` related code was added in
Gogs as part to delete inactive users, however since then the related
code to delete users has changed and this code now already delete email
addresses of the user, it's therefore not needed anymore to
`DeleteInactiveEmailAddresses`.
- The call to `DeleteInactiveEmailAddresses` can actually cause issues.
As the associated user might not have been deleted, because it
was not older than the specified `olderThan` argument. Therefore causing
a database inconsistency and lead to internal server errors if the user
tries to activate their account.
- Adds unit test to verify correct behavior (fails without this patch).
Fixes#28660
Fixes an admin api bug related to `user.LoginSource`
Fixed `/user/emails` response not identical to GitHub api
This PR unifies the user update methods. The goal is to keep the logic
only at one place (having audit logs in mind). For example, do the
password checks only in one method not everywhere a password is updated.
After that PR is merged, the user creation should be next.
This PR is a refactor at the beginning. And now it did 4 things.
- [x] Move renaming organizaiton and user logics into services layer and
merged as one function
- [x] Support rename a user capitalization only. For example, rename the
user from `Lunny` to `lunny`. We just need to change one table `user`
and others should not be touched.
- [x] Before this PR, some renaming were missed like `agit`
- [x] Fix bug the API reutrned from `http.StatusNoContent` to `http.StatusOK`
this is a simple endpoint that adds the ability to rename users to the
admin API.
Note: this is not in a mergeable state. It would be better if this was
handled by a PATCH/POST to the /api/v1/admin/users/{username} endpoint
and the username is modified.
---------
Co-authored-by: Jason Song <i@wolfogre.com>
- Unify the hashing code for repository and user avatars into a
function.
- Use a sane hash function instead of MD5.
- Only require hashing once instead of twice(w.r.t. hashing for user
avatar).
- Improve the comment for the hashing code of why it works.
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: Yarden Shoham <hrsi88@gmail.com>
Change all license headers to comply with REUSE specification.
Fix#16132
Co-authored-by: flynnnnnnnnnn <flynnnnnnnnnn@github>
Co-authored-by: John Olheiser <john.olheiser@gmail.com>
This PR adds a context parameter to a bunch of methods. Some helper
`xxxCtx()` methods got replaced with the normal name now.
Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Fix#19513
This PR introduce a new db method `InTransaction(context.Context)`,
and also builtin check on `db.TxContext` and `db.WithTx`.
There is also a new method `db.AutoTx` has been introduced but could be used by other PRs.
`WithTx` will always open a new transaction, if a transaction exist in context, return an error.
`AutoTx` will try to open a new transaction if no transaction exist in context.
That means it will always enter a transaction if there is no error.
Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: 6543 <6543@obermui.de>
* Remove `db.DefaultContext` usage in routers, use `ctx` directly
* Use `ctx` directly if there is one, remove some `db.DefaultContext` in `services`
* Use ctx instead of db.DefaultContext for `cmd` and some `modules` packages
* fix incorrect context usage
* Move keys to models/keys
* Rename models/keys -> models/asymkey
* change the missed package name
* Fix package alias
* Fix test
* Fix docs
* Fix test
* Fix test
* merge
* Some refactors related repository model
* Move more methods out of repository
* Move repository into models/repo
* Fix test
* Fix test
* some improvements
* Remove unnecessary function
* Remove unnecessary functions of User struct
* Move more database methods out of user struct
* Move more database methods out of user struct
* Fix template failure
* Fix bug
* Remove finished FIXME
* remove unnecessary code
* Use a standalone struct name for Organization
* recover unnecessary change
* make the code readable
* Fix template failure
* Fix template failure
* Move HasMemberWithUserID to org
* Fix test
* Remove unnecessary user type check
* Fix test
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
