From 2c8be3300045f29ae901ff5b73d61fa24ac5e416 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 15 Feb 2026 11:25:12 +0100 Subject: [PATCH] Clean up routes and API calls. --- app/Http/Controllers/Auth/LoginController.php | 4 + app/Http/Middleware/Authenticate.php | 32 +++-- app/Providers/RouteServiceProvider.php | 26 ++-- .../Authentication/RemoteUserGuard.php | 3 +- app/Support/Binder/CLIToken.php | 1 - bootstrap/app.php | 129 ++++++++---------- routes/api-noauth.php | 11 -- routes/api.php | 16 +++ routes/web.php | 20 +-- 9 files changed, 116 insertions(+), 126 deletions(-) diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index c76ed101b5..47c124a453 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -190,6 +190,10 @@ class LoginController extends Controller */ public function showLoginForm(Request $request): Factory|Redirector|RedirectResponse|View { + if('remote_user_guard' === config('auth.defaults.guard')) { + $message = sprintf('Firefly III is configured to use the "remote user guard", but was unable to link you to a user. Are you sure the "%s" header is in place?', config('auth.guard_header')); + return view('errors.error', ['message' => $message]); + } Log::channel('audit')->info('Show login form (1.1).'); $count = DB::table('users')->count(); diff --git a/app/Http/Middleware/Authenticate.php b/app/Http/Middleware/Authenticate.php index bedb06ab67..75a2fba3d7 100644 --- a/app/Http/Middleware/Authenticate.php +++ b/app/Http/Middleware/Authenticate.php @@ -82,22 +82,23 @@ class Authenticate protected function authenticate($request, array $guards) { if (0 === count($guards)) { - // go for default guard: - // @noinspection PhpUndefinedMethodInspection - if ($this->auth->check()) { - // do an extra check on user object. - /** @noinspection PhpUndefinedMethodInspection */ - - /** @var User $user */ - $user = $this->auth->authenticate(); + Log::debug('in Authenticate::authenticate() with zero guards.'); + // There are no guards defined, go for the default guard: + if (auth()->check()) { + Log::debug('User is authenticated.'); + $user = auth()->user(); $this->validateBlockedUser($user, $guards); + return; } - // @noinspection PhpUndefinedMethodInspection - return $this->auth->authenticate(); + $this->auth->authenticate(); + if (!$this->auth->check()) { + throw new AuthenticationException('The user is not logged in but must be.', $guards); + } } - + die('five'); foreach ($guards as $guard) { + die('six'); if ('api' !== $guard) { $this->auth->guard($guard)->authenticate(); } @@ -110,7 +111,7 @@ class Authenticate return $this->auth->shouldUse($guard); // @phpstan-ignore-line } } - + die('seven'); // this is a massive hack, but if the handler has the oauth exception // at this point we can report its error instead of a generic one. $message = 'Unauthenticated.'; @@ -130,10 +131,10 @@ class Authenticate Log::warning('User is null, throw exception?'); } // \Illuminate\Support\Facades\Log::debug(get_class($user)); - if ($user instanceof User && 1 === (int) $user->blocked) { - $message = (string) trans('firefly.block_account_logout'); + if ($user instanceof User && 1 === (int)$user->blocked) { + $message = (string)trans('firefly.block_account_logout'); if ('email_changed' === $user->blocked_code) { - $message = (string) trans('firefly.email_changed_logout'); + $message = (string)trans('firefly.email_changed_logout'); } Log::warning('User is blocked, cannot use authentication method.'); app('session')->flash('logoutMessage', $message); @@ -143,5 +144,6 @@ class Authenticate // @phpstan-ignore-line (thinks function is undefined) throw new AuthenticationException('Blocked account.', $guards); } + Log::debug(sprintf('User #%d is not blocked.', $user->id)); } } diff --git a/app/Providers/RouteServiceProvider.php b/app/Providers/RouteServiceProvider.php index f9147a91b0..e9f00bf239 100644 --- a/app/Providers/RouteServiceProvider.php +++ b/app/Providers/RouteServiceProvider.php @@ -42,20 +42,20 @@ class RouteServiceProvider extends ServiceProvider #[Override] public function boot(): void { - $this->routes(function (): void { - Route::prefix('api') - ->middleware('api') - ->namespace($this->namespace) - ->group(base_path('routes/api.php')) - ; +// $this->routes(function (): void { +// Route::prefix('api') +// ->middleware('api') +// ->namespace($this->namespace) +// ->group(base_path('routes/api.php')) +// ; - Route::prefix('api/v1/cron') - ->middleware('api_basic') - ->namespace($this->namespace) - ->group(base_path('routes/api-noauth.php')) - ; +// Route::prefix('api/v1/cron') +// ->middleware('api_basic') +// ->namespace($this->namespace) +// ->group(base_path('routes/api-noauth.php')) +// ; - Route::middleware('web')->namespace($this->namespace)->group(base_path('routes/web.php')); - }); +// Route::middleware('web')->namespace($this->namespace)->group(base_path('routes/web.php')); +// }); } } diff --git a/app/Support/Authentication/RemoteUserGuard.php b/app/Support/Authentication/RemoteUserGuard.php index f97f0a0db5..fe97600699 100644 --- a/app/Support/Authentication/RemoteUserGuard.php +++ b/app/Support/Authentication/RemoteUserGuard.php @@ -84,7 +84,8 @@ class RemoteUserGuard implements Guard if (null === $userID || '' === $userID) { Log::error(sprintf('No user in header "%s".', $header)); - throw new FireflyException('The guard header was unexpectedly empty. See the logs.'); + //throw new FireflyException('The guard header was unexpectedly empty. See the logs.'); + return; } Log::debug(sprintf('User ID found in header is "%s"', $userID)); diff --git a/app/Support/Binder/CLIToken.php b/app/Support/Binder/CLIToken.php index 66486aa356..ccd878513d 100644 --- a/app/Support/Binder/CLIToken.php +++ b/app/Support/Binder/CLIToken.php @@ -55,7 +55,6 @@ class CLIToken implements BinderInterface } } Log::error(sprintf('Recognized no users by access token "%s"', $value)); - throw new NotFoundHttpException(); } } diff --git a/bootstrap/app.php b/bootstrap/app.php index d853215949..216b9edc0e 100644 --- a/bootstrap/app.php +++ b/bootstrap/app.php @@ -33,10 +33,8 @@ use FireflyIII\Http\Middleware\Range; use FireflyIII\Http\Middleware\RedirectIfAuthenticated; use FireflyIII\Http\Middleware\SecureHeaders; use FireflyIII\Http\Middleware\StartFireflyIIISession; -use FireflyIII\Http\Middleware\TrustProxies; use FireflyIII\Http\Middleware\VerifyCsrfToken; use Illuminate\Contracts\Debug\ExceptionHandler; -use Illuminate\Contracts\Http\Kernel; use Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse; use Illuminate\Foundation\Application; use Illuminate\Foundation\Configuration\Exceptions; @@ -47,7 +45,6 @@ use Illuminate\Foundation\Http\Middleware\PreventRequestsDuringMaintenance; use Illuminate\Foundation\Http\Middleware\TrimStrings; use Illuminate\Http\Middleware\HandleCors; use Illuminate\Http\Middleware\ValidatePostSize; -use Illuminate\Routing\Middleware\SubstituteBindings; use Illuminate\View\Middleware\ShareErrorsFromSession; use Laravel\Passport\Http\Middleware\CreateFreshApiToken; use Laravel\Sanctum\Http\Middleware\EnsureFrontendRequestsAreStateful; @@ -92,116 +89,98 @@ if (!function_exists('stringIsEqual')) { $app = Application::configure(basePath: dirname(__DIR__)) ->withRouting( web : __DIR__ . '/../routes/web.php', + api : __DIR__ . '/../routes/api.php', commands: __DIR__ . '/../routes/console.php', - health : '/health', + health : '/up', ) ->withMiddleware(function (Middleware $middleware): void { // overrule the standard middleware $middleware->use( [ - InvokeDeferredCallbacks::class, - \Illuminate\Http\Middleware\TrustProxies::class, // use the DEFAULT middleware for this. - HandleCors::class, - PreventRequestsDuringMaintenance::class, - ValidatePostSize::class, - TrimStrings::class, - ConvertEmptyStringsToNull::class, - SecureHeaders::class, // is a Firefly III specific middleware class. - ] + InvokeDeferredCallbacks::class, + \Illuminate\Http\Middleware\TrustProxies::class, // use the DEFAULT middleware for this. + HandleCors::class, + PreventRequestsDuringMaintenance::class, + ValidatePostSize::class, + TrimStrings::class, + ConvertEmptyStringsToNull::class, + SecureHeaders::class, // is a Firefly III specific middleware class. + ] ); - // overrule the web group + // append and extend the default "web" middleware + // to include our own custom "StartFireflyIIISession" class. + // this class in turns contains a better "previous URL" feature. + // See https://laravel.com/docs/12.x/middleware for the default list. $middleware->group('web', [ EncryptCookies::class, AddQueuedCookiesToResponse::class, - StartFireflyIIISession::class, + StartFireflyIIISession::class, // this is different. ShareErrorsFromSession::class, VerifyCsrfToken::class, - SubstituteBindings::class, + Binder::class, // this is also different. CreateFreshApiToken::class, ] ); - // new group? - $middleware->group('binders-only', - [ - Installer::class, - EncryptCookies::class, - AddQueuedCookiesToResponse::class, - Binder::class, - ]); - // - $middleware->appendToGroup('user-not-logged-in', [ - Installer::class, - EncryptCookies::class, - AddQueuedCookiesToResponse::class, - StartFireflyIIISession::class, - ShareErrorsFromSession::class, - VerifyCsrfToken::class, - Binder::class, - RedirectIfAuthenticated::class, - ]); + // the default API group only contains "substitute bindings" middleware + // so here we replace the entire API group and add more sensible stuff. + $middleware->group('api', + [ + AcceptHeaders::class, + EnsureFrontendRequestsAreStateful::class, + 'auth:api', + ] + ); + $middleware->appendToGroup('api_basic', [AcceptHeaders::class, Binder::class]); - // more - $middleware->appendToGroup('user-logged-in-no-2fa', [ - Installer::class, - EncryptCookies::class, - AddQueuedCookiesToResponse::class, - StartFireflyIIISession::class, - ShareErrorsFromSession::class, - VerifyCsrfToken::class, - Binder::class, - Authenticate::class, - ]); - // simple auth + // "simple auth" means the user must be logged in and present, + // but does not have to be 2FA authenticated. This is so all users + // can always log out, for example. $middleware->appendToGroup('user-simple-auth', [ - EncryptCookies::class, - AddQueuedCookiesToResponse::class, - StartFireflyIIISession::class, - ShareErrorsFromSession::class, - VerifyCsrfToken::class, - Binder::class, Authenticate::class, ]); - // user full auth + // This middleware is added for all routes where the user MUST have full authentication. + // this includes 2FA etc. + // incidentally, this group also includes the range middleware and the message thing. $middleware->appendToGroup('user-full-auth', [ - EncryptCookies::class, - AddQueuedCookiesToResponse::class, - StartFireflyIIISession::class, - ShareErrorsFromSession::class, - VerifyCsrfToken::class, Authenticate::class, MFAMiddleware::class, Range::class, - Binder::class, InterestingMessage::class, - CreateFreshApiToken::class, ]); - - // admin + // This middleware is added to ensure that the user is not only logged in and + // authenticated (with MFA and everything), but also admin. $middleware->appendToGroup('admin', [ - EncryptCookies::class, - AddQueuedCookiesToResponse::class, - StartFireflyIIISession::class, - ShareErrorsFromSession::class, - VerifyCsrfToken::class, Authenticate::class, - // AuthenticateTwoFactor::class, + MFAMiddleware::class, IsAdmin::class, Range::class, - Binder::class, - CreateFreshApiToken::class, + InterestingMessage::class, ]); - // api - $middleware->appendToGroup('api', [AcceptHeaders::class, EnsureFrontendRequestsAreStateful::class, 'auth:api,sanctum', Binder::class]); - // api basic, - $middleware->appendToGroup('api_basic', [AcceptHeaders::class, Binder::class]); + // if the user is not logged in, this group applies. + // on top of everything else of course. + $middleware->appendToGroup('user-not-logged-in', [ + Installer::class, + RedirectIfAuthenticated::class, + ]); + // the "binders only" group does not need or ask for authentication + // it just makes sure strings from routes are bound to objects if possible. + $middleware->group('binders-only', + [ + Installer::class, + EncryptCookies::class, + AddQueuedCookiesToResponse::class, + Binder::class, + ]); + + // $middleware->priority([StartFireflyIIISession::class, ShareErrorsFromSession::class, Authenticate::class, Binder::class, Authorize::class]); }) ->withEvents(discover: [ __DIR__ . '/../app/Listeners', diff --git a/routes/api-noauth.php b/routes/api-noauth.php index 7c960e9033..687dba252a 100644 --- a/routes/api-noauth.php +++ b/routes/api-noauth.php @@ -26,14 +26,3 @@ use Illuminate\Support\Facades\Route; // Cron job API routes: use FireflyIII\Http\Middleware\AcceptHeaders; -Route::group( - [ - 'namespace' => 'FireflyIII\Api\V1\Controllers\System', - 'prefix' => '', - 'as' => 'api.v1.cron.', - 'middleware' => [AcceptHeaders::class], - ], - static function (): void { - Route::get('{cliToken}', ['uses' => 'CronController@cron', 'as' => 'index']); - } -); diff --git a/routes/api.php b/routes/api.php index 5836c03547..eef6a9fa8f 100644 --- a/routes/api.php +++ b/routes/api.php @@ -21,6 +21,9 @@ */ declare(strict_types=1); + +use FireflyIII\Http\Middleware\AcceptHeaders; +use FireflyIII\Http\Middleware\Binder; use Illuminate\Support\Facades\Route; use function Safe\define; @@ -38,6 +41,19 @@ if (!defined('DATEFORMAT')) { define('DATEFORMAT', '(19|20)[0-9]{2}-?[0-9]{2}-?[0-9]{2}'); } +// API route for cron +Route::group( + [ + 'namespace' => 'FireflyIII\Api\V1\Controllers\System', + 'prefix' => 'v1', + 'as' => 'api.v1.cron.', + 'middleware' => [Binder::class, AcceptHeaders::class], + ], + static function (): void { + Route::get('cron/{cliToken}', ['uses' => 'CronController@cron', 'as' => 'index'])->withoutMiddleware(['api']); + } +); + // Autocomplete controllers Route::group( [ diff --git a/routes/web.php b/routes/web.php index 12e91ec4c0..30bda7090c 100644 --- a/routes/web.php +++ b/routes/web.php @@ -73,24 +73,24 @@ Route::group( } ); -Route::group( - ['middleware' => 'binders-only', 'namespace' => 'FireflyIII\Http\Controllers\System', 'as' => 'cron.', 'prefix' => 'cron'], - static function (): void { - Route::get('run/{cliToken}', ['uses' => 'CronController@cron', 'as' => 'cron']); - } -); +//Route::group( +// ['middleware' => 'binders-only', 'namespace' => 'FireflyIII\Http\Controllers\System', 'as' => 'cron.', 'prefix' => 'cron'], +// static function (): void { +// Route::get('run/{cliToken}', ['uses' => 'CronController@cron', 'as' => 'cron']); +// } +//); Route::group( - ['middleware' => 'binders-only', 'namespace' => 'FireflyIII\Http\Controllers\System'], + ['middleware' => ['binders-only'], 'namespace' => 'FireflyIII\Http\Controllers\System'], static function (): void { // Route::get('offline', static fn () => view('errors.offline')); - // Route::get('health', ['uses' => 'HealthcheckController@check', 'as' => 'healthcheck']); + Route::get('health', ['uses' => 'HealthcheckController@check', 'as' => 'healthcheck'])->withoutMiddleware(['web']); } ); // These routes only work when the user is NOT logged in. Route::group( - ['middleware' => 'user-not-logged-in', 'namespace' => 'FireflyIII\Http\Controllers'], + ['middleware' => ['user-not-logged-in'], 'namespace' => 'FireflyIII\Http\Controllers'], static function (): void { // Authentication Routes... Route::get('login', ['uses' => 'Auth\LoginController@showLoginForm', 'as' => 'login']); @@ -128,7 +128,7 @@ Route::group( // For the two factor routes, the user must be logged in, but NOT 2FA. Account confirmation does not matter here. Route::group( - ['middleware' => 'user-logged-in-no-2fa', 'prefix' => 'two-factor', 'as' => 'two-factor.', 'namespace' => 'FireflyIII\Http\Controllers\Auth'], + ['middleware' => 'user-simple-auth', 'prefix' => 'two-factor', 'as' => 'two-factor.', 'namespace' => 'FireflyIII\Http\Controllers\Auth'], static function (): void { Route::post('submit', ['uses' => 'TwoFactorController@submitMFA', 'as' => 'submit']); Route::get('lost', ['uses' => 'TwoFactorController@lostTwoFactor', 'as' => 'lost']); // can be removed when v2 is live.