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

CC: Restrict API key auth for service-to-service #1639

Closed
Tracked by #1638
alliscode opened this issue Jun 21, 2023 · 1 comment
Closed
Tracked by #1638

CC: Restrict API key auth for service-to-service #1639

alliscode opened this issue Jun 21, 2023 · 1 comment
Assignees
Labels
sk team issue A tag to denote issues that where created by the Semantic Kernel team (i.e., not the community)

Comments

@alliscode
Copy link
Member

Exposing an API key in a browser client is not recommended.
Reference: Fix security issues in copilot chat app by DavidParks8 · Pull Request #1090 · microsoft/semantic-kernel (github.com)

@alliscode alliscode converted this from a draft issue Jun 21, 2023
@alliscode alliscode added the sk team issue A tag to denote issues that where created by the Semantic Kernel team (i.e., not the community) label Jun 21, 2023
@alliscode alliscode moved this from Backlog – Copilot Dev UX to Ready in Apps & Services Semantic Kernel Jun 21, 2023
@gitri-ms gitri-ms moved this from Ready to Bugs in Apps & Services Semantic Kernel Jul 11, 2023
@gitri-ms gitri-ms moved this from Bugs to In progress in Apps & Services Semantic Kernel Jul 11, 2023
@glahaye glahaye self-assigned this Jul 27, 2023
@gitri-ms gitri-ms linked a pull request Aug 7, 2023 that will close this issue
4 tasks
github-merge-queue bot pushed a commit to microsoft/chat-copilot that referenced this issue Aug 18, 2023
### Motivation and Context
This PR addresses
microsoft/semantic-kernel#1639. It is a
combination of PRs #92 and #110

### Description

#### Backend changes
- Remove API key authorization
- Use "AzureAD" as default authentication configuration for deployments,
"None" for running locally (Note: UI changes to disable sign in flow for
the latter case are still forthcoming)
- Enable auth policy on controllers that checks if the user is part of
the conversation they are trying to access

This PR changes the contract between the frontend and backend around how
user IDs are communicated. Users who have been signing into the frontend
with AAD will now only see their chats if the backend is also gated by
AAD authentication, which was not the case previously.

#### Frontend changes
- adds `REACT_APP_AUTH_TYPE` and changes AAD variables in `.env` to be
optional
- adds `AuthHelper.IsAuthAAD` to conditionally render different elements
throughout the app
- changes user settings menu popup to instead just show as a settings
button:

![image](https://github.com/microsoft/chat-copilot/assets/52973358/342f977d-d011-464d-b122-5eff5f8222ac)

Existing users will need to uncomment `REACT_APP_AUTH_TYPE=AzureAd` in
`webapp/.env` to continue using AAD as their authorization type.


### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [Contribution
Guidelines](https://github.com/microsoft/copilot-chat/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/copilot-chat/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄

---------

Co-authored-by: Desmond Howard <[email protected]>
@gitri-ms
Copy link
Contributor

This has been fixed with microsoft/chat-copilot#126

@github-project-automation github-project-automation bot moved this from In progress to Done in Apps & Services Semantic Kernel Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sk team issue A tag to denote issues that where created by the Semantic Kernel team (i.e., not the community)
Projects
No open projects
Development

No branches or pull requests

4 participants