diff --git a/apps/studio/components/interfaces/Account/TOTPFactors/DeleteFactorModal.test.tsx b/apps/studio/components/interfaces/Account/TOTPFactors/DeleteFactorModal.test.tsx
new file mode 100644
index 00000000000..6d375ba3223
--- /dev/null
+++ b/apps/studio/components/interfaces/Account/TOTPFactors/DeleteFactorModal.test.tsx
@@ -0,0 +1,136 @@
+import { fireEvent, screen, waitFor } from '@testing-library/react'
+import { describe, expect, test, vi } from 'vitest'
+
+import { DeleteFactorModal } from './DeleteFactorModal'
+import { auth } from '@/lib/gotrue'
+import { customRender } from '@/tests/lib/custom-render'
+
+describe('DeleteFactorModal', () => {
+ test("Requests users confirmation before deleting an MFA when it's not the last", async () => {
+ const unenroll = vi.spyOn(auth.mfa, 'unenroll').mockResolvedValue({
+ data: {
+ id: 'some_id',
+ },
+ error: null,
+ })
+ const unenrollRecoveryCodes = vi.spyOn(auth.mfa.recoveryCodes, 'unenroll').mockResolvedValue({
+ data: {
+ id: 'some_id',
+ },
+ error: null,
+ })
+ const onClose = vi.fn()
+
+ customRender(
+
+ )
+ await screen.findByText('Confirm to delete factor')
+ expect(screen.queryByText('Multi-factor authentication will be disabled')).toBeNull()
+ expect(screen.queryByText('Your recovery codes will be deleted too')).toBeNull()
+ fireEvent.click(await screen.findByRole('button', { name: 'Delete' }))
+ await waitFor(() => expect(unenroll).toHaveBeenCalled())
+ await waitFor(() => expect(onClose).toHaveBeenCalled())
+ expect(unenrollRecoveryCodes).not.toHaveBeenCalled()
+ })
+ test("Requests users confirmation before deleting an MFA when it's the last one but no recovery codes are available", async () => {
+ const unenroll = vi.spyOn(auth.mfa, 'unenroll').mockResolvedValue({
+ data: {
+ id: 'some_id',
+ },
+ error: null,
+ })
+ const unenrollRecoveryCodes = vi.spyOn(auth.mfa.recoveryCodes, 'unenroll').mockResolvedValue({
+ data: {
+ id: 'some_id',
+ },
+ error: null,
+ })
+ const onClose = vi.fn()
+
+ customRender(
+
+ )
+ await screen.findByText('Confirm to delete factor')
+ await screen.findByText('Multi-factor authentication will be disabled')
+ expect(screen.queryByText('Your recovery codes will be deleted too')).toBeNull()
+ fireEvent.click(await screen.findByRole('button', { name: 'Delete' }))
+ await waitFor(() => expect(unenroll).toHaveBeenCalled())
+ await waitFor(() => expect(onClose).toHaveBeenCalled())
+ expect(unenrollRecoveryCodes).not.toHaveBeenCalled()
+ })
+ test('Deletes the recovery codes when available and this is the last factor', async () => {
+ const unenroll = vi.spyOn(auth.mfa, 'unenroll').mockResolvedValue({
+ data: {
+ id: 'some_id',
+ },
+ error: null,
+ })
+ const unenrollRecoveryCodes = vi.spyOn(auth.mfa.recoveryCodes, 'unenroll').mockResolvedValue({
+ data: {
+ id: 'some_id',
+ },
+ error: null,
+ })
+ const onClose = vi.fn()
+
+ customRender(
+
+ )
+ await screen.findByText('Confirm to delete factor')
+ await screen.findByText('Multi-factor authentication will be disabled')
+ await screen.findByText('Your recovery codes will be deleted too')
+ fireEvent.click(await screen.findByRole('button', { name: 'Delete' }))
+ await waitFor(() => expect(unenrollRecoveryCodes).toHaveBeenCalled())
+ await waitFor(() => expect(unenroll).toHaveBeenCalled())
+ await waitFor(() => expect(onClose).toHaveBeenCalled())
+ })
+ test('Allows users to cancel the deletion', async () => {
+ const unenroll = vi.spyOn(auth.mfa, 'unenroll').mockResolvedValue({
+ data: {
+ id: 'some_id',
+ },
+ error: null,
+ })
+ const unenrollRecoveryCodes = vi.spyOn(auth.mfa.recoveryCodes, 'unenroll').mockResolvedValue({
+ data: {
+ id: 'some_id',
+ },
+ error: null,
+ })
+ const onClose = vi.fn()
+
+ customRender(
+
+ )
+ await screen.findByText('Confirm to delete factor')
+ expect(screen.queryByText('Your recovery codes will be deleted too')).toBeNull()
+ fireEvent.click(await screen.findByRole('button', { name: 'Cancel' }))
+ await waitFor(() => expect(onClose).toHaveBeenCalled())
+ expect(unenroll).not.toHaveBeenCalled()
+ expect(unenrollRecoveryCodes).not.toHaveBeenCalled()
+ })
+})
diff --git a/apps/studio/components/interfaces/Account/TOTPFactors/DeleteFactorModal.tsx b/apps/studio/components/interfaces/Account/TOTPFactors/DeleteFactorModal.tsx
index 06f887d3af9..26f01a133b0 100644
--- a/apps/studio/components/interfaces/Account/TOTPFactors/DeleteFactorModal.tsx
+++ b/apps/studio/components/interfaces/Account/TOTPFactors/DeleteFactorModal.tsx
@@ -4,25 +4,29 @@ import ConfirmationModal from 'ui-patterns/Dialogs/ConfirmationModal'
import { organizationKeys } from '@/data/organizations/keys'
import { useMfaUnenrollMutation } from '@/data/profile/mfa-unenroll-mutation'
+import { useRecoveryCodesUnenrollMutation } from '@/data/recovery-codes/recovery-codes-unenroll'
import { useLastVisitedOrganization } from '@/hooks/misc/useLastVisitedOrganization'
interface DeleteFactorModalProps {
visible: boolean
factorId: string | null
lastFactorToBeDeleted: boolean
+ hasRecoveryCodes: boolean
onClose: () => void
}
-const DeleteFactorModal = ({
+export const DeleteFactorModal = ({
visible,
factorId,
lastFactorToBeDeleted,
+ hasRecoveryCodes,
onClose,
}: DeleteFactorModalProps) => {
const queryClient = useQueryClient()
+
const { lastVisitedOrganization } = useLastVisitedOrganization()
- const { mutate: unenroll, isPending } = useMfaUnenrollMutation({
+ const unenrollMFAMutation = useMfaUnenrollMutation({
onSuccess: async () => {
if (lastVisitedOrganization) {
await queryClient.invalidateQueries({
@@ -34,6 +38,15 @@ const DeleteFactorModal = ({
},
})
+ const unenrollRecoveryCodesMutation = useRecoveryCodesUnenrollMutation({
+ onSuccess: () => {
+ if (!factorId) return // Should never happen
+ unenrollMFAMutation.mutate({ factorId })
+ },
+ })
+
+ const loading = unenrollMFAMutation.isPending || unenrollRecoveryCodesMutation.isPending
+
return (
factorId && unenroll({ factorId })}
+ onConfirm={() => {
+ // If users have recovery codes and this is the last MFA for their account,
+ // we must first delete the recovery codes (they don't make sense without any MFA)
+ const shouldDeleteRecoveryCodes = lastFactorToBeDeleted && hasRecoveryCodes
+ if (factorId && !shouldDeleteRecoveryCodes) {
+ return unenrollMFAMutation.mutate({ factorId })
+ }
+ unenrollRecoveryCodesMutation.mutate()
+ }}
alert={{
title: lastFactorToBeDeleted
? 'Multi-factor authentication will be disabled'
@@ -63,6 +84,7 @@ const DeleteFactorModal = ({
You will lose access to any organization that enforces multi-factor authentication
+ {hasRecoveryCodes && Your recovery codes will be deleted too}
>
) : (
<>
@@ -74,5 +96,3 @@ const DeleteFactorModal = ({
)
}
-
-export default DeleteFactorModal
diff --git a/apps/studio/components/interfaces/Account/TOTPFactors/index.tsx b/apps/studio/components/interfaces/Account/TOTPFactors/index.tsx
index 801e14635d1..ea277494f14 100644
--- a/apps/studio/components/interfaces/Account/TOTPFactors/index.tsx
+++ b/apps/studio/components/interfaces/Account/TOTPFactors/index.tsx
@@ -4,6 +4,7 @@ import { Plus } from 'lucide-react'
import { useState } from 'react'
import { Button, Card, CardContent, cn } from 'ui'
import { Admonition } from 'ui-patterns/Admonition'
+import { ErrorDisplay } from 'ui-patterns/ErrorDisplay/ErrorDisplay'
import {
PageSection,
PageSectionAside,
@@ -16,14 +17,14 @@ import {
import { GenericSkeletonLoader } from 'ui-patterns/ShimmeringLoader'
import { AddNewFactorModal } from './AddNewFactorModal'
-import DeleteFactorModal from './DeleteFactorModal'
+import { DeleteFactorModal } from './DeleteFactorModal'
import { GenerateRecoveryCodesModal } from './GenerateRecoveryCodesModal'
import { RegenerateRecoveryCodesModal } from './RegenerateRecoveryCodesModal'
import { UnenrollRecoveryCodesModal } from './UnenrollRecoveryCodesModal'
import { AlertError } from '@/components/ui/AlertError'
import { useMfaListFactorsQuery } from '@/data/profile/mfa-list-factors-query'
import { useRecoveryCodesStatusQuery } from '@/data/recovery-codes/recovery-codes-status-query'
-import { DATETIME_FORMAT, IS_STAGING_OR_LOCAL } from '@/lib/constants'
+import { DATETIME_FORMAT } from '@/lib/constants'
export const TOTPFactors = () => {
const [isAddNewFactorOpen, setIsAddNewFactorOpen] = useState(false)
@@ -36,15 +37,18 @@ export const TOTPFactors = () => {
const shouldShowLockoutWarning = isSuccess && totpFactors.length === 1
const shouldVerifyRecoveryCodes = enableAuthRecoveryCodes && totpFactors.length > 0
- const { data: recoveryCodesStatus } = useRecoveryCodesStatusQuery({
+ const recoveryCodesStatusQuery = useRecoveryCodesStatusQuery({
enabled: shouldVerifyRecoveryCodes,
})
const handleAddNewApp = () => setIsAddNewFactorOpen(true)
+ // If recovery codes are enabled, we can't allow to remove an MFA until we know their status
+ const disableDeleteFactor = shouldVerifyRecoveryCodes && recoveryCodesStatusQuery.isPending
+
return (
<>
- {enableAuthRecoveryCodes && shouldVerifyRecoveryCodes && (
+ {shouldVerifyRecoveryCodes && (
@@ -56,26 +60,35 @@ export const TOTPFactors = () => {
- {recoveryCodesStatus?.status === 'unenrolled' && }
- {recoveryCodesStatus?.status === 'available' && recoveryCodesStatus?.data && (
-
-
-
- {recoveryCodesStatus.data.remaining}/{recoveryCodesStatus.data.total} recovery
- codes available
-
-
-
- {IS_STAGING_OR_LOCAL && }
-
-
-
+ {recoveryCodesStatusQuery.isError && (
+
)}
+ {recoveryCodesStatusQuery.data?.status === 'unenrolled' && (
+
+ )}
+ {recoveryCodesStatusQuery.data?.status === 'available' &&
+ recoveryCodesStatusQuery.data?.data && (
+
+
+
+ {recoveryCodesStatusQuery.data.data.remaining}/
+ {recoveryCodesStatusQuery.data.data.total} recovery codes available
+
+
+
+
+
+
+
+ )}
)}
@@ -136,7 +149,11 @@ export const TOTPFactors = () => {
Added on {dayjs(factor.created_at).format(DATETIME_FORMAT)}
-