From 21ec93c4be48a3eaa6b5e339f5ee893fabdaa37c Mon Sep 17 00:00:00 2001 From: Wesley Miller Date: Thu, 12 Dec 2024 16:47:11 -0500 Subject: [PATCH 1/2] BED-5065: Display better error messages when creating a new user (#1001) * BED-5065: initial change * BED-5065: adjust where error is being checked in db Adjust where error is being checked on api * BED-5065: Add error checking * BED-5065: Make error text look better * BED-5065: Add null checks to error * BED-5065: Move error logic out of helper functions * BED-5065: Add missing error checks * BED-5065: Add field specific error to principal name Move error check to useEffect * BED-5065: Remove extraneous error type Co-authored-by: mistahj67 <26472282+mistahj67@users.noreply.github.com> --------- Co-authored-by: mistahj67 <26472282+mistahj67@users.noreply.github.com> --- cmd/api/src/api/error.go | 1 + cmd/api/src/api/v2/auth/auth.go | 12 ++++++++-- cmd/api/src/database/auth.go | 17 +++++++++++++- cmd/api/src/database/db.go | 5 +++-- cmd/api/src/database/helper.go | 3 ++- .../CreateUserForm/CreateUserForm.tsx | 22 ++++++++++++++----- 6 files changed, 48 insertions(+), 12 deletions(-) diff --git a/cmd/api/src/api/error.go b/cmd/api/src/api/error.go index 4827d1031a..92f763595f 100644 --- a/cmd/api/src/api/error.go +++ b/cmd/api/src/api/error.go @@ -66,6 +66,7 @@ const ( ErrorResponseAGNameTagEmpty = "asset group name or tag must not be empty" ErrorResponseAGDuplicateName = "asset group name must be unique" ErrorResponseAGDuplicateTag = "asset group tag must be unique" + ErrorResponseUserDuplicatePrincipal = "principal name must be unique" ErrorResponseDetailsUniqueViolation = "unique constraint was violated" ErrorResponseDetailsNotImplemented = "All good things to those who wait. Not implemented." diff --git a/cmd/api/src/api/v2/auth/auth.go b/cmd/api/src/api/v2/auth/auth.go index b5073552ad..4bf96ec3b7 100644 --- a/cmd/api/src/api/v2/auth/auth.go +++ b/cmd/api/src/api/v2/auth/auth.go @@ -356,7 +356,11 @@ func (s ManagementResource) CreateUser(response http.ResponseWriter, request *ht } if newUser, err := s.db.CreateUser(request.Context(), userTemplate); err != nil { - api.HandleDatabaseError(request, response, err) + if errors.Is(err, database.ErrDuplicateUserPrincipal) { + api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusConflict, api.ErrorResponseUserDuplicatePrincipal, request), response) + } else { + api.HandleDatabaseError(request, response, err) + } } else { api.WriteBasicResponse(request.Context(), newUser, http.StatusOK, response) } @@ -445,7 +449,11 @@ func (s ManagementResource) UpdateUser(response http.ResponseWriter, request *ht } if err := s.db.UpdateUser(request.Context(), user); err != nil { - api.HandleDatabaseError(request, response, err) + if errors.Is(err, database.ErrDuplicateUserPrincipal) { + api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusConflict, api.ErrorResponseUserDuplicatePrincipal, request), response) + } else { + api.HandleDatabaseError(request, response, err) + } } else { response.WriteHeader(http.StatusOK) } diff --git a/cmd/api/src/database/auth.go b/cmd/api/src/database/auth.go index 132114688a..8dc48ec748 100644 --- a/cmd/api/src/database/auth.go +++ b/cmd/api/src/database/auth.go @@ -264,7 +264,15 @@ func (s *BloodhoundDB) CreateUser(ctx context.Context, user model.User) (model.U Model: &updatedUser, } return updatedUser, s.AuditableTransaction(ctx, auditEntry, func(tx *gorm.DB) error { - return CheckError(tx.WithContext(ctx).Create(&updatedUser)) + result := tx.WithContext(ctx).Create(&updatedUser) + + if result.Error != nil { + if strings.Contains(result.Error.Error(), "duplicate key value violates unique constraint \"users_principal_name_key\"") { + return fmt.Errorf("%w: %v", ErrDuplicateUserPrincipal, tx.Error) + } + } + + return CheckError(result) }) } @@ -296,6 +304,13 @@ func (s *BloodhoundDB) UpdateUser(ctx context.Context, user model.User) error { } result := tx.WithContext(ctx).Save(&user) + + if result.Error != nil { + if strings.Contains(result.Error.Error(), "duplicate key value violates unique constraint \"users_principal_name_key\"") { + return fmt.Errorf("%w: %v", ErrDuplicateUserPrincipal, tx.Error) + } + } + return CheckError(result) }) } diff --git a/cmd/api/src/database/db.go b/cmd/api/src/database/db.go index 19c792f2c5..b22813eddd 100644 --- a/cmd/api/src/database/db.go +++ b/cmd/api/src/database/db.go @@ -44,8 +44,9 @@ const ( ) var ( - ErrDuplicateAGName = errors.New("duplicate asset group name") - ErrDuplicateAGTag = errors.New("duplicate asset group tag") + ErrDuplicateAGName = errors.New("duplicate asset group name") + ErrDuplicateAGTag = errors.New("duplicate asset group tag") + ErrDuplicateUserPrincipal = errors.New("duplicate user principal name") ) func IsUnexpectedDatabaseError(err error) bool { diff --git a/cmd/api/src/database/helper.go b/cmd/api/src/database/helper.go index fd361a075d..263bb27c68 100644 --- a/cmd/api/src/database/helper.go +++ b/cmd/api/src/database/helper.go @@ -17,8 +17,9 @@ package database import ( + "errors" + "github.com/gofrs/uuid" - "github.com/specterops/bloodhound/errors" "gorm.io/gorm" ) diff --git a/packages/javascript/bh-shared-ui/src/components/CreateUserForm/CreateUserForm.tsx b/packages/javascript/bh-shared-ui/src/components/CreateUserForm/CreateUserForm.tsx index 5557c6ce66..bbb5084917 100644 --- a/packages/javascript/bh-shared-ui/src/components/CreateUserForm/CreateUserForm.tsx +++ b/packages/javascript/bh-shared-ui/src/components/CreateUserForm/CreateUserForm.tsx @@ -16,6 +16,7 @@ import { Button } from '@bloodhoundenterprise/doodleui'; import { + Alert, Checkbox, DialogActions, DialogContent, @@ -48,6 +49,7 @@ const CreateUserForm: React.FC<{ handleSubmit, setValue, formState: { errors }, + setError, } = useForm({ defaultValues: { emailAddress: '', @@ -67,7 +69,15 @@ const CreateUserForm: React.FC<{ if (authenticationMethod === 'password') { setValue('SSOProviderId', undefined); } - }, [authenticationMethod, setValue]); + + if (error) { + if (error.response?.data?.errors[0]?.message == 'principal name must be unique') { + setError('principal', { type: 'custom', message: 'Principal name is already in use.' }); + } else { + setError('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) @@ -335,14 +345,14 @@ const CreateUserForm: React.FC<{ )} /> + {!!errors.generic && ( + + {errors.generic.message} + + )} - {error && ( - - An unexpected error occurred. Please try again. - - )}