Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor updates before removing auth env variables #415

Merged
merged 2 commits into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions webapi/Controllers/ServiceInfoController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ public IActionResult GetAuthConfig()
{
AuthType = this._chatAuthenticationOptions.Type.ToString(),
AadAuthority = authorityUriString,
AadClient = this._frontendOptions.AadClientId,
AadApiScope = $"api://{this._chatAuthenticationOptions.AzureAd!.ClientId}/{this._chatAuthenticationOptions.AzureAd!.Scopes}"
AadClientId = this._frontendOptions.AadClientId,
AadApiScope = $"api://{this._chatAuthenticationOptions.AzureAd!.ClientId}/{this._chatAuthenticationOptions.AzureAd!.Scopes}",
};

return this.Ok(config);
Expand Down
2 changes: 1 addition & 1 deletion webapi/Extensions/ServiceExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ internal static IServiceCollection AddUtilities(this IServiceCollection services
return services.AddScoped<AskConverter>();
}

internal static IServiceCollection AddMainetnanceServices(this IServiceCollection services)
internal static IServiceCollection AddMaintenanceServices(this IServiceCollection services)
{
// Inject migration services
services.AddSingleton<IChatMigrationMonitor, ChatMigrationMonitor>();
Expand Down
4 changes: 2 additions & 2 deletions webapi/Models/Response/FrontendAuthConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ public class FrontendAuthConfig
/// <summary>
/// Azure Active Directory client ID the frontend is to use.
/// </summary>
[JsonPropertyName("aadClient")]
public string AadClient { get; set; } = string.Empty;
[JsonPropertyName("aadClientId")]
public string AadClientId { get; set; } = string.Empty;

/// <summary>
/// Azure Active Directory scope the frontend should request.
Expand Down
2 changes: 1 addition & 1 deletion webapi/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public static async Task Main(string[] args)

// Add in the rest of the services.
builder.Services
.AddMainetnanceServices()
.AddMaintenanceServices()
.AddEndpointsApiExplorer()
.AddSwaggerGen()
.AddCorsPolicy(builder.Configuration)
Expand Down
6 changes: 3 additions & 3 deletions webapp/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ const App: FC = () => {
}
}

if ((isAuthenticated || !AuthHelper.IsAuthAAD) && appState === AppState.LoadingChats) {
if ((isAuthenticated || !AuthHelper.isAuthAAD()) && appState === AppState.LoadingChats) {
void Promise.all([
// Load all chats from memory
chat.loadChats().then((succeeded) => {
Expand Down Expand Up @@ -139,7 +139,7 @@ const App: FC = () => {
className="app-container"
theme={features[FeatureKeys.DarkMode].enabled ? semanticKernelDarkTheme : semanticKernelLightTheme}
>
{AuthHelper.IsAuthAAD ? (
{AuthHelper.isAuthAAD() ? (
<>
<UnauthenticatedTemplate>
<div className={classes.container}>
Expand Down Expand Up @@ -191,7 +191,7 @@ const Chat = ({
<BackendProbe
uri={process.env.REACT_APP_BACKEND_URI as string}
onBackendFound={() => {
if (AuthHelper.IsAuthAAD) {
if (AuthHelper.isAuthAAD()) {
setAppState(AppState.SettingUserInfo);
} else {
setAppState(AppState.LoadingChats);
Expand Down
5 changes: 3 additions & 2 deletions webapp/src/components/chat/ChatInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { RootState } from '../../redux/app/store';
import { addAlert } from '../../redux/features/app/appSlice';
import { editConversationInput, updateBotResponseStatus } from '../../redux/features/conversations/conversationsSlice';
import { Alerts } from '../shared/Alerts';
import { getErrorDetails } from '../utils/TextUtils';
import { SpeechService } from './../../libs/services/SpeechService';
import { updateUserIsTyping } from './../../redux/features/conversations/conversationsSlice';
import { ChatStatus } from './ChatStatus';
Expand Down Expand Up @@ -97,7 +98,7 @@ export const ChatInput: React.FC<ChatInputProps> = ({ isDraggingOver, onDragLeav

React.useEffect(() => {
async function initSpeechRecognizer() {
const speechService = new SpeechService(process.env.REACT_APP_BACKEND_URI as string);
const speechService = new SpeechService();
const response = await speechService.getSpeechTokenAsync(
await AuthHelper.getSKaaSAccessToken(instance, inProgress),
);
Expand All @@ -108,7 +109,7 @@ export const ChatInput: React.FC<ChatInputProps> = ({ isDraggingOver, onDragLeav
}

initSpeechRecognizer().catch((e) => {
const errorDetails = e instanceof Error ? e.message : String(e);
const errorDetails = getErrorDetails(e);
const errorMessage = `Unable to initialize speech recognizer. Details: ${errorDetails}`;
dispatch(addAlert({ message: errorMessage, type: AlertType.Error }));
});
Expand Down
5 changes: 2 additions & 3 deletions webapp/src/components/chat/shared/EditChatName.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { addAlert } from '../../../redux/features/app/appSlice';
import { editConversationTitle } from '../../../redux/features/conversations/conversationsSlice';
import { Breakpoints } from '../../../styles';
import { Checkmark20, Dismiss20 } from '../../shared/BundledIcons';
import { getErrorDetails } from '../../utils/TextUtils';

const useClasses = makeStyles({
root: {
Expand Down Expand Up @@ -67,9 +68,7 @@ export const EditChatName: React.FC<IEditChatNameProps> = ({ name, chatId, exitE

const handleSave = () => {
onSaveTitleChange().catch((e: any) => {
const errorMessage = `Unable to retrieve chat to change title. Details: ${
e instanceof Error ? e.message : String(e)
}`;
const errorMessage = `Unable to retrieve chat to change title. Details: ${getErrorDetails(e)}`;
dispatch(addAlert({ message: errorMessage, type: AlertType.Error }));
});
};
Expand Down
2 changes: 1 addition & 1 deletion webapp/src/components/header/UserSettingsMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export const UserSettingsMenu: FC<IUserSettingsProps> = ({ setLoadingState }) =>

return (
<>
{AuthHelper.IsAuthAAD ? (
{AuthHelper.isAuthAAD() ? (
<Menu>
<MenuTrigger disableButtonEnhancement>
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Divider, Switch, Text, makeStyles, shorthands, tokens } from '@fluentui/react-components';
import { useCallback } from 'react';
import { AuthHelper } from '../../../libs/auth/AuthHelper';
import { useAppDispatch, useAppSelector } from '../../../redux/app/hooks';
import { RootState } from '../../../redux/app/store';
import { FeatureKeys, Setting } from '../../../redux/features/app/AppState';
Expand Down Expand Up @@ -56,7 +57,9 @@ export const SettingSection: React.FC<ISettingsSectionProps> = ({ setting, conte
<Switch
label={feature.label}
checked={feature.enabled}
disabled={feature.inactive}
disabled={
!!feature.inactive || (key === FeatureKeys.MultiUserChat && !AuthHelper.isAuthAAD())
glahaye marked this conversation as resolved.
Show resolved Hide resolved
}
onChange={() => {
onFeatureChange(key);
}}
Expand Down
12 changes: 7 additions & 5 deletions webapp/src/components/open-api-plugins/PluginConnector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ import {
} from '@fluentui/react-components';
import { Dismiss20Regular } from '@fluentui/react-icons';
import { FormEvent, useState } from 'react';
import { AuthHelper } from '../../libs/auth/AuthHelper';
import { TokenHelper } from '../../libs/auth/TokenHelper';
import { useAppDispatch } from '../../redux/app/hooks';
import { AdditionalApiProperties, Plugin, PluginAuthRequirements } from '../../redux/features/plugins/PluginsState';
import { AdditionalApiProperties, PluginAuthRequirements } from '../../redux/features/plugins/PluginsState';
import { connectPlugin } from '../../redux/features/plugins/pluginsSlice';

const useClasses = makeStyles({
Expand Down Expand Up @@ -54,7 +55,6 @@ interface PluginConnectorProps {
publisher: string;
authRequirements: PluginAuthRequirements;
apiProperties?: AdditionalApiProperties;
inactive?: Plugin['inactive'];
}

export const PluginConnector: React.FC<PluginConnectorProps> = ({
Expand All @@ -63,7 +63,6 @@ export const PluginConnector: React.FC<PluginConnectorProps> = ({
publisher,
authRequirements,
apiProperties,
inactive,
}) => {
const classes = useClasses();

Expand Down Expand Up @@ -121,6 +120,9 @@ export const PluginConnector: React.FC<PluginConnectorProps> = ({
setOpen(false);
};

const inactive = msalRequired && !AuthHelper.isAuthAAD();
const inactiveReason = 'Only available when using Azure AD authorization.';

return (
<Dialog
open={open}
Expand All @@ -135,8 +137,8 @@ export const PluginConnector: React.FC<PluginConnectorProps> = ({
data-testid="openPluginDialogButton"
aria-label="Enable plugin"
appearance="primary"
disabled={!!inactive}
title={inactive}
disabled={inactive}
title={inactive ? inactiveReason : ''}
>
Enable
</Button>
Expand Down
3 changes: 1 addition & 2 deletions webapp/src/components/open-api-plugins/cards/PluginCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ interface PluginCardProps {
}

export const PluginCard: React.FC<PluginCardProps> = ({ plugin }) => {
const { name, publisher, enabled, authRequirements, apiProperties, icon, description, inactive } = plugin;
const { name, publisher, enabled, authRequirements, apiProperties, icon, description } = plugin;
const dispatch = useAppDispatch();

const onDisconnectClick = (event: FormEvent) => {
Expand Down Expand Up @@ -42,7 +42,6 @@ export const PluginCard: React.FC<PluginCardProps> = ({ plugin }) => {
publisher={publisher}
authRequirements={authRequirements}
apiProperties={apiProperties}
inactive={inactive}
/>
)
}
Expand Down
7 changes: 7 additions & 0 deletions webapp/src/components/utils/TextUtils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,10 @@ export function replaceCitationLinksWithIndices(formattedMessageContent: string,

return formattedMessageContent;
}

/**
* Gets message of error
*/
export function getErrorDetails(error: unknown) {
return error instanceof Error ? error.message : String(error);
}
5 changes: 4 additions & 1 deletion webapp/src/components/views/Login.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Body1, Button, Image, Title3 } from '@fluentui/react-components';
import React from 'react';
import signInLogo from '../../ms-symbollockup_signin_light.svg';
import { useSharedClasses } from '../../styles';
import { getErrorDetails } from '../utils/TextUtils';

export const Login: React.FC = () => {
const { instance } = useMsal();
Expand All @@ -24,7 +25,9 @@ export const Login: React.FC = () => {
style={{ padding: 0 }}
appearance="transparent"
onClick={() => {
instance.loginRedirect().catch(() => {});
instance.loginRedirect().catch((e: unknown) => {
alert(`Error signing in: ${getErrorDetails(e)}`);
});
}}
data-testid="signinButton"
>
Expand Down
6 changes: 4 additions & 2 deletions webapp/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ document.addEventListener('DOMContentLoaded', () => {

const missingEnvVariables = getMissingEnvVariables();
const validEnvFile = missingEnvVariables.length === 0;
const shouldUseMsal = validEnvFile && AuthHelper.IsAuthAAD;
const shouldUseMsal = validEnvFile && AuthHelper.isAuthAAD();

let msalInstance: IPublicClientApplication | null = null;
if (shouldUseMsal) {
Expand Down Expand Up @@ -58,7 +58,9 @@ document.addEventListener('DOMContentLoaded', () => {
{/* eslint-enable @typescript-eslint/no-non-null-assertion */}
</ReduxProvider>
) : (
<MissingEnvVariablesError missingVariables={missingEnvVariables} />
<FluentProvider className="app-container" theme={semanticKernelLightTheme}>
<MissingEnvVariablesError missingVariables={missingEnvVariables} />
</FluentProvider>
)}
</React.StrictMode>,
);
Expand Down
6 changes: 3 additions & 3 deletions webapp/src/libs/auth/AuthHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,12 @@ const logoutAsync = (instance: IPublicClientApplication) => {
/**
* Determines if the app is configured to use Azure AD for authorization
*/
const IsAuthAAD = process.env.REACT_APP_AUTH_TYPE === AuthType.AAD;
const isAuthAAD = () => process.env.REACT_APP_AUTH_TYPE === AuthType.AAD;

// SKaaS = Semantic Kernel as a Service
// Gets token with scopes to authorize SKaaS specifically
const getSKaaSAccessToken = async (instance: IPublicClientApplication, inProgress: InteractionStatus) => {
return IsAuthAAD
return isAuthAAD()
? await TokenHelper.getAccessTokenUsingMsal(inProgress, instance, Constants.msal.semanticKernelScopes)
: '';
};
Expand All @@ -126,5 +126,5 @@ export const AuthHelper = {
ssoSilentRequest,
loginAsync,
logoutAsync,
IsAuthAAD,
isAuthAAD,
};
6 changes: 3 additions & 3 deletions webapp/src/libs/hooks/useChat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ export const useChat = () => {
const { conversations } = useAppSelector((state: RootState) => state.conversations);
const { activeUserInfo, features } = useAppSelector((state: RootState) => state.app);

const botService = new BotService(process.env.REACT_APP_BACKEND_URI as string);
const chatService = new ChatService(process.env.REACT_APP_BACKEND_URI as string);
const documentImportService = new DocumentImportService(process.env.REACT_APP_BACKEND_URI as string);
const botService = new BotService();
const chatService = new ChatService();
const documentImportService = new DocumentImportService();

const botProfilePictures: string[] = [botIcon1, botIcon2, botIcon3, botIcon4, botIcon5];

Expand Down
2 changes: 1 addition & 1 deletion webapp/src/libs/hooks/useFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export const useFile = () => {
const { instance, inProgress } = useMsal();

const chat = useChat();
const documentImportService = new DocumentImportService(process.env.REACT_APP_BACKEND_URI as string);
const documentImportService = new DocumentImportService();

async function loadFile<T>(file: File, loadCallBack: (data: T) => Promise<void>): Promise<T> {
return await new Promise((resolve, reject) => {
Expand Down
2 changes: 1 addition & 1 deletion webapp/src/libs/hooks/useGraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const useGraph = () => {
const graphService = new GraphService();

const loadUsers = async (userIds: string[]) => {
if (!AuthHelper.IsAuthAAD) {
if (!AuthHelper.isAuthAAD()) {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion webapp/src/libs/hooks/usePlugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { getErrorDetails } from './useChat';
export const usePlugins = () => {
const dispatch = useAppDispatch();
const { instance, inProgress } = useMsal();
const chatService = React.useMemo(() => new ChatService(process.env.REACT_APP_BACKEND_URI as string), []);
const chatService = React.useMemo(() => new ChatService(), []);

const addCustomPlugin = (manifest: PluginManifest, manifestDomain: string) => {
const newPlugin: Plugin = {
Expand Down
17 changes: 11 additions & 6 deletions webapp/src/libs/services/BaseService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,26 @@ interface ServiceRequest {

const noResponseBodyStatusCodes = [202, 204];

export const BackendServiceUrl = process.env.REACT_APP_BACKEND_URI ?? window.origin;
glahaye marked this conversation as resolved.
Show resolved Hide resolved

export class BaseService {
// eslint-disable-next-line @typescript-eslint/space-before-function-paren
constructor(protected readonly serviceUrl: string) {}
constructor(protected readonly serviceUrl: string = BackendServiceUrl) {}

protected readonly getResponseAsync = async <T>(
request: ServiceRequest,
accessToken: string,
accessToken?: string,
enabledPlugins?: Plugin[],
): Promise<T> => {
const { commandPath, method, body, query } = request;
const isFormData = body instanceof FormData;

const headers = new Headers({
Authorization: `Bearer ${accessToken}`,
});
const headers = new Headers(
accessToken
? {
Authorization: `Bearer ${accessToken}`,
}
: undefined,
);

if (!isFormData) {
headers.append('Content-Type', 'application/json');
Expand Down
22 changes: 19 additions & 3 deletions webapp/src/redux/features/app/AppState.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,27 @@
// Copyright (c) Microsoft. All rights reserved.

import { AuthHelper, DefaultActiveUserInfo } from '../../../libs/auth/AuthHelper';
import { AuthHelper } from '../../../libs/auth/AuthHelper';
import { AlertType } from '../../../libs/models/AlertType';
import { IChatUser } from '../../../libs/models/ChatUser';
import { ServiceOptions } from '../../../libs/models/ServiceOptions';
import { TokenUsage } from '../../../libs/models/TokenUsage';

// This is the default user information when authentication is set to 'None'.
// It must match what is defined in PassthroughAuthenticationHandler.cs on the backend.
export const DefaultChatUser: IChatUser = {
id: 'c05c61eb-65e4-4223-915a-fe72b0c9ece1',
emailAddress: 'user@contoso.com',
fullName: 'Default User',
online: true,
isTyping: false,
};

export const DefaultActiveUserInfo: ActiveUserInfo = {
id: DefaultChatUser.id,
email: DefaultChatUser.emailAddress,
username: DefaultChatUser.fullName,
};

export interface ActiveUserInfo {
id: string;
email: string;
Expand Down Expand Up @@ -86,7 +103,6 @@ export const Features = {
enabled: false,
label: 'Live Chat Session Sharing',
description: 'Enable multi-user chat sessions. Not available when authorization is disabled.',
inactive: !AuthHelper.IsAuthAAD,
},
[FeatureKeys.RLHF]: {
enabled: false,
Expand Down Expand Up @@ -122,7 +138,7 @@ export const Settings = [

export const initialState: AppState = {
alerts: [],
activeUserInfo: AuthHelper.IsAuthAAD ? undefined : DefaultActiveUserInfo,
activeUserInfo: AuthHelper.isAuthAAD() ? undefined : DefaultActiveUserInfo,
tokenUsage: {},
features: Features,
settings: Settings,
Expand Down
Loading
Loading