From 2e0357d39fef152c3f8dab320c35e865b41a6032 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Wed, 24 Apr 2024 13:05:42 -0400 Subject: [PATCH] fix(jmxauth): remove references to client-side localstorage JMX credential passthrough (#1241) --- src/app/AppLayout/AuthModal.tsx | 13 +- ...eCredentials.tsx => StoredCredentials.tsx} | 13 +- src/app/SecurityPanel/SecurityPanel.tsx | 4 +- .../Settings/Config/CredentialsStorage.tsx | 112 ----- src/app/Settings/Settings.tsx | 2 - .../Services/AuthCredentials.service.tsx | 55 --- src/app/Shared/Services/Services.tsx | 4 - src/app/Topology/Actions/CreateTarget.tsx | 5 +- .../Credentials/StoreCredentials.test.tsx | 432 ------------------ src/test/Settings/CredentialsStorage.test.tsx | 97 ---- src/test/Settings/Settings.test.tsx | 12 - .../Services/JmxCredentials.service.test.tsx | 117 ----- 12 files changed, 12 insertions(+), 854 deletions(-) rename src/app/SecurityPanel/Credentials/{StoreCredentials.tsx => StoredCredentials.tsx} (97%) delete mode 100644 src/app/Settings/Config/CredentialsStorage.tsx delete mode 100644 src/app/Shared/Services/AuthCredentials.service.tsx delete mode 100644 src/test/SecurityPanel/Credentials/StoreCredentials.test.tsx delete mode 100644 src/test/Settings/CredentialsStorage.test.tsx delete mode 100644 src/test/Shared/Services/JmxCredentials.service.test.tsx diff --git a/src/app/AppLayout/AuthModal.tsx b/src/app/AppLayout/AuthModal.tsx index fdeb2e9e6..a7790ac18 100644 --- a/src/app/AppLayout/AuthModal.tsx +++ b/src/app/AppLayout/AuthModal.tsx @@ -43,7 +43,9 @@ export const AuthModal: React.FC = ({ onDismiss, onSave: onProps filter((target) => !!target), first(), map((target: Target) => target.connectUrl), - mergeMap((connectUrl) => context.authCredentials.setCredential(connectUrl, username, password)), + mergeMap((connectUrl) => + context.api.postCredentials(`target.connectUrl == "${connectUrl}"`, username, password), + ), ) .subscribe((ok) => { setLoading(false); @@ -53,7 +55,7 @@ export const AuthModal: React.FC = ({ onDismiss, onSave: onProps }), ); }, - [addSubscription, context.authCredentials, targetObs, setLoading, onPropsSave], + [addSubscription, context.api, targetObs, setLoading, onPropsSave], ); return ( @@ -70,12 +72,7 @@ export const AuthModal: React.FC = ({ onDismiss, onSave: onProps Security {' '} - to add a credential matching multiple targets. Visit{' '} - - Settings - {' '} - to confirm and configure whether these credentials will be held only for this browser session or stored - encrypted in the Cryostat backend. + to add a credential matching multiple targets. } > diff --git a/src/app/SecurityPanel/Credentials/StoreCredentials.tsx b/src/app/SecurityPanel/Credentials/StoredCredentials.tsx similarity index 97% rename from src/app/SecurityPanel/Credentials/StoreCredentials.tsx rename to src/app/SecurityPanel/Credentials/StoredCredentials.tsx index ea6047960..ae1af7326 100644 --- a/src/app/SecurityPanel/Credentials/StoreCredentials.tsx +++ b/src/app/SecurityPanel/Credentials/StoredCredentials.tsx @@ -44,7 +44,6 @@ import { import { OutlinedQuestionCircleIcon, SearchIcon } from '@patternfly/react-icons'; import { ExpandableRowContent, TableComposable, Tbody, Td, Th, Thead, Tr } from '@patternfly/react-table'; import * as React from 'react'; -import { Link } from 'react-router-dom'; import { forkJoin } from 'rxjs'; import { SecurityCard } from '../types'; import { CreateCredentialModal } from './CreateCredentialModal'; @@ -179,7 +178,7 @@ const tableColumns: TableColumn[] = [ const tableTitle = 'Stored Credentials'; -export const StoreCredentials = () => { +export const StoredCredentials = () => { const context = React.useContext(ServiceContext); const [state, dispatch] = React.useReducer(reducer, { credentials: [] as StoredCredential[], @@ -519,7 +518,7 @@ export const CheckBoxActions: React.FC = ({ ); }; -export const StoreCredentialsCard: SecurityCard = { +export const StoredCredentialsCard: SecurityCard = { key: 'credentials', title: ( @@ -535,13 +534,9 @@ export const StoreCredentialsCard: SecurityCard = { Credentials that Cryostat uses to connect to Cryostat agents or target JVMs over JMX are stored in encrypted - storage. - - - The locally-stored client credentials held by your browser session are not displayed here. See{' '} - Settings to configure locally-stored credentials. + storage managed by the Cryostat server. ), - content: StoreCredentials, + content: StoredCredentials, }; diff --git a/src/app/SecurityPanel/SecurityPanel.tsx b/src/app/SecurityPanel/SecurityPanel.tsx index 80644ce30..6856ba352 100644 --- a/src/app/SecurityPanel/SecurityPanel.tsx +++ b/src/app/SecurityPanel/SecurityPanel.tsx @@ -16,13 +16,13 @@ import { BreadcrumbPage } from '@app/BreadcrumbPage/BreadcrumbPage'; import { Card, CardBody, CardTitle, Text, TextVariants } from '@patternfly/react-core'; import * as React from 'react'; -import { StoreCredentialsCard } from './Credentials/StoreCredentials'; +import { StoredCredentialsCard } from './Credentials/StoredCredentials'; import { ImportCertificate } from './ImportCertificate'; export interface SecurityPanelProps {} export const SecurityPanel: React.FC = (_) => { - const securityCards = [ImportCertificate, StoreCredentialsCard].map((c) => ({ + const securityCards = [ImportCertificate, StoredCredentialsCard].map((c) => ({ key: c.key, title: c.title, description: c.description, diff --git a/src/app/Settings/Config/CredentialsStorage.tsx b/src/app/Settings/Config/CredentialsStorage.tsx deleted file mode 100644 index b3fefd223..000000000 --- a/src/app/Settings/Config/CredentialsStorage.tsx +++ /dev/null @@ -1,112 +0,0 @@ -/* - * Copyright The Cryostat Authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { getFromLocalStorage, saveToLocalStorage } from '@app/utils/LocalStorage'; -import { Select, SelectOption, SelectVariant } from '@patternfly/react-core'; -import * as React from 'react'; -import { useTranslation } from 'react-i18next'; -import { Link } from 'react-router-dom'; -import { SettingTab, UserSetting } from '../types'; - -export interface Location { - key: string; - titleKey: string; - descriptionKey: string; -} - -export class Locations { - static readonly BROWSER_SESSION: Location = { - key: 'Session (Browser Memory)', - titleKey: 'SETTINGS.CREDENTIALS_STORAGE.BROWSER_SESSION.TITLE', - descriptionKey: 'SETTINGS.CREDENTIALS_STORAGE.BROWSER_SESSION.DESCRIPTION', - }; - static readonly BACKEND: Location = { - key: 'Backend', - titleKey: 'SETTINGS.CREDENTIALS_STORAGE.BACKEND.TITLE', - descriptionKey: 'SETTINGS.CREDENTIALS_STORAGE.BACKEND.DESCRIPTION', - }; -} - -const locations = [Locations.BROWSER_SESSION, Locations.BACKEND]; - -const getLocation = (key: string): Location => { - for (const l of locations) { - if (l.key === key) { - return l; - } - } - return Locations.BACKEND; -}; - -const Component = () => { - const [t] = useTranslation(); - const [isExpanded, setExpanded] = React.useState(false); - const [selection, setSelection] = React.useState(Locations.BACKEND.key); - - const handleSelect = React.useCallback( - (_, selection) => { - const location = getLocation(selection.value); - setSelection(location.key); - setExpanded(false); - saveToLocalStorage('CREDENTIAL_LOCATION', selection.value); - }, - [setSelection, setExpanded], - ); - - React.useEffect(() => { - handleSelect(undefined, { value: getFromLocalStorage('CREDENTIAL_LOCATION', Locations.BACKEND.key) }); - }, [handleSelect]); - - return ( - <> - - - ); -}; - -export const CredentialsStorage: UserSetting = { - titleKey: 'SETTINGS.CREDENTIALS_STORAGE.TITLE', - descConstruct: { - key: 'SETTINGS.CREDENTIALS_STORAGE.DESCRIPTION', - parts: [], - }, - content: Component, - category: SettingTab.ADVANCED, -}; diff --git a/src/app/Settings/Settings.tsx b/src/app/Settings/Settings.tsx index 1bbb27d98..356446a95 100644 --- a/src/app/Settings/Settings.tsx +++ b/src/app/Settings/Settings.tsx @@ -40,7 +40,6 @@ import { useLocation, useNavigate } from 'react-router-dom'; import { AutomatedAnalysis } from './Config/AutomatedAnalysis'; import { AutoRefresh } from './Config/AutoRefresh'; import { ChartCards } from './Config/ChartCards'; -import { CredentialsStorage } from './Config/CredentialsStorage'; import { DatetimeControl } from './Config/DatetimeControl'; import { DeletionDialogControl } from './Config/DeletionDialogControl'; import { FeatureLevels } from './Config/FeatureLevels'; @@ -62,7 +61,6 @@ export const Settings: React.FC = (_) => { NotificationControl, AutomatedAnalysis, ChartCards, - CredentialsStorage, DeletionDialogControl, WebSocketDebounce, AutoRefresh, diff --git a/src/app/Shared/Services/AuthCredentials.service.tsx b/src/app/Shared/Services/AuthCredentials.service.tsx deleted file mode 100644 index 95accf910..000000000 --- a/src/app/Shared/Services/AuthCredentials.service.tsx +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Copyright The Cryostat Authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -import { Locations } from '@app/Settings/Config/CredentialsStorage'; -import { getFromLocalStorage } from '@app/utils/LocalStorage'; -import { Observable, of } from 'rxjs'; -import type { ApiService } from './Api.service'; -import { Credential } from './service.types'; - -export class AuthCredentials { - // TODO replace with Redux? - private readonly store = new Map(); - - constructor(private readonly api: () => ApiService) {} - - setCredential(targetId: string, username: string, password: string): Observable { - const location = getFromLocalStorage('CREDENTIAL_LOCATION', Locations.BACKEND.key); - switch (location) { - case Locations.BACKEND.key: - return this.api().postCredentials(`target.connectUrl == "${targetId}"`, username, password); - case Locations.BROWSER_SESSION.key: - this.store.set(targetId, { username, password }); - return of(true); - default: - console.warn('Unknown storage location', location); - return of(false); - } - } - - getCredential(targetId: string): Observable { - const location = getFromLocalStorage('CREDENTIAL_LOCATION', Locations.BACKEND.key); - switch (location) { - case Locations.BACKEND.key: - // if this is stored on the backend then Cryostat should be using those and not prompting us to request from the user - return of(undefined); - case Locations.BROWSER_SESSION.key: - return of(this.store.get(targetId)); - default: - console.warn('Unknown storage location', location); - return of(undefined); - } - } -} diff --git a/src/app/Shared/Services/Services.tsx b/src/app/Shared/Services/Services.tsx index e967c455e..1dd7f78d2 100644 --- a/src/app/Shared/Services/Services.tsx +++ b/src/app/Shared/Services/Services.tsx @@ -15,7 +15,6 @@ */ import * as React from 'react'; import { ApiService } from './Api.service'; -import { AuthCredentials } from './AuthCredentials.service'; import { LoginService } from './Login.service'; import { NotificationChannel } from './NotificationChannel.service'; import { NotificationsInstance } from './Notifications.service'; @@ -28,7 +27,6 @@ export interface Services { target: TargetService; targets: TargetsService; api: ApiService; - authCredentials: AuthCredentials; notificationChannel: NotificationChannel; reports: ReportService; settings: SettingsService; @@ -37,7 +35,6 @@ export interface Services { const target = new TargetService(); const settings = new SettingsService(); -const authCredentials = new AuthCredentials(() => api); const login = new LoginService(settings); const api = new ApiService(target, NotificationsInstance, login); const notificationChannel = new NotificationChannel(NotificationsInstance, login); @@ -48,7 +45,6 @@ const defaultServices: Services = { target, targets, api, - authCredentials, notificationChannel, reports, settings, diff --git a/src/app/Topology/Actions/CreateTarget.tsx b/src/app/Topology/Actions/CreateTarget.tsx index cc62f41fd..fc08e56f7 100644 --- a/src/app/Topology/Actions/CreateTarget.tsx +++ b/src/app/Topology/Actions/CreateTarget.tsx @@ -16,7 +16,6 @@ import openjdkSvg from '@app/assets/openjdk.svg'; import { BreadcrumbPage } from '@app/BreadcrumbPage/BreadcrumbPage'; -import { Locations } from '@app/Settings/Config/CredentialsStorage'; import { LinearDotSpinner } from '@app/Shared/Components/LinearDotSpinner'; import { LoadingProps } from '@app/Shared/Components/types'; import { Target } from '@app/Shared/Services/api.types'; @@ -24,7 +23,6 @@ import { isHttpOk } from '@app/Shared/Services/api.utils'; import { ServiceContext } from '@app/Shared/Services/Services'; import '@app/Topology/styles/base.css'; import { useSubscriptions } from '@app/utils/hooks/useSubscriptions'; -import { getFromLocalStorage } from '@app/utils/LocalStorage'; import { getAnnotation, portalRoot } from '@app/utils/utils'; import { Accordion, @@ -171,7 +169,6 @@ export const CreateTarget: React.FC = ({ prefilled }) => { const handleSubmit = React.useCallback(() => { setLoading(true); // Get storage location - const locationKey = getFromLocalStorage('CREDENTIAL_LOCATION', Locations.BACKEND.key); addSubscription( context.api .createTarget( @@ -180,7 +177,7 @@ export const CreateTarget: React.FC = ({ prefilled }) => { alias: alias.trim() || connectUrl, }, credentials, - locationKey === Locations.BACKEND.key, + true, ) .subscribe(({ status, body }) => { setLoading(false); diff --git a/src/test/SecurityPanel/Credentials/StoreCredentials.test.tsx b/src/test/SecurityPanel/Credentials/StoreCredentials.test.tsx deleted file mode 100644 index ce7bfe824..000000000 --- a/src/test/SecurityPanel/Credentials/StoreCredentials.test.tsx +++ /dev/null @@ -1,432 +0,0 @@ -/* - * Copyright The Cryostat Authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -import { DeleteCredentials, DeleteOrDisableWarningType } from '@app/Modal/types'; -import { CreateCredentialModalProps } from '@app/SecurityPanel/Credentials/CreateCredentialModal'; -import { StoreCredentials } from '@app/SecurityPanel/Credentials/StoreCredentials'; -import { StoredCredential, Target, MatchedCredential, NotificationMessage } from '@app/Shared/Services/api.types'; -import { defaultServices } from '@app/Shared/Services/Services'; -import { Modal, ModalVariant } from '@patternfly/react-core'; -import { cleanup, screen, within } from '@testing-library/react'; -import { of, throwError } from 'rxjs'; -import { render } from '../../utils'; - -const mockCredential: StoredCredential = { - id: 0, - matchExpression: 'target.connectUrl == "service:jmx:rmi://someUrl"', - numMatchingTargets: 1, -}; -const mockAnotherCredential: StoredCredential = { - id: 1, - matchExpression: - 'target.connectUrl == "service:jmx:rmi://anotherUrl" || target.connectUrl == "service:jmx:rmi://anotherMatchUrl" || target.connectUrl == "service:jmx:rmi://yetAnotherMatchUrl"', - numMatchingTargets: 2, -}; - -const mockTarget: Target = { - connectUrl: 'service:jmx:rmi://someUrl', - alias: 'someAlias', - labels: [], - annotations: { cryostat: [], platform: [] }, -}; -const mockAnotherTarget: Target = { - connectUrl: 'service:jmx:rmi://anotherUrl', - alias: 'anotherAlias', - labels: [], - annotations: { cryostat: [], platform: [] }, -}; -const mockAnotherMatchingTarget: Target = { - connectUrl: 'service:jmx:rmi://anotherMatchUrl', - alias: 'anotherMatchAlias', - labels: [], - annotations: { cryostat: [], platform: [] }, -}; -const mockYetAnotherMatchingTarget: Target = { - connectUrl: 'service:jmx:rmi://yetAnotherMatchUrl', - alias: 'yetAnotherMatchAlias', - labels: [], - annotations: { cryostat: [], platform: [] }, -}; - -const mockMatchedCredentialResponse: MatchedCredential = { - matchExpression: mockCredential.matchExpression, - targets: [mockTarget], -}; -const mockAnotherMatchedCredentialResponse: MatchedCredential = { - matchExpression: mockAnotherCredential.matchExpression, - targets: [mockAnotherTarget, mockAnotherMatchingTarget], -}; - -const mockCredentialNotification = { message: mockCredential } as NotificationMessage; - -const mockLostTargetNotification = { - message: { event: { kind: 'LOST', serviceRef: mockAnotherTarget } }, -} as NotificationMessage; - -const mockFoundTargetNotification = { - message: { event: { kind: 'FOUND', serviceRef: mockYetAnotherMatchingTarget } }, -} as NotificationMessage; - -jest.mock('@app/SecurityPanel/Credentials/CreateCredentialModal', () => { - return { - CreateCredentialModal: jest.fn((props: CreateCredentialModalProps) => { - return ( - - Auth Form + Match Expression Visualizer - - ); - }), - }; -}); - -jest - .spyOn(defaultServices.notificationChannel, 'messages') - .mockReturnValueOnce(of(mockCredentialNotification)) // adds the correct table entry when a stored notification is received - .mockReturnValueOnce(of()) - .mockReturnValueOnce(of()) - - .mockReturnValueOnce(of()) // removes the correct table entry when a deletion notification is received - .mockReturnValueOnce(of(mockCredentialNotification)) - .mockReturnValueOnce(of()) - - .mockReturnValueOnce(of()) // renders an empty table after receiving deletion notifications for all credentials' - .mockReturnValueOnce(of(mockCredentialNotification)) - .mockReturnValueOnce(of()) - - .mockReturnValueOnce(of()) // expands to show the correct nested targets - .mockReturnValueOnce(of()) - .mockReturnValueOnce(of()) - .mockReturnValueOnce(of()) - .mockReturnValueOnce(of()) - - .mockReturnValueOnce(of()) // decrements the correct count and updates the correct nested table when a lost target notification is received - .mockReturnValueOnce(of()) - .mockReturnValueOnce(of(mockLostTargetNotification)) - .mockReturnValueOnce(of()) - .mockReturnValueOnce(of()) - - .mockReturnValueOnce(of()) // increments the correct count and updates the correct nested table when a found target notification is received - .mockReturnValueOnce(of()) - .mockReturnValueOnce(of(mockFoundTargetNotification)) - .mockReturnValueOnce(of()) - .mockReturnValueOnce(of()) - - .mockReturnValue(of()); // remaining tests - -jest.spyOn(defaultServices.api, 'deleteCredentials').mockReturnValue(of(true)); -const apiRequestSpy = jest - .spyOn(defaultServices.api, 'getCredentials') - .mockReturnValueOnce(of([])) // adds the correct table entry when a stored notification is received - - .mockReturnValueOnce(of([mockCredential, mockAnotherCredential])) // removes the correct table entry when a deletion notification is received - - .mockReturnValueOnce(of([mockCredential])) // renders an empty table after receiving deletion notifications for all credentials - - .mockReturnValueOnce(of([mockCredential, mockAnotherCredential])) // expands to show the correct nested targets - - .mockReturnValueOnce(of([mockCredential, mockAnotherCredential])) // decrements the correct count and updates the correct nested table when a lost target notification is received - .mockReturnValueOnce( - of([ - mockCredential, - { ...mockAnotherCredential, numMatchingTargets: mockAnotherCredential.numMatchingTargets - 1 }, - ]), - ) - - .mockReturnValueOnce(of([mockCredential, mockAnotherCredential])) // increments the correct count and updates the correct nested table when a found target notification is received - .mockReturnValueOnce( - of([ - mockCredential, - { ...mockAnotherCredential, numMatchingTargets: mockAnotherCredential.numMatchingTargets + 1 }, - ]), - ) - - .mockReturnValueOnce(of([])) // opens the auth modal when Add is clicked - - .mockReturnValueOnce(of([mockCredential, mockAnotherCredential])) // shows a popup when Delete is clicked and makes a delete request when deleting one credential after confirming Delete - - .mockReturnValueOnce(of([mockCredential, mockAnotherCredential])) // makes multiple delete requests when all credentials are deleted at once - - .mockReturnValue(throwError(() => new Error('Too many calls'))); - -jest - .spyOn(defaultServices.api, 'getCredential') - .mockReturnValueOnce(of(mockMatchedCredentialResponse)) // expands to show the correct nested targets - .mockReturnValueOnce(of(mockAnotherMatchedCredentialResponse)) - - .mockReturnValueOnce(of(mockMatchedCredentialResponse)) // decrements the correct count and updates the correct nested table when a lost target notification is received - .mockReturnValueOnce(of({ ...mockAnotherMatchedCredentialResponse, targets: [mockAnotherMatchingTarget] })) - - .mockReturnValueOnce(of(mockMatchedCredentialResponse)) // increments the correct count and updates the correct nested table when a found target notification is received - .mockReturnValueOnce( - of({ - ...mockAnotherMatchedCredentialResponse, - targets: mockAnotherMatchedCredentialResponse.targets.concat(mockYetAnotherMatchingTarget), - }), - ) - - .mockReturnValue(throwError(() => new Error('Too many calls'))); - -jest.spyOn(defaultServices.settings, 'deletionDialogsEnabledFor').mockReturnValueOnce(true); - -describe('', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - - afterEach(cleanup); - - it('adds the correct table entry when a stored notification is received', async () => { - render({ - routerConfigs: { - routes: [ - { - path: '/security', - element: , - }, - ], - }, - }); - - expect(screen.getByText(mockCredential.matchExpression)).toBeInTheDocument(); - expect(screen.getByText(mockCredential.numMatchingTargets)).toBeInTheDocument(); - expect(apiRequestSpy).toHaveBeenCalledTimes(1); - }); - - it('removes the correct table entry when a deletion notification is received', async () => { - render({ - routerConfigs: { - routes: [ - { - path: '/security', - element: , - }, - ], - }, - }); - - expect(screen.queryByText(mockCredential.matchExpression)).not.toBeInTheDocument(); - expect(screen.queryByText(mockCredential.numMatchingTargets)).not.toBeInTheDocument(); - expect(screen.getByText(mockAnotherCredential.matchExpression)).toBeInTheDocument(); - expect(screen.getByText(mockAnotherCredential.numMatchingTargets)).toBeInTheDocument(); - expect(apiRequestSpy).toHaveBeenCalledTimes(1); - }); - - it('renders an empty table after receiving deletion notifications for all credentials', async () => { - const apiRequestSpy = jest.spyOn(defaultServices.api, 'getCredentials'); - render({ - routerConfigs: { - routes: [ - { - path: '/security', - element: , - }, - ], - }, - }); - - expect(screen.queryByText(mockCredential.matchExpression)).not.toBeInTheDocument(); - expect(screen.queryByText(mockCredential.numMatchingTargets)).not.toBeInTheDocument(); - expect(screen.queryByText(mockAnotherCredential.matchExpression)).not.toBeInTheDocument(); - expect(screen.queryByText(mockAnotherCredential.numMatchingTargets)).not.toBeInTheDocument(); - expect(screen.getByText('No Stored Credentials')).toBeInTheDocument(); - expect(apiRequestSpy).toHaveBeenCalledTimes(1); - }); - - it('expands to show the correct nested targets', async () => { - const { user } = render({ - routerConfigs: { - routes: [ - { - path: '/security', - element: , - }, - ], - }, - }); - - const targets = [mockTarget, mockAnotherTarget, mockAnotherMatchingTarget]; - targets.forEach(({ alias, connectUrl }) => { - expect(screen.queryByText(`${alias} (${connectUrl})`)).not.toBeInTheDocument(); - }); - expect(screen.queryByText('Target')).not.toBeInTheDocument(); - - const expandButtons = screen.getAllByLabelText('Details'); - expect(expandButtons.length).toBe(2); - - await user.click(expandButtons[0]); - - expect(screen.getByText(`${mockTarget.alias} (${mockTarget.connectUrl})`)).toBeInTheDocument(); - expect(screen.getByText(`${mockTarget.alias} (${mockTarget.connectUrl})`)).toBeVisible(); - - const otherTargets = targets.filter((t) => t.connectUrl !== mockTarget.connectUrl); - otherTargets.forEach(({ alias, connectUrl }) => - expect(screen.queryByText(`${alias} (${connectUrl})`)).not.toBeInTheDocument(), - ); - - await user.click(expandButtons[1]); - - otherTargets.forEach(({ alias, connectUrl }) => { - const element = screen.getByText(`${alias} (${connectUrl})`); - expect(element).toBeInTheDocument(); - expect(element).toBeVisible(); - }); - }); - - it('decrements the correct count and updates the correct nested table when a lost target notification is received', async () => { - const { user } = render({ - routerConfigs: { - routes: [ - { - path: '/security', - element: , - }, - ], - }, - }); - - // both counts should now be equal to 1 - const counts = screen.getAllByText('1'); - expect(counts.length).toBe(2); - - const expandButtons = screen.getAllByLabelText('Details'); - - await user.click(expandButtons[0]); - - expect(screen.getByText(`${mockTarget.alias} (${mockTarget.connectUrl})`)).toBeInTheDocument(); - - await user.click(expandButtons[1]); - - expect(screen.queryByText(`${mockAnotherTarget.alias} (${mockAnotherTarget.connectUrl})`)).not.toBeInTheDocument(); - - const element = screen.getByText(`${mockAnotherMatchingTarget.alias} (${mockAnotherMatchingTarget.connectUrl})`); - expect(element).toBeInTheDocument(); - expect(element).toBeVisible(); - }); - - it('increments the correct count and updates the correct nested table when a found target notification is received', async () => { - const { user } = render({ - routerConfigs: { - routes: [ - { - path: '/security', - element: , - }, - ], - }, - }); - - expect(screen.getByText(mockCredential.numMatchingTargets)).toBeInTheDocument(); - expect(screen.getByText(mockAnotherCredential.numMatchingTargets + 1)).toBeInTheDocument(); - - const expandButtons = screen.getAllByLabelText('Details'); - - await user.click(expandButtons[0]); - - expect(screen.getByText(`${mockTarget.alias} (${mockTarget.connectUrl})`)).toBeInTheDocument(); - expect( - screen.queryByText(`${mockYetAnotherMatchingTarget.alias} (${mockYetAnotherMatchingTarget.connectUrl})`), - ).not.toBeInTheDocument(); - - await user.click(expandButtons[1]); - - [mockAnotherTarget, mockAnotherMatchingTarget, mockYetAnotherMatchingTarget].forEach(({ alias, connectUrl }) => { - const element = screen.getByText(`${alias} (${connectUrl})`); - expect(element).toBeInTheDocument(); - expect(element).toBeVisible(); - }); - }); - - it('opens the auth modal when Add is clicked', async () => { - const { user } = render({ - routerConfigs: { - routes: [ - { - path: '/security', - element: , - }, - ], - }, - }); - - await user.click(screen.getByText('Add')); - expect(screen.getByText('CreateCredentialModal')).toBeInTheDocument(); - - await user.click(screen.getByLabelText('Close')); - expect(screen.queryByText('CreateCredentialModal')).not.toBeInTheDocument(); - }); - - it('shows a popup when Delete is clicked and makes a delete request when deleting one credential after confirming Delete', async () => { - const deleteRequestSpy = jest.spyOn(defaultServices.api, 'deleteCredentials'); - const { user } = render({ - routerConfigs: { - routes: [ - { - path: '/security', - element: , - }, - ], - }, - }); - - expect(screen.getByText(mockCredential.matchExpression)).toBeInTheDocument(); - expect(screen.getByText(mockAnotherCredential.matchExpression)).toBeInTheDocument(); - - await user.click(screen.getByLabelText('credentials-table-row-0-check')); - await user.click(screen.getByText('Delete')); - - expect(screen.getByLabelText(DeleteCredentials.ariaLabel)).toBeInTheDocument(); - - const dialogWarningSpy = jest.spyOn(defaultServices.settings, 'setDeletionDialogsEnabledFor'); - await user.click(screen.getByLabelText("Don't ask me again")); - await user.click(within(screen.getByLabelText(DeleteCredentials.ariaLabel)).getByText('Delete')); - - expect(dialogWarningSpy).toBeCalledTimes(1); - expect(dialogWarningSpy).toBeCalledWith(DeleteOrDisableWarningType.DeleteCredentials, false); - - expect(deleteRequestSpy).toHaveBeenCalledTimes(1); - }); - - it('makes multiple delete requests when all credentials are deleted at once w/o popup warning', async () => { - const deleteRequestSpy = jest.spyOn(defaultServices.api, 'deleteCredentials'); - const { user } = render({ - routerConfigs: { - routes: [ - { - path: '/security', - element: , - }, - ], - }, - }); - - expect(screen.getByText(mockCredential.matchExpression)).toBeInTheDocument(); - expect(screen.getByText(mockCredential.numMatchingTargets)).toBeInTheDocument(); - expect(screen.getByText(mockAnotherCredential.matchExpression)).toBeInTheDocument(); - expect(screen.getByText(mockAnotherCredential.numMatchingTargets)).toBeInTheDocument(); - - const checkboxes = screen.getAllByRole('checkbox'); - const selectAllCheck = checkboxes[0]; - await user.click(selectAllCheck); - await user.click(screen.getByText('Delete')); - - expect(deleteRequestSpy).toHaveBeenCalledTimes(2); - }); -}); diff --git a/src/test/Settings/CredentialsStorage.test.tsx b/src/test/Settings/CredentialsStorage.test.tsx deleted file mode 100644 index 110f37cad..000000000 --- a/src/test/Settings/CredentialsStorage.test.tsx +++ /dev/null @@ -1,97 +0,0 @@ -/* - * Copyright The Cryostat Authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -/* eslint @typescript-eslint/no-explicit-any: 0 */ -import { CredentialsStorage, Locations } from '@app/Settings/Config/CredentialsStorage'; -import { getFromLocalStorage, saveToLocalStorage } from '@app/utils/LocalStorage'; -import { cleanup, screen, waitFor, within } from '@testing-library/react'; -import * as React from 'react'; -import { render, testT } from '../utils'; - -jest.mock('@app/utils/LocalStorage', () => { - const map = new Map(); - return { - getFromLocalStorage: jest.fn((key: any, defaultValue: any): any => { - if (map.has(key)) { - return map.get(key); - } - return defaultValue; - }), - - saveToLocalStorage: jest.fn((key: any, value: any): any => { - map.set(key, value); - }), - }; -}); - -const storageKey = 'CREDENTIAL_LOCATION'; - -describe('', () => { - afterEach(cleanup); - - it('defaults to Backend storage', async () => { - render({ - routerConfigs: { - routes: [ - { - path: '/settings', - element: React.createElement(CredentialsStorage.content, null), - }, - ], - }, - }); - - expect(getFromLocalStorage).toHaveBeenCalledTimes(1); - expect(saveToLocalStorage).toHaveBeenCalledTimes(1); - expect(saveToLocalStorage).lastCalledWith(storageKey, Locations.BACKEND.key); - - expect(screen.getByText(testT(Locations.BACKEND.titleKey))).toBeVisible(); - expect(screen.queryByText(testT(Locations.BROWSER_SESSION.titleKey))).toBeFalsy(); - }); - - it.skip('sets value to local storage when dropdown is clicked', async () => { - const { user } = render({ - routerConfigs: { - routes: [ - { - path: '/settings', - element: React.createElement(CredentialsStorage.content, null), - }, - ], - }, - }); - - expect(getFromLocalStorage).toHaveBeenCalledTimes(1); - expect(saveToLocalStorage).toHaveBeenCalledTimes(1); - expect(saveToLocalStorage).lastCalledWith(storageKey, Locations.BACKEND.key); - - await user.click(screen.getByRole('button')); - - // the default is Backend storage. Click the dropdown and select Session (Browser Memory) to change selection - const ul = await screen.findByRole('listbox'); - const browserSession = within(ul).getByText(testT(Locations.BROWSER_SESSION.titleKey)); - await user.click(browserSession); - - await waitFor(() => expect(ul).not.toBeVisible()); // expect selection menu to close after user clicks an option - - // expect the selection to be visible, the other not - expect(screen.getByText(testT(Locations.BROWSER_SESSION.titleKey))).toBeVisible(); - expect(screen.queryByText(testT(Locations.BACKEND.titleKey))).toBeFalsy(); - - expect(getFromLocalStorage).toHaveBeenCalledTimes(1); - expect(saveToLocalStorage).toHaveBeenCalledTimes(2); - expect(saveToLocalStorage).lastCalledWith(storageKey, Locations.BROWSER_SESSION.key); - }); -}); diff --git a/src/test/Settings/Settings.test.tsx b/src/test/Settings/Settings.test.tsx index fd447911f..bd8edd127 100644 --- a/src/test/Settings/Settings.test.tsx +++ b/src/test/Settings/Settings.test.tsx @@ -52,18 +52,6 @@ jest.mock('@app/Settings/Config/ChartCards', () => ({ } as UserSetting, })); -jest.mock('@app/Settings/Config/CredentialsStorage', () => ({ - CredentialsStorage: { - titleKey: 'SETTINGS.CREDENTIALS_STORAGE.TITLE', - descConstruct: { - key: 'SETTINGS.CREDENTIALS_STORAGE.DESCRIPTION', - parts: [], // Just raw string (avoid using React component during mock) - }, - category: 'SETTINGS.CATEGORIES.ADVANCED', - content: () => Credentials Storage Component, - } as UserSetting, -})); - jest.mock('@app/Settings/Config/DeletionDialogControl', () => ({ DeletionDialogControl: { titleKey: 'SETTINGS.DELETION_DIALOG_CONTROL.TITLE', diff --git a/src/test/Shared/Services/JmxCredentials.service.test.tsx b/src/test/Shared/Services/JmxCredentials.service.test.tsx deleted file mode 100644 index c2f4e956c..000000000 --- a/src/test/Shared/Services/JmxCredentials.service.test.tsx +++ /dev/null @@ -1,117 +0,0 @@ -/* - * Copyright The Cryostat Authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -/* eslint @typescript-eslint/no-explicit-any: 0 */ -import { ApiService } from '@app/Shared/Services/Api.service'; -import { AuthCredentials } from '@app/Shared/Services/AuthCredentials.service'; -import { getFromLocalStorage, saveToLocalStorage } from '@app/utils/LocalStorage'; -import { firstValueFrom, Observable, of } from 'rxjs'; - -jest.mock('@app/utils/LocalStorage', () => { - const map = new Map(); - return { - getFromLocalStorage: jest.fn((key: any, defaultValue: any): any => { - if (map.has(key)) { - return map.get(key); - } - return defaultValue; - }), - - saveToLocalStorage: jest.fn((key: any, value: any): any => { - map.set(key, value); - }), - }; -}); - -const mockApi = { - postCredentials: (_expr: string, _user: string, _pass: string): Observable => of(true), -} as ApiService; -const postCredentialsSpy = jest.spyOn(mockApi, 'postCredentials'); - -describe('AuthCredentials.service', () => { - let svc: AuthCredentials; - beforeEach(() => { - saveToLocalStorage('CREDENTIAL_LOCATION', undefined); - postCredentialsSpy.mockReset(); - svc = new AuthCredentials(() => mockApi); - }); - - it('should not check storage on instantiation', () => { - expect(getFromLocalStorage).not.toHaveBeenCalled(); - }); - - describe('with invalid location selected in storage', () => { - beforeEach(() => { - saveToLocalStorage('CREDENTIAL_LOCATION', 'BAD_LOCATION'); - }); - - it('retrieves undefined credentials from memory map', async () => { - const cred = await firstValueFrom(svc.getCredential('myTarget')); - expect(getFromLocalStorage).toHaveBeenCalled(); - expect(cred).toBeFalsy(); - expect(postCredentialsSpy).not.toHaveBeenCalled(); - }); - - it('retrieves previously defined credentials from memory map', async () => { - svc.setCredential('myTarget', 'foouser', 'foopass'); - expect(getFromLocalStorage).toHaveBeenCalledTimes(1); - const cred = await firstValueFrom(svc.getCredential('myTarget')); - expect(getFromLocalStorage).toHaveBeenCalledTimes(2); - expect(cred).toBeUndefined(); - expect(postCredentialsSpy).not.toHaveBeenCalled(); - }); - }); - - describe('with session selected in storage', () => { - beforeEach(() => { - saveToLocalStorage('CREDENTIAL_LOCATION', 'Session (Browser Memory)'); - }); - - it('retrieves undefined credentials from memory map', async () => { - const cred = await firstValueFrom(svc.getCredential('myTarget')); - expect(getFromLocalStorage).toHaveBeenCalledTimes(1); - expect(cred).toBeUndefined(); - expect(postCredentialsSpy).not.toHaveBeenCalled(); - }); - - it('retrieves previously defined credentials from memory map', async () => { - svc.setCredential('myTarget', 'foouser', 'foopass'); - expect(getFromLocalStorage).toHaveBeenCalledTimes(1); - const cred = await firstValueFrom(svc.getCredential('myTarget')); - expect(getFromLocalStorage).toHaveBeenCalledTimes(2); - expect(cred).toBeDefined(); - expect(cred?.username).toEqual('foouser'); - expect(cred?.password).toEqual('foopass'); - expect(postCredentialsSpy).not.toHaveBeenCalled(); - }); - }); - - describe('with backend selected in storage', () => { - beforeEach(() => { - saveToLocalStorage('CREDENTIAL_LOCATION', 'Backend'); - }); - - it('does not retrieve credentials', async () => { - const cred = await firstValueFrom(svc.getCredential('myTarget')); - expect(cred).toBeUndefined(); - expect(postCredentialsSpy).not.toHaveBeenCalled(); - }); - - it('POSTs credentials to API service', async () => { - svc.setCredential('myTarget', 'foouser', 'foopass'); - expect(postCredentialsSpy).toHaveBeenCalled(); - }); - }); -});