Skip to content

Commit

Permalink
Merge pull request #365 from UW-GAC/bugfix/issue-362
Browse files Browse the repository at this point in the history
Fix AnVIL audit when service account is not directly a group admin
  • Loading branch information
amstilp authored Jun 9, 2023
2 parents 93ad7d4 + 46cb7fe commit d940c8e
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Devel

* Bugfix: correctly import AnVIL groups when the service account is both a member and an admin of the group.
* Bugfix: Fix failing ManagedGroup.anvil_audit_memership when the service account is not directly an admin of the group (but is via membership in another group).

## 0.16 (2023-06-01)

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.16.1dev1"
__version__ = "0.16.1dev2"
10 changes: 7 additions & 3 deletions anvil_consortium_manager/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -622,9 +622,13 @@ def anvil_audit_membership(self):
# Convert to case-insensitive emails.
admins_in_anvil = [x.lower() for x in response.json()]
# Remove the service account as admin.
admins_in_anvil.remove(
api_client.auth_session.credentials.service_account_email.lower()
)
try:
admins_in_anvil.remove(
api_client.auth_session.credentials.service_account_email.lower()
)
except ValueError:
# Not listed as an admin -- this is ok because it could be via group membership.
pass
# Sometimes the service account is also listed as a member. Remove that too.
try:
members_in_anvil.remove(
Expand Down
34 changes: 34 additions & 0 deletions anvil_consortium_manager/tests/test_models_anvil_api_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -2825,6 +2825,40 @@ def test_different_group_member_email_case_insensitive(self):
self.assertEqual(audit_results.get_errors(), {})
self.assertEqual(audit_results.get_not_in_app(), set())

def test_service_account_is_not_directly_admin(self):
"""Audit works when the service account is not directly an admin of a group (but is via a group admin)."""
group = factories.ManagedGroupFactory.create()
membership = factories.GroupGroupMembershipFactory.create(
parent_group=group,
child_group__email="[email protected]",
role=models.GroupGroupMembership.ADMIN,
)
api_url_members = self.get_api_url_members(group.name)
self.anvil_response_mock.add(
responses.GET,
api_url_members,
status=200,
json=api_factories.GetGroupMembershipResponseFactory().response,
)
api_url_admins = self.get_api_url_admins(group.name)
self.anvil_response_mock.add(
responses.GET,
api_url_admins,
status=200,
# Use the Membership factory because it doesn't add the service account as a direct admin.
json=api_factories.GetGroupMembershipResponseFactory(
response=["[email protected]"]
).response,
)
audit_results = group.anvil_audit_membership()
self.assertIsInstance(
audit_results, anvil_audit.ManagedGroupMembershipAuditResults
)
self.assertTrue(audit_results.ok())
self.assertEqual(audit_results.get_verified(), set([membership]))
self.assertEqual(audit_results.get_errors(), {})
self.assertEqual(audit_results.get_not_in_app(), set())


class WorkspaceAnVILAPIMockTest(AnVILAPIMockTestMixin, TestCase):
def setUp(self, *args, **kwargs):
Expand Down

0 comments on commit d940c8e

Please sign in to comment.