Skip to content

Commit

Permalink
Merge pull request #411 from UW-GAC/bugfix/405-ignore-groups-when-not…
Browse files Browse the repository at this point in the history
…-admin-in-audit

Ignore groups when not admin in audit
  • Loading branch information
amstilp authored Oct 25, 2023
2 parents 5b42501 + 72f0255 commit 649bce5
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 17 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Bugfix: Print the correct number of "ok" instances in audit emails. 0.18 introduced a bug where the email included "0 instance(s) verified even if there was more than one verified instance.
* Bugfix: ManagedGroupMembershipAudit does not unexpectedly show errors for deactivated accounts that were in the group before they were deactivated.
* Bugfix: ManagedGroupMembershipAudit now raises the correct exception when instantiated with a ManagedGroup that is not managed by the app.
* Bugfix: ManagedGroupAudit does not report missing groups where the app is only a member.

## 0.18 (2023-10-03)

Expand Down
2 changes: 1 addition & 1 deletion anvil_consortium_manager/__init__.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = "0.19dev3"
__version__ = "0.19dev4"
4 changes: 3 additions & 1 deletion anvil_consortium_manager/audit/audit.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,9 @@ def run_audit(self):

# Check for groups that exist on AnVIL but not the app.
for group_name in groups_on_anvil:
self.add_result(NotInAppResult(group_name))
# Only report the ones where the app is an admin.
if "admin" in groups_on_anvil[group_name]:
self.add_result(NotInAppResult(group_name))


class ManagedGroupMembershipAudit(AnVILAudit):
Expand Down
22 changes: 8 additions & 14 deletions anvil_consortium_manager/tests/test_audit.py
Original file line number Diff line number Diff line change
Expand Up @@ -1219,7 +1219,7 @@ def test_anvil_audit_two_groups_both_missing(self):
self.assertEqual(record_result.errors, set([audit_results.ERROR_NOT_IN_ANVIL]))

def test_anvil_audit_one_group_member_missing_in_app(self):
"""anvil_audit works correctly if the service account is a member of a group not in the app."""
"""Groups that the app is a member of are not reported in the app."""
api_url = self.get_api_groups_url()
self.anvil_response_mock.add(
responses.GET,
Expand All @@ -1233,12 +1233,10 @@ def test_anvil_audit_one_group_member_missing_in_app(self):
)
audit_results = audit.ManagedGroupAudit()
audit_results.run_audit()
self.assertFalse(audit_results.ok())
self.assertTrue(audit_results.ok())
self.assertEqual(len(audit_results.get_verified_results()), 0)
self.assertEqual(len(audit_results.get_error_results()), 0)
self.assertEqual(len(audit_results.get_not_in_app_results()), 1)
record_result = audit_results.get_not_in_app_results()[0]
self.assertEqual(record_result.record, "test-group")
self.assertEqual(len(audit_results.get_not_in_app_results()), 0)

def test_anvil_audit_one_group_admin_missing_in_app(self):
"""anvil_audit works correctly if the service account is an admin of a group not in the app."""
Expand All @@ -1262,7 +1260,7 @@ def test_anvil_audit_one_group_admin_missing_in_app(self):
record_result = audit_results.get_not_in_app_results()[0]
self.assertEqual(record_result.record, "test-group")

def test_anvil_audit_two_groups_missing_in_app(self):
def test_anvil_audit_two_groups_admin_missing_in_app(self):
"""anvil_audit works correctly if there are two groups in AnVIL that aren't in the app."""
api_url = self.get_api_groups_url()
self.anvil_response_mock.add(
Expand All @@ -1271,12 +1269,8 @@ def test_anvil_audit_two_groups_missing_in_app(self):
status=200,
json=api_factories.GetGroupsResponseFactory(
response=[
api_factories.GroupDetailsAdminFactory(
groupName="test-group-admin"
),
api_factories.GroupDetailsMemberFactory(
groupName="test-group-member"
),
api_factories.GroupDetailsAdminFactory(groupName="test-group-1"),
api_factories.GroupDetailsAdminFactory(groupName="test-group-2"),
]
).response,
)
Expand All @@ -1287,9 +1281,9 @@ def test_anvil_audit_two_groups_missing_in_app(self):
self.assertEqual(len(audit_results.get_error_results()), 0)
self.assertEqual(len(audit_results.get_not_in_app_results()), 2)
record_result = audit_results.get_not_in_app_results()[0]
self.assertEqual(record_result.record, "test-group-admin")
self.assertEqual(record_result.record, "test-group-1")
record_result = audit_results.get_not_in_app_results()[1]
self.assertEqual(record_result.record, "test-group-member")
self.assertEqual(record_result.record, "test-group-2")

def test_fails_membership_audit(self):
"""Error is reported when a group fails the membership audit."""
Expand Down
2 changes: 1 addition & 1 deletion anvil_consortium_manager/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -6654,7 +6654,7 @@ def test_audit_not_in_app_one_record(self):
responses.GET,
api_url,
status=200,
json=[self.get_api_group_json("foo", "Member")],
json=[self.get_api_group_json("foo", "Admin")],
)
self.client.force_login(self.user)
response = self.client.get(self.get_url())
Expand Down

0 comments on commit 649bce5

Please sign in to comment.