diff --git a/config-raw.json b/config-raw.json index d7e110eb2..4884d8420 100644 --- a/config-raw.json +++ b/config-raw.json @@ -665,9 +665,10 @@ }, { "key": "providers", - "comment": "A list of enabled providers. This expects an array, which means you cannot use enviroment variables.", + "comment": "A list of enabled providers. You can freely choose the ``. Note that you must add at least one key to a config file if you want to read values from an environment variable as the provider won't be known to Vikunja otherwise.", "children": [ { + "key": "", "children": [ { "key": "name", diff --git a/pkg/config/config.go b/pkg/config/config.go index 8357e279a..273d0a429 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -443,6 +443,10 @@ func getConfigValueFromFile(configKey string) string { func readConfigValuesFromFiles() { keys := viper.AllKeys() for _, key := range keys { + if strings.Contains(key, "auth.openid.providers") { + // Setting openid provider values will remove everything but the value from file + continue + } // Env is evaluated manually at runtime, so we need to check this for each key value := getConfigValueFromFile(key + ".file") if value != "" { diff --git a/pkg/modules/auth/auth.go b/pkg/modules/auth/auth.go index 45fd3d8b5..f78702387 100644 --- a/pkg/modules/auth/auth.go +++ b/pkg/modules/auth/auth.go @@ -21,9 +21,8 @@ import ( "net/http" "time" - "code.vikunja.io/api/pkg/db" - "code.vikunja.io/api/pkg/config" + "code.vikunja.io/api/pkg/db" "code.vikunja.io/api/pkg/models" "code.vikunja.io/api/pkg/user" "code.vikunja.io/api/pkg/web" diff --git a/pkg/modules/auth/openid/cron.go b/pkg/modules/auth/openid/cron.go index dfb153e6e..7fe4b58e3 100644 --- a/pkg/modules/auth/openid/cron.go +++ b/pkg/modules/auth/openid/cron.go @@ -17,13 +17,13 @@ package openid import ( - "code.vikunja.io/api/pkg/log" - "code.vikunja.io/api/pkg/models" - "xorm.io/builder" - "xorm.io/xorm" - "code.vikunja.io/api/pkg/cron" "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/log" + "code.vikunja.io/api/pkg/models" + + "xorm.io/builder" + "xorm.io/xorm" ) func RemoveEmptySSOTeams(s *xorm.Session) (err error) { diff --git a/pkg/modules/auth/openid/openid.go b/pkg/modules/auth/openid/openid.go index ac9d435c6..f1fcc06a1 100644 --- a/pkg/modules/auth/openid/openid.go +++ b/pkg/modules/auth/openid/openid.go @@ -24,14 +24,14 @@ import ( "strconv" "strings" - "code.vikunja.io/api/pkg/web/handler" - "code.vikunja.io/api/pkg/db" "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/models" "code.vikunja.io/api/pkg/modules/auth" "code.vikunja.io/api/pkg/user" "code.vikunja.io/api/pkg/utils" + "code.vikunja.io/api/pkg/web/handler" + "github.com/coreos/go-oidc/v3/oidc" petname "github.com/dustinkirkland/golang-petname" "github.com/labstack/echo/v4" @@ -100,7 +100,6 @@ func HandleCallback(c echo.Context) error { provider, err := GetProvider(providerKey) log.Debugf("Provider: %v", provider) if err != nil { - log.Error(err) return handler.HandleHTTPError(err) } if provider == nil { @@ -114,7 +113,6 @@ func HandleCallback(c echo.Context) error { if err != nil { var rerr *oauth2.RetrieveError if errors.As(err, &rerr) { - log.Error(err) details := make(map[string]interface{}) if err := json.Unmarshal(rerr.Body, &details); err != nil { @@ -122,6 +120,7 @@ func HandleCallback(c echo.Context) error { return handler.HandleHTTPError(err) } + log.Error(err) return c.JSON(http.StatusBadRequest, map[string]interface{}{ "message": "Could not authenticate against third party.", "details": details, diff --git a/pkg/modules/auth/openid/providers.go b/pkg/modules/auth/openid/providers.go index 5e14c1b31..8e9ec5a71 100644 --- a/pkg/modules/auth/openid/providers.go +++ b/pkg/modules/auth/openid/providers.go @@ -17,14 +17,12 @@ package openid import ( - "regexp" "strconv" - "strings" - - "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/config" + "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/modules/keyvalue" + "github.com/coreos/go-oidc/v3/oidc" "golang.org/x/oauth2" ) @@ -43,14 +41,14 @@ func GetAllProviders() (providers []*Provider, err error) { return nil, nil } - rawProvider := rawProviders.([]interface{}) + rawProvider := rawProviders.(map[string]interface{}) - for _, p := range rawProvider { + for key, p := range rawProvider { var pi map[string]interface{} var is bool pi, is = p.(map[string]interface{}) // JSON config is a map[string]interface{}, other providers are not. Under the hood they are all strings so - // it is save to cast. + // it is safe to cast. if !is { pis := p.(map[interface{}]interface{}) pi = make(map[string]interface{}, len(pis)) @@ -59,21 +57,21 @@ func GetAllProviders() (providers []*Provider, err error) { } } - provider, err := getProviderFromMap(pi) + provider, err := getProviderFromMap(pi, key) if err != nil { - if provider != nil { - log.Errorf("Error while getting openid provider %s: %s", provider.Name, err) - continue - } - log.Errorf("Error while getting openid provider: %s", err) + log.Errorf("Error while getting openid provider %s: %s", key, err) + continue + } + + if provider == nil { + log.Errorf("Could not openid provider %s, please check your config", key) continue } providers = append(providers, provider) - k := getKeyFromName(pi["name"].(string)) - err = keyvalue.Put("openid_provider_"+k, provider) + err = keyvalue.Put("openid_provider_"+key, provider) if err != nil { return nil, err } @@ -107,19 +105,29 @@ func GetProvider(key string) (provider *Provider, err error) { return } -func getKeyFromName(name string) string { - reg := regexp.MustCompile("[^a-z0-9]+") - return reg.ReplaceAllString(strings.ToLower(name), "") -} +func getProviderFromMap(pi map[string]interface{}, key string) (provider *Provider, err error) { + + for _, configKey := range []string{ + // Values from environment variables are evaluated at runtime, hence we need to check them explicitly + // through viper to make sure we catch all of them. + "name", + "logouturl", + "scope", + "authurl", + "clientsecret", + "clientid", + } { + valueFromFile := config.GetConfigValueFromFile("auth.openid.providers." + key + "." + configKey) + if valueFromFile != "" { + pi[configKey] = valueFromFile + } + } -func getProviderFromMap(pi map[string]interface{}) (provider *Provider, err error) { name, is := pi["name"].(string) if !is { return nil, nil } - k := getKeyFromName(name) - logoutURL, ok := pi["logouturl"].(string) if !ok { logoutURL = "" @@ -130,8 +138,8 @@ func getProviderFromMap(pi map[string]interface{}) (provider *Provider, err erro scope = "openid profile email" } provider = &Provider{ - Name: pi["name"].(string), - Key: k, + Name: name, + Key: key, AuthURL: pi["authurl"].(string), OriginalAuthURL: pi["authurl"].(string), ClientSecret: pi["clientsecret"].(string),