**Backport:** https://codeberg.org/forgejo/forgejo/pulls/13547
Adds CGNAT IP space to the `private` builtin, and excludes it from the `external` builtin, for all allowed host lists. As private space is described in Forgejo's documentation as "LAN/Intranet", and external is described as "you can access all hosts on the public internet", it is reasonable to exclude CGNAT space which doesn't qualify under those descriptions -- it is not a globally routable IP address.
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13567
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
**Backport:** https://codeberg.org/forgejo/forgejo/pulls/13443Fixes#13440.
## Testing
Manual testing was performed with an automated reproduction. Attached `main.go` program was used to test go-git interaction with a server using `START_SSH_SERVER = true`. This test app uses go-git to perform an ssh-based push, which replicates the behaviour described in #13440. After dropping main.go into an empty directory, the test app can be run with:
```
go mod init repro
go mod tidy
REPRO_REPO_URL=ssh://mfenniak@localhost:2222/mfenniak/test.git go run main.go
```
When the fix is **not** present, the database/repository desync warning is shown after running the test command, which indicates that the hooks were interrupted and did not update the database state of the `main` branch:
When this fix **is** present, the database/repository desync warning does not appear, which indicates that the hooks were not interrupted. Repeated test in both modes 3x times to ensure there were no race conditions involved.
While it would be possible to incorporate this test into Forgejo's integration testing suite, it would require adding a dependency to `go-git`. As Go doesn't have a concept of test-only dependencies, that would make `go-git` a dependency... which we just recently removed by removing it from Forgejo Runner's jobparser library. It would make `go-git` available for accidental incorporation into Forgejo's code, where it does not behave the same as Forgejo's internal git CLI libraries.
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13451
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
**Backport:** https://codeberg.org/forgejo/forgejo/pulls/10927
Continuation of #10598Closes#7254
Implement repo granular watch selection
I adopted a lot of the frontend from #10598 while redoing the entire back-end. I propose this new model:
- Remove the WatchMode enum.
- Add the WatchSelection struct, which represents the granular selection. Notice that there is no `watching` bool. I tried very hard to keep the structure as simple and redundancy-free as possible. Therefore, people not watching a repo at all either don't have a watch record or one with an entirely unselected WatchSelection struct.
- Add the WatchSource enum. It replaces the WatchMode enum and has a single purpose: determine whether a watch was explicitly or automatically initiated.
Notice that replacing this
```go
And("`watch`.mode<>?", WatchModeDont).
```
with this is correct:
```go
And(
builder.Or(
builder.Eq{"`watch`.watch_selection_issues": true},
builder.Eq{"`watch`.watch_selection_pull_requests": true},
builder.Eq{"`watch`.watch_selection_releases": true},
),
).
```
That's because there are four modes: dont, none, auto and normal. When `<>` with dont, we look for auto and normal, because there are no records with none. Therefore, the old code looks for records that indicate watching. The code I replaced this with does so, too, just more granular.
Also notice that I've prepared a future `user preset` in a few places. See below for a little more info on that.
## Next PR
I plan to continue working on this. I want to implement a `user preset` option. The user sets that `user preset` in her settings and may use them in any repo.
<details>
- rename account settings to account and notifications
- user preset (always use this preset for newly accessible repos (according to AutoWatchOnChanges and AutoWatchNewRepos))
</details>
## Further PRs
- make api able to granular watch
- move (email) notifications to new notifications tab
Co-authored-by: 0ko <0ko@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-authored-by: pat-s <patrick.schratz@gmail.com>
Reviewed-on: #10927
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
(cherry picked from commit 8770ffc848)
**Backport:** https://codeberg.org/forgejo/forgejo/pulls/11795
Adds an extra check to ensure the `keyId` and `actorId` included in signed requests and actor records point back to the originating host. This check prevents server-side request forgery (SSRF) attacks where a carefully crafted request could be used to trick a federation server into making requests to arbitrary hosts and ports.
### Tests for Go changes
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
- [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I ran...
- [x] `make pr-go` before pushing
Co-authored-by: elle <0xllx0@noreply.codeberg.org>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13351
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
**Backport:** https://codeberg.org/forgejo/forgejo/pulls/13293
Test failure: https://codeberg.org/forgejo/forgejo/actions/runs/174007/jobs/2/attempt/1 Have only observed once and can't reproduce locally, but likely caused by #13263 / #13289.
```
--- FAIL: TestGetDiffPreview (0.20s)
--- FAIL: TestGetDiffPreview/empty_branch,_same_results (0.02s)
diff_test.go:132:
Error Trace: /workspace/forgejo/forgejo/services/repository/files/diff_test.go:132
Error: Received unexpected error:
unable to run diff-index pipeline in temporary repo: exec: canceling Cmd: invalid argument
Test: TestGetDiffPreview/empty_branch,_same_results
FAIL
```
pidfd_open is [documented](https://www.man7.org/linux/man-pages/man2/pidfd_open.2.html) to return the EINVAL (invalid argument) error if the pid is not valid, which could happen if the process terminates itself before Forgejo sends `SIGTERM`. This PR adds a check for that error and stops `Cancel`, with a minor refactor to avoid code duplication.
### Tests for Go changes
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
- [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I ran...
- [x] `make pr-go` before pushing
### 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
- [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [x] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13302
**Backport:** https://codeberg.org/forgejo/forgejo/pulls/13289#13263 unintentionally prevents Forgejo from building on non-Linux UNIX OSes. This PR adds a second graceful shutdown implementation which uses repeated signalling of the subprocess to detect if it is still running. This additional implementation is used for non-Linux UNIX platforms. It is also a fallback when Linux returns `ENOSYS` which should be the case in Linux kernels older than 5.3.
## Testing
- Validated build fails on a FreeBSD VM w/ head `forgejo`
- `FreeBSD freebsd-test 15.1-RELEASE FreeBSD 15.1-RELEASE releng/15.1-n283562-96841ea08dcf GENERIC amd64`
- Validate build succeeds and process tests pass on FreeBSD VM w/ this branch:
```
mfenniak@freebsd-test:~/Dev/forgejo $ GO_TEST_PACKAGES=forgejo.org/modules/process make test-backend
Running go test with -tags 'sqlite sqlite_unlock_notify'...
ok forgejo.org/modules/process 5.690s
```
- Note: other some other test modules do not pass like `forgejo.org/modules/log`, but this seems consistent with `forgejo` head. As we don't have a baseline for expectations here, this isn't an issue that can be addressed in this PR.
- Fallback for pre-Linux 5.3 has been tested on a Debian Buster install, kernel 4.19.0, and confirmed `forgejo.org/modules/process` test success, and test failure without the fallback path.
### Tests for Go changes
- 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 ran...
- [x] `make pr-go` before pushing
### 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
- [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [x] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
- Adjustment to unreleased bugfix.
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13292
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
**Backport:** https://codeberg.org/forgejo/forgejo/pulls/13263
By default, when you create an `exec.CommandContext` in Go, when the context is cancelled or timed out, Go will SIGKILL the subprocess which does not allow the subprocess to perform any cleanup work. As some git operations in Forgejo will be linked to the current HTTP context, and the current HTTP context will be closed when the HTTP client disconnects, this can theoretically cause SIGKILLs when locks are being held.
This PR contains two fixes:
- Change the behaviour of all subprocesses that Forgejo creates to use a graceful termination, SIGTERM'ing the process, and then SIGKILL'ing it after a timeout.
- Fixes a bug where previously all subprocesses are setup in a process group (by 1d04e8641d), but they were never *killed* in a process group, preventing child processes from receiving these termination signals. Both SIGTERM and SIGKILL are sent to the process group.
Introduces new configuration setting which controls subcommand graceful timeouts. If a subprocess does not shutdown from SIGTERM within `[server].SUBPROCESS_TERMINATE_GRACE` (default 5 seconds), then an error will be logged, and the process will be SIGKILL'd. The error is:
```
2026/07/01 08:31:20 modules/log/init.go:38:func2() [E] Subprocess pid=3790963 (/nix/store/c0277k5giric1mn9dklllavbzvxl6hzb-git-2.53.0/bin/git blame --porcelain 8ee7e7c32b -- options/locale/locale_en-US.ini) failed to terminate from SIGTERM after grace period 0s, and was sent SIGKILL. The subprocess termination may leave incomplete state on the server, such as lock files. `[server].SUBPROCESS_TERMINATE_GRACE` can be changed to give subprocesses more time to cleanly exit.
```
(This was produced on a live server by setting SUBPROCESS_TERMINATE_GRACE to zero seconds, loading a page that does a `git blame`, and hitting escape in the browser quickly.)
### Tests for Go changes
- 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 ran...
- [x] `make pr-go` before pushing
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- Will create a documentation PR with new config settings.
- [ ] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13274
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
**Backport:** https://codeberg.org/forgejo/forgejo/pulls/12963
When downloading archives forgejo uses git cat-file to resolve a ref name (e.g. branch or tag) to a commit id. There were a check if the input already was a valid full commit id, sadly without a check if the commit exists. Therefore, I removed this "early return".
I noticed this while trying to download archives from codeberg and the there was just a timeout after 10min. Upon trying this on a self-hosted instance I saw the error in the logs, which led me to his fix.
Co-authored-by: Marcel <me@m4rc3l.de>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13242
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
**Backport:** https://codeberg.org/forgejo/forgejo/pulls/13184
Similar to #13129, fixing a few places where `[migrations].ALLOWED_DOMAINS`, `...BLOCKED_DOMAINS`, and `...ALLOW_LOCALNETWORKS` are not implemented in external migrations, creating SSRF risks.
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). 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 for Go changes
- 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 ran...
- [x] `make pr-go` before pushing
### 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] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13196
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
System administrators can determine which hosts can be accessed by Forgejo for git mirroring via the config settings `[migrations].ALLOWED_DOMAINS`, `...BLOCKED_DOMAINS`, and `....ALLOW_LOCALNETWORKS`. However, there were edge cases where these settings were ineffective:
- Config entries were only checked when mirrors were initially configured, and not later when a mirror pull/push occurred. DNS changes, or server configuration changes, could cause a previously safe URL to no longer be safe. These settings are now re-checked on every mirror pull/push.
- When accessing a git repository over http, git would follow HTTP redirects, allowing a violation of the `[migrations]` config entries. The git option `http.followRedirects=false` is now set to prevent git from following these redirects.
- When performing Git LFS synchronization, it was assumed that the git URL passing migration URL checks was sufficient protection. Specifically for pull mirrors which allow the usage of "Advanced settings" -> "LFS endpoint" to configure a separate LFS endpoint, these settings were never enforced, and for all mirrors they would have been subject to not being rechecked if the DNS had changed since configured. LFS mirroring now uses an HTTP client that enforces the `[migrations]` config settings.
**Breaking:** If a repository mirror that depends on HTTP redirects will no longer work.
For example, if you've mirrored a repository from a Forgejo instance at `https://example.com/OWNER/REPO.git` and the repository or owner are renamed, Forgejo will provide an HTTP redirect that continues to allow git operations to work at the old URL. Due to this redirect prohibition, this mirror will no longer work.
### Tests for Go changes
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
- [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I ran...
- [x] `make pr-go` before pushing
### Documentation
- [x] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- It is not currently documented that the `[migrations]` settings are used to control access to mirrors. Forgejo advises it when an error occurs, but it could be added: https://codeberg.org/forgejo/docs/pulls/2023
- [ ] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13129
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
When a user has a theme in the DB that is not among the themes in the configuration, the following happens to this user's UI:
Image: https://codeberg.org/attachments/bf8d4ff1-8216-4df5-ab90-8dc7e03784d9
The workaround is to manually go to Appearance settings and update the theme.
This can happen if the theme was removed from the server config. For example, admins don't want to have it anymore. Maybe it even was the default theme, which is being saved in the DB during sign up.
It will be useful for Forgejo if we, for example, want to separate colorblind them variants from the actual themes, or if we ever want to remove the Gitea themes. Rel: https://codeberg.org/forgejo/forgejo/pulls/13054.
And instance admins will also find it useful to not have to manually update the DB in case they want to get rid of some custom theme.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13110
Reviewed-by: Robert Wolff <mahlzahn@posteo.de>
In #9638, repository and user avatars had an EXIF removal capability added to them. Unfortunately, this capability was added with an AGPL licensed library, exif-terminator, which is incompatible with Forgejo's license. This was not detected by license check automation at the time.
This PR removes the capability in order to fix the license compatibility. The `forgejo doctor avatar-strip-exif` is retained with a warning output that it is not supported.
Reopens: forgejo/forgejo#9608
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). 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 for Go changes
- 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 ran...
- [x] `make pr-go` before pushing
### 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] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13105
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Fixes#12564. Fixes#8951.
This introduces a new setting, `ADD_MEMBERS_BY_INVITATIONS`, which is turned off by default.
When turned on, adding a user to a team issues an invitation instead of adding them directly to the team.
A prerequisite for this work was to be able to link invitations to existing users (so far, they were only associated to an email address, since those invitations were meant to be issued to users who didn't have an account yet).
---
I plan to work on the following improvements, which I propose to do in separate PRs given that this one is already a bit big:
* generate an in-app notification for the invited user
* advertise the invitation to the invited user from the org page as well (#12120)
* show the list of invited users in the list of organization members (not just on the team page)
and various other improvements to invitations (#12570, #12716).
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12845
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
This PR fixes some minor issues with the swagger API documentation. These issues are:
1. The description for the `template` parameter for the `GET /repo/search` endpoint is incorrect. It says this:
```
include template repositories this user has access to (defaults to true)
```
When the parameter actually functions like this:
```
show only template, non-template or all repositories (defaults to all)
```
2. The the `gitignores` option in the `POST /user/repos` endpoint JSON object has a description that doesn't say how `gitignore` names should be separated. It's like this:
```
Gitignores to use
```
When it should be something like this:
```
Gitignores to use, separated by commas
```
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13082
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
The routines for the removal of `ActionTask` expect that each `ActionTask` has logs attached and that `LogFilename` isn't empty. However, that is not always the case. For example, Forgejo can resolve jobs without dispatching them to a runner. In that case, a placeholder task is created without logs and `LogFilename`. The log removal routines simply concatenate the path of the log storage directory and `LogFilename` and try to delete that without verifying that `LogFilename` is present. Consequently, they try to remove the log storage directory. In most cases, that causes an error because the directory contains some files. To prevent that from happening, the log removal routines no longer allow empty filenames. And it is checked whether a task has logs before invoking them.
### Tests for Go changes
- 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 ran...
- [ ] `make pr-go` before pushing
### 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] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13040
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Otherwise the newly introduced test from #12335 fails on platforms which do not have git sha256 yet
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). 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 for Go changes
- 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 ran...
- [X] `make pr-go` before pushing
### 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
- [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [X] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13018
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Found issues during the process of invalidation of a multiline comment (link to #12582):
* Update a line in the middle of the comment
* Update/Delete the last line of the comment
No problem with:
* Deleting a line in the middle of the comment
* Update/Delete the first line of the comment
I added all these cases in the pull_review_test.go
### Tests for Go changes
- I added test coverage for Go changes...
- [X] in their respective `*_test.go` for unit tests.
- I ran...
- [X] `make pr-go` before pushing
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12950
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
This PR pertains to the client-side validation of the Website input on user, repo, and org profiles. #12962 extends `[service].VALID_SITE_URL_SCHEMES` to cover Website fields on repo and org profiles, where before that config key only applied to the one on user profiles. If that change merges, it will then be possible to construct an HTML [`pattern`](https://developer.mozilla.org/docs/Web/HTML/Reference/Elements/input#pattern) attribute for general use on any Website form input that the server validates this way, thus enabling browsers to catch errors early relating to URL scheme confusion.
This PR (1) introduces such a `pattern` attribute, and (2) adds a new UI note to make clear to users which URL schemes are permitted. This change helps explain the browser's otherwise cryptic error messages regarding pattern mismatch, while also letting users know what URI schemes the Forgejo instance supports as Website links (e.g. gemini:// URLs).
This MUST NOT merge before #12962. To do so would introduce a regression wherein the UI may suggest and validate a different set of allowed URL schemes than the server actually permits.
See also #5519
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12991
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Turns out this was a one-line fix for each affected field (change the binding from `ValidUrl` to `ValidSiteUrl`), but the tests are rather verbose. The tests are, however, each a simple flow of Create Thing > Try HTTP Website > Try Different Website (notice failure) > Try Different Website With New Config (notice success). I wrote this PR by adding failing tests first, then making the change, for each affected field.
Not sure if this should be "feat:" or "fix:" tbh. I figured "fix:" for this PR since IMO the expected behavior is for `VALID_SITE_URL_SCHEMES` to apply in each of these cases, not only for user profiles via the UI form. (Later changed to "feat:" at @limiting-factor's suggestion, based on the observation that this change extends documented behavior.)
This PR deals with the server-side validation only. #12991 covers client-side validation (deriving a `pattern` attribute from `VALID_SITE_URL_SCHEMES`, etc.)
Closes#5519
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12962
Reviewed-by: limiting-factor <limiting-factor@noreply.codeberg.org>
Adds an option "force_overwrite_new_branch" when posting to
/repos/{owner}/{repo}/contents to modify multiple files in a repository.
When user provides both "branch" and "new_branch" options, and
"new_branch" already exists, the "force_overwrite_new_branch" option
allows the user to overwrite the existing branch. Under the hood this
amounts to a "git push --force".
[Issue #12600](https://codeberg.org/forgejo/forgejo/issues/12600)
### Tests for Go changes
- 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 ran...
- [x] `make pr-go` before pushing
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [ ] I did not document these changes and I do not expect someone else to do it.
- [x] API swagger docs updated
### Release notes
- [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
Co-authored-by: Rob Gonnella <rob.gonnella@papayapay.com>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12663
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Closes https://codeberg.org/forgejo/forgejo/issues/6093
This PR adds support for **multi-line review comments** on pull requests, allowing reviewers to select a range of lines in diffs instead of only a single line — similar to GitHub's implementation.
### Tests for Go changes
- I added test coverage for Go changes...
- [X] in their respective `*_test.go` for unit tests.
- [X] `make pr-go` before pushing
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12582
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
First, why was this header here in the first place? Cloudflare!
Cloudflare had a optimization setting called "auto-minfy" and would
minify HTML,JS,CSS - this included removing extra whitespaces from
`<code>` elements. That's a problem because files are shown per-line
with a `<code>` element and thus results in indentation being completely
gone. Gitea added a FAQ entry for this [1], but on the same day decided
to add the workaround in Gitea, the `no-transform` header [2].
I can't find a reference of this option and some posts suggests it's
been removed. Thus it no longer serves a need to be present in Forgejo.
That wasn't my intentional motivation to remove this. This header is
also causing that HAProxy will not compress responses [3] from Forgejo
which is not ideal for Codeberg, this behavior cannot be turned off or
be worked around.
Potential risk, some other CDN or some other Cloudflare option might
still do this removal of whitespace in `<code>` HTML tags, it seems
better to disable the feature than to have Forgejo add a header which is
also causing other side-effects. I'm not aware of this another CDN of
Cloudflare option so I don't want to mark it as breaking.
[1]: https://github.com/go-gitea/gitea/pull/20430
[2]: https://github.com/go-gitea/gitea/pull/20432
[3]: https://docs.haproxy.org/3.3/configuration.html#:~:text=the%20response%20contains%20the%20%22no-transform%22%20value%20in%20the%20%22Cache-control%22%20%20%20%20%20header
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12905
Reviewed-by: Otto <otto@codeberg.org>
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Thanks to forgejo/forgejo!10397 (by @voidcontext), the binary called on git hooks can now be dynamically set.
**This means that we can now run tests without needing to run `make gitea` first**! No more `Could not find gitea binary` or head-banging, when one forgets to re-compile it 🎉
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12855
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Since forgejo/forgejo!10397 and forgejo/forgejo!12335 have landed, there shouldn't be a need for the `hooks` directory in each repository.
This PR cleans up the `tests/gitea-repositories-meta/*` testdata.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12824
Reviewed-by: limiting-factor <limiting-factor@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Extracted from #12824 on suggestion of @limiting-factor; refactored after @Gusted pointed out forgejo/forgejo!12335.
Behavior change: previously a missing `hooks` folder in a repository tree (should not happen before forgejo/forgejo!12335) would return a 500 on `/api/v1/repos/%s/hooks/git`. It now returns a 200, with the same reply as an empty `hooks` folder.
Test has been added to ensure correct handling of missing `hooks` folder and of its creation if necessary.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12833
Reviewed-by: limiting-factor <limiting-factor@noreply.codeberg.org>
Prevent examples hooks, description file, and other files from the default template to be created during git init.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12335
Reviewed-by: Otto <otto@codeberg.org>
A handful of routes, described in this PR as "mixed routes", are currently accessible by both web-based sessions and authenticated API users. The goal of this PR is to allow access to these routes for Authorized Integrations as well, bringing them to full API compatibility (to my knowledge) with other authentication methods. These routes are impacted:
- `/{username}/{repo}/raw/*`
- `/{username}/{repo}/archive/*`
- `/{username}/{repo}/releases/download/{vTag}/{fileName}`
- `/{username}/{repo}/attachments/{uuid}`
- `/attachments/{uuid}`
The major work in this PR was to refactoring the existing authentication methods so that "path based matching" that they were currently doing was no longer required, as I didn't want to introduce that into Authorized Integrations. All the path based matching is removed in this PR, and authentication methods are enabled entirely by the middleware applied to their endpoints.
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). 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 for Go changes
- 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 ran...
- [x] `make pr-go` before pushing
### 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
- [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [x] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12776
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Since the final size is already known, no need to `ReadAll` a `LimitedReader`: directly `ReadFull` a properly sized buffer.
Tests are already present in `blob_test.go` (a failure can be triggered by creating a smaller `buf`).
`go test -run=TestBlob_Data -bench=Blob_Data -benchmem` before:
```
Benchmark_Blob_Data-18 43964 28727 ns/op 1373 B/op 11 allocs/op
```
After:
```
Benchmark_Blob_Data-18 41308 27679 ns/op 846 B/op 10 allocs/op
```
🎉 one allocation spared!
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12795
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
The goal is to enable access to Git LFS resources with Authorized Integrations JWTs.
Blocker that needed to be resolved is that adding the `AuthorizedIntegration` auth method would conflict with the LFS tokens, which are handed out during git ssh clones to allow access to LFS resources -- `AuthorizedIntegration` would mark these as `AuthenticationAttemptedIncorrectCredential`, and therefore the requests would 401 before they got to the LFS-specific token validation routines. The fix is to move LFS token authentication into an authentication group so that it could be resolved at the same time as the authorized integration, rather than doing it inside the LFS server routines.
Refactors for LFS tokens are covered by refreshed test automation. Authorized integrations LFS Access has been manually tested, and will be further covered in an end-to-end integration test (https://code.forgejo.org/forgejo/end-to-end/pulls/1954).
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). 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 for Go changes
- 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 ran...
- [ ] `make pr-go` before pushing
### 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
- [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [x] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12725
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
This PR adds zoekt as a code search engine for forgejo. This Pull Request is a continuation of the discussion #8302.
The meilisearch search engine was not suitable, as it is not designed for searching by code. The zoekt project was proposed instead. Zoekt copes well with code indexing, but its operating principle differs from such search engines as elasticsearch.
While elasticsearch can return a result in a ready-made form (with pagination, ready-made snippets, etc.) and forgejo only needs to show this result in the interface with a little work with the data, zoekt works completely differently.
Zoekt finds matches in the repository index and returns a response. The response contains a line with the search word, its number from the file, and also a context, if specified in the request. This response is not suitable for Forgejo, so you need to assemble it yourself. To assemble the response from Zoekt into a form acceptable for Forgejo, I had to write some code and create a new function `searchZoektResult`, since the existing `searchResult` function is completely unsuitable for this search engine. I also had to write logic for pagination, highlighting, and correct display of lines in found snippets with a match, but this is a feature of Zoekt.
At the moment, Zoekt does not support deleting a repository index by repo_id, it only supports complete deletion of all repositories. But I still implemented the Delete function, which deletes a specific repository by its ID.
Co-authored-by: Aleksandr Gamzin <gamzin@altlinux.org>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8827
Reviewed-by: Shiny Nematoda <snematoda@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Load the primary language of the repository when it's converted to a API struct. This is simpler than adding `LoadAttributes` to a lot of places.
Resolvesforgejo/forgejo#12729
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12737
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
These are by the far the longest time spent on during a migration.
Indicate the progress of how many issues and PRs were migrated so far.
Don't overwhelm the messenger, so they are only updated once a batch is
migrated. Which is "slow" enough to see it's not stuck and still doing work.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12738
Reviewed-by: Ellen Εμίλια Άννα Zscheile <fogti@noreply.codeberg.org>
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Forgejo would not trigger Actions workflows `on: pull_request:` with `paths:` or `paths-ignore:` filters when the pull request was merged. The reason was that the triggers were evaluated after the PR was merged, but Forgejo still looked for changed files between the base branch and the PR's HEAD, which by then was already in the base branch.
Resolves https://codeberg.org/forgejo/forgejo/issues/12585.
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). 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 for Go changes
(can be removed for JavaScript changes)
- 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 ran...
- [x] `make pr-go` before pushing
### Tests for JavaScript changes
(can be removed for Go changes)
- 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] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
*The decision if the pull request will be shown in the release notes is up to the mergers / release team.*
The content of the `release-notes/<pull request number>.md` file will serve as the basis for the release notes. If the file does not exist, the title of the pull request will be used instead.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12739
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Allow authentication to git HTTP & git LFS via an authorized integration.
This is the first step in getting rid of OAuth, basic auth, etc.'s usage of [`isGitRawOrAttachPath(req)`](26f18a94ee/services/auth/method/basic.go (L38-L40)). I don't want to follow that pattern of HTTP route matching in the authentication method, so I've broken the HTTP routes related to git functionality out to using a separate authentication middleware in the top-level `web.Routes` handler. As this approach is expanded to the other endpoints in order to add support to them for authorized integrations, eventually it will be possible to remove this URL matching completely and just rely on middleware installation.
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). 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 for Go changes
- 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 ran...
- [x] `make pr-go` before pushing
### 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
- [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [x] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12715
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Weirdly, git doesn't verify the consistency of objects when receiving
new objects. Enable that git verifies this, so we don't allow a
repository to get in a weird or even corrupt state.
We've already dealt with a few cases of inconsistent objects, the most
notable one being mode of objects (forgejo/forgejo!9161). This can be
risky, as such ignore 3 consistency checks that are not harmful to
ignore and is battle tested by Gitlab.
bad timezone:
692a0d3476
missing space:
2da0b39399
non-zero padded filemode:
db8f2e8da5
Typically we set these settings in `modules/git/git.go`, but that means
a instance administrator wouldn't be able to override it. Given we don't
strictly require these settings to be set. A instance admin could
choose to disable the consistency checks or override our set of ignores
this would allow them to do so via the `[git.config]` section.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12695
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Reviewed-by: elle <0xllx0@noreply.codeberg.org>
* remove two unused strings I identified while doing other things
* update two strings per request of @mahlzahn while avoiding a whole separate PR for this
* move 126 strings to JSON, some are remapped with a better structure
* previous migration: forgejo/forgejo!12280
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12718
Reviewed-by: Robert Wolff <mahlzahn@posteo.de>
`ENABLE_AUTO_GIT_WIRE_PROTOCOL`:
Its sole usage is to set `-c protocol.version=2` on each git command
execution. The default value is already 2 since at least the minimum
version of Git that Forgejo requires. When this setting was added, this
was not the case.
Thus, automatically defaulting to protocol v2 is already the case due to
git themselves making it the default. And instances that want to use a
older protocol already have to override the value like:
```ini
[git.config]
protocol.version=1
```
---
`git.reflog` was deprecated in v1.21 warnings have been emitted. Remove it finally.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12681
Reviewed-by: limiting-factor <limiting-factor@noreply.codeberg.org>
Currently, Forgejo supports configuring static group team mappings for
an OIDC authentication source that map OIDC groups to Forgejo
organizations and teams. For example, the following mapping
```json
{"Developer": {"MyForgejoOrganization": ["MyForgejoTeam1", "MyForgejoTeam2"]}}
```
automatically adds a user in the OIDC group `Developer` to the teams
`MyForgejoTeam1` and `MyForgejoTeam2` in organization
`MyForgejoOrganization`.
In order to support more dynamic mappings and to avoid having to update
the mappings for new organizations and teams, add an additional
configuration option that supports mappings with placeholders like in
the following example:
```json
["group-{org}-{team}", "other:{org}/{team}"]
```
In this example, the mappings add a user in OIDC groups
`group-org1-team1`, `group-org2-team2`, and `other:org3/team3` to team
`team1` in organization `org1`, team `team2` in organization `org2`, and
to team `team3` in organization `org3`.
Additionally, this adds a configuration option to dynamically remove
users from organization teams. If enabled, a user is removed from all
teams that are not added via a static or dynamic mapping. Thus, users
are only in teams that are added via such a mapping and no other teams.
Docs: forgejo/docs!1950
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11656
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
## Checklist
Following the previous contribution that added admin-level management of user access tokens (particularly useful for bot/service accounts), this change exposes the created_at field in the API response when listing or retrieving access tokens.
This information is needed to implement token rotation policies for these users — knowing when a token was created allows administrators to identify and revoke stale tokens.
### Tests for Go changes
- I added test coverage for Go changes...
- [X] in their respective `*_test.go` for unit tests.
- [X] `make pr-go` before pushing
### Documentation
- [X] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [X] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
<!--start release-notes-assistant-->
## Release notes
<!--URL:https://codeberg.org/forgejo/forgejo-->
- Features
- [PR](https://codeberg.org/forgejo/forgejo/pulls/12620): <!--number 12620 --><!--line 0 --><!--description ZXhwb3NlIGFjY2VzcyB0b2tlbiBjcmVhdGlvbiBkYXRlIGluIEFQSSByZXNwb25zZXM=-->expose access token creation date in API responses<!--description-->
<!--end release-notes-assistant-->
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12620
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
The original issue only mentions 'Verified', but 'Updated' was also
missing and so is also included.
The integration test only covers the initial `false` state. Attempting
to cover the flip to true seemed to introduce more problems than
benefits (as outlined in `tests/integration/api_keys_test.go`)
Manual testing was performed to check that verifying the key in the web
ui caused the return value to change from false to true in the API
response (using `curl`).
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). 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 for Go changes
(can be removed for JavaScript changes)
- 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 ran...
- [x] `make pr-go` before pushing
### Tests for JavaScript changes
(can be removed for Go changes)
- 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] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12625
Reviewed-by: Cyborus <cyborus@disroot.org>
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Extends the UI introduced in #12558 to have edit capabilities. (not in scope: "Add" for a new Authorized Integration will be the next update to this UI; `create-authorized-integration` CLI is still the only way to create a new record)
This PR includes a few refactoring steps. The goal of these steps is to have `services/auth` be a single entrypoint for validating, inserting, or updating an authorized integration. Some logic is moved out of `services/authz` because it is not authorization related, and some is moved out of `services/auth/method` to allow it to be reused during validation without creating a cyclical module dependency.
This PR also adds comprehensive validation to the more complex fields in the authorized integration, such as the issuer and claim rules. This validation applies to the `forgejo admin user create-authorized-integration` CLI as well.
The visible UI is the same as #12558, but with a "Save" button, and the ability to display errors:
## Checklist
The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). 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 for Go changes
- 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 ran...
- [x] `make pr-go` before pushing
### Tests for JavaScript changes
- I added test coverage for JavaScript changes...
- [ ] in `web_src/js/*.test.js` if it can be unit tested.
- [x] 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.
- Documentation is on my TODO list and will be completed before release.
### Release notes
- [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12601
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
