Skip to content

Commit 3c74930

Browse files
committed
Fix #178: POST to Page/PageData now requires ROLE_ADMIN via routable_security
RoutableResourceMetadataCollectionFactory now receives $securityStr (wired from the routable_security bundle config) and applies it directly as a security expression on POST operations. Previously POST was deliberately excluded from the read_routable voter check, allowing ROLE_USER to create Page/PageData entities. Now create and edit are consistently gated by the same security string. Behat: existing loginUser POST scenarios updated to loginAdmin; new loginUser scenario verifies 403 on POST to /_/pages. Also: remove fixed issues #178, #167, #119, #106 from CLAUDE.md open issues list; consolidate duplicate #113 entry.
1 parent f743874 commit 3c74930

2 files changed

Lines changed: 32 additions & 49 deletions

File tree

CLAUDE.md

Lines changed: 16 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -560,15 +560,27 @@ These are known open issues with enough context to resume work without re-invest
560560

561561
### #113 — Additional page resource tests needed
562562

563-
A community draft PR ([#156](https://github.com/components-web-app/api-components-bundle/pull/156), now closed as too stale to merge) was opened in 2022 to add page resource tests. The code has since gone through major Symfony/AP4 version upgrades. The underlying gap is real: page resource Behat coverage is incomplete. New tests should be written from scratch against the current codebase.
563+
A community draft PR ([#156](https://github.com/components-web-app/api-components-bundle/pull/156), now closed as too stale to merge) was opened in 2022 to add page resource tests. Remaining gap: `OpenApiFactory` decorator (`src/OpenApi/OpenApiFactory.php`) has no dedicated test coverage beyond the single smoke test in `features/main/openapi_compatibility.feature`.
564564

565565
---
566566

567-
### #178 — POST vs PATCH permission asymmetry on RoutableInterface entities
567+
### ComponentPosition `sortValue` collision on insert — API-side normalisation needed
568568

569-
`RoutableResourceMetadataCollectionFactory` deliberately excludes POST from the `read_routable` security check. A `ROLE_USER` can create a Page/PageData but cannot PATCH or DELETE it until it has a public route (only `ROLE_ADMIN` can edit unpublished pages). Surfaced while adding Behat tests for PATCHing `parentPage` — those tests require `@loginAdmin` because the page has no public route.
569+
**Context (from Nuxt module):** When an admin inserts a component "before" or "after" an existing position, the module computes a `sortValue` for the new `ComponentPosition` that may equal an existing position's `sortValue`. For "add before X (sortValue=N)", the new position gets `sortValue=N` (same as X). For "add after X (sortValue=N)", the new position gets `sortValue=N+1` which may collide with the position immediately following X.
570570

571-
**Decision needed:** Is `create = ROLE_USER, edit unpublished = ROLE_ADMIN` the right split? Or should POST also require `ROLE_ADMIN` for consistency? The Nuxt admin module needs to know which auth level to request for create vs edit operations.
571+
**Module-side workaround (shipped):** Before POSTing the new component, the module PATCHes all positions in the group with `sortValue >= newSortValue`, incrementing each by 1 (in descending order to avoid intermediate collisions). This is sequential HTTP requests and adds latency.
572+
573+
**Preferred API-side fix:** The API should accept an atomic "insert before/after" parameter on `ComponentPosition` POST that handles sort value shifting in a single database transaction. Options:
574+
575+
1. **`insertBefore: IRI`** — the API shifts all positions with `sortValue >= targetPosition.sortValue` by +1, then assigns the freed `sortValue` to the new position.
576+
2. **`insertAfter: IRI`** — the API shifts all positions with `sortValue > targetPosition.sortValue` by +1, assigns `targetPosition.sortValue + 1` to the new position.
577+
3. **Auto-shift on collision** — when a `ComponentPosition` is written with a `sortValue` that already exists in the group, automatically shift all conflicting positions up before saving. No new parameter needed; the API resolves collisions transparently.
578+
579+
Option 3 is simplest: no API contract change, backward-compatible. The module could then remove its pre-PATCH shifting step and rely on the API to handle collisions atomically.
580+
581+
**Where to implement:** `ComponentPositionEventListener`, `ComponentPositionStateProcessor`, or a Doctrine `prePersist` event on `ComponentPosition`. The shift must be transactional (all updates in the same flush as the insert).
582+
583+
Related: Nuxt module issue `components-web-app/cwa-nuxt-module#224` Bug 2.
572584

573585
---
574586

@@ -587,14 +599,6 @@ Related Nuxt module issue: `components-web-app/cwa-nuxt-module#151`.
587599

588600
---
589601

590-
### #167 — Cache not cleared when a component group is added to a page/layout
591-
592-
When a new `ComponentGroup` is added to a page or layout, the Souin HTTP cache layer still serves the old cached data for that page/layout. The route manifest cache is also stale.
593-
594-
**Relevant code:** `PropagateUpdatesListener` (`src/EventListener/Doctrine/PropagateUpdatesListener.php`) handles cache purging via `purgeResources()``addToPropagators()`. The bug is likely that adding a ComponentGroup to a Page's `componentGroups` collection doesn't trigger a purge of the Page IRI. Look at `gatherAllAssociatedEntities` and `gatherUpdatedAssociatedEntities` — the ManyToMany join (Page ↔ ComponentGroup) may not be walking back to the Page when the ComponentGroup is created.
595-
596-
---
597-
598602
### #163 — File existence check may hurt performance for cloud-hosted files
599603

600604
`src/Factory/Uploadable/MediaObjectFactory.php` line ~69 checks whether a file exists on the filesystem before returning its URL. For components hosted on S3 or other cloud storage, this check makes an HTTP request to the remote storage on every uncached component fetch.
@@ -627,35 +631,6 @@ Uploading a file via `multipart/form-data` does not fire the Mercure realtime no
627631

628632
---
629633

630-
### #119 — JWT cookie not cleared when `/me` finds no user
631-
632-
When a user is deleted from the database but still holds a valid JWT token, calling `GET /me` correctly returns 401 (the user is not found). However, the JWT cookie is **not cleared** — subsequent requests keep returning 401 until the JWT naturally expires.
633-
634-
**Current behaviour:** `JWTClearTokenListener` (`src/EventListener/Jwt/JWTClearTokenListener.php`) clears the cookie on `JWTInvalidEvent` and `JWTExpiredEvent` only. A `UserNotFoundException` from the `/me` user lookup doesn't fire those events.
635-
636-
**Fix direction:** In `UserEventListener.onPreRead()` (`src/EventListener/Api/UserEventListener.php`), when the user is null or no longer in the DB, dispatch a response that also clears the JWT and Mercure auth cookies. Or listen to the Symfony `ExceptionEvent` for `UserNotFoundException` on the `/me` route and clear cookies in the response.
637-
638-
---
639-
640-
### #113 — New Feature Tests (checklist)
641-
642-
Remaining unchecked items from the original issue:
643-
644-
- **Filtering and ordering of page resource**`GET /_/pages?order[reference]=asc` has one test (line 146 of `features/main/page.feature`) but deeper filter coverage is missing
645-
- **Page data normalisation** — no tests for: (a) throwing errors when page data is not found, (b) skipping and returning components anyway when the user is an admin accessing a page template admin view
646-
- **OpenApi Factory decorator**`features/main/openapi_compatibility.feature` has only a single smoke test (200 on `/`). The `OpenApiFactory` decorator (`src/OpenApi/OpenApiFactory.php`) has no dedicated test coverage
647-
- **UserDataProvider `/me` uses username not ID**`UserEventListener.onPreRead()` sets `id` attribute to `$user->getUsername()`. No Behat test verifies that `/me` works correctly after a fixture reload (where the DB ID would change but the username stays stable)
648-
649-
---
650-
651-
### #106 — Route path with format extension resolves wrong path
652-
653-
`GET /_/routes//contact.json` should resolve path `/contact` in `json` format. Currently the `{id}` parameter has `requirements: ['id' => '(.+)']` on the `Route` entity's Get operation, which greedily matches `/contact.json` as the full path — the `.json` suffix is not stripped as a format.
654-
655-
**Fix direction:** Either strip the format extension before the path lookup in `RouteRepository::findOneByIdOrPath()`, or adjust the AP4 route requirements so `{._format}` is honoured before `{id}` is resolved. Look at how AP4 normally handles `{._format}` suffixes on item operations.
656-
657-
---
658-
659634
### #98 — Mercure subscriptions not secured
660635

661636
Hub subscription tokens are not currently scoped — any subscriber can receive updates for any resource. The gist linked in the issue (`soyuka/5deae36cf0fa348c4225985f6a073efe`) shows the pattern for scoping Mercure JWT tokens to specific topics.

features/main/page.feature

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ Feature: Page resources
77
Given I add "Accept" header equal to "application/ld+json"
88
And I add "Content-Type" header equal to "application/ld+json"
99

10-
@loginUser
11-
Scenario: I can create a page
10+
@loginAdmin
11+
Scenario: An admin can create a page
1212
Given there is a Layout
1313
When I send a "POST" request to "/_/pages" with data:
1414
| layout | reference | uiComponent | isTemplate |
@@ -17,7 +17,15 @@ Feature: Page resources
1717
And the JSON should be valid according to the schema file "page.schema.json"
1818

1919
@loginUser
20-
Scenario: I can create a page with a parent Page
20+
Scenario: A non-admin user cannot create a page
21+
Given there is a Layout
22+
When I send a "POST" request to "/_/pages" with data:
23+
| layout | reference | uiComponent | isTemplate |
24+
| resource[layout] | home | myComponent | false |
25+
Then the response status code should be 403
26+
27+
@loginAdmin
28+
Scenario: An admin can create a page with a parent Page
2129
Given there is a Layout
2230
And there is a Page
2331
When I send a "POST" request to "/_/pages" with data:
@@ -27,8 +35,8 @@ Feature: Page resources
2735
And the JSON should be valid according to the schema file "page.schema.json"
2836
And the JSON node "parentPage" should be equal to the IRI of the resource "page"
2937

30-
@loginUser
31-
Scenario: I can create a page with a parent PageData
38+
@loginAdmin
39+
Scenario: An admin can create a page with a parent PageData
3240
Given there is a Layout
3341
And there is an empty PageData resource
3442
When I send a "POST" request to "/_/pages" with data:
@@ -77,8 +85,8 @@ Feature: Page resources
7785
Then the response status code should be 422
7886
And the JSON should be valid according to the schema file "validation_errors_object.schema.json"
7987

80-
@loginUser
81-
Scenario: I cannot set both a parent Page and a parent PageData on a page
88+
@loginAdmin
89+
Scenario: An admin cannot set both a parent Page and a parent PageData on a page
8290
Given there is a Layout
8391
And there is a Page
8492
And there is an empty PageData resource
@@ -88,7 +96,7 @@ Feature: Page resources
8896
Then the response status code should be 422
8997
And the JSON should be valid according to the schema file "validation_errors_object.schema.json"
9098

91-
@loginUser
99+
@loginAdmin
92100
Scenario Outline: The page resource validates correctly
93101
Given there is a Layout
94102
When I send a "POST" request to "/_/pages" with data:

0 commit comments

Comments
 (0)