Require caller authorization for ACS token and room endpoints - #291
Merged
Ankesh N Bhoi (ankeshni) merged 1 commit intoAug 5, 2026
Merged
Conversation
The dev server holds the ACS resource connection string, but its routes had no authorization layer, and `npm start` binds 0.0.0.0 with `.azurewebsites.net` in allowedHosts. Any remote caller could mint a VoIP token for an arbitrary communicationUserId, create rooms, or re-role participants in a known room. Add a server-side session keyed by an opaque HttpOnly cookie and use it to authorize the privileged routes: - POST /getCommunicationUserToken still provisions new identities anonymously, but only re-mints a token for an identity the session already owns. - POST /createRoom requires a session holding an ACS identity, records the creator, validates participant IDs, caps participant count, and coerces pstnDialOutEnabled to a strict boolean. - PATCH /updateParticipant requires the session to be the room creator or to hold an identity that is already a Presenter in the room, and validates the requested role against an allowlist. The cookie is SameSite=Strict so these cookie-authorized, state-changing routes are not CSRF-able, and Secure when served over HTTPS. Also replace the Math.random() GUID used for OneSignal registration tokens with crypto.randomUUID(), since those tokens can be exchanged for ACS access tokens, and return JSON 500s from the room handlers instead of rethrowing, which surfaced stack traces through default error handling. Addresses CWE-639 and CWE-862 alerts on Project/webpack.config.js.
Victor Rios (vriosrada-msft)
self-requested a review
August 4, 2026 20:42
Victor Rios (vriosrada-msft)
approved these changes
Aug 4, 2026
prabhjot-msft
approved these changes
Aug 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the CWE-639 (authorization bypass through user-controlled key) and CWE-862 (missing authorization) alerts on
Project/webpack.config.js— 4 alerts covering 2 root issues.Problem
The dev server holds the ACS resource connection string, but its routes had no authorization layer — the only middleware ahead of them was
bodyParser.json(). This is not dead sample code:npm startbindswebpack-dev-serverto0.0.0.0,allowedHostsadmits.azurewebsites.net, and the README documents Azure App Service deployment.Any remote caller could:
communicationUserIdviaPOST /getCommunicationUserToken, impersonating any identity in the resource and consuming the owner's ACS quota.POST /createRoomandPATCH /updateParticipant, using the server as a confused deputy. An attacker needed no existing participant ID, since the same server would mint them one.Fix
A server-side session keyed by an opaque HttpOnly cookie, so each privileged route can prove the caller owns the identity or room it is acting on.
POST /getCommunicationUserToken403.POST /createRoompstnDialOutEnabledto a strict boolean.PATCH /updateParticipantPresenterin the room. Role validated against an allowlist.Supporting changes:
generateGuid()usedMath.random()for OneSignal registration tokens, which are exchangeable for ACS access tokens via/getCommunicationUserTokenForOneSignalRegistrationToken— guessing one bypassed the new checks entirely. Nowcrypto.randomUUID().throw e, leaking stack traces through default error handling and causing unhandled rejections. Now a JSON 500.SameSite=Strict(these are cookie-authorized state-changing routes, so CSRF would otherwise be the next bypass) andSecurebehind HTTPS. Sessions carry an 8h TTL and a 1000-entry cap to bound memory.Utils.js: surfaces the server's message on failure instead ofRequest failed with status code 403, andcreateRoomno longer throws aTypeErroronerror.response.datawhen the response is absent.Behavior change worth reviewing
Supplying a
communicationUserIdwithout a token now only re-mints within the session that created that identity. Cross-session reuse was precisely the impersonation vector, and the sample has no authentication to prove ownership otherwise. The "bring your own token + user ID" login path is unaffected, and the session cookie persists across page reloads so a normal dev session is unchanged.Verification
The real handlers were exercised against stubbed ACS clients — 17 checks, all passing: