From 53c13d221d1692869af32bedd247c931fe704110 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sat, 21 Mar 2026 07:27:10 +0100 Subject: [PATCH] Clean up API routes. --- .../TransactionCurrency/DestroyController.php | 7 -- .../TransactionLinkType/DestroyController.php | 5 -- .../TransactionLinkType/StoreController.php | 9 --- .../TransactionLinkType/UpdateController.php | 9 --- .../System/ConfigurationController.php | 5 -- .../V1/Controllers/System/UserController.php | 44 ++++++------- app/Http/Middleware/IsAdminApi.php | 66 +++++++++++++++++++ bootstrap/app.php | 3 +- routes/api.php | 35 ++++++++-- 9 files changed, 118 insertions(+), 65 deletions(-) create mode 100644 app/Http/Middleware/IsAdminApi.php diff --git a/app/Api/V1/Controllers/Models/TransactionCurrency/DestroyController.php b/app/Api/V1/Controllers/Models/TransactionCurrency/DestroyController.php index 73e0bd6c96..bc502d7e48 100644 --- a/app/Api/V1/Controllers/Models/TransactionCurrency/DestroyController.php +++ b/app/Api/V1/Controllers/Models/TransactionCurrency/DestroyController.php @@ -69,15 +69,8 @@ final class DestroyController extends Controller */ public function destroy(TransactionCurrency $currency): JsonResponse { - /** @var User $admin */ - $admin = auth()->user(); $rules = ['currency_code' => 'required']; - if (!$this->userRepository->hasRole($admin, 'owner')) { - // access denied: - $messages = ['currency_code' => '200005: You need the "owner" role to do this.']; - Validator::make([], $rules, $messages)->validate(); - } if ($this->repository->currencyInUse($currency)) { $messages = ['currency_code' => '200006: Currency in use.']; Validator::make([], $rules, $messages)->validate(); diff --git a/app/Api/V1/Controllers/Models/TransactionLinkType/DestroyController.php b/app/Api/V1/Controllers/Models/TransactionLinkType/DestroyController.php index 82718c94b3..1c4bfbd614 100644 --- a/app/Api/V1/Controllers/Models/TransactionLinkType/DestroyController.php +++ b/app/Api/V1/Controllers/Models/TransactionLinkType/DestroyController.php @@ -72,11 +72,6 @@ final class DestroyController extends Controller if (false === $linkType->editable) { throw new FireflyException('200020: Link type cannot be changed.'); } - if (false === auth()->user()->hasRole('owner')) { - Log::channel('audit')->warning('Non-owner user tries to delete a link type.'); - - return response()->json([], 401); - } $this->repository->destroy($linkType); Preferences::mark(); diff --git a/app/Api/V1/Controllers/Models/TransactionLinkType/StoreController.php b/app/Api/V1/Controllers/Models/TransactionLinkType/StoreController.php index a5c1d5a028..741456b926 100644 --- a/app/Api/V1/Controllers/Models/TransactionLinkType/StoreController.php +++ b/app/Api/V1/Controllers/Models/TransactionLinkType/StoreController.php @@ -73,15 +73,6 @@ final class StoreController extends Controller */ public function store(StoreRequest $request): JsonResponse { - /** @var User $admin */ - $admin = auth()->user(); - $rules = ['name' => 'required']; - - if (!$this->userRepository->hasRole($admin, 'owner')) { - // access denied: - $messages = ['name' => '200005: You need the "owner" role to do this.']; - Validator::make([], $rules, $messages)->validate(); - } $data = $request->getAll(); // if currency ID is 0, find the currency by the code: $linkType = $this->repository->store($data); diff --git a/app/Api/V1/Controllers/Models/TransactionLinkType/UpdateController.php b/app/Api/V1/Controllers/Models/TransactionLinkType/UpdateController.php index b9f65f661d..52876c4b1e 100644 --- a/app/Api/V1/Controllers/Models/TransactionLinkType/UpdateController.php +++ b/app/Api/V1/Controllers/Models/TransactionLinkType/UpdateController.php @@ -80,15 +80,6 @@ final class UpdateController extends Controller throw new FireflyException('200020: Link type cannot be changed.'); } - /** @var User $admin */ - $admin = auth()->user(); - $rules = ['name' => 'required']; - - if (!$this->userRepository->hasRole($admin, 'owner')) { - $messages = ['name' => '200005: You need the "owner" role to do this.']; - Validator::make([], $rules, $messages)->validate(); - } - $data = $request->getAll(); $this->repository->update($linkType, $data); $manager = $this->getManager(); diff --git a/app/Api/V1/Controllers/System/ConfigurationController.php b/app/Api/V1/Controllers/System/ConfigurationController.php index a46699330f..2353b0def4 100644 --- a/app/Api/V1/Controllers/System/ConfigurationController.php +++ b/app/Api/V1/Controllers/System/ConfigurationController.php @@ -142,11 +142,6 @@ final class ConfigurationController extends Controller */ public function update(UpdateRequest $request, string $name): JsonResponse { - $rules = ['value' => 'required']; - if (!$this->repository->hasRole(auth()->user(), 'owner')) { - $messages = ['value' => '200005: You need the "owner" role to do this.']; - Validator::make([], $rules, $messages)->validate(); - } $data = $request->getAll(); $shortName = str_replace('configuration.', '', $name); diff --git a/app/Api/V1/Controllers/System/UserController.php b/app/Api/V1/Controllers/System/UserController.php index f541855442..2291c0f04e 100644 --- a/app/Api/V1/Controllers/System/UserController.php +++ b/app/Api/V1/Controllers/System/UserController.php @@ -74,13 +74,9 @@ final class UserController extends Controller return response()->json([], 500); } - if ($this->repository->hasRole($admin, 'owner')) { - $this->repository->destroy($user); + $this->repository->destroy($user); - return response()->json([], 204); - } - - throw new FireflyException('200025: No access to function.'); + return response()->json([], 204); } /** @@ -92,24 +88,24 @@ final class UserController extends Controller public function index(): JsonResponse { // user preferences - $pageSize = $this->parameters->get('limit'); - $manager = $this->getManager(); + $pageSize = $this->parameters->get('limit'); + $manager = $this->getManager(); // build collection - $collection = $this->repository->all(); - $count = $collection->count(); - $users = $collection->slice(($this->parameters->get('page') - 1) * $pageSize, $pageSize); + $collection = $this->repository->all(); + $count = $collection->count(); + $users = $collection->slice(($this->parameters->get('page') - 1) * $pageSize, $pageSize); // make paginator: - $paginator = new LengthAwarePaginator($users, $count, $pageSize, $this->parameters->get('page')); - $paginator->setPath(route('api.v1.users.index').$this->buildParams()); + $paginator = new LengthAwarePaginator($users, $count, $pageSize, $this->parameters->get('page')); + $paginator->setPath(route('api.v1.users.index') . $this->buildParams()); // make resource /** @var UserTransformer $transformer */ $transformer = app(UserTransformer::class); $transformer->setParameters($this->parameters); - $resource = new FractalCollection($users, $transformer, 'users'); + $resource = new FractalCollection($users, $transformer, 'users'); $resource->setPaginator(new IlluminatePaginatorAdapter($paginator)); return response()->json($manager->createData($resource)->toArray())->header('Content-Type', self::CONTENT_TYPE); @@ -124,14 +120,14 @@ final class UserController extends Controller public function show(User $user): JsonResponse { // make manager - $manager = $this->getManager(); + $manager = $this->getManager(); // make resource /** @var UserTransformer $transformer */ $transformer = app(UserTransformer::class); $transformer->setParameters($this->parameters); - $resource = new Item($user, $transformer, 'users'); + $resource = new Item($user, $transformer, 'users'); return response()->json($manager->createData($resource)->toArray())->header('Content-Type', self::CONTENT_TYPE); } @@ -144,9 +140,9 @@ final class UserController extends Controller */ public function store(UserStoreRequest $request): JsonResponse { - $data = $request->getAll(); - $user = $this->repository->store($data); - $manager = $this->getManager(); + $data = $request->getAll(); + $user = $this->repository->store($data); + $manager = $this->getManager(); // make resource @@ -154,7 +150,7 @@ final class UserController extends Controller $transformer = app(UserTransformer::class); $transformer->setParameters($this->parameters); - $resource = new Item($user, $transformer, 'users'); + $resource = new Item($user, $transformer, 'users'); return response()->json($manager->createData($resource)->toArray())->header('Content-Type', self::CONTENT_TYPE); } @@ -167,7 +163,7 @@ final class UserController extends Controller */ public function update(UserUpdateRequest $request, User $user): JsonResponse { - $data = $request->getAll(); + $data = $request->getAll(); // can only update 'blocked' when user is admin. if (!$this->repository->hasRole(auth()->user(), 'owner')) { @@ -175,15 +171,15 @@ final class UserController extends Controller unset($data['blocked'], $data['blocked_code']); } - $user = $this->repository->update($user, $data); - $manager = $this->getManager(); + $user = $this->repository->update($user, $data); + $manager = $this->getManager(); // make resource /** @var UserTransformer $transformer */ $transformer = app(UserTransformer::class); $transformer->setParameters($this->parameters); - $resource = new Item($user, $transformer, 'users'); + $resource = new Item($user, $transformer, 'users'); return response()->json($manager->createData($resource)->toArray())->header('Content-Type', self::CONTENT_TYPE); } diff --git a/app/Http/Middleware/IsAdminApi.php b/app/Http/Middleware/IsAdminApi.php new file mode 100644 index 0000000000..7a23ac6bd5 --- /dev/null +++ b/app/Http/Middleware/IsAdminApi.php @@ -0,0 +1,66 @@ +. + */ +declare(strict_types=1); + +namespace FireflyIII\Http\Middleware; + +use Closure; +use FireflyIII\Repositories\User\UserRepositoryInterface; +use FireflyIII\User; +use Illuminate\Auth\Access\AuthorizationException; +use Illuminate\Http\Request; +use Illuminate\Support\Facades\Auth; + +/** + * Class IsAdmin. + */ +class IsAdminApi +{ + /** + * Handle an incoming request. Must be admin. + * + * @param null|string $guard + * + * @return mixed + */ + public function handle(Request $request, Closure $next, $guard = null) + { + if (Auth::guard($guard)->guest()) { + if ($request->ajax()) { + return response('Unauthorized.', 401); + } + + return response()->redirectTo(route('login')); + } + + /** @var User $user */ + $user = auth()->user(); + + /** @var UserRepositoryInterface $repository */ + $repository = app(UserRepositoryInterface::class); + if (!$repository->hasRole($user, 'owner')) { + throw new AuthorizationException(); + } + + return $next($request); + } +} diff --git a/bootstrap/app.php b/bootstrap/app.php index ecd9977914..40b6fb48ef 100644 --- a/bootstrap/app.php +++ b/bootstrap/app.php @@ -29,6 +29,7 @@ use FireflyIII\Http\Middleware\EncryptCookies; use FireflyIII\Http\Middleware\Installer; use FireflyIII\Http\Middleware\InterestingMessage; use FireflyIII\Http\Middleware\IsAdmin; +use FireflyIII\Http\Middleware\IsAdminApi; use FireflyIII\Http\Middleware\Range; use FireflyIII\Http\Middleware\RedirectIfAuthenticated; use FireflyIII\Http\Middleware\SecureHeaders; @@ -157,7 +158,7 @@ $app = Application::configure(basePath: dirname(__DIR__)) // 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('api-admin', [ - IsAdmin::class, + IsAdminApi::class, ]); $middleware->appendToGroup('admin', [ IsAdmin::class, diff --git a/routes/api.php b/routes/api.php index 6b9b2de127..44609c3dc9 100644 --- a/routes/api.php +++ b/routes/api.php @@ -655,7 +655,7 @@ Route::group( } ); -// transaction currency API routes that require admin rights: +// Transaction currency API routes that require admin rights: Route::group( [ 'namespace' => 'FireflyIII\Api\V1\Controllers\Models\TransactionCurrency', @@ -664,9 +664,9 @@ Route::group( 'middleware' => ['api-admin'], ], static function (): void { + Route::delete('{currency_code}', ['uses' => 'DestroyController@destroy', 'as' => 'delete']); Route::post('', ['uses' => 'StoreController@store', 'as' => 'store']); Route::put('{currency_code?}', ['uses' => 'UpdateController@update', 'as' => 'update']); - Route::delete('{currency_code}', ['uses' => 'DestroyController@destroy', 'as' => 'delete']); } ); @@ -696,11 +696,23 @@ Route::group( ], static function (): void { Route::get('', ['uses' => 'ShowController@index', 'as' => 'index']); - Route::post('', ['uses' => 'StoreController@store', 'as' => 'store']); Route::get('{linkType}', ['uses' => 'ShowController@show', 'as' => 'show']); + Route::get('{linkType}/transactions', ['uses' => 'ListController@transactions', 'as' => 'transactions']); + } +); + +// Transaction Link Type API routes that need admin rights. +Route::group( + [ + 'namespace' => 'FireflyIII\Api\V1\Controllers\Models\TransactionLinkType', + 'prefix' => 'v1/link-types', + 'as' => 'api.v1.link-types.', + 'middleware' => ['api-admin'], + ], + static function (): void { + Route::post('', ['uses' => 'StoreController@store', 'as' => 'store']); Route::put('{linkType}', ['uses' => 'UpdateController@update', 'as' => 'update']); Route::delete('{linkType}', ['uses' => 'DestroyController@destroy', 'as' => 'delete']); - Route::get('{linkType}/transactions', ['uses' => 'ListController@transactions', 'as' => 'transactions']); } ); @@ -740,10 +752,23 @@ Route::group( ], static function (): void { Route::get('', ['uses' => 'ConfigurationController@index', 'as' => 'index']); - Route::put('{dynamicConfigKey}', ['uses' => 'ConfigurationController@update', 'as' => 'update']); Route::get('{eitherConfigKey}', ['uses' => 'ConfigurationController@show', 'as' => 'show']); } ); + +// Configuration API routes that need admin rights +Route::group( + [ + 'namespace' => 'FireflyIII\Api\V1\Controllers\System', + 'prefix' => 'v1/configuration', + 'as' => 'api.v1.configuration.', + 'middleware' => ['api-admin'], + ], + static function (): void { + Route::put('{dynamicConfigKey}', ['uses' => 'ConfigurationController@update', 'as' => 'update']); + } +); + // Users API routes: Route::group( [