Add password check to users.destroy and organizations.destroy

This commit is contained in:
Constantin Graf
2026-06-17 15:51:25 +02:00
committed by Constantin Graf
parent 24c94af952
commit 6a197f7f34
16 changed files with 352 additions and 57 deletions

View File

@@ -6,6 +6,7 @@ namespace App\Http\Controllers\Api\V1;
use App\Enums\Role;
use App\Events\AfterCreateOrganization;
use App\Http\Requests\V1\Organization\OrganizationDestroyRequest;
use App\Http\Requests\V1\Organization\OrganizationStoreRequest;
use App\Http\Requests\V1\Organization\OrganizationUpdateRequest;
use App\Http\Resources\V1\Organization\OrganizationResource;
@@ -124,7 +125,7 @@ class OrganizationController extends Controller
*
* @throws AuthorizationException
*/
public function destroy(Organization $organization, DeletionService $deletionService): JsonResponse
public function destroy(Organization $organization, OrganizationDestroyRequest $request, DeletionService $deletionService): JsonResponse
{
$this->checkPermission($organization, 'organizations:delete');

View File

@@ -6,6 +6,7 @@ namespace App\Http\Controllers\Api\V1;
use App\Exceptions\Api\CanNotDeleteUserWhoIsOwnerOfOrganizationWithMultipleMembers;
use App\Exceptions\Api\UserResendEmailVerificationNoPendingEmailApiException;
use App\Http\Requests\V1\User\UserDestroyRequest;
use App\Http\Requests\V1\User\UserUpdateCurrentOrganizationRequest;
use App\Http\Requests\V1\User\UserUpdateRequest;
use App\Http\Resources\V1\User\UserResource;
@@ -193,7 +194,7 @@ class UserController extends Controller
* @throws AuthorizationException Thrown when the authenticated user does not match the user to be deleted.
* @throws CanNotDeleteUserWhoIsOwnerOfOrganizationWithMultipleMembers Thrown when the user to be deleted is the owner of an organization with multiple members.
*/
public function destroy(User $user, DeletionService $deletionService): JsonResponse
public function destroy(User $user, UserDestroyRequest $request, DeletionService $deletionService): JsonResponse
{
if ($user->getKey() !== $this->user()->getKey()) {
throw new AuthorizationException;

View File

@@ -0,0 +1,48 @@
<?php
declare(strict_types=1);
namespace App\Http\Requests\V1\Organization;
use App\Http\Requests\V1\BaseFormRequest;
use Illuminate\Support\Facades\Hash;
use Illuminate\Validation\Validator;
class OrganizationDestroyRequest extends BaseFormRequest
{
/**
* Get the validation rules that apply to the request.
*
* @return array<string, array<string>>
*/
public function rules(): array
{
return [
'password' => [
'required',
'string',
],
];
}
/**
* @return array<int, callable(Validator): void>
*/
public function after(): array
{
return [
function (Validator $validator): void {
if ($validator->errors()->has('password')) {
return;
}
$user = $this->user();
$password = $this->input('password');
if (! is_string($password) || $user === null || ! Hash::check($password, (string) $user->password)) {
$validator->errors()->add('password', __('The password is incorrect.'));
}
},
];
}
}

View File

@@ -0,0 +1,48 @@
<?php
declare(strict_types=1);
namespace App\Http\Requests\V1\User;
use App\Http\Requests\V1\BaseFormRequest;
use Illuminate\Support\Facades\Hash;
use Illuminate\Validation\Validator;
class UserDestroyRequest extends BaseFormRequest
{
/**
* Get the validation rules that apply to the request.
*
* @return array<string, array<string>>
*/
public function rules(): array
{
return [
'password' => [
'required',
'string',
],
];
}
/**
* @return array<int, callable(Validator): void>
*/
public function after(): array
{
return [
function (Validator $validator): void {
if ($validator->errors()->has('password')) {
return;
}
$user = $this->user();
$password = $this->input('password');
if (! is_string($password) || $user === null || ! Hash::check($password, (string) $user->password)) {
$validator->errors()->add('password', __('The password is incorrect.'));
}
},
];
}
}

View File

@@ -107,7 +107,7 @@ services:
- sail
- reverse-proxy
playwright:
image: mcr.microsoft.com/playwright:v1.59.1-jammy
image: mcr.microsoft.com/playwright:v1.60.0-jammy
command: ['npx', 'playwright', 'test', '--ui-port=8080', '--ui-host=0.0.0.0']
working_dir: /src
extra_hosts:

View File

@@ -1,5 +1,5 @@
import { expect, test } from '../playwright/fixtures';
import { PLAYWRIGHT_BASE_URL } from '../playwright/config';
import { PLAYWRIGHT_BASE_URL, TEST_USER_PASSWORD } from '../playwright/config';
async function goToOrganizationSettings(page) {
await page.goto(PLAYWRIGHT_BASE_URL + '/dashboard');
@@ -471,6 +471,7 @@ test.describe('Organization Create, Delete & Switch', () => {
// Open the confirmation modal, then confirm inside the dialog.
await page.getByRole('button', { name: 'Delete Organization' }).click();
await page.getByRole('dialog').getByPlaceholder('Password').fill(TEST_USER_PASSWORD);
await Promise.all([
page.waitForResponse(
(response) =>
@@ -488,6 +489,28 @@ test.describe('Organization Create, Delete & Switch', () => {
).not.toContainText(orgName);
});
test('delete organization shows an error when the password is wrong', async ({ page }) => {
const orgName = 'DeleteOrgWrongPassword' + Math.floor(Math.random() * 100000);
await createOrganization(page, orgName);
await goToOrganizationSettings(page);
await page.getByRole('button', { name: 'Delete Organization' }).click();
const dialog = page.getByRole('dialog');
await dialog.getByPlaceholder('Password').fill('not-the-real-password');
await Promise.all([
page.waitForResponse(
(response) =>
response.url().includes('/api/v1/organizations') &&
response.request().method() === 'DELETE' &&
response.status() === 422
),
dialog.getByRole('button', { name: 'Delete Organization' }).click(),
]);
await expect(dialog.getByRole('alert')).toBeVisible();
await expect(dialog).toBeVisible();
});
test('can switch the current organization via the organization switcher', async ({ page }) => {
await page.goto(PLAYWRIGHT_BASE_URL + '/dashboard');
const orgSwitcher = page.locator('[data-testid="organization_switcher"]:visible');

View File

@@ -342,8 +342,8 @@ test('delete account shows an error when the password is wrong', async ({ page }
await Promise.all([
page.waitForResponse(
(response) =>
response.url().includes('/user/confirm-password') &&
response.request().method() === 'POST' &&
response.url().includes('/api/v1/users/') &&
response.request().method() === 'DELETE' &&
response.status() === 422
),
dialog.getByRole('button', { name: 'Delete Account' }).click(),

View File

@@ -1,15 +1,14 @@
<script setup lang="ts">
import { ref } from 'vue';
import axios from 'axios';
import ActionSection from '@/Components/ActionSection.vue';
import DangerButton from '@/packages/ui/src/Buttons/DangerButton.vue';
import DialogModal from '@/packages/ui/src/DialogModal.vue';
import { Field, FieldError } from '@/packages/ui/src/field';
import SecondaryButton from '@/packages/ui/src/Buttons/SecondaryButton.vue';
import TextInput from '@/packages/ui/src/Input/TextInput.vue';
import { useDeleteUserMutation, useUserQuery } from '@/utils/useUserQuery';
import { useDeleteUserMutation } from '@/utils/useUserQuery';
import { getCurrentUserId } from '@/utils/useUser';
const { user } = useUserQuery();
const deleteUserMutation = useDeleteUserMutation();
const confirmingUserDeletion = ref(false);
@@ -24,26 +23,26 @@ function confirmUserDeletion() {
}
async function deleteUser() {
if (!user.value || processing.value) return;
if (processing.value) return;
processing.value = true;
passwordError.value = '';
try {
await axios.post(route('password.confirm'), { password: password.value });
} catch (error) {
processing.value = false;
if (axios.isAxiosError(error) && error.response?.status === 422) {
passwordError.value = error.response.data?.errors?.password?.[0] ?? 'Invalid password.';
} else {
passwordError.value = 'Could not confirm password. Please try again.';
}
passwordInput.value?.focus();
return;
}
try {
await deleteUserMutation.mutateAsync(user.value.id);
await deleteUserMutation.mutateAsync({
userId: getCurrentUserId(),
body: { password: password.value },
});
window.location.href = '/';
} catch {
} catch (error) {
if (error && typeof error === 'object' && 'response' in error) {
const response = error.response as
| { status?: number; data?: { errors?: { password?: string[] } } }
| undefined;
if (response?.status === 422) {
passwordError.value = response.data?.errors?.password?.[0] ?? 'Invalid password.';
}
}
processing.value = false;
passwordInput.value?.focus();
}
}

View File

@@ -2,9 +2,11 @@
import { ref } from 'vue';
import { router } from '@inertiajs/vue3';
import ActionSection from '@/Components/ActionSection.vue';
import ConfirmationModal from '@/Components/ConfirmationModal.vue';
import DangerButton from '@/packages/ui/src/Buttons/DangerButton.vue';
import DialogModal from '@/packages/ui/src/DialogModal.vue';
import { Field, FieldError } from '@/packages/ui/src/field';
import SecondaryButton from '@/packages/ui/src/Buttons/SecondaryButton.vue';
import TextInput from '@/packages/ui/src/Input/TextInput.vue';
import { useOrganizationStore } from '@/utils/useOrganization';
const props = defineProps<{
@@ -12,26 +14,46 @@ const props = defineProps<{
}>();
const confirmingTeamDeletion = ref(false);
const passwordInput = ref<HTMLInputElement | null>(null);
const password = ref('');
const passwordError = ref('');
const processing = ref(false);
const organizationStore = useOrganizationStore();
const confirmTeamDeletion = () => {
confirmingTeamDeletion.value = true;
setTimeout(() => passwordInput.value?.focus(), 250);
};
const deleteTeam = async () => {
if (processing.value) return;
processing.value = true;
passwordError.value = '';
try {
await organizationStore.deleteOrganization(props.team.id);
await organizationStore.deleteOrganization(props.team.id, { password: password.value });
// The backend reassigns the user's current organization after deletion,
// so flush the prefetch cache and reload into the dashboard.
router.flushAll();
router.visit(route('dashboard'));
} catch {
// Request errors are surfaced as notifications by the store.
} catch (error) {
if (error && typeof error === 'object' && 'response' in error) {
const response = error.response as
| { status?: number; data?: { errors?: { password?: string[] } } }
| undefined;
if (response?.status === 422) {
passwordError.value = response.data?.errors?.password?.[0] ?? 'Invalid password.';
}
}
processing.value = false;
passwordInput.value?.focus();
}
};
const closeModal = () => {
confirmingTeamDeletion.value = false;
password.value = '';
passwordError.value = '';
};
</script>
<template>
@@ -52,20 +74,30 @@ const deleteTeam = async () => {
</div>
<!-- Delete Organization Confirmation Modal -->
<ConfirmationModal
:show="confirmingTeamDeletion"
@close="confirmingTeamDeletion = false">
<DialogModal :show="confirmingTeamDeletion" @close="closeModal">
<template #title> Delete Organization </template>
<template #content>
Are you sure you want to delete this organization? Once a organization is
deleted, all of its resources and data will be permanently deleted.
deleted, all of its resources and data will be permanently deleted. Please enter
your password to confirm you would like to permanently delete this organization.
<Field class="mt-4">
<TextInput
ref="passwordInput"
v-model="password"
type="password"
class="block w-3/4"
placeholder="Password"
autocomplete="current-password"
@keyup.enter="deleteTeam" />
<FieldError v-if="passwordError">{{ passwordError }}</FieldError>
</Field>
</template>
<template #footer>
<SecondaryButton @click="confirmingTeamDeletion = false">
Cancel
</SecondaryButton>
<SecondaryButton @click="closeModal"> Cancel </SecondaryButton>
<DangerButton
class="ms-3"
@@ -75,7 +107,7 @@ const deleteTeam = async () => {
Delete Organization
</DangerButton>
</template>
</ConfirmationModal>
</DialogModal>
</template>
</ActionSection>
</template>

View File

@@ -126,6 +126,8 @@ export type UpdateInvoiceBody = ZodiosBodyByAlias<SolidTimeApi, 'updateInvoice'>
export type User = ZodiosResponseByAlias<SolidTimeApi, 'getMe'>['data'];
export type UpdateUserBody = ZodiosBodyByAlias<SolidTimeApi, 'updateUser'>;
export type DeleteUserBody = ZodiosBodyByAlias<SolidTimeApi, 'deleteUser'>;
export type DeleteOrganizationBody = ZodiosBodyByAlias<SolidTimeApi, 'deleteOrganization'>;
const api = createApiClient('/api', { validate: 'none' });

View File

@@ -37,6 +37,7 @@ const ClientStoreRequest = z.object({ name: z.string().min(1).max(255) }).passth
const ClientUpdateRequest = z
.object({ name: z.string().min(1).max(255), is_archived: z.boolean().optional() })
.passthrough();
const DestroyWithPasswordRequest = z.object({ password: z.string() }).passthrough();
const ImportRequest = z.object({ type: z.string(), data: z.string() }).passthrough();
const InvitationResource = z
.object({ id: z.string(), email: z.string(), role: z.string() })
@@ -917,6 +918,11 @@ const endpoints = makeApi([
alias: 'deleteOrganization',
requestFormat: 'json',
parameters: [
{
name: 'body',
type: 'Body',
schema: DestroyWithPasswordRequest,
},
{
name: 'organization',
type: 'Path',
@@ -4642,6 +4648,11 @@ the organization.`,
description: `This endpoint is independent of the organization.`,
requestFormat: 'json',
parameters: [
{
name: 'body',
type: 'Body',
schema: DestroyWithPasswordRequest,
},
{
name: 'user',
type: 'Path',

View File

@@ -2,9 +2,11 @@ import { router } from '@inertiajs/vue3';
import { initializeStores } from '@/utils/init';
import { defineStore } from 'pinia';
import { computed, ref } from 'vue';
import axios from 'axios';
import type {
Organization,
OrganizationResponse,
DeleteOrganizationBody,
UpdateOrganizationBody,
} from '@/packages/api/src';
import { useNotificationsStore } from '@/utils/notification';
@@ -38,7 +40,7 @@ export async function switchOrganization(organizationId: string) {
export const useOrganizationStore = defineStore('organization', () => {
const organizationResponse = ref<OrganizationResponse | null>(null);
const { handleApiRequestNotifications } = useNotificationsStore();
const { addNotification, handleApiRequestNotifications } = useNotificationsStore();
async function fetchOrganization() {
const organization = getCurrentOrganizationId();
@@ -78,17 +80,26 @@ export const useOrganizationStore = defineStore('organization', () => {
return response?.data ?? null;
}
async function deleteOrganization(organizationId: string) {
await handleApiRequestNotifications(
() =>
api.deleteOrganization(undefined, {
params: {
organization: organizationId,
},
}),
'Organization deleted successfully',
'Failed to delete organization'
);
async function deleteOrganization(organizationId: string, body: DeleteOrganizationBody) {
try {
await api.deleteOrganization(body, {
params: {
organization: organizationId,
},
});
addNotification('success', 'Organization deleted successfully');
} catch (error) {
if (!axios.isAxiosError(error) || error.response?.status !== 422) {
addNotification(
'error',
'Failed to delete organization',
axios.isAxiosError(error)
? (error.response?.data?.message ?? 'Please try again later.')
: 'Please try again later.'
);
}
throw error;
}
}
const organization = computed<Organization | null>(() => {

View File

@@ -1,7 +1,7 @@
import { useMutation, useQuery, useQueryClient } from '@tanstack/vue-query';
import { computed } from 'vue';
import axios from 'axios';
import { api, type UpdateUserBody, type User } from '@/packages/api/src';
import { api, type DeleteUserBody, type UpdateUserBody, type User } from '@/packages/api/src';
import { useNotificationsStore } from '@/utils/notification';
const ME_QUERY_KEY = ['me'] as const;
@@ -61,9 +61,9 @@ export function useDeleteUserMutation() {
const { addNotification } = useNotificationsStore();
return useMutation({
mutationFn: async (userId: string) => {
mutationFn: async ({ userId, body }: { userId: string; body: DeleteUserBody }) => {
try {
await api.deleteUser(undefined, { params: { user: userId } });
await api.deleteUser(body, { params: { user: userId } });
} catch (error) {
if (!axios.isAxiosError(error) || error.response?.status !== 422) {
addNotification(

View File

@@ -441,7 +441,9 @@ class OrganizationEndpointTest extends ApiEndpointTestAbstract
Passport::actingAs($data->user);
// Act
$response = $this->deleteJson(route('api.v1.organizations.destroy', [$data->organization->getKey()]));
$response = $this->deleteJson(route('api.v1.organizations.destroy', [$data->organization->getKey()]), [
'password' => 'password',
]);
// Assert
$response->assertForbidden();
@@ -456,12 +458,54 @@ class OrganizationEndpointTest extends ApiEndpointTestAbstract
Passport::actingAs($data->user);
// Act
$response = $this->deleteJson(route('api.v1.organizations.destroy', ['not-uuid']));
$response = $this->deleteJson(route('api.v1.organizations.destroy', ['not-uuid']), [
'password' => 'password',
]);
// Assert
$response->assertNotFound();
}
public function test_delete_endpoint_fails_without_password(): void
{
// Arrange
$data = $this->createUserWithPermission([
'organizations:delete',
]);
Passport::actingAs($data->user);
// Act
$response = $this->deleteJson(route('api.v1.organizations.destroy', [$data->organization->getKey()]));
// Assert
$response->assertUnprocessable();
$response->assertJsonValidationErrors(['password']);
$this->assertDatabaseHas(Organization::class, [
'id' => $data->organization->getKey(),
]);
}
public function test_delete_endpoint_fails_with_wrong_password(): void
{
// Arrange
$data = $this->createUserWithPermission([
'organizations:delete',
]);
Passport::actingAs($data->user);
// Act
$response = $this->deleteJson(route('api.v1.organizations.destroy', [$data->organization->getKey()]), [
'password' => 'wrong-password',
]);
// Assert
$response->assertUnprocessable();
$response->assertJsonValidationErrors(['password']);
$this->assertDatabaseHas(Organization::class, [
'id' => $data->organization->getKey(),
]);
}
public function test_delete_endpoint_can_delete_organization(): void
{
// Arrange
@@ -472,7 +516,9 @@ class OrganizationEndpointTest extends ApiEndpointTestAbstract
Passport::actingAs($data->user);
// Act
$response = $this->deleteJson(route('api.v1.organizations.destroy', [$data->organization->getKey()]));
$response = $this->deleteJson(route('api.v1.organizations.destroy', [$data->organization->getKey()]), [
'password' => 'password',
]);
// Assert
$response->assertNoContent();

View File

@@ -649,7 +649,9 @@ class UserEndpointTest extends ApiEndpointTestAbstract
Passport::actingAs($otherData->user);
// Act
$response = $this->deleteJson(route('api.v1.users.destroy', $data->user->getKey()));
$response = $this->deleteJson(route('api.v1.users.destroy', $data->user->getKey()), [
'password' => 'password',
]);
// Assert
$response->assertForbidden();
@@ -674,13 +676,15 @@ class UserEndpointTest extends ApiEndpointTestAbstract
Passport::actingAs($data->user);
// Act
$response = $this->deleteJson(route('api.v1.users.destroy', 'not-valid'));
$response = $this->deleteJson(route('api.v1.users.destroy', 'not-valid'), [
'password' => 'password',
]);
// Assert
$response->assertNotFound();
}
public function test_delete_removes_user(): void
public function test_delete_fails_without_password(): void
{
// Arrange
$data = $this->createUserWithPermission();
@@ -689,6 +693,40 @@ class UserEndpointTest extends ApiEndpointTestAbstract
// Act
$response = $this->deleteJson(route('api.v1.users.destroy', $data->user->getKey()));
// Assert
$response->assertUnprocessable();
$response->assertJsonValidationErrors(['password']);
$this->assertDatabaseHas(User::class, ['id' => $data->user->getKey()]);
}
public function test_delete_fails_with_wrong_password(): void
{
// Arrange
$data = $this->createUserWithPermission();
Passport::actingAs($data->user);
// Act
$response = $this->deleteJson(route('api.v1.users.destroy', $data->user->getKey()), [
'password' => 'wrong-password',
]);
// Assert
$response->assertUnprocessable();
$response->assertJsonValidationErrors(['password']);
$this->assertDatabaseHas(User::class, ['id' => $data->user->getKey()]);
}
public function test_delete_removes_user(): void
{
// Arrange
$data = $this->createUserWithPermission();
Passport::actingAs($data->user);
// Act
$response = $this->deleteJson(route('api.v1.users.destroy', $data->user->getKey()), [
'password' => 'password',
]);
// Assert
$response->assertNoContent();
$this->assertDatabaseMissing(User::class, ['id' => $data->user->getKey()]);

View File

@@ -4,12 +4,17 @@ declare(strict_types=1);
namespace Tests\Unit\Filament\Resources;
use App\Enums\Role;
use App\Events\OrganizationInvitationAdding;
use App\Filament\Resources\OrganizationResource;
use App\Mail\OrganizationInvitationMail;
use App\Models\Organization;
use App\Models\OrganizationInvitation;
use App\Models\User;
use App\Service\DeletionService;
use Illuminate\Support\Facades\Config;
use Illuminate\Support\Facades\Event;
use Illuminate\Support\Facades\Mail;
use Livewire\Livewire;
use Mockery\MockInterface;
use PHPUnit\Framework\Attributes\UsesClass;
@@ -112,4 +117,34 @@ class OrganizationResourceTest extends FilamentTestCase
$response->assertSuccessful();
$response->assertCanSeeTableRecords($organizationInvitations);
}
public function test_can_create_related_invitation(): void
{
// Arrange
Event::fake([
OrganizationInvitationAdding::class,
]);
Mail::fake();
$organization = Organization::factory()->create();
// Act
$response = Livewire::test(OrganizationResource\RelationManagers\InvitationsRelationManager::class, [
'ownerRecord' => $organization,
'pageClass' => OrganizationResource\Pages\EditOrganization::class,
])->callTableAction('create', data: [
'email' => 'new-user@example.com',
'role' => Role::Employee->value,
]);
// Assert
$response->assertSuccessful();
$response->assertHasNoTableActionErrors();
$this->assertDatabaseHas(OrganizationInvitation::class, [
'organization_id' => $organization->getKey(),
'email' => 'new-user@example.com',
'role' => Role::Employee->value,
]);
Event::assertDispatched(OrganizationInvitationAdding::class);
Mail::assertQueued(OrganizationInvitationMail::class);
}
}