Conversation
…l for improved token management
✅ Deploy Preview for afmg ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR updates Dropbox authentication flow by replacing direct window.opener callbacks / DOM events with a BroadcastChannel-based handshake between the auth popup and the main app, aiming to improve token/error propagation and reduce tight coupling between windows.
Changes:
- Switch Dropbox auth completion signaling from a custom window event to
BroadcastChannelmessaging inCloud.providers.dropbox.auth(). - Update
public/dropbox.htmlto send token/error back viaBroadcastChanneland close itself. - Minor
package-lock.jsonmetadata changes (adds"peer": truein a few entries).
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| public/modules/io/cloud.js | Receives auth results via BroadcastChannel, sets token, and resolves/rejects the auth Promise. |
| public/dropbox.html | Sends token/error messages over BroadcastChannel instead of calling window.opener APIs. |
| package-lock.json | Adds "peer": true flags to several lockfile package entries. |
| channel.close(); | ||
| window.close(); | ||
| }) | ||
| .catch(error => { | ||
| console.error(error); |
There was a problem hiding this comment.
If posting the token fails or getAccessTokenFromCode rejects, the opener tab will wait until its 120s timeout because this window never reports the failure via the channel. Consider reusing returnError(...) in the .catch(...) (and similarly in startAuth().catch(...)) so the opener can reject immediately and the auth window can close.
| const channel = new BroadcastChannel("dropbox-auth"); | ||
| channel.onmessage = async ({data}) => { | ||
| channel.close(); | ||
| clearTimeout(watchDog); |
There was a problem hiding this comment.
auth() opens a BroadcastChannel but the timeout path only closes authWindow and rejects; the channel remains open and can still deliver a late message after the Promise is settled. Consider tracking a settled flag and closing the channel / removing the handler in the watchdog (and ignoring messages if already settled) to avoid leaks and racey state updates.
| channel.onmessage = async ({data}) => { | ||
| channel.close(); | ||
| clearTimeout(watchDog); | ||
| resolve(); | ||
| }); | ||
| if (data.type === "token") { | ||
| await this.setDropBoxToken(data.token); |
There was a problem hiding this comment.
channel.onmessage is async and awaits setDropBoxToken(). If setDropBoxToken / connect throws, the exception becomes an unhandled rejection and the outer auth() Promise will never resolve/reject. Wrap the body in try/catch and call reject(err) (and show an error) on failure.
| if (data.type === "token") { | ||
| await this.setDropBoxToken(data.token); | ||
| resolve(); | ||
| } else { | ||
| this.returnError(data.description); | ||
| reject(new Error(data.description)); | ||
| } |
There was a problem hiding this comment.
The message handler assumes data.description is always a string. If error_description is missing (or a malformed message is received), returnError will throw on replaceAll and the rejection will be confusing. Guard the message schema here (e.g., default the description, and validate token exists before calling setDropBoxToken).
| const channel = new BroadcastChannel("dropbox-auth"); | ||
| channel.onmessage = async ({data}) => { | ||
| channel.close(); | ||
| clearTimeout(watchDog); | ||
| resolve(); | ||
| }); | ||
| if (data.type === "token") { | ||
| await this.setDropBoxToken(data.token); |
There was a problem hiding this comment.
Using a fixed channel name ("dropbox-auth") broadcasts the access token to any same-origin context currently listening on that channel (e.g., another open FMG tab doing Dropbox auth). This can cause cross-tab interference (wrong tab resolves, orphaned auth windows) and unnecessarily widens token exposure. Consider generating a per-auth nonce/channel name and passing it to dropbox.html (e.g., via query string) so only the initiating window receives the token/error.
| if (code) getToken(); | ||
| else if (error) window.opener.Cloud.providers.dropbox.returnError(params.get("error_description")); | ||
| else if (error) returnError(params.get("error_description")); | ||
| else startAuth(); |
There was a problem hiding this comment.
params.get("error_description") can be null, which will then be broadcast and eventually crash returnError in the opener (it calls replaceAll on the value). Provide a fallback string before calling returnError (and/or coerce description to a string inside returnError).
…l for improved token management
Description