diff --git a/api/custom_auth/constants.py b/api/custom_auth/constants.py index 2dc2110c75c2..03a224046580 100644 --- a/api/custom_auth/constants.py +++ b/api/custom_auth/constants.py @@ -1,3 +1,5 @@ USER_REGISTRATION_WITHOUT_INVITE_ERROR_MESSAGE = ( "User registration without an invite is disabled for this installation." ) +INVALID_PASSWORD_ERROR = "Invalid password." +FIELD_BLANK_ERROR = "This field may not be blank." diff --git a/api/custom_auth/serializers.py b/api/custom_auth/serializers.py index 8709eeae12f9..fe680fb5af35 100644 --- a/api/custom_auth/serializers.py +++ b/api/custom_auth/serializers.py @@ -1,15 +1,20 @@ from django.conf import settings -from djoser.serializers import UserCreateSerializer, UserDeleteSerializer +from djoser.serializers import UserCreateSerializer from rest_framework import serializers from rest_framework.authtoken.models import Token from rest_framework.exceptions import PermissionDenied from rest_framework.validators import UniqueValidator from organisations.invites.models import Invite +from users.auth_type import AuthType from users.constants import DEFAULT_DELETE_ORPHAN_ORGANISATIONS_VALUE from users.models import FFAdminUser, SignUpType -from .constants import USER_REGISTRATION_WITHOUT_INVITE_ERROR_MESSAGE +from .constants import ( + FIELD_BLANK_ERROR, + INVALID_PASSWORD_ERROR, + USER_REGISTRATION_WITHOUT_INVITE_ERROR_MESSAGE, +) class CustomTokenSerializer(serializers.ModelSerializer): @@ -72,7 +77,36 @@ def save(self, **kwargs): return super(CustomUserCreateSerializer, self).save(**kwargs) -class CustomUserDelete(UserDeleteSerializer): +class CustomUserDelete(serializers.Serializer): + current_password = serializers.CharField( + style={"input_type": "password"}, + required=False, + allow_null=True, + allow_blank=True, + ) + + default_error_messages = { + "invalid_password": INVALID_PASSWORD_ERROR, + "field_blank": FIELD_BLANK_ERROR, + } + + def validate_current_password(self, value): + user_auth_type = self.context["request"].user.auth_type + if ( + user_auth_type == AuthType.GOOGLE.value + or user_auth_type == AuthType.GITHUB.value + ): + return value + + if not value: + return self.fail("field_blank") + + is_password_valid = self.context["request"].user.check_password(value) + if is_password_valid: + return value + else: + self.fail("invalid_password") + delete_orphan_organisations = serializers.BooleanField( default=DEFAULT_DELETE_ORPHAN_ORGANISATIONS_VALUE, required=False ) diff --git a/api/tests/unit/users/test_unit_users_views.py b/api/tests/unit/users/test_unit_users_views.py index 6291f996da5f..23e96272a320 100644 --- a/api/tests/unit/users/test_unit_users_views.py +++ b/api/tests/unit/users/test_unit_users_views.py @@ -608,22 +608,25 @@ def test_group_admin_can_retrieve_group( assert response.status_code == status.HTTP_200_OK +def delete_user( + user: FFAdminUser, + password: str = None, + delete_orphan_organisations: bool = True, +): + client = APIClient() + client.force_authenticate(user) + data = { + "delete_orphan_organisations": delete_orphan_organisations, + } + if password: + data["password"] = password + + url = "/api/v1/auth/users/me/" + return client.delete(url, data=json.dumps(data), content_type="application/json") + + @pytest.mark.django_db def test_delete_user(): - def delete_user( - user: FFAdminUser, password: str, delete_orphan_organisations: bool = True - ): - client = APIClient() - client.force_authenticate(user) - data = { - "current_password": password, - "delete_orphan_organisations": delete_orphan_organisations, - } - url = "/api/v1/auth/users/me/" - return client.delete( - url, data=json.dumps(data), content_type="application/json" - ) - # create a couple of users email1 = "test1@example.com" email2 = "test2@example.com" @@ -633,7 +636,7 @@ def delete_user( user2 = FFAdminUser.objects.create_user(email=email2, password=password) user3 = FFAdminUser.objects.create_user(email=email3, password=password) - # crete some organizations + # create some organizations org1 = Organisation.objects.create(name="org1") org2 = Organisation.objects.create(name="org2") org3 = Organisation.objects.create(name="org3") @@ -681,6 +684,42 @@ def delete_user( assert Organisation.objects.filter(name="org1").count() == 1 +@pytest.mark.django_db +@pytest.mark.parametrize("password", [None, "", "random"]) +def test_delete_user_social_auth_with_no_password(password): + google_auth_user_email = "google@example.com" + github_auth_user_email = "github@example.com" + + # We have given each social auth test user their own org since all the other org + # logic has been checked in the email/password users tests and we're just doing a + # sanity check here to make sure that the related org is deleted. + google_auth_user_org = Organisation.objects.create(name="google_auth_user_org") + github_auth_user_org = Organisation.objects.create(name="github_auth_user_org") + + google_auth_user = FFAdminUser.objects.create_user( + email=google_auth_user_email, google_user_id=123456 + ) + github_auth_user = FFAdminUser.objects.create_user( + email=github_auth_user_email, github_user_id=123456 + ) + + # Add social auth users to their orgs + google_auth_user_org.users.add(google_auth_user) + github_auth_user_org.users.add(github_auth_user) + + # Delete google_auth_user + response = delete_user(google_auth_user, password, delete_orphan_organisations=True) + assert response.status_code == status.HTTP_204_NO_CONTENT + assert not FFAdminUser.objects.filter(email=google_auth_user_email).exists() + assert Organisation.objects.filter(name="google_auth_user_org").count() == 0 + + # Delete github_auth_user + response = delete_user(github_auth_user, password, delete_orphan_organisations=True) + assert response.status_code == status.HTTP_204_NO_CONTENT + assert not FFAdminUser.objects.filter(email=github_auth_user_email).exists() + assert Organisation.objects.filter(name="github_auth_user_org").count() == 0 + + @pytest.mark.django_db def test_change_email_address_api(mocker): # Given diff --git a/frontend/web/components/modals/ConfirmDeleteAccount.tsx b/frontend/web/components/modals/ConfirmDeleteAccount.tsx index 6ad53d85e34f..18c36d7ddbe9 100644 --- a/frontend/web/components/modals/ConfirmDeleteAccount.tsx +++ b/frontend/web/components/modals/ConfirmDeleteAccount.tsx @@ -1,7 +1,7 @@ import React, { FC, useEffect, useState } from 'react' import Button from 'components/base/forms/Button' import Utils from 'common/utils/utils' -import { Organisation } from 'common/types/responses' +import { AuthType, Organisation } from 'common/types/responses' import { useDeleteAccountMutation } from 'common/services/useAccount' import InputGroup from 'components/base/forms/InputGroup' import ModalHR from './ModalHR' @@ -10,13 +10,35 @@ import ErrorMessage from 'components/ErrorMessage' type ConfirmDeleteAccountType = { lastUserOrganisations: Organisation[] + email?: string + auth_type?: AuthType +} + +const ERROR_MESSAGES = { + default: 'Error deleting your account.', + mismatchEmail: + 'Error deleting your account, please ensure you have entered your current email.', + mismatchPassword: + 'Error deleting your account, please ensure you have entered your current password.', } const ConfirmDeleteAccount: FC = ({ + auth_type, + email, lastUserOrganisations, }) => { const [password, setPassword] = useState('') - const [deleteUserAccount, { isError, isSuccess: updateSuccess }] = - useDeleteAccountMutation() + const [currentEmail, setCurrentEmail] = useState('') + const [errorMessage, setErrorMessage] = useState( + ERROR_MESSAGES.default, + ) + const [isEmailMismatchError, setIsEmailMismatchError] = + useState(false) + const [ + deleteUserAccount, + { isError: isMutationError, isSuccess: updateSuccess }, + ] = useDeleteAccountMutation() + const skipPasswordConfirmation = + auth_type === 'GOOGLE' || auth_type === 'GITHUB' useEffect(() => { if (updateSuccess) { @@ -54,6 +76,20 @@ const ConfirmDeleteAccount: FC = ({
{ Utils.preventDefault(e) + + if (skipPasswordConfirmation) { + if (currentEmail !== email) { + setIsEmailMismatchError(true) + setErrorMessage(ERROR_MESSAGES.mismatchEmail) + return + } else { + setIsEmailMismatchError(false) + setErrorMessage(ERROR_MESSAGES.default) + } + } else { + setErrorMessage(ERROR_MESSAGES.mismatchPassword) + } + deleteUserAccount({ current_password: password, delete_orphan_organisations: true, @@ -64,26 +100,40 @@ const ConfirmDeleteAccount: FC = ({ - ) => { - setPassword(Utils.safeParseEventValue(event)) - }} - type='password' - name='password' - /> - {isError && ( - ) => { + setCurrentEmail(Utils.safeParseEventValue(event)) + }} + type='email' + name='currentEmail' + /> + ) : ( + ) => { + setPassword(Utils.safeParseEventValue(event)) + }} + type='password' + name='password' /> )} + {(isMutationError || isEmailMismatchError) && ( + + )}
diff --git a/frontend/web/components/pages/AccountSettingsPage.js b/frontend/web/components/pages/AccountSettingsPage.js index 3b79579cc1b6..09a0a13e5404 100644 --- a/frontend/web/components/pages/AccountSettingsPage.js +++ b/frontend/web/components/pages/AccountSettingsPage.js @@ -56,12 +56,14 @@ class TheComponent extends Component { }) } - confirmDeleteAccount = (lastUserOrganisations, id) => { + confirmDeleteAccount = (lastUserOrganisations, id, email, auth_type) => { openModal( 'Are you sure?', , 'p-0', ) @@ -126,6 +128,7 @@ class TheComponent extends Component { render() { const { state: { + auth_type, current_password, email, error, @@ -305,7 +308,12 @@ class TheComponent extends Component { id='delete-user-btn' data-test='delete-user-btn' onClick={() => - this.confirmDeleteAccount(lastUserOrganisations, id) + this.confirmDeleteAccount( + lastUserOrganisations, + id, + email, + auth_type, + ) } className='btn-with-icon btn-remove' >