From 82dbb0252e3d0e9ab84b20384a910a7c2a04a8e6 Mon Sep 17 00:00:00 2001 From: Gina Triolo <51341242+gitri-ms@users.noreply.github.com> Date: Sat, 12 Aug 2023 00:21:00 -0700 Subject: [PATCH] address pr comments --- README.md | 9 +++++---- scripts/deploy/README.md | 2 +- webapi/Controllers/ChatHistoryController.cs | 9 ++------- webapp/.env.example | 2 +- webapp/src/libs/auth/AuthHelper.ts | 2 ++ 5 files changed, 11 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index cf1d6acf9..a415c672f 100644 --- a/README.md +++ b/README.md @@ -129,14 +129,15 @@ By default, Chat Copilot runs locally without authentication, using a guest user ### Instructions 1. Create an [application registration](https://learn.microsoft.com/azure/active-directory/develop/quickstart-register-app) for the frontend web app, using the values below - - `Supported account types`: "_Accounts in any organizational directory (Any Azure AD directory - Multitenant) and personal Microsoft accounts (e.g. Skype, Xbox)_" + - `Supported account types`: "_Accounts in this organizational directory only ({YOUR TENANT} only - Single tenant)_" - `Redirect URI (optional)`: _Single-page application (SPA)_ and use _http://localhost:3000_. - 2. Create a second [application registration](https://learn.microsoft.com/azure/active-directory/develop/quickstart-register-app) for the backend web api, using the values below: - - `Supported account types`: "_Accounts in any organizational directory (Any Azure AD directory - Multitenant) and personal Microsoft accounts (e.g. Skype, Xbox)_" + - `Supported account types`: "_Accounts in this organizational directory only ({YOUR TENANT} only - Single tenant)_" - Do **not** configure a `Redirect URI (optional)` +> NOTE: Other account types can be used to allow multitenant and personal Microsoft accounts to use your application if you desire. Doing so may result in more users and therefore higher costs. + > Take note of the `Application (client) ID` for both app registrations as you will need them in future steps. 3. Expose an API within the second app registration @@ -145,7 +146,7 @@ By default, Chat Copilot runs locally without authentication, using a guest user 2. Add an _Application ID URI_ - 1. This will generate an `api://` URI with a generated for you + 1. This will generate an `api://` URI 2. Click _Save_ to store the generated URI diff --git a/scripts/deploy/README.md b/scripts/deploy/README.md index 5c5db61e6..b20aad0cf 100644 --- a/scripts/deploy/README.md +++ b/scripts/deploy/README.md @@ -47,7 +47,7 @@ You will need two Azure Active Directory (AAD) application registrations -- one 1. Select *Expose an API* from the menu 2. Add an *Application ID URI* - 1. This will generate an `api://` URI with a generated for you + 1. This will generate an `api://` URI 2. Click *Save* to store the generated URI diff --git a/webapi/Controllers/ChatHistoryController.cs b/webapi/Controllers/ChatHistoryController.cs index 64e77e322..7450e33c0 100644 --- a/webapi/Controllers/ChatHistoryController.cs +++ b/webapi/Controllers/ChatHistoryController.cs @@ -131,20 +131,15 @@ public async Task GetChatSessionByIdAsync(Guid chatId) /// /// Get all chat sessions associated with the logged in user. Return an empty list if no chats are found. /// + /// The user id. /// A list of chat sessions. An empty list if the user is not in any chat session. [HttpGet] - [Route("chatSession/getAllChats/{userId:regex(([[a-z0-9]]+-)+[[a-z0-9]]+\\.([[a-z0-9]]+-)+[[a-z0-9]]+)}")] + [Route("chatSession/getAllChats/{userId}")] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(StatusCodes.Status403Forbidden)] [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task GetAllChatSessionsAsync(string userId) { - // TODO: [Issue #141] Remove this once we remove userId from route - if (!userId.Equals(this._authInfo.UserId, StringComparison.Ordinal)) - { - return this.Forbid("User id does not match request."); - } - // Get all participants that belong to the user. // Then get all the chats from the list of participants. var chatParticipants = await this._participantRepository.FindByUserIdAsync(this._authInfo.UserId); diff --git a/webapp/.env.example b/webapp/.env.example index e9bc35477..d20b7bb6d 100644 --- a/webapp/.env.example +++ b/webapp/.env.example @@ -5,7 +5,7 @@ REACT_APP_BACKEND_URI=https://localhost:40443/ # To enable authorization using Azure Active Directory, uncomment the following variables # See paragraph "(Optional) Enable backend authorization via Azure AD" in README.md for details and setup # REACT_APP_AUTH_TYPE=AzureAd -# REACT_APP_AAD_AUTHORITY=https://login.microsoftonline.com/common +# REACT_APP_AAD_AUTHORITY=https://login.microsoftonline.com/{YOUR_TENANT_ID} # REACT_APP_AAD_CLIENT_ID= # Authorization scopes to access webapi when using Azure AD authorization. # REACT_APP_AAD_API_SCOPE= diff --git a/webapp/src/libs/auth/AuthHelper.ts b/webapp/src/libs/auth/AuthHelper.ts index c6b84011f..7131d630a 100644 --- a/webapp/src/libs/auth/AuthHelper.ts +++ b/webapp/src/libs/auth/AuthHelper.ts @@ -19,6 +19,8 @@ export const enum AuthType { AAD = 'AzureAd', } +// This is the default user information when authentication is set to 'None'. +// It must match what is defined in PassthroughAuthenticationHandler.cs on the backend. export const DefaultChatUser: IChatUser = { id: 'c05c61eb-65e4-4223-915a-fe72b0c9ece1', emailAddress: 'user@contoso.com',