From c23fa925198885131fdbe0b6d94383b9edeec843 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Parks=20=C2=A9=EF=B8=8F?= Date: Thu, 18 May 2023 15:28:07 -0700 Subject: [PATCH 01/15] fix security issues in copilot chat backend --- .../Auth/ApiKeyAuthenticationHandler.cs | 71 ------------------- .../Auth/ApiKeyAuthenticationSchemeOptions.cs | 16 ----- .../copilot-chat-app/webapi/Auth/AuthInfo.cs | 41 +++++++++++ .../webapi/Auth/AuthPolicyName.cs | 11 +++ .../Auth/ChatOwnerAuthorizationHandler.cs | 56 +++++++++++++++ .../webapi/Auth/ChatOwnerRequirement.cs | 12 ++++ .../copilot-chat-app/webapi/Auth/IAuthInfo.cs | 11 +++ .../Auth/PassThroughAuthenticationHandler.cs | 9 ++- .../webapi/Controllers/ProbeController.cs | 19 ----- .../CopilotChat/Controllers/BotController.cs | 11 ++- .../CopilotChat/Controllers/ChatController.cs | 2 - .../Controllers/ChatHistoryController.cs | 45 ++++++------ .../Controllers/DocumentImportController.cs | 33 ++++----- .../Controllers/SpeechTokenController.cs | 2 - .../webapi/CopilotChat/Models/ChatMessage.cs | 14 ++-- .../webapi/CopilotChat/Models/ChatSession.cs | 5 -- .../Models/ChatSessionCreationOptions.cs | 10 +++ .../Models/ChatSessionEditOptions.cs | 10 +++ .../CopilotChat/Models/DocumentImportForm.cs | 6 -- ...ptions.cs => ChatAuthenticationOptions.cs} | 19 ++--- .../apps/copilot-chat-app/webapi/Program.cs | 19 +++-- .../apps/copilot-chat-app/webapi/README.md | 4 +- .../webapi/ServiceExtensions.cs | 44 +++++++----- .../copilot-chat-app/webapi/appsettings.json | 6 +- 24 files changed, 256 insertions(+), 220 deletions(-) delete mode 100644 samples/apps/copilot-chat-app/webapi/Auth/ApiKeyAuthenticationHandler.cs delete mode 100644 samples/apps/copilot-chat-app/webapi/Auth/ApiKeyAuthenticationSchemeOptions.cs create mode 100644 samples/apps/copilot-chat-app/webapi/Auth/AuthInfo.cs create mode 100644 samples/apps/copilot-chat-app/webapi/Auth/AuthPolicyName.cs create mode 100644 samples/apps/copilot-chat-app/webapi/Auth/ChatOwnerAuthorizationHandler.cs create mode 100644 samples/apps/copilot-chat-app/webapi/Auth/ChatOwnerRequirement.cs create mode 100644 samples/apps/copilot-chat-app/webapi/Auth/IAuthInfo.cs delete mode 100644 samples/apps/copilot-chat-app/webapi/Controllers/ProbeController.cs create mode 100644 samples/apps/copilot-chat-app/webapi/CopilotChat/Models/ChatSessionCreationOptions.cs create mode 100644 samples/apps/copilot-chat-app/webapi/CopilotChat/Models/ChatSessionEditOptions.cs rename samples/apps/copilot-chat-app/webapi/Options/{AuthorizationOptions.cs => ChatAuthenticationOptions.cs} (67%) diff --git a/samples/apps/copilot-chat-app/webapi/Auth/ApiKeyAuthenticationHandler.cs b/samples/apps/copilot-chat-app/webapi/Auth/ApiKeyAuthenticationHandler.cs deleted file mode 100644 index 8a8ffde02c6a..000000000000 --- a/samples/apps/copilot-chat-app/webapi/Auth/ApiKeyAuthenticationHandler.cs +++ /dev/null @@ -1,71 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -using System; -using System.Security.Claims; -using System.Text.Encodings.Web; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Authentication; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Options; -using Microsoft.Extensions.Primitives; - -namespace SemanticKernel.Service.Auth; - -/// -/// Class implementing API key authentication. -/// -public class ApiKeyAuthenticationHandler : AuthenticationHandler -{ - public const string AuthenticationScheme = "ApiKey"; - public const string ApiKeyHeaderName = "x-api-key"; - - /// - /// Constructor - /// - public ApiKeyAuthenticationHandler( - IOptionsMonitor options, - ILoggerFactory loggerFactory, - UrlEncoder encoder, - ISystemClock clock) : base(options, loggerFactory, encoder, clock) - { - } - - protected override Task HandleAuthenticateAsync() - { - this.Logger.LogInformation("Checking API key"); - - if (string.IsNullOrWhiteSpace(this.Options.ApiKey)) - { - const string ErrorMessage = "API key not configured on server"; - - this.Logger.LogError(ErrorMessage); - - return Task.FromResult(AuthenticateResult.Fail(ErrorMessage)); - } - - if (!this.Request.Headers.TryGetValue(ApiKeyHeaderName, out StringValues apiKeyFromHeader)) - { - const string WarningMessage = "No API key provided"; - - this.Logger.LogWarning(WarningMessage); - - return Task.FromResult(AuthenticateResult.Fail(WarningMessage)); - } - - if (!string.Equals(apiKeyFromHeader, this.Options.ApiKey, StringComparison.Ordinal)) - { - const string WarningMessage = "Incorrect API key"; - - this.Logger.LogWarning(WarningMessage); - - return Task.FromResult(AuthenticateResult.Fail(WarningMessage)); - } - - var principal = new ClaimsPrincipal(new ClaimsIdentity(AuthenticationScheme)); - var ticket = new AuthenticationTicket(principal, this.Scheme.Name); - - this.Logger.LogInformation("Request authorized by API key"); - - return Task.FromResult(AuthenticateResult.Success(ticket)); - } -} diff --git a/samples/apps/copilot-chat-app/webapi/Auth/ApiKeyAuthenticationSchemeOptions.cs b/samples/apps/copilot-chat-app/webapi/Auth/ApiKeyAuthenticationSchemeOptions.cs deleted file mode 100644 index 33e8943f2a0a..000000000000 --- a/samples/apps/copilot-chat-app/webapi/Auth/ApiKeyAuthenticationSchemeOptions.cs +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -using Microsoft.AspNetCore.Authentication; - -namespace SemanticKernel.Service.Auth; - -/// -/// Options for API key authentication. -/// -public class ApiKeyAuthenticationSchemeOptions : AuthenticationSchemeOptions -{ - /// - /// The API key against which to authenticate. - /// - public string? ApiKey { get; set; } -} diff --git a/samples/apps/copilot-chat-app/webapi/Auth/AuthInfo.cs b/samples/apps/copilot-chat-app/webapi/Auth/AuthInfo.cs new file mode 100644 index 000000000000..e88409caf347 --- /dev/null +++ b/samples/apps/copilot-chat-app/webapi/Auth/AuthInfo.cs @@ -0,0 +1,41 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using Azure.Identity; +using Microsoft.AspNetCore.Http; +using Microsoft.IdentityModel.JsonWebTokens; + +namespace SemanticKernel.Service.Auth; + +/// +/// Class which provides validated security information for use in controllers. +/// +public class AuthInfo : IAuthInfo +{ + private readonly Lazy _userId; + + public AuthInfo(IHttpContextAccessor httpContextAccessor) + { + _userId = new Lazy(() => + { + var user = httpContextAccessor.HttpContext?.User; + if (user is null) + { + throw new InvalidOperationException("HttpContext must be present to inspect auth info."); + } + var userIdClaim = user.FindFirst("oid") + ?? user.FindFirst(JwtRegisteredClaimNames.Sub); + + if (userIdClaim is null) + { + throw new CredentialUnavailableException("User Id was not present in the request token."); + } + return userIdClaim.Value; + }, isThreadSafe: false); + } + + /// + /// The authenticated user's unique ID. + /// + public string UserId => _userId.Value; +} diff --git a/samples/apps/copilot-chat-app/webapi/Auth/AuthPolicyName.cs b/samples/apps/copilot-chat-app/webapi/Auth/AuthPolicyName.cs new file mode 100644 index 000000000000..33f703819c54 --- /dev/null +++ b/samples/apps/copilot-chat-app/webapi/Auth/AuthPolicyName.cs @@ -0,0 +1,11 @@ +// Copyright (c) Microsoft. All rights reserved. + +namespace SemanticKernel.Service.Auth; + +/// +/// Holds the policy names for custom authorization policies. +/// +public static class AuthPolicyName +{ + public const string RequireChatOwner = "RequireChatOwner"; +} diff --git a/samples/apps/copilot-chat-app/webapi/Auth/ChatOwnerAuthorizationHandler.cs b/samples/apps/copilot-chat-app/webapi/Auth/ChatOwnerAuthorizationHandler.cs new file mode 100644 index 000000000000..0a1a938a4137 --- /dev/null +++ b/samples/apps/copilot-chat-app/webapi/Auth/ChatOwnerAuthorizationHandler.cs @@ -0,0 +1,56 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Threading.Tasks; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Routing; +using SemanticKernel.Service.CopilotChat.Storage; + +namespace SemanticKernel.Service.Auth; + +/// +/// Class implementing "authorization" that validates the user has access to a chat. +/// +public class ChatOwnerAuthorizationHandler : AuthorizationHandler +{ + private readonly IAuthInfo _authInfoProvider; + private readonly ChatSessionRepository _chatSessionRepository; + + /// + /// Constructor + /// + public ChatOwnerAuthorizationHandler( + IAuthInfo authInfoProvider, + ChatSessionRepository chatSessionRepository) : base() + { + this._authInfoProvider = authInfoProvider; + this._chatSessionRepository = chatSessionRepository; + } + + protected override async Task HandleRequirementAsync( + AuthorizationHandlerContext context, + ChatOwnerRequirement requirement, + HttpContext resource) + { + string? chatId = resource.GetRouteValue("chatId")?.ToString(); + if (chatId == null) + { + // delegate to downstream validation + context.Succeed(requirement); + return; + } + + var session = await this._chatSessionRepository.FindByIdAsync(chatId); + if (session == null) + { + // delegate to downstream validation + context.Succeed(requirement); + return; + } + + if (session.UserId != this._authInfoProvider.UserId) + { + context.Fail(new AuthorizationFailureReason(this, "User does not have access to the requested chat.")); + } + } +} diff --git a/samples/apps/copilot-chat-app/webapi/Auth/ChatOwnerRequirement.cs b/samples/apps/copilot-chat-app/webapi/Auth/ChatOwnerRequirement.cs new file mode 100644 index 000000000000..7a9ea8bc8870 --- /dev/null +++ b/samples/apps/copilot-chat-app/webapi/Auth/ChatOwnerRequirement.cs @@ -0,0 +1,12 @@ +// Copyright (c) Microsoft. All rights reserved. + +using Microsoft.AspNetCore.Authorization; + +namespace SemanticKernel.Service.Auth; + +/// +/// Used to require the chat to be owned by the authenticated user. +/// +public class ChatOwnerRequirement : IAuthorizationRequirement +{ +} diff --git a/samples/apps/copilot-chat-app/webapi/Auth/IAuthInfo.cs b/samples/apps/copilot-chat-app/webapi/Auth/IAuthInfo.cs new file mode 100644 index 000000000000..e8bb425f5c8f --- /dev/null +++ b/samples/apps/copilot-chat-app/webapi/Auth/IAuthInfo.cs @@ -0,0 +1,11 @@ +// Copyright (c) Microsoft. All rights reserved. + +namespace SemanticKernel.Service.Auth; + +public interface IAuthInfo +{ + /// + /// The authenticated user's unique ID. + /// + public string UserId { get; } +} diff --git a/samples/apps/copilot-chat-app/webapi/Auth/PassThroughAuthenticationHandler.cs b/samples/apps/copilot-chat-app/webapi/Auth/PassThroughAuthenticationHandler.cs index e12320b898ec..5ba663068f88 100644 --- a/samples/apps/copilot-chat-app/webapi/Auth/PassThroughAuthenticationHandler.cs +++ b/samples/apps/copilot-chat-app/webapi/Auth/PassThroughAuthenticationHandler.cs @@ -1,5 +1,6 @@ // Copyright (c) Microsoft. All rights reserved. +using System.IdentityModel.Tokens.Jwt; using System.Security.Claims; using System.Text.Encodings.Web; using System.Threading.Tasks; @@ -15,6 +16,7 @@ namespace SemanticKernel.Service.Auth; public class PassThroughAuthenticationHandler : AuthenticationHandler { public const string AuthenticationScheme = "PassThrough"; + private const string DefaultUserId = "c05c61eb-65e4-4223-915a-fe72b0c9ece1"; /// /// Constructor @@ -30,9 +32,10 @@ public PassThroughAuthenticationHandler( protected override Task HandleAuthenticateAsync() { this.Logger.LogInformation("Allowing request to pass through"); - - var principal = new ClaimsPrincipal(new ClaimsIdentity(AuthenticationScheme)); - var ticket = new AuthenticationTicket(principal, this.Scheme.Name); + Claim userIdClaim = new(JwtRegisteredClaimNames.Sub, DefaultUserId); + ClaimsIdentity identity = new(new Claim[] { userIdClaim }, AuthenticationScheme); + ClaimsPrincipal principal = new(identity); + AuthenticationTicket ticket = new(principal, this.Scheme.Name); return Task.FromResult(AuthenticateResult.Success(ticket)); } diff --git a/samples/apps/copilot-chat-app/webapi/Controllers/ProbeController.cs b/samples/apps/copilot-chat-app/webapi/Controllers/ProbeController.cs deleted file mode 100644 index 2346e353ba0a..000000000000 --- a/samples/apps/copilot-chat-app/webapi/Controllers/ProbeController.cs +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -// TODO: replace this controller with a better health check: -// https://learn.microsoft.com/en-us/aspnet/core/host-and-deploy/health-checks?view=aspnetcore-7.0 - -using Microsoft.AspNetCore.Mvc; - -namespace SemanticKernel.Service.Controllers; - -[Route("[controller]")] -[ApiController] -public class ProbeController : ControllerBase -{ - [HttpGet] - public ActionResult Get() - { - return "Semantic Kernel service up and running"; - } -} diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs index c0c90fef5e00..9845a5d852e5 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs @@ -12,6 +12,7 @@ using Microsoft.Extensions.Options; using Microsoft.SemanticKernel; using Microsoft.SemanticKernel.Memory; +using SemanticKernel.Service.Auth; using SemanticKernel.Service.CopilotChat.Models; using SemanticKernel.Service.CopilotChat.Options; using SemanticKernel.Service.CopilotChat.Storage; @@ -62,10 +63,9 @@ public BotController( /// Upload a bot. /// /// The Semantic Kernel instance. - /// The user id. + /// The auth info instance. /// The bot object from the message body /// The HTTP action result. - [Authorize] [HttpPost] [Route("bot/upload")] [ProducesResponseType(StatusCodes.Status202Accepted)] @@ -73,10 +73,9 @@ public BotController( [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task UploadAsync( [FromServices] IKernel kernel, - [FromQuery] string userId, + [FromServices] IAuthInfo authInfoProvider, [FromBody] Bot bot) { - // TODO: We should get userId from server context instead of from request for privacy/security reasons when support multiple users. this._logger.LogDebug("Received call to upload a bot"); if (!IsBotCompatible( @@ -99,7 +98,7 @@ public async Task UploadAsync( try { // 1. Create a new chat and get the chat id. - var newChat = new ChatSession(userId, chatTitle); + var newChat = new ChatSession(authInfoProvider.UserId, chatTitle); await this._chatRepository.CreateAsync(newChat); chatId = newChat.Id; @@ -133,12 +132,12 @@ public async Task UploadAsync( /// The Semantic Kernel instance. /// The chat id to be downloaded. /// The serialized Bot object of the chat id. - [Authorize] [HttpGet] [Route("bot/download/{chatId:guid}")] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(StatusCodes.Status400BadRequest)] [ProducesResponseType(StatusCodes.Status404NotFound)] + [Authorize(Policy = AuthPolicyName.RequireChatOwner)] public async Task> DownloadAsync( [FromServices] IKernel kernel, Guid chatId) diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatController.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatController.cs index 240f0be35b1e..713e5d702e0f 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatController.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatController.cs @@ -7,7 +7,6 @@ using System.Net.Http; using System.Reflection; using System.Threading.Tasks; -using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Logging; @@ -55,7 +54,6 @@ public ChatController(ILogger logger) /// Prompt along with its parameters. /// Authentication headers to connect to OpenAPI Skills. /// Results containing the response from the model. - [Authorize] [Route("chat")] [HttpPost] [ProducesResponseType(StatusCodes.Status200OK)] diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatHistoryController.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatHistoryController.cs index 7730311856f8..7f75514968f7 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatHistoryController.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatHistoryController.cs @@ -9,6 +9,7 @@ using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using SemanticKernel.Service.Auth; using SemanticKernel.Service.CopilotChat.Models; using SemanticKernel.Service.CopilotChat.Options; using SemanticKernel.Service.CopilotChat.Storage; @@ -21,12 +22,12 @@ namespace SemanticKernel.Service.CopilotChat.Controllers; /// retrieving chat messages, and editing chat sessions. /// [ApiController] -[Authorize] public class ChatHistoryController : ControllerBase { private readonly ILogger _logger; private readonly ChatSessionRepository _chatSessionRepository; private readonly ChatMessageRepository _chatMessageRepository; + private readonly IAuthInfo _authInfoProvider; private readonly PromptsOptions _promptOptions; /// @@ -36,15 +37,18 @@ public class ChatHistoryController : ControllerBase /// The chat session repository. /// The chat message repository. /// The prompts options. + /// The auth info for the current request. public ChatHistoryController( ILogger logger, ChatSessionRepository chatSessionRepository, ChatMessageRepository chatMessageRepository, - IOptions promptsOptions) + IOptions promptsOptions, + IAuthInfo authInfoProvider) { this._logger = logger; this._chatSessionRepository = chatSessionRepository; this._chatMessageRepository = chatMessageRepository; + this._authInfoProvider = authInfoProvider; this._promptOptions = promptsOptions.Value; } @@ -54,14 +58,14 @@ public ChatHistoryController( /// Object that contains the parameters to create a new chat. /// The HTTP action result. [HttpPost] - [Route("chatSession/create")] + [Route("chatSessions")] [ProducesResponseType(StatusCodes.Status201Created)] [ProducesResponseType(StatusCodes.Status400BadRequest)] [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task CreateChatSessionAsync( - [FromBody] ChatSession chatParameters) + [FromBody] ChatSessionCreationOptions chatParameters) { - var userId = chatParameters.UserId; + var userId = _authInfoProvider.UserId; var title = chatParameters.Title; var newChat = new ChatSession(userId, title); @@ -80,10 +84,11 @@ public async Task CreateChatSessionAsync( /// The chat id. [HttpGet] [ActionName("GetChatSessionByIdAsync")] - [Route("chatSession/getChat/{chatId:guid}")] + [Route("chatSessions/{chatId:guid}")] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(StatusCodes.Status400BadRequest)] [ProducesResponseType(StatusCodes.Status404NotFound)] + [Authorize(Policy = AuthPolicyName.RequireChatOwner)] public async Task GetChatSessionByIdAsync(Guid chatId) { var chat = await this._chatSessionRepository.FindByIdAsync(chatId.ToString()); @@ -96,19 +101,16 @@ public async Task GetChatSessionByIdAsync(Guid chatId) } /// - /// Get all chat sessions associated with a user. Return an empty list if no chats are found. - /// The regex pattern that is used to match the user id will match the following format: - /// - 2 period separated groups of one or more hyphen-delimited alphanumeric strings. - /// The pattern matches two GUIDs in canonical textual representation separated by a period. + /// Get all chat sessions associated with the logged in user. Return an empty list if no chats are found. /// - /// The user id. [HttpGet] - [Route("chatSession/getAllChats/{userId:regex(([[a-z0-9]]+-)+[[a-z0-9]]+\\.([[a-z0-9]]+-)+[[a-z0-9]]+)}")] + [Route("chatSessions")] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(StatusCodes.Status400BadRequest)] [ProducesResponseType(StatusCodes.Status404NotFound)] - public async Task GetAllChatSessionsAsync(string userId) + public async Task GetAllChatSessionsAsync() { + var userId = this._authInfoProvider.UserId; var chats = await this._chatSessionRepository.FindByUserIdAsync(userId); if (chats == null) { @@ -128,10 +130,11 @@ public async Task GetAllChatSessionsAsync(string userId) /// The number of messages to return. -1 will return all messages starting from startIdx. /// [Authorize] [HttpGet] - [Route("chatSession/getChatMessages/{chatId:guid}")] + [Route("chatSessions/{chatId:guid}/messages")] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(StatusCodes.Status400BadRequest)] [ProducesResponseType(StatusCodes.Status404NotFound)] + [Authorize(Policy = AuthPolicyName.RequireChatOwner)] public async Task GetChatMessagesAsync( Guid chatId, [FromQuery] int startIdx = 0, @@ -153,20 +156,20 @@ public async Task GetChatMessagesAsync( /// /// Edit a chat session. /// + /// Chat id from the url. /// Object that contains the parameters to edit the chat. - [HttpPost] - [Route("chatSession/edit")] + [HttpPatch] + [Route("chatSessions/{chatId:guid}")] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(StatusCodes.Status400BadRequest)] [ProducesResponseType(StatusCodes.Status404NotFound)] - public async Task EditChatSessionAsync([FromBody] ChatSession chatParameters) + [Authorize(Policy = AuthPolicyName.RequireChatOwner)] + public async Task EditChatSessionAsync(Guid chatId, [FromBody] ChatSessionEditOptions chatParameters) { - string chatId = chatParameters.Id; - - ChatSession? chat = await this._chatSessionRepository.FindByIdAsync(chatId); + ChatSession? chat = await this._chatSessionRepository.FindByIdAsync(chatId.ToString()); if (chat == null) { - return this.NotFound($"Chat of id {chatId} not found."); + return this.NotFound($"Chat with id {chatId} not found."); } chat.Title = chatParameters.Title; diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/DocumentImportController.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/DocumentImportController.cs index be040d484bdc..1ad4c3a30464 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/DocumentImportController.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/DocumentImportController.cs @@ -2,15 +2,14 @@ using System; using System.IO; -using System.Linq; using System.Threading.Tasks; -using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Microsoft.SemanticKernel; using Microsoft.SemanticKernel.Text; +using SemanticKernel.Service.Auth; using SemanticKernel.Service.CopilotChat.Models; using SemanticKernel.Service.CopilotChat.Options; using SemanticKernel.Service.CopilotChat.Storage; @@ -61,13 +60,13 @@ public DocumentImportController( /// /// Service API for importing a document. /// - [Authorize] [Route("importDocument")] [HttpPost] - [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status202Accepted)] [ProducesResponseType(StatusCodes.Status400BadRequest)] public async Task ImportDocumentAsync( [FromServices] IKernel kernel, + [FromServices] IAuthInfo authInfoProvider, [FromForm] DocumentImportForm documentImportForm) { var formFile = documentImportForm.FormFile; @@ -86,12 +85,18 @@ public async Task ImportDocumentAsync( return this.BadRequest("File size exceeds the limit."); } - if (documentImportForm.DocumentScope == DocumentImportForm.DocumentScopes.Chat - && !(await this.UserHasAccessToChatAsync(documentImportForm.UserId, documentImportForm.ChatId))) + var chatSession = await this._chatSessionRepository.FindByIdAsync(documentImportForm.ChatId.ToString()); + if (chatSession == null) { - return this.BadRequest("User does not have access to the chat session."); + return this.NotFound("Session does not exist."); } + if (documentImportForm.DocumentScope == DocumentImportForm.DocumentScopes.Chat && authInfoProvider.UserId != chatSession.UserId) + { + return this.Unauthorized("User does not have access to the chat session."); + } + + this._logger.LogInformation("Importing document {0}", formFile.FileName); try @@ -117,7 +122,7 @@ public async Task ImportDocumentAsync( return this.BadRequest(ex.Message); } - return this.Ok(); + return this.Accepted(); } /// @@ -201,16 +206,4 @@ await kernel.Memory.SaveInformationAsync( Path.GetFileName(documentImportForm.FormFile?.FileName) ); } - - /// - /// Check if the user has access to the chat session. - /// - /// The user ID. - /// The chat session ID. - /// A boolean indicating whether the user has access to the chat session. - private async Task UserHasAccessToChatAsync(string userId, Guid chatId) - { - var chatSessions = await this._chatSessionRepository.FindByUserIdAsync(userId); - return chatSessions.Any(c => c.Id == chatId.ToString()); - } } diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/SpeechTokenController.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/SpeechTokenController.cs index 0880170d2b2a..f60618fa0255 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/SpeechTokenController.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/SpeechTokenController.cs @@ -4,7 +4,6 @@ using System.Net; using System.Net.Http; using System.Threading.Tasks; -using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Logging; @@ -14,7 +13,6 @@ namespace SemanticKernel.Service.CopilotChat.Controllers; -[Authorize] [ApiController] public class SpeechTokenController : ControllerBase { diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/ChatMessage.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/ChatMessage.cs index 7e67abdf2011..669117a5e285 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/ChatMessage.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/ChatMessage.cs @@ -3,7 +3,6 @@ using System; using System.Globalization; using System.Text.Json; -using System.Text.Json.Serialization; using SemanticKernel.Service.CopilotChat.Storage; namespace SemanticKernel.Service.CopilotChat.Models; @@ -13,6 +12,8 @@ namespace SemanticKernel.Service.CopilotChat.Models; /// public class ChatMessage : IStorageEntity { + private static readonly JsonSerializerOptions SerializerSettings = new() { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }; + /// /// Role of the author of a chat message. /// @@ -37,43 +38,36 @@ public enum AuthorRoles /// /// Timestamp of the message. /// - [JsonPropertyName("timestamp")] public DateTimeOffset Timestamp { get; set; } /// /// Id of the user who sent this message. /// - [JsonPropertyName("userId")] public string UserId { get; set; } /// /// Name of the user who sent this message. /// - [JsonPropertyName("userName")] public string UserName { get; set; } /// /// Id of the chat this message belongs to. /// - [JsonPropertyName("chatId")] public string ChatId { get; set; } /// /// Content of the message. /// - [JsonPropertyName("content")] public string Content { get; set; } /// /// Id of the message. /// - [JsonPropertyName("id")] public string Id { get; set; } /// /// Role of the author of the message. /// - [JsonPropertyName("authorRole")] public AuthorRoles AuthorRole { get; set; } /// @@ -120,7 +114,7 @@ public string ToFormattedString() /// A serialized json string public override string ToString() { - return JsonSerializer.Serialize(this); + return JsonSerializer.Serialize(this, SerializerSettings); } /// @@ -130,6 +124,6 @@ public override string ToString() /// A ChatMessage object public static ChatMessage? FromString(string json) { - return JsonSerializer.Deserialize(json); + return JsonSerializer.Deserialize(json, SerializerSettings); } } diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/ChatSession.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/ChatSession.cs index 29d4d9476c91..9d42e95c267b 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/ChatSession.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/ChatSession.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft. All rights reserved. using System; -using System.Text.Json.Serialization; using SemanticKernel.Service.CopilotChat.Storage; namespace SemanticKernel.Service.CopilotChat.Models; @@ -14,25 +13,21 @@ public class ChatSession : IStorageEntity /// /// Chat ID that is persistent and unique. /// - [JsonPropertyName("id")] public string Id { get; set; } /// /// User ID that is persistent and unique. /// - [JsonPropertyName("userId")] public string UserId { get; set; } /// /// Title of the chat. /// - [JsonPropertyName("title")] public string Title { get; set; } /// /// Timestamp of the chat creation. /// - [JsonPropertyName("createdOn")] public DateTimeOffset CreatedOn { get; set; } public ChatSession(string userId, string title) diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/ChatSessionCreationOptions.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/ChatSessionCreationOptions.cs new file mode 100644 index 000000000000..f3573af80130 --- /dev/null +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/ChatSessionCreationOptions.cs @@ -0,0 +1,10 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.ComponentModel.DataAnnotations; + +namespace SemanticKernel.Service.CopilotChat.Models; + +/// +/// A chat session creation option. +/// +public record struct ChatSessionCreationOptions([Required] string Title); diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/ChatSessionEditOptions.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/ChatSessionEditOptions.cs new file mode 100644 index 000000000000..c3f29c7bdee3 --- /dev/null +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/ChatSessionEditOptions.cs @@ -0,0 +1,10 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.ComponentModel.DataAnnotations; + +namespace SemanticKernel.Service.CopilotChat.Models; + +/// +/// A chat session edit option. +/// +public record struct ChatSessionEditOptions([Required] string Title); diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/DocumentImportForm.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/DocumentImportForm.cs index 109702e0738f..f4168b1936c4 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/DocumentImportForm.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/DocumentImportForm.cs @@ -36,10 +36,4 @@ public enum DocumentScopes /// If the document scope is set to global, this value is ignored. /// public Guid ChatId { get; set; } = Guid.Empty; - - /// - /// The ID of the user who is importing the document to a chat session. - /// Will be use to validate if the user has access to the chat session. - /// - public string UserId { get; set; } = string.Empty; } diff --git a/samples/apps/copilot-chat-app/webapi/Options/AuthorizationOptions.cs b/samples/apps/copilot-chat-app/webapi/Options/ChatAuthenticationOptions.cs similarity index 67% rename from samples/apps/copilot-chat-app/webapi/Options/AuthorizationOptions.cs rename to samples/apps/copilot-chat-app/webapi/Options/ChatAuthenticationOptions.cs index 3bdbf7e3ff59..8ec880f2e567 100644 --- a/samples/apps/copilot-chat-app/webapi/Options/AuthorizationOptions.cs +++ b/samples/apps/copilot-chat-app/webapi/Options/ChatAuthenticationOptions.cs @@ -7,32 +7,25 @@ namespace SemanticKernel.Service.Options; /// /// Configuration options for authorizing to the service. /// -public class AuthorizationOptions +public class ChatAuthenticationOptions { - public const string PropertyName = "Authorization"; + public const string PropertyName = "Authentication"; - public enum AuthorizationType + public enum AuthenticationType { None, - ApiKey, AzureAd } /// /// Type of authorization. /// - public AuthorizationType Type { get; set; } = AuthorizationType.None; + public AuthenticationType Type { get; set; } = AuthenticationType.None; /// - /// When is , this is the API key to use. + /// When is , these are the Azure AD options to use. /// - [Required, NotEmptyOrWhitespace] - public string ApiKey { get; set; } = string.Empty; - - /// - /// When is , these are the Azure AD options to use. - /// - [RequiredOnPropertyValue(nameof(Type), AuthorizationType.AzureAd)] + [RequiredOnPropertyValue(nameof(Type), AuthenticationType.AzureAd)] public AzureAdOptions? AzureAd { get; set; } /// diff --git a/samples/apps/copilot-chat-app/webapi/Program.cs b/samples/apps/copilot-chat-app/webapi/Program.cs index a796e1373bc4..5156b5162348 100644 --- a/samples/apps/copilot-chat-app/webapi/Program.cs +++ b/samples/apps/copilot-chat-app/webapi/Program.cs @@ -2,12 +2,14 @@ using System; using System.Linq; +using System.Text.Json; using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Hosting.Server; using Microsoft.AspNetCore.Hosting.Server.Features; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Diagnostics.HealthChecks; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using SemanticKernel.Service.CopilotChat.Extensions; @@ -48,18 +50,27 @@ public static async Task Main(string[] args) builder.Services .AddApplicationInsightsTelemetry() .AddLogging(logBuilder => logBuilder.AddApplicationInsights()) - .AddAuthorization(builder.Configuration) + .AddCopilotChatAuthentication(builder.Configuration) + .AddCopilotChatAuthorization() .AddEndpointsApiExplorer() .AddSwaggerGen() .AddCors() - .AddControllers(); + .AddControllers() + .AddJsonOptions(options => + { + options.JsonSerializerOptions.PropertyNamingPolicy = JsonNamingPolicy.CamelCase; + }); + builder.Services.AddHealthChecks().AddCheck("Default", + () => HealthCheckResult.Healthy("Semantic Kernel service up and running")); // Configure middleware and endpoints WebApplication app = builder.Build(); app.UseCors(); + app.MapHealthChecks("/healthz"); app.UseAuthentication(); app.UseAuthorization(); - app.MapControllers(); + app.MapControllers() + .RequireAuthorization(); // Enable Swagger for development environments. if (app.Environment.IsDevelopment()) @@ -75,7 +86,7 @@ public static async Task Main(string[] args) try { string? address = app.Services.GetRequiredService().Features.Get()?.Addresses.FirstOrDefault(); - app.Services.GetRequiredService().LogInformation("Health probe: {0}/probe", address); + app.Services.GetRequiredService().LogInformation("Health probe: {0}/heaalthz", address); } catch (ObjectDisposedException) { diff --git a/samples/apps/copilot-chat-app/webapi/README.md b/samples/apps/copilot-chat-app/webapi/README.md index d4701084ce7b..d3ed7d75048f 100644 --- a/samples/apps/copilot-chat-app/webapi/README.md +++ b/samples/apps/copilot-chat-app/webapi/README.md @@ -38,11 +38,11 @@ You can start the WebApi service using the command-line, Visual Studio Code, or ``` dotnet run ``` -1. Early in the startup, the service will provide a probe endpoint you can use in a web browser to verify +1. Early in the startup, the service will provide a healthz endpoint you can use in a web browser to verify the service is running. ``` info: Microsoft.SemanticKernel.Kernel[0] - Health probe: https://localhost:40443/probe + Health probe: https://localhost:40443/healthz ``` ## Visual Studio Code diff --git a/samples/apps/copilot-chat-app/webapi/ServiceExtensions.cs b/samples/apps/copilot-chat-app/webapi/ServiceExtensions.cs index bd3792c78093..36cd177b4e08 100644 --- a/samples/apps/copilot-chat-app/webapi/ServiceExtensions.cs +++ b/samples/apps/copilot-chat-app/webapi/ServiceExtensions.cs @@ -5,6 +5,7 @@ using System.Reflection; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authentication.JwtBearer; +using Microsoft.AspNetCore.Authorization; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; @@ -39,8 +40,8 @@ internal static IServiceCollection AddOptions(this IServiceCollection services, .PostConfigure(TrimStringProperties); // Authorization configuration - services.AddOptions() - .Bind(configuration.GetSection(AuthorizationOptions.PropertyName)) + services.AddOptions() + .Bind(configuration.GetSection(ChatAuthenticationOptions.PropertyName)) .ValidateOnStart() .PostConfigure(TrimStringProperties); @@ -77,26 +78,19 @@ internal static IServiceCollection AddCors(this IServiceCollection services) } /// - /// Add authorization services + /// Add authentication services /// - internal static IServiceCollection AddAuthorization(this IServiceCollection services, IConfiguration configuration) + internal static IServiceCollection AddCopilotChatAuthentication(this IServiceCollection services, IConfiguration configuration) { - AuthorizationOptions config = services.BuildServiceProvider().GetRequiredService>().Value; + var config = services.BuildServiceProvider().GetRequiredService>().Value; switch (config.Type) { - case AuthorizationOptions.AuthorizationType.AzureAd: + case ChatAuthenticationOptions.AuthenticationType.AzureAd: services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme) - .AddMicrosoftIdentityWebApi(configuration.GetSection($"{AuthorizationOptions.PropertyName}:AzureAd")); + .AddMicrosoftIdentityWebApi(configuration.GetSection($"{ChatAuthenticationOptions.PropertyName}:AzureAd")); break; - case AuthorizationOptions.AuthorizationType.ApiKey: - services.AddAuthentication(ApiKeyAuthenticationHandler.AuthenticationScheme) - .AddScheme( - ApiKeyAuthenticationHandler.AuthenticationScheme, - options => options.ApiKey = config.ApiKey); - break; - - case AuthorizationOptions.AuthorizationType.None: + case ChatAuthenticationOptions.AuthenticationType.None: services.AddAuthentication(PassThroughAuthenticationHandler.AuthenticationScheme) .AddScheme( authenticationScheme: PassThroughAuthenticationHandler.AuthenticationScheme, @@ -104,12 +98,30 @@ internal static IServiceCollection AddAuthorization(this IServiceCollection serv break; default: - throw new InvalidOperationException($"Invalid authorization type '{config.Type}'."); + throw new InvalidOperationException($"Invalid authentication type '{config.Type}'."); } return services; } + internal static IServiceCollection AddCopilotChatAuthorization(this IServiceCollection services) + { + return services.AddScoped() + .AddScoped() + .AddAuthorizationCore(options => + { + options.DefaultPolicy = new AuthorizationPolicyBuilder() + .RequireAuthenticatedUser() + .Build(); + + options.AddPolicy(AuthPolicyName.RequireChatOwner, builder => + { + builder.RequireAuthenticatedUser() + .AddRequirements(new ChatOwnerRequirement()); + }); + }); + } + /// /// Trim all string properties, recursively. /// diff --git a/samples/apps/copilot-chat-app/webapi/appsettings.json b/samples/apps/copilot-chat-app/webapi/appsettings.json index 8642defc3538..9c199860f2c3 100644 --- a/samples/apps/copilot-chat-app/webapi/appsettings.json +++ b/samples/apps/copilot-chat-app/webapi/appsettings.json @@ -77,12 +77,10 @@ // // Authorization configuration to gate access to the service. - // - Supported Types are "None", "ApiKey", or "AzureAd". - // - Set ApiKey using dotnet's user secrets (see above) (i.e. dotnet user-secret set "Authorization:ApiKey" "MY_API_KEY") + // - Supported Types are "None", or "AzureAd". // - "Authorization": { + "Authentication": { "Type": "None", - "ApiKey": "", "AzureAd": { "Instance": "https://login.microsoftonline.com/", "TenantId": "", From f3520427101d469393648de09096bd903800a396 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Parks=20=C2=A9=EF=B8=8F?= Date: Thu, 18 May 2023 19:30:01 -0700 Subject: [PATCH 02/15] fix frontend --- .../copilot-chat-app/webapi/Auth/AuthInfo.cs | 28 ++++++++++++-- .../copilot-chat-app/webapi/Auth/IAuthInfo.cs | 5 +++ .../Auth/PassThroughAuthenticationHandler.cs | 4 +- .../Controllers/SemanticKernelController.cs | 31 ++++++++++----- .../CopilotChat/Controllers/ChatController.cs | 29 +++++++++++--- .../Extensions/ServiceExtensions.cs | 4 +- .../copilot-chat-app/webapi/Models/Ask.cs | 3 ++ .../apps/copilot-chat-app/webapi/Program.cs | 3 +- .../webapi/ServiceExtensions.cs | 6 +++ .../webapi/Utilities/AskConverter.cs | 38 +++++++++++++++++++ .../src/components/views/BackendProbe.tsx | 2 +- .../webapp/src/libs/services/BotService.ts | 4 +- .../webapp/src/libs/services/ChatService.ts | 30 ++++++--------- .../webapp/src/libs/useChat.ts | 13 +------ 14 files changed, 145 insertions(+), 55 deletions(-) create mode 100644 samples/apps/copilot-chat-app/webapi/Utilities/AskConverter.cs diff --git a/samples/apps/copilot-chat-app/webapi/Auth/AuthInfo.cs b/samples/apps/copilot-chat-app/webapi/Auth/AuthInfo.cs index e88409caf347..58d46c0f00c4 100644 --- a/samples/apps/copilot-chat-app/webapi/Auth/AuthInfo.cs +++ b/samples/apps/copilot-chat-app/webapi/Auth/AuthInfo.cs @@ -12,11 +12,15 @@ namespace SemanticKernel.Service.Auth; /// public class AuthInfo : IAuthInfo { - private readonly Lazy _userId; + private record struct AuthData( + string UserId, + string UserName); + + private readonly Lazy _data; public AuthInfo(IHttpContextAccessor httpContextAccessor) { - _userId = new Lazy(() => + _data = new Lazy(() => { var user = httpContextAccessor.HttpContext?.User; if (user is null) @@ -30,12 +34,28 @@ public AuthInfo(IHttpContextAccessor httpContextAccessor) { throw new CredentialUnavailableException("User Id was not present in the request token."); } - return userIdClaim.Value; + + var userNameClaim = user.FindFirst(JwtRegisteredClaimNames.Name); + if (userNameClaim is null) + { + throw new CredentialUnavailableException("User name was not present in the request token."); + } + + return new AuthData + { + UserId = userIdClaim.Value, + UserName = userNameClaim.Value, + }; }, isThreadSafe: false); } /// /// The authenticated user's unique ID. /// - public string UserId => _userId.Value; + public string UserId => _data.Value.UserId; + + /// + /// The authenticated user's name. + /// + public string Name => _data.Value.UserName; } diff --git a/samples/apps/copilot-chat-app/webapi/Auth/IAuthInfo.cs b/samples/apps/copilot-chat-app/webapi/Auth/IAuthInfo.cs index e8bb425f5c8f..7af794a8843a 100644 --- a/samples/apps/copilot-chat-app/webapi/Auth/IAuthInfo.cs +++ b/samples/apps/copilot-chat-app/webapi/Auth/IAuthInfo.cs @@ -8,4 +8,9 @@ public interface IAuthInfo /// The authenticated user's unique ID. /// public string UserId { get; } + + /// + /// The authenticated user's name. + /// + public string Name { get; } } diff --git a/samples/apps/copilot-chat-app/webapi/Auth/PassThroughAuthenticationHandler.cs b/samples/apps/copilot-chat-app/webapi/Auth/PassThroughAuthenticationHandler.cs index 5ba663068f88..b2f37dfcc8d1 100644 --- a/samples/apps/copilot-chat-app/webapi/Auth/PassThroughAuthenticationHandler.cs +++ b/samples/apps/copilot-chat-app/webapi/Auth/PassThroughAuthenticationHandler.cs @@ -17,6 +17,7 @@ public class PassThroughAuthenticationHandler : AuthenticationHandler /// Constructor @@ -33,7 +34,8 @@ protected override Task HandleAuthenticateAsync() { this.Logger.LogInformation("Allowing request to pass through"); Claim userIdClaim = new(JwtRegisteredClaimNames.Sub, DefaultUserId); - ClaimsIdentity identity = new(new Claim[] { userIdClaim }, AuthenticationScheme); + Claim nameClaim = new(JwtRegisteredClaimNames.Name, DefaultUserName); + ClaimsIdentity identity = new(new Claim[] { userIdClaim, nameClaim }, AuthenticationScheme); ClaimsPrincipal principal = new(identity); AuthenticationTicket ticket = new(principal, this.Scheme.Name); diff --git a/samples/apps/copilot-chat-app/webapi/Controllers/SemanticKernelController.cs b/samples/apps/copilot-chat-app/webapi/Controllers/SemanticKernelController.cs index e08b32760756..c2afdcab323a 100644 --- a/samples/apps/copilot-chat-app/webapi/Controllers/SemanticKernelController.cs +++ b/samples/apps/copilot-chat-app/webapi/Controllers/SemanticKernelController.cs @@ -3,7 +3,6 @@ using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; -using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Logging; @@ -11,7 +10,10 @@ using Microsoft.SemanticKernel.AI; using Microsoft.SemanticKernel.Orchestration; using Microsoft.SemanticKernel.SkillDefinition; +using SemanticKernel.Service.Auth; +using SemanticKernel.Service.CopilotChat.Storage; using SemanticKernel.Service.Models; +using SemanticKernel.Service.Utilities; namespace SemanticKernel.Service.Controllers; @@ -34,11 +36,13 @@ public SemanticKernelController(ILogger logger) /// and attempt to invoke the function with the given name. /// /// Semantic kernel obtained through dependency injection + /// Converter to use for converting Asks. + /// Storage for chat sessions. + /// Authenticated info about the user for the current request. /// Prompt along with its parameters /// Skill in which function to invoke resides /// Name of function to invoke /// Results consisting of text generated by invoked function along with the variable in the SK that generated it - [Authorize] [Route("skills/{skillName}/functions/{functionName}/invoke")] [HttpPost] [ProducesResponseType(StatusCodes.Status200OK)] @@ -46,22 +50,31 @@ public SemanticKernelController(ILogger logger) [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task> InvokeFunctionAsync( [FromServices] IKernel kernel, + [FromServices] AskConverter askConverter, + [FromServices] ChatSessionRepository chatSessionRepository, + [FromServices] IAuthInfo authInfo, [FromBody] Ask ask, string skillName, string functionName) { this._logger.LogDebug("Received call to invoke {SkillName}/{FunctionName}", skillName, functionName); - if (string.IsNullOrWhiteSpace(ask.Input)) + const string chatIdKey = "chatId"; + var chatIdFromContext = ask.Variables.FirstOrDefault(x => x.Key == chatIdKey); + if (chatIdFromContext.Key is chatIdKey) { - return this.BadRequest("Input is required."); + var chat = await chatSessionRepository.FindByIdAsync(chatIdFromContext.Value); + if (chat == null) + { + return this.NotFound("Failed to find chat session for the chatId specified in variables."); + } + if (chat.UserId != authInfo.UserId) + { + return this.Unauthorized("User does not have access to the chatId specified in variables."); + } } // Put ask's variables in the context we will use. - var contextVariables = new ContextVariables(ask.Input); - foreach (var input in ask.Variables) - { - contextVariables.Set(input.Key, input.Value); - } + var contextVariables = askConverter.GetContextVariables(ask); // Get the function to invoke ISKFunction? function = null; diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatController.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatController.cs index 713e5d702e0f..43888b83b4dd 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatController.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatController.cs @@ -21,10 +21,13 @@ using Microsoft.SemanticKernel.Skills.MsGraph.Connectors; using Microsoft.SemanticKernel.Skills.MsGraph.Connectors.Client; using Microsoft.SemanticKernel.Skills.OpenAPI.Authentication; +using SemanticKernel.Service.Auth; using SemanticKernel.Service.CopilotChat.Models; using SemanticKernel.Service.CopilotChat.Options; using SemanticKernel.Service.CopilotChat.Skills.ChatSkills; +using SemanticKernel.Service.CopilotChat.Storage; using SemanticKernel.Service.Models; +using SemanticKernel.Service.Utilities; namespace SemanticKernel.Service.CopilotChat.Controllers; @@ -51,6 +54,9 @@ public ChatController(ILogger logger) /// Semantic kernel obtained through dependency injection. /// Planner to use to create function sequences. /// Options for the planner. + /// Converter to use for converting Asks. + /// Storage for chat sessions. + /// Authenticated info about the user for the current request. /// Prompt along with its parameters. /// Authentication headers to connect to OpenAPI Skills. /// Results containing the response from the model. @@ -63,18 +69,31 @@ public async Task ChatAsync( [FromServices] IKernel kernel, [FromServices] CopilotChatPlanner? planner, [FromServices] IOptions plannerOptions, + [FromServices] AskConverter askConverter, + [FromServices] ChatSessionRepository chatSessionRepository, + [FromServices] IAuthInfo authInfo, [FromBody] Ask ask, [FromHeader] OpenApiSkillsAuthHeaders openApiSkillsAuthHeaders) { this._logger.LogDebug("Chat request received."); - - // Put ask's variables in the context we will use. - var contextVariables = new ContextVariables(ask.Input); - foreach (var input in ask.Variables) + const string chatIdKey = "chatId"; + var chatIdFromContext = ask.Variables.FirstOrDefault(x => x.Key == chatIdKey); + if (chatIdFromContext.Key is chatIdKey) { - contextVariables.Set(input.Key, input.Value); + var chat = await chatSessionRepository.FindByIdAsync(chatIdFromContext.Value); + if (chat == null) + { + return this.NotFound("Failed to find chat session for the chatId specified in variables."); + } + if (chat.UserId != authInfo.UserId) + { + return this.Unauthorized("User does not have access to the chatId specified in variables."); + } } + // Put ask's variables in the context we will use. + var contextVariables = askConverter.GetContextVariables(ask); + // Register plugins that have been enabled if (planner != null && plannerOptions.Value.Enabled) { diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Extensions/ServiceExtensions.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Extensions/ServiceExtensions.cs index 049512193426..e8e9374842f9 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Extensions/ServiceExtensions.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Extensions/ServiceExtensions.cs @@ -78,7 +78,7 @@ public static IServiceCollection AddCopilotChatOptions(this IServiceCollection s /// /// Add persistent chat store services. /// - public static void AddPersistentChatStore(this IServiceCollection services) + public static IServiceCollection AddPersistentChatStore(this IServiceCollection services) { IStorageContext chatSessionInMemoryContext; IStorageContext chatMessageInMemoryContext; @@ -134,7 +134,7 @@ public static void AddPersistentChatStore(this IServiceCollection services) } services.AddSingleton(new ChatSessionRepository(chatSessionInMemoryContext)); - services.AddSingleton(new ChatMessageRepository(chatMessageInMemoryContext)); + return services.AddSingleton(new ChatMessageRepository(chatMessageInMemoryContext)); } /// diff --git a/samples/apps/copilot-chat-app/webapi/Models/Ask.cs b/samples/apps/copilot-chat-app/webapi/Models/Ask.cs index d72f95b3025e..0379e65dc513 100644 --- a/samples/apps/copilot-chat-app/webapi/Models/Ask.cs +++ b/samples/apps/copilot-chat-app/webapi/Models/Ask.cs @@ -1,12 +1,15 @@ // Copyright (c) Microsoft. All rights reserved. using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; using System.Linq; +using SemanticKernel.Service.Options; namespace SemanticKernel.Service.Models; public class Ask { + [Required, NotEmptyOrWhitespace] public string Input { get; set; } = string.Empty; public IEnumerable> Variables { get; set; } = Enumerable.Empty>(); diff --git a/samples/apps/copilot-chat-app/webapi/Program.cs b/samples/apps/copilot-chat-app/webapi/Program.cs index 5156b5162348..6d93ef65ce68 100644 --- a/samples/apps/copilot-chat-app/webapi/Program.cs +++ b/samples/apps/copilot-chat-app/webapi/Program.cs @@ -44,7 +44,8 @@ public static async Task Main(string[] args) builder.Services .AddCopilotChatOptions(builder.Configuration) .AddCopilotChatPlannerServices() - .AddPersistentChatStore(); + .AddPersistentChatStore() + .AddCopilotChatUtilities(); // Add in the rest of the services. builder.Services diff --git a/samples/apps/copilot-chat-app/webapi/ServiceExtensions.cs b/samples/apps/copilot-chat-app/webapi/ServiceExtensions.cs index 36cd177b4e08..8b0b229240a0 100644 --- a/samples/apps/copilot-chat-app/webapi/ServiceExtensions.cs +++ b/samples/apps/copilot-chat-app/webapi/ServiceExtensions.cs @@ -12,6 +12,7 @@ using Microsoft.Identity.Web; using SemanticKernel.Service.Auth; using SemanticKernel.Service.Options; +using SemanticKernel.Service.Utilities; namespace SemanticKernel.Service; @@ -54,6 +55,11 @@ internal static IServiceCollection AddOptions(this IServiceCollection services, return services; } + internal static IServiceCollection AddCopilotChatUtilities(this IServiceCollection services) + { + return services.AddScoped(); + } + /// /// Add CORS settings. /// diff --git a/samples/apps/copilot-chat-app/webapi/Utilities/AskConverter.cs b/samples/apps/copilot-chat-app/webapi/Utilities/AskConverter.cs new file mode 100644 index 000000000000..851672ca50e7 --- /dev/null +++ b/samples/apps/copilot-chat-app/webapi/Utilities/AskConverter.cs @@ -0,0 +1,38 @@ +// Copyright (c) Microsoft. All rights reserved. + +using Microsoft.SemanticKernel.Orchestration; +using SemanticKernel.Service.Auth; +using SemanticKernel.Service.Models; + +namespace SemanticKernel.Service.Utilities; + +public class AskConverter +{ + private readonly IAuthInfo _authInfo; + + public AskConverter(IAuthInfo authInfo) + { + this._authInfo = authInfo; + } + + /// + /// Converts variables to , inserting some system variables along the way. + /// + public ContextVariables GetContextVariables(Ask ask) + { + const string userIdKey = "userId"; + const string userNameKey = "userName"; + var contextVariables = new ContextVariables(ask.Input); + foreach (var input in ask.Variables) + { + if (input.Key != userIdKey && input.Key != userNameKey) + { + contextVariables.Set(input.Key, input.Value); + } + } + + contextVariables.Set(userIdKey, this._authInfo.UserId); + contextVariables.Set(userNameKey, this._authInfo.Name); + return contextVariables; + } +} diff --git a/samples/apps/copilot-chat-app/webapp/src/components/views/BackendProbe.tsx b/samples/apps/copilot-chat-app/webapp/src/components/views/BackendProbe.tsx index e70411301112..3e45ca93fe01 100644 --- a/samples/apps/copilot-chat-app/webapp/src/components/views/BackendProbe.tsx +++ b/samples/apps/copilot-chat-app/webapp/src/components/views/BackendProbe.tsx @@ -10,7 +10,7 @@ interface IData { const BackendProbe: FC = ({ uri, onBackendFound }) => { useEffect(() => { - const requestUrl = new URL('probe', uri); + const requestUrl = new URL('healthz', uri); const fetchAsync = async () => { try { var result = await fetch(requestUrl); diff --git a/samples/apps/copilot-chat-app/webapp/src/libs/services/BotService.ts b/samples/apps/copilot-chat-app/webapp/src/libs/services/BotService.ts index e06d045bb9da..f93f36c87405 100644 --- a/samples/apps/copilot-chat-app/webapp/src/libs/services/BotService.ts +++ b/samples/apps/copilot-chat-app/webapp/src/libs/services/BotService.ts @@ -19,11 +19,11 @@ export class BotService extends BaseService { return result; }; - public uploadAsync = async (bot: Bot, userId: string, accessToken: string): Promise => { + public uploadAsync = async (bot: Bot, accessToken: string): Promise => { // TODO: return type const result = await this.getResponseAsync( { - commandPath: `bot/upload?userId=${userId}`, + commandPath: `bot/upload`, method: 'Post', body: bot, }, diff --git a/samples/apps/copilot-chat-app/webapp/src/libs/services/ChatService.ts b/samples/apps/copilot-chat-app/webapp/src/libs/services/ChatService.ts index 58d525554b1b..32a5a7c3915d 100644 --- a/samples/apps/copilot-chat-app/webapp/src/libs/services/ChatService.ts +++ b/samples/apps/copilot-chat-app/webapp/src/libs/services/ChatService.ts @@ -9,22 +9,18 @@ import { BaseService } from './BaseService'; export class ChatService extends BaseService { public createChatAsync = async ( - userId: string, - userName: string, title: string, accessToken: string, ): Promise => { const body = { - userId: userId, - userName: userName, - title: title, + title, }; const result = await this.getResponseAsync( { - commandPath: 'chatSession/create', + commandPath: 'chatSessions', method: 'POST', - body: body, + body, }, accessToken, ); @@ -35,7 +31,7 @@ export class ChatService extends BaseService { public getChatAsync = async (chatId: string, accessToken: string): Promise => { const result = await this.getResponseAsync( { - commandPath: `chatSession/getChat/${chatId}`, + commandPath: `chatSessions/${chatId}`, method: 'GET', }, accessToken, @@ -44,10 +40,10 @@ export class ChatService extends BaseService { return result; }; - public getAllChatsAsync = async (userId: string, accessToken: string): Promise => { + public getAllChatsAsync = async (accessToken: string): Promise => { const result = await this.getResponseAsync( { - commandPath: `chatSession/getAllChats/${userId}`, + commandPath: `chatSessions`, method: 'GET', }, accessToken, @@ -63,7 +59,7 @@ export class ChatService extends BaseService { ): Promise => { const result = await this.getResponseAsync( { - commandPath: `chatSession/getChatMessages/${chatId}?startIdx=${startIdx}&count=${count}`, + commandPath: `chatSessions/${chatId}/messages?startIdx=${startIdx}&count=${count}`, method: 'GET', }, accessToken, @@ -73,17 +69,15 @@ export class ChatService extends BaseService { }; public editChatAsync = async (chatId: string, title: string, accessToken: string): Promise => { - const body: IChatSession = { - id: chatId, - userId: '', - title: title, + const body: Partial = { + title, }; const result = await this.getResponseAsync( { - commandPath: `chatSession/edit`, - method: 'POST', - body: body, + commandPath: `chatSessions/${chatId}`, + method: 'PATCH', + body, }, accessToken, ); diff --git a/samples/apps/copilot-chat-app/webapp/src/libs/useChat.ts b/samples/apps/copilot-chat-app/webapp/src/libs/useChat.ts index de6100907a3c..16fa9ada232a 100644 --- a/samples/apps/copilot-chat-app/webapp/src/libs/useChat.ts +++ b/samples/apps/copilot-chat-app/webapp/src/libs/useChat.ts @@ -61,8 +61,6 @@ export const useChat = () => { try { await chatService .createChatAsync( - account?.homeAccountId!, - account?.name!, chatTitle, await AuthHelper.getSKaaSAccessToken(instance), ) @@ -103,14 +101,6 @@ export const useChat = () => { const ask = { input: value, variables: [ - { - key: 'userId', - value: account?.homeAccountId!, - }, - { - key: 'userName', - value: account?.name!, - }, { key: 'chatId', value: chatId, @@ -164,7 +154,6 @@ export const useChat = () => { const loadChats = async () => { try { const chatSessions = await chatService.getAllChatsAsync( - account?.homeAccountId!, await AuthHelper.getSKaaSAccessToken(instance), ); @@ -222,7 +211,7 @@ export const useChat = () => { const uploadBot = async (bot: Bot) => { botService - .uploadAsync(bot, account?.homeAccountId || '', await AuthHelper.getSKaaSAccessToken(instance)) + .uploadAsync(bot, await AuthHelper.getSKaaSAccessToken(instance)) .then(() => loadChats()) .catch((e: any) => { const errorMessage = `Unable to upload the bot. Details: ${e.message ?? e}`; From 6d10c376faf185e2ec24f03021ab0cc0838d916d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Parks=20=C2=A9=EF=B8=8F?= Date: Thu, 18 May 2023 19:56:38 -0700 Subject: [PATCH 03/15] ran dotnet format --- .../webapi/Auth/ChatOwnerAuthorizationHandler.cs | 8 ++++---- .../webapi/Controllers/SemanticKernelController.cs | 2 +- .../webapi/CopilotChat/Controllers/BotController.cs | 6 +++--- .../CopilotChat/Controllers/ChatHistoryController.cs | 12 ++++++------ .../Controllers/DocumentImportController.cs | 4 ++-- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/samples/apps/copilot-chat-app/webapi/Auth/ChatOwnerAuthorizationHandler.cs b/samples/apps/copilot-chat-app/webapi/Auth/ChatOwnerAuthorizationHandler.cs index 0a1a938a4137..6e3f090ef242 100644 --- a/samples/apps/copilot-chat-app/webapi/Auth/ChatOwnerAuthorizationHandler.cs +++ b/samples/apps/copilot-chat-app/webapi/Auth/ChatOwnerAuthorizationHandler.cs @@ -13,17 +13,17 @@ namespace SemanticKernel.Service.Auth; /// public class ChatOwnerAuthorizationHandler : AuthorizationHandler { - private readonly IAuthInfo _authInfoProvider; + private readonly IAuthInfo _authInfo; private readonly ChatSessionRepository _chatSessionRepository; /// /// Constructor /// public ChatOwnerAuthorizationHandler( - IAuthInfo authInfoProvider, + IAuthInfo authInfo, ChatSessionRepository chatSessionRepository) : base() { - this._authInfoProvider = authInfoProvider; + this._authInfo = authInfo; this._chatSessionRepository = chatSessionRepository; } @@ -48,7 +48,7 @@ protected override async Task HandleRequirementAsync( return; } - if (session.UserId != this._authInfoProvider.UserId) + if (session.UserId != this._authInfo.UserId) { context.Fail(new AuthorizationFailureReason(this, "User does not have access to the requested chat.")); } diff --git a/samples/apps/copilot-chat-app/webapi/Controllers/SemanticKernelController.cs b/samples/apps/copilot-chat-app/webapi/Controllers/SemanticKernelController.cs index c2afdcab323a..8b1bea36e38b 100644 --- a/samples/apps/copilot-chat-app/webapi/Controllers/SemanticKernelController.cs +++ b/samples/apps/copilot-chat-app/webapi/Controllers/SemanticKernelController.cs @@ -74,7 +74,7 @@ public async Task> InvokeFunctionAsync( } // Put ask's variables in the context we will use. - var contextVariables = askConverter.GetContextVariables(ask); + var contextVariables = askConverter.GetContextVariables(ask); // Get the function to invoke ISKFunction? function = null; diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs index 996739d009ec..7ae14f626c9c 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs @@ -63,7 +63,7 @@ public BotController( /// Upload a bot. /// /// The Semantic Kernel instance. - /// The auth info instance. + /// The auth info instance. /// The bot object from the message body /// The HTTP action result with new chat session object. [HttpPost] @@ -73,7 +73,7 @@ public BotController( [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task> UploadAsync( [FromServices] IKernel kernel, - [FromServices] IAuthInfo authInfoProvider, + [FromServices] IAuthInfo authInfo, [FromBody] Bot bot) { this._logger.LogDebug("Received call to upload a bot"); @@ -98,7 +98,7 @@ public async Task> UploadAsync( // Upload chat history into chat repository and embeddings into memory. // 1. Create a new chat and get the chat id. - newChat = new ChatSession(authInfoProvider.UserId, chatTitle); + newChat = new ChatSession(authInfo.UserId, chatTitle); await this._chatRepository.CreateAsync(newChat); chatId = newChat.Id; diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatHistoryController.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatHistoryController.cs index 7f75514968f7..aa8f04f6cf7c 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatHistoryController.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatHistoryController.cs @@ -27,7 +27,7 @@ public class ChatHistoryController : ControllerBase private readonly ILogger _logger; private readonly ChatSessionRepository _chatSessionRepository; private readonly ChatMessageRepository _chatMessageRepository; - private readonly IAuthInfo _authInfoProvider; + private readonly IAuthInfo _authInfo; private readonly PromptsOptions _promptOptions; /// @@ -37,18 +37,18 @@ public class ChatHistoryController : ControllerBase /// The chat session repository. /// The chat message repository. /// The prompts options. - /// The auth info for the current request. + /// The auth info for the current request. public ChatHistoryController( ILogger logger, ChatSessionRepository chatSessionRepository, ChatMessageRepository chatMessageRepository, IOptions promptsOptions, - IAuthInfo authInfoProvider) + IAuthInfo authInfo) { this._logger = logger; this._chatSessionRepository = chatSessionRepository; this._chatMessageRepository = chatMessageRepository; - this._authInfoProvider = authInfoProvider; + this._authInfo = authInfo; this._promptOptions = promptsOptions.Value; } @@ -65,7 +65,7 @@ public ChatHistoryController( public async Task CreateChatSessionAsync( [FromBody] ChatSessionCreationOptions chatParameters) { - var userId = _authInfoProvider.UserId; + var userId = _authInfo.UserId; var title = chatParameters.Title; var newChat = new ChatSession(userId, title); @@ -110,7 +110,7 @@ public async Task GetChatSessionByIdAsync(Guid chatId) [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task GetAllChatSessionsAsync() { - var userId = this._authInfoProvider.UserId; + var userId = this._authInfo.UserId; var chats = await this._chatSessionRepository.FindByUserIdAsync(userId); if (chats == null) { diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/DocumentImportController.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/DocumentImportController.cs index 1ad4c3a30464..8c1c82f718ec 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/DocumentImportController.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/DocumentImportController.cs @@ -66,7 +66,7 @@ public DocumentImportController( [ProducesResponseType(StatusCodes.Status400BadRequest)] public async Task ImportDocumentAsync( [FromServices] IKernel kernel, - [FromServices] IAuthInfo authInfoProvider, + [FromServices] IAuthInfo authInfo, [FromForm] DocumentImportForm documentImportForm) { var formFile = documentImportForm.FormFile; @@ -91,7 +91,7 @@ public async Task ImportDocumentAsync( return this.NotFound("Session does not exist."); } - if (documentImportForm.DocumentScope == DocumentImportForm.DocumentScopes.Chat && authInfoProvider.UserId != chatSession.UserId) + if (documentImportForm.DocumentScope == DocumentImportForm.DocumentScopes.Chat && authInfo.UserId != chatSession.UserId) { return this.Unauthorized("User does not have access to the chat session."); } From 6719f2fb842d55ba70e5dde8764411031c0e3ea3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Parks=20=C2=A9=EF=B8=8F?= Date: Thu, 18 May 2023 20:14:05 -0700 Subject: [PATCH 04/15] missed a few places --- samples/apps/copilot-chat-app/importdocument/Program.cs | 2 -- .../webapi/CopilotChat/Controllers/BotController.cs | 4 ++-- .../webapp/src/libs/services/DocumentImportService.ts | 8 +++----- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/samples/apps/copilot-chat-app/importdocument/Program.cs b/samples/apps/copilot-chat-app/importdocument/Program.cs index 2fb00bf56c9e..1ec526e85590 100644 --- a/samples/apps/copilot-chat-app/importdocument/Program.cs +++ b/samples/apps/copilot-chat-app/importdocument/Program.cs @@ -115,10 +115,8 @@ private static async Task UploadFileAsync(FileInfo file, Config config, Guid cha { Console.WriteLine($"Successfully acquired User ID. Continuing..."); using var chatScopeContent = new StringContent("Chat"); - using var userIdContent = new StringContent(userId); using var chatCollectionIdContent = new StringContent(chatCollectionId.ToString()); formContent.Add(chatScopeContent, "documentScope"); - formContent.Add(userIdContent, "userId"); formContent.Add(chatCollectionIdContent, "chatId"); // Calling UploadAsync here to make sure disposable objects are still in scope. diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs index 7ae14f626c9c..26485fb69add 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs @@ -139,14 +139,14 @@ public async Task> UploadAsync( [ProducesResponseType(StatusCodes.Status400BadRequest)] [ProducesResponseType(StatusCodes.Status404NotFound)] [Authorize(Policy = AuthPolicyName.RequireChatOwner)] - public async Task> DownloadAsync( + public async Task> DownloadAsync( [FromServices] IKernel kernel, Guid chatId) { this._logger.LogDebug("Received call to download a bot"); var memory = await this.CreateBotAsync(kernel: kernel, chatId: chatId); - return JsonSerializer.Serialize(memory); + return this.Ok(memory); } /// diff --git a/samples/apps/copilot-chat-app/webapp/src/libs/services/DocumentImportService.ts b/samples/apps/copilot-chat-app/webapp/src/libs/services/DocumentImportService.ts index 90f40aaf8403..4b021b278f1b 100644 --- a/samples/apps/copilot-chat-app/webapp/src/libs/services/DocumentImportService.ts +++ b/samples/apps/copilot-chat-app/webapp/src/libs/services/DocumentImportService.ts @@ -5,17 +5,15 @@ export class DocumentImportService { constructor(private readonly serviceUrl: string) { } importDocumentAsync = async ( - userId: string, chatId: string, document: File, accessToken: string, ) => { const formData = new FormData(); - formData.append('userId', userId); formData.append('chatId', chatId); formData.append('documentScope', 'Chat'); formData.append('formFile', document); - + const commandPath = `importDocument`; const requestUrl = new URL(commandPath, this.serviceUrl); @@ -27,7 +25,7 @@ export class DocumentImportService { const response = await fetch(requestUrl, { method: 'POST', body: formData, - headers: headers, + headers, }); if (!response.ok) { @@ -43,4 +41,4 @@ export class DocumentImportService { throw Object.assign(new Error(e + additional_error_msg)); } }; -} \ No newline at end of file +} From 03d7ea933968983c8050e7bf08f8eeb56788389e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Parks=20=C2=A9=EF=B8=8F?= Date: Thu, 18 May 2023 20:14:57 -0700 Subject: [PATCH 05/15] format --- .../webapi/CopilotChat/Controllers/BotController.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs index 26485fb69add..bc58f85debe7 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Text.Json; using System.Threading.Tasks; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; From dd74c84ef470f860b5776a9411a9ceb853b6eef7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Parks=20=C2=A9=EF=B8=8F?= Date: Thu, 18 May 2023 20:44:30 -0700 Subject: [PATCH 06/15] remove unused arg --- .../copilot-chat-app/webapp/src/components/chat/ChatInput.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/samples/apps/copilot-chat-app/webapp/src/components/chat/ChatInput.tsx b/samples/apps/copilot-chat-app/webapp/src/components/chat/ChatInput.tsx index 7a9ceb375a44..2d76c06c23ba 100644 --- a/samples/apps/copilot-chat-app/webapp/src/components/chat/ChatInput.tsx +++ b/samples/apps/copilot-chat-app/webapp/src/components/chat/ChatInput.tsx @@ -113,7 +113,6 @@ export const ChatInput: React.FC = (props) => { try { SetDocumentImporting(true); await documentImportService.importDocumentAsync( - account!.homeAccountId!, selectedId, documentFile, await AuthHelper.getSKaaSAccessToken(instance, inProgress), From a7b6953f726e4ca68b65cc329c84f9ba40233e55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Parks=20=C2=A9=EF=B8=8F?= Date: Thu, 18 May 2023 21:15:54 -0700 Subject: [PATCH 07/15] remove unused param --- .../webapp/src/components/chat/ChatInput.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/samples/apps/copilot-chat-app/webapp/src/components/chat/ChatInput.tsx b/samples/apps/copilot-chat-app/webapp/src/components/chat/ChatInput.tsx index 2d76c06c23ba..3100a1fc0f85 100644 --- a/samples/apps/copilot-chat-app/webapp/src/components/chat/ChatInput.tsx +++ b/samples/apps/copilot-chat-app/webapp/src/components/chat/ChatInput.tsx @@ -1,6 +1,6 @@ // Copyright (c) Microsoft. All rights reserved. -import { useAccount, useMsal } from '@azure/msal-react'; +import { useMsal } from '@azure/msal-react'; import { Button, Spinner, Textarea, makeStyles, shorthands, tokens } from '@fluentui/react-components'; import { AttachRegular, MicRegular, SendRegular } from '@fluentui/react-icons'; import debug from 'debug'; @@ -63,8 +63,7 @@ interface ChatInputProps { export const ChatInput: React.FC = (props) => { const { isTyping, onSubmit } = props; const classes = useClasses(); - const { instance, accounts, inProgress } = useMsal(); - const account = useAccount(accounts[0] || {}); + const { instance, inProgress } = useMsal(); const dispatch = useAppDispatch(); const [value, setValue] = React.useState(''); const [previousValue, setPreviousValue] = React.useState(''); From 7e0b1aeb8c04e8880c51149dd49e47c9a4ee72d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Parks=20=C2=A9=EF=B8=8F?= Date: Sat, 20 May 2023 10:22:50 -0700 Subject: [PATCH 08/15] Address comments --- .../{ => CopilotChat}/Auth/AuthPolicyName.cs | 2 +- .../Auth/ChatOwnerAuthorizationHandler.cs | 3 ++- .../Auth/ChatOwnerRequirement.cs | 2 +- .../CopilotChat/Controllers/BotController.cs | 1 + .../CopilotChat/Controllers/ChatController.cs | 2 +- .../Controllers/ChatHistoryController.cs | 1 + .../Controllers/DocumentImportController.cs | 18 ++++++++------- .../Extensions/ServiceExtensions.cs | 22 +++++++++++++++++++ .../webapi/CopilotChat/Models/ChatAsk.cs | 13 +++++++++++ .../copilot-chat-app/webapi/Models/Ask.cs | 5 +---- .../webapi/ServiceExtensions.cs | 20 +---------------- 11 files changed, 54 insertions(+), 35 deletions(-) rename samples/apps/copilot-chat-app/webapi/{ => CopilotChat}/Auth/AuthPolicyName.cs (82%) rename samples/apps/copilot-chat-app/webapi/{ => CopilotChat}/Auth/ChatOwnerAuthorizationHandler.cs (95%) rename samples/apps/copilot-chat-app/webapi/{ => CopilotChat}/Auth/ChatOwnerRequirement.cs (83%) create mode 100644 samples/apps/copilot-chat-app/webapi/CopilotChat/Models/ChatAsk.cs diff --git a/samples/apps/copilot-chat-app/webapi/Auth/AuthPolicyName.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Auth/AuthPolicyName.cs similarity index 82% rename from samples/apps/copilot-chat-app/webapi/Auth/AuthPolicyName.cs rename to samples/apps/copilot-chat-app/webapi/CopilotChat/Auth/AuthPolicyName.cs index 33f703819c54..751b4a782d00 100644 --- a/samples/apps/copilot-chat-app/webapi/Auth/AuthPolicyName.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Auth/AuthPolicyName.cs @@ -1,6 +1,6 @@ // Copyright (c) Microsoft. All rights reserved. -namespace SemanticKernel.Service.Auth; +namespace SemanticKernel.Service.CopilotChat.Auth; /// /// Holds the policy names for custom authorization policies. diff --git a/samples/apps/copilot-chat-app/webapi/Auth/ChatOwnerAuthorizationHandler.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Auth/ChatOwnerAuthorizationHandler.cs similarity index 95% rename from samples/apps/copilot-chat-app/webapi/Auth/ChatOwnerAuthorizationHandler.cs rename to samples/apps/copilot-chat-app/webapi/CopilotChat/Auth/ChatOwnerAuthorizationHandler.cs index 6e3f090ef242..0ecbc64c8d07 100644 --- a/samples/apps/copilot-chat-app/webapi/Auth/ChatOwnerAuthorizationHandler.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Auth/ChatOwnerAuthorizationHandler.cs @@ -4,9 +4,10 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; +using SemanticKernel.Service.Auth; using SemanticKernel.Service.CopilotChat.Storage; -namespace SemanticKernel.Service.Auth; +namespace SemanticKernel.Service.CopilotChat.Auth; /// /// Class implementing "authorization" that validates the user has access to a chat. diff --git a/samples/apps/copilot-chat-app/webapi/Auth/ChatOwnerRequirement.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Auth/ChatOwnerRequirement.cs similarity index 83% rename from samples/apps/copilot-chat-app/webapi/Auth/ChatOwnerRequirement.cs rename to samples/apps/copilot-chat-app/webapi/CopilotChat/Auth/ChatOwnerRequirement.cs index 7a9ea8bc8870..6d931c8baebf 100644 --- a/samples/apps/copilot-chat-app/webapi/Auth/ChatOwnerRequirement.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Auth/ChatOwnerRequirement.cs @@ -2,7 +2,7 @@ using Microsoft.AspNetCore.Authorization; -namespace SemanticKernel.Service.Auth; +namespace SemanticKernel.Service.CopilotChat.Auth; /// /// Used to require the chat to be owned by the authenticated user. diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs index bc58f85debe7..655873e60db3 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs @@ -12,6 +12,7 @@ using Microsoft.SemanticKernel; using Microsoft.SemanticKernel.Memory; using SemanticKernel.Service.Auth; +using SemanticKernel.Service.CopilotChat.Auth; using SemanticKernel.Service.CopilotChat.Models; using SemanticKernel.Service.CopilotChat.Options; using SemanticKernel.Service.CopilotChat.Storage; diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatController.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatController.cs index bdf3ece6a32e..dd8bede635ff 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatController.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatController.cs @@ -72,7 +72,7 @@ public async Task ChatAsync( [FromServices] AskConverter askConverter, [FromServices] ChatSessionRepository chatSessionRepository, [FromServices] IAuthInfo authInfo, - [FromBody] Ask ask, + [FromBody] ChatAsk ask, [FromHeader] OpenApiSkillsAuthHeaders openApiSkillsAuthHeaders) { this._logger.LogDebug("Chat request received."); diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatHistoryController.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatHistoryController.cs index aa8f04f6cf7c..5510dba66831 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatHistoryController.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatHistoryController.cs @@ -10,6 +10,7 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using SemanticKernel.Service.Auth; +using SemanticKernel.Service.CopilotChat.Auth; using SemanticKernel.Service.CopilotChat.Models; using SemanticKernel.Service.CopilotChat.Options; using SemanticKernel.Service.CopilotChat.Storage; diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/DocumentImportController.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/DocumentImportController.cs index 8c1c82f718ec..02f5ea9a65ca 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/DocumentImportController.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/DocumentImportController.cs @@ -85,18 +85,20 @@ public async Task ImportDocumentAsync( return this.BadRequest("File size exceeds the limit."); } - var chatSession = await this._chatSessionRepository.FindByIdAsync(documentImportForm.ChatId.ToString()); - if (chatSession == null) + if (documentImportForm.DocumentScope == DocumentImportForm.DocumentScopes.Chat) { - return this.NotFound("Session does not exist."); - } + var chatSession = await this._chatSessionRepository.FindByIdAsync(documentImportForm.ChatId.ToString()); + if (chatSession == null) + { + return this.NotFound("Session does not exist."); + } - if (documentImportForm.DocumentScope == DocumentImportForm.DocumentScopes.Chat && authInfo.UserId != chatSession.UserId) - { - return this.Unauthorized("User does not have access to the chat session."); + if (authInfo.UserId != chatSession.UserId) + { + return this.Unauthorized("User does not have access to the chat session."); + } } - this._logger.LogInformation("Importing document {0}", formFile.FileName); try diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Extensions/ServiceExtensions.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Extensions/ServiceExtensions.cs index 0d748f42ca7c..46cddf83163f 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Extensions/ServiceExtensions.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Extensions/ServiceExtensions.cs @@ -4,9 +4,11 @@ using System.Collections.Generic; using System.IO; using System.Reflection; +using Microsoft.AspNetCore.Authorization; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; +using SemanticKernel.Service.CopilotChat.Auth; using SemanticKernel.Service.CopilotChat.Models; using SemanticKernel.Service.CopilotChat.Options; using SemanticKernel.Service.CopilotChat.Storage; @@ -70,6 +72,26 @@ public static IServiceCollection AddCopilotChatOptions(this IServiceCollection s return services; } + /// + /// Add authorization services for the copilot chat. + /// + public static IServiceCollection AddCopilotChatAuthorization(this IServiceCollection services) + { + return services.AddScoped() + .AddAuthorizationCore(options => + { + options.DefaultPolicy = new AuthorizationPolicyBuilder() + .RequireAuthenticatedUser() + .Build(); + + options.AddPolicy(AuthPolicyName.RequireChatOwner, builder => + { + builder.RequireAuthenticatedUser() + .AddRequirements(new ChatOwnerRequirement()); + }); + }); + } + /// /// Add persistent chat store services. /// diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/ChatAsk.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/ChatAsk.cs new file mode 100644 index 000000000000..bf459334ad5f --- /dev/null +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Models/ChatAsk.cs @@ -0,0 +1,13 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.ComponentModel.DataAnnotations; +using SemanticKernel.Service.Models; +using SemanticKernel.Service.Options; + +namespace SemanticKernel.Service.CopilotChat.Models; + +public class ChatAsk : Ask +{ + [Required, NotEmptyOrWhitespace] + public override string Input { get => base.Input; set => base.Input = value; } +} diff --git a/samples/apps/copilot-chat-app/webapi/Models/Ask.cs b/samples/apps/copilot-chat-app/webapi/Models/Ask.cs index 0379e65dc513..2406dceb906c 100644 --- a/samples/apps/copilot-chat-app/webapi/Models/Ask.cs +++ b/samples/apps/copilot-chat-app/webapi/Models/Ask.cs @@ -1,16 +1,13 @@ // Copyright (c) Microsoft. All rights reserved. using System.Collections.Generic; -using System.ComponentModel.DataAnnotations; using System.Linq; -using SemanticKernel.Service.Options; namespace SemanticKernel.Service.Models; public class Ask { - [Required, NotEmptyOrWhitespace] - public string Input { get; set; } = string.Empty; + public virtual string Input { get; set; } = string.Empty; public IEnumerable> Variables { get; set; } = Enumerable.Empty>(); } diff --git a/samples/apps/copilot-chat-app/webapi/ServiceExtensions.cs b/samples/apps/copilot-chat-app/webapi/ServiceExtensions.cs index 663a4531d189..0095150b3552 100644 --- a/samples/apps/copilot-chat-app/webapi/ServiceExtensions.cs +++ b/samples/apps/copilot-chat-app/webapi/ServiceExtensions.cs @@ -5,7 +5,6 @@ using System.Reflection; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authentication.JwtBearer; -using Microsoft.AspNetCore.Authorization; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; @@ -89,6 +88,7 @@ internal static IServiceCollection AddCors(this IServiceCollection services) /// internal static IServiceCollection AddCopilotChatAuthentication(this IServiceCollection services, IConfiguration configuration) { + services.AddScoped(); var config = services.BuildServiceProvider().GetRequiredService>().Value; switch (config.Type) { @@ -111,24 +111,6 @@ internal static IServiceCollection AddCopilotChatAuthentication(this IServiceCol return services; } - internal static IServiceCollection AddCopilotChatAuthorization(this IServiceCollection services) - { - return services.AddScoped() - .AddScoped() - .AddAuthorizationCore(options => - { - options.DefaultPolicy = new AuthorizationPolicyBuilder() - .RequireAuthenticatedUser() - .Build(); - - options.AddPolicy(AuthPolicyName.RequireChatOwner, builder => - { - builder.RequireAuthenticatedUser() - .AddRequirements(new ChatOwnerRequirement()); - }); - }); - } - /// /// Trim all string properties, recursively. /// From 7014e91e7add68625ea889777c834a66f3e649b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Parks=20=C2=A9=EF=B8=8F?= Date: Sat, 20 May 2023 10:30:55 -0700 Subject: [PATCH 09/15] only add userId and user name to context if they are detected --- .../webapi/Utilities/AskConverter.cs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/samples/apps/copilot-chat-app/webapi/Utilities/AskConverter.cs b/samples/apps/copilot-chat-app/webapi/Utilities/AskConverter.cs index 851672ca50e7..cf49a2c78b1b 100644 --- a/samples/apps/copilot-chat-app/webapi/Utilities/AskConverter.cs +++ b/samples/apps/copilot-chat-app/webapi/Utilities/AskConverter.cs @@ -25,14 +25,20 @@ public ContextVariables GetContextVariables(Ask ask) var contextVariables = new ContextVariables(ask.Input); foreach (var input in ask.Variables) { - if (input.Key != userIdKey && input.Key != userNameKey) + if (input.Key == userIdKey) + { + contextVariables.Set(userIdKey, this._authInfo.UserId); + } + else if (input.Key == userNameKey) + { + contextVariables.Set(userNameKey, this._authInfo.Name); + } + else { contextVariables.Set(input.Key, input.Value); } } - contextVariables.Set(userIdKey, this._authInfo.UserId); - contextVariables.Set(userNameKey, this._authInfo.Name); return contextVariables; } } From f14886deb60175c7dcac17537913001ec7785ca0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Parks=20=C2=A9=EF=B8=8F?= Date: Sat, 20 May 2023 13:37:18 -0700 Subject: [PATCH 10/15] revert last commit --- .../webapi/Utilities/AskConverter.cs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/samples/apps/copilot-chat-app/webapi/Utilities/AskConverter.cs b/samples/apps/copilot-chat-app/webapi/Utilities/AskConverter.cs index cf49a2c78b1b..851672ca50e7 100644 --- a/samples/apps/copilot-chat-app/webapi/Utilities/AskConverter.cs +++ b/samples/apps/copilot-chat-app/webapi/Utilities/AskConverter.cs @@ -25,20 +25,14 @@ public ContextVariables GetContextVariables(Ask ask) var contextVariables = new ContextVariables(ask.Input); foreach (var input in ask.Variables) { - if (input.Key == userIdKey) - { - contextVariables.Set(userIdKey, this._authInfo.UserId); - } - else if (input.Key == userNameKey) - { - contextVariables.Set(userNameKey, this._authInfo.Name); - } - else + if (input.Key != userIdKey && input.Key != userNameKey) { contextVariables.Set(input.Key, input.Value); } } + contextVariables.Set(userIdKey, this._authInfo.UserId); + contextVariables.Set(userNameKey, this._authInfo.Name); return contextVariables; } } From 93b88d885e104a38921fa6c2a2917da97c2225a8 Mon Sep 17 00:00:00 2001 From: David Parks Date: Mon, 22 May 2023 12:49:47 -0700 Subject: [PATCH 11/15] Update DocumentImportController.cs --- .../CopilotChat/Controllers/DocumentImportController.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/DocumentImportController.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/DocumentImportController.cs index 02f5ea9a65ca..93b8125aa351 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/DocumentImportController.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/DocumentImportController.cs @@ -62,7 +62,7 @@ public DocumentImportController( /// [Route("importDocument")] [HttpPost] - [ProducesResponseType(StatusCodes.Status202Accepted)] + [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(StatusCodes.Status400BadRequest)] public async Task ImportDocumentAsync( [FromServices] IKernel kernel, @@ -124,7 +124,7 @@ public async Task ImportDocumentAsync( return this.BadRequest(ex.Message); } - return this.Accepted(); + return this.Ok(); } /// From 33bb80aab199c4e0fb2e41abf4fc1bf7b8a7d08a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Parks=20=C2=A9=EF=B8=8F?= Date: Mon, 22 May 2023 13:00:06 -0700 Subject: [PATCH 12/15] fix build after merge --- samples/apps/copilot-chat-app/webapi/Auth/AuthInfo.cs | 6 +++--- .../webapi/CopilotChat/Controllers/BotController.cs | 1 - .../webapi/CopilotChat/Controllers/ChatController.cs | 3 +-- .../webapi/CopilotChat/Controllers/ChatHistoryController.cs | 2 +- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/samples/apps/copilot-chat-app/webapi/Auth/AuthInfo.cs b/samples/apps/copilot-chat-app/webapi/Auth/AuthInfo.cs index 58d46c0f00c4..ef4323638b12 100644 --- a/samples/apps/copilot-chat-app/webapi/Auth/AuthInfo.cs +++ b/samples/apps/copilot-chat-app/webapi/Auth/AuthInfo.cs @@ -20,7 +20,7 @@ private record struct AuthData( public AuthInfo(IHttpContextAccessor httpContextAccessor) { - _data = new Lazy(() => + this._data = new Lazy(() => { var user = httpContextAccessor.HttpContext?.User; if (user is null) @@ -52,10 +52,10 @@ public AuthInfo(IHttpContextAccessor httpContextAccessor) /// /// The authenticated user's unique ID. /// - public string UserId => _data.Value.UserId; + public string UserId => this._data.Value.UserId; /// /// The authenticated user's name. /// - public string Name => _data.Value.UserName; + public string Name => this._data.Value.UserName; } diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs index 4f1cc5f23ef4..bd9c8fb98765 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/BotController.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Text.Json; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Authorization; diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatController.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatController.cs index 0dbc38a366a2..5c67cc754b00 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatController.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatController.cs @@ -65,8 +65,7 @@ public ChatController(ILogger logger) [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task ChatAsync( [FromServices] IKernel kernel, - [FromServices] CopilotChatPlanner? planner, - [FromServices] IOptions plannerOptions, + [FromServices] CopilotChatPlanner planner, [FromServices] AskConverter askConverter, [FromServices] ChatSessionRepository chatSessionRepository, [FromServices] IAuthInfo authInfo, diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatHistoryController.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatHistoryController.cs index 5510dba66831..078022c4ffbb 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatHistoryController.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Controllers/ChatHistoryController.cs @@ -66,7 +66,7 @@ public ChatHistoryController( public async Task CreateChatSessionAsync( [FromBody] ChatSessionCreationOptions chatParameters) { - var userId = _authInfo.UserId; + var userId = this._authInfo.UserId; var title = chatParameters.Title; var newChat = new ChatSession(userId, title); From eae5abee2baf31430b15a0a7f8d799d4f9758678 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Parks=20=C2=A9=EF=B8=8F?= Date: Mon, 22 May 2023 16:20:41 -0700 Subject: [PATCH 13/15] update deployment templates --- .../DeploySK-Existing-AzureOpenAI.ps1 | 5 -- .../DeploySK-Existing-AzureOpenAI.sh | 8 --- .../DeploySK-Existing-OpenAI.ps1 | 5 -- .../DeploySK-Existing-OpenAI.sh | 8 --- .../webapi/DeploymentTemplates/DeploySK.ps1 | 5 -- .../webapi/DeploymentTemplates/DeploySK.sh | 8 --- .../webapi/DeploymentTemplates/README.md | 56 ++++++++++++------- .../webapi/DeploymentTemplates/main.bicep | 11 +--- .../sk-existing-azureopenai.bicep | 4 -- .../sk-existing-azureopenai.json | 25 +-------- .../sk-existing-openai.bicep | 4 -- .../sk-existing-openai.json | 25 +-------- .../webapi/DeploymentTemplates/sk-new.bicep | 4 -- .../webapi/DeploymentTemplates/sk-new.json | 25 +-------- 14 files changed, 44 insertions(+), 149 deletions(-) diff --git a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK-Existing-AzureOpenAI.ps1 b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK-Existing-AzureOpenAI.ps1 index d848110a619a..cc21f471b246 100644 --- a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK-Existing-AzureOpenAI.ps1 +++ b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK-Existing-AzureOpenAI.ps1 @@ -52,10 +52,6 @@ param( # SKU for the Azure App Service plan $AppServiceSku = "B1", - [string] - # API key to access Semantic Kernel server's endpoints - $SemanticKernelApiKey = "$([guid]::NewGuid())", - # TODO: Temporarily disabling qdrant deployment while we secure its endpoint. # [switch] # # Don't deploy Qdrant for memory storage - Use volatile memory instead @@ -86,7 +82,6 @@ $jsonConfig = " `\`"plannerModel`\`": { `\`"value`\`": `\`"$PlannerModel`\`" }, `\`"packageUri`\`": { `\`"value`\`": `\`"$PackageUri`\`" }, `\`"appServiceSku`\`": { `\`"value`\`": `\`"$AppServiceSku`\`" }, - `\`"semanticKernelApiKey`\`": { `\`"value`\`": `\`"$SemanticKernelApiKey`\`" }, `\`"deployQdrant`\`": { `\`"value`\`": $(If (!($NoQdrant)) {"true"} Else {"false"}) }, `\`"deployCosmosDB`\`": { `\`"value`\`": $(If (!($NoSpeechServices)) {"true"} Else {"false"}) }, `\`"deploySpeechServices`\`": { `\`"value`\`": $(If (!($NoSpeechServices)) {"true"} Else {"false"}) } diff --git a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK-Existing-AzureOpenAI.sh b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK-Existing-AzureOpenAI.sh index 3270c7e9bb25..9dd44ebe75a3 100644 --- a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK-Existing-AzureOpenAI.sh +++ b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK-Existing-AzureOpenAI.sh @@ -16,7 +16,6 @@ usage() { echo " -r, --region REGION Region to which to make the deployment (default: \"South Central US\")" echo " -p, --package-uri PACKAGE_URI Package to deploy to web service (default: 'https://skaasdeploy.blob.core.windows.net/api/semantickernelapi.zip')" echo " -a, --app-service-sku APP_SERVICE_SKU SKU for the Azure App Service plan (default: \"B1\")" - echo " -k, --semker-server-api-key SEMKER_SERVER_API_KEY API key to access Semantic Kernel server's endpoints (default: random UUID)" echo " -cm, --completion-model COMPLETION_MODEL Completion model to use (default: \"gpt-35-turbo\")" echo " -em, --embedding-model EMBEDDING_MODEL Embedding model to use (default: \"text-embedding-ada-002\")" echo " -pm, --planner-model PLANNER_MODEL Planner model to use (default: \"gpt-35-turbo\")" @@ -71,11 +70,6 @@ while [[ $# -gt 0 ]]; do shift shift ;; - -k|--semker-server-api-key) - SEMKER_SERVER_API_KEY="$2" - shift - shift - ;; -cm|--completion-model) COMPLETION_MODEL="$2" shift @@ -135,7 +129,6 @@ az account set -s "$SUBSCRIPTION" : "${REGION:="South Central US"}" : "${PACKAGE_URI:="https://skaasdeploy.blob.core.windows.net/api/semantickernelapi.zip"}" : "${APP_SERVICE_SKU:="B1"}" -: "${SEMKER_SERVER_API_KEY:="$(uuidgen)"}" : "${NO_QDRANT:=true}" # TODO: Temporarily disabling qdrant deployment while we secure its endpoint. : "${NO_COSMOS_DB:=false}" : "${NO_SPEECH_SERVICES:=false}" @@ -154,7 +147,6 @@ JSON_CONFIG=$(cat << EOF "plannerModel": { "value": "$PLANNER_MODEL" }, "packageUri": { "value": "$PACKAGE_URI" }, "appServiceSku": { "value": "$APP_SERVICE_SKU" }, - "semanticKernelApiKey": { "value": "$SEMKER_SERVER_API_KEY" }, "deployQdrant": { "value": $([ "$NO_QDRANT" = true ] && echo "false" || echo "true") }, "deployCosmosDB": { "value": $([ "$NO_COSMOS_DB" = true ] && echo "false" || echo "true") }, "deploySpeechServices": { "value": $([ "$NO_SPEECH_SERVICES" = true ] && echo "false" || echo "true") } diff --git a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK-Existing-OpenAI.ps1 b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK-Existing-OpenAI.ps1 index 1a04f2ac2cf8..228fb5d93ce1 100644 --- a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK-Existing-OpenAI.ps1 +++ b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK-Existing-OpenAI.ps1 @@ -47,10 +47,6 @@ param( # SKU for the Azure App Service plan $AppServiceSku = "B1", - [string] - # API key to access Semantic Kernel server's endpoints - $SemanticKernelApiKey = "$([guid]::NewGuid())", - # TODO: Temporarily disabling qdrant deployment while we secure its endpoint. # [switch] # # Don't deploy Qdrant for memory storage - Use volatile memory instead @@ -80,7 +76,6 @@ $jsonConfig = " `\`"plannerModel`\`": { `\`"value`\`": `\`"$PlannerModel`\`" }, `\`"packageUri`\`": { `\`"value`\`": `\`"$PackageUri`\`" }, `\`"appServiceSku`\`": { `\`"value`\`": `\`"$AppServiceSku`\`" }, - `\`"semanticKernelApiKey`\`": { `\`"value`\`": `\`"$SemanticKernelApiKey`\`" }, `\`"deployQdrant`\`": { `\`"value`\`": $(If (!($NoQdrant)) {"true"} Else {"false"}) }, `\`"deployCosmosDB`\`": { `\`"value`\`": $(If (!($NoSpeechServices)) {"true"} Else {"false"}) }, `\`"deploySpeechServices`\`": { `\`"value`\`": $(If (!($NoSpeechServices)) {"true"} Else {"false"}) } diff --git a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK-Existing-OpenAI.sh b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK-Existing-OpenAI.sh index db441b596c52..7f9166941ea5 100644 --- a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK-Existing-OpenAI.sh +++ b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK-Existing-OpenAI.sh @@ -15,7 +15,6 @@ usage() { echo " -r, --region REGION Region to which to make the deployment (default: \"South Central US\")" echo " -p, --package-uri PACKAGE_URI Package to deploy to web service (default: 'https://skaasdeploy.blob.core.windows.net/api/semantickernelapi.zip')" echo " -a, --app-service-sku APP_SERVICE_SKU SKU for the Azure App Service plan (default: \"B1\")" - echo " -k, --semker-server-api-key SEMKER_SERVER_API_KEY API key to access Semantic Kernel server's endpoints (default: random UUID)" echo " -cm, --completion-model COMPLETION_MODEL Completion model to use (default: \"gpt-3.5-turbo\")" echo " -em, --embedding-model EMBEDDING_MODEL Embedding model to use (default: \"text-embedding-ada-002\")" echo " -pm, --planner-model PLANNER_MODEL Planner model to use (default: \"gpt-3.5-turbo\")" @@ -65,11 +64,6 @@ while [[ $# -gt 0 ]]; do shift shift ;; - -k|--semker-server-api-key) - SEMKER_SERVER_API_KEY="$2" - shift - shift - ;; -cm|--completion-model) COMPLETION_MODEL="$2" shift @@ -129,7 +123,6 @@ az account set -s "$SUBSCRIPTION" : "${REGION:="South Central US"}" : "${PACKAGE_URI:="https://skaasdeploy.blob.core.windows.net/api/semantickernelapi.zip"}" : "${APP_SERVICE_SKU:="B1"}" -: "${SEMKER_SERVER_API_KEY:="$(uuidgen)"}" : "${NO_QDRANT:=true}" # TODO: Temporarily disabling qdrant deployment while we secure its endpoint. : "${NO_COSMOS_DB:=false}" : "${NO_SPEECH_SERVICES:=false}" @@ -147,7 +140,6 @@ JSON_CONFIG=$(cat << EOF "plannerModel": { "value": "$PLANNER_MODEL" }, "packageUri": { "value": "$PACKAGE_URI" }, "appServiceSku": { "value": "$APP_SERVICE_SKU" }, - "semanticKernelApiKey": { "value": "$SEMKER_SERVER_API_KEY" }, "deployQdrant": { "value": $([ "$NO_QDRANT" = true ] && echo "false" || echo "true") }, "deployCosmosDB": { "value": $([ "$NO_COSMOS_DB" = true ] && echo "false" || echo "true") }, "deploySpeechServices": { "value": $([ "$NO_SPEECH_SERVICES" = true ] && echo "false" || echo "true") } diff --git a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK.ps1 b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK.ps1 index 1612d2fa6561..748026a9b9df 100644 --- a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK.ps1 +++ b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK.ps1 @@ -30,10 +30,6 @@ param( # SKU for the Azure App Service plan $AppServiceSku = "B1", - [string] - # API key to access Semantic Kernel server's endpoints - $SemanticKernelApiKey = "$([guid]::NewGuid())", - # TODO: Temporarily disabling qdrant deployment while we secure its endpoint. # [switch] # # Don't deploy Qdrant for memory storage - Use volatile memory instead @@ -59,7 +55,6 @@ $jsonConfig = " `\`"name`\`": { `\`"value`\`": `\`"$DeploymentName`\`" }, `\`"packageUri`\`": { `\`"value`\`": `\`"$PackageUri`\`" }, `\`"appServiceSku`\`": { `\`"value`\`": `\`"$AppServiceSku`\`" }, - `\`"semanticKernelApiKey`\`": { `\`"value`\`": `\`"$SemanticKernelApiKey`\`" }, `\`"deployQdrant`\`": { `\`"value`\`": $(If (!($NoQdrant)) {"true"} Else {"false"}) }, `\`"deployCosmosDB`\`": { `\`"value`\`": $(If (!($NoSpeechServices)) {"true"} Else {"false"}) }, `\`"deploySpeechServices`\`": { `\`"value`\`": $(If (!($NoSpeechServices)) {"true"} Else {"false"}) } diff --git a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK.sh b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK.sh index 6b7de71bd8fa..863cf3bd7c0b 100644 --- a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK.sh +++ b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/DeploySK.sh @@ -14,7 +14,6 @@ usage() { echo " -r, --region REGION Region to which to make the deployment (default: \"South Central US\")" echo " -p, --package-uri PACKAGE_URI Package to deploy to web service (default: 'https://skaasdeploy.blob.core.windows.net/api/semantickernelapi.zip')" echo " -a, --app-service-sku APP_SERVICE_SKU SKU for the Azure App Service plan (default: \"B1\")" - echo " -k, --semker-server-api-key SEMKER_SERVER_API_KEY API key to access Semantic Kernel server's endpoints (default: random UUID)" # TODO: Temporarily disabling qdrant deployment while we secure its endpoint. # echo " -nq, --no-qdrant Don't deploy Qdrant for memory storage - Use volatile memory instead" echo " -nc, --no-cosmos-db Don't deploy Cosmos DB for chat storage - Use volatile memory instead" @@ -56,11 +55,6 @@ while [[ $# -gt 0 ]]; do shift shift ;; - -k|--semker-server-api-key) - SEMKER_SERVER_API_KEY="$2" - shift - shift - ;; # TODO: Temporarily disabling qdrant deployment while we secure its endpoint. # -nq|--no-qdrant) # NO_QDRANT=true @@ -105,7 +99,6 @@ az account set -s "$SUBSCRIPTION" : "${REGION:="South Central US"}" : "${PACKAGE_URI:="https://skaasdeploy.blob.core.windows.net/api/semantickernelapi.zip"}" : "${APP_SERVICE_SKU:="B1"}" -: "${SEMKER_SERVER_API_KEY:="$(uuidgen)"}" : "${NO_QDRANT:=true}" # TODO: Temporarily disabling qdrant deployment while we secure its endpoint. : "${NO_COSMOS_DB:=false}" : "${NO_SPEECH_SERVICES:=false}" @@ -116,7 +109,6 @@ JSON_CONFIG=$(cat << EOF "name": { "value": "$DEPLOYMENT_NAME" }, "packageUri": { "value": "$PACKAGE_URI" }, "appServiceSku": { "value": "$APP_SERVICE_SKU" }, - "semanticKernelApiKey": { "value": "$SEMKER_SERVER_API_KEY" }, "deployQdrant": { "value": $([ "$NO_QDRANT" = true ] && echo "false" || echo "true") }, "deployCosmosDB": { "value": $([ "$NO_COSMOS_DB" = true ] && echo "false" || echo "true") }, "deploySpeechServices": { "value": $([ "$NO_SPEECH_SERVICES" = true ] && echo "false" || echo "true") } diff --git a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/README.md b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/README.md index 5282d3883d4f..093b81592762 100644 --- a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/README.md +++ b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/README.md @@ -1,9 +1,10 @@ # Deploying Semantic Kernel to Azure in a web app service ## Things to know -- Access to Azure OpenAI is currently limited as we navigate high demand, upcoming product improvements, and Microsoft’s commitment to responsible AI. - For more details and information on applying for access, go [here](https://learn.microsoft.com/en-us/azure/cognitive-services/openai/overview?ocid=AID3051475#how-do-i-get-access-to-azure-openai). - For region availability of Azure OpenAI, see the [availability map](https://azure.microsoft.com/en-us/explore/global-infrastructure/products-by-region/?products=cognitive-services). + +- Access to Azure OpenAI is currently limited as we navigate high demand, upcoming product improvements, and Microsoft’s commitment to responsible AI. + For more details and information on applying for access, go [here](https://learn.microsoft.com/azure/cognitive-services/openai/overview?ocid=AID3051475#how-do-i-get-access-to-azure-openai). + For region availability of Azure OpenAI, see the [availability map](https://azure.microsoft.com/explore/global-infrastructure/products-by-region/?products=cognitive-services). - Due to the limited availability of Azure OpenAI, consider using the same Azure OpenAI instance for multiple deployments of the Semantic Kernel web api and CopilotChat: - [Deploying with an existing Azure OpenAI account](#deploying-with-an-existing-azure-openai-account) @@ -11,53 +12,66 @@ - F1 and D1 SKUs for the App Service Plans are not currently supported for this deployment. - # Deploying with a new Azure OpenAI instance + You can deploy an instance of Semantic Kernel in a web app service within a resource group that bears the name YOUR_DEPLOYMENT_NAME preceded by the "rg-" prefix using any of the following methods. ## PowerShell + Use the [DeploySK.ps1](DeploySK.ps1) file found in this folder: + ```powershell .\DeploySK.ps1 -DeploymentName YOUR_DEPLOYMENT_NAME -Subscription YOUR_SUBSCRIPTION_ID ``` + For additional deployment options, see the deployment script. ## Bash + Use the [DeploySK.sh](DeploySK.sh) file found in this folder: + ```bash chmod +x ./DeploySK.sh ./DeploySK.sh -d DEPLOYMENT_NAME -s SUBSCRIPTION_ID ``` ## Azure Portal + You can also deploy by clicking on: [![Deploy to Azure](https://aka.ms/deploytoazurebutton)](https://portal.azure.com/#create/Microsoft.Template/uri/https%3A%2F%2Fraw.githubusercontent.com%2Fmicrosoft%2Fsemantic-kernel%2Fmain%2Fsamples%2Fapps%2Fcopilot-chat-app%2Fwebapi%2FDeploymentTemplates%2Fsk-new.json) - # Deploying with an existing Azure OpenAI account + ## PowerShell + Use the [DeploySK-Existing-AzureOpenAI.ps1](DeploySK-Existing-AzureOpenAI.ps1) file found in this folder: + ```powershell .\DeploySK-Existing-AzureOpenAI.ps1 -DeploymentName YOUR_DEPLOYMENT_NAME -Subscription YOUR_SUBSCRIPTION_ID -Endpoint YOUR_AZURE_OPENAI_ENDPOINT -ApiKey YOUR_AZURE_OPENAI_API_KEY ``` ## Bash + Use the [DeploySK-Existing-AzureOpenAI.sh](DeploySK-Existing-AzureOpenAI.sh) file found in this folder: + ```bash chmod +x ./DeploySK-Existing-AzureOpenAI.sh ./DeploySK-Existing-AzureOpenAI.sh -d YOUR_DEPLOYMENT_NAME -s YOUR_SUBSCRIPTION_ID -e YOUR_AZURE_OPENAI_ENDPOINT -o YOUR_AZURE_OPENAI_API_KEY ``` ## Azure Portal + You can also deploy by clicking on: [![Deploy to Azure](https://aka.ms/deploytoazurebutton)](https://portal.azure.com/#create/Microsoft.Template/uri/https%3A%2F%2Fraw.githubusercontent.com%2Fmicrosoft%2Fsemantic-kernel%2Fmain%2Fsamples%2Fapps%2Fcopilot-chat-app%2Fwebapi%2FDeploymentTemplates%2Fsk-existing-azureopenai.json) - # Deploying with an existing OpenAI account + ## PowerShell + Use the [DeploySK-Existing-OpenAI.ps1](DeploySK-Existing-OpenAI.ps1) file found in this folder: + ```powershell .\DeploySK-Existing-OpenAI.ps1 -DeploymentName YOUR_DEPLOYMENT_NAME -Subscription YOUR_SUBSCRIPTION_ID ``` @@ -65,6 +79,7 @@ Use the [DeploySK-Existing-OpenAI.ps1](DeploySK-Existing-OpenAI.ps1) file found After entering the command above, you will be prompted to enter your OpenAI API key. (You can also pass in the API key using the -ApiKey parameter) ## Bash + After ensuring DeploySK-Existing-OpenAI.sh file found in this folder is executable, enter the following command: ```bash @@ -72,20 +87,21 @@ After ensuring DeploySK-Existing-OpenAI.sh file found in this folder is executab ``` ## Azure Portal + You can also deploy by clicking on: [![Deploy to Azure](https://aka.ms/deploytoazurebutton)](https://portal.azure.com/#create/Microsoft.Template/uri/https%3A%2F%2Fraw.githubusercontent.com%2Fmicrosoft%2Fsemantic-kernel%2Fmain%2Fsamples%2Fapps%2Fcopilot-chat-app%2Fwebapi%2FDeploymentTemplates%2Fsk-existing-openai.json) - # Verifying the deployment -To make sure your web app service is running, go to https://YOUR_INSTANCE_NAME.azurewebsites.net/probe + +To make sure your web app service is running, go to To get your instance's URL, click on the "Go to resource group" button you see at the end of your deployment. Then click on the resource whose name starts with "app-". This will bring you to the Overview page on your web service. Your instance's URL is the value that appears next to the "Default domain" field. - # Changing your configuration, monitoring your deployment and troubleshooting + From the page just mentioned in the section above, you can change your configuration by clicking on the "Configuration" item in the "Settings" section of the left pane. Scrolling down in that same pane to the "Monitoring" section gives you access to a multitude of ways to monitor your deployment. @@ -94,34 +110,33 @@ In addition to this, the "Diagnose and "solve problems" item near the top of the If the service itself if functioning properly but you keep getting errors (perhaps reported as 400 HTTP errors) when making calls to the Semantic Kernel, check that you have correctly entered the values for the following settings: + - AIService:AzureOpenAI - AIService:Endpoint Both Completion:Endpoint and Embedding:Endpoint are ignored for OpenAI instances from [openai.com](https://openai.com) but MUST be properly populated when using Azure OpenAI instances. # Authorization -All of the server's endpoints other than the /probe one require authorization to access. -By default, the deployment templates set up the server so that an API key is required to access its endpoints. -AAD authentication and authorization can also be set up manually after the automated deployment is done. +All of the server's endpoints other than the /healthz one require authorization to access when using Azure AD. +By default, the deployment templates set up the server so that no authentication is required to access its endpoints. -To view the API key required by your instance, access the page for your Semantic Kernel app service in the Azure portal. -From that page, click on the "Configuration" item in the "Settings" section of the left pane. Then click on the text that reads "Hidden value. -Click to show value" next to the "Authorization:ApiKey" setting. - -To authorize requests with the API key, it must be added as the value of an "x-sk-api-key" header added to the requests. +AAD authentication and authorization should be set up manually after the automated deployment is done. # Using web frontends to access your deployment + Make sure to include your frontend's URL as an allowed origin in your deployment's CORS settings. Otherwise, web browsers will refuse to let JavaScript make calls to your deployment. To do this, go on the Azure portal, select your Semantic Kernel App Service, then click on "CORS" under the "API" section of the resource menu on the left of the page. This will get you to the CORS page where you can add your allowed hosts. # Deploying your custom version of Semantic Kernel + You can build and upload a customized version of the Semantic Kernel service. To do so, clone the code from this repo then modify it to your needs (for example, by adding your own skills). Once that is done, go into the ../semantic-kernel/samples/apps/copilot-chat-app/webapi directory and enter the following command: + ```powershell dotnet publish CopilotChatWebApi.csproj --configuration Release --arch x64 --os win ``` @@ -135,10 +150,11 @@ Put its URI in the "Package Uri" field in the web deployment page you access thr Your deployment will then use your customized deployment package. - ## Cleaning up + Once you are done with your resources, you can delete them from the Azure portal. You can also simply delete the resource group in which they are from the portal or through the -following [Azure CLI](https://learn.microsoft.com/en-us/cli/azure/) command: +following [Azure CLI](https://learn.microsoft.com/cli/azure/) command: + ```powershell az group delete --name YOUR_RESOURCE_GROUP -``` \ No newline at end of file +``` diff --git a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/main.bicep b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/main.bicep index cf0fec85cea9..d0b33d9d48ab 100644 --- a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/main.bicep +++ b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/main.bicep @@ -42,9 +42,6 @@ param endpoint string = '' @description('Azure OpenAI or OpenAI API key') param apiKey string = '' -@description('Semantic Kernel server API key - Generated GUID by default\nProvide empty string to disable API key auth') -param semanticKernelApiKey string = newGuid() - @description('Whether to deploy a new Azure OpenAI instance') param deployNewAzureOpenAI bool = true @@ -182,12 +179,8 @@ resource appServiceWebConfig 'Microsoft.Web/sites/config@2022-09-01' = { value: plannerModel } { - name: 'Authorization:Type' - value: empty(semanticKernelApiKey) ? 'None' : 'ApiKey' - } - { - name: 'Authorization:ApiKey' - value: semanticKernelApiKey + name: 'Authentication:Type' + value: 'None' } { name: 'ChatStore:Type' diff --git a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-existing-azureopenai.bicep b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-existing-azureopenai.bicep index e7f2484daeb6..5ea474ce2823 100644 --- a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-existing-azureopenai.bicep +++ b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-existing-azureopenai.bicep @@ -32,9 +32,6 @@ param endpoint string @description('Azure OpenAI API key') param apiKey string -@description('Semantic Kernel server API key - Generated GUID by default\nProvide empty string to disable API key auth') -param semanticKernelApiKey string = newGuid() - @description('Whether to deploy Cosmos DB for chat storage') param deployCosmosDB bool = true @@ -57,7 +54,6 @@ module openAI 'main.bicep' = { plannerModel: plannerModel endpoint: endpoint apiKey: apiKey - semanticKernelApiKey: semanticKernelApiKey deployCosmosDB: deployCosmosDB deployQdrant: deployQdrant deploySpeechServices: deploySpeechServices diff --git a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-existing-azureopenai.json b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-existing-azureopenai.json index 2faf33e61830..689f25feaaa0 100644 --- a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-existing-azureopenai.json +++ b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-existing-azureopenai.json @@ -73,13 +73,6 @@ "description": "Azure OpenAI API key" } }, - "semanticKernelApiKey": { - "type": "string", - "defaultValue": "[newGuid()]", - "metadata": { - "description": "Semantic Kernel server API key - Generated GUID by default\nProvide empty string to disable API key auth" - } - }, "deployCosmosDB": { "type": "bool", "defaultValue": true, @@ -140,9 +133,6 @@ "apiKey": { "value": "[parameters('apiKey')]" }, - "semanticKernelApiKey": { - "value": "[parameters('semanticKernelApiKey')]" - }, "deployCosmosDB": { "value": "[parameters('deployCosmosDB')]" }, @@ -244,13 +234,6 @@ "description": "Azure OpenAI or OpenAI API key" } }, - "semanticKernelApiKey": { - "type": "string", - "defaultValue": "[newGuid()]", - "metadata": { - "description": "Semantic Kernel server API key - Generated GUID by default\nProvide empty string to disable API key auth" - } - }, "deployNewAzureOpenAI": { "type": "bool", "defaultValue": true, @@ -425,12 +408,8 @@ "value": "[parameters('plannerModel')]" }, { - "name": "Authorization:Type", - "value": "[if(empty(parameters('semanticKernelApiKey')), 'None', 'ApiKey')]" - }, - { - "name": "Authorization:ApiKey", - "value": "[parameters('semanticKernelApiKey')]" + "name": "Authentication:Type", + "value": "None" }, { "name": "ChatStore:Type", diff --git a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-existing-openai.bicep b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-existing-openai.bicep index 1105fbfbc5c9..c2fc2475af08 100644 --- a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-existing-openai.bicep +++ b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-existing-openai.bicep @@ -29,9 +29,6 @@ param plannerModel string = 'gpt-3.5-turbo' @description('OpenAI API key') param apiKey string = '' -@description('Semantic Kernel server API key - Generated GUID by default\nProvide empty string to disable API key auth') -param semanticKernelApiKey string = newGuid() - @description('Whether to deploy Cosmos DB for chat storage') param deployCosmosDB bool = true @@ -54,7 +51,6 @@ module openAI 'main.bicep' = { plannerModel: plannerModel endpoint: 'not-used' apiKey: apiKey - semanticKernelApiKey: semanticKernelApiKey deployCosmosDB: deployCosmosDB deployQdrant: deployQdrant deploySpeechServices: deploySpeechServices diff --git a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-existing-openai.json b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-existing-openai.json index e06eb8763a9c..c75c93e09f44 100644 --- a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-existing-openai.json +++ b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-existing-openai.json @@ -68,13 +68,6 @@ "description": "OpenAI API key" } }, - "semanticKernelApiKey": { - "type": "string", - "defaultValue": "[newGuid()]", - "metadata": { - "description": "Semantic Kernel server API key - Generated GUID by default\nProvide empty string to disable API key auth" - } - }, "deployCosmosDB": { "type": "bool", "defaultValue": true, @@ -135,9 +128,6 @@ "apiKey": { "value": "[parameters('apiKey')]" }, - "semanticKernelApiKey": { - "value": "[parameters('semanticKernelApiKey')]" - }, "deployCosmosDB": { "value": "[parameters('deployCosmosDB')]" }, @@ -239,13 +229,6 @@ "description": "Azure OpenAI or OpenAI API key" } }, - "semanticKernelApiKey": { - "type": "string", - "defaultValue": "[newGuid()]", - "metadata": { - "description": "Semantic Kernel server API key - Generated GUID by default\nProvide empty string to disable API key auth" - } - }, "deployNewAzureOpenAI": { "type": "bool", "defaultValue": true, @@ -420,12 +403,8 @@ "value": "[parameters('plannerModel')]" }, { - "name": "Authorization:Type", - "value": "[if(empty(parameters('semanticKernelApiKey')), 'None', 'ApiKey')]" - }, - { - "name": "Authorization:ApiKey", - "value": "[parameters('semanticKernelApiKey')]" + "name": "Authentication:Type", + "value": "None" }, { "name": "ChatStore:Type", diff --git a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-new.bicep b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-new.bicep index 7a061bdcae00..febc546a9ce9 100644 --- a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-new.bicep +++ b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-new.bicep @@ -25,9 +25,6 @@ param embeddingModel string = 'text-embedding-ada-002' @description('Completion model the task planner should use') param plannerModel string = 'gpt-35-turbo' -@description('Semantic Kernel server API key - Generated GUID by default\nProvide empty string to disable API key auth') -param semanticKernelApiKey string = newGuid() - @description('Whether to deploy Cosmos DB for chat storage') param deployCosmosDB bool = true @@ -49,7 +46,6 @@ module openAI 'main.bicep' = { completionModel: completionModel embeddingModel: embeddingModel plannerModel: plannerModel - semanticKernelApiKey: semanticKernelApiKey deployCosmosDB: deployCosmosDB deployQdrant: deployQdrant deploySpeechServices: deploySpeechServices diff --git a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-new.json b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-new.json index 8aa3aadfaddc..589700761c1b 100644 --- a/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-new.json +++ b/samples/apps/copilot-chat-app/webapi/DeploymentTemplates/sk-new.json @@ -61,13 +61,6 @@ "description": "Completion model the task planner should use" } }, - "semanticKernelApiKey": { - "type": "string", - "defaultValue": "[newGuid()]", - "metadata": { - "description": "Semantic Kernel server API key - Generated GUID by default\nProvide empty string to disable API key auth" - } - }, "deployCosmosDB": { "type": "bool", "defaultValue": true, @@ -122,9 +115,6 @@ "plannerModel": { "value": "[parameters('plannerModel')]" }, - "semanticKernelApiKey": { - "value": "[parameters('semanticKernelApiKey')]" - }, "deployCosmosDB": { "value": "[parameters('deployCosmosDB')]" }, @@ -226,13 +216,6 @@ "description": "Azure OpenAI or OpenAI API key" } }, - "semanticKernelApiKey": { - "type": "string", - "defaultValue": "[newGuid()]", - "metadata": { - "description": "Semantic Kernel server API key - Generated GUID by default\nProvide empty string to disable API key auth" - } - }, "deployNewAzureOpenAI": { "type": "bool", "defaultValue": true, @@ -407,12 +390,8 @@ "value": "[parameters('plannerModel')]" }, { - "name": "Authorization:Type", - "value": "[if(empty(parameters('semanticKernelApiKey')), 'None', 'ApiKey')]" - }, - { - "name": "Authorization:ApiKey", - "value": "[parameters('semanticKernelApiKey')]" + "name": "AuthenticationType", + "value": "None" }, { "name": "ChatStore:Type", From 5ca3f27d4c00d3f443f0f10b4c28048848b6b6e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Parks=20=C2=A9=EF=B8=8F?= Date: Tue, 23 May 2023 19:20:14 -0700 Subject: [PATCH 14/15] attempt to address comments --- .../Extensions/ServiceExtensions.cs | 39 ++++++++++++++++++ .../apps/copilot-chat-app/webapi/Program.cs | 6 +-- .../webapi/ServiceExtensions.cs | 41 +------------------ .../webapi/Utilities/AskConverter.cs | 3 ++ 4 files changed, 46 insertions(+), 43 deletions(-) diff --git a/samples/apps/copilot-chat-app/webapi/CopilotChat/Extensions/ServiceExtensions.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Extensions/ServiceExtensions.cs index 2e64b1fa09b9..3e6a983410ff 100644 --- a/samples/apps/copilot-chat-app/webapi/CopilotChat/Extensions/ServiceExtensions.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Extensions/ServiceExtensions.cs @@ -4,10 +4,14 @@ using System.Collections.Generic; using System.IO; using System.Reflection; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Authentication.JwtBearer; using Microsoft.AspNetCore.Authorization; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; +using Microsoft.Identity.Web; +using SemanticKernel.Service.Auth; using SemanticKernel.Service.CopilotChat.Auth; using SemanticKernel.Service.CopilotChat.Models; using SemanticKernel.Service.CopilotChat.Options; @@ -34,6 +38,13 @@ public static IServiceCollection AddCopilotChatOptions(this IServiceCollection s .ValidateOnStart() .PostConfigure(TrimStringProperties); + // Authorization configuration + services.AddOptions() + .Bind(configuration.GetSection(ChatAuthenticationOptions.PropertyName)) + .ValidateOnStart() + .ValidateDataAnnotations() + .PostConfigure(TrimStringProperties); + // Chat log storage configuration services.AddOptions() .Bind(configuration.GetSection(ChatStoreOptions.PropertyName)) @@ -149,6 +160,34 @@ public static IServiceCollection AddPersistentChatStore(this IServiceCollection return services.AddSingleton(new ChatMessageRepository(chatMessageInMemoryContext)); } + /// + /// Add authentication services + /// + public static IServiceCollection AddCopilotChatAuthentication(this IServiceCollection services, IConfiguration configuration) + { + services.AddScoped(); + var config = services.BuildServiceProvider().GetRequiredService>().Value; + switch (config.Type) + { + case ChatAuthenticationOptions.AuthenticationType.AzureAd: + services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme) + .AddMicrosoftIdentityWebApi(configuration.GetSection($"{ChatAuthenticationOptions.PropertyName}:AzureAd")); + break; + + case ChatAuthenticationOptions.AuthenticationType.None: + services.AddAuthentication(PassThroughAuthenticationHandler.AuthenticationScheme) + .AddScheme( + authenticationScheme: PassThroughAuthenticationHandler.AuthenticationScheme, + configureOptions: null); + break; + + default: + throw new InvalidOperationException($"Invalid authentication type '{config.Type}'."); + } + + return services; + } + /// /// Trim all string properties, recursively. /// diff --git a/samples/apps/copilot-chat-app/webapi/Program.cs b/samples/apps/copilot-chat-app/webapi/Program.cs index 6d93ef65ce68..6529571d77a6 100644 --- a/samples/apps/copilot-chat-app/webapi/Program.cs +++ b/samples/apps/copilot-chat-app/webapi/Program.cs @@ -45,14 +45,14 @@ public static async Task Main(string[] args) .AddCopilotChatOptions(builder.Configuration) .AddCopilotChatPlannerServices() .AddPersistentChatStore() - .AddCopilotChatUtilities(); + .AddUtilities() + .AddCopilotChatAuthentication(builder.Configuration) + .AddCopilotChatAuthorization(); // Add in the rest of the services. builder.Services .AddApplicationInsightsTelemetry() .AddLogging(logBuilder => logBuilder.AddApplicationInsights()) - .AddCopilotChatAuthentication(builder.Configuration) - .AddCopilotChatAuthorization() .AddEndpointsApiExplorer() .AddSwaggerGen() .AddCors() diff --git a/samples/apps/copilot-chat-app/webapi/ServiceExtensions.cs b/samples/apps/copilot-chat-app/webapi/ServiceExtensions.cs index f7cd01c7c487..613ea66f83e7 100644 --- a/samples/apps/copilot-chat-app/webapi/ServiceExtensions.cs +++ b/samples/apps/copilot-chat-app/webapi/ServiceExtensions.cs @@ -3,13 +3,9 @@ using System; using System.Collections.Generic; using System.Reflection; -using Microsoft.AspNetCore.Authentication; -using Microsoft.AspNetCore.Authentication.JwtBearer; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; -using Microsoft.Identity.Web; -using SemanticKernel.Service.Auth; using SemanticKernel.Service.Options; using SemanticKernel.Service.Utilities; @@ -38,13 +34,6 @@ internal static IServiceCollection AddOptions(this IServiceCollection services, var foo = services.BuildServiceProvider().GetService>(); - // Authorization configuration - services.AddOptions() - .Bind(configuration.GetSection(ChatAuthenticationOptions.PropertyName)) - .ValidateOnStart() - .ValidateDataAnnotations() - .PostConfigure(TrimStringProperties); - // Memory store configuration services.AddOptions() .Bind(configuration.GetSection(MemoriesStoreOptions.PropertyName)) @@ -55,7 +44,7 @@ internal static IServiceCollection AddOptions(this IServiceCollection services, return services; } - internal static IServiceCollection AddCopilotChatUtilities(this IServiceCollection services) + internal static IServiceCollection AddUtilities(this IServiceCollection services) { return services.AddScoped(); } @@ -83,34 +72,6 @@ internal static IServiceCollection AddCors(this IServiceCollection services) return services; } - /// - /// Add authentication services - /// - internal static IServiceCollection AddCopilotChatAuthentication(this IServiceCollection services, IConfiguration configuration) - { - services.AddScoped(); - var config = services.BuildServiceProvider().GetRequiredService>().Value; - switch (config.Type) - { - case ChatAuthenticationOptions.AuthenticationType.AzureAd: - services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme) - .AddMicrosoftIdentityWebApi(configuration.GetSection($"{ChatAuthenticationOptions.PropertyName}:AzureAd")); - break; - - case ChatAuthenticationOptions.AuthenticationType.None: - services.AddAuthentication(PassThroughAuthenticationHandler.AuthenticationScheme) - .AddScheme( - authenticationScheme: PassThroughAuthenticationHandler.AuthenticationScheme, - configureOptions: null); - break; - - default: - throw new InvalidOperationException($"Invalid authentication type '{config.Type}'."); - } - - return services; - } - /// /// Trim all string properties, recursively. /// diff --git a/samples/apps/copilot-chat-app/webapi/Utilities/AskConverter.cs b/samples/apps/copilot-chat-app/webapi/Utilities/AskConverter.cs index 851672ca50e7..f5e38871dc9f 100644 --- a/samples/apps/copilot-chat-app/webapi/Utilities/AskConverter.cs +++ b/samples/apps/copilot-chat-app/webapi/Utilities/AskConverter.cs @@ -6,6 +6,9 @@ namespace SemanticKernel.Service.Utilities; +/// +/// Converts variables to , inserting some system variables along the way. +/// public class AskConverter { private readonly IAuthInfo _authInfo; From 5e35abc04cf8f3f51afb30b1de9f073dd2d41449 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Parks=20=C2=A9=EF=B8=8F?= Date: Tue, 23 May 2023 19:59:19 -0700 Subject: [PATCH 15/15] move options file --- .../{ => CopilotChat}/Options/ChatAuthenticationOptions.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) rename samples/apps/copilot-chat-app/webapi/{ => CopilotChat}/Options/ChatAuthenticationOptions.cs (94%) diff --git a/samples/apps/copilot-chat-app/webapi/Options/ChatAuthenticationOptions.cs b/samples/apps/copilot-chat-app/webapi/CopilotChat/Options/ChatAuthenticationOptions.cs similarity index 94% rename from samples/apps/copilot-chat-app/webapi/Options/ChatAuthenticationOptions.cs rename to samples/apps/copilot-chat-app/webapi/CopilotChat/Options/ChatAuthenticationOptions.cs index 8ae2d51affc1..7198f2e67df4 100644 --- a/samples/apps/copilot-chat-app/webapi/Options/ChatAuthenticationOptions.cs +++ b/samples/apps/copilot-chat-app/webapi/CopilotChat/Options/ChatAuthenticationOptions.cs @@ -1,8 +1,9 @@ // Copyright (c) Microsoft. All rights reserved. using System.ComponentModel.DataAnnotations; +using SemanticKernel.Service.Options; -namespace SemanticKernel.Service.Options; +namespace SemanticKernel.Service.CopilotChat.Options; /// /// Configuration options for authorizing to the service.