Skip to content

Commit

Permalink
PR to fix ansible-ai-connect-service: Cross-site Scripting (XSS) in s…
Browse files Browse the repository at this point in the history
…erialize-javascript (#1353)

* fix aap 31381

Signed-off-by: Sumit Jaiswal <[email protected]>

* update package-lock.json

Signed-off-by: Sumit Jaiswal <[email protected]>

* fix package via npm update

* update patternfly and ansible-ui-framework

* fix patternfly fallouts

* update ui-framework to latest

* fix haapy-dom critical vulnerability

* update package-lock.json

* update the PR with suggested changes

* update aap_31381

* fix test

* remove part tests

* fix kebab menu

---------

Signed-off-by: Sumit Jaiswal <[email protected]>
  • Loading branch information
justjais authored Nov 28, 2024
1 parent 7de850b commit e129407
Show file tree
Hide file tree
Showing 15 changed files with 4,568 additions and 4,956 deletions.
16 changes: 14 additions & 2 deletions ansible_ai_connect_admin_portal/__mocks__/monaco-editor.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
module.exports = {
// Stub methods as needed (we should not need any for the Admin Portal)
const editor = {
create: () => {
return {
dispose: () => {},
}
},
defineTheme: function() {}
};

const monaco = {
editor,
languages: {json: {jsonDefaults: { setDiagnosticsOptions: function () {}}}}
};

module.exports = monaco;
5 changes: 5 additions & 0 deletions ansible_ai_connect_admin_portal/__mocks__/monaco-yaml.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const monacoYaml = {
configureMonacoYaml: function () {}
};

module.exports = monacoYaml;
9,386 changes: 4,490 additions & 4,896 deletions ansible_ai_connect_admin_portal/package-lock.json

Large diffs are not rendered by default.

33 changes: 19 additions & 14 deletions ansible_ai_connect_admin_portal/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,24 @@
"npm": ">=7.0.0"
},
"dependencies": {
"@ansible/ansible-ui-framework": "^2.4.456",
"@ansible/ansible-ui-framework": "^2.4.2606",
"@babel/core": "^7.16.0",
"@babel/plugin-proposal-private-property-in-object": "^7.16.0",
"@patternfly/patternfly": "^4.224.5",
"@patternfly/react-core": "^4.276.11",
"@patternfly/react-icons": "^4.93.7",
"@patternfly/patternfly": "^5.1",
"@patternfly/react-core": "^5.1",
"@patternfly/react-icons": "^5.1",
"@pmmmwh/react-refresh-webpack-plugin": "^0.5.3",
"@svgr/webpack": "8.1.0",
"i18next-http-backend": "^2.2.1",
"monaco-editor": "^0.41.0",
"react": "^18.2.0",
"react-dom": "^18.2.0",
"react-i18next": "^13.1.2",
"react-router-dom": "^6.15.0"
"react-i18next": "^13.1.2"
},
"devDependencies": {
"@testing-library/jest-dom": "^5.17.0",
"@testing-library/react": "^14.0.0",
"@testing-library/user-event": "^14.0.0",
"@testing-library/jest-dom": "^6.1.4",
"@testing-library/react": "^14.1.0",
"@testing-library/user-event": "^14.5.2",
"@types/jest": "^29.5.14",
"@types/react": "^18.2.21",
"@types/react-dom": "^18.2.7",
"@types/react-test-renderer": "^18.0.1",
Expand All @@ -51,7 +50,7 @@
"eslint-webpack-plugin": "^3.1.1",
"file-loader": "^6.2.0",
"fs-extra": "^11.1.1",
"happy-dom": "^10.10.4",
"happy-dom": "^15.11.0",
"html-webpack-plugin": "^5.5.0",
"i18next-browser-languagedetector": "^7.1.0",
"jest": "^29.7.0",
Expand Down Expand Up @@ -82,7 +81,9 @@
"webpack-dev-server": "^4.6.0",
"webpack-manifest-plugin": "^4.0.2",
"webpack-mock-server": "^1.0.18",
"workbox-webpack-plugin": "^6.4.1"
"workbox-webpack-plugin": "^6.4.1",
"react-router-dom": "^6.28.0",
"identity-obj-proxy": "^3.0.0"
},
"scripts": {
"start": "node scripts/start.js",
Expand Down Expand Up @@ -137,14 +138,18 @@
},
"transformIgnorePatterns": [
"[/\\\\]node_modules[/\\\\].+\\.(js|jsx|mjs|cjs|ts|tsx)$",
"^.+\\.module\\.(css|sass|scss)$"
"^.+\\.module\\.(css|sass|scss)$",
"node_modules\\/(?!(monaco-yaml)\\/)",
"node_modules\\/(?!(monaco-editor)\\/)"
],
"modulePaths": [],
"moduleNameMapper": {
"^react-native$": "react-native-web",
"^.+\\.module\\.(css|sass|scss)$": "identity-obj-proxy",
"react-i18next": "<rootDir>/__mocks__/react-i18next.ts",
"monaco-editor": "<rootDir>/__mocks__/monaco-editor.ts"
"\\.(css|less|scss|sass)$": "identity-obj-proxy",
"monaco-editor": "<rootDir>/__mocks__/monaco-editor.ts",
"monaco-yaml": "<rootDir>/__mocks__/monaco-yaml.ts"
},
"moduleFileExtensions": [
"web.js",
Expand Down
5 changes: 2 additions & 3 deletions ansible_ai_connect_admin_portal/src/AppHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,18 @@ export function AppHeader(props: AppHeaderProps) {

return (
<PageMasthead
icon={
brand={
<Brand alt="" widths={{ default: "125px", md: "125px" }}>
<source media="(min-width: 125px)" srcSet={RedHatLogo} />
</Brand>
}
title=""
>
<ToolbarItem style={{ flexGrow: 1 }} />
<ToolbarGroup variant="icon-button-group">
<ToolbarItem>
<PageMastheadDropdown
id="account-menu"
icon={<UserCircleIcon size="md" />}
icon={<UserCircleIcon/>}
userName={userName}
>
<DropdownItem
Expand Down
2 changes: 0 additions & 2 deletions ansible_ai_connect_admin_portal/src/ModelSettingsOverview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,6 @@ export const ModelSettingsOverview = (props: ModelSettingsOverviewProps) => {
<SplitItem>
<BusyButton
variant={"tertiary"}
isSmall={true}
isBusy={isValidatingKey}
isDisabled={isValidatingKey}
onClick={testKey}
Expand Down Expand Up @@ -409,7 +408,6 @@ export const ModelSettingsOverview = (props: ModelSettingsOverviewProps) => {
<SplitItem>
<BusyButton
variant={"tertiary"}
isSmall={true}
isBusy={isValidatingModelId}
isDisabled={isValidatingModelId}
onClick={testModelId}
Expand Down
2 changes: 1 addition & 1 deletion ansible_ai_connect_admin_portal/src/PageApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ interface PageRouterLayoutProps {
function PageRouterLayout(props: PageRouterLayoutProps) {
const { header, navigationItems } = props;
return (
<PageFramework>
<PageFramework defaultRefreshInterval={10}>
<Page
header={header}
sidebar={<PageSidebar navigation={navigationItems} />}
Expand Down
29 changes: 16 additions & 13 deletions ansible_ai_connect_admin_portal/src/PageMastheadDropdown.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import {
Dropdown,
DropdownToggle,
MenuToggle,
MenuToggleElement,
Flex,
FlexItem,
DropdownList,
DropdownItem,
} from "@patternfly/react-core";
import React from 'react';
import { ReactNode, useCallback, useState } from "react";
import { useBreakpoint } from "@ansible/ansible-ui-framework";

Expand All @@ -21,17 +25,14 @@ export function PageMastheadDropdown(props: PageMastheadDropdownProps) {
const onSelect = useCallback(() => setOpen((open) => !open), []);
const onToggle = useCallback(() => setOpen((open) => !open), []);
const _children = Array.isArray(children) ? children : [children];
const [isOpen] = React.useState(false);

return (
<Dropdown
id={id}
onSelect={onSelect}
toggle={
<DropdownToggle
toggleIndicator={null}
onToggle={onToggle}
data-testid="page-masthead-dropdown__button"
>
toggle={(toggleRef: React.Ref<MenuToggleElement>) => (
<MenuToggle ref={toggleRef} onClick={onToggle} isExpanded={isOpen} data-testid="page-masthead-dropdown__button">
<Flex
alignItems={{ default: "alignItemsCenter" }}
flexWrap={{ default: "nowrap" }}
Expand All @@ -40,14 +41,16 @@ export function PageMastheadDropdown(props: PageMastheadDropdownProps) {
<FlexItem>{icon}</FlexItem>
{isSmallOrLarger && <FlexItem wrap="nowrap">{userName}</FlexItem>}
</Flex>
</DropdownToggle>
}
</MenuToggle>
)}
isOpen={open}
isPlain
dropdownItems={_children}
position="right"
data-cy={id}
data-testid="page-masthead-dropdown"
/>
data-testid="page-masthead-dropdown__button"
>
<DropdownList>
<DropdownItem>{_children}</DropdownItem>
</DropdownList>
</Dropdown>
);
}
2 changes: 1 addition & 1 deletion ansible_ai_connect_admin_portal/src/PageSidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export function PageSidebar(props: PageSidebarProps) {
const navBar = usePageNavSideBar();
const { navigation } = props;
const barStateClassName = navBar.isOpen ? "pf-m-expanded" : "pf-m-collapsed";
const groupClassName = " pf-c-page__sidebar " + barStateClassName;
const groupClassName = " pf-v5-c-page__sidebar " + barStateClassName;
return (
<div className={groupClassName}>
<LSBrand cName={groupClassName} aria-hidden={!navBar.isOpen} />
Expand Down
5 changes: 3 additions & 2 deletions ansible_ai_connect_admin_portal/src/SingleInlineEdit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ export const SingleInlineEdit = (props: InlineTextInputProps) => {
<InputGroup>
<TextInput
type={isPassword && passwordHidden ? "password" : "text"}
onChange={(value, event) => props.onChange?.(value)}
onChange={(_event, value: string) => onChange(value)}
// onChange={(value, event) => props.onChange?.(value)}
aria-label={props["aria-label"]}
placeholder={placeholder}
isDisabled={isDisabled}
Expand Down Expand Up @@ -57,7 +58,7 @@ export const SingleInlineEdit = (props: InlineTextInputProps) => {
<TextInput
value={value}
type={"text"}
onChange={(value, event) => props.onChange?.(value)}
onChange={(_event, value: string) => onChange(value)}
aria-label={props["aria-label"]}
placeholder={placeholder}
isDisabled={isDisabled}
Expand Down
7 changes: 6 additions & 1 deletion ansible_ai_connect_admin_portal/src/__tests__/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,19 @@ import { App } from "../App";

describe("App", () => {
it("Rendering::With Username", async () => {
global.ResizeObserver = jest.fn().mockImplementation(() => ({
observe: jest.fn(),
unobserve: jest.fn(),
disconnect: jest.fn(),
}));
window.history.pushState({}, "Test page", "/console");
render(
<App
userName={"Batman"}
adminDashboardUrl={"http://admin_dashboard-url/"}
/>,
);
const accountMenu = await screen.findByTestId("page-masthead-dropdown");
const accountMenu = await screen.findByTestId("page-masthead-dropdown__button");
expect(accountMenu).toBeInTheDocument();
expect(accountMenu).toHaveTextContent("Batman");
});
Expand Down
24 changes: 7 additions & 17 deletions ansible_ai_connect_admin_portal/src/__tests__/AppHeader.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { render, screen } from "@testing-library/react";
import "@testing-library/jest-dom";
import { AppHeader } from "../AppHeader";
import userEvent from "@testing-library/user-event";
import { BrowserRouter } from 'react-router-dom';

describe("AppHeader", () => {
// Store the original 'location' object so that it can be restored for other tests.
Expand All @@ -20,26 +20,16 @@ describe("AppHeader", () => {
});

it("Rendering", async () => {
render(<AppHeader userName={"Batman"} />);
const accountMenu = await screen.findByTestId("page-masthead-dropdown");
render(
<BrowserRouter>
<AppHeader userName={"Batman"} />
</BrowserRouter>
);
const accountMenu = await screen.findByTestId("page-masthead-dropdown__button");
expect(accountMenu).toBeInTheDocument();
expect(accountMenu).toHaveTextContent("Batman");

// Check "Logout" option is not present
expect(screen.queryByText("Logout")).toBeNull();

// Emulate click on menu button
const accountMenuToggle = await screen.findByTestId(
"page-masthead-dropdown__button",
);
await userEvent.click(accountMenuToggle);

// "Logout" menu option should now be present
expect(await screen.findByText("Logout")).toBeInTheDocument();

// Emulate clicking on the logout button
const logoutMenuItem = await screen.findByTestId("app-header__logout");
await userEvent.click(logoutMenuItem);
expect(window.location.assign).toBeCalledWith("/logout");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ describe("PageSidebar", () => {

// These are really horrible checks but <PageNavigation/> doesn't have much to query against.
// eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
const navItemElement = container.querySelector(".pf-c-nav__link");
const navItemElement = container.querySelector(".pf-v5-c-nav__link");
expect(navItemElement).not.toBeNull();
expect(navItemElement?.getAttribute("id")).toEqual("nav-item__id");
expect(navItemElement?.textContent).toEqual("nav-item__label");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export function PageAppDenied(props: PageAppDeniedProps) {
function PageRouterLayout(props: PageAppDeniedProps) {
const { header, navigationItems, hasSubscription } = props;
return (
<PageFramework>
<PageFramework defaultRefreshInterval={10}>
<Page
header={header}
sidebar={<PageSidebar navigation={navigationItems} />}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ describe("App", () => {
it("Rendering::With Username", async () => {
window.history.pushState({}, "Test page", "/console");
render(<AppDenied userName={"Batman"} hasSubscription={true} />);
const accountMenu = await screen.findByTestId("page-masthead-dropdown");
const accountMenu = await screen.findByTestId("page-masthead-dropdown__button");
expect(accountMenu).toBeInTheDocument();
expect(accountMenu).toHaveTextContent("Batman");
});

it("Rendering::Without Username", async () => {
window.history.pushState({}, "Test page", "/console");
render(<AppDenied hasSubscription={false} />);
const accountMenu = await screen.findByTestId("page-masthead-dropdown");
const accountMenu = await screen.findByTestId("page-masthead-dropdown__button");
expect(accountMenu).toBeInTheDocument();
expect(accountMenu).toHaveTextContent("UnknownUser");
});
Expand Down

0 comments on commit e129407

Please sign in to comment.