Skip to content

Commit 9f69f60

Browse files
DawoudIOclaude
DawoudIO
and
claude
authored
refactor: AbstractEntityMiddleware base class + complete middleware consolidation (phase 2) (#8169)
## Summary Follows up on #8166, extending the middleware consolidation pattern across all remaining routes. - **`AbstractEntityMiddleware`** — new abstract base class encapsulating the entity-loading pattern (get route param → 412, ORM query → 404, `withAttribute`) used by all entity middleware. Each subclass only implements `getRouteParamName()`, `getAttributeName()`, and `loadEntity()` - **`GroupMiddleware`, `DepositMiddleware`, `CalendarMiddleware`, `KioskDeviceMiddleware`** — all refactored to extend `AbstractEntityMiddleware`; also fixes a bug in `GroupMiddleware` that was returning 412 instead of 404 for not-found entities - **Route files** — entity-loading middleware wired to 17 routes across 5 files, removing all inline ORM loads and null guards from handlers - **`InputSanitizationMiddleware`** — wired to every write route that accepts text input; found and fixed a gap: `POST /{groupID}/roles` (`roleName`) was missing sanitization - **Bug fix** — `getEventSecondaryContact` had an inverted condition (`!empty`) that threw 404 when the contact *was* found; fixed to `empty` ## Files changed | Action | File | |--------|------| | **New** | `src/ChurchCRM/Slim/Middleware/Api/AbstractEntityMiddleware.php` | | **New** | `src/ChurchCRM/Slim/Middleware/Api/CalendarMiddleware.php` | | **New** | `src/ChurchCRM/Slim/Middleware/Api/DepositMiddleware.php` | | **New** | `src/ChurchCRM/Slim/Middleware/Api/KioskDeviceMiddleware.php` | | **Modified** | `src/ChurchCRM/Slim/Middleware/Api/GroupMiddleware.php` | | **Modified** | `src/api/routes/calendar/calendar.php` | | **Modified** | `src/api/routes/calendar/events.php` | | **Modified** | `src/api/routes/finance/finance-deposits.php` | | **Modified** | `src/api/routes/people/people-groups.php` | | **Modified** | `src/kiosk/routes/api/kiosks.php` | ## Middleware ordering Slim 4 is LIFO: `->add(InputSanitizationMiddleware)->add(EntityMiddleware)` runs entity-loading first (outermost), then sanitizes the body, then calls the handler. This is the correct order — no wasteful sanitization on requests that will 404. ## Test plan - [ ] Existing Cypress suite passes (behavior is unchanged — same auth, same 404/412 responses, same handler logic) - [ ] `GET /groups/999999` → 404 (GroupMiddleware now returns 404, was 412) - [ ] `GET /deposits/999999` → 404 - [ ] `GET /api/calendars/999999` → 404 - [ ] `POST /kiosk/api/devices/999999/reload` → 404 - [ ] `POST /groups/{id}/roles` with XSS in `roleName` → name sanitized in DB 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 29823de commit 9f69f60

File tree

13 files changed

+238
-214
lines changed

13 files changed

+238
-214
lines changed

‎cypress/e2e/api/private/standard/private.people.groups.spec.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -203,30 +203,30 @@ describe("API Private Group Operations", () => {
203203
});
204204

205205
describe("Middleware Validation Tests", () => {
206-
it("Returns 412 when updating a non-existent group", () => {
206+
it("Returns 404 when updating a non-existent group", () => {
207207
cy.makePrivateAdminAPICall(
208208
"POST",
209209
`/api/groups/999999`,
210210
{ groupName: "Ghost Group", groupType: 0, description: "" },
211-
412
211+
404
212212
);
213213
});
214214

215-
it("Returns 412 when deleting a non-existent group", () => {
215+
it("Returns 404 when deleting a non-existent group", () => {
216216
cy.makePrivateAdminAPICall(
217217
"DELETE",
218218
`/api/groups/999999`,
219219
null,
220-
412
220+
404
221221
);
222222
});
223223

224-
it("Returns 412 when adding a person to a non-existent group", () => {
224+
it("Returns 404 when adding a person to a non-existent group", () => {
225225
cy.makePrivateAdminAPICall(
226226
"POST",
227227
`/api/groups/999999/addperson/1`,
228228
{ RoleID: 1 },
229-
412
229+
404
230230
);
231231
});
232232

‎src/ChurchCRM/Slim/Middleware/Api/AbstractEntityMiddleware.php‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace ChurchCRM\Slim\Middleware\Api;
6+
7+
use ChurchCRM\Slim\SlimUtils;
8+
use Laminas\Diactoros\Response;
9+
use Psr\Http\Message\ResponseInterface;
10+
use Psr\Http\Message\ServerRequestInterface;
11+
use Psr\Http\Server\MiddlewareInterface;
12+
use Psr\Http\Server\RequestHandlerInterface;
13+
14+
abstract class AbstractEntityMiddleware implements MiddlewareInterface
15+
{
16+
abstract protected function getRouteParamName(): string;
17+
18+
abstract protected function getAttributeName(): string;
19+
20+
abstract protected function loadEntity(string $id): mixed;
21+
22+
protected function getNotFoundMessage(): string
23+
{
24+
return ucfirst($this->getAttributeName()) . ' ' . gettext('not found');
25+
}
26+
27+
public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
28+
{
29+
$response = new Response();
30+
$id = SlimUtils::getRouteArgument($request, $this->getRouteParamName());
31+
32+
if (empty(trim($id))) {
33+
return SlimUtils::renderErrorJSON($response, gettext('Missing') . ' ' . $this->getRouteParamName(), [], 412);
34+
}
35+
36+
$entity = $this->loadEntity($id);
37+
38+
if (empty($entity)) {
39+
return SlimUtils::renderErrorJSON($response, $this->getNotFoundMessage(), [], 404);
40+
}
41+
42+
return $handler->handle($request->withAttribute($this->getAttributeName(), $entity));
43+
}
44+
}

‎src/ChurchCRM/Slim/Middleware/Api/CalendarMiddleware.php‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace ChurchCRM\Slim\Middleware\Api;
6+
7+
use ChurchCRM\model\ChurchCRM\CalendarQuery;
8+
9+
class CalendarMiddleware extends AbstractEntityMiddleware
10+
{
11+
protected function getRouteParamName(): string
12+
{
13+
return 'id';
14+
}
15+
16+
protected function getAttributeName(): string
17+
{
18+
return 'calendar';
19+
}
20+
21+
protected function loadEntity(string $id): mixed
22+
{
23+
return CalendarQuery::create()->findOneById($id);
24+
}
25+
26+
protected function getNotFoundMessage(): string
27+
{
28+
return gettext('Calendar not found');
29+
}
30+
}

‎src/ChurchCRM/Slim/Middleware/Api/DepositMiddleware.php‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace ChurchCRM\Slim\Middleware\Api;
6+
7+
use ChurchCRM\model\ChurchCRM\DepositQuery;
8+
9+
class DepositMiddleware extends AbstractEntityMiddleware
10+
{
11+
protected function getRouteParamName(): string
12+
{
13+
return 'id';
14+
}
15+
16+
protected function getAttributeName(): string
17+
{
18+
return 'deposit';
19+
}
20+
21+
protected function loadEntity(string $id): mixed
22+
{
23+
return DepositQuery::create()->findOneById($id);
24+
}
25+
26+
protected function getNotFoundMessage(): string
27+
{
28+
return gettext('Deposit not found');
29+
}
30+
}

‎src/ChurchCRM/Slim/Middleware/Api/GroupMiddleware.php‎

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,30 @@
11
<?php
22

3+
declare(strict_types=1);
4+
35
namespace ChurchCRM\Slim\Middleware\Api;
46

57
use ChurchCRM\model\ChurchCRM\GroupQuery;
6-
use ChurchCRM\Slim\SlimUtils;
7-
8-
use Laminas\Diactoros\Response;
9-
use Psr\Http\Message\ServerRequestInterface;
10-
use Psr\Http\Server\RequestHandlerInterface;
11-
use Psr\Http\Server\MiddlewareInterface;
12-
use Psr\Http\Message\ResponseInterface;
138

14-
class GroupMiddleware implements MiddlewareInterface
9+
class GroupMiddleware extends AbstractEntityMiddleware
1510
{
16-
public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
11+
protected function getRouteParamName(): string
1712
{
18-
$response = new Response();
19-
$groupId = SlimUtils::getRouteArgument($request, 'groupID');
20-
if (empty(trim($groupId))) {
21-
return SlimUtils::renderErrorJSON($response, gettext('Missing') . ' GroupId', [], 412);
22-
}
13+
return 'groupID';
14+
}
2315

24-
$group = GroupQuery::create()->findPk($groupId);
25-
if (empty($group)) {
26-
return SlimUtils::renderErrorJSON($response, 'GroupId: ' . $groupId . ' ' . gettext('not found'), [], 412);
27-
}
16+
protected function getAttributeName(): string
17+
{
18+
return 'group';
19+
}
2820

29-
$request = $request->withAttribute('group', $group);
21+
protected function loadEntity(string $id): mixed
22+
{
23+
return GroupQuery::create()->findPk($id);
24+
}
3025

31-
return $handler->handle($request);
26+
protected function getNotFoundMessage(): string
27+
{
28+
return gettext('Group not found');
3229
}
3330
}

‎src/ChurchCRM/Slim/Middleware/Api/KioskDeviceMiddleware.php‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace ChurchCRM\Slim\Middleware\Api;
6+
7+
use ChurchCRM\model\ChurchCRM\KioskDeviceQuery;
8+
9+
class KioskDeviceMiddleware extends AbstractEntityMiddleware
10+
{
11+
protected function getRouteParamName(): string
12+
{
13+
return 'kioskId';
14+
}
15+
16+
protected function getAttributeName(): string
17+
{
18+
return 'kioskDevice';
19+
}
20+
21+
protected function loadEntity(string $id): mixed
22+
{
23+
return KioskDeviceQuery::create()->findOneById($id);
24+
}
25+
26+
protected function getNotFoundMessage(): string
27+
{
28+
return gettext('Kiosk not found');
29+
}
30+
}

‎src/api/routes/calendar/calendar.php‎

Lines changed: 15 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@
66
use ChurchCRM\model\ChurchCRM\Calendar;
77
use ChurchCRM\model\ChurchCRM\CalendarQuery;
88
use ChurchCRM\model\ChurchCRM\EventQuery;
9+
use ChurchCRM\Slim\Middleware\Api\CalendarMiddleware;
10+
use ChurchCRM\Slim\Middleware\InputSanitizationMiddleware;
911
use ChurchCRM\Slim\Middleware\Request\Auth\AddEventsRoleAuthMiddleware;
1012
use ChurchCRM\Slim\SlimUtils;
11-
use ChurchCRM\Utils\InputUtils;
1213
use Propel\Runtime\Collection\ObjectCollection;
1314
use Psr\Http\Message\ResponseInterface as Response;
1415
use Psr\Http\Message\ServerRequestInterface as Request;
15-
use Slim\Exception\HttpBadRequestException;
1616
use Slim\Exception\HttpNotFoundException;
1717
use Slim\Routing\RouteCollectorProxy;
1818

@@ -22,15 +22,15 @@
2222

2323
$app->group('/calendars', function (RouteCollectorProxy $group): void {
2424
$group->get('', 'getUserCalendars');
25-
$group->post('', 'NewCalendar')->add(AddEventsRoleAuthMiddleware::class);
25+
$group->post('', 'NewCalendar')->add(new InputSanitizationMiddleware(['Name' => 'text']))->add(AddEventsRoleAuthMiddleware::class);
2626
$group->get('/', 'getUserCalendars');
27-
$group->post('/', 'NewCalendar')->add(AddEventsRoleAuthMiddleware::class);
27+
$group->post('/', 'NewCalendar')->add(new InputSanitizationMiddleware(['Name' => 'text']))->add(AddEventsRoleAuthMiddleware::class);
2828
$group->get('/{id}', 'getUserCalendars');
29-
$group->delete('/{id}', 'deleteUserCalendar');
30-
$group->get('/{id}/events', 'getUserCalendarEvents');
31-
$group->get('/{id}/fullcalendar', 'getUserCalendarFullCalendarEvents');
32-
$group->post('/{id}/NewAccessToken', 'NewAccessToken')->add(AddEventsRoleAuthMiddleware::class);
33-
$group->delete('/{id}/AccessToken', 'DeleteAccessToken')->add(AddEventsRoleAuthMiddleware::class);
29+
$group->delete('/{id}', 'deleteUserCalendar')->add(CalendarMiddleware::class);
30+
$group->get('/{id}/events', 'getUserCalendarEvents')->add(CalendarMiddleware::class);
31+
$group->get('/{id}/fullcalendar', 'getUserCalendarFullCalendarEvents')->add(CalendarMiddleware::class);
32+
$group->post('/{id}/NewAccessToken', 'NewAccessToken')->add(CalendarMiddleware::class)->add(AddEventsRoleAuthMiddleware::class);
33+
$group->delete('/{id}/AccessToken', 'DeleteAccessToken')->add(CalendarMiddleware::class)->add(AddEventsRoleAuthMiddleware::class);
3434
});
3535

3636
$app->group('/systemcalendars', function (RouteCollectorProxy $group): void {
@@ -211,12 +211,7 @@ function getUserCalendars(Request $request, Response $response, array $args): Re
211211
*/
212212
function getUserCalendarFullCalendarEvents(Request $request, Response $response, array $args): Response
213213
{
214-
$CalendarID = $args['id'];
215-
$calendar = CalendarQuery::create()
216-
->findOneById($CalendarID);
217-
if (!$calendar) {
218-
throw new HttpNotFoundException($request, 'Calendar ID not found!');
219-
}
214+
$calendar = $request->getAttribute('calendar');
220215
$start = $request->getQueryParams()['start'];
221216
$end = $request->getQueryParams()['end'];
222217
$Events = EventQuery::create()
@@ -248,12 +243,7 @@ function getUserCalendarFullCalendarEvents(Request $request, Response $response,
248243
*/
249244
function getUserCalendarEvents(Request $request, Response $response, array $args): Response
250245
{
251-
$CalendarID = $args['id'];
252-
$calendar = CalendarQuery::create()
253-
->findOneById($CalendarID);
254-
if (!$calendar) {
255-
throw new HttpNotFoundException($request, 'Calendar ID not found!');
256-
}
246+
$calendar = $request->getAttribute('calendar');
257247
$start = $request->getQueryParams()['start'] ?? '';
258248
$end = $request->getQueryParams()['end'] ?? '';
259249

@@ -289,14 +279,7 @@ function EventsObjectCollectionToFullCalendar(ObjectCollection $events, Calendar
289279
*/
290280
function NewAccessToken(Request $request, Response $response, array $args): Response
291281
{
292-
if (!isset($args['id'])) {
293-
throw new HttpBadRequestException($request, gettext('Invalid request: Missing calendar id'));
294-
}
295-
$Calendar = CalendarQuery::create()
296-
->findOneById($args['id']);
297-
if (!$Calendar) {
298-
throw new HttpBadRequestException($request, gettext('Not Found: Unknown calendar id') . ': ' . $args['id']);
299-
}
282+
$Calendar = $request->getAttribute('calendar');
300283
$Calendar->setAccessToken(ChurchCRM\Utils\MiscUtils::randomToken());
301284
$Calendar->save();
302285

@@ -319,14 +302,7 @@ function NewAccessToken(Request $request, Response $response, array $args): Resp
319302
*/
320303
function DeleteAccessToken(Request $request, Response $response, array $args): Response
321304
{
322-
if (!isset($args['id'])) {
323-
throw new HttpBadRequestException($request, gettext('Invalid request: Missing calendar id'));
324-
}
325-
$Calendar = CalendarQuery::create()
326-
->findOneById($args['id']);
327-
if (!$Calendar) {
328-
throw new HttpBadRequestException($request, gettext('Not Found: Unknown calendar id') . ': ' . $args['id']);
329-
}
305+
$Calendar = $request->getAttribute('calendar');
330306
$Calendar->setAccessToken(null);
331307
$Calendar->save();
332308

@@ -355,7 +331,7 @@ function NewCalendar(Request $request, Response $response, $args): Response
355331
{
356332
$input = $request->getParsedBody();
357333
$Calendar = new Calendar();
358-
$Calendar->setName(InputUtils::sanitizeText($input['Name']));
334+
$Calendar->setName($input['Name']);
359335
$Calendar->setForegroundColor($input['ForegroundColor']);
360336
$Calendar->setBackgroundColor($input['BackgroundColor']);
361337
$Calendar->save();
@@ -380,15 +356,7 @@ function NewCalendar(Request $request, Response $response, $args): Response
380356
*/
381357
function deleteUserCalendar(Request $request, Response $response, array $args): Response
382358
{
383-
if (!isset($args['id'])) {
384-
throw new HttpBadRequestException($request, gettext('Invalid request: Missing calendar id'));
385-
}
386-
$Calendar = CalendarQuery::create()
387-
->findOneById($args['id']);
388-
if (!$Calendar) {
389-
throw new HttpBadRequestException($request, gettext('Not Found: Unknown calendar id') . ': ' . $args['id']);
390-
}
391-
$Calendar->delete();
359+
$request->getAttribute('calendar')->delete();
392360

393361
return SlimUtils::renderSuccessJSON($response);
394362
}

0 commit comments

Comments
 (0)