feat!: config for auth providers now use a map instead of an array

The config values for openid providers now use a map with the provider as key instead of an array. For example before:

auth:
  openid:
    providers:
      - name: foo
        clientid: ...

now becomes:

auth:
  openid:
    providers:
      foo:
        clientid: ...

This allows us to read values for openid providers from files using the same syntax as everywhere and makes the configuration more predictable. It also allows configuring providers through env variables, though it is still required to set at least one value via the config file because Vikunja won't discover the provider otherwise.
This commit is contained in:
kolaente 2024-11-18 10:34:30 +01:00
parent 3479fa1228
commit 05349ddb5c
Signed by: konrad
GPG Key ID: F40E70337AB24C9B
6 changed files with 47 additions and 36 deletions

View File

@ -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 `<provider key>`. 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": "<provider key>",
"children": [
{
"key": "name",

View File

@ -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 != "" {

View File

@ -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"

View File

@ -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) {

View File

@ -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,

View File

@ -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),