From 5b7e54f72f7b85b3394d7af20b27152d26e26256 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Tue, 14 May 2024 23:00:38 +0800 Subject: [PATCH] Always load or generate oauth2 jwt secret (#30942) (#30978) Backport #30942 by @wxiaoguang Fix #30923 Co-authored-by: wxiaoguang --- modules/setting/oauth2.go | 17 ++++++----------- modules/setting/oauth2_test.go | 28 +++++++++++++++++++++++++++- routers/install/install.go | 11 +++++++++++ 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/modules/setting/oauth2.go b/modules/setting/oauth2.go index e59f54420b..0d3e63e0b4 100644 --- a/modules/setting/oauth2.go +++ b/modules/setting/oauth2.go @@ -126,16 +126,15 @@ func loadOAuth2From(rootCfg ConfigProvider) { OAuth2.Enabled = sec.Key("ENABLE").MustBool(OAuth2.Enabled) } - if !OAuth2.Enabled { - return - } - - jwtSecretBase64 := loadSecret(sec, "JWT_SECRET_URI", "JWT_SECRET") - if !filepath.IsAbs(OAuth2.JWTSigningPrivateKeyFile) { OAuth2.JWTSigningPrivateKeyFile = filepath.Join(AppDataPath, OAuth2.JWTSigningPrivateKeyFile) } + // FIXME: at the moment, no matter oauth2 is enabled or not, it must generate a "oauth2 JWT_SECRET" + // Because this secret is also used as GeneralTokenSigningSecret (as a quick not-that-breaking fix for some legacy problems). + // Including: CSRF token, account validation token, etc ... + // In main branch, the signing token should be refactored (eg: one unique for LFS/OAuth2/etc ...) + jwtSecretBase64 := loadSecret(sec, "JWT_SECRET_URI", "JWT_SECRET") if InstallLock { jwtSecretBytes, err := generate.DecodeJwtSecretBase64(jwtSecretBase64) if err != nil { @@ -157,8 +156,6 @@ func loadOAuth2From(rootCfg ConfigProvider) { } } -// generalSigningSecret is used as container for a []byte value -// instead of an additional mutex, we use CompareAndSwap func to change the value thread save var generalSigningSecret atomic.Pointer[[]byte] func GetGeneralTokenSigningSecret() []byte { @@ -166,11 +163,9 @@ func GetGeneralTokenSigningSecret() []byte { if old == nil || len(*old) == 0 { jwtSecret, _, err := generate.NewJwtSecretWithBase64() if err != nil { - log.Fatal("Unable to generate general JWT secret: %s", err.Error()) + log.Fatal("Unable to generate general JWT secret: %v", err) } if generalSigningSecret.CompareAndSwap(old, &jwtSecret) { - // FIXME: in main branch, the signing token should be refactored (eg: one unique for LFS/OAuth2/etc ...) - LogStartupProblem(1, log.WARN, "OAuth2 is not enabled, unable to use a persistent signing secret, a new one is generated, which is not persistent between restarts and cluster nodes") return jwtSecret } return *generalSigningSecret.Load() diff --git a/modules/setting/oauth2_test.go b/modules/setting/oauth2_test.go index 4403f35892..38ee4d248d 100644 --- a/modules/setting/oauth2_test.go +++ b/modules/setting/oauth2_test.go @@ -4,6 +4,7 @@ package setting import ( + "os" "testing" "code.gitea.io/gitea/modules/generate" @@ -14,7 +15,7 @@ import ( func TestGetGeneralSigningSecret(t *testing.T) { // when there is no general signing secret, it should be generated, and keep the same value - assert.Nil(t, generalSigningSecret.Load()) + generalSigningSecret.Store(nil) s1 := GetGeneralTokenSigningSecret() assert.NotNil(t, s1) s2 := GetGeneralTokenSigningSecret() @@ -33,6 +34,31 @@ JWT_SECRET = BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB assert.EqualValues(t, expected, actual) } +func TestGetGeneralSigningSecretSave(t *testing.T) { + defer test.MockVariableValue(&InstallLock, true)() + + old := GetGeneralTokenSigningSecret() + assert.Len(t, old, 32) + + tmpFile := t.TempDir() + "/app.ini" + _ = os.WriteFile(tmpFile, nil, 0o644) + cfg, _ := NewConfigProviderFromFile(tmpFile) + loadOAuth2From(cfg) + generated := GetGeneralTokenSigningSecret() + assert.Len(t, generated, 32) + assert.NotEqual(t, old, generated) + + generalSigningSecret.Store(nil) + cfg, _ = NewConfigProviderFromFile(tmpFile) + loadOAuth2From(cfg) + again := GetGeneralTokenSigningSecret() + assert.Equal(t, generated, again) + + iniContent, err := os.ReadFile(tmpFile) + assert.NoError(t, err) + assert.Contains(t, string(iniContent), "JWT_SECRET = ") +} + func TestOauth2DefaultApplications(t *testing.T) { cfg, _ := NewConfigProviderFromData(``) loadOAuth2From(cfg) diff --git a/routers/install/install.go b/routers/install/install.go index 9c6a8849b6..fde8b37ed5 100644 --- a/routers/install/install.go +++ b/routers/install/install.go @@ -481,6 +481,17 @@ func SubmitInstall(ctx *context.Context) { cfg.Section("security").Key("INTERNAL_TOKEN").SetValue(internalToken) } + // FIXME: at the moment, no matter oauth2 is enabled or not, it must generate a "oauth2 JWT_SECRET" + // see the "loadOAuth2From" in "setting/oauth2.go" + if !cfg.Section("oauth2").HasKey("JWT_SECRET") && !cfg.Section("oauth2").HasKey("JWT_SECRET_URI") { + _, jwtSecretBase64, err := generate.NewJwtSecretWithBase64() + if err != nil { + ctx.RenderWithErr(ctx.Tr("install.secret_key_failed", err), tplInstall, &form) + return + } + cfg.Section("oauth2").Key("JWT_SECRET").SetValue(jwtSecretBase64) + } + // if there is already a SECRET_KEY, we should not overwrite it, otherwise the encrypted data will not be able to be decrypted if setting.SecretKey == "" { var secretKey string