From cf5d3a72585c0c2e2267e7f0ba1079e666a19814 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 13 Oct 2023 16:22:12 -0400 Subject: [PATCH] feat(jvmid): enhance UI hints when JVM ID cannot be generated (#1119) (cherry picked from commit ba88f7103606a377c56a03696f441c06bdcc4672) --- .../SecurityPanel/CertificateUploadModal.tsx | 2 +- .../Credentials/StoreCredentials.tsx | 34 ++++++++--- src/app/SecurityPanel/ImportCertificate.tsx | 23 ++++++- src/app/SecurityPanel/SecurityPanel.tsx | 5 +- src/app/SecurityPanel/types.ts | 5 +- src/app/Shared/Components/FileUploads.tsx | 2 +- .../Shared/Components/JmxAuthDescription.tsx | 44 ++++++++++++++ .../Shared/Components/JmxSslDescription.tsx | 54 +++++++++++++++++ src/app/Shared/Components/types.ts | 4 ++ src/app/Topology/Entity/EntityDetails.tsx | 22 +++---- src/app/Topology/Shared/utils.tsx | 60 +++++++++++++------ src/test/Agent/AgentProbeTemplates.test.tsx | 2 +- src/test/Events/EventTemplates.test.tsx | 2 +- .../ArchivedRecordingsTable.test.tsx | 8 +-- src/test/Rules/Rules.test.tsx | 4 +- 15 files changed, 218 insertions(+), 53 deletions(-) create mode 100644 src/app/Shared/Components/JmxAuthDescription.tsx create mode 100644 src/app/Shared/Components/JmxSslDescription.tsx diff --git a/src/app/SecurityPanel/CertificateUploadModal.tsx b/src/app/SecurityPanel/CertificateUploadModal.tsx index 7226d09e3..e4b1f8a5d 100644 --- a/src/app/SecurityPanel/CertificateUploadModal.tsx +++ b/src/app/SecurityPanel/CertificateUploadModal.tsx @@ -121,7 +121,7 @@ export const CertificateUploadModal: React.FC = ({ showClose={true} onClose={handleClose} title="Upload SSL certificate" - description="Select a certificate file to upload. Certificates must be DER-encoded (can be binary or base64) and can have .der or .cer extensions." + description="Select a certificate file to upload. Certificates must be DER-encoded (can be binary or base64) and should have .der or .cer extensions." >
diff --git a/src/app/SecurityPanel/Credentials/StoreCredentials.tsx b/src/app/SecurityPanel/Credentials/StoreCredentials.tsx index 8d360978c..395b88d7e 100644 --- a/src/app/SecurityPanel/Credentials/StoreCredentials.tsx +++ b/src/app/SecurityPanel/Credentials/StoreCredentials.tsx @@ -15,6 +15,7 @@ */ import { DeleteWarningModal } from '@app/Modal/DeleteWarningModal'; import { DeleteOrDisableWarningType } from '@app/Modal/types'; +import { JmxAuthDescription } from '@app/Shared/Components/JmxAuthDescription'; import { LoadingView } from '@app/Shared/Components/LoadingView'; import { StoredCredential, NotificationCategory } from '@app/Shared/Services/api.types'; import { ServiceContext } from '@app/Shared/Services/Services'; @@ -31,13 +32,18 @@ import { DropdownToggleCheckbox, EmptyState, EmptyStateIcon, + Icon, + Popover, Text, + TextContent, + TextVariants, Title, Toolbar, ToolbarContent, ToolbarItem, + Tooltip, } from '@patternfly/react-core'; -import { SearchIcon } from '@patternfly/react-icons'; +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'; @@ -516,15 +522,27 @@ export const CheckBoxActions: React.FC = ({ }; export const StoreCredentialsCard: SecurityCard = { - title: 'Store Credentials', - description: ( + key: 'credentials', + title: ( - Credentials that Cryostat uses to connect to Cryostat agents or target JVMs over JMX are stored here. These are - stored in encrypted storage managed by the Cryostat backend. These credentials may be used for manually managing - recordings and event templates on target JVMs, as well as for Automated Rules which run in the background and open - unattended target connections. Any locally-stored client credentials held by your browser session are not - displayed here. See Settings to configure locally-stored credentials. + Store Credentials + }> + + ), + description: ( + + + Credentials that Cryostat uses to connect to Cryostat agents or target JVMs over JMX are stored here. These are + stored in encrypted storage managed by the Cryostat backend. These credentials may be used for manually managing + recordings and event templates on target JVMs, as well as for Automated Rules which run in the background and + open unattended target connections. Any locally-stored client credentials held by your browser session are not + displayed here. See Settings to configure locally-stored credentials. + + + ), content: StoreCredentials, }; diff --git a/src/app/SecurityPanel/ImportCertificate.tsx b/src/app/SecurityPanel/ImportCertificate.tsx index f608a40fa..9dfa6b996 100644 --- a/src/app/SecurityPanel/ImportCertificate.tsx +++ b/src/app/SecurityPanel/ImportCertificate.tsx @@ -14,7 +14,9 @@ * limitations under the License. */ -import { Button } from '@patternfly/react-core'; +import { JmxSslDescription } from '@app/Shared/Components/JmxSslDescription'; +import { Button, Icon, Popover, Text, TextContent, TextVariants, Tooltip } from '@patternfly/react-core'; +import { OutlinedQuestionCircleIcon } from '@patternfly/react-icons'; import * as React from 'react'; import { CertificateUploadModal } from './CertificateUploadModal'; import { SecurityCard } from './types'; @@ -37,7 +39,22 @@ export const CertificateImport: React.FC = () => { }; export const ImportCertificate: SecurityCard = { - title: 'Import SSL Certificates', - description: 'The Cryostat server must be restarted in order to reload the certificate store.', + key: 'ssl', + title: ( + + Import SSL Certificates + }> + + + + ), + description: ( + + Add SSL certificates to the Cryostat server truststore. + You must restart the Cryostat server to reload the certificate store. + + ), content: CertificateImport, }; diff --git a/src/app/SecurityPanel/SecurityPanel.tsx b/src/app/SecurityPanel/SecurityPanel.tsx index 877979960..80644ce30 100644 --- a/src/app/SecurityPanel/SecurityPanel.tsx +++ b/src/app/SecurityPanel/SecurityPanel.tsx @@ -23,6 +23,7 @@ export interface SecurityPanelProps {} export const SecurityPanel: React.FC = (_) => { const securityCards = [ImportCertificate, StoreCredentialsCard].map((c) => ({ + key: c.key, title: c.title, description: c.description, element: React.createElement(c.content, null), @@ -31,10 +32,10 @@ export const SecurityPanel: React.FC = (_) => { return ( {securityCards.map((s) => ( - + {s.title} - {s.description} + {s.description} {s.element} diff --git a/src/app/SecurityPanel/types.ts b/src/app/SecurityPanel/types.ts index 2a83da277..195ab40ec 100644 --- a/src/app/SecurityPanel/types.ts +++ b/src/app/SecurityPanel/types.ts @@ -15,7 +15,8 @@ */ export interface SecurityCard { - title: string; - description: JSX.Element | string; + key: string; + title: JSX.Element; + description: JSX.Element; content: React.FC; } diff --git a/src/app/Shared/Components/FileUploads.tsx b/src/app/Shared/Components/FileUploads.tsx index b42600251..1ea31d5c2 100644 --- a/src/app/Shared/Components/FileUploads.tsx +++ b/src/app/Shared/Components/FileUploads.tsx @@ -276,7 +276,7 @@ export const MultiFileUpload: React.FC = ({ > } - titleText={titleText || 'Drag and drop files here'} + titleText={titleText || 'Drag a file here'} titleTextSeparator={titleTextSeparator || 'or'} infoText={infoText || `Accepted file types: ${displayAccepts.join(', ')}`} /> diff --git a/src/app/Shared/Components/JmxAuthDescription.tsx b/src/app/Shared/Components/JmxAuthDescription.tsx new file mode 100644 index 000000000..118855c35 --- /dev/null +++ b/src/app/Shared/Components/JmxAuthDescription.tsx @@ -0,0 +1,44 @@ +/* + * 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 { Text, TextContent, TextList, TextListItem, TextVariants } from '@patternfly/react-core'; +import * as React from 'react'; +import { DescriptionProps } from './types'; + +export const JmxAuthDescription: React.FC> = ({ children }) => { + return ( + + {children} + + JVM applications can be configured to require clients, such as Cryostat, to pass a challenge based + authentication before establishing a connection. + + + Check the deployment configuration of your JVM application for system properties such as: + + + + com.sun.management.jmxremote.authenticate + + + com.sun.management.jmxremote.password.file + + + com.sun.management.jmxremote.login.config + + + + ); +}; diff --git a/src/app/Shared/Components/JmxSslDescription.tsx b/src/app/Shared/Components/JmxSslDescription.tsx new file mode 100644 index 000000000..8fb138ae9 --- /dev/null +++ b/src/app/Shared/Components/JmxSslDescription.tsx @@ -0,0 +1,54 @@ +/* + * 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 { Text, TextContent, TextList, TextListItem, TextVariants } from '@patternfly/react-core'; +import * as React from 'react'; +import { DescriptionProps } from './types'; + +export const JmxSslDescription: React.FC> = ({ children }) => { + return ( + + {children} + + JVM applications can be configured to present an SSL certificate for incoming JMX connections. Clients, such as + Cryostat, should be configured to trust these certificates so that the origin and authenticity of the connection + data can be verified. + + + Check the deployment configuration of your JVM application for system properties such as: + + + + javax.net.ssl.keyStore + + + javax.net.ssl.keyStorePassword + + + com.sun.management.jmxremote.ssl.need.client.auth + + + javax.net.ssl.trustStore + + + javax.net.ssl.trustStorePassword + + + com.sun.management.jmxremote.registry.ssl + + + + ); +}; diff --git a/src/app/Shared/Components/types.ts b/src/app/Shared/Components/types.ts index 98052db17..149e9df74 100644 --- a/src/app/Shared/Components/types.ts +++ b/src/app/Shared/Components/types.ts @@ -20,3 +20,7 @@ export interface LoadingProps { spinnerAriaLabel?: string; // Accessible label for the spinner to describe what is loading isLoading: boolean; } + +export type DescriptionProps = { + children?: React.ReactNode; +}; diff --git a/src/app/Topology/Entity/EntityDetails.tsx b/src/app/Topology/Entity/EntityDetails.tsx index fb84f404e..8fd419964 100644 --- a/src/app/Topology/Entity/EntityDetails.tsx +++ b/src/app/Topology/Entity/EntityDetails.tsx @@ -191,7 +191,7 @@ export const TargetDetails: React.FC<{ <> JVM ID {!serviceRef.jvmId && ( - + )} @@ -201,7 +201,7 @@ export const TargetDetails: React.FC<{ <> JVM ID {!serviceRef.jvmId && ( - + )} @@ -666,15 +666,17 @@ export const EntityDetailHeader: React.FC = ({ actionClose={ setShowBanner(false)} />} > - {extra?.description} {extra?.callForAction && !alertOptions.hideActions ? ( - - - {extra.callForAction.map((action, index) => ( - {action} - ))} - - + <> + {extra?.description} + + + {extra.callForAction.map((action, index) => ( + {action} + ))} + + + ) : null} diff --git a/src/app/Topology/Shared/utils.tsx b/src/app/Topology/Shared/utils.tsx index 77e823f31..fa5699cc1 100644 --- a/src/app/Topology/Shared/utils.tsx +++ b/src/app/Topology/Shared/utils.tsx @@ -14,14 +14,23 @@ * limitations under the License. */ +import { JmxAuthDescription } from '@app/Shared/Components/JmxAuthDescription'; +import { JmxSslDescription } from '@app/Shared/Components/JmxSslDescription'; import { TopologyFilters } from '@app/Shared/Redux/Filters/TopologyFilterSlice'; import { NodeType, EnvironmentNode, TargetNode } from '@app/Shared/Services/api.types'; import { DEFAULT_EMPTY_UNIVERSE, isTargetNode } from '@app/Shared/Services/api.utils'; -import { Button, Text, TextVariants } from '@patternfly/react-core'; -import { GraphElement, NodeStatus } from '@patternfly/react-topology'; +import { + Button, + Text, + TextVariants, + DescriptionListTermHelpText, + DescriptionListTermHelpTextButton, + Popover, +} from '@patternfly/react-core'; +import { NodeStatus } from '@patternfly/react-topology'; import * as React from 'react'; import { WarningResolverAsCredModal, WarningResolverAsLink } from '../Actions/WarningResolver'; -import { ListElement, StatusExtra } from './types'; +import { GraphElement, ListElement, StatusExtra } from './types'; export const TOPOLOGY_GRAPH_ID = 'cryostat-target-topology-graph'; @@ -41,26 +50,41 @@ export const getStatusTargetNode = (node: TargetNode | EnvironmentNode): [NodeSt : [ NodeStatus.warning, { - title: 'Failed to compute JVM ID', + title: 'Failed to generate the JVM identifier', description: ( <> - - If target has JMX Authentication enabled, add the credential to Cryostat keyring. - - - If the target has SSL enabled over JMX, add its certificate to Cryostat truststore. - + Check the target authentication settings: ), callForAction: [ - - - , - - Add certificates - , + + If{' '} + + }> + JMX Authentication + + {' '} + is enabled,{' '} + + + . + + , + + If{' '} + + }> + SSL is enabled + + {' '} + for JMX,{' '} + + add the SSL certificate + + . + , ], }, ]; diff --git a/src/test/Agent/AgentProbeTemplates.test.tsx b/src/test/Agent/AgentProbeTemplates.test.tsx index aa5324107..e2f6d1574 100644 --- a/src/test/Agent/AgentProbeTemplates.test.tsx +++ b/src/test/Agent/AgentProbeTemplates.test.tsx @@ -155,7 +155,7 @@ describe('', () => { expect(modalTitle).toBeInTheDocument(); expect(modalTitle).toBeVisible(); - const dropZoneText = within(modal).getByText('Drag and drop files here'); + const dropZoneText = within(modal).getByText('Drag a file here'); expect(dropZoneText).toBeInTheDocument(); expect(dropZoneText).toBeVisible(); diff --git a/src/test/Events/EventTemplates.test.tsx b/src/test/Events/EventTemplates.test.tsx index e37fbf6fe..bd0ca2e9b 100644 --- a/src/test/Events/EventTemplates.test.tsx +++ b/src/test/Events/EventTemplates.test.tsx @@ -264,7 +264,7 @@ describe('', () => { expect(modalTitle).toBeInTheDocument(); expect(modalTitle).toBeVisible(); - const dropZoneText = within(modal).getByText('Drag and drop files here'); + const dropZoneText = within(modal).getByText('Drag a file here'); expect(dropZoneText).toBeInTheDocument(); expect(dropZoneText).toBeVisible(); diff --git a/src/test/Recordings/ArchivedRecordingsTable.test.tsx b/src/test/Recordings/ArchivedRecordingsTable.test.tsx index 58a50c434..860471ea5 100644 --- a/src/test/Recordings/ArchivedRecordingsTable.test.tsx +++ b/src/test/Recordings/ArchivedRecordingsTable.test.tsx @@ -461,7 +461,7 @@ describe('', () => { expect(fileLabel).toBeInTheDocument(); expect(fileLabel).toBeInTheDocument(); - const dropZoneText = within(modal).getByText('Drag and drop files here'); + const dropZoneText = within(modal).getByText('Drag a file here'); expect(dropZoneText).toBeInTheDocument(); expect(dropZoneText).toBeVisible(); @@ -521,7 +521,7 @@ describe('', () => { expect(fileLabel).toBeInTheDocument(); expect(fileLabel).toBeInTheDocument(); - const dropZoneText = within(modal).getByText('Drag and drop files here'); + const dropZoneText = within(modal).getByText('Drag a file here'); expect(dropZoneText).toBeInTheDocument(); expect(dropZoneText).toBeVisible(); @@ -609,7 +609,7 @@ describe('', () => { expect(fileLabel).toBeInTheDocument(); expect(fileLabel).toBeInTheDocument(); - const dropZoneText = within(modal).getByText('Drag and drop files here'); + const dropZoneText = within(modal).getByText('Drag a file here'); expect(dropZoneText).toBeInTheDocument(); expect(dropZoneText).toBeVisible(); @@ -693,7 +693,7 @@ describe('', () => { expect(fileLabel).toBeInTheDocument(); expect(fileLabel).toBeInTheDocument(); - const dropZoneText = within(modal).getByText('Drag and drop files here'); + const dropZoneText = within(modal).getByText('Drag a file here'); expect(dropZoneText).toBeInTheDocument(); expect(dropZoneText).toBeVisible(); diff --git a/src/test/Rules/Rules.test.tsx b/src/test/Rules/Rules.test.tsx index 111adda3a..4a776006f 100644 --- a/src/test/Rules/Rules.test.tsx +++ b/src/test/Rules/Rules.test.tsx @@ -147,7 +147,7 @@ describe('', () => { expect(modalTitle).toBeInTheDocument(); expect(modalTitle).toBeVisible(); - const dropZoneText = within(modal).getByText('Drag and drop files here'); + const dropZoneText = within(modal).getByText('Drag a file here'); expect(dropZoneText).toBeInTheDocument(); expect(dropZoneText).toBeVisible(); }); @@ -265,7 +265,7 @@ describe('', () => { expect(modalTitle).toBeInTheDocument(); expect(modalTitle).toBeVisible(); - const dropZoneText = within(modal).getByText('Drag and drop files here'); + const dropZoneText = within(modal).getByText('Drag a file here'); expect(dropZoneText).toBeInTheDocument(); expect(dropZoneText).toBeVisible();