Skip to content

fixed#541

Open
ruchI9897 wants to merge 4 commits into
mainfrom
testing-preauth
Open

fixed#541
ruchI9897 wants to merge 4 commits into
mainfrom
testing-preauth

Conversation

@ruchI9897
Copy link
Copy Markdown
Contributor

No description provided.

@ruchI9897 ruchI9897 requested a review from a team as a code owner May 26, 2026 06:59
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the preauth cache success trigger in ts-embed.ts to pass an empty object instead of the data payload. The reviewer pointed out that passing an empty object {} breaks the preauth cache functionality because the embedded application relies on the session information (such as data.info). They suggested sanitizing the payload and passing { info: data.info } instead.

Comment thread src/embed/ts-embed.ts
getPreauthInfo().then((data) => {
if (data?.info) {
this.trigger(HostEvent.InfoSuccess, data);
this.trigger(HostEvent.InfoSuccess, {});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Passing an empty object {} as the payload for HostEvent.InfoSuccess will prevent the embedded ThoughtSpot application from receiving the preauth session information (such as data.info). This breaks the preauth cache functionality, as the embedded app relies on this data to populate its cache.

If the intention was to avoid exposing sensitive headers or other properties in data, you should sanitize the payload and send only the necessary fields (e.g., { info: data.info }) instead of an empty object.

Suggested change
this.trigger(HostEvent.InfoSuccess, {});
this.trigger(HostEvent.InfoSuccess, { info: data.info });

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 26, 2026

Open in StackBlitz

npm i https://pkg.pr.new/@thoughtspot/visual-embed-sdk@541

commit: f341d99

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.

1 participant