Fix changing a user password to not incorrectly handle logging out old sessions; closes #3531

This commit is contained in:
Dane Everitt 2021-08-15 17:37:12 -07:00
parent 25d9ba4779
commit 2b3303c46b
No known key found for this signature in database
GPG key ID: EEA66103B3D71F53
5 changed files with 32 additions and 28 deletions

View file

@ -58,12 +58,17 @@ class AccountController extends ClientApiController
* Update the authenticated user's password. All existing sessions will be logged * Update the authenticated user's password. All existing sessions will be logged
* out immediately. * out immediately.
* *
* @throws \Pterodactyl\Exceptions\Model\DataValidationException * @throws \Throwable
* @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException
*/ */
public function updatePassword(UpdatePasswordRequest $request): JsonResponse public function updatePassword(UpdatePasswordRequest $request): JsonResponse
{ {
$this->updateService->handle($request->user(), $request->validated()); $user = $this->updateService->handle($request->user(), $request->validated());
// If you do not update the user in the session you'll end up working with a
// cached copy of the user that does not include the updated password. Do this
// to correctly store the new user details in the guard and allow the logout
// other devices functionality to work.
$this->sessionGuard->setUser($user);
$this->sessionGuard->logoutOtherDevices($request->input('password')); $this->sessionGuard->logoutOtherDevices($request->input('password'));

View file

@ -3,6 +3,8 @@
namespace Pterodactyl\Http\Requests\Api\Client\Account; namespace Pterodactyl\Http\Requests\Api\Client\Account;
use Pterodactyl\Models\User; use Pterodactyl\Models\User;
use Illuminate\Container\Container;
use Illuminate\Contracts\Hashing\Hasher;
use Pterodactyl\Http\Requests\Api\Client\ClientApiRequest; use Pterodactyl\Http\Requests\Api\Client\ClientApiRequest;
use Pterodactyl\Exceptions\Http\Base\InvalidPasswordProvidedException; use Pterodactyl\Exceptions\Http\Base\InvalidPasswordProvidedException;
@ -17,8 +19,10 @@ class UpdateEmailRequest extends ClientApiRequest
return false; return false;
} }
$hasher = Container::getInstance()->make(Hasher::class);
// Verify password matches when changing password or email. // Verify password matches when changing password or email.
if (!password_verify($this->input('password'), $this->user()->password)) { if (!$hasher->check($this->input('password'), $this->user()->password)) {
throw new InvalidPasswordProvidedException(trans('validation.internal.invalid_password')); throw new InvalidPasswordProvidedException(trans('validation.internal.invalid_password'));
} }

View file

@ -2,6 +2,8 @@
namespace Pterodactyl\Http\Requests\Api\Client\Account; namespace Pterodactyl\Http\Requests\Api\Client\Account;
use Illuminate\Container\Container;
use Illuminate\Contracts\Hashing\Hasher;
use Pterodactyl\Http\Requests\Api\Client\ClientApiRequest; use Pterodactyl\Http\Requests\Api\Client\ClientApiRequest;
use Pterodactyl\Exceptions\Http\Base\InvalidPasswordProvidedException; use Pterodactyl\Exceptions\Http\Base\InvalidPasswordProvidedException;
@ -16,8 +18,10 @@ class UpdatePasswordRequest extends ClientApiRequest
return false; return false;
} }
$hasher = Container::getInstance()->make(Hasher::class);
// Verify password matches when changing password or email. // Verify password matches when changing password or email.
if (!password_verify($this->input('current_password'), $this->user()->password)) { if (!$hasher->check($this->input('current_password'), $this->user()->password)) {
throw new InvalidPasswordProvidedException(trans('validation.internal.invalid_password')); throw new InvalidPasswordProvidedException(trans('validation.internal.invalid_password'));
} }

View file

@ -5,7 +5,6 @@ namespace Pterodactyl\Services\Users;
use Pterodactyl\Models\User; use Pterodactyl\Models\User;
use Illuminate\Contracts\Hashing\Hasher; use Illuminate\Contracts\Hashing\Hasher;
use Pterodactyl\Traits\Services\HasUserLevels; use Pterodactyl\Traits\Services\HasUserLevels;
use Pterodactyl\Repositories\Eloquent\UserRepository;
class UserUpdateService class UserUpdateService
{ {
@ -16,29 +15,20 @@ class UserUpdateService
*/ */
private $hasher; private $hasher;
/**
* @var \Pterodactyl\Repositories\Eloquent\UserRepository
*/
private $repository;
/** /**
* UpdateService constructor. * UpdateService constructor.
*/ */
public function __construct(Hasher $hasher, UserRepository $repository) public function __construct(Hasher $hasher)
{ {
$this->hasher = $hasher; $this->hasher = $hasher;
$this->repository = $repository;
} }
/** /**
* Update the user model instance. * Update the user model instance and return the updated model.
* *
* @return \Pterodactyl\Models\User * @throws \Throwable
*
* @throws \Pterodactyl\Exceptions\Model\DataValidationException
* @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException
*/ */
public function handle(User $user, array $data) public function handle(User $user, array $data): User
{ {
if (!empty(array_get($data, 'password'))) { if (!empty(array_get($data, 'password'))) {
$data['password'] = $this->hasher->make($data['password']); $data['password'] = $this->hasher->make($data['password']);
@ -46,9 +36,8 @@ class UserUpdateService
unset($data['password']); unset($data['password']);
} }
/** @var \Pterodactyl\Models\User $response */ $user->forceFill($data)->saveOrFail();
$response = $this->repository->update($user->id, $data);
return $response; return $user->refresh();
} }
} }

View file

@ -2,10 +2,9 @@
namespace Pterodactyl\Tests\Integration\Api\Client; namespace Pterodactyl\Tests\Integration\Api\Client;
use Mockery;
use Pterodactyl\Models\User; use Pterodactyl\Models\User;
use Illuminate\Http\Response; use Illuminate\Http\Response;
use Illuminate\Auth\AuthManager; use Illuminate\Support\Facades\Hash;
class AccountControllerTest extends ClientApiIntegrationTestCase class AccountControllerTest extends ClientApiIntegrationTestCase
{ {
@ -106,10 +105,7 @@ class AccountControllerTest extends ClientApiIntegrationTestCase
/** @var \Pterodactyl\Models\User $user */ /** @var \Pterodactyl\Models\User $user */
$user = User::factory()->create(); $user = User::factory()->create();
$mock = Mockery::mock(AuthManager::class); $initialHash = $user->password;
$mock->expects('logoutOtherDevices')->with('New_Password1');
$this->app->instance(AuthManager::class, $mock);
$response = $this->actingAs($user)->putJson('/api/client/account/password', [ $response = $this->actingAs($user)->putJson('/api/client/account/password', [
'current_password' => 'password', 'current_password' => 'password',
@ -117,6 +113,12 @@ class AccountControllerTest extends ClientApiIntegrationTestCase
'password_confirmation' => 'New_Password1', 'password_confirmation' => 'New_Password1',
]); ]);
$user = $user->refresh();
$this->assertNotEquals($user->password, $initialHash);
$this->assertTrue(Hash::check('New_Password1', $user->password));
$this->assertFalse(Hash::check('password', $user->password));
$response->assertStatus(Response::HTTP_NO_CONTENT); $response->assertStatus(Response::HTTP_NO_CONTENT);
} }