jojo/modules/setting/security.go

369 lines
13 KiB
Go
Raw Permalink Normal View History

// Copyright 2023 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package setting
import (
chore: unify signing key configuration across modules (#11194) ## Context the three commits in this series are the first step towards the goal of removing the special casing around `JWT_SECRET`, which is used for various modules via `GetGeneralTokenSigningSecret()`. Ultimately, I want to work towards enabling seamless migration away from general use of the common secret. To enable this, we need proper secret/key rotation support, that is, we need to allow for configuration of additional secrets/keys which are accepted for token validation, but not used to issue tokens. I have this _Verifier_ support basically implemented, but this PR is not it. This PR contains cleanup refactoring which I worked on before writing the _Verifier_ support, because I noticed that the existing secret/key handling across modules was inconsistent and required duplicated code. I am submitting this part now to allow for incremental review of not too large a diff, and because these commits remained unchanged during two weeks since I moved on the the next task. ## The problem being addressed Configuration of JWT signing secrets/keys was inconsistent: Under `[oauth2]` the full configuration set was supported: - `JWT_SIGNING_ALGORITHM` configured the algorithm - `JWT_SECRET` configured a literal secret for symmetric algorithms - `JWT_SECRET_URI` configured a `file:` uri of a secret for symmetric algorithms - `JWT_SIGNING_PRIVATE_KEY_FILE` configured a file for asymmetric algorithms For `[server]`, the LFS module only supported `LFS_JWT_SECRET`, and the signing method was hardcoded to `HS256` For `[actions]`, only asymmetric signing methods were supported via `ID_TOKEN_SIGNING_ALGORITHM` and `ID_TOKEN_SIGNING_PRIVATE_KEY_FILE`. ## ini unification The proposed code centralizes ini parsing to always support the following ini keys: - `[pfx]SIGNING_ALGORITHM` determines the algorithm - `[pfx]SECRET` is a literal secret for symmetric algorithms - `[pfx]SECRET_URI` is the uri of a secret for symmetric algorithms - `[pfx]SIGNING_PRIVATE_KEY_FILE` is a file with a private key for asymmetric algorithms `[pfx]` is specific to the module and chosen to support the existing ini keys Centralizing this code and unifying the ini keys will come handy for at least the following reasons: - consistent behavior across modules is easier to understand - less duplicated code - easier to expand later, which is my main motivation ## implementation notes as might be apparent by the _take3_ branch name, this is the third iteration of this patch series. The main reason why I abandoned the other two is that I first tried to move all the key initialization into the code called from settings.go when the ini file is parsed. But that lead to a lot of friction with test cases, because private key files which are configured, but do not exist will get created and hence require a writable `AppDataPath` and additional clean up. To avoid a lot of noise and complications in test cases, I kept the existing two stage process, where - the settings component creates missing symmetric signing keys and writes them to the .ini - the settings component creates a simple configuration struct - which is then used from the module init to create the actual key, which also includes creating a private key file if asymmetric crypto is configured and the key file does not exist. I would have wished this patch was a net negative in terms of LOCs, but I hope it contributes to clarity and many added lines are in test cases. ## Commits Because sometimes PRs are merged as squashes with the PR text remaining, I am repeating here the individual messages of the individual commits for future reference: ### Refactor signing key initalization and oauth2 use of it This commit is the first in a series towards the goal of addressing the FIXME comment in modules/setting/oauth2.go to remove GeneralTokenSigningSecret To do it properly, the task also requires addition of signing secret/key rotation: We ultimately want to be able to change a signing key, but continue to accept the previous one. This is particularly relevant to offer a path from GeneralTokenSigningSecret aka JWT_SECRET to new, specific component key configuration, where it should be possible to add the former JWT_SECRET as a key accepted for verification to enable a seamless transition. This perspective, in turn, calls for refactoring of the existing secret initialization code to centralize the common functions of parsing signing key related configuration directives: The oauth2 module currently is the only component accepting symmetric and asymmetric keys, with the limitation of the symmetric key being also the GeneralTokenSigningSecret. Other components either enforce HS256 or public key algorithms. We should really give the choice of algorithm selection and avoid code duplication in other places, so this commit - generalizes setting parsing into a configuration struct: A prefix can be provided, with which the common configuration directives are processed: - [pfx]SIGNING_ALGORITHM determines the algorithm - [pfx]SECRET is a literal secret for symmetric algorithms - [pfx]SECRET_URI is the uri of a secret for symmetric algorithms - [pfx]SIGNING_PRIVATE_KEY_FILE is a file with a private key for asymmetric algorithms - which is then accepted by jwtx.InitSigningKey() to create an actual signing key The reasons for the two stage process are explained in a long-ish comment in modules/setting/security.go. In short, other options would either violate sensible module boundaries or cause too much friction. These other options have actually been tried, this is take 3 of the proposed changes. ### Refactor services/lfs: Change token code to use SigningKey This now also enables use of token algorithms other than HS256. In this case, signing key initialization also happens during settings initialization, because LFS is also used in CLI commands. ### Refactor api/actions to use new signingkey API This now also enables use of symmetric token algorithms. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11194 Reviewed-by: Gusted <gusted@noreply.codeberg.org> Co-authored-by: Nils Goroll <nils.goroll@uplex.de> Co-committed-by: Nils Goroll <nils.goroll@uplex.de>
2026-04-21 19:39:33 +02:00
"fmt"
"net/url"
"os"
chore: unify signing key configuration across modules (#11194) ## Context the three commits in this series are the first step towards the goal of removing the special casing around `JWT_SECRET`, which is used for various modules via `GetGeneralTokenSigningSecret()`. Ultimately, I want to work towards enabling seamless migration away from general use of the common secret. To enable this, we need proper secret/key rotation support, that is, we need to allow for configuration of additional secrets/keys which are accepted for token validation, but not used to issue tokens. I have this _Verifier_ support basically implemented, but this PR is not it. This PR contains cleanup refactoring which I worked on before writing the _Verifier_ support, because I noticed that the existing secret/key handling across modules was inconsistent and required duplicated code. I am submitting this part now to allow for incremental review of not too large a diff, and because these commits remained unchanged during two weeks since I moved on the the next task. ## The problem being addressed Configuration of JWT signing secrets/keys was inconsistent: Under `[oauth2]` the full configuration set was supported: - `JWT_SIGNING_ALGORITHM` configured the algorithm - `JWT_SECRET` configured a literal secret for symmetric algorithms - `JWT_SECRET_URI` configured a `file:` uri of a secret for symmetric algorithms - `JWT_SIGNING_PRIVATE_KEY_FILE` configured a file for asymmetric algorithms For `[server]`, the LFS module only supported `LFS_JWT_SECRET`, and the signing method was hardcoded to `HS256` For `[actions]`, only asymmetric signing methods were supported via `ID_TOKEN_SIGNING_ALGORITHM` and `ID_TOKEN_SIGNING_PRIVATE_KEY_FILE`. ## ini unification The proposed code centralizes ini parsing to always support the following ini keys: - `[pfx]SIGNING_ALGORITHM` determines the algorithm - `[pfx]SECRET` is a literal secret for symmetric algorithms - `[pfx]SECRET_URI` is the uri of a secret for symmetric algorithms - `[pfx]SIGNING_PRIVATE_KEY_FILE` is a file with a private key for asymmetric algorithms `[pfx]` is specific to the module and chosen to support the existing ini keys Centralizing this code and unifying the ini keys will come handy for at least the following reasons: - consistent behavior across modules is easier to understand - less duplicated code - easier to expand later, which is my main motivation ## implementation notes as might be apparent by the _take3_ branch name, this is the third iteration of this patch series. The main reason why I abandoned the other two is that I first tried to move all the key initialization into the code called from settings.go when the ini file is parsed. But that lead to a lot of friction with test cases, because private key files which are configured, but do not exist will get created and hence require a writable `AppDataPath` and additional clean up. To avoid a lot of noise and complications in test cases, I kept the existing two stage process, where - the settings component creates missing symmetric signing keys and writes them to the .ini - the settings component creates a simple configuration struct - which is then used from the module init to create the actual key, which also includes creating a private key file if asymmetric crypto is configured and the key file does not exist. I would have wished this patch was a net negative in terms of LOCs, but I hope it contributes to clarity and many added lines are in test cases. ## Commits Because sometimes PRs are merged as squashes with the PR text remaining, I am repeating here the individual messages of the individual commits for future reference: ### Refactor signing key initalization and oauth2 use of it This commit is the first in a series towards the goal of addressing the FIXME comment in modules/setting/oauth2.go to remove GeneralTokenSigningSecret To do it properly, the task also requires addition of signing secret/key rotation: We ultimately want to be able to change a signing key, but continue to accept the previous one. This is particularly relevant to offer a path from GeneralTokenSigningSecret aka JWT_SECRET to new, specific component key configuration, where it should be possible to add the former JWT_SECRET as a key accepted for verification to enable a seamless transition. This perspective, in turn, calls for refactoring of the existing secret initialization code to centralize the common functions of parsing signing key related configuration directives: The oauth2 module currently is the only component accepting symmetric and asymmetric keys, with the limitation of the symmetric key being also the GeneralTokenSigningSecret. Other components either enforce HS256 or public key algorithms. We should really give the choice of algorithm selection and avoid code duplication in other places, so this commit - generalizes setting parsing into a configuration struct: A prefix can be provided, with which the common configuration directives are processed: - [pfx]SIGNING_ALGORITHM determines the algorithm - [pfx]SECRET is a literal secret for symmetric algorithms - [pfx]SECRET_URI is the uri of a secret for symmetric algorithms - [pfx]SIGNING_PRIVATE_KEY_FILE is a file with a private key for asymmetric algorithms - which is then accepted by jwtx.InitSigningKey() to create an actual signing key The reasons for the two stage process are explained in a long-ish comment in modules/setting/security.go. In short, other options would either violate sensible module boundaries or cause too much friction. These other options have actually been tried, this is take 3 of the proposed changes. ### Refactor services/lfs: Change token code to use SigningKey This now also enables use of token algorithms other than HS256. In this case, signing key initialization also happens during settings initialization, because LFS is also used in CLI commands. ### Refactor api/actions to use new signingkey API This now also enables use of symmetric token algorithms. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11194 Reviewed-by: Gusted <gusted@noreply.codeberg.org> Co-authored-by: Nils Goroll <nils.goroll@uplex.de> Co-committed-by: Nils Goroll <nils.goroll@uplex.de>
2026-04-21 19:39:33 +02:00
"path/filepath"
"strings"
"forgejo.org/modules/auth/password/hash"
"forgejo.org/modules/generate"
chore: unify signing key configuration across modules (#11194) ## Context the three commits in this series are the first step towards the goal of removing the special casing around `JWT_SECRET`, which is used for various modules via `GetGeneralTokenSigningSecret()`. Ultimately, I want to work towards enabling seamless migration away from general use of the common secret. To enable this, we need proper secret/key rotation support, that is, we need to allow for configuration of additional secrets/keys which are accepted for token validation, but not used to issue tokens. I have this _Verifier_ support basically implemented, but this PR is not it. This PR contains cleanup refactoring which I worked on before writing the _Verifier_ support, because I noticed that the existing secret/key handling across modules was inconsistent and required duplicated code. I am submitting this part now to allow for incremental review of not too large a diff, and because these commits remained unchanged during two weeks since I moved on the the next task. ## The problem being addressed Configuration of JWT signing secrets/keys was inconsistent: Under `[oauth2]` the full configuration set was supported: - `JWT_SIGNING_ALGORITHM` configured the algorithm - `JWT_SECRET` configured a literal secret for symmetric algorithms - `JWT_SECRET_URI` configured a `file:` uri of a secret for symmetric algorithms - `JWT_SIGNING_PRIVATE_KEY_FILE` configured a file for asymmetric algorithms For `[server]`, the LFS module only supported `LFS_JWT_SECRET`, and the signing method was hardcoded to `HS256` For `[actions]`, only asymmetric signing methods were supported via `ID_TOKEN_SIGNING_ALGORITHM` and `ID_TOKEN_SIGNING_PRIVATE_KEY_FILE`. ## ini unification The proposed code centralizes ini parsing to always support the following ini keys: - `[pfx]SIGNING_ALGORITHM` determines the algorithm - `[pfx]SECRET` is a literal secret for symmetric algorithms - `[pfx]SECRET_URI` is the uri of a secret for symmetric algorithms - `[pfx]SIGNING_PRIVATE_KEY_FILE` is a file with a private key for asymmetric algorithms `[pfx]` is specific to the module and chosen to support the existing ini keys Centralizing this code and unifying the ini keys will come handy for at least the following reasons: - consistent behavior across modules is easier to understand - less duplicated code - easier to expand later, which is my main motivation ## implementation notes as might be apparent by the _take3_ branch name, this is the third iteration of this patch series. The main reason why I abandoned the other two is that I first tried to move all the key initialization into the code called from settings.go when the ini file is parsed. But that lead to a lot of friction with test cases, because private key files which are configured, but do not exist will get created and hence require a writable `AppDataPath` and additional clean up. To avoid a lot of noise and complications in test cases, I kept the existing two stage process, where - the settings component creates missing symmetric signing keys and writes them to the .ini - the settings component creates a simple configuration struct - which is then used from the module init to create the actual key, which also includes creating a private key file if asymmetric crypto is configured and the key file does not exist. I would have wished this patch was a net negative in terms of LOCs, but I hope it contributes to clarity and many added lines are in test cases. ## Commits Because sometimes PRs are merged as squashes with the PR text remaining, I am repeating here the individual messages of the individual commits for future reference: ### Refactor signing key initalization and oauth2 use of it This commit is the first in a series towards the goal of addressing the FIXME comment in modules/setting/oauth2.go to remove GeneralTokenSigningSecret To do it properly, the task also requires addition of signing secret/key rotation: We ultimately want to be able to change a signing key, but continue to accept the previous one. This is particularly relevant to offer a path from GeneralTokenSigningSecret aka JWT_SECRET to new, specific component key configuration, where it should be possible to add the former JWT_SECRET as a key accepted for verification to enable a seamless transition. This perspective, in turn, calls for refactoring of the existing secret initialization code to centralize the common functions of parsing signing key related configuration directives: The oauth2 module currently is the only component accepting symmetric and asymmetric keys, with the limitation of the symmetric key being also the GeneralTokenSigningSecret. Other components either enforce HS256 or public key algorithms. We should really give the choice of algorithm selection and avoid code duplication in other places, so this commit - generalizes setting parsing into a configuration struct: A prefix can be provided, with which the common configuration directives are processed: - [pfx]SIGNING_ALGORITHM determines the algorithm - [pfx]SECRET is a literal secret for symmetric algorithms - [pfx]SECRET_URI is the uri of a secret for symmetric algorithms - [pfx]SIGNING_PRIVATE_KEY_FILE is a file with a private key for asymmetric algorithms - which is then accepted by jwtx.InitSigningKey() to create an actual signing key The reasons for the two stage process are explained in a long-ish comment in modules/setting/security.go. In short, other options would either violate sensible module boundaries or cause too much friction. These other options have actually been tried, this is take 3 of the proposed changes. ### Refactor services/lfs: Change token code to use SigningKey This now also enables use of token algorithms other than HS256. In this case, signing key initialization also happens during settings initialization, because LFS is also used in CLI commands. ### Refactor api/actions to use new signingkey API This now also enables use of symmetric token algorithms. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11194 Reviewed-by: Gusted <gusted@noreply.codeberg.org> Co-authored-by: Nils Goroll <nils.goroll@uplex.de> Co-committed-by: Nils Goroll <nils.goroll@uplex.de>
2026-04-21 19:39:33 +02:00
"forgejo.org/modules/jwtx"
"forgejo.org/modules/keying"
"forgejo.org/modules/log"
)
var (
// Security settings
InstallLock bool
SecretKey string
InternalToken string // internal access token
LogInRememberDays int
feat: Global 2FA enforcement (#8753) resolves #8549 This PR add a config to enforce 2FA for the whole Forgejo instance. It can be configured to `none`, `admin` or `all`. A user who is required to enable 2FA is like a disabled user. He can only see the `/user/settings/security`-Page to enable 2FA, this should be similar to a user which needs to change his password. Also api and git-commands are not allowed. ## 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. - [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. - [ ] I did not document these changes and I do not expect someone else to do it. I will do it, if the general idea of this PR is a good feature. ### 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--> - Security features - [PR](https://codeberg.org/forgejo/forgejo/pulls/8753): <!--number 8753 --><!--line 0 --><!--description R2xvYmFsIDJGQSBlbmZvcmNlbWVudA==-->Global 2FA enforcement<!--description--> <!--end release-notes-assistant--> Co-authored-by: 0ko <0ko@noreply.codeberg.org> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8753 Reviewed-by: 0ko <0ko@noreply.codeberg.org> Reviewed-by: Ellen Εμιλία Άννα Zscheile <fogti@noreply.codeberg.org> Reviewed-by: Gusted <gusted@noreply.codeberg.org> Co-authored-by: zokki <zokki.softwareschmiede@gmail.com> Co-committed-by: zokki <zokki.softwareschmiede@gmail.com>
2025-08-15 10:56:45 +02:00
GlobalTwoFactorRequirement TwoFactorRequirementType
CookieRememberName string
ReverseProxyAuthUser string
ReverseProxyAuthEmail string
ReverseProxyAuthFullName string
ReverseProxyLimit int
ReverseProxyTrustedProxies []string
MinPasswordLength int
ImportLocalPaths bool
DisableGitHooks bool
DisableWebhooks bool
OnlyAllowPushIfGiteaEnvironmentSet bool
PasswordComplexity []string
PasswordHashAlgo string
PasswordCheckPwn bool
SuccessfulTokensCacheSize int
DisableQueryAuthToken bool
)
chore: unify signing key configuration across modules (#11194) ## Context the three commits in this series are the first step towards the goal of removing the special casing around `JWT_SECRET`, which is used for various modules via `GetGeneralTokenSigningSecret()`. Ultimately, I want to work towards enabling seamless migration away from general use of the common secret. To enable this, we need proper secret/key rotation support, that is, we need to allow for configuration of additional secrets/keys which are accepted for token validation, but not used to issue tokens. I have this _Verifier_ support basically implemented, but this PR is not it. This PR contains cleanup refactoring which I worked on before writing the _Verifier_ support, because I noticed that the existing secret/key handling across modules was inconsistent and required duplicated code. I am submitting this part now to allow for incremental review of not too large a diff, and because these commits remained unchanged during two weeks since I moved on the the next task. ## The problem being addressed Configuration of JWT signing secrets/keys was inconsistent: Under `[oauth2]` the full configuration set was supported: - `JWT_SIGNING_ALGORITHM` configured the algorithm - `JWT_SECRET` configured a literal secret for symmetric algorithms - `JWT_SECRET_URI` configured a `file:` uri of a secret for symmetric algorithms - `JWT_SIGNING_PRIVATE_KEY_FILE` configured a file for asymmetric algorithms For `[server]`, the LFS module only supported `LFS_JWT_SECRET`, and the signing method was hardcoded to `HS256` For `[actions]`, only asymmetric signing methods were supported via `ID_TOKEN_SIGNING_ALGORITHM` and `ID_TOKEN_SIGNING_PRIVATE_KEY_FILE`. ## ini unification The proposed code centralizes ini parsing to always support the following ini keys: - `[pfx]SIGNING_ALGORITHM` determines the algorithm - `[pfx]SECRET` is a literal secret for symmetric algorithms - `[pfx]SECRET_URI` is the uri of a secret for symmetric algorithms - `[pfx]SIGNING_PRIVATE_KEY_FILE` is a file with a private key for asymmetric algorithms `[pfx]` is specific to the module and chosen to support the existing ini keys Centralizing this code and unifying the ini keys will come handy for at least the following reasons: - consistent behavior across modules is easier to understand - less duplicated code - easier to expand later, which is my main motivation ## implementation notes as might be apparent by the _take3_ branch name, this is the third iteration of this patch series. The main reason why I abandoned the other two is that I first tried to move all the key initialization into the code called from settings.go when the ini file is parsed. But that lead to a lot of friction with test cases, because private key files which are configured, but do not exist will get created and hence require a writable `AppDataPath` and additional clean up. To avoid a lot of noise and complications in test cases, I kept the existing two stage process, where - the settings component creates missing symmetric signing keys and writes them to the .ini - the settings component creates a simple configuration struct - which is then used from the module init to create the actual key, which also includes creating a private key file if asymmetric crypto is configured and the key file does not exist. I would have wished this patch was a net negative in terms of LOCs, but I hope it contributes to clarity and many added lines are in test cases. ## Commits Because sometimes PRs are merged as squashes with the PR text remaining, I am repeating here the individual messages of the individual commits for future reference: ### Refactor signing key initalization and oauth2 use of it This commit is the first in a series towards the goal of addressing the FIXME comment in modules/setting/oauth2.go to remove GeneralTokenSigningSecret To do it properly, the task also requires addition of signing secret/key rotation: We ultimately want to be able to change a signing key, but continue to accept the previous one. This is particularly relevant to offer a path from GeneralTokenSigningSecret aka JWT_SECRET to new, specific component key configuration, where it should be possible to add the former JWT_SECRET as a key accepted for verification to enable a seamless transition. This perspective, in turn, calls for refactoring of the existing secret initialization code to centralize the common functions of parsing signing key related configuration directives: The oauth2 module currently is the only component accepting symmetric and asymmetric keys, with the limitation of the symmetric key being also the GeneralTokenSigningSecret. Other components either enforce HS256 or public key algorithms. We should really give the choice of algorithm selection and avoid code duplication in other places, so this commit - generalizes setting parsing into a configuration struct: A prefix can be provided, with which the common configuration directives are processed: - [pfx]SIGNING_ALGORITHM determines the algorithm - [pfx]SECRET is a literal secret for symmetric algorithms - [pfx]SECRET_URI is the uri of a secret for symmetric algorithms - [pfx]SIGNING_PRIVATE_KEY_FILE is a file with a private key for asymmetric algorithms - which is then accepted by jwtx.InitSigningKey() to create an actual signing key The reasons for the two stage process are explained in a long-ish comment in modules/setting/security.go. In short, other options would either violate sensible module boundaries or cause too much friction. These other options have actually been tried, this is take 3 of the proposed changes. ### Refactor services/lfs: Change token code to use SigningKey This now also enables use of token algorithms other than HS256. In this case, signing key initialization also happens during settings initialization, because LFS is also used in CLI commands. ### Refactor api/actions to use new signingkey API This now also enables use of symmetric token algorithms. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11194 Reviewed-by: Gusted <gusted@noreply.codeberg.org> Co-authored-by: Nils Goroll <nils.goroll@uplex.de> Co-committed-by: Nils Goroll <nils.goroll@uplex.de>
2026-04-21 19:39:33 +02:00
/*
* key loading is a two-stage process to avoid complications for unit tests:
*
* For symmetric keys, we want to add a random key to the configuration. We would
* not want to change the configuration after loading has completed to maintain
* isolation. So from this perspective, we would want to initialize keys only
* during setting.load...From()
*
* For asymmetric keys, we want to create a random private key _file_.
* Doing so during the setting load phase, however, creates key files for unit
* tests, because they usually complete the full settings load phase, but do not
* initialize modules which they do not need. Depending on the test case, it
* might not provide a writable AppDataPath, or it would leave private key files
* in the source tree. All of this can be avoided with
* specifically adjusting the ini loaded for unit tests, but adds considerable
* friction.
*
* So to avoid all this, we split key loading in two phases:
* - settings parse the config and save missing symmetric keys
* - module init takes the parsed config, creates missing asymmetric keys and
* creates the actual signingkey objects
*
* jwtx.SigningKeyCfg and jwtx.KeyCfg are used for handover
*/
// loadSecret load the secret from ini by uriKey or verbatimKey, only one of them could be set
// If the secret is loaded from uriKey (file), the file should be non-empty, to guarantee the behavior stable and clear.
func loadSecret(sec ConfigSection, uriKey, verbatimKey string) string {
// don't allow setting both URI and verbatim string
uri := sec.Key(uriKey).String()
verbatim := sec.Key(verbatimKey).String()
if uri != "" && verbatim != "" {
log.Fatal("Cannot specify both %s and %s", uriKey, verbatimKey)
}
// if we have no URI, use verbatim
if uri == "" {
return verbatim
}
chore: unify signing key configuration across modules (#11194) ## Context the three commits in this series are the first step towards the goal of removing the special casing around `JWT_SECRET`, which is used for various modules via `GetGeneralTokenSigningSecret()`. Ultimately, I want to work towards enabling seamless migration away from general use of the common secret. To enable this, we need proper secret/key rotation support, that is, we need to allow for configuration of additional secrets/keys which are accepted for token validation, but not used to issue tokens. I have this _Verifier_ support basically implemented, but this PR is not it. This PR contains cleanup refactoring which I worked on before writing the _Verifier_ support, because I noticed that the existing secret/key handling across modules was inconsistent and required duplicated code. I am submitting this part now to allow for incremental review of not too large a diff, and because these commits remained unchanged during two weeks since I moved on the the next task. ## The problem being addressed Configuration of JWT signing secrets/keys was inconsistent: Under `[oauth2]` the full configuration set was supported: - `JWT_SIGNING_ALGORITHM` configured the algorithm - `JWT_SECRET` configured a literal secret for symmetric algorithms - `JWT_SECRET_URI` configured a `file:` uri of a secret for symmetric algorithms - `JWT_SIGNING_PRIVATE_KEY_FILE` configured a file for asymmetric algorithms For `[server]`, the LFS module only supported `LFS_JWT_SECRET`, and the signing method was hardcoded to `HS256` For `[actions]`, only asymmetric signing methods were supported via `ID_TOKEN_SIGNING_ALGORITHM` and `ID_TOKEN_SIGNING_PRIVATE_KEY_FILE`. ## ini unification The proposed code centralizes ini parsing to always support the following ini keys: - `[pfx]SIGNING_ALGORITHM` determines the algorithm - `[pfx]SECRET` is a literal secret for symmetric algorithms - `[pfx]SECRET_URI` is the uri of a secret for symmetric algorithms - `[pfx]SIGNING_PRIVATE_KEY_FILE` is a file with a private key for asymmetric algorithms `[pfx]` is specific to the module and chosen to support the existing ini keys Centralizing this code and unifying the ini keys will come handy for at least the following reasons: - consistent behavior across modules is easier to understand - less duplicated code - easier to expand later, which is my main motivation ## implementation notes as might be apparent by the _take3_ branch name, this is the third iteration of this patch series. The main reason why I abandoned the other two is that I first tried to move all the key initialization into the code called from settings.go when the ini file is parsed. But that lead to a lot of friction with test cases, because private key files which are configured, but do not exist will get created and hence require a writable `AppDataPath` and additional clean up. To avoid a lot of noise and complications in test cases, I kept the existing two stage process, where - the settings component creates missing symmetric signing keys and writes them to the .ini - the settings component creates a simple configuration struct - which is then used from the module init to create the actual key, which also includes creating a private key file if asymmetric crypto is configured and the key file does not exist. I would have wished this patch was a net negative in terms of LOCs, but I hope it contributes to clarity and many added lines are in test cases. ## Commits Because sometimes PRs are merged as squashes with the PR text remaining, I am repeating here the individual messages of the individual commits for future reference: ### Refactor signing key initalization and oauth2 use of it This commit is the first in a series towards the goal of addressing the FIXME comment in modules/setting/oauth2.go to remove GeneralTokenSigningSecret To do it properly, the task also requires addition of signing secret/key rotation: We ultimately want to be able to change a signing key, but continue to accept the previous one. This is particularly relevant to offer a path from GeneralTokenSigningSecret aka JWT_SECRET to new, specific component key configuration, where it should be possible to add the former JWT_SECRET as a key accepted for verification to enable a seamless transition. This perspective, in turn, calls for refactoring of the existing secret initialization code to centralize the common functions of parsing signing key related configuration directives: The oauth2 module currently is the only component accepting symmetric and asymmetric keys, with the limitation of the symmetric key being also the GeneralTokenSigningSecret. Other components either enforce HS256 or public key algorithms. We should really give the choice of algorithm selection and avoid code duplication in other places, so this commit - generalizes setting parsing into a configuration struct: A prefix can be provided, with which the common configuration directives are processed: - [pfx]SIGNING_ALGORITHM determines the algorithm - [pfx]SECRET is a literal secret for symmetric algorithms - [pfx]SECRET_URI is the uri of a secret for symmetric algorithms - [pfx]SIGNING_PRIVATE_KEY_FILE is a file with a private key for asymmetric algorithms - which is then accepted by jwtx.InitSigningKey() to create an actual signing key The reasons for the two stage process are explained in a long-ish comment in modules/setting/security.go. In short, other options would either violate sensible module boundaries or cause too much friction. These other options have actually been tried, this is take 3 of the proposed changes. ### Refactor services/lfs: Change token code to use SigningKey This now also enables use of token algorithms other than HS256. In this case, signing key initialization also happens during settings initialization, because LFS is also used in CLI commands. ### Refactor api/actions to use new signingkey API This now also enables use of symmetric token algorithms. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11194 Reviewed-by: Gusted <gusted@noreply.codeberg.org> Co-authored-by: Nils Goroll <nils.goroll@uplex.de> Co-committed-by: Nils Goroll <nils.goroll@uplex.de>
2026-04-21 19:39:33 +02:00
verbatim, err := loadSecretFromURI(uri)
if err == nil {
return verbatim
}
log.Fatal("%s: %w", uriKey, err)
// unreached
return ""
}
chore: unify signing key configuration across modules (#11194) ## Context the three commits in this series are the first step towards the goal of removing the special casing around `JWT_SECRET`, which is used for various modules via `GetGeneralTokenSigningSecret()`. Ultimately, I want to work towards enabling seamless migration away from general use of the common secret. To enable this, we need proper secret/key rotation support, that is, we need to allow for configuration of additional secrets/keys which are accepted for token validation, but not used to issue tokens. I have this _Verifier_ support basically implemented, but this PR is not it. This PR contains cleanup refactoring which I worked on before writing the _Verifier_ support, because I noticed that the existing secret/key handling across modules was inconsistent and required duplicated code. I am submitting this part now to allow for incremental review of not too large a diff, and because these commits remained unchanged during two weeks since I moved on the the next task. ## The problem being addressed Configuration of JWT signing secrets/keys was inconsistent: Under `[oauth2]` the full configuration set was supported: - `JWT_SIGNING_ALGORITHM` configured the algorithm - `JWT_SECRET` configured a literal secret for symmetric algorithms - `JWT_SECRET_URI` configured a `file:` uri of a secret for symmetric algorithms - `JWT_SIGNING_PRIVATE_KEY_FILE` configured a file for asymmetric algorithms For `[server]`, the LFS module only supported `LFS_JWT_SECRET`, and the signing method was hardcoded to `HS256` For `[actions]`, only asymmetric signing methods were supported via `ID_TOKEN_SIGNING_ALGORITHM` and `ID_TOKEN_SIGNING_PRIVATE_KEY_FILE`. ## ini unification The proposed code centralizes ini parsing to always support the following ini keys: - `[pfx]SIGNING_ALGORITHM` determines the algorithm - `[pfx]SECRET` is a literal secret for symmetric algorithms - `[pfx]SECRET_URI` is the uri of a secret for symmetric algorithms - `[pfx]SIGNING_PRIVATE_KEY_FILE` is a file with a private key for asymmetric algorithms `[pfx]` is specific to the module and chosen to support the existing ini keys Centralizing this code and unifying the ini keys will come handy for at least the following reasons: - consistent behavior across modules is easier to understand - less duplicated code - easier to expand later, which is my main motivation ## implementation notes as might be apparent by the _take3_ branch name, this is the third iteration of this patch series. The main reason why I abandoned the other two is that I first tried to move all the key initialization into the code called from settings.go when the ini file is parsed. But that lead to a lot of friction with test cases, because private key files which are configured, but do not exist will get created and hence require a writable `AppDataPath` and additional clean up. To avoid a lot of noise and complications in test cases, I kept the existing two stage process, where - the settings component creates missing symmetric signing keys and writes them to the .ini - the settings component creates a simple configuration struct - which is then used from the module init to create the actual key, which also includes creating a private key file if asymmetric crypto is configured and the key file does not exist. I would have wished this patch was a net negative in terms of LOCs, but I hope it contributes to clarity and many added lines are in test cases. ## Commits Because sometimes PRs are merged as squashes with the PR text remaining, I am repeating here the individual messages of the individual commits for future reference: ### Refactor signing key initalization and oauth2 use of it This commit is the first in a series towards the goal of addressing the FIXME comment in modules/setting/oauth2.go to remove GeneralTokenSigningSecret To do it properly, the task also requires addition of signing secret/key rotation: We ultimately want to be able to change a signing key, but continue to accept the previous one. This is particularly relevant to offer a path from GeneralTokenSigningSecret aka JWT_SECRET to new, specific component key configuration, where it should be possible to add the former JWT_SECRET as a key accepted for verification to enable a seamless transition. This perspective, in turn, calls for refactoring of the existing secret initialization code to centralize the common functions of parsing signing key related configuration directives: The oauth2 module currently is the only component accepting symmetric and asymmetric keys, with the limitation of the symmetric key being also the GeneralTokenSigningSecret. Other components either enforce HS256 or public key algorithms. We should really give the choice of algorithm selection and avoid code duplication in other places, so this commit - generalizes setting parsing into a configuration struct: A prefix can be provided, with which the common configuration directives are processed: - [pfx]SIGNING_ALGORITHM determines the algorithm - [pfx]SECRET is a literal secret for symmetric algorithms - [pfx]SECRET_URI is the uri of a secret for symmetric algorithms - [pfx]SIGNING_PRIVATE_KEY_FILE is a file with a private key for asymmetric algorithms - which is then accepted by jwtx.InitSigningKey() to create an actual signing key The reasons for the two stage process are explained in a long-ish comment in modules/setting/security.go. In short, other options would either violate sensible module boundaries or cause too much friction. These other options have actually been tried, this is take 3 of the proposed changes. ### Refactor services/lfs: Change token code to use SigningKey This now also enables use of token algorithms other than HS256. In this case, signing key initialization also happens during settings initialization, because LFS is also used in CLI commands. ### Refactor api/actions to use new signingkey API This now also enables use of symmetric token algorithms. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11194 Reviewed-by: Gusted <gusted@noreply.codeberg.org> Co-authored-by: Nils Goroll <nils.goroll@uplex.de> Co-committed-by: Nils Goroll <nils.goroll@uplex.de>
2026-04-21 19:39:33 +02:00
func loadSecretFromURI(uri string) (string, error) {
tempURI, err := url.Parse(uri)
if err != nil {
chore: unify signing key configuration across modules (#11194) ## Context the three commits in this series are the first step towards the goal of removing the special casing around `JWT_SECRET`, which is used for various modules via `GetGeneralTokenSigningSecret()`. Ultimately, I want to work towards enabling seamless migration away from general use of the common secret. To enable this, we need proper secret/key rotation support, that is, we need to allow for configuration of additional secrets/keys which are accepted for token validation, but not used to issue tokens. I have this _Verifier_ support basically implemented, but this PR is not it. This PR contains cleanup refactoring which I worked on before writing the _Verifier_ support, because I noticed that the existing secret/key handling across modules was inconsistent and required duplicated code. I am submitting this part now to allow for incremental review of not too large a diff, and because these commits remained unchanged during two weeks since I moved on the the next task. ## The problem being addressed Configuration of JWT signing secrets/keys was inconsistent: Under `[oauth2]` the full configuration set was supported: - `JWT_SIGNING_ALGORITHM` configured the algorithm - `JWT_SECRET` configured a literal secret for symmetric algorithms - `JWT_SECRET_URI` configured a `file:` uri of a secret for symmetric algorithms - `JWT_SIGNING_PRIVATE_KEY_FILE` configured a file for asymmetric algorithms For `[server]`, the LFS module only supported `LFS_JWT_SECRET`, and the signing method was hardcoded to `HS256` For `[actions]`, only asymmetric signing methods were supported via `ID_TOKEN_SIGNING_ALGORITHM` and `ID_TOKEN_SIGNING_PRIVATE_KEY_FILE`. ## ini unification The proposed code centralizes ini parsing to always support the following ini keys: - `[pfx]SIGNING_ALGORITHM` determines the algorithm - `[pfx]SECRET` is a literal secret for symmetric algorithms - `[pfx]SECRET_URI` is the uri of a secret for symmetric algorithms - `[pfx]SIGNING_PRIVATE_KEY_FILE` is a file with a private key for asymmetric algorithms `[pfx]` is specific to the module and chosen to support the existing ini keys Centralizing this code and unifying the ini keys will come handy for at least the following reasons: - consistent behavior across modules is easier to understand - less duplicated code - easier to expand later, which is my main motivation ## implementation notes as might be apparent by the _take3_ branch name, this is the third iteration of this patch series. The main reason why I abandoned the other two is that I first tried to move all the key initialization into the code called from settings.go when the ini file is parsed. But that lead to a lot of friction with test cases, because private key files which are configured, but do not exist will get created and hence require a writable `AppDataPath` and additional clean up. To avoid a lot of noise and complications in test cases, I kept the existing two stage process, where - the settings component creates missing symmetric signing keys and writes them to the .ini - the settings component creates a simple configuration struct - which is then used from the module init to create the actual key, which also includes creating a private key file if asymmetric crypto is configured and the key file does not exist. I would have wished this patch was a net negative in terms of LOCs, but I hope it contributes to clarity and many added lines are in test cases. ## Commits Because sometimes PRs are merged as squashes with the PR text remaining, I am repeating here the individual messages of the individual commits for future reference: ### Refactor signing key initalization and oauth2 use of it This commit is the first in a series towards the goal of addressing the FIXME comment in modules/setting/oauth2.go to remove GeneralTokenSigningSecret To do it properly, the task also requires addition of signing secret/key rotation: We ultimately want to be able to change a signing key, but continue to accept the previous one. This is particularly relevant to offer a path from GeneralTokenSigningSecret aka JWT_SECRET to new, specific component key configuration, where it should be possible to add the former JWT_SECRET as a key accepted for verification to enable a seamless transition. This perspective, in turn, calls for refactoring of the existing secret initialization code to centralize the common functions of parsing signing key related configuration directives: The oauth2 module currently is the only component accepting symmetric and asymmetric keys, with the limitation of the symmetric key being also the GeneralTokenSigningSecret. Other components either enforce HS256 or public key algorithms. We should really give the choice of algorithm selection and avoid code duplication in other places, so this commit - generalizes setting parsing into a configuration struct: A prefix can be provided, with which the common configuration directives are processed: - [pfx]SIGNING_ALGORITHM determines the algorithm - [pfx]SECRET is a literal secret for symmetric algorithms - [pfx]SECRET_URI is the uri of a secret for symmetric algorithms - [pfx]SIGNING_PRIVATE_KEY_FILE is a file with a private key for asymmetric algorithms - which is then accepted by jwtx.InitSigningKey() to create an actual signing key The reasons for the two stage process are explained in a long-ish comment in modules/setting/security.go. In short, other options would either violate sensible module boundaries or cause too much friction. These other options have actually been tried, this is take 3 of the proposed changes. ### Refactor services/lfs: Change token code to use SigningKey This now also enables use of token algorithms other than HS256. In this case, signing key initialization also happens during settings initialization, because LFS is also used in CLI commands. ### Refactor api/actions to use new signingkey API This now also enables use of symmetric token algorithms. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11194 Reviewed-by: Gusted <gusted@noreply.codeberg.org> Co-authored-by: Nils Goroll <nils.goroll@uplex.de> Co-committed-by: Nils Goroll <nils.goroll@uplex.de>
2026-04-21 19:39:33 +02:00
return "", fmt.Errorf("Failed to parse %s: %v", uri, err)
}
switch tempURI.Scheme {
case "file":
chore: unify signing key configuration across modules (#11194) ## Context the three commits in this series are the first step towards the goal of removing the special casing around `JWT_SECRET`, which is used for various modules via `GetGeneralTokenSigningSecret()`. Ultimately, I want to work towards enabling seamless migration away from general use of the common secret. To enable this, we need proper secret/key rotation support, that is, we need to allow for configuration of additional secrets/keys which are accepted for token validation, but not used to issue tokens. I have this _Verifier_ support basically implemented, but this PR is not it. This PR contains cleanup refactoring which I worked on before writing the _Verifier_ support, because I noticed that the existing secret/key handling across modules was inconsistent and required duplicated code. I am submitting this part now to allow for incremental review of not too large a diff, and because these commits remained unchanged during two weeks since I moved on the the next task. ## The problem being addressed Configuration of JWT signing secrets/keys was inconsistent: Under `[oauth2]` the full configuration set was supported: - `JWT_SIGNING_ALGORITHM` configured the algorithm - `JWT_SECRET` configured a literal secret for symmetric algorithms - `JWT_SECRET_URI` configured a `file:` uri of a secret for symmetric algorithms - `JWT_SIGNING_PRIVATE_KEY_FILE` configured a file for asymmetric algorithms For `[server]`, the LFS module only supported `LFS_JWT_SECRET`, and the signing method was hardcoded to `HS256` For `[actions]`, only asymmetric signing methods were supported via `ID_TOKEN_SIGNING_ALGORITHM` and `ID_TOKEN_SIGNING_PRIVATE_KEY_FILE`. ## ini unification The proposed code centralizes ini parsing to always support the following ini keys: - `[pfx]SIGNING_ALGORITHM` determines the algorithm - `[pfx]SECRET` is a literal secret for symmetric algorithms - `[pfx]SECRET_URI` is the uri of a secret for symmetric algorithms - `[pfx]SIGNING_PRIVATE_KEY_FILE` is a file with a private key for asymmetric algorithms `[pfx]` is specific to the module and chosen to support the existing ini keys Centralizing this code and unifying the ini keys will come handy for at least the following reasons: - consistent behavior across modules is easier to understand - less duplicated code - easier to expand later, which is my main motivation ## implementation notes as might be apparent by the _take3_ branch name, this is the third iteration of this patch series. The main reason why I abandoned the other two is that I first tried to move all the key initialization into the code called from settings.go when the ini file is parsed. But that lead to a lot of friction with test cases, because private key files which are configured, but do not exist will get created and hence require a writable `AppDataPath` and additional clean up. To avoid a lot of noise and complications in test cases, I kept the existing two stage process, where - the settings component creates missing symmetric signing keys and writes them to the .ini - the settings component creates a simple configuration struct - which is then used from the module init to create the actual key, which also includes creating a private key file if asymmetric crypto is configured and the key file does not exist. I would have wished this patch was a net negative in terms of LOCs, but I hope it contributes to clarity and many added lines are in test cases. ## Commits Because sometimes PRs are merged as squashes with the PR text remaining, I am repeating here the individual messages of the individual commits for future reference: ### Refactor signing key initalization and oauth2 use of it This commit is the first in a series towards the goal of addressing the FIXME comment in modules/setting/oauth2.go to remove GeneralTokenSigningSecret To do it properly, the task also requires addition of signing secret/key rotation: We ultimately want to be able to change a signing key, but continue to accept the previous one. This is particularly relevant to offer a path from GeneralTokenSigningSecret aka JWT_SECRET to new, specific component key configuration, where it should be possible to add the former JWT_SECRET as a key accepted for verification to enable a seamless transition. This perspective, in turn, calls for refactoring of the existing secret initialization code to centralize the common functions of parsing signing key related configuration directives: The oauth2 module currently is the only component accepting symmetric and asymmetric keys, with the limitation of the symmetric key being also the GeneralTokenSigningSecret. Other components either enforce HS256 or public key algorithms. We should really give the choice of algorithm selection and avoid code duplication in other places, so this commit - generalizes setting parsing into a configuration struct: A prefix can be provided, with which the common configuration directives are processed: - [pfx]SIGNING_ALGORITHM determines the algorithm - [pfx]SECRET is a literal secret for symmetric algorithms - [pfx]SECRET_URI is the uri of a secret for symmetric algorithms - [pfx]SIGNING_PRIVATE_KEY_FILE is a file with a private key for asymmetric algorithms - which is then accepted by jwtx.InitSigningKey() to create an actual signing key The reasons for the two stage process are explained in a long-ish comment in modules/setting/security.go. In short, other options would either violate sensible module boundaries or cause too much friction. These other options have actually been tried, this is take 3 of the proposed changes. ### Refactor services/lfs: Change token code to use SigningKey This now also enables use of token algorithms other than HS256. In this case, signing key initialization also happens during settings initialization, because LFS is also used in CLI commands. ### Refactor api/actions to use new signingkey API This now also enables use of symmetric token algorithms. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11194 Reviewed-by: Gusted <gusted@noreply.codeberg.org> Co-authored-by: Nils Goroll <nils.goroll@uplex.de> Co-committed-by: Nils Goroll <nils.goroll@uplex.de>
2026-04-21 19:39:33 +02:00
path := tempURI.RequestURI()
if !filepath.IsAbs(path) {
path = filepath.Join(AppDataPath, path)
}
buf, err := os.ReadFile(path)
if err != nil {
chore: unify signing key configuration across modules (#11194) ## Context the three commits in this series are the first step towards the goal of removing the special casing around `JWT_SECRET`, which is used for various modules via `GetGeneralTokenSigningSecret()`. Ultimately, I want to work towards enabling seamless migration away from general use of the common secret. To enable this, we need proper secret/key rotation support, that is, we need to allow for configuration of additional secrets/keys which are accepted for token validation, but not used to issue tokens. I have this _Verifier_ support basically implemented, but this PR is not it. This PR contains cleanup refactoring which I worked on before writing the _Verifier_ support, because I noticed that the existing secret/key handling across modules was inconsistent and required duplicated code. I am submitting this part now to allow for incremental review of not too large a diff, and because these commits remained unchanged during two weeks since I moved on the the next task. ## The problem being addressed Configuration of JWT signing secrets/keys was inconsistent: Under `[oauth2]` the full configuration set was supported: - `JWT_SIGNING_ALGORITHM` configured the algorithm - `JWT_SECRET` configured a literal secret for symmetric algorithms - `JWT_SECRET_URI` configured a `file:` uri of a secret for symmetric algorithms - `JWT_SIGNING_PRIVATE_KEY_FILE` configured a file for asymmetric algorithms For `[server]`, the LFS module only supported `LFS_JWT_SECRET`, and the signing method was hardcoded to `HS256` For `[actions]`, only asymmetric signing methods were supported via `ID_TOKEN_SIGNING_ALGORITHM` and `ID_TOKEN_SIGNING_PRIVATE_KEY_FILE`. ## ini unification The proposed code centralizes ini parsing to always support the following ini keys: - `[pfx]SIGNING_ALGORITHM` determines the algorithm - `[pfx]SECRET` is a literal secret for symmetric algorithms - `[pfx]SECRET_URI` is the uri of a secret for symmetric algorithms - `[pfx]SIGNING_PRIVATE_KEY_FILE` is a file with a private key for asymmetric algorithms `[pfx]` is specific to the module and chosen to support the existing ini keys Centralizing this code and unifying the ini keys will come handy for at least the following reasons: - consistent behavior across modules is easier to understand - less duplicated code - easier to expand later, which is my main motivation ## implementation notes as might be apparent by the _take3_ branch name, this is the third iteration of this patch series. The main reason why I abandoned the other two is that I first tried to move all the key initialization into the code called from settings.go when the ini file is parsed. But that lead to a lot of friction with test cases, because private key files which are configured, but do not exist will get created and hence require a writable `AppDataPath` and additional clean up. To avoid a lot of noise and complications in test cases, I kept the existing two stage process, where - the settings component creates missing symmetric signing keys and writes them to the .ini - the settings component creates a simple configuration struct - which is then used from the module init to create the actual key, which also includes creating a private key file if asymmetric crypto is configured and the key file does not exist. I would have wished this patch was a net negative in terms of LOCs, but I hope it contributes to clarity and many added lines are in test cases. ## Commits Because sometimes PRs are merged as squashes with the PR text remaining, I am repeating here the individual messages of the individual commits for future reference: ### Refactor signing key initalization and oauth2 use of it This commit is the first in a series towards the goal of addressing the FIXME comment in modules/setting/oauth2.go to remove GeneralTokenSigningSecret To do it properly, the task also requires addition of signing secret/key rotation: We ultimately want to be able to change a signing key, but continue to accept the previous one. This is particularly relevant to offer a path from GeneralTokenSigningSecret aka JWT_SECRET to new, specific component key configuration, where it should be possible to add the former JWT_SECRET as a key accepted for verification to enable a seamless transition. This perspective, in turn, calls for refactoring of the existing secret initialization code to centralize the common functions of parsing signing key related configuration directives: The oauth2 module currently is the only component accepting symmetric and asymmetric keys, with the limitation of the symmetric key being also the GeneralTokenSigningSecret. Other components either enforce HS256 or public key algorithms. We should really give the choice of algorithm selection and avoid code duplication in other places, so this commit - generalizes setting parsing into a configuration struct: A prefix can be provided, with which the common configuration directives are processed: - [pfx]SIGNING_ALGORITHM determines the algorithm - [pfx]SECRET is a literal secret for symmetric algorithms - [pfx]SECRET_URI is the uri of a secret for symmetric algorithms - [pfx]SIGNING_PRIVATE_KEY_FILE is a file with a private key for asymmetric algorithms - which is then accepted by jwtx.InitSigningKey() to create an actual signing key The reasons for the two stage process are explained in a long-ish comment in modules/setting/security.go. In short, other options would either violate sensible module boundaries or cause too much friction. These other options have actually been tried, this is take 3 of the proposed changes. ### Refactor services/lfs: Change token code to use SigningKey This now also enables use of token algorithms other than HS256. In this case, signing key initialization also happens during settings initialization, because LFS is also used in CLI commands. ### Refactor api/actions to use new signingkey API This now also enables use of symmetric token algorithms. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11194 Reviewed-by: Gusted <gusted@noreply.codeberg.org> Co-authored-by: Nils Goroll <nils.goroll@uplex.de> Co-committed-by: Nils Goroll <nils.goroll@uplex.de>
2026-04-21 19:39:33 +02:00
return "", fmt.Errorf("Failed to read %s: %v", path, err)
}
val := strings.TrimSpace(string(buf))
if val == "" {
// The file shouldn't be empty, otherwise we can not know whether the user has ever set the KEY or KEY_URI
// For example: if INTERNAL_TOKEN_URI=file:///empty-file,
// Then if the token is re-generated during installation and saved to INTERNAL_TOKEN
// Then INTERNAL_TOKEN and INTERNAL_TOKEN_URI both exist, that's a fatal error (they shouldn't)
chore: unify signing key configuration across modules (#11194) ## Context the three commits in this series are the first step towards the goal of removing the special casing around `JWT_SECRET`, which is used for various modules via `GetGeneralTokenSigningSecret()`. Ultimately, I want to work towards enabling seamless migration away from general use of the common secret. To enable this, we need proper secret/key rotation support, that is, we need to allow for configuration of additional secrets/keys which are accepted for token validation, but not used to issue tokens. I have this _Verifier_ support basically implemented, but this PR is not it. This PR contains cleanup refactoring which I worked on before writing the _Verifier_ support, because I noticed that the existing secret/key handling across modules was inconsistent and required duplicated code. I am submitting this part now to allow for incremental review of not too large a diff, and because these commits remained unchanged during two weeks since I moved on the the next task. ## The problem being addressed Configuration of JWT signing secrets/keys was inconsistent: Under `[oauth2]` the full configuration set was supported: - `JWT_SIGNING_ALGORITHM` configured the algorithm - `JWT_SECRET` configured a literal secret for symmetric algorithms - `JWT_SECRET_URI` configured a `file:` uri of a secret for symmetric algorithms - `JWT_SIGNING_PRIVATE_KEY_FILE` configured a file for asymmetric algorithms For `[server]`, the LFS module only supported `LFS_JWT_SECRET`, and the signing method was hardcoded to `HS256` For `[actions]`, only asymmetric signing methods were supported via `ID_TOKEN_SIGNING_ALGORITHM` and `ID_TOKEN_SIGNING_PRIVATE_KEY_FILE`. ## ini unification The proposed code centralizes ini parsing to always support the following ini keys: - `[pfx]SIGNING_ALGORITHM` determines the algorithm - `[pfx]SECRET` is a literal secret for symmetric algorithms - `[pfx]SECRET_URI` is the uri of a secret for symmetric algorithms - `[pfx]SIGNING_PRIVATE_KEY_FILE` is a file with a private key for asymmetric algorithms `[pfx]` is specific to the module and chosen to support the existing ini keys Centralizing this code and unifying the ini keys will come handy for at least the following reasons: - consistent behavior across modules is easier to understand - less duplicated code - easier to expand later, which is my main motivation ## implementation notes as might be apparent by the _take3_ branch name, this is the third iteration of this patch series. The main reason why I abandoned the other two is that I first tried to move all the key initialization into the code called from settings.go when the ini file is parsed. But that lead to a lot of friction with test cases, because private key files which are configured, but do not exist will get created and hence require a writable `AppDataPath` and additional clean up. To avoid a lot of noise and complications in test cases, I kept the existing two stage process, where - the settings component creates missing symmetric signing keys and writes them to the .ini - the settings component creates a simple configuration struct - which is then used from the module init to create the actual key, which also includes creating a private key file if asymmetric crypto is configured and the key file does not exist. I would have wished this patch was a net negative in terms of LOCs, but I hope it contributes to clarity and many added lines are in test cases. ## Commits Because sometimes PRs are merged as squashes with the PR text remaining, I am repeating here the individual messages of the individual commits for future reference: ### Refactor signing key initalization and oauth2 use of it This commit is the first in a series towards the goal of addressing the FIXME comment in modules/setting/oauth2.go to remove GeneralTokenSigningSecret To do it properly, the task also requires addition of signing secret/key rotation: We ultimately want to be able to change a signing key, but continue to accept the previous one. This is particularly relevant to offer a path from GeneralTokenSigningSecret aka JWT_SECRET to new, specific component key configuration, where it should be possible to add the former JWT_SECRET as a key accepted for verification to enable a seamless transition. This perspective, in turn, calls for refactoring of the existing secret initialization code to centralize the common functions of parsing signing key related configuration directives: The oauth2 module currently is the only component accepting symmetric and asymmetric keys, with the limitation of the symmetric key being also the GeneralTokenSigningSecret. Other components either enforce HS256 or public key algorithms. We should really give the choice of algorithm selection and avoid code duplication in other places, so this commit - generalizes setting parsing into a configuration struct: A prefix can be provided, with which the common configuration directives are processed: - [pfx]SIGNING_ALGORITHM determines the algorithm - [pfx]SECRET is a literal secret for symmetric algorithms - [pfx]SECRET_URI is the uri of a secret for symmetric algorithms - [pfx]SIGNING_PRIVATE_KEY_FILE is a file with a private key for asymmetric algorithms - which is then accepted by jwtx.InitSigningKey() to create an actual signing key The reasons for the two stage process are explained in a long-ish comment in modules/setting/security.go. In short, other options would either violate sensible module boundaries or cause too much friction. These other options have actually been tried, this is take 3 of the proposed changes. ### Refactor services/lfs: Change token code to use SigningKey This now also enables use of token algorithms other than HS256. In this case, signing key initialization also happens during settings initialization, because LFS is also used in CLI commands. ### Refactor api/actions to use new signingkey API This now also enables use of symmetric token algorithms. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11194 Reviewed-by: Gusted <gusted@noreply.codeberg.org> Co-authored-by: Nils Goroll <nils.goroll@uplex.de> Co-committed-by: Nils Goroll <nils.goroll@uplex.de>
2026-04-21 19:39:33 +02:00
return "", fmt.Errorf("Failed to read %s: the file is empty", path)
}
chore: unify signing key configuration across modules (#11194) ## Context the three commits in this series are the first step towards the goal of removing the special casing around `JWT_SECRET`, which is used for various modules via `GetGeneralTokenSigningSecret()`. Ultimately, I want to work towards enabling seamless migration away from general use of the common secret. To enable this, we need proper secret/key rotation support, that is, we need to allow for configuration of additional secrets/keys which are accepted for token validation, but not used to issue tokens. I have this _Verifier_ support basically implemented, but this PR is not it. This PR contains cleanup refactoring which I worked on before writing the _Verifier_ support, because I noticed that the existing secret/key handling across modules was inconsistent and required duplicated code. I am submitting this part now to allow for incremental review of not too large a diff, and because these commits remained unchanged during two weeks since I moved on the the next task. ## The problem being addressed Configuration of JWT signing secrets/keys was inconsistent: Under `[oauth2]` the full configuration set was supported: - `JWT_SIGNING_ALGORITHM` configured the algorithm - `JWT_SECRET` configured a literal secret for symmetric algorithms - `JWT_SECRET_URI` configured a `file:` uri of a secret for symmetric algorithms - `JWT_SIGNING_PRIVATE_KEY_FILE` configured a file for asymmetric algorithms For `[server]`, the LFS module only supported `LFS_JWT_SECRET`, and the signing method was hardcoded to `HS256` For `[actions]`, only asymmetric signing methods were supported via `ID_TOKEN_SIGNING_ALGORITHM` and `ID_TOKEN_SIGNING_PRIVATE_KEY_FILE`. ## ini unification The proposed code centralizes ini parsing to always support the following ini keys: - `[pfx]SIGNING_ALGORITHM` determines the algorithm - `[pfx]SECRET` is a literal secret for symmetric algorithms - `[pfx]SECRET_URI` is the uri of a secret for symmetric algorithms - `[pfx]SIGNING_PRIVATE_KEY_FILE` is a file with a private key for asymmetric algorithms `[pfx]` is specific to the module and chosen to support the existing ini keys Centralizing this code and unifying the ini keys will come handy for at least the following reasons: - consistent behavior across modules is easier to understand - less duplicated code - easier to expand later, which is my main motivation ## implementation notes as might be apparent by the _take3_ branch name, this is the third iteration of this patch series. The main reason why I abandoned the other two is that I first tried to move all the key initialization into the code called from settings.go when the ini file is parsed. But that lead to a lot of friction with test cases, because private key files which are configured, but do not exist will get created and hence require a writable `AppDataPath` and additional clean up. To avoid a lot of noise and complications in test cases, I kept the existing two stage process, where - the settings component creates missing symmetric signing keys and writes them to the .ini - the settings component creates a simple configuration struct - which is then used from the module init to create the actual key, which also includes creating a private key file if asymmetric crypto is configured and the key file does not exist. I would have wished this patch was a net negative in terms of LOCs, but I hope it contributes to clarity and many added lines are in test cases. ## Commits Because sometimes PRs are merged as squashes with the PR text remaining, I am repeating here the individual messages of the individual commits for future reference: ### Refactor signing key initalization and oauth2 use of it This commit is the first in a series towards the goal of addressing the FIXME comment in modules/setting/oauth2.go to remove GeneralTokenSigningSecret To do it properly, the task also requires addition of signing secret/key rotation: We ultimately want to be able to change a signing key, but continue to accept the previous one. This is particularly relevant to offer a path from GeneralTokenSigningSecret aka JWT_SECRET to new, specific component key configuration, where it should be possible to add the former JWT_SECRET as a key accepted for verification to enable a seamless transition. This perspective, in turn, calls for refactoring of the existing secret initialization code to centralize the common functions of parsing signing key related configuration directives: The oauth2 module currently is the only component accepting symmetric and asymmetric keys, with the limitation of the symmetric key being also the GeneralTokenSigningSecret. Other components either enforce HS256 or public key algorithms. We should really give the choice of algorithm selection and avoid code duplication in other places, so this commit - generalizes setting parsing into a configuration struct: A prefix can be provided, with which the common configuration directives are processed: - [pfx]SIGNING_ALGORITHM determines the algorithm - [pfx]SECRET is a literal secret for symmetric algorithms - [pfx]SECRET_URI is the uri of a secret for symmetric algorithms - [pfx]SIGNING_PRIVATE_KEY_FILE is a file with a private key for asymmetric algorithms - which is then accepted by jwtx.InitSigningKey() to create an actual signing key The reasons for the two stage process are explained in a long-ish comment in modules/setting/security.go. In short, other options would either violate sensible module boundaries or cause too much friction. These other options have actually been tried, this is take 3 of the proposed changes. ### Refactor services/lfs: Change token code to use SigningKey This now also enables use of token algorithms other than HS256. In this case, signing key initialization also happens during settings initialization, because LFS is also used in CLI commands. ### Refactor api/actions to use new signingkey API This now also enables use of symmetric token algorithms. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11194 Reviewed-by: Gusted <gusted@noreply.codeberg.org> Co-authored-by: Nils Goroll <nils.goroll@uplex.de> Co-committed-by: Nils Goroll <nils.goroll@uplex.de>
2026-04-21 19:39:33 +02:00
return val, nil
// only file URIs are allowed
default:
chore: unify signing key configuration across modules (#11194) ## Context the three commits in this series are the first step towards the goal of removing the special casing around `JWT_SECRET`, which is used for various modules via `GetGeneralTokenSigningSecret()`. Ultimately, I want to work towards enabling seamless migration away from general use of the common secret. To enable this, we need proper secret/key rotation support, that is, we need to allow for configuration of additional secrets/keys which are accepted for token validation, but not used to issue tokens. I have this _Verifier_ support basically implemented, but this PR is not it. This PR contains cleanup refactoring which I worked on before writing the _Verifier_ support, because I noticed that the existing secret/key handling across modules was inconsistent and required duplicated code. I am submitting this part now to allow for incremental review of not too large a diff, and because these commits remained unchanged during two weeks since I moved on the the next task. ## The problem being addressed Configuration of JWT signing secrets/keys was inconsistent: Under `[oauth2]` the full configuration set was supported: - `JWT_SIGNING_ALGORITHM` configured the algorithm - `JWT_SECRET` configured a literal secret for symmetric algorithms - `JWT_SECRET_URI` configured a `file:` uri of a secret for symmetric algorithms - `JWT_SIGNING_PRIVATE_KEY_FILE` configured a file for asymmetric algorithms For `[server]`, the LFS module only supported `LFS_JWT_SECRET`, and the signing method was hardcoded to `HS256` For `[actions]`, only asymmetric signing methods were supported via `ID_TOKEN_SIGNING_ALGORITHM` and `ID_TOKEN_SIGNING_PRIVATE_KEY_FILE`. ## ini unification The proposed code centralizes ini parsing to always support the following ini keys: - `[pfx]SIGNING_ALGORITHM` determines the algorithm - `[pfx]SECRET` is a literal secret for symmetric algorithms - `[pfx]SECRET_URI` is the uri of a secret for symmetric algorithms - `[pfx]SIGNING_PRIVATE_KEY_FILE` is a file with a private key for asymmetric algorithms `[pfx]` is specific to the module and chosen to support the existing ini keys Centralizing this code and unifying the ini keys will come handy for at least the following reasons: - consistent behavior across modules is easier to understand - less duplicated code - easier to expand later, which is my main motivation ## implementation notes as might be apparent by the _take3_ branch name, this is the third iteration of this patch series. The main reason why I abandoned the other two is that I first tried to move all the key initialization into the code called from settings.go when the ini file is parsed. But that lead to a lot of friction with test cases, because private key files which are configured, but do not exist will get created and hence require a writable `AppDataPath` and additional clean up. To avoid a lot of noise and complications in test cases, I kept the existing two stage process, where - the settings component creates missing symmetric signing keys and writes them to the .ini - the settings component creates a simple configuration struct - which is then used from the module init to create the actual key, which also includes creating a private key file if asymmetric crypto is configured and the key file does not exist. I would have wished this patch was a net negative in terms of LOCs, but I hope it contributes to clarity and many added lines are in test cases. ## Commits Because sometimes PRs are merged as squashes with the PR text remaining, I am repeating here the individual messages of the individual commits for future reference: ### Refactor signing key initalization and oauth2 use of it This commit is the first in a series towards the goal of addressing the FIXME comment in modules/setting/oauth2.go to remove GeneralTokenSigningSecret To do it properly, the task also requires addition of signing secret/key rotation: We ultimately want to be able to change a signing key, but continue to accept the previous one. This is particularly relevant to offer a path from GeneralTokenSigningSecret aka JWT_SECRET to new, specific component key configuration, where it should be possible to add the former JWT_SECRET as a key accepted for verification to enable a seamless transition. This perspective, in turn, calls for refactoring of the existing secret initialization code to centralize the common functions of parsing signing key related configuration directives: The oauth2 module currently is the only component accepting symmetric and asymmetric keys, with the limitation of the symmetric key being also the GeneralTokenSigningSecret. Other components either enforce HS256 or public key algorithms. We should really give the choice of algorithm selection and avoid code duplication in other places, so this commit - generalizes setting parsing into a configuration struct: A prefix can be provided, with which the common configuration directives are processed: - [pfx]SIGNING_ALGORITHM determines the algorithm - [pfx]SECRET is a literal secret for symmetric algorithms - [pfx]SECRET_URI is the uri of a secret for symmetric algorithms - [pfx]SIGNING_PRIVATE_KEY_FILE is a file with a private key for asymmetric algorithms - which is then accepted by jwtx.InitSigningKey() to create an actual signing key The reasons for the two stage process are explained in a long-ish comment in modules/setting/security.go. In short, other options would either violate sensible module boundaries or cause too much friction. These other options have actually been tried, this is take 3 of the proposed changes. ### Refactor services/lfs: Change token code to use SigningKey This now also enables use of token algorithms other than HS256. In this case, signing key initialization also happens during settings initialization, because LFS is also used in CLI commands. ### Refactor api/actions to use new signingkey API This now also enables use of symmetric token algorithms. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11194 Reviewed-by: Gusted <gusted@noreply.codeberg.org> Co-authored-by: Nils Goroll <nils.goroll@uplex.de> Co-committed-by: Nils Goroll <nils.goroll@uplex.de>
2026-04-21 19:39:33 +02:00
return "", fmt.Errorf("Unsupported URI-Scheme %q in %q", tempURI.Scheme, uri)
}
}
// createSymmeticSigningKey creates a new symmetric signing key and saves it to
// the setting named cfgSecret (usually [PFX_]SECRET) in section cfgSection
func createSymmeticSigningKeyCfg(rootCfg ConfigProvider, cfgSection, cfgSecret string) (*[]byte, error) {
jwtSecretBytes, jwtSecretBase64 := generate.NewJwtSecret()
saveCfg, err := rootCfg.PrepareSaving()
if err == nil {
rootCfg.Section(cfgSection).Key(cfgSecret).SetValue(jwtSecretBase64)
saveCfg.Section(cfgSection).Key(cfgSecret).SetValue(jwtSecretBase64)
err = saveCfg.Save()
}
if err != nil {
return nil, fmt.Errorf("save %s.%s failed: %v", cfgSection, cfgSecret, err)
}
return &jwtSecretBytes, nil
}
// loadSymmeticSigningKey loads a signing key and creates it unless present
// loads from [pfx]SECRET_URI
// loads from or saves to [pfx]SECRET
// in section sec
func loadSymmeticSigningKeyCfg(rootCfg ConfigProvider, sec ConfigSection, pfx string) (*[]byte, error) {
cfgSecretURI := pfx + "SECRET_URI"
cfgSecret := pfx + "SECRET"
secretBase64 := loadSecret(sec, cfgSecretURI, cfgSecret)
secret, err := generate.DecodeJwtSecret(secretBase64)
if err == nil {
return &secret, nil
}
log.Info("[%s] %s or %s failed loading: %v - creating new key", sec.Name(), cfgSecret, cfgSecretURI, err)
return createSymmeticSigningKeyCfg(rootCfg, sec.Name(), cfgSecret)
}
// loadAsymmeticSigningKey loads a signing key from [pfx]SIGNING_PRIVATE_KEY_FILE
// or creates it if it does not exist
func loadAsymmeticSigningKeyPath(sec ConfigSection, pfx, defaultFile string) *string {
cfgFile := pfx + "SIGNING_PRIVATE_KEY_FILE"
keyPath := sec.Key(cfgFile).MustString(defaultFile)
if !filepath.IsAbs(keyPath) {
keyPath = filepath.Join(AppDataPath, keyPath)
}
return &keyPath
}
type checkKeyCfg func(rootCfg ConfigProvider, cfgSection, pfx string) error
func onlyAsymmetric() checkKeyCfg {
return func(rootCfg ConfigProvider, cfgSection, pfx string) error {
sec := rootCfg.Section(cfgSection)
cfgAlg := pfx + "SIGNING_ALGORITHM"
if sec.HasKey(cfgAlg) {
alg := sec.Key(cfgAlg).String()
if !jwtx.IsValidAsymmetricAlgorithm(alg) {
return fmt.Errorf("Unexpected algorithm: %s = %s, needs to be one of %v",
cfgAlg, alg, jwtx.ValidAsymmetricAlgorithms)
}
}
noCfg := []string{
pfx + "SECRET_URI",
pfx + "SECRET",
}
for _, cfg := range noCfg {
if sec.HasKey(cfg) {
return fmt.Errorf("Invalid config key: %s - must be removed", cfg)
}
}
return nil
}
}
// loadSigningKey() loads a or creates signing key based on settings in section cfgSection
// [pfx]SIGNING_ALGORITHM determines the algorithm
// [pfx]SECRET is a literal secret for symmetric algorithms
// [pfx]SECRET_URI is the uri of a secret for symmetric algorithms
// [pfx]SIGNING_PRIVATE_KEY_FILE is a file with a private key for asymmetric algorithms
//
// [pfx]SECRET might get written to literally in the config if needed but not present
func loadSigningKeyCfg(rootCfg ConfigProvider, cfgSection, pfx, defaultAlg, defaultPrivateKeyFile string, checks ...checkKeyCfg) (*jwtx.SigningKeyCfg, error) {
for _, check := range checks {
err := check(rootCfg, cfgSection, pfx)
if err != nil {
return nil, err
}
}
sec := rootCfg.Section(cfgSection)
cfgAlg := pfx + "SIGNING_ALGORITHM"
algorithm := sec.Key(cfgAlg).MustString(defaultAlg)
cfg := jwtx.SigningKeyCfg{Algorithm: algorithm}
var err error
if jwtx.IsValidSymmetricAlgorithm(algorithm) {
cfg.SecretBytes, err = loadSymmeticSigningKeyCfg(rootCfg, sec, pfx)
} else if jwtx.IsValidAsymmetricAlgorithm(algorithm) {
cfg.PrivateKeyPath = loadAsymmeticSigningKeyPath(sec, pfx, defaultPrivateKeyFile)
} else {
err = fmt.Errorf("invalid algorithm: %s = %s", cfgAlg, algorithm)
}
return &cfg, err
}
func loadKeyCfg(rootCfg ConfigProvider, cfgSection, pfx, defaultAlg, defaultPrivateKeyFile string, checks ...checkKeyCfg) (*jwtx.KeyCfg, error) {
signing, err := loadSigningKeyCfg(rootCfg, cfgSection, pfx, defaultAlg, defaultPrivateKeyFile, checks...)
if err != nil {
err = fmt.Errorf("[%s] %v", cfgSection, err)
return nil, err
}
chore: unify signing key configuration across modules (#11194) ## Context the three commits in this series are the first step towards the goal of removing the special casing around `JWT_SECRET`, which is used for various modules via `GetGeneralTokenSigningSecret()`. Ultimately, I want to work towards enabling seamless migration away from general use of the common secret. To enable this, we need proper secret/key rotation support, that is, we need to allow for configuration of additional secrets/keys which are accepted for token validation, but not used to issue tokens. I have this _Verifier_ support basically implemented, but this PR is not it. This PR contains cleanup refactoring which I worked on before writing the _Verifier_ support, because I noticed that the existing secret/key handling across modules was inconsistent and required duplicated code. I am submitting this part now to allow for incremental review of not too large a diff, and because these commits remained unchanged during two weeks since I moved on the the next task. ## The problem being addressed Configuration of JWT signing secrets/keys was inconsistent: Under `[oauth2]` the full configuration set was supported: - `JWT_SIGNING_ALGORITHM` configured the algorithm - `JWT_SECRET` configured a literal secret for symmetric algorithms - `JWT_SECRET_URI` configured a `file:` uri of a secret for symmetric algorithms - `JWT_SIGNING_PRIVATE_KEY_FILE` configured a file for asymmetric algorithms For `[server]`, the LFS module only supported `LFS_JWT_SECRET`, and the signing method was hardcoded to `HS256` For `[actions]`, only asymmetric signing methods were supported via `ID_TOKEN_SIGNING_ALGORITHM` and `ID_TOKEN_SIGNING_PRIVATE_KEY_FILE`. ## ini unification The proposed code centralizes ini parsing to always support the following ini keys: - `[pfx]SIGNING_ALGORITHM` determines the algorithm - `[pfx]SECRET` is a literal secret for symmetric algorithms - `[pfx]SECRET_URI` is the uri of a secret for symmetric algorithms - `[pfx]SIGNING_PRIVATE_KEY_FILE` is a file with a private key for asymmetric algorithms `[pfx]` is specific to the module and chosen to support the existing ini keys Centralizing this code and unifying the ini keys will come handy for at least the following reasons: - consistent behavior across modules is easier to understand - less duplicated code - easier to expand later, which is my main motivation ## implementation notes as might be apparent by the _take3_ branch name, this is the third iteration of this patch series. The main reason why I abandoned the other two is that I first tried to move all the key initialization into the code called from settings.go when the ini file is parsed. But that lead to a lot of friction with test cases, because private key files which are configured, but do not exist will get created and hence require a writable `AppDataPath` and additional clean up. To avoid a lot of noise and complications in test cases, I kept the existing two stage process, where - the settings component creates missing symmetric signing keys and writes them to the .ini - the settings component creates a simple configuration struct - which is then used from the module init to create the actual key, which also includes creating a private key file if asymmetric crypto is configured and the key file does not exist. I would have wished this patch was a net negative in terms of LOCs, but I hope it contributes to clarity and many added lines are in test cases. ## Commits Because sometimes PRs are merged as squashes with the PR text remaining, I am repeating here the individual messages of the individual commits for future reference: ### Refactor signing key initalization and oauth2 use of it This commit is the first in a series towards the goal of addressing the FIXME comment in modules/setting/oauth2.go to remove GeneralTokenSigningSecret To do it properly, the task also requires addition of signing secret/key rotation: We ultimately want to be able to change a signing key, but continue to accept the previous one. This is particularly relevant to offer a path from GeneralTokenSigningSecret aka JWT_SECRET to new, specific component key configuration, where it should be possible to add the former JWT_SECRET as a key accepted for verification to enable a seamless transition. This perspective, in turn, calls for refactoring of the existing secret initialization code to centralize the common functions of parsing signing key related configuration directives: The oauth2 module currently is the only component accepting symmetric and asymmetric keys, with the limitation of the symmetric key being also the GeneralTokenSigningSecret. Other components either enforce HS256 or public key algorithms. We should really give the choice of algorithm selection and avoid code duplication in other places, so this commit - generalizes setting parsing into a configuration struct: A prefix can be provided, with which the common configuration directives are processed: - [pfx]SIGNING_ALGORITHM determines the algorithm - [pfx]SECRET is a literal secret for symmetric algorithms - [pfx]SECRET_URI is the uri of a secret for symmetric algorithms - [pfx]SIGNING_PRIVATE_KEY_FILE is a file with a private key for asymmetric algorithms - which is then accepted by jwtx.InitSigningKey() to create an actual signing key The reasons for the two stage process are explained in a long-ish comment in modules/setting/security.go. In short, other options would either violate sensible module boundaries or cause too much friction. These other options have actually been tried, this is take 3 of the proposed changes. ### Refactor services/lfs: Change token code to use SigningKey This now also enables use of token algorithms other than HS256. In this case, signing key initialization also happens during settings initialization, because LFS is also used in CLI commands. ### Refactor api/actions to use new signingkey API This now also enables use of symmetric token algorithms. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11194 Reviewed-by: Gusted <gusted@noreply.codeberg.org> Co-authored-by: Nils Goroll <nils.goroll@uplex.de> Co-committed-by: Nils Goroll <nils.goroll@uplex.de>
2026-04-21 19:39:33 +02:00
return &jwtx.KeyCfg{Signing: signing}, nil
}
// generateSaveInternalToken generates and saves the internal token to app.ini
func generateSaveInternalToken(rootCfg ConfigProvider) {
token, err := generate.NewInternalToken()
if err != nil {
log.Fatal("Error generate internal token: %v", err)
}
InternalToken = token
saveCfg, err := rootCfg.PrepareSaving()
if err != nil {
log.Fatal("Error saving internal token: %v", err)
}
rootCfg.Section("security").Key("INTERNAL_TOKEN").SetValue(token)
saveCfg.Section("security").Key("INTERNAL_TOKEN").SetValue(token)
if err = saveCfg.Save(); err != nil {
log.Fatal("Error saving internal token: %v", err)
}
}
func loadSecurityFrom(rootCfg ConfigProvider) {
sec := rootCfg.Section("security")
InstallLock = HasInstallLock(rootCfg)
LogInRememberDays = sec.Key("LOGIN_REMEMBER_DAYS").MustInt(31)
SecretKey = loadSecret(sec, "SECRET_KEY_URI", "SECRET_KEY")
if SecretKey == "" {
// FIXME: https://github.com/go-gitea/gitea/issues/16832
// Until it supports rotating an existing secret key, we shouldn't move users off of the widely used default value
SecretKey = "!#@FDEWREWR&*(" //nolint:gosec
}
keying.Init([]byte(SecretKey))
feat: Global 2FA enforcement (#8753) resolves #8549 This PR add a config to enforce 2FA for the whole Forgejo instance. It can be configured to `none`, `admin` or `all`. A user who is required to enable 2FA is like a disabled user. He can only see the `/user/settings/security`-Page to enable 2FA, this should be similar to a user which needs to change his password. Also api and git-commands are not allowed. ## 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. - [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. - [ ] I did not document these changes and I do not expect someone else to do it. I will do it, if the general idea of this PR is a good feature. ### 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--> - Security features - [PR](https://codeberg.org/forgejo/forgejo/pulls/8753): <!--number 8753 --><!--line 0 --><!--description R2xvYmFsIDJGQSBlbmZvcmNlbWVudA==-->Global 2FA enforcement<!--description--> <!--end release-notes-assistant--> Co-authored-by: 0ko <0ko@noreply.codeberg.org> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8753 Reviewed-by: 0ko <0ko@noreply.codeberg.org> Reviewed-by: Ellen Εμιλία Άννα Zscheile <fogti@noreply.codeberg.org> Reviewed-by: Gusted <gusted@noreply.codeberg.org> Co-authored-by: zokki <zokki.softwareschmiede@gmail.com> Co-committed-by: zokki <zokki.softwareschmiede@gmail.com>
2025-08-15 10:56:45 +02:00
GlobalTwoFactorRequirement = NewTwoFactorRequirementType(sec.Key("GLOBAL_TWO_FACTOR_REQUIREMENT").String())
CookieRememberName = sec.Key("COOKIE_REMEMBER_NAME").MustString("persistent")
ReverseProxyAuthUser = sec.Key("REVERSE_PROXY_AUTHENTICATION_USER").MustString("X-WEBAUTH-USER")
ReverseProxyAuthEmail = sec.Key("REVERSE_PROXY_AUTHENTICATION_EMAIL").MustString("X-WEBAUTH-EMAIL")
ReverseProxyAuthFullName = sec.Key("REVERSE_PROXY_AUTHENTICATION_FULL_NAME").MustString("X-WEBAUTH-FULLNAME")
ReverseProxyLimit = sec.Key("REVERSE_PROXY_LIMIT").MustInt(1)
ReverseProxyTrustedProxies = sec.Key("REVERSE_PROXY_TRUSTED_PROXIES").Strings(",")
if len(ReverseProxyTrustedProxies) == 0 {
ReverseProxyTrustedProxies = []string{"127.0.0.0/8", "::1/128"}
}
MinPasswordLength = sec.Key("MIN_PASSWORD_LENGTH").MustInt(8)
ImportLocalPaths = sec.Key("IMPORT_LOCAL_PATHS").MustBool(false)
DisableGitHooks = sec.Key("DISABLE_GIT_HOOKS").MustBool(true)
DisableWebhooks = sec.Key("DISABLE_WEBHOOKS").MustBool(false)
OnlyAllowPushIfGiteaEnvironmentSet = sec.Key("ONLY_ALLOW_PUSH_IF_GITEA_ENVIRONMENT_SET").MustBool(true)
// Ensure that the provided default hash algorithm is a valid hash algorithm
var algorithm *hash.PasswordHashAlgorithm
PasswordHashAlgo, algorithm = hash.SetDefaultPasswordHashAlgorithm(sec.Key("PASSWORD_HASH_ALGO").MustString(""))
if algorithm == nil {
log.Fatal("The provided password hash algorithm was invalid: %s", sec.Key("PASSWORD_HASH_ALGO").MustString(""))
}
PasswordCheckPwn = sec.Key("PASSWORD_CHECK_PWN").MustBool(false)
SuccessfulTokensCacheSize = sec.Key("SUCCESSFUL_TOKENS_CACHE_SIZE").MustInt(20)
InternalToken = loadSecret(sec, "INTERNAL_TOKEN_URI", "INTERNAL_TOKEN")
if InstallLock && InternalToken == "" {
// if Gitea has been installed but the InternalToken hasn't been generated (upgrade from an old release), we should generate
// some users do cluster deployment, they still depend on this auto-generating behavior.
generateSaveInternalToken(rootCfg)
}
cfgdata := sec.Key("PASSWORD_COMPLEXITY").Strings(",")
if len(cfgdata) == 0 {
cfgdata = []string{"off"}
}
PasswordComplexity = make([]string, 0, len(cfgdata))
for _, name := range cfgdata {
name := strings.ToLower(strings.Trim(name, `"`))
if name != "" {
PasswordComplexity = append(PasswordComplexity, name)
}
}
sectionHasDisableQueryAuthToken := sec.HasKey("DISABLE_QUERY_AUTH_TOKEN")
// TODO: default value should be true in future releases
DisableQueryAuthToken = sec.Key("DISABLE_QUERY_AUTH_TOKEN").MustBool(false)
// warn if the setting is set to false explicitly
if sectionHasDisableQueryAuthToken && !DisableQueryAuthToken {
fix: query token auth version mismatch (#8666) It's now scheduled for Forgejo v13 see #8633 for more context I used Github Copilot for some auto completion of code. ## 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. - [ ] 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. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8666 Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org> Co-authored-by: Michael Kriese <michael.kriese@visualon.de> Co-committed-by: Michael Kriese <michael.kriese@visualon.de>
2025-07-25 12:24:26 +02:00
log.Warn("Enabling Query API Auth tokens is not recommended. DISABLE_QUERY_AUTH_TOKEN will be removed in Forgejo v13.0.0.")
}
}
feat: Global 2FA enforcement (#8753) resolves #8549 This PR add a config to enforce 2FA for the whole Forgejo instance. It can be configured to `none`, `admin` or `all`. A user who is required to enable 2FA is like a disabled user. He can only see the `/user/settings/security`-Page to enable 2FA, this should be similar to a user which needs to change his password. Also api and git-commands are not allowed. ## 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. - [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. - [ ] I did not document these changes and I do not expect someone else to do it. I will do it, if the general idea of this PR is a good feature. ### 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--> - Security features - [PR](https://codeberg.org/forgejo/forgejo/pulls/8753): <!--number 8753 --><!--line 0 --><!--description R2xvYmFsIDJGQSBlbmZvcmNlbWVudA==-->Global 2FA enforcement<!--description--> <!--end release-notes-assistant--> Co-authored-by: 0ko <0ko@noreply.codeberg.org> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8753 Reviewed-by: 0ko <0ko@noreply.codeberg.org> Reviewed-by: Ellen Εμιλία Άννα Zscheile <fogti@noreply.codeberg.org> Reviewed-by: Gusted <gusted@noreply.codeberg.org> Co-authored-by: zokki <zokki.softwareschmiede@gmail.com> Co-committed-by: zokki <zokki.softwareschmiede@gmail.com>
2025-08-15 10:56:45 +02:00
type TwoFactorRequirementType string
// llu:TrKeysSuffix admin.config.global_2fa_requirement.
const (
NoneTwoFactorRequirement TwoFactorRequirementType = "none"
AllTwoFactorRequirement TwoFactorRequirementType = "all"
AdminTwoFactorRequirement TwoFactorRequirementType = "admin"
)
func NewTwoFactorRequirementType(twoFactorRequirement string) TwoFactorRequirementType {
switch twoFactorRequirement {
case AllTwoFactorRequirement.String():
return AllTwoFactorRequirement
case AdminTwoFactorRequirement.String():
return AdminTwoFactorRequirement
default:
return NoneTwoFactorRequirement
}
}
func (r TwoFactorRequirementType) String() string {
return string(r)
}
func (r TwoFactorRequirementType) IsNone() bool {
return r == NoneTwoFactorRequirement
}
func (r TwoFactorRequirementType) IsAll() bool {
return r == AllTwoFactorRequirement
}
func (r TwoFactorRequirementType) IsAdmin() bool {
return r == AdminTwoFactorRequirement
}