Refactor IdP Config Structure (#879)

management/cmd/management.go

@@ -394,12 +394,6 @@ func loadMgmtConfig(mgmtConfigPath string) (*server.Config, error) {
 		}
 		log.Infof("loaded OIDC configuration from the provided IDP configuration endpoint: %s", oidcEndpoint)
 
-		log.Infof("configuring IdpManagerConfig.OIDCConfig.Issuer with a new value %s,", oidcConfig.Issuer)
-		config.IdpManagerConfig.OIDCConfig.Issuer = strings.TrimRight(oidcConfig.Issuer, "/")
-
-		log.Infof("configuring IdpManagerConfig.OIDCConfig.TokenEndpoint with a new value %s,", oidcConfig.TokenEndpoint)
-		config.IdpManagerConfig.OIDCConfig.TokenEndpoint = oidcConfig.TokenEndpoint
-
 		log.Infof("overriding HttpConfig.AuthIssuer with a new value %s, previously configured value: %s",
 			oidcConfig.Issuer, config.HttpConfig.AuthIssuer)
 		config.HttpConfig.AuthIssuer = oidcConfig.Issuer

management/server/idp/auth0.go

@@ -32,10 +32,10 @@ type Auth0Manager struct {
 // Auth0ClientConfig auth0 manager client configurations
 type Auth0ClientConfig struct {
 	Audience     string
-	AuthIssuer   string `json:"-"`
+	AuthIssuer   string
 	ClientID     string
 	ClientSecret string
-	GrantType    string `json:"-"`
+	GrantType    string
 }
 
 // auth0JWTRequest payload struct to request a JWT Token
@@ -110,23 +110,22 @@ type auth0Profile struct {
 }
 
 // NewAuth0Manager creates a new instance of the Auth0Manager
-func NewAuth0Manager(oidcConfig OIDCConfig, config Auth0ClientConfig,
-	appMetrics telemetry.AppMetrics) (*Auth0Manager, error) {
-
+func NewAuth0Manager(config Auth0ClientConfig, appMetrics telemetry.AppMetrics) (*Auth0Manager, error) {
 	httpTransport := http.DefaultTransport.(*http.Transport).Clone()
 	httpTransport.MaxIdleConns = 5
 
 	httpClient := &http.Client{
 		Timeout:   10 * time.Second,
 		Transport: httpTransport,
 	}
-
 	helper := JsonParser{}
-	config.AuthIssuer = oidcConfig.Issuer
-	config.GrantType = "client_credentials"
+
+	if config.AuthIssuer == "" {
+		return nil, fmt.Errorf("auth0 IdP configuration is incomplete, AuthIssuer is missing")
+	}
 
 	if config.ClientID == "" {
-		return nil, fmt.Errorf("auth0 IdP configuration is incomplete, clientID is missing")
+		return nil, fmt.Errorf("auth0 IdP configuration is incomplete, ClientID is missing")
 	}
 
 	if config.ClientSecret == "" {
@@ -137,6 +136,10 @@ func NewAuth0Manager(oidcConfig OIDCConfig, config Auth0ClientConfig,
 		return nil, fmt.Errorf("auth0 IdP configuration is incomplete, Audience is missing")
 	}
 
+	if config.GrantType == "" {
+		return nil, fmt.Errorf("auth0 IdP configuration is incomplete, GrantType is missing")
+	}
+
 	credentials := &Auth0Credentials{
 		clientConfig: config,
 		httpClient:   httpClient,

management/server/idp/auth0_test.go

@@ -461,7 +461,7 @@ func TestNewAuth0Manager(t *testing.T) {
 
 	for _, testCase := range []test{testCase1, testCase2} {
 		t.Run(testCase.name, func(t *testing.T) {
-			_, err := NewAuth0Manager(OIDCConfig{}, testCase.inputConfig, &telemetry.MockAppMetrics{})
+			_, err := NewAuth0Manager(testCase.inputConfig, &telemetry.MockAppMetrics{})
 			testCase.assertErrFunc(t, err, testCase.assertErrFuncMessage)
 		})
 	}

management/server/idp/azure.go

@@ -37,13 +37,12 @@ type AzureManager struct {
 
 // AzureClientConfig azure manager client configurations.
 type AzureClientConfig struct {
-	ClientID     string
-	ClientSecret string
-	ObjectID     string
-
-	GraphAPIEndpoint string `json:"-"`
-	TokenEndpoint    string `json:"-"`
-	GrantType        string `json:"-"`
+	ClientID         string
+	ClientSecret     string
+	ObjectID         string
+	GraphAPIEndpoint string
+	TokenEndpoint    string
+	GrantType        string
 }
 
 // AzureCredentials azure authentication information.
@@ -75,20 +74,15 @@ type azureExtension struct {
 }
 
 // NewAzureManager creates a new instance of the AzureManager.
-func NewAzureManager(oidcConfig OIDCConfig, config AzureClientConfig,
-	appMetrics telemetry.AppMetrics) (*AzureManager, error) {
+func NewAzureManager(config AzureClientConfig, appMetrics telemetry.AppMetrics) (*AzureManager, error) {
 	httpTransport := http.DefaultTransport.(*http.Transport).Clone()
 	httpTransport.MaxIdleConns = 5
 
 	httpClient := &http.Client{
 		Timeout:   10 * time.Second,
 		Transport: httpTransport,
 	}
-
 	helper := JsonParser{}
-	config.TokenEndpoint = oidcConfig.TokenEndpoint
-	config.GraphAPIEndpoint = "https://graph.microsoft.com"
-	config.GrantType = "client_credentials"
 
 	if config.ClientID == "" {
 		return nil, fmt.Errorf("azure IdP configuration is incomplete, clientID is missing")
@@ -98,10 +92,22 @@ func NewAzureManager(oidcConfig OIDCConfig, config AzureClientConfig,
 		return nil, fmt.Errorf("azure IdP configuration is incomplete, ClientSecret is missing")
 	}
 
+	if config.TokenEndpoint == "" {
+		return nil, fmt.Errorf("azure IdP configuration is incomplete, TokenEndpoint is missing")
+	}
+
+	if config.GraphAPIEndpoint == "" {
+		return nil, fmt.Errorf("azure IdP configuration is incomplete, GraphAPIEndpoint is missing")
+	}
+
 	if config.ObjectID == "" {
 		return nil, fmt.Errorf("azure IdP configuration is incomplete, ObjectID is missing")
 	}
 
+	if config.GrantType == "" {
+		return nil, fmt.Errorf("azure IdP configuration is incomplete, GrantType is missing")
+	}
+
 	credentials := &AzureCredentials{
 		clientConfig: config,
 		httpClient:   httpClient,

management/server/idp/idp.go

@@ -19,17 +19,23 @@ type Manager interface {
 	GetUserByEmail(email string) ([]*UserData, error)
 }
 
-// OIDCConfig specifies configuration for OpenID Connect provider
-// These configurations are automatically loaded from the OIDC endpoint
-type OIDCConfig struct {
+// ClientConfig defines common client configuration for all IdP manager
+type ClientConfig struct {
 	Issuer        string
 	TokenEndpoint string
+	ClientID      string
+	ClientSecret  string
+	GrantType     string
 }
 
+// ExtraConfig stores IdP specific config that are unique to individual IdPs
+type ExtraConfig map[string]string
+
 // Config an idp configuration struct to be loaded from management server's config file
 type Config struct {
 	ManagerType               string
-	OIDCConfig                OIDCConfig `json:"-"`
+	ClientConfig              *ClientConfig
+	ExtraConfig               ExtraConfig
 	Auth0ClientCredentials    Auth0ClientConfig
 	AzureClientCredentials    AzureClientConfig
 	KeycloakClientCredentials KeycloakClientConfig
@@ -82,13 +88,58 @@ func NewManager(config Config, appMetrics telemetry.AppMetrics) (Manager, error)
 	case "none", "":
 		return nil, nil
 	case "auth0":
-		return NewAuth0Manager(config.OIDCConfig, config.Auth0ClientCredentials, appMetrics)
+		auth0ClientConfig := config.Auth0ClientCredentials
+		if config.ClientConfig != nil {
+			auth0ClientConfig = Auth0ClientConfig{
+				Audience:     config.ExtraConfig["Audience"],
+				AuthIssuer:   config.ClientConfig.Issuer,
+				ClientID:     config.ClientConfig.ClientID,
+				ClientSecret: config.ClientConfig.ClientSecret,
+				GrantType:    config.ClientConfig.GrantType,
+			}
+		}
+
+		return NewAuth0Manager(auth0ClientConfig, appMetrics)
 	case "azure":
-		return NewAzureManager(config.OIDCConfig, config.AzureClientCredentials, appMetrics)
+		azureClientConfig := config.AzureClientCredentials
+		if config.ClientConfig != nil {
+			azureClientConfig = AzureClientConfig{
+				ClientID:         config.ClientConfig.ClientID,
+				ClientSecret:     config.ClientConfig.ClientSecret,
+				GrantType:        config.ClientConfig.GrantType,
+				TokenEndpoint:    config.ClientConfig.TokenEndpoint,
+				ObjectID:         config.ExtraConfig["ObjectID"],
+				GraphAPIEndpoint: config.ExtraConfig["GraphAPIEndpoint"],
+			}
+		}
+
+		return NewAzureManager(azureClientConfig, appMetrics)
 	case "keycloak":
-		return NewKeycloakManager(config.OIDCConfig, config.KeycloakClientCredentials, appMetrics)
+		keycloakClientConfig := config.KeycloakClientCredentials
+		if config.ClientConfig != nil {
+			keycloakClientConfig = KeycloakClientConfig{
+				ClientID:      config.ClientConfig.ClientID,
+				ClientSecret:  config.ClientConfig.ClientSecret,
+				GrantType:     config.ClientConfig.GrantType,
+				TokenEndpoint: config.ClientConfig.TokenEndpoint,
+				AdminEndpoint: config.ExtraConfig["AdminEndpoint"],
+			}
+		}
+
+		return NewKeycloakManager(keycloakClientConfig, appMetrics)
 	case "zitadel":
-		return NewZitadelManager(config.OIDCConfig, config.ZitadelClientCredentials, appMetrics)
+		zitadelClientConfig := config.ZitadelClientCredentials
+		if config.ClientConfig != nil {
+			zitadelClientConfig = ZitadelClientConfig{
+				ClientID:           config.ClientConfig.ClientID,
+				ClientSecret:       config.ClientConfig.ClientSecret,
+				GrantType:          config.ClientConfig.GrantType,
+				TokenEndpoint:      config.ClientConfig.TokenEndpoint,
+				ManagementEndpoint: config.ExtraConfig["ManagementEndpoint"],
+			}
+		}
+
+		return NewZitadelManager(zitadelClientConfig, appMetrics)
 	default:
 		return nil, fmt.Errorf("invalid manager type: %s", config.ManagerType)
 	}

management/server/idp/keycloak.go

@@ -37,8 +37,8 @@ type KeycloakClientConfig struct {
 	ClientID      string
 	ClientSecret  string
 	AdminEndpoint string
-	TokenEndpoint string `json:"-"`
-	GrantType     string `json:"-"`
+	TokenEndpoint string
+	GrantType     string
 }
 
 // KeycloakCredentials keycloak authentication information.
@@ -82,19 +82,15 @@ type keycloakProfile struct {
 }
 
 // NewKeycloakManager creates a new instance of the KeycloakManager.
-func NewKeycloakManager(oidcConfig OIDCConfig, config KeycloakClientConfig,
-	appMetrics telemetry.AppMetrics) (*KeycloakManager, error) {
+func NewKeycloakManager(config KeycloakClientConfig, appMetrics telemetry.AppMetrics) (*KeycloakManager, error) {
 	httpTransport := http.DefaultTransport.(*http.Transport).Clone()
 	httpTransport.MaxIdleConns = 5
 
 	httpClient := &http.Client{
 		Timeout:   10 * time.Second,
 		Transport: httpTransport,
 	}
-
 	helper := JsonParser{}
-	config.TokenEndpoint = oidcConfig.TokenEndpoint
-	config.GrantType = "client_credentials"
 
 	if config.ClientID == "" {
 		return nil, fmt.Errorf("keycloak IdP configuration is incomplete, clientID is missing")
@@ -104,10 +100,18 @@ func NewKeycloakManager(oidcConfig OIDCConfig, config KeycloakClientConfig,
 		return nil, fmt.Errorf("keycloak IdP configuration is incomplete, ClientSecret is missing")
 	}
 
+	if config.TokenEndpoint == "" {
+		return nil, fmt.Errorf("keycloak IdP configuration is incomplete, TokenEndpoint is missing")
+	}
+
 	if config.AdminEndpoint == "" {
 		return nil, fmt.Errorf("keycloak IdP configuration is incomplete, AdminEndpoint is missing")
 	}
 
+	if config.GrantType == "" {
+		return nil, fmt.Errorf("keycloak IdP configuration is incomplete, GrantType is missing")
+	}
+
 	credentials := &KeycloakCredentials{
 		clientConfig: config,
 		httpClient:   httpClient,

management/server/idp/keycloak_test.go

@@ -56,9 +56,29 @@ func TestNewKeycloakManager(t *testing.T) {
 		assertErrFuncMessage: "should return error when field empty",
 	}
 
-	for _, testCase := range []test{testCase1, testCase2, testCase3} {
+	testCase4Config := defaultTestConfig
+	testCase4Config.TokenEndpoint = ""
+
+	testCase4 := test{
+		name:                 "Missing TokenEndpoint Configuration",
+		inputConfig:          testCase3Config,
+		assertErrFunc:        require.Error,
+		assertErrFuncMessage: "should return error when field empty",
+	}
+
+	testCase5Config := defaultTestConfig
+	testCase5Config.GrantType = ""
+
+	testCase5 := test{
+		name:                 "Missing GrantType Configuration",
+		inputConfig:          testCase3Config,
+		assertErrFunc:        require.Error,
+		assertErrFuncMessage: "should return error when field empty",
+	}
+
+	for _, testCase := range []test{testCase1, testCase2, testCase3, testCase4, testCase5} {
 		t.Run(testCase.name, func(t *testing.T) {
-			_, err := NewKeycloakManager(OIDCConfig{}, testCase.inputConfig, &telemetry.MockAppMetrics{})
+			_, err := NewKeycloakManager(testCase.inputConfig, &telemetry.MockAppMetrics{})
 			testCase.assertErrFunc(t, err, testCase.assertErrFuncMessage)
 		})
 	}

management/server/idp/zitadel.go

@@ -30,9 +30,9 @@ type ZitadelManager struct {
 type ZitadelClientConfig struct {
 	ClientID           string
 	ClientSecret       string
-	GrantType          string `json:"-"`
-	TokenEndpoint      string `json:"-"`
-	ManagementEndpoint string `json:"-"`
+	GrantType          string
+	TokenEndpoint      string
+	ManagementEndpoint string
 }
 
 // ZitadelCredentials zitadel authentication information.
@@ -85,20 +85,15 @@ type zitadelProfile struct {
 }
 
 // NewZitadelManager creates a new instance of the ZitadelManager.
-func NewZitadelManager(oidcConfig OIDCConfig, config ZitadelClientConfig,
-	appMetrics telemetry.AppMetrics) (*ZitadelManager, error) {
+func NewZitadelManager(config ZitadelClientConfig, appMetrics telemetry.AppMetrics) (*ZitadelManager, error) {
 	httpTransport := http.DefaultTransport.(*http.Transport).Clone()
 	httpTransport.MaxIdleConns = 5
 
 	httpClient := &http.Client{
 		Timeout:   10 * time.Second,
 		Transport: httpTransport,
 	}
-
 	helper := JsonParser{}
-	config.TokenEndpoint = oidcConfig.TokenEndpoint
-	config.ManagementEndpoint = fmt.Sprintf("%s/management/v1", oidcConfig.Issuer)
-	config.GrantType = "client_credentials"
 
 	if config.ClientID == "" {
 		return nil, fmt.Errorf("zitadel IdP configuration is incomplete, clientID is missing")
@@ -108,6 +103,18 @@ func NewZitadelManager(oidcConfig OIDCConfig, config ZitadelClientConfig,
 		return nil, fmt.Errorf("zitadel IdP configuration is incomplete, ClientSecret is missing")
 	}
 
+	if config.TokenEndpoint == "" {
+		return nil, fmt.Errorf("zitadel IdP configuration is incomplete, TokenEndpoint is missing")
+	}
+
+	if config.ManagementEndpoint == "" {
+		return nil, fmt.Errorf("zitadel IdP configuration is incomplete, ManagementEndpoint is missing")
+	}
+
+	if config.GrantType == "" {
+		return nil, fmt.Errorf("zitadel IdP configuration is incomplete, GrantType is missing")
+	}
+
 	credentials := &ZitadelCredentials{
 		clientConfig: config,
 		httpClient:   httpClient,

management/server/idp/zitadel_test.go

@@ -57,7 +57,7 @@ func TestNewZitadelManager(t *testing.T) {
 
 	for _, testCase := range []test{testCase1, testCase2, testCase3} {
 		t.Run(testCase.name, func(t *testing.T) {
-			_, err := NewZitadelManager(OIDCConfig{}, testCase.inputConfig, &telemetry.MockAppMetrics{})
+			_, err := NewZitadelManager(testCase.inputConfig, &telemetry.MockAppMetrics{})
 			testCase.assertErrFunc(t, err, testCase.assertErrFuncMessage)
 		})
 	}