Always load or generate oauth2 jwt secret (#30942) (#30978)

Backport #30942 by @wxiaoguang

 Fix #30923

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

diff --git a/modules/setting/oauth2.go b/modules/setting/oauth2.go
index e59f54420b293799ecaa93e4702ec80623d3445b..0d3e63e0b4aa3096b9358bbb4f8d78791516ff41 100644 (file)
--- 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 4403f358926f58a980fafc58c30eefe5d935d635..38ee4d248d6ffe1ed6934451cbf57678fa75bbad 100644 (file)
--- 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 9c6a8849b63ffc9ead82145e275230016f794603..fde8b37ed59428ba956fcf2ac6434de7eb82324e 100644 (file)
--- 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