fix: cloisonnement des canaux de commande et suivi public restreint a la borne #156

Merged

Corentin merged 12 commits from fix/rbac-order-channel into dev 2026-09-26 11:50:51 +02:00

Conversation 0 Commits 12 Files changed 23 +1340 -147

Corentin commented 2026-09-26 11:46:35 +02:00

Owner

Copy link

No description provided.

Corentin added 12 commits 2026-09-26 11:46:35 +02:00

fix: enforce order channel isolation on counter, drive and order pages ... 466801e6ed

A role with a fixed order_source could open the other channel's page and create an order tagged for it, cancel an order outside its channels, and list every order on /admin/orders. roleFixedSource() moves to AdminController and gates counter/drive index, create and store; cancel and its confirmation page check role_visible_source before the PIN, answering 403 for unknown and hidden orders alike; the order list applies the same visible-source filter as the kitchen display.

fix: show order status in the counter and drive in-progress queue ... 59c7d18eb1

Staff had to open the kitchen display to know whether an order was ready. The queue now shows the status label already returned by paidQueue().

fix: keep the font toggle from covering the checkout button at 768px ... 64b6b6f203

Below 860px the order panel falls under the product grid and its checkout button could sit under the fixed font toggle on first paint. The panel foot is now sticky at the same 80px margin; an e2e test measures both boxes at 768 and 1366.

fix: recognize any non-null order_source as a fixed channel ... 6934130481

roleFixedSource() only special-cased 'counter'/'drive'. The role form
also offers 'kiosk' as an order_source (RoleController::SOURCES), but a
role configured that way has no dedicated HTML page at all, so it fell
through to null and was treated as having no fixed channel -- opening
both /counter/orders and /drive/orders. Block by default instead: any
non-empty order_source is a fixed channel, whether or not a page for it
exists.

fix: bound channelGuard and order creation by role_visible_source ... 9f1d05cbc0

Both channelGuard() (HTML) and apiStore() (JSON) only checked a role's
FIXED order_source against the page/chosen channel. A role with no
fixed source but a restricted role_visible_source (e.g. a custom role
scoped to 'drive' only) could still open /counter/orders and create a
counter order there, and could still pick "source": "counter" in the
JSON body -- neither path consulted visibleSources() at all.

channelGuard() now also requires the page's source to be in the role's
visible sources; apiStore() now also rejects a resolved source (fixed
or chosen) that isn't visible to the role, with the same 422
VALIDATION_ERROR shape as the existing source-choice validation.

fix: restrict the public order-tracking endpoint to kiosk orders ... a88dd7e7ec

GET /api/orders/{number} is anonymous, unauthenticated, and order
numbers are sequential (channel prefix + auto-increment id).
findByNumber() didn't filter by source, so this endpoint would return
the status AND total_ttc_cents of any counter/drive order too, not
just kiosk ones -- despite those orders being placed by an identified
staff member, not an anonymous customer.

findByNumber() now also returns source (additive; create()/pay() keep
using the existing present(), unaffected). show() treats a non-kiosk
order the same as an unknown one (404 ORDER_NOT_FOUND, identical body,
anti-enumeration), and drops total_ttc_cents from its response: no
kiosk screen (checkout.js, page-confirmation.js, confirm-modal.js)
reads that field from this endpoint today.

fix: filter the admin orders list by source in SQL before the limit ... 5d41fc0f43

OrderAdminController::index() called recent(50) (no source filter,
LIMIT already applied across all channels) and then filtered the
result down to the role's visible sources in PHP. A role with a
restricted role_visible_source could get an empty or truncated list
even when it has real, older orders on its channel, simply because
those orders weren't among the 50 most recent across every channel.

Add OrderQueryRepository::recentVisible(), which applies the source
filter in the WHERE clause before the LIMIT, and use it in index()
instead. Also reword the cancel() docblock's anti-enumeration claim to
scope it to this controller's own actions (it doesn't hold project-wide
on its own -- see the public tracking endpoint fixed separately), and
correct a stale example in the filtering comment (kitchen sees all
three sources at seed 0001, so 'drive' is the actual restricted case).

docs: correct RBAC claims and update demo proof documents after review ... 0d021cb3d4

Several statements were inaccurate or went stale once the channel
guard and visibility checks changed:

- "admin/manager" kept coming up as the roles without a fixed order
  channel; manager has no order.create at all (decision D5) and never
  reaches that code path, so it was misleading to name it alongside
  admin as if both relied on the same NULL-order_source branch.
- "a role with a fixed channel only accesses its own page" no longer
  covers a fixed channel with no page at all (kiosk).
- docs/demo/matrice-rbac.md section 3 and docs/demo/comptes-demo.md's
  "Verification" section documented the channel-isolation gap and the
  unfiltered /admin/orders list as known, accepted limitations -- both
  are fixed now, and the demo docs are the proof shown to the jury, so
  they needed to describe the current behavior, not the old one.
- src/public/admin/index.php referenced a roleOrderSource() method that
  isn't the actual method name (roleFixedSource()).
- conventions.md's public tracking endpoint entry still listed "total"
  among the non-sensitive fields it returns.
- Documented that sales KPIs aggregate across all channels regardless
  of role_visible_source (a deliberate choice, distinct from the order
  list, now stated rather than left implicit).

tests/e2e/rbac-channel.spec.js's header comment is updated to match:
rbac-demo.spec.js (from PR #152) now exists and covers the seeded demo
accounts; this file stays separate and self-provisions its own.

test: add a DB-backed integration test for recentVisible() ... 43e5b4c35f

recentVisible()'s only coverage was a PHP double that reimplements the
SQL filtering semantics -- nothing proved the real query filters by
source before the LIMIT, binds multiple sources correctly, or resists
a hostile string passed as a source. Port the adversarial reviewer's
throwaway test (scratchpad/rev2-copy) into the suite as
OrderQueryRepositoryVisibleDbTest, keeping its four cases (filter
before the limit, several bound sources, hostile string, limit
clamping). Dropped its fifth case (an ENUM-strictness check on
role.order_source) -- that's about a different method entirely.

Cleans customer_order in setUp/tearDown rather than the sibling
OrderQueryRepositoryDbTest's per-prefix delete: this file's assertions
need exact counts to prove the before/after-LIMIT distinction, which a
prefix-only cleanup doesn't guarantee. Safe here because PHPUnit runs
setUp/tearDown per method, in order, and no seed inserts into
customer_order.

fix: filter the counter/drive orders list by source in SQL before the limit ... 5eef57c51b

CounterOrderController::index() had the same limit-then-filter pattern
already fixed for OrderAdminController::index(): it called recent(50)
(no source filter, LIMIT applied across every channel) and then kept
only the rows matching the current channel in PHP. A channel with
enough traffic on OTHER sources could push its own older orders past
the 50-row window before the PHP filter ever saw them.

Use recentVisible([$source], 50) instead, which applies the source
filter in SQL before the LIMIT.

fix: 403 (not 422) for a channel a role can't use, plus an explicit message for a non-staff fixed channel ... 2c982e8c71

apiStore() answered 422 VALIDATION_ERROR when a role's resolved channel
(fixed or chosen) wasn't in its visible sources. That's an absent
right, not malformed input -- switch to 403 FORBIDDEN, matching the
same family of response as the other visibility checks in this file
(apiShow(), transition()).

Separately, a role with a fixed channel that isn't 'counter' or
'drive' (kiosk, recognized as a fixed channel since the first review
round) has no staff order-entry path at all. Before, such a request
fell through to service_mode/items validation and failed there with a
generic message that didn't explain why an otherwise well-formed
request was rejected. Detect this case up front and return 403 with a
message naming the channel explicitly.

conventions.md updated to match, plus a stray typo (reveleriat ->
revelerait) fixed in the neighboring row.

docs: prove channel isolation with real demo accounts, fix stale references ... 72941fe514

matrice-rbac.md claimed scenarios C6/D6 (a fixed-channel role denied
the other channel) were "verified with real demo accounts on a
throwaway stack" -- nothing did that; the only test covering channel
refusal (rbac-channel.spec.js) provisions its own accounts and wasn't
even cited. Add the C6/D6 cases to rbac-demo.spec.js's existing
comptoir/drive tests, using the real comptoir@/drive@wakdo.local
accounts, and cite both spec files (plus the new DB integration test)
in section 4. Also correct that section's path for the route-matrix
test: tests/Unit/Admin/Api/RouteMatrixRoleTest.php doesn't exist, the
real file is tests/Integration/RouteMatrixRoleDbTest.php.

comptes-demo.md: the kitchen row blamed both "no fixed channel" and
"no order.create permission" for the counter/drive pages being closed
to that role. Only the missing permission does anything here --
kitchen's order_source is NULL like admin/manager's, so the fixed-
channel guard was never the relevant mechanism for this role.

Corentin scheduled this pull request to auto merge when all checks succeed 2026-09-26 11:46:36 +02:00

Corentin merged commit 6b8ecf0a72 into dev 2026-09-26 11:50:51 +02:00

Corentin referenced this pull request from a commit 2026-09-26 11:50:53 +02:00

fix: cloisonnement des canaux de commande et suivi public restreint a la borne (#156)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

auto-merge

Fusion automatique sur CI verte

No labels

auto-merge

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

AcadeNice/corentin_wakdo!156

No description provided.