Skip to content

Commit

Permalink
Merge pull request #929 from UW-GAC/bugfix/audits-inactive-users
Browse files Browse the repository at this point in the history 
Check for Account active/inactive status when running dbGaP and CDSA audits
  • Loading branch information
@amstilp
amstilp authored Feb 14, 2025
2 parents ea95893 + d540c83 commit da96636
Show file tree
Hide file tree
Showing 5 changed files with 257 additions and 43 deletions.
52 changes: 37 additions & 15 deletions primed/cdsa/audit/accessor_audit.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ class AccessorAudit(PRIMEDAudit):
NOT_ACCESSOR = "Not an accessor."
GROUP_WITHOUT_ACCESS = "Groups do not have access."

# Active status
INACTIVE_ACCOUNT = "Account is inactive."
# # Unexpected.
# ERROR_HAS_ACCESS = "Has access for an unknown reason."
UNEXPECTED_GROUP_ACCESS = "Group should not have access."
Expand Down Expand Up @@ -222,6 +224,7 @@ def _audit_agreement_and_account(self, signed_agreement, account):
is_in_access_group = GroupAccountMembership.objects.filter(
account=account, group=signed_agreement.anvil_access_group
).exists()
is_active = account.status == account.ACTIVE_STATUS

# Get the user.
if hasattr(account, "user") and account.user:
Expand All @@ -248,17 +251,26 @@ def _audit_agreement_and_account(self, signed_agreement, account):
return

is_accessor = user in signed_agreement.accessors.all()

if is_in_access_group:
if is_accessor:
self.verified.append(
VerifiedAccess(
signed_agreement=signed_agreement,
user=user,
member=account,
note=self.ACCESSOR_IN_ACCESS_GROUP,
if is_active:
self.verified.append(
VerifiedAccess(
signed_agreement=signed_agreement,
user=user,
member=account,
note=self.ACCESSOR_IN_ACCESS_GROUP,
)
)
else:
self.needs_action.append(
RemoveAccess(
signed_agreement=signed_agreement,
user=user,
member=account,
note=self.INACTIVE_ACCOUNT,
)
)
)
else:
self.needs_action.append(
RemoveAccess(
Expand All @@ -270,14 +282,24 @@ def _audit_agreement_and_account(self, signed_agreement, account):
)
else:
if is_accessor:
self.needs_action.append(
GrantAccess(
signed_agreement=signed_agreement,
user=user,
member=account,
note=self.ACCESSOR_LINKED_ACCOUNT,
if is_active:
self.needs_action.append(
GrantAccess(
signed_agreement=signed_agreement,
user=user,
member=account,
note=self.ACCESSOR_LINKED_ACCOUNT,
)
)
else:
self.verified.append(
VerifiedNoAccess(
signed_agreement=signed_agreement,
user=user,
member=account,
note=self.INACTIVE_ACCOUNT,
)
)
)
else:
self.verified.append(
VerifiedNoAccess(
Expand Down
52 changes: 38 additions & 14 deletions primed/cdsa/audit/uploader_audit.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,9 @@ class UploaderAudit(PRIMEDAudit):
NOT_UPLOADER = "Not an uploader."
GROUP_WITHOUT_ACCESS = "Groups do not have access."

# message when an account is inactive.
INACTIVE_ACCOUNT = "Account is inactive."

# # Unexpected.
# ERROR_HAS_ACCESS = "Has access for an unknown reason."
UNEXPECTED_GROUP_ACCESS = "Group should not have access."
Expand Down Expand Up @@ -254,17 +257,28 @@ def _audit_agreement_and_account(self, data_affiliate_agreement, account):
return

is_uploader = user in data_affiliate_agreement.uploaders.all()
is_active = account.status == account.ACTIVE_STATUS

if is_in_access_group:
if is_uploader:
self.verified.append(
VerifiedAccess(
data_affiliate_agreement=data_affiliate_agreement,
user=user,
member=account,
note=self.UPLOADER_IN_ACCESS_GROUP,
if is_active:
self.verified.append(
VerifiedAccess(
data_affiliate_agreement=data_affiliate_agreement,
user=user,
member=account,
note=self.UPLOADER_IN_ACCESS_GROUP,
)
)
else:
self.needs_action.append(
RemoveAccess(
data_affiliate_agreement=data_affiliate_agreement,
user=user,
member=account,
note=self.INACTIVE_ACCOUNT,
)
)
)
else:
self.needs_action.append(
RemoveAccess(
Expand All @@ -276,14 +290,24 @@ def _audit_agreement_and_account(self, data_affiliate_agreement, account):
)
else:
if is_uploader:
self.needs_action.append(
GrantAccess(
data_affiliate_agreement=data_affiliate_agreement,
user=user,
member=account,
note=self.UPLOADER_LINKED_ACCOUNT,
if is_active:
self.needs_action.append(
GrantAccess(
data_affiliate_agreement=data_affiliate_agreement,
user=user,
member=account,
note=self.UPLOADER_LINKED_ACCOUNT,
)
)
else:
self.verified.append(
VerifiedNoAccess(
data_affiliate_agreement=data_affiliate_agreement,
user=user,
member=account,
note=self.INACTIVE_ACCOUNT,
)
)
)
else:
self.verified.append(
VerifiedNoAccess(
Expand Down
96 changes: 96 additions & 0 deletions primed/cdsa/tests/test_audit.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2680,6 +2680,54 @@ def test_not_accessor_and_account_has_no_user(self):
self.assertEqual(record.member, account)
self.assertEqual(record.note, accessor_audit.AccessorAudit.ACCOUNT_NOT_LINKED_TO_USER)

def test_accessor_inactive_account_not_in_access_group(self):
# Create applications.
signed_agreement = factories.SignedAgreementFactory.create()
# Create accounts.
account = AccountFactory.create(verified=True)
# Set up accessors.
signed_agreement.accessors.add(account.user)
# Deactivate account.
account.deactivate()
# Set up audit
audit = accessor_audit.AccessorAudit()
# Run audit
audit.audit_agreement(signed_agreement)
self.assertEqual(len(audit.verified), 1)
self.assertEqual(len(audit.needs_action), 0)
self.assertEqual(len(audit.errors), 0)
record = audit.verified[0]
self.assertIsInstance(record, accessor_audit.VerifiedNoAccess)
self.assertEqual(record.signed_agreement, signed_agreement)
self.assertEqual(record.user, account.user)
self.assertEqual(record.member, account)
self.assertEqual(record.note, accessor_audit.AccessorAudit.INACTIVE_ACCOUNT)

def test_accessor_inactive_account_in_access_group(self):
# Create applications.
signed_agreement = factories.SignedAgreementFactory.create()
# Create accounts.
account = AccountFactory.create(verified=True)
# Set up accessors.
signed_agreement.accessors.add(account.user)
# Deactivate account.
account.deactivate()
# Access group membership.
GroupAccountMembershipFactory.create(group=signed_agreement.anvil_access_group, account=account)
# Set up audit
audit = accessor_audit.AccessorAudit()
# Run audit
audit.audit_agreement(signed_agreement)
self.assertEqual(len(audit.verified), 0)
self.assertEqual(len(audit.needs_action), 1)
self.assertEqual(len(audit.errors), 0)
record = audit.needs_action[0]
self.assertIsInstance(record, accessor_audit.RemoveAccess)
self.assertEqual(record.signed_agreement, signed_agreement)
self.assertEqual(record.user, account.user)
self.assertEqual(record.member, account)
self.assertEqual(record.note, accessor_audit.AccessorAudit.INACTIVE_ACCOUNT)

def test_two_accessors(self):
"""Audit works when there are two accessors."""
# Create applications.
Expand Down Expand Up @@ -3122,6 +3170,54 @@ def test_uploader_no_account(self):
self.assertEqual(record.member, None)
self.assertEqual(record.note, uploader_audit.UploaderAudit.UPLOADER_NO_ACCOUNT)

def test_uploader_inactive_account_not_in_access_group(self):
# Create agreement.
data_affiliate_agreement = factories.DataAffiliateAgreementFactory.create()
# Create accounts.
account = AccountFactory.create(verified=True)
# Set up uploaders.
data_affiliate_agreement.uploaders.add(account.user)
# Deactivate account.
account.deactivate()
# Set up audit
audit = uploader_audit.UploaderAudit()
# Run audit
audit.audit_agreement(data_affiliate_agreement)
self.assertEqual(len(audit.verified), 1)
self.assertEqual(len(audit.needs_action), 0)
self.assertEqual(len(audit.errors), 0)
record = audit.verified[0]
self.assertIsInstance(record, uploader_audit.VerifiedNoAccess)
self.assertEqual(record.data_affiliate_agreement, data_affiliate_agreement)
self.assertEqual(record.user, account.user)
self.assertEqual(record.member, account)
self.assertEqual(record.note, uploader_audit.UploaderAudit.INACTIVE_ACCOUNT)

def test_uploader_inactive_account_in_access_group(self):
# Create agreement.
data_affiliate_agreement = factories.DataAffiliateAgreementFactory.create()
# Create accounts.
account = AccountFactory.create(verified=True)
# Set up uploaders.
data_affiliate_agreement.uploaders.add(account.user)
# Deactivate account.
account.deactivate()
# Access group membership.
GroupAccountMembershipFactory.create(group=data_affiliate_agreement.anvil_upload_group, account=account)
# Set up audit
audit = uploader_audit.UploaderAudit()
# Run audit
audit.audit_agreement(data_affiliate_agreement)
self.assertEqual(len(audit.verified), 0)
self.assertEqual(len(audit.needs_action), 1)
self.assertEqual(len(audit.errors), 0)
record = audit.needs_action[0]
self.assertIsInstance(record, uploader_audit.RemoveAccess)
self.assertEqual(record.data_affiliate_agreement, data_affiliate_agreement)
self.assertEqual(record.user, account.user)
self.assertEqual(record.member, account)
self.assertEqual(record.note, uploader_audit.UploaderAudit.INACTIVE_ACCOUNT)

def test_user_in_group_not_uploader(self):
# Create applications.
data_affiliate_agreement = factories.DataAffiliateAgreementFactory.create()
Expand Down
52 changes: 38 additions & 14 deletions primed/dbgap/audit/collaborator_audit.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ class dbGaPCollaboratorAudit(PRIMEDAudit):
NOT_COLLABORATOR = "Not a collaborator."
GROUP_WITHOUT_ACCESS = "Groups do not have access."

# Inactive account.
INACTIVE_ACCOUNT = "Account is inactive."

# # Unexpected.
# ERROR_HAS_ACCESS = "Has access for an unknown reason."
UNEXPECTED_GROUP_ACCESS = "Group should not have access."
Expand Down Expand Up @@ -267,6 +270,7 @@ def _audit_application_and_account(self, dbgap_application, account):

is_pi = user == dbgap_application.principal_investigator
is_collaborator = user in dbgap_application.collaborators.all()
is_active = account.status == account.ACTIVE_STATUS

if is_in_access_group:
if is_pi:
Expand All @@ -279,14 +283,24 @@ def _audit_application_and_account(self, dbgap_application, account):
)
)
elif is_collaborator:
self.verified.append(
VerifiedAccess(
dbgap_application=dbgap_application,
user=user,
member=account,
note=self.COLLABORATOR_IN_ACCESS_GROUP,
if is_active:
self.verified.append(
VerifiedAccess(
dbgap_application=dbgap_application,
user=user,
member=account,
note=self.COLLABORATOR_IN_ACCESS_GROUP,
)
)
else:
self.needs_action.append(
RemoveAccess(
dbgap_application=dbgap_application,
user=user,
member=account,
note=self.INACTIVE_ACCOUNT,
)
)
)
else:
self.needs_action.append(
RemoveAccess(
Expand All @@ -307,14 +321,24 @@ def _audit_application_and_account(self, dbgap_application, account):
)
)
elif is_collaborator:
self.needs_action.append(
GrantAccess(
dbgap_application=dbgap_application,
user=user,
member=account,
note=self.COLLABORATOR_LINKED_ACCOUNT,
if is_active:
self.needs_action.append(
GrantAccess(
dbgap_application=dbgap_application,
user=user,
member=account,
note=self.COLLABORATOR_LINKED_ACCOUNT,
)
)
else:
self.verified.append(
VerifiedNoAccess(
dbgap_application=dbgap_application,
user=user,
member=account,
note=self.INACTIVE_ACCOUNT,
)
)
)
else:
self.verified.append(
VerifiedNoAccess(
Expand Down
Loading

0 comments on commit da96636

Please sign in to comment.