Auth: add the missing fields for all SSO providers (#83813)

* add the missing fields for sso providers

* remove fields array

* add the validate_hd field for google

* submit only fields defined on the provider

* fix unit tests

* add unit tests for the new fields/sections from the form

* add hosted_domain field for the google provider

* reorder fields in user mapping

* remove authStyle field from gitlab and okta

---------

Co-authored-by: Mihaly Gyongyosi <mgyongyosi@users.noreply.github.com>
This commit is contained in:
Mihai Doarna 2024-03-19 12:20:19 +02:00 committed by GitHub
parent c1c9ccb8e8
commit 6febfdffd2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 250 additions and 107 deletions

View File

@ -77,24 +77,54 @@ describe('ProviderConfigForm', () => {
jest.clearAllMocks();
});
it('renders all fields correctly', async () => {
it('renders all general settings fields correctly', async () => {
setup(<ProviderConfigForm config={testConfig} provider={testConfig.provider} />);
expect(screen.getByRole('textbox', { name: /Client ID/i })).toBeInTheDocument();
expect(screen.getByRole('combobox', { name: /Team IDs/i })).toBeInTheDocument();
expect(screen.getByRole('combobox', { name: /Allowed organizations/i })).toBeInTheDocument();
expect(screen.getByRole('textbox', { name: /Client secret/i })).toBeInTheDocument();
expect(screen.getByRole('combobox', { name: /Scopes/i })).toBeInTheDocument();
expect(screen.getByRole('checkbox', { name: /Allow Sign Up/i })).toBeInTheDocument();
expect(screen.getByRole('checkbox', { name: /Auto login/i })).toBeInTheDocument();
expect(screen.getByRole('textbox', { name: /Sign out redirect URL/i })).toBeInTheDocument();
expect(screen.getByRole('button', { name: /Save/i })).toBeInTheDocument();
expect(screen.getByRole('link', { name: /Discard/i })).toBeInTheDocument();
});
it('renders all user mapping fields correctly', async () => {
const { user } = setup(<ProviderConfigForm config={testConfig} provider={testConfig.provider} />);
await user.click(screen.getByText('User mapping'));
expect(screen.getByRole('textbox', { name: /Role attribute path/i })).toBeInTheDocument();
expect(screen.getByRole('checkbox', { name: /Role attribute strict mode/i })).toBeInTheDocument();
expect(screen.getByRole('checkbox', { name: /Skip organization role sync/i })).toBeInTheDocument();
});
it('renders all extra security fields correctly', async () => {
const { user } = setup(<ProviderConfigForm config={testConfig} provider={testConfig.provider} />);
await user.click(screen.getByText('Extra security measures'));
expect(screen.getByRole('combobox', { name: /Allowed organizations/i })).toBeInTheDocument();
expect(screen.getByRole('combobox', { name: /Allowed domains/i })).toBeInTheDocument();
expect(screen.getByRole('combobox', { name: /Team Ids/i })).toBeInTheDocument();
expect(screen.getByRole('checkbox', { name: /Use PKCE/i })).toBeInTheDocument();
expect(screen.getByRole('checkbox', { name: /Use refresh token/i })).toBeInTheDocument();
expect(screen.getByRole('checkbox', { name: /TLS skip verify/i })).toBeInTheDocument();
});
it('should save and enable on form submit', async () => {
const { user } = setup(<ProviderConfigForm config={emptyConfig} provider={emptyConfig.provider} />);
await user.type(screen.getByRole('textbox', { name: /Client ID/i }), 'test-client-id');
await user.type(screen.getByLabelText(/Client secret/i), 'test-client-secret');
// Type a team name and press enter to select it
await user.type(screen.getByRole('combobox', { name: /Team IDs/i }), '12324{enter}');
// Add two orgs
await user.type(screen.getByRole('combobox', { name: /Allowed organizations/i }), 'test-org1{enter}');
await user.type(screen.getByRole('combobox', { name: /Allowed organizations/i }), 'test-org2{enter}');
// Type a scope and press enter to select it
await user.type(screen.getByRole('combobox', { name: /Scopes/i }), 'user:email{enter}');
await user.click(screen.getByRole('checkbox', { name: /Auto login/i }));
await user.click(screen.getByText('User mapping'));
await user.type(screen.getByRole('textbox', { name: /Role attribute path/i }), 'new-attribute-path');
await user.click(screen.getByRole('checkbox', { name: /Role attribute strict mode/i }));
await user.click(screen.getByText('Extra security measures'));
await user.type(screen.getByRole('combobox', { name: /Allowed domains/i }), 'grafana.com{enter}');
await user.click(screen.getByRole('checkbox', { name: /Use PKCE/i }));
await user.click(screen.getByRole('button', { name: /Save and enable/i }));
await waitFor(() => {
@ -104,12 +134,27 @@ describe('ProviderConfigForm', () => {
id: '300f9b7c-0488-40db-9763-a22ce8bf6b3e',
provider: 'github',
settings: {
name: 'GitHub',
allowedOrganizations: 'test-org1,test-org2',
allowAssignGrafanaAdmin: false,
allowSignUp: false,
allowedDomains: 'grafana.com',
allowedOrganizations: '',
autoLogin: true,
clientId: 'test-client-id',
clientSecret: 'test-client-secret',
teamIds: '12324',
enabled: true,
name: 'GitHub',
roleAttributePath: 'new-attribute-path',
roleAttributeStrict: true,
scopes: 'user:email',
signoutRedirectUrl: '',
skipOrgRoleSync: false,
teamIds: '',
tlsClientCa: '',
tlsClientCert: '',
tlsClientKey: '',
tlsSkipVerifyInsecure: false,
usePkce: true,
useRefreshToken: false,
},
},
{ showErrorAlert: false }
@ -126,11 +171,9 @@ describe('ProviderConfigForm', () => {
const { user } = setup(<ProviderConfigForm config={emptyConfig} provider={emptyConfig.provider} />);
await user.type(screen.getByRole('textbox', { name: /Client ID/i }), 'test-client-id');
await user.type(screen.getByLabelText(/Client secret/i), 'test-client-secret');
// Type a team name and press enter to select it
await user.type(screen.getByRole('combobox', { name: /Team IDs/i }), '12324{enter}');
// Add two orgs
await user.type(screen.getByRole('combobox', { name: /Allowed organizations/i }), 'test-org1{enter}');
await user.type(screen.getByRole('combobox', { name: /Allowed organizations/i }), 'test-org2{enter}');
// Type a scope and press enter to select it
await user.type(screen.getByRole('combobox', { name: /Scopes/i }), 'user:email{enter}');
await user.click(screen.getByRole('checkbox', { name: /Auto login/i }));
await user.click(screen.getByText('Save'));
await waitFor(() => {
@ -140,12 +183,26 @@ describe('ProviderConfigForm', () => {
id: '300f9b7c-0488-40db-9763-a22ce8bf6b3e',
provider: 'github',
settings: {
name: 'GitHub',
allowedOrganizations: 'test-org1,test-org2',
allowAssignGrafanaAdmin: false,
allowSignUp: false,
allowedDomains: '',
allowedOrganizations: '',
autoLogin: true,
clientId: 'test-client-id',
clientSecret: 'test-client-secret',
teamIds: '12324',
enabled: false,
name: 'GitHub',
roleAttributePath: '',
roleAttributeStrict: false,
scopes: 'user:email',
signoutRedirectUrl: '',
skipOrgRoleSync: false,
teamIds: '',
tlsClientCa: '',
tlsClientCert: '',
tlsClientKey: '',
usePkce: false,
useRefreshToken: false,
},
},
{ showErrorAlert: false }

View File

@ -21,7 +21,7 @@ import { FormPrompt } from '../../core/components/FormPrompt/FormPrompt';
import { Page } from '../../core/components/Page/Page';
import { FieldRenderer } from './FieldRenderer';
import { fields, sectionFields } from './fields';
import { sectionFields } from './fields';
import { SSOProvider, SSOProviderDTO } from './types';
import { dataToDTO, dtoToData } from './utils/data';
@ -45,7 +45,6 @@ export const ProviderConfigForm = ({ config, provider, isLoading }: ProviderConf
formState: { errors, dirtyFields, isSubmitted },
} = useForm({ defaultValues: dataToDTO(config), mode: 'onSubmit', reValidateMode: 'onChange' });
const [isSaving, setIsSaving] = useState(false);
const providerFields = fields[provider];
const [submitError, setSubmitError] = useState(false);
const dataSubmitted = isSubmitted && !submitError;
const sections = sectionFields[provider];
@ -155,55 +154,34 @@ export const ProviderConfigForm = ({ config, provider, isLoading }: ProviderConf
<Field label="Enabled" hidden={true}>
<Switch {...register('enabled')} id="enabled" label={'Enabled'} />
</Field>
{sections ? (
<Stack gap={2} direction={'column'}>
{sections
.filter((section) => !section.hidden)
.map((section, index) => {
return (
<CollapsableSection label={section.name} isOpen={index === 0} key={section.name}>
{section.fields
.filter((field) => (typeof field !== 'string' ? !field.hidden : true))
.map((field) => {
return (
<FieldRenderer
key={typeof field === 'string' ? field : field.name}
field={field}
control={control}
errors={errors}
setValue={setValue}
register={register}
watch={watch}
unregister={unregister}
provider={provider}
secretConfigured={!!config?.settings.clientSecret}
/>
);
})}
</CollapsableSection>
);
})}
</Stack>
) : (
<>
{providerFields.map((field) => {
<Stack gap={2} direction={'column'}>
{sections
.filter((section) => !section.hidden)
.map((section, index) => {
return (
<FieldRenderer
key={field}
field={field}
control={control}
errors={errors}
setValue={setValue}
register={register}
watch={watch}
unregister={unregister}
provider={provider}
secretConfigured={!!config?.settings.clientSecret}
/>
<CollapsableSection label={section.name} isOpen={index === 0} key={section.name}>
{section.fields
.filter((field) => (typeof field !== 'string' ? !field.hidden : true))
.map((field) => {
return (
<FieldRenderer
key={typeof field === 'string' ? field : field.name}
field={field}
control={control}
errors={errors}
setValue={setValue}
register={register}
watch={watch}
unregister={unregister}
provider={provider}
secretConfigured={!!config?.settings.clientSecret}
/>
);
})}
</CollapsableSection>
);
})}
</>
)}
</Stack>
<Box display={'flex'} gap={2} marginTop={5}>
<Stack alignItems={'center'} gap={2}>
<Button

View File

@ -9,24 +9,6 @@ import { FieldData, SSOProvider, SSOSettingsField } from './types';
import { isSelectableValue } from './utils/guards';
import { isUrlValid } from './utils/url';
/** Map providers to their settings */
export const fields: Record<SSOProvider['provider'], Array<keyof SSOProvider['settings']>> = {
github: ['name', 'clientId', 'clientSecret', 'teamIds', 'allowedOrganizations'],
google: ['name', 'clientId', 'clientSecret', 'allowedDomains'],
gitlab: ['name', 'clientId', 'clientSecret', 'allowedOrganizations', 'teamIds'],
okta: [
'name',
'clientId',
'clientSecret',
'authUrl',
'tokenUrl',
'apiUrl',
'roleAttributePath',
'allowedGroups',
'allowedDomains',
],
};
type Section = Record<
SSOProvider['provider'],
Array<{
@ -131,6 +113,113 @@ export const sectionFields: Section = {
],
},
],
google: [
{
name: 'General settings',
id: 'general',
fields: ['name', 'clientId', 'clientSecret', 'scopes', 'allowSignUp', 'autoLogin', 'signoutRedirectUrl'],
},
{
name: 'User mapping',
id: 'user',
fields: ['roleAttributePath', 'roleAttributeStrict', 'allowAssignGrafanaAdmin', 'skipOrgRoleSync'],
},
{
name: 'Extra security measures',
id: 'extra',
fields: [
'validateHd',
'hostedDomain',
'allowedDomains',
'allowedGroups',
'usePkce',
'useRefreshToken',
'tlsSkipVerifyInsecure',
'tlsClientCert',
'tlsClientKey',
'tlsClientCa',
],
},
],
github: [
{
name: 'General settings',
id: 'general',
fields: ['name', 'clientId', 'clientSecret', 'scopes', 'allowSignUp', 'autoLogin', 'signoutRedirectUrl'],
},
{
name: 'User mapping',
id: 'user',
fields: ['roleAttributePath', 'roleAttributeStrict', 'allowAssignGrafanaAdmin', 'skipOrgRoleSync'],
},
{
name: 'Extra security measures',
id: 'extra',
fields: [
'allowedOrganizations',
'allowedDomains',
'teamIds',
'usePkce',
'useRefreshToken',
'tlsSkipVerifyInsecure',
'tlsClientCert',
'tlsClientKey',
'tlsClientCa',
],
},
],
gitlab: [
{
name: 'General settings',
id: 'general',
fields: ['name', 'clientId', 'clientSecret', 'scopes', 'allowSignUp', 'autoLogin', 'signoutRedirectUrl'],
},
{
name: 'User mapping',
id: 'user',
fields: ['roleAttributePath', 'roleAttributeStrict', 'allowAssignGrafanaAdmin', 'skipOrgRoleSync'],
},
{
name: 'Extra security measures',
id: 'extra',
fields: [
'allowedDomains',
'allowedGroups',
'usePkce',
'useRefreshToken',
'tlsSkipVerifyInsecure',
'tlsClientCert',
'tlsClientKey',
'tlsClientCa',
],
},
],
okta: [
{
name: 'General settings',
id: 'general',
fields: ['name', 'clientId', 'clientSecret', 'scopes', 'authUrl', 'tokenUrl', 'apiUrl', 'signoutRedirectUrl'],
},
{
name: 'User mapping',
id: 'user',
fields: ['roleAttributePath', 'roleAttributeStrict', 'allowAssignGrafanaAdmin', 'skipOrgRoleSync'],
},
{
name: 'Extra security measures',
id: 'extra',
fields: [
'allowedDomains',
'allowedGroups',
'usePkce',
'useRefreshToken',
'tlsSkipVerifyInsecure',
'tlsClientCert',
'tlsClientKey',
'tlsClientCa',
],
},
],
};
/**
@ -485,6 +574,17 @@ export function fieldMap(provider: string): Record<string, FieldData> {
}
: undefined,
},
hostedDomain: {
label: 'Hosted domain',
description: 'The domain under which Grafana is hosted and accessible.',
type: 'text',
},
validateHd: {
label: 'Validate hosted domain',
description:
'If enabled, Grafana will match the Hosted Domain retrieved from the Google ID Token against the Allowed Domains list specified by the user.',
type: 'checkbox',
},
};
}

View File

@ -55,6 +55,8 @@ export type SSOProviderSettingsBase = {
tlsSkipVerifyInsecure?: boolean;
// For Azure AD
forceUseGraphApi?: boolean;
// For Google
validateHd?: boolean;
};
// SSO data received from the API and sent to it

View File

@ -1,6 +1,6 @@
import { SelectableValue } from '@grafana/data';
import { fieldMap, fields } from '../fields';
import { fieldMap, sectionFields } from '../fields';
import { FieldData, SSOProvider, SSOProviderDTO } from '../types';
import { isSelectableValue } from './guards';
@ -58,7 +58,8 @@ export function dataToDTO(data?: SSOProvider): SSOProviderDTO {
if (!data) {
return emptySettings;
}
const arrayFields = getArrayFields(fieldMap(data.provider));
const providerFields = getFieldsForProvider(data.provider);
const arrayFields = getArrayFields(fieldMap(data.provider), providerFields);
const settings = { ...data.settings };
for (const field of arrayFields) {
//@ts-expect-error
@ -72,30 +73,35 @@ const valuesToString = (values: Array<SelectableValue<string>>) => {
return values.map(({ value }) => value).join(',');
};
const includeRequiredKeysOnly = (
obj: SSOProviderDTO,
requiredKeys: Array<keyof SSOProvider['settings']>
): Partial<SSOProviderDTO> => {
if (!requiredKeys) {
return obj;
}
let result: Partial<SSOProviderDTO> = {};
for (const key of requiredKeys) {
//@ts-expect-error
result[key] = obj[key];
}
return result;
const getFieldsForProvider = (provider: string) => {
const sections = sectionFields[provider];
// include the enabled field because it is not part of the fields defined for providers
const fields = ['enabled'];
return Object.values(sections).reduce(
(result, section) => [
...result,
...section.fields.map((field) => (typeof field === 'string' ? field : field.name)),
],
fields
);
};
// Convert the DTO to the data format used by the API
export function dtoToData(dto: SSOProviderDTO, provider: string) {
const arrayFields = getArrayFields(fieldMap(provider));
let current: Partial<SSOProviderDTO> = dto;
if (fields[provider]) {
current = includeRequiredKeysOnly(dto, [...fields[provider], 'enabled']);
}
const settings = { ...current };
const providerFields = getFieldsForProvider(provider);
const arrayFields = getArrayFields(fieldMap(provider), providerFields);
// filter out the fields that are not defined on the provider
const settings: Partial<SSOProviderDTO> = Object.keys(current)
.filter((key) => providerFields.includes(key))
.reduce((obj, key) => {
//@ts-expect-error
return { ...obj, [key]: current[key] };
}, {});
for (const field of arrayFields) {
const value = current[field];
@ -112,8 +118,8 @@ export function dtoToData(dto: SSOProviderDTO, provider: string) {
return settings;
}
export function getArrayFields(obj: Record<string, FieldData>): Array<keyof SSOProviderDTO> {
export function getArrayFields(obj: Record<string, FieldData>, providerFields: string[]): Array<keyof SSOProviderDTO> {
return Object.entries(obj)
.filter(([_, value]) => value.type === 'select')
.filter(([key, value]) => providerFields.includes(key) && value.type === 'select')
.map(([key]) => key as keyof SSOProviderDTO);
}