diff --git a/app/Api/V1/Controllers/System/UserController.php b/app/Api/V1/Controllers/System/UserController.php index b96ea03f24d..c16d25e988e 100644 --- a/app/Api/V1/Controllers/System/UserController.php +++ b/app/Api/V1/Controllers/System/UserController.php @@ -33,6 +33,7 @@ use FireflyIII\User; use Illuminate\Http\JsonResponse; use Illuminate\Pagination\LengthAwarePaginator; +use Illuminate\Support\Facades\Log; use League\Fractal\Pagination\IlluminatePaginatorAdapter; use League\Fractal\Resource\Collection as FractalCollection; use League\Fractal\Resource\Item; @@ -191,6 +192,13 @@ public function store(UserStoreRequest $request): JsonResponse public function update(UserUpdateRequest $request, User $user): JsonResponse { $data = $request->getAll(); + + // can only update 'blocked' when user is admin. + if(!$this->repository->hasRole(auth()->user(), 'owner')) { + Log::debug('Quietly drop fields "blocked" and "blocked_code" from request.'); + unset($data['blocked'], $data['blocked_code']); + } + $user = $this->repository->update($user, $data); $manager = $this->getManager(); // make resource diff --git a/app/Http/Middleware/Authenticate.php b/app/Http/Middleware/Authenticate.php index 6d26cade687..e296c4560b2 100644 --- a/app/Http/Middleware/Authenticate.php +++ b/app/Http/Middleware/Authenticate.php @@ -87,7 +87,7 @@ public function handle($request, Closure $next, ...$guards) */ protected function authenticate($request, array $guards) { - // Log::debug(sprintf('Now in %s', __METHOD__)); + Log::debug(sprintf('Now in %s', __METHOD__)); if (0 === count($guards)) { // Log::debug('No guards present.'); // go for default guard: @@ -98,22 +98,7 @@ protected function authenticate($request, array $guards) /** @noinspection PhpUndefinedMethodInspection */ /** @var User $user */ $user = $this->auth->authenticate(); - if (null === $user) { - Log::warning('User is null, throw exception?'); - } - if (null !== $user) { - // Log::debug(get_class($user)); - if (1 === (int)$user->blocked) { - $message = (string)trans('firefly.block_account_logout'); - if ('email_changed' === $user->blocked_code) { - $message = (string)trans('firefly.email_changed_logout'); - } - app('session')->flash('logoutMessage', $message); - $this->auth->logout(); // @phpstan-ignore-line (thinks function is undefined) - - throw new AuthenticationException('Blocked account.', $guards); - } - } + $this->validateBlockedUser($user, $guards); } return $this->auth->authenticate(); // @phpstan-ignore-line (thinks function returns void) @@ -121,13 +106,15 @@ protected function authenticate($request, array $guards) // Log::debug('Guard array is not empty.'); foreach ($guards as $guard) { - // Log::debug(sprintf('Now in guard loop, guard is "%s"', $guard)); - if('api' !== $guard) { + Log::debug(sprintf('Now in guard loop, guard is "%s"', $guard)); + if ('api' !== $guard) { $this->auth->guard($guard)->authenticate(); } $result = $this->auth->guard($guard)->check(); - // Log::debug(sprintf('Result is %s', var_export($result, true))); + Log::debug(sprintf('Result is %s', var_export($result, true))); if ($result) { + $user = $this->auth->guard($guard)->user(); + $this->validateBlockedUser($user, $guards); // According to PHPstan the method returns void, but we'll see. return $this->auth->shouldUse($guard); // @phpstan-ignore-line } @@ -135,4 +122,32 @@ protected function authenticate($request, array $guards) throw new AuthenticationException('Unauthenticated.', $guards); } + + /** + * @param User|null $user + * @param array $guards + * @return void + * @throws AuthenticationException + */ + private function validateBlockedUser(?User $user, array $guards): void + { + Log::debug(sprintf('Now in %s', __METHOD__)); + if (null === $user) { + Log::warning('User is null, throw exception?'); + } + if (null !== $user) { + // Log::debug(get_class($user)); + if (1 === (int)$user->blocked) { + $message = (string)trans('firefly.block_account_logout'); + if ('email_changed' === $user->blocked_code) { + $message = (string)trans('firefly.email_changed_logout'); + } + Log::warning('User is blocked, cannot use authentication method.'); + app('session')->flash('logoutMessage', $message); + $this->auth->logout(); // @phpstan-ignore-line (thinks function is undefined) + + throw new AuthenticationException('Blocked account.', $guards); + } + } + } } diff --git a/changelog.md b/changelog.md index 16b3c730988..4a664a66ca6 100644 --- a/changelog.md +++ b/changelog.md @@ -6,8 +6,7 @@ Alpha 2 - Data import: when you submit a transaction but give the ID of an account of the wrong type, Firefly III will try to create an account of the right type. For example: you submit a deposit but the source account is an expense account: Firefly III will try to create a revenue account instead. - - +- Security: blocked users can access API, users can unblock themselves using the API. ## 5.8.0-alpha.1 - 2023-01-08