Skip to content

Commit

Permalink
Enhance Share Health Status Verify/ReApply (#1346)
Browse files Browse the repository at this point in the history
### Feature or Bugfix
<!-- please choose -->
- Feature

### Detail
- Enhance Share UI for re-apply and verify share health status


### Relates
- #1107 

### Security
Please answer the questions below briefly where applicable, or write
`N/A`. Based on
[OWASP 10](https://owasp.org/Top10/en/).

- Does this PR introduce or modify any input fields or queries - this
includes
fetching data from storage outside the application (e.g. a database, an
S3 bucket)?
  - Is the input sanitized?
- What precautions are you taking before deserializing the data you
consume?
  - Is injection prevented by parametrizing queries?
  - Have you ensured no `eval` or similar functions are used?
- Does this PR introduce any functionality or component that requires
authorization?
- How have you ensured it respects the existing AuthN/AuthZ mechanisms?
  - Are you logging failed auth attempts?
- Are you using or adding any cryptographic features?
  - Do you use a standard proven implementations?
  - Are the used keys controlled by the customer? Where are they stored?
- Are you introducing any new policies/roles/users?
  - Have you used the least-privilege principle? How?


By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
  • Loading branch information
noah-paige committed Jun 25, 2024
1 parent 406cc3b commit cc927d0
Show file tree
Hide file tree
Showing 12 changed files with 209 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,17 @@ def get_share_items_states(session, share_uri, item_uris=None):
query = query.filter(ShareObjectItem.shareItemUri.in_(item_uris))
return [item.status for item in query.distinct(ShareObjectItem.status)]

@staticmethod
def get_share_items_health_states(session, share_uri, item_uris=None):
query = session.query(ShareObjectItem).filter(
and_(
ShareObjectItem.shareUri == share_uri,
)
)
if item_uris:
query = query.filter(ShareObjectItem.shareItemUri.in_(item_uris))
return [item.healthStatus for item in query.distinct(ShareObjectItem.healthStatus)]

@staticmethod
def update_share_object_status(session, share_uri: str, status: str) -> ShareObject:
share = ShareObjectRepository.get_share_by_uri(session, share_uri)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ def revoke_items_share_object(uri, revoked_uris):
share = ShareObjectRepository.get_share_by_uri(session, uri)
dataset = DatasetBaseRepository.get_dataset_by_uri(session, share.datasetUri)
revoked_items_states = ShareStatusRepository.get_share_items_states(session, uri, revoked_uris)
revoked_items_health_states = ShareStatusRepository.get_share_items_health_states(
session, uri, revoked_uris
)
revoked_items = [ShareObjectRepository.get_share_item_by_uri(session, uri) for uri in revoked_uris]

if not revoked_items_states:
Expand All @@ -88,6 +91,12 @@ def revoke_items_share_object(uri, revoked_uris):
message='Nothing to be revoked.',
)

if ShareItemHealthStatus.PendingReApply.value in revoked_items_health_states:
raise UnauthorizedOperation(
action='Revoke Items from Share Object',
message='Cannot revoke while reapply pending for one or more items.',
)

share_sm = ShareObjectSM(share.status)
new_share_state = share_sm.run_transition(ShareObjectActions.RevokeItems.value)

Expand Down
72 changes: 72 additions & 0 deletions frontend/src/design/components/ShareHealthStatus.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import VerifiedUserOutlinedIcon from '@mui/icons-material/VerifiedUserOutlined';
import GppBadOutlinedIcon from '@mui/icons-material/GppBadOutlined';
import PendingOutlinedIcon from '@mui/icons-material/PendingOutlined';
import DangerousOutlinedIcon from '@mui/icons-material/DangerousOutlined';
import * as PropTypes from 'prop-types';
import { Typography } from '@mui/material';
import { Label } from './Label';

export const ShareHealthStatus = (props) => {
const { status, healthStatus, lastVerificationTime } = props;

const isShared = ['Revoke_Failed', 'Share_Succeeded'].includes(status);
const isHealthPending = ['PendingReApply', 'PendingVerify', null].includes(
healthStatus
);
const setStatus = () => {
if (!healthStatus) return 'Undefined';
return healthStatus;
};

const setColor = () => {
if (!healthStatus) return 'info';
if (['Healthy'].includes(healthStatus)) return 'success';
if (['Unhealthy'].includes(healthStatus)) return 'error';
if (isHealthPending) return 'warning';
return 'info';
};

const setIcon = () => {
if (!healthStatus) return <DangerousOutlinedIcon color={setColor()} />;
if (['Healthy'].includes(healthStatus))
return <VerifiedUserOutlinedIcon color={setColor()} />;
if (['Unhealthy'].includes(healthStatus))
return <GppBadOutlinedIcon color={setColor()} />;
if (['PendingReApply', 'PendingVerify'].includes(healthStatus))
return <PendingOutlinedIcon color={setColor()} />;
return <DangerousOutlinedIcon color={setColor()} />;
};

if (!isShared) {
return (
<Typography color="textSecondary" variant="subtitle2">
{'Item is not Shared'}
</Typography>
);
}

return (
<div style={{ display: 'flex', alignItems: 'left' }}>
{setIcon()}
<Label color={setColor()}>{setStatus().toUpperCase()} </Label>
{!isHealthPending && (
<Typography color="textSecondary" variant="subtitle2" noWrap>
{(lastVerificationTime &&
'(' +
lastVerificationTime.substring(
0,
lastVerificationTime.indexOf('.')
) +
')') ||
''}
</Typography>
)}
</div>
);
};

ShareHealthStatus.propTypes = {
status: PropTypes.string.isRequired,
healthStatus: PropTypes.string,
lastVerificationTime: PropTypes.string
};
1 change: 1 addition & 0 deletions frontend/src/design/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export * from './Scrollbar';
export * from './SearchInput';
export * from './SettingsDrawer';
export * from './ShareStatus';
export * from './ShareHealthStatus';
export * from './SplashScreen';
export * from './StackStatus';
export * from './TagsInput';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export const DatasetListItem = (props) => {
variant="h6"
onClick={() => {
navigate(
dataset.datasetType === 'DatasetType.S3'
dataset.datasetType === 'DatasetTypes.S3'
? `/console/s3-datasets/${dataset.datasetUri}`
: '-'
);
Expand Down Expand Up @@ -208,7 +208,7 @@ export const DatasetListItem = (props) => {
color="primary"
component={RouterLink}
to={
dataset.datasetType === 'DatasetType.S3'
dataset.datasetType === 'DatasetTypes.S3'
? `/console/s3-datasets/${dataset.datasetUri}`
: '-'
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ export const EnvironmentOwnedDatasets = ({ environment }) => {
<IconButton
onClick={() => {
navigate(
dataset.datasetType === 'DatasetType.S3'
dataset.datasetType === 'DatasetTypes.S3'
? `/console/s3-datasets/${dataset.datasetUri}`
: '-'
);
Expand Down
21 changes: 18 additions & 3 deletions frontend/src/modules/Shared/Shares/ShareEditForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
TextField,
Typography
} from '@mui/material';
import { Defaults, Pager, ShareStatus } from '../../../design';
import { Defaults, Pager, ShareHealthStatus, ShareStatus } from 'design';
import SendIcon from '@mui/icons-material/Send';
import React, { useCallback, useEffect, useState } from 'react';
import {
Expand Down Expand Up @@ -50,7 +50,10 @@ const ItemRow = (props) => {
item.status === 'Share_Failed'
)
return 'Delete';
if (item.status === 'Share_Succeeded' || item.status === 'Revoke_Failed')
if (
(item.status === 'Share_Succeeded' || item.status === 'Revoke_Failed') &&
item.healthStatus !== 'PendingReApply'
)
return 'Revoke';
return 'Nothing';
};
Expand Down Expand Up @@ -135,6 +138,17 @@ const ItemRow = (props) => {
<TableCell>
{item.status ? <ShareStatus status={item.status} /> : 'Not requested'}
</TableCell>
<TableCell>
{item.status ? (
<ShareHealthStatus
status={item.status}
healthStatus={item.healthStatus}
lastVerificationTime={item.lastVerificationTime}
/>
) : (
'Not requested'
)}
</TableCell>
{(shareStatus === 'Draft' ||
shareStatus === 'Processed' ||
shareStatus === 'Rejected' ||
Expand Down Expand Up @@ -173,7 +187,7 @@ const ItemRow = (props) => {
)}
{possibleAction === 'Nothing' && (
<Typography color="textSecondary" variant="subtitle2">
Wait until this item is processed
Wait until this item is processed and/or re-apply task is complete
</Typography>
)}
</TableCell>
Expand Down Expand Up @@ -376,6 +390,7 @@ export const ShareEditForm = (props) => {
<TableCell>Type</TableCell>
<TableCell>Name</TableCell>
<TableCell>Status</TableCell>
<TableCell>Health Status</TableCell>
{(shareStatus === 'Draft' ||
shareStatus === 'Processed' ||
shareStatus === 'Rejected' ||
Expand Down
60 changes: 60 additions & 0 deletions frontend/src/modules/Shares/components/NavigateShareViewModal.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import { Box, Dialog, Divider, Typography, Button } from '@mui/material';
import PropTypes from 'prop-types';
import { Link as RouterLink } from 'react-router-dom';

export const NavigateShareViewModal = (props) => {
const { dataset, onApply, onClose, open, ...other } = props;

return (
<Dialog maxWidth="md" fullWidth onClose={onClose} open={open} {...other}>
<Box sx={{ p: 3 }}>
<Typography
align="center"
color="textPrimary"
gutterBottom
variant="h4"
>
Dataset: {dataset.label} - Share Object Verification Task(s) Started
</Typography>
<Typography
align="center"
color="textSecondary"
variant="subtitle2"
sx={{ p: 1 }}
>
Navigate to the Share View Page and select the desired share object to
view the progress of each of verification task
</Typography>
<Divider />
<Button
sx={{ p: 1 }}
color="primary"
type="button"
fullWidth
component={RouterLink}
to={`/console/shares`}
variant="contained"
>
Navigate to Shares View
</Button>
<Button
sx={{ p: 1 }}
onClick={onClose}
fullWidth
color="primary"
variant="outlined"
>
Close
</Button>
</Box>
</Dialog>
);
};

NavigateShareViewModal.propTypes = {
shares: PropTypes.array.isRequired,
dataset: PropTypes.object.isRequired,
onApply: PropTypes.func,
onClose: PropTypes.func,
open: PropTypes.bool.isRequired
};
18 changes: 18 additions & 0 deletions frontend/src/modules/Shares/components/ShareBoxList.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { getShareRequestsFromMe, listOwnedDatasets } from '../services';

import { ShareBoxListItem } from './ShareBoxListItem';
import { ShareObjectSelectorModal } from './ShareObjectSelectorModal';
import { NavigateShareViewModal } from './NavigateShareViewModal';
import { ShareStatusList } from '../constants';

const icon = <CheckBoxOutlineBlankIcon fontSize="small" />;
Expand All @@ -50,13 +51,22 @@ export const ShareBoxList = (props) => {
const [datasets, setDatasets] = useState([]);
const [isVerifyObjectItemsModalOpen, setIsVerifyObjectItemsModalOpen] =
useState(false);
const [isNavigateShareViewModalOpen, setIsNavigateShareViewModalOpen] =
useState(false);
const statusOptions = ShareStatusList;

const handleVerifyObjectItemsModalOpen = () => {
setIsVerifyObjectItemsModalOpen(true);
};
const handleVerifyObjectItemsModalClose = () => {
setIsVerifyObjectItemsModalOpen(false);
if (dataset) {
setIsNavigateShareViewModalOpen(true);
}
};

const handleNavigateShareViewModalClose = () => {
setIsNavigateShareViewModalOpen(false);
};

const handlePageChange = async (event, value) => {
Expand Down Expand Up @@ -529,6 +539,14 @@ export const ShareBoxList = (props) => {
open={isVerifyObjectItemsModalOpen}
/>
)}
{isNavigateShareViewModalOpen && (
<NavigateShareViewModal
dataset={dataset}
onApply={handleNavigateShareViewModalClose}
onClose={handleNavigateShareViewModalClose}
open={isNavigateShareViewModalOpen}
/>
)}
</>
);
};
Expand Down
1 change: 1 addition & 0 deletions frontend/src/modules/Shares/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ export * from './ShareUpdateReject';
export * from './ShareUpdateRequest';
export * from './ShareItemsSelectorModal';
export * from './ShareObjectSelectorModal';
export * from './NavigateShareViewModal';
36 changes: 6 additions & 30 deletions frontend/src/modules/Shares/views/ShareView.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@ import {
DeleteOutlined,
RefreshRounded
} from '@mui/icons-material';
import VerifiedUserIcon from '@mui/icons-material/VerifiedUser';
import GppBadIcon from '@mui/icons-material/GppBad';
import SecurityIcon from '@mui/icons-material/Security';
import PendingIcon from '@mui/icons-material/Pending';
import { LoadingButton } from '@mui/lab';
import {
Box,
Expand Down Expand Up @@ -49,6 +46,7 @@ import {
PencilAltIcon,
Scrollbar,
ShareStatus,
ShareHealthStatus,
TextAvatar,
useSettings
} from 'design';
Expand Down Expand Up @@ -489,33 +487,11 @@ export function SharedItem(props) {
)}
</TableCell>
<TableCell>
<div style={{ display: 'flex', alignItems: 'left' }}>
{item.healthStatus === 'Unhealthy' ? (
<Tooltip title={<Typography>{item.healthStatus}</Typography>}>
<GppBadIcon color={'error'} />
</Tooltip>
) : item.healthStatus === 'Healthy' ? (
<Tooltip title={<Typography>{item.healthStatus}</Typography>}>
<VerifiedUserIcon color={'success'} />
</Tooltip>
) : (
<Tooltip
title={
<Typography>{item.healthStatus || 'Undefined'}</Typography>
}
>
<PendingIcon color={'info'} />
</Tooltip>
)}
<Typography color="textSecondary" variant="subtitle2">
{(item.lastVerificationTime &&
item.lastVerificationTime.substring(
0,
item.lastVerificationTime.indexOf('.')
)) ||
''}
</Typography>
</div>
<ShareHealthStatus
status={item.status}
healthStatus={item.healthStatus}
lastVerificationTime={item.lastVerificationTime}
/>
</TableCell>
<TableCell>
{item.healthMessage ? (
Expand Down
11 changes: 10 additions & 1 deletion tests/modules/s3_datasets_shares/test_share.py
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,15 @@ def share3_processed(
if delete_share_object_response.data.deleteShareObject == True:
return

# Revert healthStatus back to healthy
with db.scoped_session() as session:
ShareStatusRepository.update_share_item_health_status_batch(
session=session,
share_uri=share3.shareUri,
old_status=ShareItemHealthStatus.PendingReApply.value,
new_status=ShareItemHealthStatus.Healthy.value,
)

# Given share item in shared states
get_share_object_response = get_share_object(
client=client,
Expand Down Expand Up @@ -1522,7 +1531,7 @@ def test_reapply_items_share_request(db, client, user, group, share3_processed,
client=client, user=user, group=group, shareUri=share3_processed.shareUri, reapply_items_uris=reapply_items_uris
)

# Then share item health Status changes to PendingVerify
# Then share item health Status changes to PendingReApply
get_share_object_response = get_share_object(
client=client,
user=user,
Expand Down

0 comments on commit cc927d0

Please sign in to comment.