From 7438190f64e8f4f1d0659583b1a46e5a712584a3 Mon Sep 17 00:00:00 2001 From: Luke Campbell <64781228+lcampbell2@users.noreply.github.com> Date: Mon, 31 Jul 2023 07:56:12 -0300 Subject: [PATCH] Bugfix separate close account functions (#4721) * separate close-account mutations into self and other * refactor close-account tests * refactor frontend mutation calls --- .../mutations/__tests__/close-account.test.js | 52 +++---- api/src/user/mutations/close-account.js | 143 ++++++++++++++---- frontend/src/admin/SuperAdminUserList.js | 4 +- frontend/src/graphql/mutations.js | 22 ++- frontend/src/user/UserPage.js | 4 +- 5 files changed, 163 insertions(+), 62 deletions(-) diff --git a/api/src/user/mutations/__tests__/close-account.test.js b/api/src/user/mutations/__tests__/close-account.test.js index be1356e126..388929d13f 100644 --- a/api/src/user/mutations/__tests__/close-account.test.js +++ b/api/src/user/mutations/__tests__/close-account.test.js @@ -143,7 +143,7 @@ describe('given the closeAccount mutation', () => { schema, ` mutation { - closeAccount(input: {}) { + closeAccountSelf(input: {}) { result { ... on CloseAccountResult { status @@ -221,7 +221,7 @@ describe('given the closeAccount mutation', () => { schema, ` mutation { - closeAccount(input: {}) { + closeAccountSelf(input: {}) { result { ... on CloseAccountResult { status @@ -271,7 +271,7 @@ describe('given the closeAccount mutation', () => { const expectedResponse = { data: { - closeAccount: { + closeAccountSelf: { result: { status: 'Successfully closed account.', }, @@ -303,7 +303,7 @@ describe('given the closeAccount mutation', () => { schema, ` mutation { - closeAccount(input: {}) { + closeAccountSelf(input: {}) { result { ... on CloseAccountResult { status @@ -353,7 +353,7 @@ describe('given the closeAccount mutation', () => { const expectedResponse = { data: { - closeAccount: { + closeAccountSelf: { result: { status: 'Le compte a été fermé avec succès.', }, @@ -370,7 +370,7 @@ describe('given the closeAccount mutation', () => { schema, ` mutation { - closeAccount(input: {}) { + closeAccountSelf(input: {}) { result { ... on CloseAccountResult { status @@ -538,7 +538,7 @@ describe('given the closeAccount mutation', () => { schema, ` mutation { - closeAccount(input: {}) { + closeAccountSelf(input: {}) { result { ... on CloseAccountResult { status @@ -622,7 +622,7 @@ describe('given the closeAccount mutation', () => { schema, ` mutation { - closeAccount(input: {}) { + closeAccountSelf(input: {}) { result { ... on CloseAccountResult { status @@ -677,7 +677,7 @@ describe('given the closeAccount mutation', () => { const expectedResponse = { data: { - closeAccount: { + closeAccountSelf: { result: { status: 'Successfully closed account.', }, @@ -709,7 +709,7 @@ describe('given the closeAccount mutation', () => { schema, ` mutation { - closeAccount(input: {}) { + closeAccountSelf(input: {}) { result { ... on CloseAccountResult { status @@ -764,7 +764,7 @@ describe('given the closeAccount mutation', () => { const expectedResponse = { data: { - closeAccount: { + closeAccountSelf: { result: { status: 'Le compte a été fermé avec succès.', }, @@ -781,7 +781,7 @@ describe('given the closeAccount mutation', () => { schema, ` mutation { - closeAccount(input: { + closeAccountOther(input:{ userId: "${toGlobalId('user', user._key)}" }) { result { @@ -867,7 +867,7 @@ describe('given the closeAccount mutation', () => { schema, ` mutation { - closeAccount(input: { + closeAccountOther(input:{ userId: "${toGlobalId('user', '456')}" }) { result { @@ -907,7 +907,7 @@ describe('given the closeAccount mutation', () => { const expectedResponse = { data: { - closeAccount: { + closeAccountOther: { result: { code: 400, description: "Permission error: Unable to close other user's account.", @@ -928,7 +928,7 @@ describe('given the closeAccount mutation', () => { schema, ` mutation { - closeAccount(input: { + closeAccountOther(input:{ userId: "${toGlobalId('user', '456')}" }) { result { @@ -971,7 +971,7 @@ describe('given the closeAccount mutation', () => { const expectedResponse = { data: { - closeAccount: { + closeAccountOther: { result: { code: 400, description: 'Unable to close account of an undefined user.', @@ -1005,7 +1005,7 @@ describe('given the closeAccount mutation', () => { schema, ` mutation { - closeAccount(input: {}) { + closeAccountSelf(input: {}) { result { ... on CloseAccountResult { status @@ -1069,7 +1069,7 @@ describe('given the closeAccount mutation', () => { schema, ` mutation { - closeAccount(input: {}) { + closeAccountSelf(input: {}) { result { ... on CloseAccountResult { status @@ -1134,7 +1134,7 @@ describe('given the closeAccount mutation', () => { schema, ` mutation { - closeAccount(input: {}) { + closeAccountSelf(input: {}) { result { ... on CloseAccountResult { status @@ -1204,7 +1204,7 @@ describe('given the closeAccount mutation', () => { schema, ` mutation { - closeAccount(input: { + closeAccountOther(input:{ userId: "${toGlobalId('user', '456')}" }) { result { @@ -1244,7 +1244,7 @@ describe('given the closeAccount mutation', () => { const expectedResponse = { data: { - closeAccount: { + closeAccountOther: { result: { code: 400, description: "Erreur de permission: Impossible de fermer le compte d'un autre utilisateur.", @@ -1265,7 +1265,7 @@ describe('given the closeAccount mutation', () => { schema, ` mutation { - closeAccount(input: { + closeAccountOther(input:{ userId: "${toGlobalId('user', '456')}" }) { result { @@ -1308,7 +1308,7 @@ describe('given the closeAccount mutation', () => { const expectedResponse = { data: { - closeAccount: { + closeAccountOther: { result: { code: 400, description: "Impossible de fermer le compte d'un utilisateur non défini.", @@ -1342,7 +1342,7 @@ describe('given the closeAccount mutation', () => { schema, ` mutation { - closeAccount(input: {}) { + closeAccountSelf(input: {}) { result { ... on CloseAccountResult { status @@ -1406,7 +1406,7 @@ describe('given the closeAccount mutation', () => { schema, ` mutation { - closeAccount(input: {}) { + closeAccountSelf(input: {}) { result { ... on CloseAccountResult { status @@ -1471,7 +1471,7 @@ describe('given the closeAccount mutation', () => { schema, ` mutation { - closeAccount(input: {}) { + closeAccountSelf(input: {}) { result { ... on CloseAccountResult { status diff --git a/api/src/user/mutations/close-account.js b/api/src/user/mutations/close-account.js index 012dbc0b76..9f6d8b2443 100644 --- a/api/src/user/mutations/close-account.js +++ b/api/src/user/mutations/close-account.js @@ -5,9 +5,98 @@ import { logActivity } from '../../audit-logs/mutations/log-activity' import { closeAccountUnion } from '../unions' -export const closeAccount = new mutationWithClientMutationId({ - name: 'CloseAccount', - description: `This mutation allows a user to close their account, or a super admin to close another user's account.`, +export const closeAccountSelf = new mutationWithClientMutationId({ + name: 'CloseAccountSelf', + description: `This mutation allows a user to close their account.`, + outputFields: () => ({ + result: { + type: closeAccountUnion, + description: '`CloseAccountUnion` returning either a `CloseAccountResult`, or `CloseAccountError` object.', + resolve: (payload) => payload, + }, + }), + mutateAndGetPayload: async ( + args, + { i18n, query, collections, transaction, auth: { userRequired }, validators: { cleanseInput } }, + ) => { + let submittedUserId + if (args?.userId) { + submittedUserId = fromGlobalId(cleanseInput(args.userId)).id + } + + const user = await userRequired() + + const userId = user._id + const targetUserName = user.userName + + // Setup Trans action + const trx = await transaction(collections) + + try { + await trx.step( + () => query` + WITH affiliations, organizations, users + FOR v, e IN 1..1 INBOUND ${userId} affiliations + REMOVE { _key: e._key } IN affiliations + OPTIONS { waitForSync: true } + `, + ) + } catch (err) { + console.error( + `Trx step error occurred when removing users remaining affiliations when user: ${user._key} attempted to close account: ${userId}: ${err}`, + ) + throw new Error(i18n._(t`Unable to close account. Please try again.`)) + } + + try { + await trx.step( + () => query` + WITH users + REMOVE PARSE_IDENTIFIER(${userId}).key + IN users OPTIONS { waitForSync: true } + `, + ) + } catch (err) { + console.error( + `Trx step error occurred when removing user: ${user._key} attempted to close account: ${userId}: ${err}`, + ) + throw new Error(i18n._(t`Unable to close account. Please try again.`)) + } + + try { + await trx.commit() + } catch (err) { + console.error(`Trx commit error occurred when user: ${user._key} attempted to close account: ${userId}: ${err}`) + throw new Error(i18n._(t`Unable to close account. Please try again.`)) + } + + console.info(`User: ${user._key} successfully closed user: ${userId} account.`) + await logActivity({ + transaction, + collections, + query, + initiatedBy: { + id: user._key, + userName: user.userName, + role: submittedUserId ? 'SUPER_ADMIN' : '', + }, + action: 'delete', + target: { + resource: targetUserName, // name of resource being acted upon + resourceType: 'user', // user, org, domain + }, + }) + + return { + _type: 'regular', + status: i18n._(t`Successfully closed account.`), + } + }, +}) + +export const closeAccountOther = new mutationWithClientMutationId({ + name: 'CloseAccountOther', + description: `This mutation allows a super admin to close another user's account.`, inputFields: () => ({ userId: { type: GraphQLID, @@ -42,36 +131,32 @@ export const closeAccount = new mutationWithClientMutationId({ let userId = '' let targetUserName = '' - if (submittedUserId) { - const permission = await checkSuperAdmin() - if (!permission) { - console.warn( - `User: ${user._key} attempted to close user: ${submittedUserId} account, but requesting user is not a super admin.`, - ) - return { - _type: 'error', - code: 400, - description: i18n._(t`Permission error: Unable to close other user's account.`), - } + + const permission = await checkSuperAdmin() + if (!permission) { + console.warn( + `User: ${user._key} attempted to close user: ${submittedUserId} account, but requesting user is not a super admin.`, + ) + return { + _type: 'error', + code: 400, + description: i18n._(t`Permission error: Unable to close other user's account.`), } + } - const checkUser = await loadUserByKey.load(submittedUserId) - if (typeof checkUser === 'undefined') { - console.warn( - `User: ${user._key} attempted to close user: ${submittedUserId} account, but requested user is undefined.`, - ) - return { - _type: 'error', - code: 400, - description: i18n._(t`Unable to close account of an undefined user.`), - } + const checkUser = await loadUserByKey.load(submittedUserId) + if (typeof checkUser === 'undefined') { + console.warn( + `User: ${user._key} attempted to close user: ${submittedUserId} account, but requested user is undefined.`, + ) + return { + _type: 'error', + code: 400, + description: i18n._(t`Unable to close account of an undefined user.`), } - userId = checkUser._id - targetUserName = checkUser.userName - } else { - userId = user._id - targetUserName = user.userName } + userId = checkUser._id + targetUserName = checkUser.userName // Setup Trans action const trx = await transaction(collections) diff --git a/frontend/src/admin/SuperAdminUserList.js b/frontend/src/admin/SuperAdminUserList.js index 0f24ad3cd3..3b5416a51e 100644 --- a/frontend/src/admin/SuperAdminUserList.js +++ b/frontend/src/admin/SuperAdminUserList.js @@ -23,7 +23,7 @@ import { } from '@chakra-ui/react' import { FIND_MY_USERS } from '../graphql/queries' -import { CLOSE_ACCOUNT, SIGN_OUT } from '../graphql/mutations' +import { CLOSE_ACCOUNT_OTHER } from '../graphql/mutations' import { LoadingMessage } from '../components/LoadingMessage' import { ErrorFallbackMessage } from '../components/ErrorFallbackMessage' import { RelayPaginationControls } from '../components/RelayPaginationControls' @@ -70,7 +70,7 @@ export function SuperAdminUserList({ permission }) { useDebouncedFunction(memoizedSetDebouncedSearchTermCallback, 500) const [closeAccount, { loading: loadingCloseAccount }] = useMutation( - CLOSE_ACCOUNT, + CLOSE_ACCOUNT_OTHER, { refetchQueries: ['FindMyUsers'], awaitRefetchQueries: true, diff --git a/frontend/src/graphql/mutations.js b/frontend/src/graphql/mutations.js index 07ab745caf..dbef8d8fd2 100644 --- a/frontend/src/graphql/mutations.js +++ b/frontend/src/graphql/mutations.js @@ -404,9 +404,25 @@ export const VERIFY_ACCOUNT = gql` } ` -export const CLOSE_ACCOUNT = gql` - mutation CloseAccount($userId: ID) { - closeAccount(input: { userId: $userId }) { +export const CLOSE_ACCOUNT_OTHER = gql` + mutation CloseAccountOther($userId: ID) { + closeAccountOther(input: { userId: $userId }) { + result { + ... on CloseAccountError { + code + description + } + ... on CloseAccountResult { + status + } + } + } + } +` + +export const CLOSE_ACCOUNT_SELF = gql` + mutation CloseAccountSelf($userId: ID) { + closeAccountSelf(input: {}) { result { ... on CloseAccountError { code diff --git a/frontend/src/user/UserPage.js b/frontend/src/user/UserPage.js index 94e2899c25..1b7dce3a61 100644 --- a/frontend/src/user/UserPage.js +++ b/frontend/src/user/UserPage.js @@ -36,7 +36,7 @@ import { LoadingMessage } from '../components/LoadingMessage' import { ErrorFallbackMessage } from '../components/ErrorFallbackMessage' import { createValidationSchema } from '../utilities/fieldRequirements' import { useUserVar } from '../utilities/userState' -import { SEND_EMAIL_VERIFICATION, CLOSE_ACCOUNT, SIGN_OUT } from '../graphql/mutations' +import { SEND_EMAIL_VERIFICATION, CLOSE_ACCOUNT_SELF, SIGN_OUT } from '../graphql/mutations' import { NotificationBanner } from '../app/NotificationBanner' import { InsideUserSwitch } from './InsideUserSwitch' import { EmailUpdatesSwitch } from './EmailUpdatesSwitch' @@ -71,7 +71,7 @@ export default function UserPage() { }, }) - const [closeAccount, { loading: loadingCloseAccount }] = useMutation(CLOSE_ACCOUNT, { + const [closeAccount, { loading: loadingCloseAccount }] = useMutation(CLOSE_ACCOUNT_SELF, { onError(error) { toast({ title: i18n._(t`An error occurred.`),