diff --git a/pkg/services/ssosettings/ssosettingsimpl/service.go b/pkg/services/ssosettings/ssosettingsimpl/service.go index 9466848d7ce..b753f75cc8d 100644 --- a/pkg/services/ssosettings/ssosettingsimpl/service.go +++ b/pkg/services/ssosettings/ssosettingsimpl/service.go @@ -433,6 +433,7 @@ func removeSecrets(settings map[string]any) map[string]any { // mergeSettings merges two maps in a way that the values from the first map are preserved // and the values from the second map are added only if they don't exist in the first map +// or if they contain empty URLs. func mergeSettings(storedSettings, systemSettings map[string]any) map[string]any { settings := make(map[string]any) @@ -443,6 +444,12 @@ func mergeSettings(storedSettings, systemSettings map[string]any) map[string]any for k, v := range systemSettings { if _, ok := settings[k]; !ok { settings[k] = v + } else if isURL(k) && isEmptyString(settings[k]) { + // Overwrite all URL settings from the DB containing an empty string with their value + // from the system settings. This fixes an issue with empty auth_url, api_url and token_url + // from the DB not being replaced with their values defined in the system settings for + // the Google provider. + settings[k] = v } } @@ -486,6 +493,15 @@ func isSecret(fieldName string) bool { return false } +func isURL(fieldName string) bool { + return strings.HasSuffix(fieldName, "_url") +} + +func isEmptyString(val any) bool { + _, ok := val.(string) + return ok && val == "" +} + func isNewSecretValue(value string) bool { return value != setting.RedactedPassword } diff --git a/pkg/services/ssosettings/ssosettingsimpl/service_test.go b/pkg/services/ssosettings/ssosettingsimpl/service_test.go index 6f736c5479e..10fa239fdbb 100644 --- a/pkg/services/ssosettings/ssosettingsimpl/service_test.go +++ b/pkg/services/ssosettings/ssosettingsimpl/service_test.go @@ -185,6 +185,42 @@ func TestService_GetForProvider(t *testing.T) { }, wantErr: true, }, + { + name: "correctly merge the DB and system settings", + setup: func(env testEnv) { + env.store.ExpectedSSOSetting = &models.SSOSettings{ + Provider: "github", + Settings: map[string]any{ + "enabled": true, + "auth_url": "", + "api_url": "https://overwritten-api.com/user", + "team_ids": "", + }, + Source: models.DB, + } + env.fallbackStrategy.ExpectedIsMatch = true + env.fallbackStrategy.ExpectedConfigs = map[string]map[string]any{ + "github": { + "auth_url": "https://github.com/login/oauth/authorize", + "token_url": "https://github.com/login/oauth/access_token", + "api_url": "https://api.github.com/user", + "team_ids": "10,11,12", + }, + } + }, + want: &models.SSOSettings{ + Provider: "github", + Settings: map[string]any{ + "enabled": true, + "auth_url": "https://github.com/login/oauth/authorize", + "token_url": "https://github.com/login/oauth/access_token", + "api_url": "https://overwritten-api.com/user", + "team_ids": "", + }, + Source: models.DB, + }, + wantErr: false, + }, } for _, tc := range testCases {