Skip to content

Commit

Permalink
[Bugfix] | GH-1749 -Fixing share expiration task (#1750)
Browse files Browse the repository at this point in the history
### Feature or Bugfix

- Bugfix

### Detail
- Simplified the logic for share item state transition
- Resolved bug by adding 

### Relates
- (#1749)

### Testing

1. Created a share with expiration and all the share items are in
Revoke_Succeded state. After running share exp task, no error were
thrown ✅
2. Created a share with expiration and few shares are in Share_Succeeded
and few are in Revoke_Succeded state. After running share expiration
task, share succeeded items were revoked successfully. ✅
3. Share with expiration and items in Revoke_succeeded and few in
revoke_failed, submitted, then share exp task doesn't process those
items and doesn't throw any error ✅

### 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.

---------

Co-authored-by: trajopadhye <[email protected]>
  • Loading branch information
TejasRGitHub and trajopadhye authored Feb 4, 2025
1 parent 00fcc93 commit 851d756
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@
from dataall.modules.datasets_base.db.dataset_repositories import DatasetBaseRepository
from dataall.modules.notifications.db.notification_models import Notification
from dataall.modules.shares_base.db.share_object_models import ShareObjectItem, ShareObject
from dataall.modules.shares_base.services.shares_enums import ShareItemHealthStatus, PrincipalType, ShareableType
from dataall.modules.shares_base.services.shares_enums import (
ShareItemHealthStatus,
PrincipalType,
ShareableType,
ShareObjectStatus,
)

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -486,7 +491,13 @@ def fetch_submitted_shares_with_notifications(session):
def get_all_active_shares_with_expiration(session):
return (
session.query(ShareObject)
.filter(and_(ShareObject.expiryDate.isnot(None), ShareObject.deleted.is_(None)))
.filter(
and_(
ShareObject.expiryDate.isnot(None),
ShareObject.deleted.is_(None),
ShareObject.status == ShareObjectStatus.Processed.value,
)
)
.all()
)

Expand Down
23 changes: 11 additions & 12 deletions backend/dataall/modules/shares_base/tasks/share_expiration_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from dataall.modules.shares_base.db.share_state_machines_repositories import ShareStatusRepository
from dataall.modules.shares_base.services.share_notification_service import ShareNotificationService
from dataall.modules.datasets_base.db.dataset_repositories import DatasetBaseRepository
from dataall.modules.shares_base.services.shares_enums import ShareObjectActions
from dataall.modules.shares_base.services.shares_enums import ShareObjectActions, ShareItemStatus
from dataall.modules.shares_base.services.sharing_service import SharingService

log = logging.getLogger(__name__)
Expand All @@ -27,24 +27,23 @@ def share_expiration_checker(engine):
try:
if share.expiryDate.date() < datetime.today().date():
log.info(f'Revoking share with uri: {share.shareUri} as it is expired')
# Put all share items in revoke state and then revoke
# If a share is expired, pull all the share items which are in Share_Succeeded state
# Update status for each share item to Revoke_Approved and Revoke the share
share_items_to_revoke = ShareObjectRepository.get_all_share_items_in_share(
session, share.shareUri, ['Share_Succeeded']
)
item_uris = [share_item.shareItemUri for share_item in share_items_to_revoke]
revoked_items_states = ShareStatusRepository.get_share_items_states(
session, share.shareUri, item_uris
session, share.shareUri, [ShareItemStatus.Share_Succeeded.value]
)

# If the share doesn't have any share items in Share_Succeeded state then skip this share
if len(share_items_to_revoke) == 0:
continue

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

for item_state in revoked_items_states:
item_sm = ShareItemSM(item_state)
for item in share_items_to_revoke:
item_sm = ShareItemSM(item.status)
new_state = item_sm.run_transition(ShareObjectActions.RevokeItems.value)
for item in share_items_to_revoke:
if item.status == item_state:
item_sm.update_state_single_item(session, item, new_state)
item_sm.update_state_single_item(session, item, new_state)

share_sm.update_state(session, share, new_share_state)
SharingService.revoke_share(engine=engine, share_uri=share.shareUri)
Expand Down

0 comments on commit 851d756

Please sign in to comment.