### 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>
(cherry picked from commit 3c9b3ddf5c)
- If for some reason a action log file does not longer exist in the
database or on the file system, then mark it as being cleaned up.
- Unit test added.
(cherry picked from commit 4e8677a911)
- `GetSubModules` already solely stores the URL of the submodule and not
a `*SubModule` entry, so don't try to type assert it to be a struct.
- I am not able to pinpoint when this was regressed but if I had to
guess it might be #4941.
- Added integration test.
(cherry picked from commit e7cffc378f)
Replace wiki page filenames with page titles in the search results,
fixing the problem with them showing unreadable URI-encoded names.
(cherry picked from commit fc31fa0eeb)
We have some actions that leverage the Gitea API that began receiving
401 errors, with a message that the user was not found. These actions
use the `ACTIONS_RUNTIME_TOKEN` env var in the actions job to
authenticate with the Gitea API. The format of this env var in actions
jobs changed with go-gitea/gitea/pull/28885 to be a JWT (with a
corresponding update to `act_runner`) Since it was a JWT, the OAuth
parsing logic attempted to parse it as an OAuth token, and would return
user not found, instead of falling back to look up the running task and
assigning it to the actions user.
Make ACTIONS_RUNTIME_TOKEN in action runners could be used,
attempting to parse Oauth JWTs. The code to parse potential old
`ACTION_RUNTIME_TOKEN` was kept in case someone is running an older
version of act_runner that doesn't support the Actions JWT.
(cherry picked from commit 407b6e6dfc7ee9ebb8a16c7f1a786e4c24d0516e)
Conflicts:
services/auth/oauth2.go
trivial context conflicts because OAuth2 scopes are in Forgejo and
not yet in Gitea
- Only send a review request based on the code owner file if the code
owner user has read permissions to the pull requests of that repository.
- This avoids leaking title of PRs from private repository when a
CODEOWNER file is present which contains users that do not have access
to the private repository.
- Found by @oliverpool.
- Integration test added.
(cherry picked from commit 693f7731f9)
- When RS256, RS384, ES384, ES512 was specified as the JWT signing
algorithm they would generate RS512 and ES256 respectively.
- Added unit test.
(cherry picked from commit 7d59060dc6)
- This unifies the security behavior of enrolling security keys with
enrolling TOTP as a 2FA method. When TOTP is enrolled, you cannot use
basic authorization (user:password) to make API request on behalf of the
user, this is now also the case when you enroll security keys.
- The usage of access tokens are the only method to make API requests on
behalf of the user when a 2FA method is enrolled for the user.
- Integration test added.
(cherry picked from commit e6bbecb02d)
- Add a `purpose` column, this allows the `forgejo_auth_token` table to
be used by other parts of Forgejo, while still enjoying the
no-compromise architecture.
- Remove the 'roll your own crypto' time limited code functions and
migrate them to the `forgejo_auth_token` table. This migration ensures
generated codes can only be used for their purpose and ensure they are
invalidated after their usage by deleting it from the database, this
also should help making auditing of the security code easier, as we're
no longer trying to stuff a lot of data into a HMAC construction.
-Helper functions are rewritten to ensure a safe-by-design approach to
these tokens.
- Add the `forgejo_auth_token` to dbconsistency doctor and add it to the
`deleteUser` function.
- TODO: Add cron job to delete expired authorization tokens.
- Unit and integration tests added.
(cherry picked from commit 1ce33aa38d)
v9: Removed migration - XORM can handle this case automatically without
migration. Add `DEFAULT 'long_term_authorization'`.
- If the incoming mail feature is enabled, tokens are being sent with
outgoing mails. These tokens contains information about what type of
action is allow with such token (such as replying to a certain issue
ID), to verify these tokens the code uses the HMAC-SHA256 construction.
- The output of the HMAC is truncated to 80 bits, because this is
recommended by RFC2104, but RFC2104 actually doesn't recommend this. It
recommends, if truncation should need to take place, it should use
max(80, hash_len/2) of the leftmost bits. For HMAC-SHA256 this works out
to 128 bits instead of the currently used 80 bits.
- Update to token version 2 and disallow any usage of token version 1,
token version 2 are generated with 128 bits of HMAC output.
- Add test to verify the deprecation of token version 1 and a general
MAC check test.
(cherry picked from commit 9508aa7713)
When a workflow has
on:
pull_request:
types:
- labeled
- unlabeled
The outcome of the workflow (success or failure) must be associated
with the head sha commit status. Otherwise it cannot be used as a
requirement for merging the pull request (branch protections).
(cherry picked from commit 66c85b7d8b)
When a workflow has
on:
pull_request:
types:
- labeled
- unlabeled
The payload misses the label field describing the added or removed
label.
The unlabeled event type was also incorrectly mapped to the labeled
event type.
(cherry picked from commit 58e3c1fbdb)
- Add a permission check that the doer has write permissions to the head
repository if the the 'delete branch after merge' is enabled when
merging a pull request.
- Unify the checks in the web and API router to `DeleteBranchAfterMerge`.
- Added integration tests.
(cherry picked from commit 266e0b2ce9)
Add log for the problem #31480
(cherry picked from commit a264c46fb04112c5ec2c1b2acd523a2e4450da40)
Conflicts:
- services/user/user.go
Resolved by manually adding the log line.
- On editting a team, only update the units if the team isn't the
'Owners' team. Otherwise the 'Owners' team end up having all of their
unit access modes set to 'None'; because the request form doesn't send
over any units, as it's simply not shown in the UI.
- Adds a database inconstency check and fix for the case where the
'Owners' team is affected by this bug.
- Adds unit test.
- Adds integration test.
- Resolves#5528
- Regression of https://github.com/go-gitea/gitea/pull/24012
(cherry picked from commit 9de9034400)
Port of https://github.com/go-gitea/gitea/pull/32204
(cherry picked from commit d6d3c96e6555fc91b3e2ef21f4d8d7475564bb3e)
Conflicts:
routers/api/v1/api.go
services/context/api.go
trivial context conflicts
(cherry picked from commit a052d2b602)
Conflicts:
routers/api/v1/user/user.go
trivial context conflict (search by email is not in v9.0)
Fix#31423
(cherry picked from commit f4b8f6fc40ce2869135372a5c6ec6418d27ebfba)
Conflicts:
models/fixtures/comment.yml
comment fixtures have to be shifted because there is one more in Forgejo
The inventory of the sha256:* images and the manifest index that
reference them is incomplete because it does not take into account any
image older than the expiration limit. As a result some sha256:* will
be considered orphaned although they are referenced from a manifest
index that was created more recently than the expiration limit.
There must not be any filtering based on the creation time when
building the inventory. The expiration limit must only be taken into
account when deleting orphaned images: those that are more recent than
the expiration limit must not be deleted.
This limit is specially important because it protects against a race
between a cleanup task and an ongoing mirroring task. A mirroring
task (such as skopeo sync) will first upload sha256:* images and then
create the corresponding manifest index. If a cleanup races against
it, the sha256:* images that are not yet referenced will be deleted
without skopeo noticing and the published index manifest that happens
at a later time will contain references to non-existent images.
(cherry picked from commit 0a5fd7fdb8)
After migrating a repository with pull request, the branch is missed and
after the pull request merged, the branch cannot be deleted.
(cherry picked from commit 5a8568459d22e57cac506465463660526ca6a08f)
Conflicts:
services/repository/branch.go
conflict because of [GITEA] Fix typo in formatting error e71b5a038e
- [x] add architecture-specific removal support
- [x] Fix upload competition
- [x] Fix not checking input when downloading
docs: https://codeberg.org/forgejo/docs/pulls/874
### Release notes
- [ ] I do not want this change to show in the release notes.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/5351
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Exploding Dragon <explodingfkl@gmail.com>
Co-committed-by: Exploding Dragon <explodingfkl@gmail.com>
(cherry picked from commit 89742c4913)
Remove unused CSRF options, decouple "new csrf protector" and "prepare"
logic, do not redirect to home page if CSRF validation falis (it
shouldn't happen in daily usage, if it happens, redirecting to home
doesn't help either but just makes the problem more complex for "fetch")
(cherry picked from commit 1fede04b83288d8a91304a83b7601699bb5cba04)
Conflicts:
options/locale/locale_en-US.ini
tests/integration/repo_branch_test.go
trivial context conflicts
(cherry picked from commit 1ae3b127fc)
Fixes#32021
Do not escape the relative path.
(cherry picked from commit f528df944bb9436afcb9272add2ee0cccefbdb55)
(cherry picked from commit 0cafec4c7a)
A 500 status code was thrown when passing a non-existent target to the
create release API. This snapshot handles this error and instead throws
a 404 status code.
Discovered while working on #31840.
(cherry picked from commit f05d9c98c4cb95e3a8a71bf3e2f8f4529e09f96f)
---
`status == "rename"` should have read `status == "renamed"`. The typo
means that file.PreviousFilename would never be populated, which e.g.
breaks usage of the Github Action at
https://github.com/dorny/paths-filter.
(cherry picked from commit 7c6edf1ba06d4c3269eaa78f4039c9123b006c51)
