From 3bfc12f93bd1a9e1cc4e842f419bd9aa0df85482 Mon Sep 17 00:00:00 2001 From: Sander Dorigo Date: Thu, 12 Jun 2025 15:36:38 +0200 Subject: [PATCH] Try to be more clear about OAuth errors --- app/Exceptions/Handler.php | 4 +++- app/Http/Kernel.php | 2 +- app/Http/Middleware/Authenticate.php | 16 +++++++++++----- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 77f198dae1..a9022ed2af 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -58,6 +58,7 @@ use function Safe\parse_url; */ class Handler extends ExceptionHandler { + public static ?Throwable $lastError = null; /** * @var array> */ @@ -123,7 +124,7 @@ class Handler extends ExceptionHandler // somehow Laravel handler does not catch this: app('log')->debug('Return JSON unauthenticated error.'); - return response()->json(['message' => 'Unauthenticated', 'exception' => 'AuthenticationException'], 401); + return response()->json(['message' => $e->getMessage(), 'exception' => 'AuthenticationException'], 401); } if ($e instanceof OAuthServerException && $expectsJson) { @@ -215,6 +216,7 @@ class Handler extends ExceptionHandler #[Override] public function report(Throwable $e): void { + self::$lastError = $e; $doMailError = (bool) config('firefly.send_error_message'); if ($this->shouldntReportLocal($e) || !$doMailError) { parent::report($e); diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index 139547582c..7b6815dadd 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -178,7 +178,7 @@ class Kernel extends HttpKernel 'api' => [ AcceptHeaders::class, EnsureFrontendRequestsAreStateful::class, - 'auth:api,sanctum', + 'auth:api', 'bindings', ], // do only bindings, no auth diff --git a/app/Http/Middleware/Authenticate.php b/app/Http/Middleware/Authenticate.php index 7f11d50cba..75d23b70e1 100644 --- a/app/Http/Middleware/Authenticate.php +++ b/app/Http/Middleware/Authenticate.php @@ -26,10 +26,12 @@ namespace FireflyIII\Http\Middleware; use Closure; use FireflyIII\Exceptions\FireflyException; +use FireflyIII\Exceptions\Handler; use FireflyIII\User; use Illuminate\Auth\AuthenticationException; use Illuminate\Contracts\Auth\Factory as Auth; use Illuminate\Http\Request; +use League\OAuth2\Server\Exception\OAuthServerException; /** * Class Authenticate @@ -84,6 +86,7 @@ class Authenticate if ($this->auth->check()) { // do an extra check on user object. /** @noinspection PhpUndefinedMethodInspection */ + /** @var User $user */ $user = $this->auth->authenticate(); $this->validateBlockedUser($user, $guards); @@ -94,20 +97,23 @@ class Authenticate } foreach ($guards as $guard) { - if ('api' !== $guard) { - $this->auth->guard($guard)->authenticate(); - } $result = $this->auth->guard($guard)->check(); 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 } } - throw new AuthenticationException('Unauthenticated.', $guards); + // this is a massive hack, but if the hander has the oauth exception + // at this point we can report its error instead of a generic one. + $message = 'Unauthenticated.'; + if(Handler::$lastError instanceof OAuthServerException) { + $message = Handler::$lastError->getHint(); + } + + throw new AuthenticationException($message, $guards); } /**