Skip to content

fix: pass WS auth token via sub-protocol instead of query param in canvas editor#1004

Open
kptdobe wants to merge 1 commit into
mainfrom
wsproto
Open

fix: pass WS auth token via sub-protocol instead of query param in canvas editor#1004
kptdobe wants to merge 1 commit into
mainfrom
wsproto

Conversation

@kptdobe

@kptdobe kptdobe commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • The canvas editor (ew-editor-doc/prose.js) was sending the Bearer token as an Authorization URL query parameter on WebSocket connections, causing it to appear verbatim in server/CDN logs
  • edit/prose/index.js already used the correct approach: passing the token as a WebSocket sub-protocol value (in sec-websocket-protocol headers), which da-collab already supports
  • This PR aligns the canvas editor to the same pattern, eliminating the token from all URLs

Test plan

Test page: https://wsproto--da-live--adobe.aem.live/

  • Open a document in the canvas editor while authenticated — confirm collab connects and syncs normally
  • Verify no Authorization=Bearer%20... appears in da-collab request logs after deploy
  • Let token expire mid-session — confirm 4401 reconnect flow still works with the new protocols assignment

🤖 Generated with Claude Code

…nvas editor

Aligns canvas editor (ew-editor-doc/prose.js) with the existing pattern in
edit/prose/index.js so the Bearer token never appears in the URL or server logs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@aem-code-sync

aem-code-sync Bot commented Jun 16, 2026

Copy link
Copy Markdown

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
In case there are problems, just click the checkbox below to rerun the respective action.

  • Re-sync branch
Commits

@kptdobe kptdobe requested a review from hannessolo June 16, 2026 06:58
return;
}
wsProvider.params = { Authorization: `Bearer ${fresh}` };
wsProvider.protocols = ['yjs', fresh];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'yjs' is hardcoded in three spots: init, the non-expired refresh path, and the expired refresh path. we could extract a const BASE_PROTOCOLS = ['yjs'] and use [...BASE_PROTOCOLS, token] so they can't drift. tiny thing but worth not having to hunt them all down later.

@kptdobe kptdobe Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while I agree with the theory, replacing 5 characters with 17 and making this less readable does not necessarily help here :)
And Claude is good at search and replace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants