Skip to content

Commit de8f8ab

Browse files
dehowarddehoward
authored
Fetch AAD authentication info from backend (microsoft#427)
### Motivation and Context <!-- Thank you for your contribution to the chat-copilot repo! Please help reviewers and future users, providing the following information: 1. Why is this change required? 2. What problem does it solve? 3. What scenario does it contribute to? 4. If it fixes an open issue, please link to the issue here. --> this PR prepares the frontend for microsoft#377 and removes the need for any AAD configuration environment variables. ### Description - removes `REACT_APP_AUTH_TYPE` and all variables starting with `REACT_APP_AAD_` - calls the `/authConfig` endpoint when the app first loads and if needed, renders the `MsalProvider` using the fetched config. - updates workflows and deployment scripts accordingly <!-- Describe your changes, the overall approach, the underlying design. These notes will help understanding how your code works. Thanks! --> ### 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/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 😄
1 parent 2241167 commit de8f8ab

22 files changed

+189
-284
lines changed

‎.github/workflows/copilot-build-images.yml

-12
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,6 @@ on:
66
REACT_APP_BACKEND_URI:
77
required: true
88
type: string
9-
REACT_APP_AAD_AUTHORITY:
10-
required: true
11-
type: string
12-
REACT_APP_AAD_CLIENT_ID:
13-
required: true
14-
type: string
15-
REACT_APP_AAD_API_SCOPE:
16-
required: false
17-
type: string
189
env:
1910
REGISTRY: ghcr.io
2011

@@ -38,9 +29,6 @@ jobs:
3829
image: ${{ github.repository }}-webapp-nginx
3930
build-args: |
4031
REACT_APP_BACKEND_URI=${{ inputs.REACT_APP_BACKEND_URI }}
41-
REACT_APP_AAD_AUTHORITY=${{ inputs.REACT_APP_AAD_AUTHORITY }}
42-
REACT_APP_AAD_CLIENT_ID=${{ inputs.REACT_APP_AAD_CLIENT_ID }}
43-
REACT_APP_AAD_API_SCOPE=${{ inputs.REACT_APP_AAD_API_SCOPE }}
4432
permissions:
4533
contents: read
4634
packages: write

‎.github/workflows/copilot-test-e2e.yml

-5
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,6 @@ jobs:
7171
env:
7272
REACT_APP_BACKEND_URI: https://localhost:40443/
7373

74-
REACT_APP_AUTH_TYPE: AzureAd
75-
REACT_APP_AAD_AUTHORITY: https://login.microsoftonline.com/${{ secrets.COPILOT_CHAT_TEST_TENANT_ID }}
76-
REACT_APP_AAD_CLIENT_ID: ${{ secrets.COPILOT_CHAT_TEST_APP_AAD_WEBAPP_CLIENT_ID }}
77-
REACT_APP_AAD_API_SCOPE: api://${{ secrets.COPILOT_CHAT_TEST_APP_AAD_WEBAPI_CLIENT_ID }}/access_as_user
78-
7974
REACT_APP_TEST_USER_ACCOUNT1: ${{ secrets.COPILOT_CHAT_TEST_USER_ACCOUNT1 }}
8075
REACT_APP_TEST_USER_ACCOUNT1_INITIALS: ${{ secrets.COPILOT_CHAT_TEST_USER_ACCOUNT1_INITIALS }}
8176
REACT_APP_TEST_USER_ACCOUNT2: ${{ secrets.COPILOT_CHAT_TEST_USER_ACCOUNT2 }}

‎docker/webapp/Dockerfile.nginx

+1-10
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,12 @@
11
# source webapp/.env
2-
# docker build --build-arg REACT_APP_BACKEND_URI=$REACT_APP_BACKEND_URI --build-arg REACT_APP_AAD_AUTHORITY=$REACT_APP_AAD_AUTHORITY --build-arg REACT_APP_AAD_CLIENT_ID=$REACT_APP_AAD_CLIENT_ID --build-arg REACT_APP_AAD_API_SCOPE=$REACT_APP_AAD_API_SCOPE -f docker/webapp/Dockerfile.nginx -t chat-copilot-webapp-nginx .
2+
# docker build --build-arg REACT_APP_BACKEND_URI=$REACT_APP_BACKEND_URI -f docker/webapp/Dockerfile.nginx -t chat-copilot-webapp-nginx .
33

44
# builder
55
FROM node:lts-alpine as builder
66

77
ARG REACT_APP_BACKEND_URI
88
ENV REACT_APP_BACKEND_URI $REACT_APP_BACKEND_URI
99

10-
ARG REACT_APP_AAD_AUTHORITY
11-
ENV REACT_APP_AAD_AUTHORITY $REACT_APP_AAD_AUTHORITY
12-
13-
ARG REACT_APP_AAD_CLIENT_ID
14-
ENV REACT_APP_AAD_CLIENT_ID $REACT_APP_AAD_CLIENT_ID
15-
16-
ARG REACT_APP_AAD_API_SCOPE
17-
ENV REACT_APP_AAD_API_SCOPE $REACT_APP_AAD_API_SCOPE
18-
1910
WORKDIR /app
2011
COPY webapp/ .
2112
RUN yarn install \

‎scripts/Configure.ps1

-8
Original file line numberDiff line numberDiff line change
@@ -217,14 +217,6 @@ $webappEnvFilePath = Join-Path "$webappProjectPath" '/.env'
217217
Write-Host "Setting up '.env'..."
218218
Set-Content -Path $webappEnvFilePath -Value "REACT_APP_BACKEND_URI=https://localhost:40443/"
219219

220-
if ($authType -eq $varAzureAd) {
221-
Write-Host "Configuring Azure AD authentication..."
222-
Add-Content -Path $webappEnvFilePath -Value "REACT_APP_AUTH_TYPE=AzureAd"
223-
Add-Content -Path $webappEnvFilePath -Value "REACT_APP_AAD_AUTHORITY=$($Instance.Trim("/"))/$TenantId"
224-
Add-Content -Path $webappEnvFilePath -Value "REACT_APP_AAD_CLIENT_ID=$FrontendClientId"
225-
Add-Content -Path $webappEnvFilePath -Value "REACT_APP_AAD_API_SCOPE=api://$BackendClientId/access_as_user"
226-
}
227-
228220
Write-Host "($webappEnvFilePath)"
229221
Write-Host "========"
230222
Get-Content $webappEnvFilePath | Write-Host

‎scripts/configure.sh

+3-10
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,9 @@ APPSETTINGS_OVERRIDES="{
206206
\"EmbeddingGeneratorType\": \"${AI_SERVICE}\"
207207
},
208208
\"Services\": ${AISERVICE_OVERRIDES}
209+
},
210+
\"Frontend\": {
211+
\"AadClientId\": \"${FRONTEND_CLIENT_ID}\"
209212
}
210213
}"
211214
APPSETTINGS_OVERRIDES_FILEPATH="${WEBAPI_PROJECT_PATH}/appsettings.${ENV_ASPNETCORE}.json"
@@ -229,16 +232,6 @@ WEBAPP_ENV_FILEPATH="${WEBAPP_PROJECT_PATH}/.env"
229232
echo "Setting up '.env' for webapp..."
230233
echo "REACT_APP_BACKEND_URI=https://localhost:40443/" >$WEBAPP_ENV_FILEPATH
231234

232-
if [ "$AUTH_TYPE" = "$ENV_AZURE_AD" ]; then
233-
echo "Configuring Azure AD authentication..."
234-
echo "REACT_APP_AUTH_TYPE=AzureAd" >>$WEBAPP_ENV_FILEPATH
235-
# Trim any trailing slash from instance before generating authority
236-
INSTANCE=${INSTANCE%/}
237-
echo "REACT_APP_AAD_AUTHORITY=$INSTANCE/$TENANT_ID" >>$WEBAPP_ENV_FILEPATH
238-
echo "REACT_APP_AAD_CLIENT_ID=$FRONTEND_CLIENT_ID" >>$WEBAPP_ENV_FILEPATH
239-
echo "REACT_APP_AAD_API_SCOPE=api://$BACKEND_CLIENT_ID/access_as_user" >>$WEBAPP_ENV_FILEPATH
240-
fi
241-
242235
echo "($WEBAPP_ENV_FILEPATH)"
243236
echo "========"
244237
cat $WEBAPP_ENV_FILEPATH

‎scripts/deploy/README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ This document details how to deploy Chat Copilot's required resources to your Az
1111

1212
- `F1` and `D1` SKUs for the App Service Plans are not currently supported for this deployment in order to support private networking.
1313

14-
- Chat Copilot deployments use Azure Active Directory for authentication. All endpoints (except `/healthz`) require authentication to access.
14+
- Chat Copilot deployments use Azure Active Directory for authentication. All endpoints (except `/healthz` and `/authInfo`) require authentication to access.
1515

1616
# Configure your environment
1717

‎scripts/deploy/deploy-webapp.ps1

-10
Original file line numberDiff line numberDiff line change
@@ -65,22 +65,12 @@ foreach ($pluginName in $pluginNames) {
6565
Write-Host "pluginName: $pluginName"
6666
}
6767

68-
$webapiSettings = $(az webapp config appsettings list --name $webapiName --resource-group $ResourceGroupName | ConvertFrom-JSON)
69-
$webapiClientId = ($webapiSettings | Where-Object -Property name -EQ -Value Authentication:AzureAd:ClientId).value
70-
$webapiTenantId = ($webapiSettings | Where-Object -Property name -EQ -Value Authentication:AzureAd:TenantId).value
71-
$webapiInstance = ($webapiSettings | Where-Object -Property name -EQ -Value Authentication:AzureAd:Instance).value
72-
$webapiScope = ($webapiSettings | Where-Object -Property name -EQ -Value Authentication:AzureAd:Scopes).value
73-
7468
# Set ASCII as default encoding for Out-File
7569
$PSDefaultParameterValues['Out-File:Encoding'] = 'ascii'
7670

7771
$envFilePath = "$PSScriptRoot/../../webapp/.env"
7872
Write-Host "Writing environment variables to '$envFilePath'..."
7973
"REACT_APP_BACKEND_URI=https://$webapiUrl/" | Out-File -FilePath $envFilePath
80-
"REACT_APP_AUTH_TYPE=AzureAd" | Out-File -FilePath $envFilePath -Append
81-
"REACT_APP_AAD_AUTHORITY=$($webapiInstance.Trim("/"))/$webapiTenantId" | Out-File -FilePath $envFilePath -Append
82-
"REACT_APP_AAD_CLIENT_ID=$FrontendClientId" | Out-File -FilePath $envFilePath -Append
83-
"REACT_APP_AAD_API_SCOPE=api://$webapiClientId/$webapiScope" | Out-File -FilePath $envFilePath -Append
8474
"REACT_APP_SK_VERSION=$Version" | Out-File -FilePath $envFilePath -Append
8575
"REACT_APP_SK_BUILD_INFO=$VersionInfo" | Out-File -FilePath $envFilePath -Append
8676

‎scripts/deploy/deploy-webapp.sh

+3-15
Original file line numberDiff line numberDiff line change
@@ -102,23 +102,11 @@ echo "WEB_API_NAME: $WEB_API_NAME"
102102
eval PLUGIN_NAMES=$(echo $DEPLOYMENT_JSON | jq -r '.properties.outputs.pluginNames.value[]')
103103
echo "PLUGIN_NAMES: $PLUGIN_NAMES"
104104

105-
WEB_API_SETTINGS=$(az webapp config appsettings list --name $WEB_API_NAME --resource-group $RESOURCE_GROUP --output json)
106-
eval WEB_API_CLIENT_ID=$(echo $WEB_API_SETTINGS | jq '.[] | select(.name=="Authentication:AzureAd:ClientId").value')
107-
eval WEB_API_TENANT_ID=$(echo $WEB_API_SETTINGS | jq '.[] | select(.name=="Authentication:AzureAd:TenantId").value')
108-
eval WEB_API_INSTANCE=$(echo $WEB_API_SETTINGS | jq '.[] | select(.name=="Authentication:AzureAd:Instance").value')
109-
eval WEB_API_SCOPE=$(echo $WEB_API_SETTINGS | jq '.[] | select(.name=="Authentication:AzureAd:Scopes").value')
110-
111105
ENV_FILE_PATH="$SCRIPT_ROOT/../../webapp/.env"
112106
echo "Writing environment variables to '$ENV_FILE_PATH'..."
113-
echo "REACT_APP_BACKEND_URI=https://$WEB_API_URL/" >$ENV_FILE_PATH
114-
echo "REACT_APP_AUTH_TYPE=AzureAd" >>$ENV_FILE_PATH
115-
# Trim any trailing slash from instance before generating authority
116-
WEB_API_INSTANCE=${WEB_API_INSTANCE%/}
117-
echo "REACT_APP_AAD_AUTHORITY=$WEB_API_INSTANCE/$WEB_API_TENANT_ID" >>$ENV_FILE_PATH
118-
echo "REACT_APP_AAD_CLIENT_ID=$FRONTEND_CLIENT_ID" >>$ENV_FILE_PATH
119-
echo "REACT_APP_AAD_API_SCOPE=api://$WEB_API_CLIENT_ID/$WEB_API_SCOPE" >>$ENV_FILE_PATH
120-
echo "REACT_APP_SK_VERSION=$VERSION" >>$ENV_FILE_PATH
121-
echo "REACT_APP_SK_BUILD_INFO=$VERSION_INFO" >>$ENV_FILE_PATH
107+
echo "REACT_APP_BACKEND_URI=https://$WEB_API_URL/" > $ENV_FILE_PATH
108+
echo "REACT_APP_SK_VERSION=$VERSION" >> $ENV_FILE_PATH
109+
echo "REACT_APP_SK_BUILD_INFO=$VERSION_INFO" >> $ENV_FILE_PATH
122110

123111
echo "Writing swa-cli.config.json..."
124112
SWA_CONFIG_FILE_PATH="$SCRIPT_ROOT/../../webapp/swa-cli.config.json"

‎webapi/Controllers/MaintenanceController.cs

+1-5
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
using System.Threading;
44
using System.Threading.Tasks;
5-
using CopilotChat.WebApi.Auth;
65
using CopilotChat.WebApi.Models.Response;
76
using CopilotChat.WebApi.Options;
87
using CopilotChat.WebApi.Services.MemoryMigration;
@@ -23,19 +22,16 @@ public class MaintenanceController : ControllerBase
2322

2423
private readonly ILogger<MaintenanceController> _logger;
2524
private readonly IOptions<ServiceOptions> _serviceOptions;
26-
private readonly IAuthInfo _authInfo;
2725

2826
/// <summary>
2927
/// Initializes a new instance of the <see cref="MaintenanceController"/> class.
3028
/// </summary>
3129
public MaintenanceController(
3230
ILogger<MaintenanceController> logger,
33-
IOptions<ServiceOptions> serviceOptions,
34-
IAuthInfo authInfo)
31+
IOptions<ServiceOptions> serviceOptions)
3532
{
3633
this._logger = logger;
3734
this._serviceOptions = serviceOptions;
38-
this._authInfo = authInfo;
3935
}
4036

4137
/// <summary>

‎webapp/.env.example

-9
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,6 @@
11
# Required Variables
2-
# If you add any new required variables, make sure you update the variables list in the checkEnv.ts file as well.
32
REACT_APP_BACKEND_URI=https://localhost:40443/
43

5-
# To enable authorization using Azure Active Directory, uncomment the following variables
6-
# See paragraph "(Optional) Enable backend authorization via Azure AD" in README.md for details and setup
7-
# REACT_APP_AUTH_TYPE=AzureAd
8-
# REACT_APP_AAD_AUTHORITY=https://login.microsoftonline.com/{YOUR_TENANT_ID}
9-
# REACT_APP_AAD_CLIENT_ID=
10-
# Authorization scopes to access webapi when using Azure AD authorization.
11-
# REACT_APP_AAD_API_SCOPE=
12-
134
# To enable HTTPS, uncomment the following variables
145
# HTTPS="true"
156
# Replace with your locally-trusted cert file

‎webapp/src/App.tsx

+50-50
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { AuthenticatedTemplate, UnauthenticatedTemplate, useIsAuthenticated, use
44
import { FluentProvider, Subtitle1, makeStyles, shorthands, tokens } from '@fluentui/react-components';
55

66
import * as React from 'react';
7-
import { FC, useEffect } from 'react';
7+
import { useEffect } from 'react';
88
import { UserSettingsMenu } from './components/header/UserSettingsMenu';
99
import { PluginGallery } from './components/open-api-plugins/PluginGallery';
1010
import { BackendProbe, ChatView, Error, Loading, Login } from './components/views';
@@ -50,20 +50,21 @@ export const useClasses = makeStyles({
5050
enum AppState {
5151
ProbeForBackend,
5252
SettingUserInfo,
53+
ErrorLoadingChats,
5354
ErrorLoadingUserInfo,
5455
LoadingChats,
5556
Chat,
5657
SigningOut,
5758
}
5859

59-
const App: FC = () => {
60+
const App = () => {
6061
const classes = useClasses();
6162

6263
const [appState, setAppState] = React.useState(AppState.ProbeForBackend);
6364
const dispatch = useAppDispatch();
6465

6566
const { instance, inProgress } = useMsal();
66-
const { activeUserInfo, features, isMaintenance } = useAppSelector((state: RootState) => state.app);
67+
const { features, isMaintenance } = useAppSelector((state: RootState) => state.app);
6768
const isAuthenticated = useIsAuthenticated();
6869

6970
const chat = useChat();
@@ -75,48 +76,45 @@ const App: FC = () => {
7576
return;
7677
}
7778

78-
if (isAuthenticated) {
79-
if (appState === AppState.SettingUserInfo) {
80-
if (activeUserInfo === undefined) {
81-
const account = instance.getActiveAccount();
82-
if (!account) {
83-
setAppState(AppState.ErrorLoadingUserInfo);
84-
} else {
85-
dispatch(
86-
setActiveUserInfo({
87-
id: `${account.localAccountId}.${account.tenantId}`,
88-
email: account.username, // Username in an AccountInfo object is the email address
89-
username: account.name ?? account.username,
90-
}),
91-
);
92-
93-
// Privacy disclaimer for internal Microsoft users
94-
if (account.username.split('@')[1] === 'microsoft.com') {
95-
dispatch(
96-
addAlert({
97-
message:
98-
'By using Chat Copilot, you agree to protect sensitive data, not store it in chat, and allow chat history collection for service improvements. This tool is for internal use only.',
99-
type: AlertType.Info,
100-
}),
101-
);
102-
}
103-
104-
setAppState(AppState.LoadingChats);
105-
}
106-
} else {
107-
setAppState(AppState.LoadingChats);
79+
if (isAuthenticated && appState === AppState.SettingUserInfo) {
80+
const account = instance.getActiveAccount();
81+
if (!account) {
82+
setAppState(AppState.ErrorLoadingUserInfo);
83+
} else {
84+
dispatch(
85+
setActiveUserInfo({
86+
id: `${account.localAccountId}.${account.tenantId}`,
87+
email: account.username, // username is the email address
88+
username: account.name ?? account.username,
89+
}),
90+
);
91+
92+
// Privacy disclaimer for internal Microsoft users
93+
if (account.username.split('@')[1] === 'microsoft.com') {
94+
dispatch(
95+
addAlert({
96+
message:
97+
'By using Chat Copilot, you agree to protect sensitive data, not store it in chat, and allow chat history collection for service improvements. This tool is for internal use only.',
98+
type: AlertType.Info,
99+
}),
100+
);
108101
}
102+
103+
setAppState(AppState.LoadingChats);
109104
}
110105
}
111106

112107
if ((isAuthenticated || !AuthHelper.isAuthAAD()) && appState === AppState.LoadingChats) {
113108
void Promise.all([
114109
// Load all chats from memory
115-
chat.loadChats().then((succeeded) => {
116-
if (succeeded) {
110+
chat
111+
.loadChats()
112+
.then(() => {
117113
setAppState(AppState.Chat);
118-
}
119-
}),
114+
})
115+
.catch(() => {
116+
setAppState(AppState.ErrorLoadingChats);
117+
}),
120118

121119
// Check if content safety is enabled
122120
file.getContentSafetyStatus(),
@@ -133,7 +131,7 @@ const App: FC = () => {
133131
// eslint-disable-next-line react-hooks/exhaustive-deps
134132
}, [instance, inProgress, isAuthenticated, appState, isMaintenance]);
135133

136-
// TODO: [Issue #41] handle error case of missing account information
134+
const content = <Chat classes={classes} appState={appState} setAppState={setAppState} />;
137135
return (
138136
<FluentProvider
139137
className="app-container"
@@ -150,12 +148,10 @@ const App: FC = () => {
150148
{appState !== AppState.SigningOut && <Login />}
151149
</div>
152150
</UnauthenticatedTemplate>
153-
<AuthenticatedTemplate>
154-
<Chat classes={classes} appState={appState} setAppState={setAppState} />
155-
</AuthenticatedTemplate>
151+
<AuthenticatedTemplate>{content}</AuthenticatedTemplate>
156152
</>
157153
) : (
158-
<Chat classes={classes} appState={appState} setAppState={setAppState} />
154+
content
159155
)}
160156
</FluentProvider>
161157
);
@@ -189,23 +185,27 @@ const Chat = ({
189185
</div>
190186
{appState === AppState.ProbeForBackend && (
191187
<BackendProbe
192-
uri={process.env.REACT_APP_BACKEND_URI as string}
193188
onBackendFound={() => {
194-
if (AuthHelper.isAuthAAD()) {
195-
setAppState(AppState.SettingUserInfo);
196-
} else {
197-
setAppState(AppState.LoadingChats);
198-
}
189+
setAppState(
190+
AuthHelper.isAuthAAD()
191+
? // if AAD is enabled, we need to set the active account before loading chats
192+
AppState.SettingUserInfo
193+
: // otherwise, we can load chats immediately
194+
AppState.LoadingChats,
195+
);
199196
}}
200197
/>
201198
)}
202199
{appState === AppState.SettingUserInfo && (
203200
<Loading text={'Hang tight while we fetch your information...'} />
204201
)}
205202
{appState === AppState.ErrorLoadingUserInfo && (
206-
<Error text={'Oops, something went wrong. Please try signing out and signing back in.'} />
203+
<Error text={'Unable to load user info. Please try signing out and signing back in.'} />
204+
)}
205+
{appState === AppState.ErrorLoadingChats && (
206+
<Error text={'Unable to load chats. Please try refreshing the page.'} />
207207
)}
208-
{appState === AppState.LoadingChats && <Loading text="Loading Chats..." />}
208+
{appState === AppState.LoadingChats && <Loading text="Loading chats..." />}
209209
{appState === AppState.Chat && <ChatView />}
210210
</div>
211211
);

‎webapp/src/Constants.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,7 @@ export const Constants = {
1313
cacheLocation: 'localStorage',
1414
storeAuthStateInCookie: false,
1515
},
16-
semanticKernelScopes: ['openid', 'offline_access', 'profile'].concat(
17-
(process.env.REACT_APP_AAD_API_SCOPE as string) ? [process.env.REACT_APP_AAD_API_SCOPE as string] : [],
18-
),
16+
semanticKernelScopes: ['openid', 'offline_access', 'profile'],
1917
// MS Graph scopes required for loading user information
2018
msGraphAppScopes: ['User.ReadBasic.All'],
2119
},

0 commit comments

Comments
 (0)