Skip to content

Commit

Permalink
fix: user delete social auth (#3693)
Browse files Browse the repository at this point in the history
  • Loading branch information
shubham-padia authored Apr 29, 2024
1 parent 905c9fb commit 3372207
Show file tree
Hide file tree
Showing 5 changed files with 174 additions and 41 deletions.
2 changes: 2 additions & 0 deletions api/custom_auth/constants.py
Original file line number Diff line number Diff line change
@@ -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."
40 changes: 37 additions & 3 deletions api/custom_auth/serializers.py
Original file line number Diff line number Diff line change
@@ -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):
Expand Down Expand Up @@ -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
)
69 changes: 54 additions & 15 deletions api/tests/unit/users/test_unit_users_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "[email protected]"
email2 = "[email protected]"
Expand All @@ -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")
Expand Down Expand Up @@ -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 = "[email protected]"
github_auth_user_email = "[email protected]"

# 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
Expand Down
92 changes: 71 additions & 21 deletions frontend/web/components/modals/ConfirmDeleteAccount.tsx
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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<ConfirmDeleteAccountType> = ({
auth_type,
email,
lastUserOrganisations,
}) => {
const [password, setPassword] = useState<string>('')
const [deleteUserAccount, { isError, isSuccess: updateSuccess }] =
useDeleteAccountMutation()
const [currentEmail, setCurrentEmail] = useState<string>('')
const [errorMessage, setErrorMessage] = useState<string>(
ERROR_MESSAGES.default,
)
const [isEmailMismatchError, setIsEmailMismatchError] =
useState<boolean>(false)
const [
deleteUserAccount,
{ isError: isMutationError, isSuccess: updateSuccess },
] = useDeleteAccountMutation()
const skipPasswordConfirmation =
auth_type === 'GOOGLE' || auth_type === 'GITHUB'

useEffect(() => {
if (updateSuccess) {
Expand Down Expand Up @@ -54,6 +76,20 @@ const ConfirmDeleteAccount: FC<ConfirmDeleteAccountType> = ({
<form
onSubmit={(e) => {
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,
Expand All @@ -64,26 +100,40 @@ const ConfirmDeleteAccount: FC<ConfirmDeleteAccountType> = ({
<FormGroup>
<ModalBody lastUserOrganisations={lastUserOrganisations} />
</FormGroup>
<InputGroup
title='Confirm Password'
className='mb-0'
inputProps={{
className: 'full-width',
name: 'currentPassword',
}}
value={password}
onChange={(event: React.ChangeEvent<HTMLInputElement>) => {
setPassword(Utils.safeParseEventValue(event))
}}
type='password'
name='password'
/>
{isError && (
<ErrorMessage
error='Error deleting your account, please ensure you have entered your
current password.'
{skipPasswordConfirmation ? (
<InputGroup
title='Confirm Email'
className='mb-0'
inputProps={{
className: 'full-width',
name: 'currentEmail',
}}
value={currentEmail}
onChange={(event: React.ChangeEvent<HTMLInputElement>) => {
setCurrentEmail(Utils.safeParseEventValue(event))
}}
type='email'
name='currentEmail'
/>
) : (
<InputGroup
title='Confirm Password'
className='mb-0'
inputProps={{
className: 'full-width',
name: 'currentPassword',
}}
value={password}
onChange={(event: React.ChangeEvent<HTMLInputElement>) => {
setPassword(Utils.safeParseEventValue(event))
}}
type='password'
name='password'
/>
)}
{(isMutationError || isEmailMismatchError) && (
<ErrorMessage error={errorMessage} />
)}
</div>
<ModalHR />
<div className='modal-footer'>
Expand Down
12 changes: 10 additions & 2 deletions frontend/web/components/pages/AccountSettingsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,14 @@ class TheComponent extends Component {
})
}

confirmDeleteAccount = (lastUserOrganisations, id) => {
confirmDeleteAccount = (lastUserOrganisations, id, email, auth_type) => {
openModal(
'Are you sure?',
<ConfirmDeleteAccount
userId={id}
lastUserOrganisations={lastUserOrganisations}
email={email}
auth_type={auth_type}
/>,
'p-0',
)
Expand Down Expand Up @@ -126,6 +128,7 @@ class TheComponent extends Component {
render() {
const {
state: {
auth_type,
current_password,
email,
error,
Expand Down Expand Up @@ -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'
>
Expand Down

0 comments on commit 3372207

Please sign in to comment.