Auth: Add all settings to Azure AD SSO config UI (#83618)

* Add all settings to AzureAD UI * prettify * Fixes * Load extra keys with type assertion
grafana · Mar 4, 2024 · 07e2622 · 07e2622
1 parent fa44aeb
commit 07e2622
Show file tree

Hide file tree

Showing 10 changed files with 97 additions and 13 deletions.
diff --git a/pkg/login/social/connectors/azuread_oauth.go b/pkg/login/social/connectors/azuread_oauth.go
@@ -31,7 +31,10 @@ import (
 const forceUseGraphAPIKey = "force_use_graph_api" // #nosec G101 not a hardcoded credential
 
 var (
-	ExtraAzureADSettingKeys = []string{forceUseGraphAPIKey, allowedOrganizationsKey}
+	ExtraAzureADSettingKeys = map[string]ExtraKeyInfo{
+		forceUseGraphAPIKey:     {Type: Bool, DefaultValue: false},
+		allowedOrganizationsKey: {Type: String},
+	}
 	errAzureADMissingGroups = &SocialError{"either the user does not have any group membership or the groups claim is missing from the token."}
 )
 
@@ -80,7 +83,7 @@ func NewAzureADProvider(info *social.OAuthInfo, cfg *setting.Cfg, ssoSettings ss
 		SocialBase:           newSocialBase(social.AzureADProviderName, info, features, cfg),
 		cache:                cache,
 		allowedOrganizations: util.SplitString(info.Extra[allowedOrganizationsKey]),
-		forceUseGraphAPI:     MustBool(info.Extra[forceUseGraphAPIKey], false),
+		forceUseGraphAPI:     MustBool(info.Extra[forceUseGraphAPIKey], ExtraAzureADSettingKeys[forceUseGraphAPIKey].DefaultValue.(bool)),
 	}
 
 	if info.UseRefreshToken {
@@ -200,6 +203,8 @@ func (s *SocialAzureAD) Validate(ctx context.Context, settings ssoModels.SSOSett
 
 	return validation.Validate(info, requester,
 		validateAllowedGroups,
+		// FIXME: uncomment this after the Terraform provider is updated
+		//validation.MustBeEmptyValidator(info.ApiUrl, "API URL"),
 		validation.RequiredUrlValidator(info.AuthUrl, "Auth URL"),
 		validation.RequiredUrlValidator(info.TokenUrl, "Token URL"))
 }

diff --git a/pkg/login/social/connectors/common.go b/pkg/login/social/connectors/common.go
@@ -18,6 +18,18 @@ import (
 	"github.com/grafana/grafana/pkg/util"
 )
 
+type ExtraFieldType int
+
+const (
+	String ExtraFieldType = iota
+	Bool
+)
+
+type ExtraKeyInfo struct {
+	Type         ExtraFieldType
+	DefaultValue any
+}
+
 const (
 	// consider moving this to OAuthInfo
 	teamIdsKey = "team_ids"

diff --git a/pkg/login/social/connectors/generic_oauth.go b/pkg/login/social/connectors/generic_oauth.go
@@ -28,7 +28,13 @@ const (
 	idTokenAttributeNameKey = "id_token_attribute_name" // #nosec G101 not a hardcoded credential
 )
 
-var ExtraGenericOAuthSettingKeys = []string{nameAttributePathKey, loginAttributePathKey, idTokenAttributeNameKey, teamIdsKey, allowedOrganizationsKey}
+var ExtraGenericOAuthSettingKeys = map[string]ExtraKeyInfo{
+	nameAttributePathKey:    {Type: String},
+	loginAttributePathKey:   {Type: String},
+	idTokenAttributeNameKey: {Type: String},
+	teamIdsKey:              {Type: String},
+	allowedOrganizationsKey: {Type: String},
+}
 
 var _ social.SocialConnector = (*SocialGenericOAuth)(nil)
 var _ ssosettings.Reloadable = (*SocialGenericOAuth)(nil)

diff --git a/pkg/login/social/connectors/github_oauth.go b/pkg/login/social/connectors/github_oauth.go
@@ -24,7 +24,10 @@ import (
 	"github.com/grafana/grafana/pkg/util/errutil"
 )
 
-var ExtraGithubSettingKeys = []string{allowedOrganizationsKey, teamIdsKey}
+var ExtraGithubSettingKeys = map[string]ExtraKeyInfo{
+	allowedOrganizationsKey: {Type: String},
+	teamIdsKey:              {Type: String},
+}
 
 var _ social.SocialConnector = (*SocialGithub)(nil)
 var _ ssosettings.Reloadable = (*SocialGithub)(nil)

diff --git a/pkg/login/social/connectors/google_oauth.go b/pkg/login/social/connectors/google_oauth.go
@@ -27,9 +27,12 @@ const (
 	validateHDKey           = "validate_hd"
 )
 
+var ExtraGoogleSettingKeys = map[string]ExtraKeyInfo{
+	validateHDKey: {Type: Bool, DefaultValue: true},
+}
+
 var _ social.SocialConnector = (*SocialGoogle)(nil)
 var _ ssosettings.Reloadable = (*SocialGoogle)(nil)
-var ExtraGoogleSettingKeys = []string{validateHDKey}
 
 type SocialGoogle struct {
 	*SocialBase

diff --git a/pkg/login/social/connectors/grafana_com_oauth.go b/pkg/login/social/connectors/grafana_com_oauth.go
@@ -20,7 +20,9 @@ import (
 	"github.com/grafana/grafana/pkg/util"
 )
 
-var ExtraGrafanaComSettingKeys = []string{allowedOrganizationsKey}
+var ExtraGrafanaComSettingKeys = map[string]ExtraKeyInfo{
+	allowedOrganizationsKey: {Type: String, DefaultValue: ""},
+}
 
 var _ social.SocialConnector = (*SocialGrafanaCom)(nil)
 var _ ssosettings.Reloadable = (*SocialGrafanaCom)(nil)

diff --git a/pkg/services/ssosettings/strategies/oauth_strategy.go b/pkg/services/ssosettings/strategies/oauth_strategy.go
@@ -15,7 +15,7 @@ type OAuthStrategy struct {
 	settingsByProvider map[string]map[string]any
 }
 
-var extraKeysByProvider = map[string][]string{
+var extraKeysByProvider = map[string]map[string]connectors.ExtraKeyInfo{
 	social.AzureADProviderName:      connectors.ExtraAzureADSettingKeys,
 	social.GenericOAuthProviderName: connectors.ExtraGenericOAuthSettingKeys,
 	social.GitHubProviderName:       connectors.ExtraGithubSettingKeys,
@@ -104,9 +104,18 @@ func (s *OAuthStrategy) loadSettingsForProvider(provider string) map[string]any
 		"signout_redirect_url":       section.Key("signout_redirect_url").Value(),
 	}
 
-	extraFields := extraKeysByProvider[provider]
-	for _, key := range extraFields {
-		result[key] = section.Key(key).Value()
+	extraKeys := extraKeysByProvider[provider]
+	for key, keyInfo := range extraKeys {
+		switch keyInfo.Type {
+		case connectors.Bool:
+			result[key] = section.Key(key).MustBool(keyInfo.DefaultValue.(bool))
+		default:
+			if _, ok := keyInfo.DefaultValue.(string); !ok {
+				result[key] = section.Key(key).Value()
+			} else {
+				result[key] = section.Key(key).MustString(keyInfo.DefaultValue.(string))
+			}
+		}
 	}
 
 	return result

diff --git a/pkg/services/ssosettings/strategies/oauth_strategy_test.go b/pkg/services/ssosettings/strategies/oauth_strategy_test.go
@@ -147,7 +147,7 @@ func TestGetProviderConfig_ExtraFields(t *testing.T) {
 		result, err := strategy.GetProviderConfig(context.Background(), social.AzureADProviderName)
 		require.NoError(t, err)
 
-		require.Equal(t, "true", result["force_use_graph_api"])
+		require.Equal(t, true, result["force_use_graph_api"])
 		require.Equal(t, "org1, org2", result["allowed_organizations"])
 	})
 
@@ -181,7 +181,7 @@ func TestGetProviderConfig_ExtraFields(t *testing.T) {
 		result, err := strategy.GetProviderConfig(context.Background(), social.GoogleProviderName)
 		require.NoError(t, err)
 
-		require.Equal(t, "true", result["validate_hd"])
+		require.Equal(t, true, result["validate_hd"])
 	})
 }
 

diff --git a/public/app/features/auth-config/fields.tsx b/public/app/features/auth-config/fields.tsx
@@ -14,7 +14,6 @@ export const fields: Record<SSOProvider['provider'], Array<keyof SSOProvider['se
   github: ['name', 'clientId', 'clientSecret', 'teamIds', 'allowedOrganizations'],
   google: ['name', 'clientId', 'clientSecret', 'allowedDomains'],
   gitlab: ['name', 'clientId', 'clientSecret', 'allowedOrganizations', 'teamIds'],
-  azuread: ['name', 'clientId', 'clientSecret', 'authUrl', 'tokenUrl', 'scopes', 'allowedGroups', 'allowedDomains'],
   okta: [
     'name',
     'clientId',
@@ -39,6 +38,44 @@ type Section = Record<
 >;
 
 export const sectionFields: Section = {
+  azuread: [
+    {
+      name: 'General settings',
+      id: 'general',
+      fields: [
+        'name',
+        'clientId',
+        'clientSecret',
+        'scopes',
+        'authUrl',
+        'tokenUrl',
+        'allowSignUp',
+        'autoLogin',
+        'signoutRedirectUrl',
+      ],
+    },
+    {
+      name: 'User mapping',
+      id: 'user',
+      fields: ['roleAttributePath', 'roleAttributeStrict', 'allowAssignGrafanaAdmin', 'skipOrgRoleSync'],
+    },
+    {
+      name: 'Extra security measures',
+      id: 'extra',
+      fields: [
+        'allowedOrganizations',
+        'allowedDomains',
+        'allowedGroups',
+        'forceUseGraphApi',
+        'usePkce',
+        'useRefreshToken',
+        'tlsSkipVerifyInsecure',
+        'tlsClientCert',
+        'tlsClientKey',
+        'tlsClientCa',
+      ],
+    },
+  ],
   generic_oauth: [
     {
       name: 'General settings',
@@ -320,6 +357,11 @@ export function fieldMap(provider: string): Record<string, FieldData> {
       label: 'Define allowed teams ids',
       type: 'switch',
     },
+    forceUseGraphApi: {
+      label: 'Force use Graph API',
+      description: "If enabled, Grafana will fetch the users' groups using the Microsoft Graph API.",
+      type: 'checkbox',
+    },
     usePkce: {
       label: 'Use PKCE',
       description: (

diff --git a/public/app/features/auth-config/types.ts b/public/app/features/auth-config/types.ts
@@ -53,6 +53,8 @@ export type SSOProviderSettingsBase = {
   defineAllowedTeamsIds?: boolean;
   configureTLS?: boolean;
   tlsSkipVerifyInsecure?: boolean;
+  // For Azure AD
+  forceUseGraphApi?: boolean;
 };
 
 // SSO data received from the API and sent to it