From 07e26226b7708be83d6c3c7ef21b48e20af5a59b Mon Sep 17 00:00:00 2001 From: Misi Date: Mon, 4 Mar 2024 11:55:59 +0100 Subject: [PATCH] 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 --- pkg/login/social/connectors/azuread_oauth.go | 9 +++- pkg/login/social/connectors/common.go | 12 +++++ pkg/login/social/connectors/generic_oauth.go | 8 +++- pkg/login/social/connectors/github_oauth.go | 5 ++- pkg/login/social/connectors/google_oauth.go | 5 ++- .../social/connectors/grafana_com_oauth.go | 4 +- .../ssosettings/strategies/oauth_strategy.go | 17 +++++-- .../strategies/oauth_strategy_test.go | 4 +- public/app/features/auth-config/fields.tsx | 44 ++++++++++++++++++- public/app/features/auth-config/types.ts | 2 + 10 files changed, 97 insertions(+), 13 deletions(-) diff --git a/pkg/login/social/connectors/azuread_oauth.go b/pkg/login/social/connectors/azuread_oauth.go index d40999e4cd5..05f23c706f7 100644 --- 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 index 96bf84020ba..6dcc051a319 100644 --- 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 index eec06750da0..885736e7aea 100644 --- 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 index 007e576a8cf..46a82de22a8 100644 --- 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 index 84c6d9f96f4..955c70ff9c4 100644 --- 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 index c5961cb07da..694400aea47 100644 --- 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 index 7f67ad9c8e3..578e80e31f7 100644 --- 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 index 3cf7031d7dc..2d5a91bb548 100644 --- 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 index 5a095e4f954..38c28e519c8 100644 --- a/public/app/features/auth-config/fields.tsx +++ b/public/app/features/auth-config/fields.tsx @@ -14,7 +14,6 @@ export const fields: 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 { 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 index db9040efd4c..02d5d1fe3ba 100644 --- 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