From 22fa130531a50f3aafe7b0358a33569067715b02 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Thu, 7 Mar 2024 16:33:13 -0800 Subject: [PATCH 1/6] Add is_requester_pays field to Workspace model --- .../0017_workspace_is_requester_pays.py | 23 +++++++++++++++++++ anvil_consortium_manager/models.py | 5 +++- anvil_consortium_manager/tests/test_models.py | 21 +++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 anvil_consortium_manager/migrations/0017_workspace_is_requester_pays.py diff --git a/anvil_consortium_manager/migrations/0017_workspace_is_requester_pays.py b/anvil_consortium_manager/migrations/0017_workspace_is_requester_pays.py new file mode 100644 index 00000000..04166410 --- /dev/null +++ b/anvil_consortium_manager/migrations/0017_workspace_is_requester_pays.py @@ -0,0 +1,23 @@ +# Generated by Django 5.0 on 2024-03-08 00:23 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('anvil_consortium_manager', '0016_max_length_limits'), + ] + + operations = [ + migrations.AddField( + model_name='historicalworkspace', + name='is_requester_pays', + field=models.BooleanField(default=False, help_text='Indicator of whether the workspace is set to requester pays.'), + ), + migrations.AddField( + model_name='workspace', + name='is_requester_pays', + field=models.BooleanField(default=False, help_text='Indicator of whether the workspace is set to requester pays.'), + ), + ] diff --git a/anvil_consortium_manager/models.py b/anvil_consortium_manager/models.py index ceba9481..a318620e 100644 --- a/anvil_consortium_manager/models.py +++ b/anvil_consortium_manager/models.py @@ -652,7 +652,10 @@ class Workspace(TimeStampedModel): help_text="Indicator of whether the workspace is locked or not.", default=False, ) - + is_requester_pays = models.BooleanField( + help_text="Indicator of whether the workspace is set to requester pays.", + default=False, + ) history = HistoricalRecords() class Meta: diff --git a/anvil_consortium_manager/tests/test_models.py b/anvil_consortium_manager/tests/test_models.py index 992a3d3c..fe9f080f 100644 --- a/anvil_consortium_manager/tests/test_models.py +++ b/anvil_consortium_manager/tests/test_models.py @@ -1531,6 +1531,27 @@ def test_is_locked_true(self): ) self.assertTrue(instance.is_locked) + def test_is_requester_pays_default(self): + """Default value for is_requester_pays is set as expected.""" + billing_project = factories.BillingProjectFactory.create() + instance = Workspace( + billing_project=billing_project, + name="my-name", + workspace_type=DefaultWorkspaceAdapter().get_type(), + ) + self.assertFalse(instance.is_requester_pays) + + def test_is_requester_pays_true(self): + """is_requester_pays can be set to True.""" + billing_project = factories.BillingProjectFactory.create() + instance = Workspace( + billing_project=billing_project, + name="my-name", + workspace_type=DefaultWorkspaceAdapter().get_type(), + is_requester_pays=True, + ) + self.assertTrue(instance.is_requester_pays) + def test_str_method(self): """The custom __str__ method returns the correct string.""" instance = factories.WorkspaceFactory.build( From 3614bb565939ed8399e9b3a8ab6404a904e33808 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 8 Mar 2024 09:03:13 -0800 Subject: [PATCH 2/6] Show a pill on the detail page for requester_pays workspaces --- .../anvil_consortium_manager/workspace_detail.html | 10 ++++++++++ anvil_consortium_manager/tests/test_views.py | 14 ++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/anvil_consortium_manager/templates/anvil_consortium_manager/workspace_detail.html b/anvil_consortium_manager/templates/anvil_consortium_manager/workspace_detail.html index 61696b80..f98b25b2 100644 --- a/anvil_consortium_manager/templates/anvil_consortium_manager/workspace_detail.html +++ b/anvil_consortium_manager/templates/anvil_consortium_manager/workspace_detail.html @@ -24,6 +24,16 @@ > + {% if object.is_requester_pays %} + Requester pays + + + {% endif %} + {% if has_access or user.is_superuser %} diff --git a/anvil_consortium_manager/tests/test_views.py b/anvil_consortium_manager/tests/test_views.py index be8c5abf..3e46fdae 100644 --- a/anvil_consortium_manager/tests/test_views.py +++ b/anvil_consortium_manager/tests/test_views.py @@ -6919,6 +6919,20 @@ def test_edit_permission_is_locked(self): ), ) + def test_is_requester_pays_true(self): + """An indicator of whether a workspace is requester_pays appears on the page.""" + workspace = factories.DefaultWorkspaceDataFactory.create(workspace__is_requester_pays=True) + self.client.force_login(self.user) + response = self.client.get(workspace.get_absolute_url()) + self.assertContains(response, "Requester pays") + + def test_is_requester_pays_false(self): + """An indicator of whether a workspace is requester_pays appears on the page.""" + workspace = factories.DefaultWorkspaceDataFactory.create(workspace__is_requester_pays=False) + self.client.force_login(self.user) + response = self.client.get(workspace.get_absolute_url()) + self.assertNotContains(response, "Requester pays") + def test_clone_links_with_two_registered_workspace_adapters(self): """Links to clone into each type of workspace appear when there are two registered workspace types.""" workspace_adapter_registry.register(TestWorkspaceAdapter) From 55a2702e7b68a967674181cab8c98012ede416f4 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 8 Mar 2024 14:20:19 -0800 Subject: [PATCH 3/6] Add is_requester_pays field to workspace form --- anvil_consortium_manager/forms.py | 1 + anvil_consortium_manager/models.py | 1 + anvil_consortium_manager/tests/test_forms.py | 12 ++++++++++++ anvil_consortium_manager/tests/test_views.py | 4 ++-- 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/anvil_consortium_manager/forms.py b/anvil_consortium_manager/forms.py index 0e291e0d..b6ff9128 100644 --- a/anvil_consortium_manager/forms.py +++ b/anvil_consortium_manager/forms.py @@ -158,6 +158,7 @@ class Meta: "billing_project", "name", "authorization_domains", + "is_requester_pays", "note", ) widgets = { diff --git a/anvil_consortium_manager/models.py b/anvil_consortium_manager/models.py index a318620e..394d4b1d 100644 --- a/anvil_consortium_manager/models.py +++ b/anvil_consortium_manager/models.py @@ -653,6 +653,7 @@ class Workspace(TimeStampedModel): default=False, ) is_requester_pays = models.BooleanField( + verbose_name="Requester pays", help_text="Indicator of whether the workspace is set to requester pays.", default=False, ) diff --git a/anvil_consortium_manager/tests/test_forms.py b/anvil_consortium_manager/tests/test_forms.py index 598d9de2..f248dce0 100644 --- a/anvil_consortium_manager/tests/test_forms.py +++ b/anvil_consortium_manager/tests/test_forms.py @@ -328,6 +328,18 @@ def test_valid_with_note(self): form = self.form_class(data=form_data) self.assertTrue(form.is_valid()) + def test_valid_with_is_requester_pays(self): + """Form is valid with necessary input and note is specified.""" + billing_project = factories.BillingProjectFactory.create() + form_data = { + "billing_project": billing_project, + "name": "test-workspace", + "note": "test note", + "is_requester_pays": True, + } + form = self.form_class(data=form_data) + self.assertTrue(form.is_valid()) + def test_invalid_missing_billing_project(self): """Form is invalid when missing billing_project_name.""" form_data = {"name": "test-workspace"} diff --git a/anvil_consortium_manager/tests/test_views.py b/anvil_consortium_manager/tests/test_views.py index 3e46fdae..1342027a 100644 --- a/anvil_consortium_manager/tests/test_views.py +++ b/anvil_consortium_manager/tests/test_views.py @@ -10634,7 +10634,7 @@ def test_form_fields(self): self.client.force_login(self.user) response = self.client.get(self.get_url(self.workspace.billing_project.name, self.workspace.name)) form = response.context_data.get("form") - self.assertEqual(len(form.fields), 1) + self.assertEqual(len(form.fields), 2) # is_requester_pays and is_locked self.assertIn("note", form.fields) def test_has_formset_in_context(self): @@ -10793,7 +10793,7 @@ def test_custom_adapter_workspace_form(self): self.assertTrue("form" in response.context_data) form = response.context_data["form"] self.assertIsInstance(form, TestWorkspaceAdapter().get_workspace_form_class()) - self.assertEqual(len(form.fields), 1) + self.assertEqual(len(form.fields), 2) # is_requester_pays and is_locked self.assertIn("note", form.fields) def test_get_workspace_data_with_second_foreign_key_to_workspace(self): From 3e323d1315dcd71b04bff3c4778f338a870632c3 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 8 Mar 2024 16:23:09 -0800 Subject: [PATCH 4/6] Verify requester pays status in AnVIL audits --- anvil_consortium_manager/anvil_api.py | 7 +- anvil_consortium_manager/audit/audit.py | 14 +- anvil_consortium_manager/tests/test_audit.py | 312 +++++++++++++++++++ anvil_consortium_manager/tests/test_views.py | 31 ++ 4 files changed, 361 insertions(+), 3 deletions(-) diff --git a/anvil_consortium_manager/anvil_api.py b/anvil_consortium_manager/anvil_api.py index a209a640..ede2ed62 100644 --- a/anvil_consortium_manager/anvil_api.py +++ b/anvil_consortium_manager/anvil_api.py @@ -237,7 +237,7 @@ def list_workspaces(self, fields=None): else: return self.auth_session.get(url, 200) - def get_workspace(self, workspace_namespace, workspace_name): + def get_workspace(self, workspace_namespace, workspace_name, fields=None): """Get information about a specific workspace on AnVIL. Calls the Rawls /api/workspaces/{workspace_namespace}/{workspace_name} GET method. @@ -250,7 +250,10 @@ def get_workspace(self, workspace_namespace, workspace_name): requests.Response """ url = self.rawls_entry_point + "/api/workspaces/" + workspace_namespace + "/" + workspace_name - return self.auth_session.get(url, 200) + if fields: + return self.auth_session.get(url, 200, params={"fields": fields}) + else: + return self.auth_session.get(url, 200) def create_workspace(self, workspace_namespace, workspace_name, authorization_domains=[]): """Create a workspace on AnVIL. diff --git a/anvil_consortium_manager/audit/audit.py b/anvil_consortium_manager/audit/audit.py index 6e1583eb..51711c5f 100644 --- a/anvil_consortium_manager/audit/audit.py +++ b/anvil_consortium_manager/audit/audit.py @@ -363,6 +363,9 @@ class WorkspaceAudit(AnVILAudit): ERROR_DIFFERENT_LOCK = "Workspace lock status does not match on AnVIL" """Error when the workspace.is_locked status does not match the lock status on AnVIL.""" + ERROR_DIFFERENT_REQUESTER_PAYS = "Workspace bucket requester_pays status does not match on AnVIL" + """Error when the workspace.is_locked status does not match the lock status on AnVIL.""" + def run_audit(self): """Run an audit on Workspaces in the app.""" # Check the list of workspaces. @@ -370,7 +373,8 @@ def run_audit(self): "workspace.namespace", "workspace.name", "workspace.authorizationDomain", - "workspace.isLocked,accessLevel", + "workspace.isLocked", + "accessLevel", ] response = AnVILAPIClient().list_workspaces(fields=",".join(fields)) workspaces_on_anvil = response.json() @@ -408,6 +412,14 @@ def run_audit(self): # Check lock status. if workspace.is_locked != workspace_details["workspace"]["isLocked"]: model_instance_result.add_error(self.ERROR_DIFFERENT_LOCK) + # Check is_requester_pays status. Unfortunately we have to make a separate API call. + response = AnVILAPIClient().get_workspace( + workspace.billing_project.name, + workspace.name, + fields=["bucketOptions"], + ) + if workspace.is_requester_pays != response.json()["bucketOptions"]["requesterPays"]: + model_instance_result.add_error(self.ERROR_DIFFERENT_REQUESTER_PAYS) self.add_result(model_instance_result) diff --git a/anvil_consortium_manager/tests/test_audit.py b/anvil_consortium_manager/tests/test_audit.py index 3ae49e0e..d0453fbc 100644 --- a/anvil_consortium_manager/tests/test_audit.py +++ b/anvil_consortium_manager/tests/test_audit.py @@ -2696,6 +2696,13 @@ def get_api_workspace_acl_response(self): } } + def get_api_bucket_options_url(self, billing_project_name, workspace_name): + return self.api_client.rawls_entry_point + "/api/workspaces/" + billing_project_name + "/" + workspace_name + + def get_api_bucket_options_response(self): + """Return a json for the workspace/acl method that is not requester pays.""" + return {"bucketOptions": {"requesterPays": False}} + def test_anvil_audit_no_workspaces(self): """anvil_audit works correct if there are no Workspaces in the app.""" api_url = self.get_api_url() @@ -2730,6 +2737,14 @@ def test_anvil_audit_one_workspace_no_errors(self): status=200, json=self.get_api_workspace_acl_response(), ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace.billing_project.name, workspace.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertTrue(audit_results.ok()) @@ -2769,6 +2784,14 @@ def test_anvil_audit_one_workspace_owner_in_app_reader_on_anvil(self): status=200, json=[self.get_api_workspace_json(workspace.billing_project.name, workspace.name, "READER")], ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace.billing_project.name, workspace.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertFalse(audit_results.ok()) @@ -2789,6 +2812,14 @@ def test_anvil_audit_one_workspace_owner_in_app_writer_on_anvil(self): status=200, json=[self.get_api_workspace_json(workspace.billing_project.name, workspace.name, "WRITER")], ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace.billing_project.name, workspace.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertFalse(audit_results.ok()) @@ -2824,6 +2855,14 @@ def test_anvil_audit_one_workspace_is_locked_in_app_not_on_anvil(self): status=200, json=self.get_api_workspace_acl_response(), ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace.billing_project.name, workspace.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertFalse(audit_results.ok()) @@ -2859,6 +2898,14 @@ def test_anvil_audit_one_workspace_is_not_locked_in_app_but_is_on_anvil(self): status=200, json=self.get_api_workspace_acl_response(), ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace.billing_project.name, workspace.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertFalse(audit_results.ok()) @@ -2869,6 +2916,89 @@ def test_anvil_audit_one_workspace_is_not_locked_in_app_but_is_on_anvil(self): self.assertFalse(record_result.ok()) self.assertEqual(record_result.errors, set([audit_results.ERROR_DIFFERENT_LOCK])) + def test_anvil_audit_one_workspace_is_requester_pays_in_app_not_on_anvil(self): + """anvil_audit raises exception if workspace is requester_pays in the app but not on AnVIL.""" + workspace = factories.WorkspaceFactory.create(is_requester_pays=True) + api_url = self.get_api_url() + self.anvil_response_mock.add( + responses.GET, + api_url, + status=200, + json=[ + self.get_api_workspace_json( + workspace.billing_project.name, + workspace.name, + "OWNER", + is_locked=False, + ) + ], + ) + # Response to check workspace access. + workspace_acl_url = self.get_api_workspace_acl_url(workspace.billing_project.name, workspace.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_workspace_acl_response(), + ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace.billing_project.name, workspace.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) + audit_results = audit.WorkspaceAudit() + audit_results.run_audit() + self.assertFalse(audit_results.ok()) + self.assertEqual(len(audit_results.get_verified_results()), 0) + self.assertEqual(len(audit_results.get_error_results()), 1) + self.assertEqual(len(audit_results.get_not_in_app_results()), 0) + record_result = audit_results.get_result_for_model_instance(workspace) + self.assertFalse(record_result.ok()) + self.assertEqual(record_result.errors, set([audit_results.ERROR_DIFFERENT_REQUESTER_PAYS])) + + def test_anvil_audit_one_workspace_is_not_requester_pays_in_app_but_is_on_anvil(self): + """anvil_audit raises exception if workspace is requester_pays in the app but not on AnVIL.""" + workspace = factories.WorkspaceFactory.create(is_requester_pays=False) + api_url = self.get_api_url() + self.anvil_response_mock.add( + responses.GET, + api_url, + status=200, + json=[ + self.get_api_workspace_json( + workspace.billing_project.name, + workspace.name, + "OWNER", + is_locked=False, + ) + ], + ) + # Response to check workspace access. + workspace_acl_url = self.get_api_workspace_acl_url(workspace.billing_project.name, workspace.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_workspace_acl_response(), + ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace.billing_project.name, workspace.name) + response = self.get_api_bucket_options_response() + response["bucketOptions"]["requesterPays"] = True + self.anvil_response_mock.add(responses.GET, workspace_acl_url, status=200, json=response) + audit_results = audit.WorkspaceAudit() + audit_results.run_audit() + self.assertFalse(audit_results.ok()) + self.assertEqual(len(audit_results.get_verified_results()), 0) + self.assertEqual(len(audit_results.get_error_results()), 1) + self.assertEqual(len(audit_results.get_not_in_app_results()), 0) + record_result = audit_results.get_result_for_model_instance(workspace) + self.assertFalse(record_result.ok()) + self.assertEqual(record_result.errors, set([audit_results.ERROR_DIFFERENT_REQUESTER_PAYS])) + def test_anvil_audit_two_workspaces_no_errors(self): """anvil_audit returns None if if two workspaces exist in both the app and AnVIL.""" workspace_1 = factories.WorkspaceFactory.create() @@ -2899,6 +3029,22 @@ def test_anvil_audit_two_workspaces_no_errors(self): status=200, json=self.get_api_workspace_acl_response(), ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace_1.billing_project.name, workspace_1.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace_2.billing_project.name, workspace_2.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertTrue(audit_results.ok()) @@ -2940,6 +3086,22 @@ def test_anvil_audit_two_groups_json_response_order_does_not_matter(self): status=200, json=self.get_api_workspace_acl_response(), ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace_1.billing_project.name, workspace_1.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace_2.billing_project.name, workspace_2.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertTrue(audit_results.ok()) @@ -2972,6 +3134,14 @@ def test_anvil_audit_two_workspaces_first_not_on_anvil(self): status=200, json=self.get_api_workspace_acl_response(), ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace_2.billing_project.name, workspace_2.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertFalse(audit_results.ok()) @@ -3006,6 +3176,22 @@ def test_anvil_audit_two_workspaces_first_different_access(self): status=200, json=self.get_api_workspace_acl_response(), ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace_1.billing_project.name, workspace_1.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace_2.billing_project.name, workspace_2.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertFalse(audit_results.ok()) @@ -3163,6 +3349,16 @@ def test_one_workspace_one_auth_domain(self): status=200, json=self.get_api_workspace_acl_response(), ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url( + auth_domain.workspace.billing_project.name, auth_domain.workspace.name + ) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertTrue(audit_results.ok()) @@ -3199,6 +3395,14 @@ def test_one_workspace_two_auth_domains(self): status=200, json=self.get_api_workspace_acl_response(), ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace.billing_project.name, workspace.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertTrue(audit_results.ok()) @@ -3235,6 +3439,14 @@ def test_one_workspace_two_auth_domains_order_does_not_matter(self): status=200, json=self.get_api_workspace_acl_response(), ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace.billing_project.name, workspace.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertTrue(audit_results.ok()) @@ -3269,6 +3481,14 @@ def test_one_workspace_no_auth_domain_in_app_one_auth_domain_on_anvil(self): status=200, json=self.get_api_workspace_acl_response(), ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace.billing_project.name, workspace.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertFalse(audit_results.ok()) @@ -3306,6 +3526,16 @@ def test_one_workspace_one_auth_domain_in_app_no_auth_domain_on_anvil(self): status=200, json=self.get_api_workspace_acl_response(), ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url( + auth_domain.workspace.billing_project.name, auth_domain.workspace.name + ) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertFalse(audit_results.ok()) @@ -3341,6 +3571,14 @@ def test_one_workspace_no_auth_domain_in_app_two_auth_domains_on_anvil(self): status=200, json=self.get_api_workspace_acl_response(), ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace.billing_project.name, workspace.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertFalse(audit_results.ok()) @@ -3378,6 +3616,14 @@ def test_one_workspace_two_auth_domains_in_app_no_auth_domain_on_anvil(self): status=200, json=self.get_api_workspace_acl_response(), ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace.billing_project.name, workspace.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertFalse(audit_results.ok()) @@ -3415,6 +3661,14 @@ def test_one_workspace_two_auth_domains_in_app_one_auth_domain_on_anvil(self): status=200, json=self.get_api_workspace_acl_response(), ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace.billing_project.name, workspace.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertFalse(audit_results.ok()) @@ -3452,6 +3706,16 @@ def test_one_workspace_different_auth_domains(self): status=200, json=self.get_api_workspace_acl_response(), ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url( + auth_domain.workspace.billing_project.name, auth_domain.workspace.name + ) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertFalse(audit_results.ok()) @@ -3502,6 +3766,22 @@ def test_two_workspaces_first_auth_domains_do_not_match(self): status=200, json=self.get_api_workspace_acl_response(), ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace_1.billing_project.name, workspace_1.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace_2.billing_project.name, workspace_2.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertFalse(audit_results.ok()) @@ -3554,6 +3834,22 @@ def test_two_workspaces_auth_domains_do_not_match_for_both(self): status=200, json=self.get_api_workspace_acl_response(), ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace_1.billing_project.name, workspace_1.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace_2.billing_project.name, workspace_2.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertFalse(audit_results.ok()) @@ -3578,6 +3874,14 @@ def test_one_workspace_with_two_errors(self): status=200, json=[self.get_api_workspace_json(workspace.billing_project.name, workspace.name, "READER")], ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace.billing_project.name, workspace.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertFalse(audit_results.ok()) @@ -3622,6 +3926,14 @@ def test_fails_sharing_audit(self): status=200, json=self.get_api_workspace_acl_response(), ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace.billing_project.name, workspace.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) audit_results = audit.WorkspaceAudit() audit_results.run_audit() self.assertFalse(audit_results.ok()) diff --git a/anvil_consortium_manager/tests/test_views.py b/anvil_consortium_manager/tests/test_views.py index 1342027a..06a8d1e9 100644 --- a/anvil_consortium_manager/tests/test_views.py +++ b/anvil_consortium_manager/tests/test_views.py @@ -11836,6 +11836,13 @@ def get_api_workspace_acl_response(self): } } + def get_api_bucket_options_url(self, billing_project_name, workspace_name): + return self.api_client.rawls_entry_point + "/api/workspaces/" + billing_project_name + "/" + workspace_name + + def get_api_bucket_options_response(self): + """Return a json for the workspace/acl method that is not requester pays.""" + return {"bucketOptions": {"requesterPays": False}} + def get_view(self): """Return the view being tested.""" return views.WorkspaceAudit.as_view() @@ -11924,6 +11931,14 @@ def test_audit_verified_one_record(self): status=200, json=self.get_api_workspace_acl_response(), ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace.billing_project.name, workspace.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) self.client.force_login(self.user) response = self.client.get(self.get_url()) self.assertIn("verified_table", response.context_data) @@ -11955,6 +11970,14 @@ def test_audit_errors_one_record(self): # Error - we are not an owner. json=[self.get_api_workspace_json(workspace.billing_project.name, workspace.name, "READER")], ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace.billing_project.name, workspace.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) self.client.force_login(self.user) response = self.client.get(self.get_url()) self.assertIn("error_table", response.context_data) @@ -12014,6 +12037,14 @@ def test_audit_ok_is_not_ok(self): # Error - we are not admin. json=[self.get_api_workspace_json(workspace.billing_project.name, workspace.name, "READER")], ) + # Response to check workspace bucket options. + workspace_acl_url = self.get_api_bucket_options_url(workspace.billing_project.name, workspace.name) + self.anvil_response_mock.add( + responses.GET, + workspace_acl_url, + status=200, + json=self.get_api_bucket_options_response(), + ) self.client.force_login(self.user) response = self.client.get(self.get_url()) self.assertIn("audit_ok", response.context_data) From f7c70585ae708599ebfb62111483753c95600461 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 8 Mar 2024 16:25:45 -0800 Subject: [PATCH 5/6] Update docs for is_requester_pays audit checks --- docs/auditing.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/auditing.rst b/docs/auditing.rst index 6fa226a3..9f4bf390 100644 --- a/docs/auditing.rst +++ b/docs/auditing.rst @@ -76,6 +76,8 @@ The :meth:`~anvil_consortium_manager.models.Workspace.anvil_audit` method runs t 3. The :class:`~anvil_consortium_manager.models.Workspace` has the same authorization domains in the app as on AnVIL. 4. The access to each :class:`~anvil_consortium_manager.models.Workspace` in the app matches the access on AnVIL (using :meth:`~anvil_consortium_manager.models.Workspace.anvil_audit_access` method for each Workspace). 5. No workspaces that have the app service account as an owner exist on AnVIL. + 6. The workspace ``is_locked`` status matches AnVIL. + 7. The workspace ``is_requester_pays`` status matches AnVIL. The :meth:`~anvil_consortium_manager.models.Workspace.anvil_audit_membership` method runs the following checks for a single :class:`~anvil_consortium_manager.models.Workspace` instance: From 9a966b3bb624dc2d96f644330e33acb87091db88 Mon Sep 17 00:00:00 2001 From: Adrienne Stilp Date: Fri, 8 Mar 2024 16:26:01 -0800 Subject: [PATCH 6/6] Bump version number and update CHANGELOG --- CHANGELOG.md | 1 + anvil_consortium_manager/__init__.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1807199b..4e5f105c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * Add customizeable `get_extra_detail_context_data` method for workspace adapters. Users can override this method to provide additional context data on a workspace detail page. * Add filtering by workspace type to the admin interface. * Set max_length for `ManagedGroup` and `Workspace` models to match what AnVIL allows. +* Track requester pays status for `Workspace` objects. ## 0.21.0 (2023-12-04) diff --git a/anvil_consortium_manager/__init__.py b/anvil_consortium_manager/__init__.py index 19a568ab..e30a9e18 100644 --- a/anvil_consortium_manager/__init__.py +++ b/anvil_consortium_manager/__init__.py @@ -1 +1 @@ -__version__ = "0.22.0.dev2" +__version__ = "0.22.0.dev3"