Skip to content

[Chat] Verify the fetch endpoints match#4251

Merged
JoshuaLai merged 19 commits intomainfrom
feature/inlineimage-authfetch
Mar 12, 2024
Merged

[Chat] Verify the fetch endpoints match#4251
JoshuaLai merged 19 commits intomainfrom
feature/inlineimage-authfetch

Conversation

@JoshuaLai
Copy link
Copy Markdown
Member

What

Verify the endpoints match before making the call

Why

Adding security to the fetch call to not pass the token into a call sourced from a different endpoint.

How Tested

Running call with chat scenario all inline images should work as expected

Process & policy checklist

  • I have updated the project documentation to reflect my changes if necessary.
  • I have read the CONTRIBUTING documentation.

Is this a breaking change?

  • This change causes current functionality to break.

Comment thread packages/chat-stateful-client/src/ChatContext.ts
Comment thread packages/chat-stateful-client/src/ResourceDownloadQueue.ts Fixed
Comment thread packages/chat-stateful-client/src/ResourceDownloadQueue.ts Outdated
editedMessage.content = { message: 'edited message', attachments: secondAttachments };

const queue = new ResourceDownloadQueue(context, tokenCredential);
const queue = new ResourceDownloadQueue(context, { credential: tokenCredential, endpoint: 'endpoint' });
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.

Let's not use magic text?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is a test file, so we kind of have magic text everywhere. Should we just refactor the whole test file?

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.

Extracting this to a const value doesn't seem like much.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 11, 2024

CallWithChat bundle size is increased❗.

  • Current size: 5953280
  • Base size: 5953070
  • Diff size: 210

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 11, 2024

Calling bundle size is not changed.

  • Current size: 4783304
  • Base size: 4783304
  • Diff size: 0

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 11, 2024

Chat bundle size is increased❗.

  • Current size: 1895136
  • Base size: 1894926
  • Diff size: 210

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@JoshuaLai JoshuaLai marked this pull request as ready for review March 11, 2024 21:47
@JoshuaLai JoshuaLai requested review from a team as code owners March 11, 2024 21:47
@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 11, 2024

@azure/communication-react jest test coverage for stable.

Lines Statements Functions Branches
Base 23106 / 36051
64.09%
23106 / 36051
64.09%
635 / 1105
57.46%
1868 / 2939
63.55%
Current 23208 / 36070
64.34%
23208 / 36070
64.34%
635 / 1105
57.46%
1873 / 2946
63.57%
Diff 102 / 19
0.25%
102 / 19
0.25%
0 / 0
0%
5 / 7
0.02%

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@JoshuaLai JoshuaLai enabled auto-merge (squash) March 12, 2024 20:37
@JoshuaLai JoshuaLai merged commit c1d7a43 into main Mar 12, 2024
@JoshuaLai JoshuaLai deleted the feature/inlineimage-authfetch branch March 12, 2024 20:38
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.

5 participants