Skip to content
This repository was archived by the owner on Aug 20, 2026. It is now read-only.

Change AAD authorization to be opt-in within webapp - #110

Closed
Desmond Howard (dehoward) wants to merge 1 commit into
microsoft:mainfrom
dehoward:no-auth-webapp
Closed

Desmond Howard (dehoward) wants to merge 1 commit into
microsoft:mainfrom
dehoward:no-auth-webapp

Conversation

@dehoward

@dehoward Desmond Howard (dehoward) commented Aug 5, 2023 •

Copy link
Copy Markdown

Description

dependent on #92.
removing AAD as the default authorization method will allow users to get the app running more quickly.

breaking change: existing users will need to uncomment REACT_APP_AUTH_TYPE=AzureAd in webapp/.env to continue using AAD as their authorization type.

Contribution Checklist

  • 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

@dehoward Desmond Howard (dehoward) added the PR: breaking change Pull requests that introduce breaking changes label Aug 5, 2023
@github-actions github-actions Bot added webapp Pull requests that update Typescript code PR: ready for review labels Aug 5, 2023
@dehoward Desmond Howard (dehoward) changed the title Change AAD authorization to opt-in within webapp Change AAD authorization to be opt-in within webapp Aug 5, 2023
@gitri-ms

Copy link
Copy Markdown
Contributor

Closing this PR as it is now covered by #126

github-merge-queue Bot pushed a commit that referenced this pull request 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]>
Team Leader (teamleader-dev) pushed a commit to vlink-group/chat-copilot that referenced this pull request Oct 7, 2024
### Motivation and Context
This PR addresses
microsoft/semantic-kernel#1639. It is a
combination of PRs microsoft#92 and microsoft#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]>
Kyle Beachem (kb0039) pushed a commit to aaronba/chat-copilot that referenced this pull request Jan 8, 2025
### Motivation and Context
This PR addresses
microsoft/semantic-kernel#1639. It is a
combination of PRs microsoft#92 and microsoft#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]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

PR: breaking change Pull requests that introduce breaking changes webapp Pull requests that update Typescript code

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

2 participants