mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-16 16:45:07 +00:00
3244be6bac
Reported by Nooraldden Khalel as GHSA-f32x-fgmp-q353. The profile screen let a signed-in session change its own email address with nothing else, and that address is where a password reset is sent. So a stolen session was enough: point the account at your own inbox, ask for a reset, set a password, and temporary access is permanent ownership. Clearing email_verified_at did not stand in the way, because the model does not implement MustVerifyEmail and the reset broker never asks. destroy(), thirty lines further down the same controller, has always required the current password, and its comment says why: "the rule every other door into this already asks". This door leads to the same place and was not asking. Only a *different* address asks. A name, a timezone or a custom field is not a credential, so the rest of the screen saves with nothing extra — which is why the rule is excluded rather than flat, and why the comparison is trimmed and lowercased: re-saving a profile with the address typed in a different case must not demand a password for nothing. An account whose credentials live in a directory or at an identity provider is refused outright and told why, rather than being asked for a password it does not have. LdapProvisioner stores Str::password(64) exactly so that local password can never be used, so asking would be a dead end dressed as a form error — and the address is not theirs to change here anyway: it is what the directory says it is. The test walks the whole chain rather than checking the field is validated, because the chain is what made this high: change the address, ask for a reset there, and confirm nothing is sent and no such account exists.
140 lines
4.3 KiB
PHP
140 lines
4.3 KiB
PHP
<?php
|
|
|
|
namespace Tests\Feature\Settings;
|
|
|
|
use App\Models\User;
|
|
use App\Modules\Platform\Settings\Setting;
|
|
use App\Modules\Platform\Settings\Settings;
|
|
use Illuminate\Foundation\Testing\RefreshDatabase;
|
|
use Tests\TestCase;
|
|
|
|
class ProfileUpdateTest extends TestCase
|
|
{
|
|
use RefreshDatabase;
|
|
|
|
public function test_profile_page_is_displayed()
|
|
{
|
|
$user = User::factory()->create();
|
|
|
|
$response = $this
|
|
->actingAs($user)
|
|
->get('/settings/profile');
|
|
|
|
$response->assertOk();
|
|
}
|
|
|
|
public function test_profile_information_can_be_updated()
|
|
{
|
|
// The password is here because the address changes. Changing it is
|
|
// a credential change — it is where a password reset is sent — so
|
|
// it asks for the current password, exactly as deleting the account
|
|
// does. See ProfileEmailReauthTest (GHSA-f32x-fgmp-q353).
|
|
$user = User::factory()->create(['password' => 'the-real-password']);
|
|
|
|
$response = $this
|
|
->actingAs($user)
|
|
->patch('/settings/profile', [
|
|
'name' => 'Test User',
|
|
'email' => 'test@example.com',
|
|
'current_password' => 'the-real-password',
|
|
]);
|
|
|
|
$response
|
|
->assertSessionHasNoErrors()
|
|
->assertRedirect('/settings/profile');
|
|
|
|
$user->refresh();
|
|
|
|
$this->assertSame('Test User', $user->name);
|
|
$this->assertSame('test@example.com', $user->email);
|
|
$this->assertNull($user->email_verified_at);
|
|
}
|
|
|
|
public function test_email_verification_status_is_unchanged_when_the_email_address_is_unchanged()
|
|
{
|
|
$user = User::factory()->create();
|
|
|
|
$response = $this
|
|
->actingAs($user)
|
|
->patch('/settings/profile', [
|
|
'name' => 'Test User',
|
|
'email' => $user->email,
|
|
]);
|
|
|
|
$response
|
|
->assertSessionHasNoErrors()
|
|
->assertRedirect('/settings/profile');
|
|
|
|
$this->assertNotNull($user->refresh()->email_verified_at);
|
|
}
|
|
|
|
public function test_user_can_delete_their_account()
|
|
{
|
|
$user = User::factory()->create();
|
|
|
|
// A second administrator, so what is under test is self-deletion
|
|
// and not the last-administrator refusal: the factory makes an
|
|
// administrator, and one on their own may no longer remove
|
|
// themselves — see SoleAdministratorSelfDeletionTest.
|
|
User::factory()->create();
|
|
|
|
$response = $this
|
|
->actingAs($user)
|
|
->delete('/settings/profile', [
|
|
'password' => 'password',
|
|
]);
|
|
|
|
$response
|
|
->assertSessionHasNoErrors()
|
|
->assertRedirect('/');
|
|
|
|
$this->assertGuest();
|
|
// Deletion is soft: gone from every query, recoverable by an admin.
|
|
$this->assertNull(User::query()->find($user->id));
|
|
$this->assertNotNull(User::withTrashed()->find($user->id));
|
|
}
|
|
|
|
public function test_correct_password_must_be_provided_to_delete_account()
|
|
{
|
|
$user = User::factory()->create();
|
|
|
|
$response = $this
|
|
->actingAs($user)
|
|
->from('/settings/profile')
|
|
->delete('/settings/profile', [
|
|
'password' => 'wrong-password',
|
|
]);
|
|
|
|
$response
|
|
->assertSessionHasErrors('password')
|
|
->assertRedirect('/settings/profile');
|
|
|
|
$this->assertNotNull($user->fresh());
|
|
}
|
|
|
|
public function test_erasure_grace_period_reflects_the_privacy_setting()
|
|
{
|
|
app(Settings::class)->set(Setting::AccountErasureGraceDays, 5);
|
|
|
|
$user = User::factory()->create();
|
|
|
|
// Same reason as above: this is about the grace period, not about
|
|
// who is allowed to go.
|
|
User::factory()->create();
|
|
|
|
// Deleting your account has its own screen; the profile form no
|
|
// longer carries the block or the grace period behind it.
|
|
$this->actingAs($user)
|
|
->get('/settings/delete-account')
|
|
->assertInertia(fn ($page) => $page
|
|
->component('settings/delete-account')
|
|
->where('erasureGraceDays', 5));
|
|
|
|
$this->actingAs($user)->delete('/settings/profile', ['password' => 'password']);
|
|
|
|
$this->assertTrue(
|
|
User::withTrashed()->find($user->id)->erase_after->isSameDay(now()->addDays(5)),
|
|
);
|
|
}
|
|
}
|