diff --git a/pkg/login/social/connectors/azuread_oauth.go b/pkg/login/social/connectors/azuread_oauth.go index 98f21607146..712d0f145ba 100644 --- a/pkg/login/social/connectors/azuread_oauth.go +++ b/pkg/login/social/connectors/azuread_oauth.go @@ -12,6 +12,7 @@ import ( jose "github.com/go-jose/go-jose/v3" "github.com/go-jose/go-jose/v3/jwt" + "github.com/google/uuid" "golang.org/x/oauth2" "github.com/grafana/grafana/pkg/infra/remotecache" @@ -195,7 +196,12 @@ func (s *SocialAzureAD) Validate(ctx context.Context, settings ssoModels.SSOSett return err } - // add specific validation rules for AzureAD + for _, groupId := range info.AllowedGroups { + _, err := uuid.Parse(groupId) + if err != nil { + return ssosettings.ErrInvalidOAuthConfig("One or more of the Allowed groups are not in the correct format. Allowed groups should be a list of Object Ids.") + } + } return nil } diff --git a/pkg/login/social/connectors/azuread_oauth_test.go b/pkg/login/social/connectors/azuread_oauth_test.go index aed48e98d9b..e59803135b2 100644 --- a/pkg/login/social/connectors/azuread_oauth_test.go +++ b/pkg/login/social/connectors/azuread_oauth_test.go @@ -19,6 +19,7 @@ import ( "github.com/grafana/grafana/pkg/infra/remotecache" "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/ssosettings" ssoModels "github.com/grafana/grafana/pkg/services/ssosettings/models" "github.com/grafana/grafana/pkg/services/ssosettings/ssosettingstests" "github.com/grafana/grafana/pkg/setting" @@ -991,18 +992,18 @@ func TestSocialAzureAD_InitializeExtraFields(t *testing.T) { func TestSocialAzureAD_Validate(t *testing.T) { testCases := []struct { - name string - settings ssoModels.SSOSettings - expectError bool + name string + settings ssoModels.SSOSettings + wantErr error }{ { name: "SSOSettings is valid", settings: ssoModels.SSOSettings{ Settings: map[string]any{ - "client_id": "client-id", + "client_id": "client-id", + "allowed_groups": "0bb9c9cc-4945-418f-9b6a-c1d3b81141b0, 6034d328-0e6a-4240-8d03-cb9f2c1f16e4", }, }, - expectError: false, }, { name: "fails if settings map contains an invalid field", @@ -1012,7 +1013,7 @@ func TestSocialAzureAD_Validate(t *testing.T) { "invalid_field": []int{1, 2, 3}, }, }, - expectError: true, + wantErr: ssosettings.ErrInvalidSettings, }, { name: "fails if client id is empty", @@ -1021,14 +1022,35 @@ func TestSocialAzureAD_Validate(t *testing.T) { "client_id": "", }, }, - expectError: true, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, }, { name: "fails if client id does not exist", settings: ssoModels.SSOSettings{ Settings: map[string]any{}, }, - expectError: true, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if allowed groups are not uuids", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "allowed_groups": "abc, def", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if both allow assign grafana admin and skip org role sync are enabled", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "allow_assign_grafana_admin": "true", + "skip_org_role_sync": "true", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, }, } @@ -1037,11 +1059,11 @@ func TestSocialAzureAD_Validate(t *testing.T) { s := NewAzureADProvider(&social.OAuthInfo{}, &setting.Cfg{}, &ssosettingstests.MockService{}, featuremgmt.WithFeatures(), nil) err := s.Validate(context.Background(), tc.settings) - if tc.expectError { - require.Error(t, err) - } else { - require.NoError(t, err) + if tc.wantErr != nil { + require.ErrorIs(t, err, tc.wantErr) + return } + require.NoError(t, err) }) } } diff --git a/pkg/login/social/connectors/generic_oauth.go b/pkg/login/social/connectors/generic_oauth.go index 87dce610fc7..3e1c3cecb38 100644 --- a/pkg/login/social/connectors/generic_oauth.go +++ b/pkg/login/social/connectors/generic_oauth.go @@ -78,7 +78,13 @@ func (s *SocialGenericOAuth) Validate(ctx context.Context, settings ssoModels.SS return err } - // add specific validation rules for Generic OAuth + if info.Extra[teamIdsKey] != "" && (info.TeamIdsAttributePath == "" || info.TeamsUrl == "") { + return ssosettings.ErrInvalidOAuthConfig("If Team Ids are configured then Team Ids attribute path and Teams URL must be configured.") + } + + if info.AllowedGroups != nil && len(info.AllowedGroups) > 0 && info.GroupsAttributePath == "" { + return ssosettings.ErrInvalidOAuthConfig("If Allowed groups are configured then Groups attribute path must be configured.") + } return nil } diff --git a/pkg/login/social/connectors/generic_oauth_test.go b/pkg/login/social/connectors/generic_oauth_test.go index bf6dc50c366..edcc5eb76e3 100644 --- a/pkg/login/social/connectors/generic_oauth_test.go +++ b/pkg/login/social/connectors/generic_oauth_test.go @@ -15,6 +15,7 @@ import ( "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/org" + "github.com/grafana/grafana/pkg/services/ssosettings" ssoModels "github.com/grafana/grafana/pkg/services/ssosettings/models" "github.com/grafana/grafana/pkg/services/ssosettings/ssosettingstests" "github.com/grafana/grafana/pkg/setting" @@ -919,9 +920,9 @@ func TestSocialGenericOAuth_InitializeExtraFields(t *testing.T) { func TestSocialGenericOAuth_Validate(t *testing.T) { testCases := []struct { - name string - settings ssoModels.SSOSettings - expectError bool + name string + settings ssoModels.SSOSettings + wantErr error }{ { name: "SSOSettings is valid", @@ -930,7 +931,6 @@ func TestSocialGenericOAuth_Validate(t *testing.T) { "client_id": "client-id", }, }, - expectError: false, }, { name: "fails if settings map contains an invalid field", @@ -940,7 +940,7 @@ func TestSocialGenericOAuth_Validate(t *testing.T) { "invalid_field": []int{1, 2, 3}, }, }, - expectError: true, + wantErr: ssosettings.ErrInvalidSettings, }, { name: "fails if client id is empty", @@ -949,14 +949,25 @@ func TestSocialGenericOAuth_Validate(t *testing.T) { "client_id": "", }, }, - expectError: true, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, }, { name: "fails if client id does not exist", settings: ssoModels.SSOSettings{ Settings: map[string]any{}, }, - expectError: true, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if both allow assign grafana admin and skip org role sync are enabled", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "allow_assign_grafana_admin": "true", + "skip_org_role_sync": "true", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, }, } @@ -965,11 +976,11 @@ func TestSocialGenericOAuth_Validate(t *testing.T) { s := NewGenericOAuthProvider(&social.OAuthInfo{}, &setting.Cfg{}, &ssosettingstests.MockService{}, featuremgmt.WithFeatures()) err := s.Validate(context.Background(), tc.settings) - if tc.expectError { - require.Error(t, err) - } else { - require.NoError(t, err) + if tc.wantErr != nil { + require.ErrorIs(t, err, tc.wantErr) + return } + require.NoError(t, err) }) } } diff --git a/pkg/login/social/connectors/github_oauth.go b/pkg/login/social/connectors/github_oauth.go index 4a89bac5eed..de7fc6def39 100644 --- a/pkg/login/social/connectors/github_oauth.go +++ b/pkg/login/social/connectors/github_oauth.go @@ -89,8 +89,7 @@ func (s *SocialGithub) Validate(ctx context.Context, settings ssoModels.SSOSetti teamIds := mustInts(teamIdsSplitted) if len(teamIdsSplitted) != len(teamIds) { - s.log.Warn("Failed to parse team ids. Team ids must be a list of numbers.", "teamIds", teamIdsSplitted) - return ssosettings.ErrInvalidSettings.Errorf("Failed to parse team ids. Team ids must be a list of numbers.") + return ssosettings.ErrInvalidOAuthConfig("Failed to parse Team Ids. Team Ids must be a list of numbers.") } return nil diff --git a/pkg/login/social/connectors/github_oauth_test.go b/pkg/login/social/connectors/github_oauth_test.go index 974969518b9..f0e09a85e07 100644 --- a/pkg/login/social/connectors/github_oauth_test.go +++ b/pkg/login/social/connectors/github_oauth_test.go @@ -13,6 +13,7 @@ import ( "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/ssosettings" ssoModels "github.com/grafana/grafana/pkg/services/ssosettings/models" "github.com/grafana/grafana/pkg/services/ssosettings/ssosettingstests" "github.com/grafana/grafana/pkg/setting" @@ -345,9 +346,9 @@ func TestSocialGitHub_InitializeExtraFields(t *testing.T) { func TestSocialGitHub_Validate(t *testing.T) { testCases := []struct { - name string - settings ssoModels.SSOSettings - expectError bool + name string + settings ssoModels.SSOSettings + wantErr error }{ { name: "SSOSettings is valid", @@ -356,7 +357,6 @@ func TestSocialGitHub_Validate(t *testing.T) { "client_id": "client-id", }, }, - expectError: false, }, { name: "fails if settings map contains an invalid field", @@ -366,7 +366,7 @@ func TestSocialGitHub_Validate(t *testing.T) { "invalid_field": []int{1, 2, 3}, }, }, - expectError: true, + wantErr: ssosettings.ErrInvalidSettings, }, { name: "fails if client id is empty", @@ -375,14 +375,35 @@ func TestSocialGitHub_Validate(t *testing.T) { "client_id": "", }, }, - expectError: true, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, }, { name: "fails if client id does not exist", settings: ssoModels.SSOSettings{ Settings: map[string]any{}, }, - expectError: true, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if team ids are not integers", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "team_ids": "abc1234,5678,def", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if both allow assign grafana admin and skip org role sync are enabled", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "allow_assign_grafana_admin": "true", + "skip_org_role_sync": "true", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, }, } @@ -391,11 +412,11 @@ func TestSocialGitHub_Validate(t *testing.T) { s := NewGitHubProvider(&social.OAuthInfo{}, &setting.Cfg{}, &ssosettingstests.MockService{}, featuremgmt.WithFeatures()) err := s.Validate(context.Background(), tc.settings) - if tc.expectError { - require.Error(t, err) - } else { - require.NoError(t, err) + if tc.wantErr != nil { + require.ErrorIs(t, err, tc.wantErr) + return } + require.NoError(t, err) }) } } diff --git a/pkg/login/social/connectors/gitlab_oauth_test.go b/pkg/login/social/connectors/gitlab_oauth_test.go index 00a86153159..f2bf015da03 100644 --- a/pkg/login/social/connectors/gitlab_oauth_test.go +++ b/pkg/login/social/connectors/gitlab_oauth_test.go @@ -18,6 +18,7 @@ import ( "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/org" + "github.com/grafana/grafana/pkg/services/ssosettings" ssoModels "github.com/grafana/grafana/pkg/services/ssosettings/models" "github.com/grafana/grafana/pkg/services/ssosettings/ssosettingstests" "github.com/grafana/grafana/pkg/setting" @@ -463,9 +464,9 @@ func TestSocialGitlab_GetGroupsNextPage(t *testing.T) { func TestSocialGitlab_Validate(t *testing.T) { testCases := []struct { - name string - settings ssoModels.SSOSettings - expectError bool + name string + settings ssoModels.SSOSettings + wantErr error }{ { name: "SSOSettings is valid", @@ -474,7 +475,6 @@ func TestSocialGitlab_Validate(t *testing.T) { "client_id": "client-id", }, }, - expectError: false, }, { name: "fails if settings map contains an invalid field", @@ -484,7 +484,7 @@ func TestSocialGitlab_Validate(t *testing.T) { "invalid_field": []int{1, 2, 3}, }, }, - expectError: true, + wantErr: ssosettings.ErrInvalidSettings, }, { name: "fails if client id is empty", @@ -493,14 +493,25 @@ func TestSocialGitlab_Validate(t *testing.T) { "client_id": "", }, }, - expectError: true, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, }, { name: "fails if client id does not exist", settings: ssoModels.SSOSettings{ Settings: map[string]any{}, }, - expectError: true, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if both allow assign grafana admin and skip org role sync are enabled", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "allow_assign_grafana_admin": "true", + "skip_org_role_sync": "true", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, }, } @@ -509,11 +520,11 @@ func TestSocialGitlab_Validate(t *testing.T) { s := NewGitLabProvider(&social.OAuthInfo{}, &setting.Cfg{}, &ssosettingstests.MockService{}, featuremgmt.WithFeatures()) err := s.Validate(context.Background(), tc.settings) - if tc.expectError { - require.Error(t, err) - } else { - require.NoError(t, err) + if tc.wantErr != nil { + require.ErrorIs(t, err, tc.wantErr) + return } + require.NoError(t, err) }) } } diff --git a/pkg/login/social/connectors/google_oauth_test.go b/pkg/login/social/connectors/google_oauth_test.go index 79a4cb9d8cf..df66caa0d7c 100644 --- a/pkg/login/social/connectors/google_oauth_test.go +++ b/pkg/login/social/connectors/google_oauth_test.go @@ -17,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/models/roletype" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/ssosettings" ssoModels "github.com/grafana/grafana/pkg/services/ssosettings/models" "github.com/grafana/grafana/pkg/services/ssosettings/ssosettingstests" "github.com/grafana/grafana/pkg/setting" @@ -668,9 +669,9 @@ func TestSocialGoogle_UserInfo(t *testing.T) { func TestSocialGoogle_Validate(t *testing.T) { testCases := []struct { - name string - settings ssoModels.SSOSettings - expectError bool + name string + settings ssoModels.SSOSettings + wantErr error }{ { name: "SSOSettings is valid", @@ -679,7 +680,6 @@ func TestSocialGoogle_Validate(t *testing.T) { "client_id": "client-id", }, }, - expectError: false, }, { name: "fails if settings map contains an invalid field", @@ -689,7 +689,7 @@ func TestSocialGoogle_Validate(t *testing.T) { "invalid_field": []int{1, 2, 3}, }, }, - expectError: true, + wantErr: ssosettings.ErrInvalidSettings, }, { name: "fails if client id is empty", @@ -698,14 +698,25 @@ func TestSocialGoogle_Validate(t *testing.T) { "client_id": "", }, }, - expectError: true, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, }, { name: "fails if client id does not exist", settings: ssoModels.SSOSettings{ Settings: map[string]any{}, }, - expectError: true, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if both allow assign grafana admin and skip org role sync are enabled", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "allow_assign_grafana_admin": "true", + "skip_org_role_sync": "true", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, }, } @@ -714,11 +725,11 @@ func TestSocialGoogle_Validate(t *testing.T) { s := NewGoogleProvider(&social.OAuthInfo{}, &setting.Cfg{}, &ssosettingstests.MockService{}, featuremgmt.WithFeatures()) err := s.Validate(context.Background(), tc.settings) - if tc.expectError { - require.Error(t, err) - } else { - require.NoError(t, err) + if tc.wantErr != nil { + require.ErrorIs(t, err, tc.wantErr) + return } + require.NoError(t, err) }) } } diff --git a/pkg/login/social/connectors/okta_oauth_test.go b/pkg/login/social/connectors/okta_oauth_test.go index 6e332a0a1df..2bddad48984 100644 --- a/pkg/login/social/connectors/okta_oauth_test.go +++ b/pkg/login/social/connectors/okta_oauth_test.go @@ -15,6 +15,7 @@ import ( "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/models/roletype" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/ssosettings" ssoModels "github.com/grafana/grafana/pkg/services/ssosettings/models" "github.com/grafana/grafana/pkg/services/ssosettings/ssosettingstests" "github.com/grafana/grafana/pkg/setting" @@ -136,9 +137,9 @@ func TestSocialOkta_UserInfo(t *testing.T) { func TestSocialOkta_Validate(t *testing.T) { testCases := []struct { - name string - settings ssoModels.SSOSettings - expectError bool + name string + settings ssoModels.SSOSettings + wantErr error }{ { name: "SSOSettings is valid", @@ -147,7 +148,6 @@ func TestSocialOkta_Validate(t *testing.T) { "client_id": "client-id", }, }, - expectError: false, }, { name: "fails if settings map contains an invalid field", @@ -157,7 +157,7 @@ func TestSocialOkta_Validate(t *testing.T) { "invalid_field": []int{1, 2, 3}, }, }, - expectError: true, + wantErr: ssosettings.ErrInvalidSettings, }, { name: "fails if client id is empty", @@ -166,14 +166,25 @@ func TestSocialOkta_Validate(t *testing.T) { "client_id": "", }, }, - expectError: true, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, }, { name: "fails if client id does not exist", settings: ssoModels.SSOSettings{ Settings: map[string]any{}, }, - expectError: true, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if both allow assign grafana admin and skip org role sync are enabled", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "allow_assign_grafana_admin": "true", + "skip_org_role_sync": "true", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, }, } @@ -182,11 +193,11 @@ func TestSocialOkta_Validate(t *testing.T) { s := NewOktaProvider(&social.OAuthInfo{}, &setting.Cfg{}, &ssosettingstests.MockService{}, featuremgmt.WithFeatures()) err := s.Validate(context.Background(), tc.settings) - if tc.expectError { - require.Error(t, err) - } else { - require.NoError(t, err) + if tc.wantErr != nil { + require.ErrorIs(t, err, tc.wantErr) + return } + require.NoError(t, err) }) } } diff --git a/pkg/login/social/connectors/social_base.go b/pkg/login/social/connectors/social_base.go index 10f990f5600..31f8163cbba 100644 --- a/pkg/login/social/connectors/social_base.go +++ b/pkg/login/social/connectors/social_base.go @@ -222,7 +222,11 @@ func getRoleFromSearch(role string) (org.RoleType, bool) { func validateInfo(info *social.OAuthInfo) error { if info.ClientId == "" { - return ssosettings.ErrEmptyClientId.Errorf("clientId is empty") + return ssosettings.ErrInvalidOAuthConfig("ClientId is empty") + } + + if info.AllowAssignGrafanaAdmin && info.SkipOrgRoleSync { + return ssosettings.ErrInvalidOAuthConfig("Allow assign Grafana Admin and Skip org role sync are both set thus Grafana Admin role will not be synced. Consider setting one or the other.") } return nil diff --git a/pkg/services/ssosettings/errors.go b/pkg/services/ssosettings/errors.go index 6c548624625..9e1169d1a17 100644 --- a/pkg/services/ssosettings/errors.go +++ b/pkg/services/ssosettings/errors.go @@ -10,6 +10,14 @@ var ( ErrNotConfigurable = errNotFoundBase.Errorf("not configurable") + ErrBaseInvalidOAuthConfig = errutil.ValidationFailed("sso.invalidOauthConfig") + + ErrInvalidOAuthConfig = func(msg string) error { + base := ErrBaseInvalidOAuthConfig.Errorf("OAuth settings are invalid") + base.PublicMessage = msg + return base + } + ErrInvalidProvider = errutil.ValidationFailed("sso.invalidProvider", errutil.WithPublicMessage("Provider is invalid")) ErrInvalidSettings = errutil.ValidationFailed("sso.settings", errutil.WithPublicMessage("Settings field is invalid")) ErrEmptyClientId = errutil.ValidationFailed("sso.emptyClientId", errutil.WithPublicMessage("ClientId cannot be empty")) diff --git a/public/app/features/auth-config/FieldRenderer.tsx b/public/app/features/auth-config/FieldRenderer.tsx index 4402561ca1c..4c74ff5c0c8 100644 --- a/public/app/features/auth-config/FieldRenderer.tsx +++ b/public/app/features/auth-config/FieldRenderer.tsx @@ -1,8 +1,8 @@ import { css } from '@emotion/css'; import React, { useEffect, useState } from 'react'; -import { UseFormReturn } from 'react-hook-form'; +import { UseFormReturn, Controller } from 'react-hook-form'; -import { Checkbox, Field, Input, InputControl, SecretInput, Select, Switch, useTheme2 } from '@grafana/ui'; +import { Checkbox, Field, Input, SecretInput, Select, Switch, useTheme2 } from '@grafana/ui'; import { fieldMap } from './fields'; import { SSOProviderDTO, SSOSettingsField } from './types'; @@ -13,6 +13,7 @@ interface FieldRendererProps field: SSOSettingsField; errors: UseFormReturn['formState']['errors']; secretConfigured: boolean; + provider: string; } export const FieldRenderer = ({ @@ -24,12 +25,13 @@ export const FieldRenderer = ({ control, unregister, secretConfigured, + provider, }: FieldRendererProps) => { const [isSecretConfigured, setIsSecretConfigured] = useState(secretConfigured); const isDependantField = typeof field !== 'string'; const name = isDependantField ? field.name : field; const parentValue = isDependantField ? watch(field.dependsOn) : null; - const fieldData = fieldMap[name]; + const fieldData = fieldMap(provider)[name]; const theme = useTheme2(); // Unregister a field that depends on a toggle to clear its data useEffect(() => { @@ -66,18 +68,13 @@ export const FieldRenderer = ({ case 'text': return ( - + ); case 'secret': return ( - - { await user.click(screen.getByRole('button', { name: /Save/i })); await waitFor(() => { - expect(putMock).toHaveBeenCalledWith('/api/v1/sso-settings/github', { - ...testConfig, - settings: { - allowedOrganizations: 'test-org1,test-org2', - clientId: 'test-client-id', - clientSecret: 'test-client-secret', - teamIds: '12324', - enabled: true, + expect(putMock).toHaveBeenCalledWith( + '/api/v1/sso-settings/github', + { + ...testConfig, + settings: { + allowedOrganizations: 'test-org1,test-org2', + clientId: 'test-client-id', + clientSecret: 'test-client-secret', + teamIds: '12324', + enabled: true, + }, }, - }); + { showErrorAlert: false } + ); }); }); diff --git a/public/app/features/auth-config/ProviderConfigForm.tsx b/public/app/features/auth-config/ProviderConfigForm.tsx index 97e235468c2..1fcf5d5c733 100644 --- a/public/app/features/auth-config/ProviderConfigForm.tsx +++ b/public/app/features/auth-config/ProviderConfigForm.tsx @@ -31,7 +31,7 @@ export const ProviderConfigForm = ({ config, provider, isLoading }: ProviderConf setValue, unregister, formState: { errors, dirtyFields, isSubmitted }, - } = useForm({ defaultValues: dataToDTO(config), mode: 'onChange', reValidateMode: 'onChange' }); + } = useForm({ defaultValues: dataToDTO(config), mode: 'onSubmit', reValidateMode: 'onChange' }); const [isSaving, setIsSaving] = useState(false); const providerFields = fields[provider]; const [submitError, setSubmitError] = useState(false); @@ -44,11 +44,17 @@ export const ProviderConfigForm = ({ config, provider, isLoading }: ProviderConf setSubmitError(false); const requestData = dtoToData(data, provider); try { - await getBackendSrv().put(`/api/v1/sso-settings/${provider}`, { - id: config?.id, - provider: config?.provider, - settings: { ...requestData }, - }); + await getBackendSrv().put( + `/api/v1/sso-settings/${provider}`, + { + id: config?.id, + provider: config?.provider, + settings: { ...requestData }, + }, + { + showErrorAlert: false, + } + ); appEvents.publish({ type: AppEvents.alertSuccess.name, @@ -133,6 +139,7 @@ export const ProviderConfigForm = ({ config, provider, isLoading }: ProviderConf register={register} watch={watch} unregister={unregister} + provider={provider} secretConfigured={!!config?.settings.clientSecret} /> ); @@ -154,6 +161,7 @@ export const ProviderConfigForm = ({ config, provider, isLoading }: ProviderConf register={register} watch={watch} unregister={unregister} + provider={provider} secretConfigured={!!config?.settings.clientSecret} /> ); diff --git a/public/app/features/auth-config/fields.tsx b/public/app/features/auth-config/fields.tsx index 974c6eae13b..c2362ca8623 100644 --- a/public/app/features/auth-config/fields.tsx +++ b/public/app/features/auth-config/fields.tsx @@ -1,4 +1,5 @@ import React from 'react'; +import { validate as uuidValidate } from 'uuid'; import { TextLink } from '@grafana/ui'; @@ -95,276 +96,314 @@ export const sectionFields: Section = { /** * List all the fields that can be used in the form */ -export const fieldMap: Record = { - clientId: { - label: 'Client Id', - type: 'text', - description: 'The client ID of your OAuth2 app.', - validation: { - required: true, - message: 'This field is required', - }, - }, - clientSecret: { - label: 'Client Secret', - type: 'secret', - description: 'The client secret of your OAuth2 app.', - }, - teamIds: { - label: 'Team Ids', - type: 'select', - description: - 'String list of team IDs. If set, the user must be a member of one of the given teams to log in. \n' + - 'If you configure team_ids, you must also configure teams_url and team_ids_attribute_path.', - multi: true, - allowCustomValue: true, - options: [], - placeholder: 'Enter team IDs and press Enter to add', - validation: { - validate: (value) => { - if (typeof value === 'string') { - return isNumeric(value); - } - if (isSelectableValue(value)) { - return value.every((v) => v?.value && isNumeric(v.value)); - } - return true; +export function fieldMap(provider: string): Record { + return { + clientId: { + label: 'Client Id', + type: 'text', + description: 'The client Id of your OAuth2 app.', + validation: { + required: true, + message: 'This field is required', }, - message: 'Team ID must be a number.', }, - }, - allowedOrganizations: { - label: 'Allowed Organizations', - type: 'select', - description: - 'List of comma- or space-separated organizations. The user should be a member \n' + - 'of at least one organization to log in.', - multi: true, - allowCustomValue: true, - options: [], - placeholder: 'Enter organizations (my-team, myteam...) and press Enter to add', - }, - allowedDomains: { - label: 'Allowed Domains', - type: 'select', - description: - 'List of comma- or space-separated domains. The user should belong to at least \n' + 'one domain to log in.', - multi: true, - allowCustomValue: true, - options: [], - }, - authUrl: { - label: 'Auth Url', - type: 'text', - description: 'The authorization endpoint of your OAuth2 provider.', - validation: { - required: false, + clientSecret: { + label: 'Client secret', + type: 'secret', + description: 'The client secret of your OAuth2 app.', }, - }, - authStyle: { - label: 'Auth Style', - type: 'select', - description: 'It determines how client_id and client_secret are sent to Oauth2 provider. Default is AutoDetect.', - multi: false, - options: [ - { value: 'AutoDetect', label: 'AutoDetect' }, - { value: 'InParams', label: 'InParams' }, - { value: 'InHeader', label: 'InHeader' }, - ], - defaultValue: 'AutoDetect', - }, - tokenUrl: { - label: 'Token Url', - type: 'text', - description: 'The token endpoint of your OAuth2 provider.', - validation: { - required: false, + allowedOrganizations: { + label: 'Allowed organizations', + type: 'select', + description: + 'List of comma- or space-separated organizations. The user should be a member \n' + + 'of at least one organization to log in.', + multi: true, + allowCustomValue: true, + options: [], + placeholder: 'Enter organizations (my-team, myteam...) and press Enter to add', }, - }, - scopes: { - label: 'Scopes', - type: 'select', - description: 'List of comma- or space-separated OAuth2 scopes.', - multi: true, - allowCustomValue: true, - options: [], - }, - allowedGroups: { - label: 'Allowed Groups', - type: 'select', - description: - 'List of comma- or space-separated groups. The user should be a member of \n' + - 'at least one group to log in. If you configure allowed_groups, you must also configure \n' + - 'groups_attribute_path.', - multi: true, - allowCustomValue: true, - options: [], - }, - apiUrl: { - label: 'API Url', - type: 'text', - description: ( - <> - The user information endpoint of your OAuth2 provider. Information returned by this endpoint must be compatible - with{' '} - - OpenID UserInfo - - . - - ), - validation: { - required: false, + allowedDomains: { + label: 'Allowed domains', + type: 'select', + description: + 'List of comma- or space-separated domains. The user should belong to at least \n' + 'one domain to log in.', + multi: true, + allowCustomValue: true, + options: [], }, - }, - roleAttributePath: { - label: 'Role Attribute Path', - description: 'JMESPath expression to use for Grafana role lookup.', - type: 'text', - validation: { - required: false, + authUrl: { + label: 'Auth URL', + type: 'text', + description: 'The authorization endpoint of your OAuth2 provider.', + validation: { + required: false, + }, }, - }, - name: { - label: 'Display name', - description: 'Helpful if you use more than one identity providers or SSO protocols.', - type: 'text', - }, - allowSignUp: { - label: 'Allow sign up', - description: 'If not enabled, only existing Grafana users can log in using OAuth.', - type: 'switch', - }, - autoLogin: { - label: 'Auto login', - description: 'Log in automatically, skipping the login screen.', - type: 'switch', - }, - signoutRedirectUrl: { - label: 'Sign out redirect URL', - description: 'The URL to redirect the user to after signing out from Grafana.', - type: 'text', - validation: { - required: false, + authStyle: { + label: 'Auth style', + type: 'select', + description: 'It determines how client_id and client_secret are sent to Oauth2 provider. Default is AutoDetect.', + multi: false, + options: [ + { value: 'AutoDetect', label: 'AutoDetect' }, + { value: 'InParams', label: 'InParams' }, + { value: 'InHeader', label: 'InHeader' }, + ], + defaultValue: 'AutoDetect', }, - }, - emailAttributeName: { - label: 'Email attribute name', - description: 'Name of the key to use for user email lookup within the attributes map of OAuth2 ID token.', - type: 'text', - }, - emailAttributePath: { - label: 'Email attribute path', - description: 'JMESPath expression to use for user email lookup from the user information.', - type: 'text', - }, - nameAttributePath: { - label: 'Name attribute path', - description: - 'JMESPath expression to use for user name lookup from the user ID token. \n' + - 'This name will be used as the user’s display name.', - type: 'text', - }, - loginAttributePath: { - label: 'Login attribute path', - description: 'JMESPath expression to use for user login lookup from the user ID token.', - type: 'text', - }, - idTokenAttributeName: { - label: 'ID token attribute name', - description: 'The name of the key used to extract the ID token from the returned OAuth2 token.', - type: 'text', - }, - roleAttributeStrict: { - label: 'Role attribute strict mode', - description: 'If enabled, denies user login if the Grafana role cannot be extracted using Role attribute path.', - type: 'switch', - }, - allowAssignGrafanaAdmin: { - label: 'Allow assign Grafana admin', - description: 'If enabled, it will automatically sync the Grafana server administrator role.', - type: 'switch', - }, - skipOrgRoleSync: { - label: 'Skip organization role sync', - description: 'Prevent synchronizing users’ organization roles from your IdP.', - type: 'switch', - }, - defineAllowedGroups: { - label: 'Define Allowed Groups', - type: 'switch', - }, - defineAllowedTeamsIds: { - label: 'Define Allowed Teams Ids', - type: 'switch', - }, - usePkce: { - label: 'Use Pkce', - description: ( - <> - If enabled, Grafana will use{' '} - - Proof Key for Code Exchange (PKCE) - {' '} - with the OAuth2 Authorization Code Grant. - - ), - type: 'checkbox', - }, - useRefreshToken: { - label: 'Use Refresh Token', - description: - 'If enabled, Grafana will fetch a new access token using the refresh token provided by the OAuth2 provider.', - type: 'checkbox', - }, - configureTLS: { - label: 'Configure TLS', - type: 'switch', - }, - tlsClientCa: { - label: 'TLS Client CA', - description: 'The path to the trusted certificate authority list.', - type: 'text', - }, - tlsClientCert: { - label: 'TLS Client Cert', - description: 'The path to the certificate', - type: 'text', - }, - tlsClientKey: { - label: 'TLS Client Key', - description: 'The path to the key', - type: 'text', - }, - tlsSkipVerifyInsecure: { - label: 'TLS Skip Verify', - description: - 'If enabled, the client accepts any certificate presented by the server and any host \n' + - 'name in that certificate. You should only use this for testing, because this mode leaves \n' + - 'SSL/TLS susceptible to man-in-the-middle attacks.', - type: 'switch', - }, - groupsAttributePath: { - label: 'Groups attribute path', - description: - 'JMESPath expression to use for user group lookup. If you configure allowed_groups, \n' + - 'you must also configure groups_attribute_path.', - type: 'text', - }, - teamsUrl: { - label: 'Teams URL', - description: - 'The URL used to query for team IDs. If not set, the default value is /teams. \n' + - 'If you configure teams_url, you must also configure team_ids_attribute_path.', - type: 'text', - }, - teamIdsAttributePath: { - label: 'Team IDs attribute path', - description: - 'The JMESPath expression to use for Grafana team ID lookup within the results returned by the teams_url endpoint.', - type: 'text', - }, -}; + tokenUrl: { + label: 'Token URL', + type: 'text', + description: 'The token endpoint of your OAuth2 provider.', + validation: { + required: false, + }, + }, + scopes: { + label: 'Scopes', + type: 'select', + description: 'List of comma- or space-separated OAuth2 scopes.', + multi: true, + allowCustomValue: true, + options: [], + }, + allowedGroups: { + label: 'Allowed groups', + type: 'select', + description: + 'List of comma- or space-separated groups. The user should be a member of \n' + + 'at least one group to log in. If you configure allowed_groups, you must also configure \n' + + 'groups_attribute_path.', + multi: true, + allowCustomValue: true, + options: [], + validation: + provider === 'azuread' + ? { + validate: (value) => { + if (typeof value === 'string') { + return uuidValidate(value); + } + if (isSelectableValue(value)) { + return value.every((v) => v?.value && uuidValidate(v.value)); + } + return true; + }, + message: 'Allowed groups must be Object Ids.', + } + : undefined, + }, + apiUrl: { + label: 'API URL', + type: 'text', + description: ( + <> + The user information endpoint of your OAuth2 provider. Information returned by this endpoint must be + compatible with{' '} + + OpenID UserInfo + + . + + ), + validation: { + required: false, + }, + }, + roleAttributePath: { + label: 'Role attribute path', + description: 'JMESPath expression to use for Grafana role lookup.', + type: 'text', + validation: { + required: false, + }, + }, + name: { + label: 'Display name', + description: 'Helpful if you use more than one identity providers or SSO protocols.', + type: 'text', + }, + allowSignUp: { + label: 'Allow sign up', + description: 'If not enabled, only existing Grafana users can log in using OAuth.', + type: 'switch', + }, + autoLogin: { + label: 'Auto login', + description: 'Log in automatically, skipping the login screen.', + type: 'switch', + }, + signoutRedirectUrl: { + label: 'Sign out redirect URL', + description: 'The URL to redirect the user to after signing out from Grafana.', + type: 'text', + validation: { + required: false, + }, + }, + emailAttributeName: { + label: 'Email attribute name', + description: 'Name of the key to use for user email lookup within the attributes map of OAuth2 ID token.', + type: 'text', + }, + emailAttributePath: { + label: 'Email attribute path', + description: 'JMESPath expression to use for user email lookup from the user information.', + type: 'text', + }, + nameAttributePath: { + label: 'Name attribute path', + description: + 'JMESPath expression to use for user name lookup from the user ID token. \n' + + 'This name will be used as the user’s display name.', + type: 'text', + }, + loginAttributePath: { + label: 'Login attribute path', + description: 'JMESPath expression to use for user login lookup from the user ID token.', + type: 'text', + }, + idTokenAttributeName: { + label: 'ID token attribute name', + description: 'The name of the key used to extract the ID token from the returned OAuth2 token.', + type: 'text', + }, + roleAttributeStrict: { + label: 'Role attribute strict mode', + description: 'If enabled, denies user login if the Grafana role cannot be extracted using Role attribute path.', + type: 'switch', + }, + allowAssignGrafanaAdmin: { + label: 'Allow assign Grafana admin', + description: 'If enabled, it will automatically sync the Grafana server administrator role.', + type: 'switch', + }, + skipOrgRoleSync: { + label: 'Skip organization role sync', + description: 'Prevent synchronizing users’ organization roles from your IdP.', + type: 'switch', + }, + defineAllowedGroups: { + label: 'Define allowed groups', + type: 'switch', + }, + defineAllowedTeamsIds: { + label: 'Define allowed teams ids', + type: 'switch', + }, + usePkce: { + label: 'Use PKCE', + description: ( + <> + If enabled, Grafana will use{' '} + + Proof Key for Code Exchange (PKCE) + {' '} + with the OAuth2 Authorization Code Grant. + + ), + type: 'checkbox', + }, + useRefreshToken: { + label: 'Use refresh token', + description: + 'If enabled, Grafana will fetch a new access token using the refresh token provided by the OAuth2 provider.', + type: 'checkbox', + }, + configureTLS: { + label: 'Configure TLS', + type: 'switch', + }, + tlsClientCa: { + label: 'TLS client ca', + description: 'The path to the trusted certificate authority list. Does not applicable on Grafana Cloud.', + type: 'text', + }, + tlsClientCert: { + label: 'TLS client cert', + description: 'The path to the certificate. Does not applicable on Grafana Cloud.', + type: 'text', + }, + tlsClientKey: { + label: 'TLS client key', + description: 'The path to the key. Does not applicable on Grafana Cloud.', + type: 'text', + }, + tlsSkipVerifyInsecure: { + label: 'TLS skip verify', + description: + 'If enabled, the client accepts any certificate presented by the server and any host \n' + + 'name in that certificate. You should only use this for testing, because this mode leaves \n' + + 'SSL/TLS susceptible to man-in-the-middle attacks.', + type: 'switch', + }, + groupsAttributePath: { + label: 'Groups attribute path', + description: + 'JMESPath expression to use for user group lookup. If you configure allowed_groups, \n' + + 'you must also configure groupsƒ_attribute_path.', + type: 'text', + }, + teamsUrl: { + label: 'Teams URL', + description: + 'The URL used to query for Team Ids. If not set, the default value is /teams. \n' + + 'If you configure teams_url, you must also configure team_ids_attribute_path.', + type: 'text', + validation: { + validate: (value, formValues) => { + if (formValues.teamIds.length) { + return !!value; + } + return true; + }, + message: 'This field must be set if Team Ids are configured.', + }, + }, + teamIdsAttributePath: { + label: 'Team Ids attribute path', + description: + 'The JMESPath expression to use for Grafana Team Id lookup within the results returned by the teams_url endpoint.', + type: 'text', + validation: { + validate: (value, formValues) => { + if (formValues.teamIds.length) { + return !!value; + } + return true; + }, + message: 'This field must be set if Team Ids are configured.', + }, + }, + teamIds: { + label: 'Team Ids', + type: 'select', + description: + 'String list of Team Ids. If set, the user must be a member of one of the given teams to log in. \n' + + 'If you configure team_ids, you must also configure teams_url and team_ids_attribute_path.', + multi: true, + allowCustomValue: true, + options: [], + placeholder: 'Enter Team Ids and press Enter to add', + validation: + provider === 'github' + ? { + validate: (value) => { + if (typeof value === 'string') { + return isNumeric(value); + } + if (isSelectableValue(value)) { + return value.every((v) => v?.value && isNumeric(v.value)); + } + return true; + }, + message: 'Team Ids must be numbers.', + } + : undefined, + }, + }; +} // Check if a string contains only numeric values function isNumeric(value: string) { diff --git a/public/app/features/auth-config/utils/data.ts b/public/app/features/auth-config/utils/data.ts index 55fdbd0fdb1..d269dcc0ae8 100644 --- a/public/app/features/auth-config/utils/data.ts +++ b/public/app/features/auth-config/utils/data.ts @@ -58,7 +58,7 @@ export function dataToDTO(data?: SSOProvider): SSOProviderDTO { if (!data) { return emptySettings; } - const arrayFields = getArrayFields(fieldMap); + const arrayFields = getArrayFields(fieldMap(data.provider)); const settings = { ...data.settings }; for (const field of arrayFields) { //@ts-expect-error @@ -89,12 +89,16 @@ const includeRequiredKeysOnly = ( // Convert the DTO to the data format used by the API export function dtoToData(dto: SSOProviderDTO, provider: string) { - const arrayFields = getArrayFields(fieldMap); - const dtoWithRequiredFields = includeRequiredKeysOnly(dto, [...fields[provider], 'enabled']); - const settings = { ...dtoWithRequiredFields }; + const arrayFields = getArrayFields(fieldMap(provider)); + let current: Partial = dto; + + if (fields[provider]) { + current = includeRequiredKeysOnly(dto, [...fields[provider], 'enabled']); + } + const settings = { ...current }; for (const field of arrayFields) { - const value = dtoWithRequiredFields[field]; + const value = current[field]; if (value) { if (isSelectableValue(value)) { //@ts-expect-error