Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BED-5065: Display better error messages when creating a new user #1001

Merged
merged 12 commits into from
Dec 12, 2024
1 change: 1 addition & 0 deletions cmd/api/src/api/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -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."

Expand Down
12 changes: 10 additions & 2 deletions cmd/api/src/api/v2/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
17 changes: 16 additions & 1 deletion cmd/api/src/database/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}

Expand Down Expand Up @@ -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)
})
}
Expand Down
5 changes: 3 additions & 2 deletions cmd/api/src/database/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion cmd/api/src/database/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@
package database

import (
"errors"

"github.com/gofrs/uuid"
"github.com/specterops/bloodhound/errors"
"gorm.io/gorm"
)

Expand Down
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,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)
Expand Down Expand Up @@ -335,14 +345,14 @@ const CreateUserForm: React.FC<{
)}
/>
</Grid>
{!!errors.generic && (
<Grid item xs={12}>
<Alert severity='error'>{errors.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
Loading