Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use chatId from URL rather than from payload for chats #700

Merged
merged 4 commits into from
Dec 9, 2023
Merged

Use chatId from URL rather than from payload for chats #700

merged 4 commits into from
Dec 9, 2023

Conversation

glahaye
Copy link
Contributor

@glahaye glahaye commented Dec 7, 2023

Motivation and Context

The verify access to a chat, we use HandleRequest() with the chatId provided. Currently, we get this from the payload, which can differ from the chatId from the URL, which opens us to a security problem where a user could inject an arbitrary chatId in the payload, which doesn't match what's in the URL.

Description

  • Use chatId from URL and only from URL
  • Add integrations test to validate this

Contribution Checklist

@github-actions github-actions bot added the webapi Pull requests that update .net code label Dec 7, 2023
alliscode
alliscode previously approved these changes Dec 7, 2023
@glahaye glahaye requested a review from gitri-ms December 8, 2023 17:35
@glahaye glahaye requested a review from TaoChenOSU December 9, 2023 06:09
@TaoChenOSU TaoChenOSU added this pull request to the merge queue Dec 9, 2023
Merged via the queue into microsoft:main with commit 6a31dd5 Dec 9, 2023
7 checks passed
@glahaye glahaye deleted the fix_access_bug branch March 1, 2024 00:07
teamleader-dev pushed a commit to vlink-group/chat-copilot that referenced this pull request Oct 7, 2024
### Motivation and Context
The verify access to a chat, we use HandleRequest() with the chatId
provided. Currently, we get this from the payload, which can differ from
the chatId from the URL, which opens us to a security problem where a
user could inject an arbitrary chatId in the payload, which doesn't
match what's in the URL.

### Description
- Use chatId from URL and only from URL
- Add integrations test to validate this

### Contribution Checklist
- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows the [Contribution
Guidelines](https://github.com/microsoft/chat-copilot/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/chat-copilot/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
kb0039 pushed a commit to aaronba/chat-copilot that referenced this pull request Jan 8, 2025
### Motivation and Context
The verify access to a chat, we use HandleRequest() with the chatId
provided. Currently, we get this from the payload, which can differ from
the chatId from the URL, which opens us to a security problem where a
user could inject an arbitrary chatId in the payload, which doesn't
match what's in the URL.

### Description
- Use chatId from URL and only from URL
- Add integrations test to validate this

### Contribution Checklist
- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows the [Contribution
Guidelines](https://github.com/microsoft/chat-copilot/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/chat-copilot/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
webapi Pull requests that update .net code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants