From cc927d0dae4aa823a1714503d81d0b0a97f86ab1 Mon Sep 17 00:00:00 2001
From: Noah Paige <69586985+noah-paige@users.noreply.github.com>
Date: Tue, 25 Jun 2024 10:18:44 -0400
Subject: [PATCH] Enhance Share Health Status Verify/ReApply (#1346)

### 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.
---
 .../db/share_state_machines_repositories.py   | 11 +++
 .../services/share_item_service.py            |  9 +++
 .../design/components/ShareHealthStatus.js    | 72 +++++++++++++++++++
 frontend/src/design/components/index.js       |  1 +
 .../components/DatasetListItem.js             |  4 +-
 .../components/EnvironmentOwnedDatasets.js    |  2 +-
 .../modules/Shared/Shares/ShareEditForm.js    | 21 +++++-
 .../components/NavigateShareViewModal.js      | 60 ++++++++++++++++
 .../modules/Shares/components/ShareBoxList.js | 18 +++++
 .../src/modules/Shares/components/index.js    |  1 +
 .../src/modules/Shares/views/ShareView.js     | 36 ++--------
 .../modules/s3_datasets_shares/test_share.py  | 11 ++-
 12 files changed, 209 insertions(+), 37 deletions(-)
 create mode 100644 frontend/src/design/components/ShareHealthStatus.js
 create mode 100644 frontend/src/modules/Shares/components/NavigateShareViewModal.js

diff --git a/backend/dataall/modules/shares_base/db/share_state_machines_repositories.py b/backend/dataall/modules/shares_base/db/share_state_machines_repositories.py
index 2da805cef..1b9905dcf 100644
--- a/backend/dataall/modules/shares_base/db/share_state_machines_repositories.py
+++ b/backend/dataall/modules/shares_base/db/share_state_machines_repositories.py
@@ -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)
diff --git a/backend/dataall/modules/shares_base/services/share_item_service.py b/backend/dataall/modules/shares_base/services/share_item_service.py
index 058820c23..5ad7c1472 100644
--- a/backend/dataall/modules/shares_base/services/share_item_service.py
+++ b/backend/dataall/modules/shares_base/services/share_item_service.py
@@ -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:
@@ -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)
 
diff --git a/frontend/src/design/components/ShareHealthStatus.js b/frontend/src/design/components/ShareHealthStatus.js
new file mode 100644
index 000000000..983536f67
--- /dev/null
+++ b/frontend/src/design/components/ShareHealthStatus.js
@@ -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
+};
diff --git a/frontend/src/design/components/index.js b/frontend/src/design/components/index.js
index cd01380bb..8b5ce3bc0 100644
--- a/frontend/src/design/components/index.js
+++ b/frontend/src/design/components/index.js
@@ -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';
diff --git a/frontend/src/modules/DatasetsBase/components/DatasetListItem.js b/frontend/src/modules/DatasetsBase/components/DatasetListItem.js
index 432883398..85726a435 100644
--- a/frontend/src/modules/DatasetsBase/components/DatasetListItem.js
+++ b/frontend/src/modules/DatasetsBase/components/DatasetListItem.js
@@ -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}`
                           : '-'
                       );
@@ -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}`
                   : '-'
               }
diff --git a/frontend/src/modules/Environments/components/EnvironmentOwnedDatasets.js b/frontend/src/modules/Environments/components/EnvironmentOwnedDatasets.js
index 1ed875a3f..5ef920184 100644
--- a/frontend/src/modules/Environments/components/EnvironmentOwnedDatasets.js
+++ b/frontend/src/modules/Environments/components/EnvironmentOwnedDatasets.js
@@ -150,7 +150,7 @@ export const EnvironmentOwnedDatasets = ({ environment }) => {
                         <IconButton
                           onClick={() => {
                             navigate(
-                              dataset.datasetType === 'DatasetType.S3'
+                              dataset.datasetType === 'DatasetTypes.S3'
                                 ? `/console/s3-datasets/${dataset.datasetUri}`
                                 : '-'
                             );
diff --git a/frontend/src/modules/Shared/Shares/ShareEditForm.js b/frontend/src/modules/Shared/Shares/ShareEditForm.js
index e357ece47..5ad742a53 100644
--- a/frontend/src/modules/Shared/Shares/ShareEditForm.js
+++ b/frontend/src/modules/Shared/Shares/ShareEditForm.js
@@ -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 {
@@ -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';
   };
@@ -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' ||
@@ -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>
@@ -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' ||
diff --git a/frontend/src/modules/Shares/components/NavigateShareViewModal.js b/frontend/src/modules/Shares/components/NavigateShareViewModal.js
new file mode 100644
index 000000000..7f3effd2b
--- /dev/null
+++ b/frontend/src/modules/Shares/components/NavigateShareViewModal.js
@@ -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
+};
diff --git a/frontend/src/modules/Shares/components/ShareBoxList.js b/frontend/src/modules/Shares/components/ShareBoxList.js
index 0436e4426..adc48099c 100644
--- a/frontend/src/modules/Shares/components/ShareBoxList.js
+++ b/frontend/src/modules/Shares/components/ShareBoxList.js
@@ -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" />;
@@ -50,6 +51,8 @@ export const ShareBoxList = (props) => {
   const [datasets, setDatasets] = useState([]);
   const [isVerifyObjectItemsModalOpen, setIsVerifyObjectItemsModalOpen] =
     useState(false);
+  const [isNavigateShareViewModalOpen, setIsNavigateShareViewModalOpen] =
+    useState(false);
   const statusOptions = ShareStatusList;
 
   const handleVerifyObjectItemsModalOpen = () => {
@@ -57,6 +60,13 @@ export const ShareBoxList = (props) => {
   };
   const handleVerifyObjectItemsModalClose = () => {
     setIsVerifyObjectItemsModalOpen(false);
+    if (dataset) {
+      setIsNavigateShareViewModalOpen(true);
+    }
+  };
+
+  const handleNavigateShareViewModalClose = () => {
+    setIsNavigateShareViewModalOpen(false);
   };
 
   const handlePageChange = async (event, value) => {
@@ -529,6 +539,14 @@ export const ShareBoxList = (props) => {
           open={isVerifyObjectItemsModalOpen}
         />
       )}
+      {isNavigateShareViewModalOpen && (
+        <NavigateShareViewModal
+          dataset={dataset}
+          onApply={handleNavigateShareViewModalClose}
+          onClose={handleNavigateShareViewModalClose}
+          open={isNavigateShareViewModalOpen}
+        />
+      )}
     </>
   );
 };
diff --git a/frontend/src/modules/Shares/components/index.js b/frontend/src/modules/Shares/components/index.js
index e00ca86a8..86373e838 100644
--- a/frontend/src/modules/Shares/components/index.js
+++ b/frontend/src/modules/Shares/components/index.js
@@ -6,3 +6,4 @@ export * from './ShareUpdateReject';
 export * from './ShareUpdateRequest';
 export * from './ShareItemsSelectorModal';
 export * from './ShareObjectSelectorModal';
+export * from './NavigateShareViewModal';
diff --git a/frontend/src/modules/Shares/views/ShareView.js b/frontend/src/modules/Shares/views/ShareView.js
index 16a5c311f..7bb05feda 100644
--- a/frontend/src/modules/Shares/views/ShareView.js
+++ b/frontend/src/modules/Shares/views/ShareView.js
@@ -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,
@@ -49,6 +46,7 @@ import {
   PencilAltIcon,
   Scrollbar,
   ShareStatus,
+  ShareHealthStatus,
   TextAvatar,
   useSettings
 } from 'design';
@@ -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 ? (
diff --git a/tests/modules/s3_datasets_shares/test_share.py b/tests/modules/s3_datasets_shares/test_share.py
index 22ca395ee..988b14f5e 100644
--- a/tests/modules/s3_datasets_shares/test_share.py
+++ b/tests/modules/s3_datasets_shares/test_share.py
@@ -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,
@@ -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,