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

Fix formatting of Aad url in react env variables #303

Merged
merged 1 commit into from
Sep 6, 2023

Conversation

jimmystridh
Copy link
Contributor

@jimmystridh jimmystridh commented Sep 5, 2023

Motivation and Context

The powershell script does not include the protocol when generating the REACT_APP_AAD_AUTHORITY environment variable and the .env file default is based on this notion.

Without this fix, the resulting webapp/.env will look something like this:

> cat webapp/.env
REACT_APP_BACKEND_URI=https://localhost:40443/
REACT_APP_AUTH_TYPE=AzureAd
REACT_APP_AAD_AUTHORITY=https://https://login.microsoftonline.com/XXX

so the protocol will be duplicated into an invalid url.

Description

The change removes the prefixing https:// in the configure.sh script to match the behavior of the Configure.ps1 script.

Contribution Checklist

this matches the behavior of the .ps1 script and the default values in scripts/.env
@crickman crickman assigned glahaye and TaoChenOSU and unassigned glahaye and TaoChenOSU Sep 6, 2023
@crickman crickman added the deployment Issues related to deploying Chat-Copilot label Sep 6, 2023
@crickman crickman added the external dependency issue Issues related to external dependencies (e.g. Azure) label Sep 6, 2023
@gitri-ms gitri-ms added this pull request to the merge queue Sep 6, 2023
@gitri-ms
Copy link
Collaborator

gitri-ms commented Sep 6, 2023

Thanks @jimmystridh for catching this!

Merged via the queue into microsoft:main with commit 6f6ed60 Sep 6, 2023
5 checks passed
teamleader-dev pushed a commit to vlink-group/chat-copilot that referenced this pull request Oct 7, 2024
### Motivation and Context

The powershell script does not include the protocol when generating the
`REACT_APP_AAD_AUTHORITY` environment variable and the .env file default
is based on this notion.

Without this fix, the resulting webapp/.env will look something like
this:
```
> cat webapp/.env
REACT_APP_BACKEND_URI=https://localhost:40443/
REACT_APP_AUTH_TYPE=AzureAd
REACT_APP_AAD_AUTHORITY=https://https://login.microsoftonline.com/XXX
```

so the protocol will be duplicated into an invalid url.

### Description

The change removes the prefixing `https://` in the `configure.sh` script
to match the behavior of the `Configure.ps1` script.

### Contribution Checklist

- [x] The code builds clean without any errors or warnings
- [x] 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
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployment Issues related to deploying Chat-Copilot external dependency issue Issues related to external dependencies (e.g. Azure)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants