Skip to content

Commit

Permalink
feat(jvmid): enhance UI hints when JVM ID cannot be generated (#1119)
Browse files Browse the repository at this point in the history
(cherry picked from commit ba88f71)
  • Loading branch information
andrewazores authored and mergify[bot] committed Oct 13, 2023
1 parent 6d7630d commit cf5d3a7
Show file tree
Hide file tree
Showing 15 changed files with 218 additions and 53 deletions.
2 changes: 1 addition & 1 deletion src/app/SecurityPanel/CertificateUploadModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ export const CertificateUploadModal: React.FC<CertificateUploadModalProps> = ({
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."
>
<Form>
<FormGroup label="Certificate File" isRequired fieldId="file">
Expand Down
34 changes: 26 additions & 8 deletions src/app/SecurityPanel/Credentials/StoreCredentials.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -31,13 +32,18 @@ import {
DropdownToggleCheckbox,
EmptyState,
EmptyStateIcon,
Icon,

Check warning on line 35 in src/app/SecurityPanel/Credentials/StoreCredentials.tsx

View workflow job for this annotation

GitHub Actions / eslint-check (16.x)

'Icon' is defined but never used

Check warning on line 35 in src/app/SecurityPanel/Credentials/StoreCredentials.tsx

View workflow job for this annotation

GitHub Actions / eslint-check (18.x)

'Icon' is defined but never used
Popover,
Text,
TextContent,
TextVariants,
Title,
Toolbar,
ToolbarContent,
ToolbarItem,
Tooltip,

Check warning on line 44 in src/app/SecurityPanel/Credentials/StoreCredentials.tsx

View workflow job for this annotation

GitHub Actions / eslint-check (16.x)

'Tooltip' is defined but never used

Check warning on line 44 in src/app/SecurityPanel/Credentials/StoreCredentials.tsx

View workflow job for this annotation

GitHub Actions / eslint-check (18.x)

'Tooltip' is defined but never used
} 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';
Expand Down Expand Up @@ -516,15 +522,27 @@ export const CheckBoxActions: React.FC<CheckBoxActionsProps> = ({
};

export const StoreCredentialsCard: SecurityCard = {
title: 'Store Credentials',
description: (
key: 'credentials',
title: (
<Text>
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 <Link to="/settings">Settings</Link> to configure locally-stored credentials.
Store Credentials
<Popover maxWidth="40rem" headerContent="JMX Authentication" bodyContent={<JmxAuthDescription />}>
<Button variant="plain">
<OutlinedQuestionCircleIcon />
</Button>
</Popover>
</Text>
),
description: (
<TextContent>
<Text component={TextVariants.small}>
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 <Link to="/settings">Settings</Link> to configure locally-stored credentials.
</Text>
</TextContent>
),
content: StoreCredentials,
};
23 changes: 20 additions & 3 deletions src/app/SecurityPanel/ImportCertificate.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Check warning on line 18 in src/app/SecurityPanel/ImportCertificate.tsx

View workflow job for this annotation

GitHub Actions / eslint-check (16.x)

'Icon' is defined but never used

Check warning on line 18 in src/app/SecurityPanel/ImportCertificate.tsx

View workflow job for this annotation

GitHub Actions / eslint-check (16.x)

'Tooltip' is defined but never used

Check warning on line 18 in src/app/SecurityPanel/ImportCertificate.tsx

View workflow job for this annotation

GitHub Actions / eslint-check (18.x)

'Icon' is defined but never used

Check warning on line 18 in src/app/SecurityPanel/ImportCertificate.tsx

View workflow job for this annotation

GitHub Actions / eslint-check (18.x)

'Tooltip' is defined but never used
import { OutlinedQuestionCircleIcon } from '@patternfly/react-icons';
import * as React from 'react';
import { CertificateUploadModal } from './CertificateUploadModal';
import { SecurityCard } from './types';
Expand All @@ -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: (
<Text>
Import SSL Certificates
<Popover maxWidth="40rem" headerContent="JMX over SSL" bodyContent={<JmxSslDescription />}>
<Button variant="plain">
<OutlinedQuestionCircleIcon />
</Button>
</Popover>
</Text>
),
description: (
<TextContent>
<Text component={TextVariants.small}>Add SSL certificates to the Cryostat server truststore.</Text>
<Text component={TextVariants.small}>You must restart the Cryostat server to reload the certificate store.</Text>
</TextContent>
),
content: CertificateImport,
};
5 changes: 3 additions & 2 deletions src/app/SecurityPanel/SecurityPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export interface SecurityPanelProps {}

export const SecurityPanel: React.FC<SecurityPanelProps> = (_) => {
const securityCards = [ImportCertificate, StoreCredentialsCard].map((c) => ({
key: c.key,
title: c.title,
description: c.description,
element: React.createElement(c.content, null),
Expand All @@ -31,10 +32,10 @@ export const SecurityPanel: React.FC<SecurityPanelProps> = (_) => {
return (
<BreadcrumbPage pageTitle="Security">
{securityCards.map((s) => (
<Card key={s.title}>
<Card key={s.key}>
<CardTitle>
<Text component={TextVariants.h1}>{s.title}</Text>
<Text component={TextVariants.small}>{s.description}</Text>
{s.description}
</CardTitle>
<CardBody>{s.element}</CardBody>
</Card>
Expand Down
5 changes: 3 additions & 2 deletions src/app/SecurityPanel/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
*/

export interface SecurityCard {
title: string;
description: JSX.Element | string;
key: string;
title: JSX.Element;
description: JSX.Element;
content: React.FC;
}
2 changes: 1 addition & 1 deletion src/app/Shared/Components/FileUploads.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ export const MultiFileUpload: React.FC<MultiFileUploadProps> = ({
>
<MultipleFileUploadMain
titleIcon={titleIcon || <UploadIcon />}
titleText={titleText || 'Drag and drop files here'}
titleText={titleText || 'Drag a file here'}
titleTextSeparator={titleTextSeparator || 'or'}
infoText={infoText || `Accepted file types: ${displayAccepts.join(', ')}`}
/>
Expand Down
44 changes: 44 additions & 0 deletions src/app/Shared/Components/JmxAuthDescription.tsx
Original file line number Diff line number Diff line change
@@ -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<React.PropsWithChildren<DescriptionProps>> = ({ children }) => {
return (
<TextContent>
{children}
<Text component={TextVariants.p}>
JVM applications can be configured to require clients, such as Cryostat, to pass a challenge based
authentication before establishing a connection.
</Text>
<Text component={TextVariants.p}>
Check the deployment configuration of your JVM application for system properties such as:
</Text>
<TextList>
<TextListItem>
<Text component={TextVariants.pre}>com.sun.management.jmxremote.authenticate</Text>
</TextListItem>
<TextListItem>
<Text component={TextVariants.pre}>com.sun.management.jmxremote.password.file</Text>
</TextListItem>
<TextListItem>
<Text component={TextVariants.pre}>com.sun.management.jmxremote.login.config</Text>
</TextListItem>
</TextList>
</TextContent>
);
};
54 changes: 54 additions & 0 deletions src/app/Shared/Components/JmxSslDescription.tsx
Original file line number Diff line number Diff line change
@@ -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<React.PropsWithChildren<DescriptionProps>> = ({ children }) => {
return (
<TextContent>
{children}
<Text component={TextVariants.p}>
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.
</Text>
<Text component={TextVariants.p}>
Check the deployment configuration of your JVM application for system properties such as:
</Text>
<TextList>
<TextListItem>
<Text component={TextVariants.pre}>javax.net.ssl.keyStore</Text>
</TextListItem>
<TextListItem>
<Text component={TextVariants.pre}>javax.net.ssl.keyStorePassword</Text>
</TextListItem>
<TextListItem>
<Text component={TextVariants.pre}>com.sun.management.jmxremote.ssl.need.client.auth</Text>
</TextListItem>
<TextListItem>
<Text component={TextVariants.pre}>javax.net.ssl.trustStore</Text>
</TextListItem>
<TextListItem>
<Text component={TextVariants.pre}>javax.net.ssl.trustStorePassword</Text>
</TextListItem>
<TextListItem>
<Text component={TextVariants.pre}>com.sun.management.jmxremote.registry.ssl</Text>
</TextListItem>
</TextList>
</TextContent>
);
};
4 changes: 4 additions & 0 deletions src/app/Shared/Components/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
22 changes: 12 additions & 10 deletions src/app/Topology/Entity/EntityDetails.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ export const TargetDetails: React.FC<{
<>
<span style={{ marginRight: '0.5em' }}>JVM ID</span>
{!serviceRef.jvmId && (
<Tooltip content={'Failed to compute JVM ID'}>
<Tooltip content={'Failed to generate the JVM identifier'}>
<ExclamationTriangleIcon color="orange" />
</Tooltip>
)}
Expand All @@ -201,7 +201,7 @@ export const TargetDetails: React.FC<{
<>
<span style={{ marginRight: '0.5em' }}>JVM ID</span>
{!serviceRef.jvmId && (
<Tooltip content={'Failed to compute JVM ID'}>
<Tooltip content={'Failed to generate the JVM identifier'}>
<ExclamationTriangleIcon color="orange" />
</Tooltip>
)}
Expand Down Expand Up @@ -666,15 +666,17 @@ export const EntityDetailHeader: React.FC<EntityDetailHeaderProps> = ({
actionClose={<AlertActionCloseButton onClose={() => setShowBanner(false)} />}
>
<Stack hasGutter>
<StackItem key={'alert-description'}>{extra?.description}</StackItem>
{extra?.callForAction && !alertOptions.hideActions ? (
<StackItem key={'alert-call-for-action'}>
<Flex>
{extra.callForAction.map((action, index) => (
<FlexItem key={index}>{action}</FlexItem>
))}
</Flex>
</StackItem>
<>
<StackItem key={'alert-description'}>{extra?.description}</StackItem>
<StackItem key={'alert-call-for-action'}>
<Flex>
{extra.callForAction.map((action, index) => (
<FlexItem key={index}>{action}</FlexItem>
))}
</Flex>
</StackItem>
</>
) : null}
</Stack>
</Alert>
Expand Down
60 changes: 42 additions & 18 deletions src/app/Topology/Shared/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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: (
<>
<Text component={TextVariants.p}>
If target has JMX Authentication enabled, add the credential to Cryostat keyring.
</Text>
<Text component={TextVariants.p}>
If the target has SSL enabled over JMX, add its certificate to Cryostat truststore.
</Text>
<Text component={TextVariants.p}>Check the target authentication settings:</Text>
</>
),
callForAction: [
<WarningResolverAsCredModal key={`${node.target.alias}-resolver-as-credential-modal`}>
<Button variant="link" isSmall style={{ padding: 0 }}>
Add credentials
</Button>
</WarningResolverAsCredModal>,
<WarningResolverAsLink key={`${node.target.alias}-resolver-as-link-to-security`} to="/security">
Add certificates
</WarningResolverAsLink>,
<Text key="jmx-auth" component={TextVariants.p}>
If{' '}
<DescriptionListTermHelpText>
<Popover maxWidth="40rem" headerContent="JMX Authentication" bodyContent={<JmxAuthDescription />}>
<DescriptionListTermHelpTextButton>JMX Authentication</DescriptionListTermHelpTextButton>
</Popover>
</DescriptionListTermHelpText>{' '}
is enabled,{' '}
<WarningResolverAsCredModal key={`${node.target.alias}-resolver-as-credential-modal`}>
<Button variant="link" isSmall style={{ padding: 0 }}>
add credentials
</Button>
.
</WarningResolverAsCredModal>
</Text>,
<Text key="jmx-ssl" component={TextVariants.p}>
If{' '}
<DescriptionListTermHelpText>
<Popover maxWidth="40rem" headerContent="JMX over SSL" bodyContent={<JmxSslDescription />}>
<DescriptionListTermHelpTextButton>SSL is enabled</DescriptionListTermHelpTextButton>
</Popover>
</DescriptionListTermHelpText>{' '}
for JMX,{' '}
<WarningResolverAsLink key={`${node.target.alias}-resolver-as-link-to-security`} to="/security">
add the SSL certificate
</WarningResolverAsLink>
.
</Text>,
],
},
];
Expand Down
2 changes: 1 addition & 1 deletion src/test/Agent/AgentProbeTemplates.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ describe('<AgentProbeTemplates />', () => {
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();

Expand Down
Loading

0 comments on commit cf5d3a7

Please sign in to comment.