Skip to content

Commit

Permalink
Exec error messaging when creating SSO providers (#1011)
Browse files Browse the repository at this point in the history
* BED-5066: Return conflict error if SSO provider name conflicts
Add better error messaging on UI for this case

* BED-5066: Add error check to SSO update routes
Make front-end error check more robust

* BED-5055: Back change the way the errors are handled on the front-end

* BED-5066: Fix error check
Run prepare for codereview

* BED-5066: resolve TS errors
Raise name error on duplicate SSO slug

* BED-5066: Add duplicate slug check to UpdateSSOProvider as well
  • Loading branch information
byt3n33dl3 committed Dec 21, 2024
1 parent 7d42e7d commit c43e108
Show file tree
Hide file tree
Showing 8 changed files with 113 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import { Button } from '@bloodhoundenterprise/doodleui';
import {
Alert,
Checkbox,
DialogActions,
DialogContent,
Expand Down Expand Up @@ -48,6 +49,7 @@ const CreateUserForm: React.FC<{
handleSubmit,
setValue,
formState: { errors },
setError,
} = useForm<CreateUserRequestForm>({
defaultValues: {
emailAddress: '',
Expand All @@ -67,7 +69,22 @@ const CreateUserForm: React.FC<{
if (authenticationMethod === 'password') {
setValue('SSOProviderId', undefined);
}
}, [authenticationMethod, setValue]);

if (error) {
if (error?.response?.status === 409) {
if (error.response?.data?.errors[0]?.message.toLowerCase().includes('principal name')) {
setError('principal', { type: 'custom', message: 'Principal name is already in use.' });
} else {
setError('root.generic', { type: 'custom', message: `A conflict has occured.` });
}
} else {
setError('root.generic', {
type: 'custom',
message: 'An unexpected error occurred. Please try again.',
});
}
}
}, [authenticationMethod, setValue, error, setError]);

const getRolesQuery = useQuery(['getRoles'], ({ signal }) =>
apiClient.getRoles({ signal }).then((res) => res.data?.data?.roles)
Expand Down Expand Up @@ -335,14 +352,14 @@ const CreateUserForm: React.FC<{
)}
/>
</Grid>
{!!errors.root?.generic && (
<Grid item xs={12}>
<Alert severity='error'>{errors.root.generic.message}</Alert>
</Grid>
)}
</Grid>
</DialogContent>
<DialogActions>
{error && (
<FormHelperText error style={{ margin: 0 }}>
An unexpected error occurred. Please try again.
</FormHelperText>
)}
<Button
type='button'
variant={'tertiary'}
Expand All @@ -361,4 +378,4 @@ const CreateUserForm: React.FC<{
);
};

export default CreateUserForm;
export default CreateUserForm;
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ describe('UpdateUserDialog', () => {
onSave={testOnSave}
isLoading={options?.renderLoading || false}
error={options?.renderErrors}
hasSelectedSelf={false}
/>
);

Expand Down Expand Up @@ -352,4 +353,4 @@ describe('UpdateUserDialog', () => {
}
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@

import { Button } from '@bloodhoundenterprise/doodleui';
import {
Alert,
DialogActions,
DialogContent,
DialogContentText,
FormControl,
FormHelperText,
Grid,
InputLabel,
MenuItem,
Expand Down Expand Up @@ -141,6 +141,7 @@ const UpdateUserFormInner: React.FC<{
handleSubmit,
setValue,
formState: { errors },
setError,
watch,
} = useForm<UpdateUserRequestForm & { authenticationMethod: 'sso' | 'password' }>({
defaultValues: {
Expand All @@ -155,7 +156,22 @@ const UpdateUserFormInner: React.FC<{
if (authenticationMethod === 'password') {
setValue('SSOProviderId', undefined);
}
}, [authenticationMethod, setValue]);

if (error) {
if (error?.response?.status === 409) {
if (error.response?.data?.errors[0]?.message.toLowerCase().includes('principal name')) {
setError('principal', { type: 'custom', message: 'Principal name is already in use.' });
} else {
setError('root.generic', { type: 'custom', message: `A conflict has occured.` });
}
} else {
setError('root.generic', {
type: 'custom',
message: 'An unexpected error occurred. Please try again.',
});
}
}
}, [authenticationMethod, setValue, error, setError]);

return (
<form autoComplete='off' onSubmit={handleSubmit(onSubmit)}>
Expand Down Expand Up @@ -359,14 +375,14 @@ const UpdateUserFormInner: React.FC<{
)}
/>
</Grid>
{!!errors.root?.generic && (
<Grid item xs={12}>
<Alert severity='error'>{errors.root.generic.message}</Alert>
</Grid>
)}
</Grid>
</DialogContent>
<DialogActions>
{error && (
<FormHelperText error style={{ margin: 0 }}>
An unexpected error occurred. Please try again.
</FormHelperText>
)}
<Button
type='button'
variant={'tertiary'}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import UpsertOIDCProviderForm from './UpsertOIDCProviderForm';

const UpsertOIDCProviderDialog: React.FC<{
open: boolean;
error?: string;
error: any;
oldSSOProvider?: SSOProvider;
onClose: () => void;
onSubmit: (data: UpsertOIDCProviderRequest) => void;
Expand All @@ -46,4 +46,4 @@ const UpsertOIDCProviderDialog: React.FC<{
);
};

export default UpsertOIDCProviderDialog;
export default UpsertOIDCProviderDialog;
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@

import { Button } from '@bloodhoundenterprise/doodleui';
import { Alert, DialogContent, DialogActions, Grid, TextField } from '@mui/material';
import { FC } from 'react';
import { useEffect, FC } from 'react';
import { Controller, useForm } from 'react-hook-form';
import { OIDCProviderInfo, SSOProvider, UpsertOIDCProviderRequest } from 'js-client-library';

const UpsertOIDCProviderForm: FC<{
error?: string;
error: any;
oldSSOProvider?: SSOProvider;
onClose: () => void;
onSubmit: (data: UpsertOIDCProviderRequest) => void;
Expand All @@ -37,8 +37,29 @@ const UpsertOIDCProviderForm: FC<{
handleSubmit,
reset,
formState: { errors },
setError,
} = useForm<UpsertOIDCProviderRequest>({ defaultValues });

useEffect(() => {
if (error) {
if (error?.response?.status === 409) {
if (error.response?.data?.errors[0]?.message.toLowerCase().includes('sso provider name')) {
setError('name', { type: 'custom', message: 'SSO Provider Name is already in use.' });
} else {
setError('root.generic', {
type: 'custom',
message: 'A conflict has occured.',
});
}
} else {
setError('root.generic', {
type: 'custom',
message: `Unable to ${oldSSOProvider ? 'update' : 'create new'} OIDC Provider configuration. Please try again.`,
});
}
}
}, [error, setError, oldSSOProvider]);

const handleClose = () => {
onClose();
reset();
Expand Down Expand Up @@ -113,9 +134,9 @@ const UpsertOIDCProviderForm: FC<{
)}
/>
</Grid>
{error && (
{!!errors.root?.generic && (
<Grid item xs={12}>
<Alert severity='error'>{error}</Alert>
<Alert severity='error'>{errors.root.generic.message}</Alert>
</Grid>
)}
</Grid>
Expand All @@ -136,4 +157,4 @@ const UpsertOIDCProviderForm: FC<{
);
};

export default UpsertOIDCProviderForm;
export default UpsertOIDCProviderForm;
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import UpsertSAMLProviderForm from '../UpsertSAMLProviderForm';

const UpsertSAMLProviderDialog: React.FC<{
open: boolean;
error?: string;
error: any;
oldSSOProvider?: SSOProvider;
onClose: () => void;
onSubmit: (data: UpsertSAMLProviderFormInputs) => void;
Expand All @@ -46,4 +46,4 @@ const UpsertSAMLProviderDialog: React.FC<{
);
};

export default UpsertSAMLProviderDialog;
export default UpsertSAMLProviderDialog;
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ import {
Typography,
useTheme,
} from '@mui/material';
import { useState, FC } from 'react';
import { useState, useEffect, FC } from 'react';
import { Controller, useForm } from 'react-hook-form';
import { SSOProvider, UpsertSAMLProviderFormInputs } from 'js-client-library';

const UpsertSAMLProviderForm: FC<{
error?: string;
error: any;
oldSSOProvider?: SSOProvider;
onClose: () => void;
onSubmit: (data: UpsertSAMLProviderFormInputs) => void;
Expand All @@ -42,6 +42,7 @@ const UpsertSAMLProviderForm: FC<{
handleSubmit,
reset,
formState: { errors },
setError,
} = useForm<UpsertSAMLProviderFormInputs>({
defaultValues: {
name: oldSSOProvider?.name ?? '',
Expand All @@ -50,6 +51,26 @@ const UpsertSAMLProviderForm: FC<{
});
const [fileValue, setFileValue] = useState(''); // small workaround to use the file input

useEffect(() => {
if (error) {
if (error?.response?.status === 409) {
if (error.response?.data?.errors[0]?.message.toLowerCase().includes('sso provider name')) {
setError('name', { type: 'custom', message: 'SSO Provider Name is already in use.' });
} else {
setError('root.generic', {
type: 'custom',
message: `A conflict has occured.`,
});
}
} else {
setError('root.generic', {
type: 'custom',
message: `Unable to ${oldSSOProvider ? 'update' : 'create new'} SAML Provider configuration. Please try again.`,
});
}
}
}, [error, setError, oldSSOProvider]);

const handleClose = () => {
onClose();
setFileValue('');
Expand Down Expand Up @@ -126,9 +147,9 @@ const UpsertSAMLProviderForm: FC<{
: 'Upload the Metadata file provided by your SAML Provider'}
</FormHelperText>
</Grid>
{error && (
{!!errors.root?.generic && (
<Grid item xs={12}>
<Alert severity='error'>{error}</Alert>
<Alert severity='error'>{errors.root.generic.message}</Alert>
</Grid>
)}
</Grid>
Expand All @@ -149,4 +170,4 @@ const UpsertSAMLProviderForm: FC<{
);
};

export default UpsertSAMLProviderForm;
export default UpsertSAMLProviderForm;
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const SSOConfiguration: FC = () => {
const [ssoProviderIdToDeleteOrUpdate, setSSOProviderIdToDeleteOrUpdate] = useState<SSOProvider['id'] | undefined>();
const [dialogOpen, setDialogOpen] = useState<'SAML' | 'OIDC' | 'DELETE' | ''>('');
const [nameFilter, setNameFilter] = useState<string>('');
const [upsertProviderError, setUpsertProviderError] = useState<string>('');
const [upsertProviderError, setUpsertProviderError] = useState<any>();
const [typeSortOrder, setTypeSortOrder] = useState<SortOrder>();

const listSSOProvidersQuery = useQuery(['listSSOProviders'], ({ signal }) =>
Expand Down Expand Up @@ -115,7 +115,7 @@ const SSOConfiguration: FC = () => {
};

const closeDialog = () => {
setUpsertProviderError('');
setUpsertProviderError(null);
setDialogOpen('');
setTimeout(() => setSSOProviderIdToDeleteOrUpdate(undefined), 500);
};
Expand Down Expand Up @@ -171,7 +171,7 @@ const SSOConfiguration: FC = () => {
};

const upsertSAMLProvider = async (samlProvider: UpsertSAMLProviderFormInputs) => {
setUpsertProviderError('');
setUpsertProviderError(null);
try {
const payload = { name: samlProvider.name, metadata: samlProvider.metadata && samlProvider.metadata[0] };
if (ssoProviderIdToDeleteOrUpdate) {
Expand All @@ -183,16 +183,14 @@ const SSOConfiguration: FC = () => {
}
listSSOProvidersQuery.refetch();
closeDialog();
} catch (error) {
} catch (error: any) {
console.error(error);
setUpsertProviderError(
`Unable to ${ssoProviderIdToDeleteOrUpdate ? 'update' : 'create new'} SAML Provider configuration. Please try again.`
);
setUpsertProviderError(error);
}
};

const upsertOIDCProvider = async (oidcProvider: UpsertOIDCProviderRequest) => {
setUpsertProviderError('');
setUpsertProviderError(null);
try {
if (ssoProviderIdToDeleteOrUpdate) {
await apiClient.updateOIDCProvider(ssoProviderIdToDeleteOrUpdate, oidcProvider);
Expand All @@ -209,9 +207,7 @@ const SSOConfiguration: FC = () => {
closeDialog();
} catch (error) {
console.error(error);
setUpsertProviderError(
`Unable to ${ssoProviderIdToDeleteOrUpdate ? 'update' : 'create new'} OIDC Provider configuration. Please try again.`
);
setUpsertProviderError(error);
}
};

Expand Down Expand Up @@ -314,4 +310,4 @@ const SSOConfiguration: FC = () => {
);
};

export default SSOConfiguration;
export default SSOConfiguration;

0 comments on commit c43e108

Please sign in to comment.