fix(ENG-12199): extend allowed origins and set default mode#103
fix(ENG-12199): extend allowed origins and set default mode#103adefreitas merged 3 commits intoStackOneHQ:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends origin validation for cross-origin messaging in the integration picker and sets a default mode when none is provided. The changes address ENG-12199 by allowing messages from both the current window origin and the dashboard URL origin, and by defaulting to 'integration-picker' mode when no mode is specified.
Changes:
- Extended
allowedOriginsto include bothwindow.location.originand thedashboardUrlorigin for message validation - Removed the "No mode selected" error UI and defaulted
modeto 'integration-picker' when undefined - Updated origin validation from single-origin check to Set-based multiple origins check
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/modules/integration-picker/hooks/useIntegrationPicker.ts | Added allowedOrigins memoized Set to validate incoming messages from multiple origins (current origin and dashboard URL) |
| src/StackOneHub.tsx | Removed "no mode selected" validation and set default mode to 'integration-picker' when undefined |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const processMessageCallback = useCallback( | ||
| (event: MessageEvent) => { | ||
| if (event.origin !== window.location.origin) { | ||
| if (!allowedOrigins.has(event.origin)) { | ||
| return; | ||
| } | ||
|
|
||
| if (!event.data?.type) { | ||
| return; | ||
| } | ||
| if (event.data.type === EventType.AccountConnected) { | ||
| handleSuccess({ id: event.data.account.id, provider: event.data.account.provider }); | ||
| parent.postMessage(event.data, '*'); | ||
| } else if (event.data.type === EventType.CloseOAuth2) { | ||
| if (event.data.error) { | ||
| setConnectionState({ | ||
| loading: false, | ||
| success: false, | ||
| error: { | ||
| message: event.data.error, | ||
| provider_response: event.data.errorDescription || 'No description', | ||
| }, | ||
| }); | ||
| } else { | ||
| setConnectionState({ loading: false, success: false, error: undefined }); | ||
| } | ||
| } | ||
|
|
||
| if (connectWindow.current) { | ||
| connectWindow.current.close(); | ||
| connectWindow.current = null; | ||
| } | ||
|
|
||
| window.removeEventListener('message', processMessageCallback, false); | ||
| }, | ||
| [handleSuccess], | ||
| ); |
There was a problem hiding this comment.
The processMessageCallback dependency array is missing allowedOrigins. Since allowedOrigins is a memoized Set that changes when dashboardUrl changes, the callback should include it in its dependencies to ensure it always uses the latest allowed origins for validation. Without this, the callback will continue using stale origin validation if dashboardUrl changes.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
No description provided.