fix(routerlicious): enforce scope checks on PATCH /root endpoint (#27…#27097
fix(routerlicious): enforce scope checks on PATCH /root endpoint (#27…#27097sonalideshpandemsft merged 2 commits intorelease/server/6.0from
Conversation
) The `PATCH /:tenantId/:id/root` endpoint in Alfred's REST API called `verifyToken()` without passing the `requiredScopes` parameter. This meant any valid JWT — including tokens with only `doc:read` scope — could write arbitrary key-value data into a document's root map. The injected operations were permanently sequenced into the document's op log and replayed by all connected clients. The fix threads a `requiredScopes` parameter through the `verifyRequest` → `verifyTokenWrapper` → `verifyToken` call chain and passes `[ScopeType.DocRead, ScopeType.DocWrite]` from the PATCH handler. This matches the scope enforcement already used by the adjacent `POST /broadcast-signal` endpoint via `verifyStorageToken`. Two new tests confirm that: - A `doc:read`-only token is rejected with 403 - A `doc:write`-only token (missing `doc:read`) is also rejected with 403 The existing "process normally" test with a full-scope token continues to pass, confirming no regression for legitimate callers. The review process is outlined on [this wiki page](https://github.com/microsoft/FluidFramework/wiki/PR-Guidelines#guidelines). - This is a security fix (MSRC 113235). The changes are intentionally minimal and surgical — 4 hunks in the production file, all adding a single `requiredScopes` parameter through the existing function chain. - `verifyRequest` and `verifyTokenWrapper` are module-private (not exported). The `create()` export signature is unchanged. Zero consumer impact. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Warning WARNING: This PR is targeting a release branch! All changes must first be merged into Changes to release branches require approval from the Patch Triage group before merging. For more details, see our internal documentation for the patch policy and processes for |
7d7d84a to
a0fadbd
Compare
| } | ||
| } | ||
|
|
||
| if (requiredScopes) { |
There was a problem hiding this comment.
The PATCH /:tenantId/:id/root endpoint in Alfred's REST API had no scope enforcement — any valid JWT (even doc:read-only) could write to a document's root map.
PR #27073 added requiredScopes plumbing through verifyRequest → verifyTokenWrapper → verifyToken and passed [ScopeType.DocRead, ScopeType.DocWrite] from the PATCH handler. However, on the release/server/6.0 branch, verifyToken() in @fluidframework/server-services-utils didn't have a requiredScopes parameter — that was added on main by PR #24853. So the scopes were silently ignored and tests expecting 403 got 200.
The additional fix (in auth.ts) backports the minimal piece from #24853:
- Adds
requiredScopes?: string[]as an optional parameter toverifyToken() - Adds scope validation logic: checks that every required scope is present in the token's
claims.scopes, throwing a403 NetworkErrorif not
…073)
The
PATCH /:tenantId/:id/rootendpoint in Alfred's REST API calledverifyToken()without passing therequiredScopesparameter. This meant any valid JWT — including tokens with onlydoc:readscope — could write arbitrary key-value data into a document's root map. The injected operations were permanently sequenced into the document's op log and replayed by all connected clients.The fix threads a
requiredScopesparameter through theverifyRequest→verifyTokenWrapper→verifyTokencall chain and passes[ScopeType.DocRead, ScopeType.DocWrite]from the PATCH handler. This matches the scope enforcement already used by the adjacentPOST /broadcast-signalendpoint viaverifyStorageToken.Two new tests confirm that:
doc:read-only token is rejected with 403doc:write-only token (missingdoc:read) is also rejected with 403The existing "process normally" test with a full-scope token continues to pass, confirming no regression for legitimate callers.
The review process is outlined on this wiki
page.
requiredScopesparameter through the existing function chain.verifyRequestandverifyTokenWrapperare module-private (not exported). Thecreate()export signature is unchanged. Zero consumer impact.