mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-17 00:55:07 +00:00
7c7ba7cd53
Filling the "Storage quota (MB)" field on the new-client form raised a TypeError and the request died with a 500. Leaving it blank worked, which is why it reached a release: that path goes through `null ?? 0`, and the 0 is an int. The `integer` validation rule checks that a value looks like an integer. It does not convert it. `$request->validate()` returns the raw input, so the form field arrives as the string "2048" -- and the create form types that field as a string in React, so it is a string even over JSON. Both controllers declare strict_types, so handing it to `ClientAccounts::create()`'s `int $storageQuotaMb` is a TypeError. Fixed on both surfaces that call create(): the staff screen and /api/v1/clients. The API twin had the same defect, reachable by sending the quota as a quoted JSON value or a form-encoded body -- its own create test only ever sent a JSON number. Two more call sites had the same shape and are cast too, though nothing sends them a string today: the share-link download cap and a comment's reply_to. Both are safe only because a frontend file happens to call Number() first, which is a fact about that file rather than anything the signature guarantees. The null in each is preserved rather than collapsed to 0 -- "no cap" is not a cap of zero. `storage_quota_mb` is also cast on User and Invitation. The column is an unsignedInteger and both docblocks already promise int; it is read straight into provision()'s typed parameter when an invitation is redeemed, and which type a driver hands back is not something that call site should depend on. Found on the new files-test rehearsal instance, on its first real use, against the same build the whole fleet is running.
419 lines
16 KiB
PHP
419 lines
16 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
use App\Models\User;
|
|
use App\Modules\Audit\Action;
|
|
use App\Modules\Audit\ActivityLog;
|
|
use App\Modules\Clients\ClientCustomFieldType;
|
|
use App\Modules\Clients\Models\ClientCustomField;
|
|
use App\Modules\Clients\Models\ClientCustomFieldValue;
|
|
use App\Modules\Files\Models\File;
|
|
use App\Modules\Identity\Models\Role;
|
|
use App\Modules\Identity\Models\RolePermission;
|
|
use App\Modules\Identity\Permissions\Permission;
|
|
use Illuminate\Support\Facades\Storage;
|
|
|
|
beforeEach(function () {
|
|
Storage::fake('files');
|
|
$this->admin = User::factory()->create();
|
|
$this->token = $this->admin->createToken('t', [
|
|
Permission::ManageClients->value,
|
|
Permission::CreateClients->value,
|
|
Permission::EditClients->value,
|
|
Permission::DeleteClients->value,
|
|
])->plainTextToken;
|
|
});
|
|
|
|
/*
|
|
|--------------------------------------------------------------------------
|
|
| Privacy
|
|
|--------------------------------------------------------------------------
|
|
|
|
|
| `users` is the most sensitive table here. These assert on the raw
|
|
| response body rather than on parsed fields, so a leak through a nested
|
|
| relation or a future column is caught too.
|
|
|
|
|
*/
|
|
|
|
test('no client response carries credentials', function () {
|
|
$client = User::factory()->client()->create();
|
|
|
|
$bodies = [
|
|
$this->withToken($this->token)->getJson('/api/v1/clients')->getContent(),
|
|
$this->withToken($this->token)->getJson("/api/v1/clients/{$client->id}")->getContent(),
|
|
];
|
|
|
|
foreach ($bodies as $body) {
|
|
foreach (['password', 'two_factor_secret', 'two_factor_recovery_codes', 'remember_token'] as $forbidden) {
|
|
expect($body)->not->toContain($forbidden);
|
|
}
|
|
}
|
|
});
|
|
|
|
test('a staff account is not reachable through the clients surface', function () {
|
|
$staff = User::factory()->create();
|
|
|
|
$this->withToken($this->token)->getJson("/api/v1/clients/{$staff->id}")->assertNotFound();
|
|
$this->withToken($this->token)->patchJson("/api/v1/clients/{$staff->id}", ['name' => 'x'])->assertNotFound();
|
|
$this->withToken($this->token)->deleteJson("/api/v1/clients/{$staff->id}")->assertNotFound();
|
|
|
|
expect(User::query()->find($staff->id))->not->toBeNull();
|
|
});
|
|
|
|
test('the listing does not hand out every clients custom field data', function () {
|
|
$field = ClientCustomField::query()->create([
|
|
'name' => 'vat', 'label' => 'VAT number', 'type' => ClientCustomFieldType::Text,
|
|
'required' => false, 'sort_order' => 1,
|
|
]);
|
|
$client = User::factory()->client()->create();
|
|
ClientCustomFieldValue::query()->create([
|
|
'client_custom_field_id' => $field->id, 'user_id' => $client->id, 'value' => 'SECRET-VAT',
|
|
]);
|
|
|
|
expect($this->withToken($this->token)->getJson('/api/v1/clients')->getContent())
|
|
->not->toContain('SECRET-VAT');
|
|
|
|
// But reading one client does include them — the same data the edit
|
|
// screen shows to anyone with edit_clients.
|
|
$this->withToken($this->token)->getJson("/api/v1/clients/{$client->id}")
|
|
->assertOk()
|
|
->assertJsonPath('data.custom_fields.0.value', 'SECRET-VAT');
|
|
});
|
|
|
|
/*
|
|
|--------------------------------------------------------------------------
|
|
| CRUD
|
|
|--------------------------------------------------------------------------
|
|
*/
|
|
|
|
test('a PATCH leaves custom fields it does not name alone', function () {
|
|
// The same rule the rest of update() states: "an absent key means
|
|
// 'leave alone', not 'clear'". The custom-field pass was create()'s,
|
|
// which writes every field there is.
|
|
$name = ClientCustomField::query()->create([
|
|
'name' => 'contact', 'label' => 'Contact', 'type' => ClientCustomFieldType::Text,
|
|
'required' => false, 'sort_order' => 1,
|
|
]);
|
|
$vat = ClientCustomField::query()->create([
|
|
'name' => 'vat', 'label' => 'VAT number', 'type' => ClientCustomFieldType::Text,
|
|
'required' => false, 'sort_order' => 2,
|
|
]);
|
|
|
|
$client = User::factory()->client()->create();
|
|
ClientCustomFieldValue::query()->create([
|
|
'client_custom_field_id' => $name->id, 'user_id' => $client->id, 'value' => 'Alex',
|
|
]);
|
|
ClientCustomFieldValue::query()->create([
|
|
'client_custom_field_id' => $vat->id, 'user_id' => $client->id, 'value' => 'ATU12345678',
|
|
]);
|
|
|
|
$this->withToken($this->token)->patchJson("/api/v1/clients/{$client->id}", [
|
|
'custom_field_values' => [$name->id => 'Robin'],
|
|
])->assertOk();
|
|
|
|
$values = ClientCustomFieldValue::query()->where('user_id', $client->id)
|
|
->pluck('value', 'client_custom_field_id');
|
|
|
|
expect($values[$name->id])->toBe('Robin')
|
|
->and($values[$vat->id])->toBe('ATU12345678');
|
|
});
|
|
|
|
test('a PATCH can still clear a field by naming it', function () {
|
|
$vat = ClientCustomField::query()->create([
|
|
'name' => 'vat', 'label' => 'VAT number', 'type' => ClientCustomFieldType::Text,
|
|
'required' => false, 'sort_order' => 1,
|
|
]);
|
|
$client = User::factory()->client()->create();
|
|
ClientCustomFieldValue::query()->create([
|
|
'client_custom_field_id' => $vat->id, 'user_id' => $client->id, 'value' => 'ATU12345678',
|
|
]);
|
|
|
|
$this->withToken($this->token)->patchJson("/api/v1/clients/{$client->id}", [
|
|
'custom_field_values' => [$vat->id => ''],
|
|
])->assertOk();
|
|
|
|
expect(ClientCustomFieldValue::query()
|
|
->where('user_id', $client->id)->where('client_custom_field_id', $vat->id)->value('value'))->toBeNull();
|
|
});
|
|
|
|
test('creating a client still records every field', function () {
|
|
// create() is not a partial update, and a checkbox nobody ticked is a
|
|
// recorded "no" rather than an absent row.
|
|
$optIn = ClientCustomField::query()->create([
|
|
'name' => 'newsletter', 'label' => 'Newsletter', 'type' => ClientCustomFieldType::Checkbox,
|
|
'required' => false, 'sort_order' => 1,
|
|
]);
|
|
|
|
$this->withToken($this->token)->postJson('/api/v1/clients', [
|
|
'name' => 'Acme Ltd',
|
|
'email' => 'crud-custom@example.com',
|
|
'password' => 'super-secret-password',
|
|
])->assertCreated();
|
|
|
|
$client = User::query()->where('email', 'crud-custom@example.com')->sole();
|
|
|
|
expect(ClientCustomFieldValue::query()
|
|
->where('user_id', $client->id)->where('client_custom_field_id', $optIn->id)->value('value'))->toBe('0');
|
|
});
|
|
|
|
test('a client can be created with the quota sent as a string', function () {
|
|
// Same defect as the staff screen: the `integer` rule accepts "2048"
|
|
// and hands it on unconverted, into a strict_types call expecting an
|
|
// int. A JSON number was always fine, which is why the API's own
|
|
// create test did not see it -- so this sends the quoted form a
|
|
// form-encoded caller or a cautious JSON serialiser would.
|
|
$this->withToken($this->token)->postJson('/api/v1/clients', [
|
|
'name' => 'Acme Ltd',
|
|
'email' => 'string-quota@acme.test',
|
|
'password' => 'a-sufficiently-long-password',
|
|
'storage_quota_mb' => '2048',
|
|
])->assertStatus(201);
|
|
|
|
expect(User::query()->where('email', 'string-quota@acme.test')->sole()->storage_quota_mb)->toBe(2048);
|
|
});
|
|
|
|
test('a client can be created', function () {
|
|
$this->withToken($this->token)->postJson('/api/v1/clients', [
|
|
'name' => 'Acme Ltd',
|
|
'email' => 'billing@acme.test',
|
|
'password' => 'a-sufficiently-long-password',
|
|
])->assertStatus(201)->assertJsonPath('data.email', 'billing@acme.test');
|
|
|
|
$client = User::query()->where('email', 'billing@acme.test')->firstOrFail();
|
|
|
|
expect($client->isClient())->toBeTrue()
|
|
->and($client->active)->toBeTrue()
|
|
->and(ActivityLog::query()->where('action', Action::UserCreated)->exists())->toBeTrue();
|
|
});
|
|
|
|
test('a client-scoped token keeps the client it creates', function () {
|
|
// The API twin of the same rule: without the roster entry, the token's
|
|
// owner gets a 404 from every route that binds the client they just
|
|
// created, including show and update.
|
|
$role = Role::query()->create(['name' => 'Scoped creator', 'client_scoped' => true]);
|
|
RolePermission::query()->insert([
|
|
['role_id' => $role->id, 'permission' => Permission::ManageClients->value],
|
|
['role_id' => $role->id, 'permission' => Permission::CreateClients->value],
|
|
['role_id' => $role->id, 'permission' => Permission::EditClients->value],
|
|
]);
|
|
$creator = User::factory()->create(['role_id' => $role->id]);
|
|
$token = $creator->createToken('t', [
|
|
Permission::ManageClients->value,
|
|
Permission::CreateClients->value,
|
|
Permission::EditClients->value,
|
|
])->plainTextToken;
|
|
|
|
$id = $this->withToken($token)->postJson('/api/v1/clients', [
|
|
'name' => 'Brand New',
|
|
'email' => 'api-brand-new@example.com',
|
|
'password' => 'super-secret-password',
|
|
])->assertCreated()->json('data.id');
|
|
|
|
$this->withToken($token)->getJson("/api/v1/clients/{$id}")->assertOk();
|
|
});
|
|
|
|
test('a required custom field is enforced on create', function () {
|
|
ClientCustomField::query()->create([
|
|
'name' => 'vat', 'label' => 'VAT number', 'type' => ClientCustomFieldType::Text,
|
|
'required' => true, 'sort_order' => 1,
|
|
]);
|
|
|
|
$this->withToken($this->token)->postJson('/api/v1/clients', [
|
|
'name' => 'No VAT',
|
|
'email' => 'novat@acme.test',
|
|
'password' => 'a-sufficiently-long-password',
|
|
])->assertStatus(422);
|
|
|
|
expect(User::query()->where('email', 'novat@acme.test')->exists())->toBeFalse();
|
|
});
|
|
|
|
test('a weak password is refused', function () {
|
|
$this->withToken($this->token)->postJson('/api/v1/clients', [
|
|
'name' => 'Weak',
|
|
'email' => 'weak@acme.test',
|
|
'password' => 'short',
|
|
])->assertStatus(422);
|
|
});
|
|
|
|
test('update changes only the fields sent', function () {
|
|
$client = User::factory()->client()->create(['name' => 'Before', 'storage_quota_mb' => 500]);
|
|
|
|
$this->withToken($this->token)->patchJson("/api/v1/clients/{$client->id}", ['name' => 'After'])
|
|
->assertOk()
|
|
->assertJsonPath('data.name', 'After');
|
|
|
|
$client->refresh();
|
|
expect($client->name)->toBe('After')
|
|
// Untouched by a PATCH that did not mention it — unlike the web
|
|
// form, which always submits every field.
|
|
->and($client->storage_quota_mb)->toBe(500);
|
|
});
|
|
|
|
test('deactivating a client is recorded', function () {
|
|
$client = User::factory()->client()->create(['active' => true]);
|
|
|
|
$this->withToken($this->token)->patchJson("/api/v1/clients/{$client->id}", ['active' => false])->assertOk();
|
|
|
|
expect($client->refresh()->active)->toBeFalse()
|
|
->and(ActivityLog::query()->where('action', Action::UserDeactivated)->exists())->toBeTrue();
|
|
});
|
|
|
|
/*
|
|
|--------------------------------------------------------------------------
|
|
| Deletion
|
|
|--------------------------------------------------------------------------
|
|
*/
|
|
|
|
test('a client with no content deletes without ceremony', function () {
|
|
$client = User::factory()->client()->create();
|
|
|
|
$this->withToken($this->token)->deleteJson("/api/v1/clients/{$client->id}")->assertNoContent();
|
|
|
|
expect(User::query()->find($client->id))->toBeNull();
|
|
});
|
|
|
|
test('deleting a client that owns files demands an explicit decision', function () {
|
|
$client = User::factory()->client()->create();
|
|
File::factory()->create(['uploaded_by' => $client->id]);
|
|
|
|
// No default is possible: one would silently destroy the files, the
|
|
// other would silently transfer them to somebody else.
|
|
$this->withToken($this->token)->deleteJson("/api/v1/clients/{$client->id}")
|
|
->assertStatus(422)
|
|
->assertJsonPath('type', 'validation_failed');
|
|
|
|
expect(User::query()->find($client->id))->not->toBeNull();
|
|
});
|
|
|
|
test('cascade_delete removes the clients content', function () {
|
|
$client = User::factory()->client()->create();
|
|
$file = File::factory()->create(['uploaded_by' => $client->id]);
|
|
|
|
$this->withToken($this->token)
|
|
->deleteJson("/api/v1/clients/{$client->id}", ['content_action' => 'cascade_delete'])
|
|
->assertNoContent();
|
|
|
|
expect(User::query()->find($client->id))->toBeNull()
|
|
->and(File::query()->find($file->id))->toBeNull();
|
|
});
|
|
|
|
test('reassign moves the content to the named account', function () {
|
|
$client = User::factory()->client()->create();
|
|
$file = File::factory()->create(['uploaded_by' => $client->id]);
|
|
|
|
$this->withToken($this->token)->deleteJson("/api/v1/clients/{$client->id}", [
|
|
'content_action' => 'reassign',
|
|
'reassign_to_id' => $this->admin->id,
|
|
])->assertNoContent();
|
|
|
|
expect(File::query()->find($file->id)?->uploaded_by)->toBe($this->admin->id);
|
|
});
|
|
|
|
test('a failure while disposing of a deleted client\'s content rolls the deletion back', function () {
|
|
$client = User::factory()->client()->create();
|
|
|
|
failAccountContentDisposal();
|
|
|
|
$this->withToken($this->token)->deleteJson("/api/v1/clients/{$client->id}", [
|
|
'content_action' => 'reassign',
|
|
'reassign_to_id' => $this->admin->id,
|
|
])->assertStatus(500);
|
|
|
|
// The soft-delete shares a transaction with the content step, so its
|
|
// failure leaves the client intact rather than deleted-but-orphaning.
|
|
expect(User::query()->whereKey($client->id)->exists())->toBeTrue()
|
|
->and(ActivityLog::query()->where('action', Action::UserDeleted)->exists())->toBeFalse();
|
|
});
|
|
|
|
test('show reports what a delete would have to decide about', function () {
|
|
$client = User::factory()->client()->create();
|
|
File::factory()->count(2)->create(['uploaded_by' => $client->id]);
|
|
|
|
$this->withToken($this->token)->getJson("/api/v1/clients/{$client->id}")
|
|
->assertOk()
|
|
->assertJsonPath('data.content.files', 2);
|
|
});
|
|
|
|
/*
|
|
|--------------------------------------------------------------------------
|
|
| Permissions
|
|
|--------------------------------------------------------------------------
|
|
*/
|
|
|
|
test('each route needs its own permission', function () {
|
|
$client = User::factory()->client()->create();
|
|
$readOnly = staffWithPermissions([Permission::ManageClients->value]);
|
|
$token = $readOnly->createToken('t', [Permission::ManageClients->value])->plainTextToken;
|
|
|
|
$this->withToken($token)->getJson('/api/v1/clients')->assertOk();
|
|
$this->withToken($token)->postJson('/api/v1/clients', [])->assertForbidden();
|
|
$this->withToken($token)->patchJson("/api/v1/clients/{$client->id}", [])->assertForbidden();
|
|
$this->withToken($token)->deleteJson("/api/v1/clients/{$client->id}")->assertForbidden();
|
|
});
|
|
|
|
test('storage quota reporting distinguishes inherited from unlimited', function () {
|
|
$client = User::factory()->client()->create(['storage_quota_mb' => 0]);
|
|
|
|
$this->withToken($this->token)->getJson("/api/v1/clients/{$client->id}")
|
|
->assertOk()
|
|
// 0 on the client means "inherit the site default", which is what
|
|
// effective_quota_mb resolves — a caller should not have to know
|
|
// that rule to display the number correctly.
|
|
->assertJsonPath('data.storage.quota_mb', 0)
|
|
->assertJsonStructure(['data' => ['storage' => ['effective_quota_mb', 'unlimited', 'used_mb']]]);
|
|
});
|
|
|
|
/*
|
|
|--------------------------------------------------------------------------
|
|
| Two-factor reset
|
|
|--------------------------------------------------------------------------
|
|
*/
|
|
|
|
test('a client\'s second factor can be removed', function () {
|
|
$client = User::factory()->client()->create();
|
|
enableTwoFactor($client);
|
|
forgetRequestState();
|
|
|
|
$this->withToken($this->token)
|
|
->deleteJson("/api/v1/clients/{$client->id}/two-factor")
|
|
->assertNoContent();
|
|
|
|
expect($client->refresh()->hasTwoFactorEnabled())->toBeFalse();
|
|
|
|
expect(ActivityLog::query()->where('action', Action::TwoFactorReset)->sole()->subject_id)
|
|
->toBe($client->id);
|
|
});
|
|
|
|
test('removing a second factor needs edit_clients, not just manage_clients', function () {
|
|
$token = $this->admin->createToken('narrow', [Permission::ManageClients->value])->plainTextToken;
|
|
$client = User::factory()->client()->create();
|
|
enableTwoFactor($client);
|
|
forgetRequestState();
|
|
|
|
$this->withToken($token)
|
|
->deleteJson("/api/v1/clients/{$client->id}/two-factor")
|
|
->assertForbidden();
|
|
|
|
expect($client->refresh()->hasTwoFactorEnabled())->toBeTrue();
|
|
});
|
|
|
|
test('a staff account is not addressable through the client two-factor route', function () {
|
|
$staff = User::factory()->create();
|
|
|
|
$this->withToken($this->token)
|
|
->deleteJson("/api/v1/clients/{$staff->id}/two-factor")
|
|
->assertNotFound();
|
|
});
|
|
|
|
test('the listing reports whether a client has a second factor', function () {
|
|
$client = User::factory()->client()->create();
|
|
enableTwoFactor($client);
|
|
forgetRequestState();
|
|
|
|
$this->withToken($this->token)
|
|
->getJson('/api/v1/clients')
|
|
->assertOk()
|
|
->assertJsonPath('data.0.two_factor_enabled', true);
|
|
});
|