From 5cf7da63ee74939595b8800787dcdb4c7290fa4f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 25 Apr 2023 23:06:39 +0800 Subject: [PATCH] Refactor config provider (#24245) This PR introduces more abstract about `ConfigProvider` and hides more `ini` references. --------- Co-authored-by: delvh --- cmd/web.go | 8 +- modules/indexer/issues/indexer_test.go | 3 +- modules/indexer/stats/indexer_test.go | 3 +- modules/setting/config_provider.go | 145 ++++++++++++++++++- modules/setting/cron_test.go | 9 +- modules/setting/lfs.go | 13 +- modules/setting/log.go | 8 +- modules/setting/mailer_test.go | 3 +- modules/setting/markup.go | 8 +- modules/setting/oauth2.go | 21 ++- modules/setting/packages.go | 3 +- modules/setting/queue.go | 4 +- modules/setting/security.go | 15 +- modules/setting/setting.go | 93 ++---------- modules/setting/storage.go | 8 +- modules/setting/storage_test.go | 19 ++- services/auth/source/oauth2/jwtsigningkey.go | 28 ++-- 17 files changed, 227 insertions(+), 164 deletions(-) diff --git a/cmd/web.go b/cmd/web.go index 11620c6b0..e451cf7df 100644 --- a/cmd/web.go +++ b/cmd/web.go @@ -24,7 +24,6 @@ import ( "github.com/felixge/fgprof" "github.com/urfave/cli" - ini "gopkg.in/ini.v1" ) // PIDFile could be set from build tag @@ -223,9 +222,10 @@ func setPort(port string) error { defaultLocalURL += ":" + setting.HTTPPort + "/" // Save LOCAL_ROOT_URL if port changed - setting.CreateOrAppendToCustomConf("server.LOCAL_ROOT_URL", func(cfg *ini.File) { - cfg.Section("server").Key("LOCAL_ROOT_URL").SetValue(defaultLocalURL) - }) + setting.CfgProvider.Section("server").Key("LOCAL_ROOT_URL").SetValue(defaultLocalURL) + if err := setting.CfgProvider.Save(); err != nil { + return fmt.Errorf("Failed to save config file: %v", err) + } } return nil } diff --git a/modules/indexer/issues/indexer_test.go b/modules/indexer/issues/indexer_test.go index bf9d6d0d1..c7c7cea90 100644 --- a/modules/indexer/issues/indexer_test.go +++ b/modules/indexer/issues/indexer_test.go @@ -16,7 +16,6 @@ import ( _ "code.gitea.io/gitea/models" "github.com/stretchr/testify/assert" - "gopkg.in/ini.v1" ) func TestMain(m *testing.M) { @@ -27,7 +26,7 @@ func TestMain(m *testing.M) { func TestBleveSearchIssues(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - setting.CfgProvider = ini.Empty() + setting.CfgProvider = setting.NewEmptyConfigProvider() tmpIndexerDir := t.TempDir() diff --git a/modules/indexer/stats/indexer_test.go b/modules/indexer/stats/indexer_test.go index 50a5fade7..8d9b4e36d 100644 --- a/modules/indexer/stats/indexer_test.go +++ b/modules/indexer/stats/indexer_test.go @@ -18,7 +18,6 @@ import ( _ "code.gitea.io/gitea/models" "github.com/stretchr/testify/assert" - "gopkg.in/ini.v1" ) func TestMain(m *testing.M) { @@ -29,7 +28,7 @@ func TestMain(m *testing.M) { func TestRepoStatsIndex(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - setting.CfgProvider = ini.Empty() + setting.CfgProvider = setting.NewEmptyConfigProvider() setting.LoadQueueSettings() diff --git a/modules/setting/config_provider.go b/modules/setting/config_provider.go index 0244a8d06..92c8c97fe 100644 --- a/modules/setting/config_provider.go +++ b/modules/setting/config_provider.go @@ -4,20 +4,157 @@ package setting import ( + "fmt" + "os" + "path/filepath" + "strings" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" ini "gopkg.in/ini.v1" ) +type ConfigSection interface { + Name() string + MapTo(interface{}) error + HasKey(key string) bool + NewKey(name, value string) (*ini.Key, error) + Key(key string) *ini.Key + Keys() []*ini.Key + ChildSections() []*ini.Section +} + // ConfigProvider represents a config provider type ConfigProvider interface { - Section(section string) *ini.Section - NewSection(name string) (*ini.Section, error) - GetSection(name string) (*ini.Section, error) + Section(section string) ConfigSection + NewSection(name string) (ConfigSection, error) + GetSection(name string) (ConfigSection, error) + DeleteSection(name string) error + Save() error +} + +type iniFileConfigProvider struct { + *ini.File + filepath string // the ini file path + newFile bool // whether the file has not existed previously + allowEmpty bool // whether not finding configuration files is allowed (only true for the tests) +} + +// NewEmptyConfigProvider create a new empty config provider +func NewEmptyConfigProvider() ConfigProvider { + cp, _ := newConfigProviderFromData("") + return cp +} + +// newConfigProviderFromData this function is only for testing +func newConfigProviderFromData(configContent string) (ConfigProvider, error) { + var cfg *ini.File + var err error + if configContent == "" { + cfg = ini.Empty() + } else { + cfg, err = ini.Load(strings.NewReader(configContent)) + if err != nil { + return nil, err + } + } + cfg.NameMapper = ini.SnackCase + return &iniFileConfigProvider{ + File: cfg, + newFile: true, + }, nil +} + +// newConfigProviderFromFile load configuration from file. +// NOTE: do not print any log except error. +func newConfigProviderFromFile(customConf string, allowEmpty bool, extraConfig string) (*iniFileConfigProvider, error) { + cfg := ini.Empty() + newFile := true + + if customConf != "" { + isFile, err := util.IsFile(customConf) + if err != nil { + return nil, fmt.Errorf("unable to check if %s is a file. Error: %v", customConf, err) + } + if isFile { + if err := cfg.Append(customConf); err != nil { + return nil, fmt.Errorf("failed to load custom conf '%s': %v", customConf, err) + } + newFile = false + } + } + + if newFile && !allowEmpty { + return nil, fmt.Errorf("unable to find configuration file: %q, please ensure you are running in the correct environment or set the correct configuration file with -c", CustomConf) + } + + if extraConfig != "" { + if err := cfg.Append([]byte(extraConfig)); err != nil { + return nil, fmt.Errorf("unable to append more config: %v", err) + } + } + + cfg.NameMapper = ini.SnackCase + return &iniFileConfigProvider{ + File: cfg, + filepath: customConf, + newFile: newFile, + allowEmpty: allowEmpty, + }, nil +} + +func (p *iniFileConfigProvider) Section(section string) ConfigSection { + return p.File.Section(section) +} + +func (p *iniFileConfigProvider) NewSection(name string) (ConfigSection, error) { + return p.File.NewSection(name) +} + +func (p *iniFileConfigProvider) GetSection(name string) (ConfigSection, error) { + return p.File.GetSection(name) +} + +func (p *iniFileConfigProvider) DeleteSection(name string) error { + p.File.DeleteSection(name) + return nil +} + +// Save save the content into file +func (p *iniFileConfigProvider) Save() error { + if p.filepath == "" { + if !p.allowEmpty { + return fmt.Errorf("custom config path must not be empty") + } + return nil + } + + if p.newFile { + if err := os.MkdirAll(filepath.Dir(CustomConf), os.ModePerm); err != nil { + return fmt.Errorf("failed to create '%s': %v", CustomConf, err) + } + } + if err := p.SaveTo(p.filepath); err != nil { + return fmt.Errorf("failed to save '%s': %v", p.filepath, err) + } + + // Change permissions to be more restrictive + fi, err := os.Stat(CustomConf) + if err != nil { + return fmt.Errorf("failed to determine current conf file permissions: %v", err) + } + + if fi.Mode().Perm() > 0o600 { + if err = os.Chmod(CustomConf, 0o600); err != nil { + log.Warn("Failed changing conf file permissions to -rw-------. Consider changing them manually.") + } + } + return nil } // a file is an implementation ConfigProvider and other implementations are possible, i.e. from docker, k8s, … -var _ ConfigProvider = &ini.File{} +var _ ConfigProvider = &iniFileConfigProvider{} func mustMapSetting(rootCfg ConfigProvider, sectionName string, setting interface{}) { if err := rootCfg.Section(sectionName).MapTo(setting); err != nil { diff --git a/modules/setting/cron_test.go b/modules/setting/cron_test.go index be97e59bd..8d58cf8b4 100644 --- a/modules/setting/cron_test.go +++ b/modules/setting/cron_test.go @@ -7,7 +7,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - ini "gopkg.in/ini.v1" ) func Test_getCronSettings(t *testing.T) { @@ -23,11 +22,11 @@ func Test_getCronSettings(t *testing.T) { iniStr := ` [cron.test] -Base = true -Second = white rabbit -Extend = true +BASE = true +SECOND = white rabbit +EXTEND = true ` - cfg, err := ini.Load([]byte(iniStr)) + cfg, err := newConfigProviderFromData(iniStr) assert.NoError(t, err) extended := &Extended{ diff --git a/modules/setting/lfs.go b/modules/setting/lfs.go index e04cde100..1b659dd22 100644 --- a/modules/setting/lfs.go +++ b/modules/setting/lfs.go @@ -9,8 +9,6 @@ import ( "code.gitea.io/gitea/modules/generate" "code.gitea.io/gitea/modules/log" - - ini "gopkg.in/ini.v1" ) // LFS represents the configuration for Git LFS @@ -38,8 +36,7 @@ func loadLFSFrom(rootCfg ConfigProvider) { // DEPRECATED should not be removed because users maybe upgrade from lower version to the latest version // if these are removed, the warning will not be shown deprecatedSetting(rootCfg, "server", "LFS_CONTENT_PATH", "lfs", "PATH", "v1.19.0") - lfsSec.Key("PATH").MustString( - sec.Key("LFS_CONTENT_PATH").String()) + lfsSec.Key("PATH").MustString(sec.Key("LFS_CONTENT_PATH").String()) LFS.Storage = getStorage(rootCfg, "lfs", storageType, lfsSec) @@ -62,9 +59,11 @@ func loadLFSFrom(rootCfg ConfigProvider) { } // Save secret - CreateOrAppendToCustomConf("server.LFS_JWT_SECRET", func(cfg *ini.File) { - cfg.Section("server").Key("LFS_JWT_SECRET").SetValue(LFS.JWTSecretBase64) - }) + sec.Key("LFS_JWT_SECRET").SetValue(LFS.JWTSecretBase64) + if err := rootCfg.Save(); err != nil { + log.Fatal("Error saving JWT Secret for custom config: %v", err) + return + } } } } diff --git a/modules/setting/log.go b/modules/setting/log.go index 1ff710073..d9a9e5af8 100644 --- a/modules/setting/log.go +++ b/modules/setting/log.go @@ -15,8 +15,6 @@ import ( "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/util" - - ini "gopkg.in/ini.v1" ) var ( @@ -131,12 +129,12 @@ type LogDescription struct { SubLogDescriptions []SubLogDescription } -func getLogLevel(section *ini.Section, key string, defaultValue log.Level) log.Level { +func getLogLevel(section ConfigSection, key string, defaultValue log.Level) log.Level { value := section.Key(key).MustString(defaultValue.String()) return log.FromString(value) } -func getStacktraceLogLevel(section *ini.Section, key, defaultValue string) string { +func getStacktraceLogLevel(section ConfigSection, key, defaultValue string) string { value := section.Key(key).MustString(defaultValue) return log.FromString(value).String() } @@ -165,7 +163,7 @@ func loadLogFrom(rootCfg ConfigProvider) { Log.EnableXORMLog = rootCfg.Section("log").Key("ENABLE_XORM_LOG").MustBool(true) } -func generateLogConfig(sec *ini.Section, name string, defaults defaultLogOptions) (mode, jsonConfig, levelName string) { +func generateLogConfig(sec ConfigSection, name string, defaults defaultLogOptions) (mode, jsonConfig, levelName string) { level := getLogLevel(sec, "LEVEL", Log.Level) levelName = level.String() stacktraceLevelName := getStacktraceLogLevel(sec, "STACKTRACE_LEVEL", Log.StacktraceLogLevel) diff --git a/modules/setting/mailer_test.go b/modules/setting/mailer_test.go index 4cfd6142b..f65dbaf8f 100644 --- a/modules/setting/mailer_test.go +++ b/modules/setting/mailer_test.go @@ -7,11 +7,10 @@ import ( "testing" "github.com/stretchr/testify/assert" - ini "gopkg.in/ini.v1" ) func Test_loadMailerFrom(t *testing.T) { - iniFile := ini.Empty() + iniFile := NewEmptyConfigProvider() kases := map[string]*Mailer{ "smtp.mydomain.com": { SMTPAddr: "smtp.mydomain.com", diff --git a/modules/setting/markup.go b/modules/setting/markup.go index b71a902be..969e30e88 100644 --- a/modules/setting/markup.go +++ b/modules/setting/markup.go @@ -8,8 +8,6 @@ import ( "strings" "code.gitea.io/gitea/modules/log" - - "gopkg.in/ini.v1" ) // ExternalMarkupRenderers represents the external markup renderers @@ -82,7 +80,7 @@ func loadMarkupFrom(rootCfg ConfigProvider) { } } -func newMarkupSanitizer(name string, sec *ini.Section) { +func newMarkupSanitizer(name string, sec ConfigSection) { rule, ok := createMarkupSanitizerRule(name, sec) if ok { if strings.HasPrefix(name, "sanitizer.") { @@ -99,7 +97,7 @@ func newMarkupSanitizer(name string, sec *ini.Section) { } } -func createMarkupSanitizerRule(name string, sec *ini.Section) (MarkupSanitizerRule, bool) { +func createMarkupSanitizerRule(name string, sec ConfigSection) (MarkupSanitizerRule, bool) { var rule MarkupSanitizerRule ok := false @@ -141,7 +139,7 @@ func createMarkupSanitizerRule(name string, sec *ini.Section) (MarkupSanitizerRu return rule, true } -func newMarkupRenderer(name string, sec *ini.Section) { +func newMarkupRenderer(name string, sec ConfigSection) { extensionReg := regexp.MustCompile(`\.\w`) extensions := sec.Key("FILE_EXTENSIONS").Strings(",") diff --git a/modules/setting/oauth2.go b/modules/setting/oauth2.go index 44f5568ef..4dab468c1 100644 --- a/modules/setting/oauth2.go +++ b/modules/setting/oauth2.go @@ -4,12 +4,12 @@ package setting import ( + "encoding/base64" "math" "path/filepath" + "code.gitea.io/gitea/modules/generate" "code.gitea.io/gitea/modules/log" - - "gopkg.in/ini.v1" ) // OAuth2UsernameType is enum describing the way gitea 'name' should be generated from oauth2 data @@ -80,7 +80,7 @@ func loadOAuth2ClientFrom(rootCfg ConfigProvider) { } } -func parseScopes(sec *ini.Section, name string) []string { +func parseScopes(sec ConfigSection, name string) []string { parts := sec.Key(name).Strings(" ") scopes := make([]string, 0, len(parts)) for _, scope := range parts { @@ -119,4 +119,19 @@ func loadOAuth2From(rootCfg ConfigProvider) { if !filepath.IsAbs(OAuth2.JWTSigningPrivateKeyFile) { OAuth2.JWTSigningPrivateKeyFile = filepath.Join(AppDataPath, OAuth2.JWTSigningPrivateKeyFile) } + + key := make([]byte, 32) + n, err := base64.RawURLEncoding.Decode(key, []byte(OAuth2.JWTSecretBase64)) + if err != nil || n != 32 { + key, err = generate.NewJwtSecret() + if err != nil { + log.Fatal("error generating JWT secret: %v", err) + } + + secretBase64 := base64.RawURLEncoding.EncodeToString(key) + rootCfg.Section("oauth2").Key("JWT_SECRET").SetValue(secretBase64) + if err := rootCfg.Save(); err != nil { + log.Fatal("save oauth2.JWT_SECRET failed: %v", err) + } + } } diff --git a/modules/setting/packages.go b/modules/setting/packages.go index ac0ad62bc..89601c3b9 100644 --- a/modules/setting/packages.go +++ b/modules/setting/packages.go @@ -12,7 +12,6 @@ import ( "code.gitea.io/gitea/modules/log" "github.com/dustin/go-humanize" - ini "gopkg.in/ini.v1" ) // Package registry settings @@ -86,7 +85,7 @@ func loadPackagesFrom(rootCfg ConfigProvider) { Packages.LimitSizeVagrant = mustBytes(sec, "LIMIT_SIZE_VAGRANT") } -func mustBytes(section *ini.Section, key string) int64 { +func mustBytes(section ConfigSection, key string) int64 { const noLimit = "-1" value := section.Key(key).MustString(noLimit) diff --git a/modules/setting/queue.go b/modules/setting/queue.go index bd4bf48e3..8c37e538b 100644 --- a/modules/setting/queue.go +++ b/modules/setting/queue.go @@ -10,8 +10,6 @@ import ( "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/log" - - ini "gopkg.in/ini.v1" ) // QueueSettings represent the settings for a queue from the ini @@ -195,7 +193,7 @@ func handleOldLengthConfiguration(rootCfg ConfigProvider, queueName, oldSection, // toDirectlySetKeysSet returns a set of keys directly set by this section // Note: we cannot use section.HasKey(...) as that will immediately set the Key if a parent section has the Key // but this section does not. -func toDirectlySetKeysSet(section *ini.Section) container.Set[string] { +func toDirectlySetKeysSet(section ConfigSection) container.Set[string] { sections := make(container.Set[string]) for _, key := range section.Keys() { sections.Add(key.Name()) diff --git a/modules/setting/security.go b/modules/setting/security.go index b9841cdb9..ce2e7711f 100644 --- a/modules/setting/security.go +++ b/modules/setting/security.go @@ -11,8 +11,6 @@ import ( "code.gitea.io/gitea/modules/auth/password/hash" "code.gitea.io/gitea/modules/generate" "code.gitea.io/gitea/modules/log" - - ini "gopkg.in/ini.v1" ) var ( @@ -43,7 +41,7 @@ var ( // 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 *ini.Section, uriKey, verbatimKey string) string { +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() @@ -84,16 +82,17 @@ func loadSecret(sec *ini.Section, uriKey, verbatimKey string) string { } // generateSaveInternalToken generates and saves the internal token to app.ini -func generateSaveInternalToken() { +func generateSaveInternalToken(rootCfg ConfigProvider) { token, err := generate.NewInternalToken() if err != nil { log.Fatal("Error generate internal token: %v", err) } InternalToken = token - CreateOrAppendToCustomConf("security.INTERNAL_TOKEN", func(cfg *ini.File) { - cfg.Section("security").Key("INTERNAL_TOKEN").SetValue(token) - }) + rootCfg.Section("security").Key("INTERNAL_TOKEN").SetValue(token) + if err := rootCfg.Save(); err != nil { + log.Fatal("Error saving internal token: %v", err) + } } func loadSecurityFrom(rootCfg ConfigProvider) { @@ -141,7 +140,7 @@ func loadSecurityFrom(rootCfg ConfigProvider) { 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() + generateSaveInternalToken(rootCfg) } cfgdata := sec.Key("PASSWORD_COMPLEXITY").Strings(",") diff --git a/modules/setting/setting.go b/modules/setting/setting.go index e1a57615a..7a1b7d17a 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -18,9 +18,6 @@ import ( "code.gitea.io/gitea/modules/auth/password/hash" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/user" - "code.gitea.io/gitea/modules/util" - - ini "gopkg.in/ini.v1" ) // settings @@ -208,17 +205,29 @@ func PrepareAppDataPath() error { // InitProviderFromExistingFile initializes config provider from an existing config file (app.ini) func InitProviderFromExistingFile() { - CfgProvider = newFileProviderFromConf(CustomConf, false, "") + var err error + CfgProvider, err = newConfigProviderFromFile(CustomConf, false, "") + if err != nil { + log.Fatal("InitProviderFromExistingFile: %v", err) + } } // InitProviderAllowEmpty initializes config provider from file, it's also fine that if the config file (app.ini) doesn't exist func InitProviderAllowEmpty() { - CfgProvider = newFileProviderFromConf(CustomConf, true, "") + var err error + CfgProvider, err = newConfigProviderFromFile(CustomConf, true, "") + if err != nil { + log.Fatal("InitProviderAllowEmpty: %v", err) + } } // InitProviderAndLoadCommonSettingsForTest initializes config provider and load common setttings for tests func InitProviderAndLoadCommonSettingsForTest(extraConfigs ...string) { - CfgProvider = newFileProviderFromConf(CustomConf, true, strings.Join(extraConfigs, "\n")) + var err error + CfgProvider, err = newConfigProviderFromFile(CustomConf, true, strings.Join(extraConfigs, "\n")) + if err != nil { + log.Fatal("InitProviderAndLoadCommonSettingsForTest: %v", err) + } loadCommonSettingsFrom(CfgProvider) if err := PrepareAppDataPath(); err != nil { log.Fatal("Can not prepare APP_DATA_PATH: %v", err) @@ -229,33 +238,6 @@ func InitProviderAndLoadCommonSettingsForTest(extraConfigs ...string) { PasswordHashAlgo, _ = hash.SetDefaultPasswordHashAlgorithm("dummy") } -// newFileProviderFromConf initializes configuration context. -// NOTE: do not print any log except error. -func newFileProviderFromConf(customConf string, allowEmpty bool, extraConfig string) *ini.File { - cfg := ini.Empty() - - isFile, err := util.IsFile(customConf) - if err != nil { - log.Error("Unable to check if %s is a file. Error: %v", customConf, err) - } - if isFile { - if err := cfg.Append(customConf); err != nil { - log.Fatal("Failed to load custom conf '%s': %v", customConf, err) - } - } else if !allowEmpty { - log.Fatal("Unable to find configuration file: %q.\nEnsure you are running in the correct environment or set the correct configuration file with -c.", CustomConf) - } // else: no config file, a config file might be created at CustomConf later (might not) - - if extraConfig != "" { - if err = cfg.Append([]byte(extraConfig)); err != nil { - log.Fatal("Unable to append more config: %v", err) - } - } - - cfg.NameMapper = ini.SnackCase - return cfg -} - // LoadCommonSettings loads common configurations from a configuration provider. func LoadCommonSettings() { loadCommonSettingsFrom(CfgProvider) @@ -319,51 +301,6 @@ func loadRunModeFrom(rootCfg ConfigProvider) { } } -// CreateOrAppendToCustomConf creates or updates the custom config. -// Use the callback to set individual values. -func CreateOrAppendToCustomConf(purpose string, callback func(cfg *ini.File)) { - if CustomConf == "" { - log.Error("Custom config path must not be empty") - return - } - - cfg := ini.Empty() - isFile, err := util.IsFile(CustomConf) - if err != nil { - log.Error("Unable to check if %s is a file. Error: %v", CustomConf, err) - } - if isFile { - if err := cfg.Append(CustomConf); err != nil { - log.Error("failed to load custom conf %s: %v", CustomConf, err) - return - } - } - - callback(cfg) - - if err := os.MkdirAll(filepath.Dir(CustomConf), os.ModePerm); err != nil { - log.Fatal("failed to create '%s': %v", CustomConf, err) - return - } - if err := cfg.SaveTo(CustomConf); err != nil { - log.Fatal("error saving to custom config: %v", err) - } - log.Info("Settings for %s saved to: %q", purpose, CustomConf) - - // Change permissions to be more restrictive - fi, err := os.Stat(CustomConf) - if err != nil { - log.Error("Failed to determine current conf file permissions: %v", err) - return - } - - if fi.Mode().Perm() > 0o600 { - if err = os.Chmod(CustomConf, 0o600); err != nil { - log.Warn("Failed changing conf file permissions to -rw-------. Consider changing them manually.") - } - } -} - // LoadSettings initializes the settings for normal start up func LoadSettings() { loadDBSetting(CfgProvider) diff --git a/modules/setting/storage.go b/modules/setting/storage.go index 50d4c8439..6da52807e 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -6,15 +6,13 @@ package setting import ( "path/filepath" "reflect" - - ini "gopkg.in/ini.v1" ) // Storage represents configuration of storages type Storage struct { Type string Path string - Section *ini.Section + Section ConfigSection ServeDirect bool } @@ -30,7 +28,7 @@ func (s *Storage) MapTo(v interface{}) error { return nil } -func getStorage(rootCfg ConfigProvider, name, typ string, targetSec *ini.Section) Storage { +func getStorage(rootCfg ConfigProvider, name, typ string, targetSec ConfigSection) Storage { const sectionName = "storage" sec := rootCfg.Section(sectionName) @@ -52,7 +50,7 @@ func getStorage(rootCfg ConfigProvider, name, typ string, targetSec *ini.Section storage.Section = targetSec storage.Type = typ - overrides := make([]*ini.Section, 0, 3) + overrides := make([]ConfigSection, 0, 3) nameSec, err := rootCfg.GetSection(sectionName + "." + name) if err == nil { overrides = append(overrides, nameSec) diff --git a/modules/setting/storage_test.go b/modules/setting/storage_test.go index 7737d233b..9c51bbc08 100644 --- a/modules/setting/storage_test.go +++ b/modules/setting/storage_test.go @@ -7,7 +7,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - ini "gopkg.in/ini.v1" ) func Test_getStorageCustomType(t *testing.T) { @@ -20,7 +19,7 @@ MINIO_BUCKET = gitea-attachment STORAGE_TYPE = minio MINIO_ENDPOINT = my_minio:9000 ` - cfg, err := ini.Load([]byte(iniStr)) + cfg, err := newConfigProviderFromData(iniStr) assert.NoError(t, err) sec := cfg.Section("attachment") @@ -43,7 +42,7 @@ MINIO_BUCKET = gitea-attachment [storage.minio] MINIO_BUCKET = gitea ` - cfg, err := ini.Load([]byte(iniStr)) + cfg, err := newConfigProviderFromData(iniStr) assert.NoError(t, err) sec := cfg.Section("attachment") @@ -65,7 +64,7 @@ MINIO_BUCKET = gitea-minio [storage] MINIO_BUCKET = gitea ` - cfg, err := ini.Load([]byte(iniStr)) + cfg, err := newConfigProviderFromData(iniStr) assert.NoError(t, err) sec := cfg.Section("attachment") @@ -88,7 +87,7 @@ MINIO_BUCKET = gitea [storage] STORAGE_TYPE = local ` - cfg, err := ini.Load([]byte(iniStr)) + cfg, err := newConfigProviderFromData(iniStr) assert.NoError(t, err) sec := cfg.Section("attachment") @@ -100,7 +99,7 @@ STORAGE_TYPE = local } func Test_getStorageGetDefaults(t *testing.T) { - cfg, err := ini.Load([]byte("")) + cfg, err := newConfigProviderFromData("") assert.NoError(t, err) sec := cfg.Section("attachment") @@ -121,7 +120,7 @@ MINIO_BUCKET = gitea-attachment [storage] MINIO_BUCKET = gitea-storage ` - cfg, err := ini.Load([]byte(iniStr)) + cfg, err := newConfigProviderFromData(iniStr) assert.NoError(t, err) { @@ -155,7 +154,7 @@ STORAGE_TYPE = lfs [storage.lfs] MINIO_BUCKET = gitea-storage ` - cfg, err := ini.Load([]byte(iniStr)) + cfg, err := newConfigProviderFromData(iniStr) assert.NoError(t, err) { @@ -179,7 +178,7 @@ func Test_getStorageInheritStorageType(t *testing.T) { [storage] STORAGE_TYPE = minio ` - cfg, err := ini.Load([]byte(iniStr)) + cfg, err := newConfigProviderFromData(iniStr) assert.NoError(t, err) sec := cfg.Section("attachment") @@ -194,7 +193,7 @@ func Test_getStorageInheritNameSectionType(t *testing.T) { [storage.attachments] STORAGE_TYPE = minio ` - cfg, err := ini.Load([]byte(iniStr)) + cfg, err := newConfigProviderFromData(iniStr) assert.NoError(t, err) sec := cfg.Section("attachment") diff --git a/services/auth/source/oauth2/jwtsigningkey.go b/services/auth/source/oauth2/jwtsigningkey.go index 94feddbf6..ed60952ac 100644 --- a/services/auth/source/oauth2/jwtsigningkey.go +++ b/services/auth/source/oauth2/jwtsigningkey.go @@ -18,14 +18,12 @@ import ( "path/filepath" "strings" - "code.gitea.io/gitea/modules/generate" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" "github.com/golang-jwt/jwt/v4" "github.com/minio/sha256-simd" - ini "gopkg.in/ini.v1" ) // ErrInvalidAlgorithmType represents an invalid algorithm error. @@ -316,8 +314,7 @@ func InitSigningKey() error { case "HS384": fallthrough case "HS512": - key, err = loadOrCreateSymmetricKey() - + key, err = loadSymmetricKey() case "RS256": fallthrough case "RS384": @@ -332,7 +329,6 @@ func InitSigningKey() error { fallthrough case "EdDSA": key, err = loadOrCreateAsymmetricKey() - default: return ErrInvalidAlgorithmType{setting.OAuth2.JWTSigningAlgorithm} } @@ -351,22 +347,16 @@ func InitSigningKey() error { return nil } -// loadOrCreateSymmetricKey checks if the configured secret is valid. -// If it is not valid a new secret is created and saved in the configuration file. -func loadOrCreateSymmetricKey() (interface{}, error) { +// loadSymmetricKey checks if the configured secret is valid. +// If it is not valid, it will return an error. +func loadSymmetricKey() (interface{}, error) { key := make([]byte, 32) n, err := base64.RawURLEncoding.Decode(key, []byte(setting.OAuth2.JWTSecretBase64)) - if err != nil || n != 32 { - key, err = generate.NewJwtSecret() - if err != nil { - log.Fatal("error generating JWT secret: %v", err) - return nil, err - } - - setting.CreateOrAppendToCustomConf("oauth2.JWT_SECRET", func(cfg *ini.File) { - secretBase64 := base64.RawURLEncoding.EncodeToString(key) - cfg.Section("oauth2").Key("JWT_SECRET").SetValue(secretBase64) - }) + if err != nil { + return nil, err + } + if n != 32 { + return nil, fmt.Errorf("JWT secret must be 32 bytes long") } return key, nil