From 37228ea0802ab2ae1340b25bff3989ef475f81c2 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 14 May 2024 22:21:38 +0800 Subject: [PATCH 1/4] Always load or generate oauth2 jwt secret (#30942) Fix #30923 (cherry picked from commit effb405cae88474c27f5c8322a2627019af1cf64) Signed-off-by: Gergely Nagy Conflicts: - modules/setting/oauth2.go Conflicted due to different ways of logging. Since the log message is removed anyway, resolved by removing it. - modules/setting/oauth2_test.go Manually copied the test added by Gitea. - routers/install/install.go Not a conflict per se, but adjusted to use NewJwtSecret(). (cherry picked from commit 193ac67176afc72e9d108bc1730c354bfbf9a442) Equivalent to the Gitea v1.22 commit (cherry picked from commit 5b7e54f72f7b85b3394d7af20b27152d26e26256) --- 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 d3c4d5c387..76820adff0 100644 --- a/modules/setting/oauth2.go +++ b/modules/setting/oauth2.go @@ -124,16 +124,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.DecodeJwtSecret(jwtSecretBase64) if err != nil { @@ -155,8 +154,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 { @@ -164,11 +161,9 @@ func GetGeneralTokenSigningSecret() []byte { if old == nil || len(*old) == 0 { jwtSecret, _, err := generate.NewJwtSecret() 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 ...) - 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 da36d100aa..1951c4c0a2 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() @@ -32,3 +33,28 @@ JWT_SECRET = BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB assert.Len(t, actual, 32) 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 = ") +} diff --git a/routers/install/install.go b/routers/install/install.go index 282ebe9ead..b84d77cfc2 100644 --- a/routers/install/install.go +++ b/routers/install/install.go @@ -486,6 +486,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.NewJwtSecret() + 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 From 6e172625679bb3a518463881ce6fb035e023afb6 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 15 May 2024 07:06:12 +0800 Subject: [PATCH 2/4] Remove unnecessary double quotes on language file (#30977) The double quotes and the prefix/suffix space are unnecessary. Co-authored-by: KN4CK3R (cherry picked from commit 5b6f80989fbd0574ca188ab683389ff7659de30d) (cherry picked from commit a20e924ee729ec6c53c0e768389f90e95d1861c1) Equivalent to the Gitea v1.22 commit (cherry picked from commit cb52eb639e5d9b36794e46ec2e09d633c23c7322) --- options/locale/locale_en-US.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 4eb86a17ea..c5270322b0 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -3413,7 +3413,7 @@ mirror_sync_create = synced new reference %[3]s to %[3]s from mirror approve_pull_request = `approved %[3]s#%[2]s` reject_pull_request = `suggested changes for %[3]s#%[2]s` -publish_release = `released "%[4]s" at %[3]s` +publish_release = `released %[4]s at %[3]s` review_dismissed = `dismissed review from %[4]s for %[3]s#%[2]s` review_dismissed_reason = Reason: create_branch = created branch %[3]s in %[4]s From 50ef4ecefe3eb1f7045d9d392654006a91b0906f Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 16 May 2024 21:40:57 +0200 Subject: [PATCH 3/4] Upgrade `tqdm` dependency (#30996) Result of `make update-py` Fixes: https://github.com/go-gitea/gitea/security/dependabot/65 (cherry picked from commit a73e3c6a696029541ebd423f4eb2fec1ba151f79) (cherry picked from commit 87def3837bc4715bfd20fee4c80af8a6689dba48) Equivalent to the Gitea v1.22 commit (cherry picked from commit f0e74da71929196dfdcab48616303d8b1c2c30da) --- poetry.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/poetry.lock b/poetry.lock index 951a0fa7a8..5ca5afc0a3 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.8.2 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand. [[package]] name = "click" @@ -318,13 +318,13 @@ files = [ [[package]] name = "tqdm" -version = "4.66.2" +version = "4.66.4" description = "Fast, Extensible Progress Meter" optional = false python-versions = ">=3.7" files = [ - {file = "tqdm-4.66.2-py3-none-any.whl", hash = "sha256:1ee4f8a893eb9bef51c6e35730cebf234d5d0b6bd112b0271e10ed7c24a02bd9"}, - {file = "tqdm-4.66.2.tar.gz", hash = "sha256:6cd52cdf0fef0e0f543299cfc96fec90d7b8a7e88745f411ec33eb44d5ed3531"}, + {file = "tqdm-4.66.4-py3-none-any.whl", hash = "sha256:b75ca56b413b030bc3f00af51fd2c1a1a5eac6a0c1cca83cbb37a5c52abce644"}, + {file = "tqdm-4.66.4.tar.gz", hash = "sha256:e4d936c9de8727928f3be6079590e97d9abfe8d39a590be678eb5919ffc186bb"}, ] [package.dependencies] From 1fd1100ac92effa97ce5833583a1b80f228bda47 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 18 May 2024 00:07:41 +0800 Subject: [PATCH 4/4] Simplify mirror repository API logic (#30963) Fix #30921 (cherry picked from commit 821d2fc2a3cc897f21d707455850177077b72410) (cherry picked from commit 50b4e7f26ea7e24ce193b43144054bff60d08c44) Equivalent to the Gitea v1.22 commit (cherry picked from commit 8eac16de217978c1f7034f8e360f54d8d638e95e) --- modules/structs/repo.go | 2 +- routers/api/v1/repo/repo.go | 12 +++--------- templates/swagger/v1_json.tmpl | 2 +- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/modules/structs/repo.go b/modules/structs/repo.go index f6cc9803a4..b457be4aa4 100644 --- a/modules/structs/repo.go +++ b/modules/structs/repo.go @@ -227,7 +227,7 @@ type EditRepoOption struct { Archived *bool `json:"archived,omitempty"` // set to a string like `8h30m0s` to set the mirror interval time MirrorInterval *string `json:"mirror_interval,omitempty"` - // enable prune - remove obsolete remote-tracking references + // enable prune - remove obsolete remote-tracking references when mirroring EnablePrune *bool `json:"enable_prune,omitempty"` } diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 494f238ffb..1f2d5c768c 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -1054,16 +1054,10 @@ func updateRepoArchivedState(ctx *context.APIContext, opts api.EditRepoOption) e func updateMirror(ctx *context.APIContext, opts api.EditRepoOption) error { repo := ctx.Repo.Repository - // only update mirror if interval or enable prune are provided - if opts.MirrorInterval == nil && opts.EnablePrune == nil { - return nil - } - - // these values only make sense if the repo is a mirror + // Skip this update if the repo is not a mirror, do not return error. + // Because reporting errors only makes the logic more complex&fragile, it doesn't really help end users. if !repo.IsMirror { - err := fmt.Errorf("repo is not a mirror, can not change mirror interval") - ctx.Error(http.StatusUnprocessableEntity, err.Error(), err) - return err + return nil } // get the mirror from the repo diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 41c7cc61ef..12d89d3049 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -20115,7 +20115,7 @@ "x-go-name": "Description" }, "enable_prune": { - "description": "enable prune - remove obsolete remote-tracking references", + "description": "enable prune - remove obsolete remote-tracking references when mirroring", "type": "boolean", "x-go-name": "EnablePrune" },