diff --git a/backend/audit/views/pre_dissemination_download_view.py b/backend/audit/views/pre_dissemination_download_view.py index 950c718359..788095b959 100644 --- a/backend/audit/views/pre_dissemination_download_view.py +++ b/backend/audit/views/pre_dissemination_download_view.py @@ -1,3 +1,4 @@ +from django.http import HttpResponse from django.shortcuts import get_object_or_404, redirect from django.views.generic import View @@ -33,7 +34,11 @@ def get(self, request, report_id): ) i2d_data = intake_to_dissem.load_all() - filename = generate_presubmission_report(i2d_data) - download_url = get_download_url(filename) + filename, workbook_bytes = generate_presubmission_report(i2d_data) + response = HttpResponse( + workbook_bytes, + content_type="application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", + ) + response["Content-Disposition"] = f"attachment; filename={filename}" - return redirect(download_url) + return response diff --git a/backend/cypress/support/auditee-certification.js b/backend/cypress/support/auditee-certification.js index 75d0938253..df4d1bb6d4 100644 --- a/backend/cypress/support/auditee-certification.js +++ b/backend/cypress/support/auditee-certification.js @@ -13,7 +13,7 @@ export function testAuditeeCertification() { // 2. Sign and date, submit and go back to checklist cy.get('#auditee_name').type('John Doe'); cy.get('#auditee_title').type('Auditee'); - cy.get('.usa-date-picker__button').click(); + cy.get('.usa-date-picker__button').trigger('click'); cy.get('.usa-date-picker__calendar__date--today').click(); cy.get('.usa-button').contains('Agree to auditee certification').click(); }; diff --git a/backend/cypress/support/auditor-certification.js b/backend/cypress/support/auditor-certification.js index e45e963d15..8e46a20a44 100644 --- a/backend/cypress/support/auditor-certification.js +++ b/backend/cypress/support/auditor-certification.js @@ -11,7 +11,7 @@ export function testAuditorCertification() { // 2. Sign and date, submit and go back to checklist cy.get('#auditor_name').type('Jane Doe'); cy.get('#auditor_title').type('Auditor'); - cy.get('.usa-date-picker__button').click(); + cy.get('.usa-date-picker__button').trigger('click'); cy.get('.usa-date-picker__calendar__date--today').click(); cy.get('.usa-button').contains('Agree to auditor certification').click(); }; diff --git a/backend/dissemination/api_versions.py b/backend/dissemination/api_versions.py index 574f015b0f..56e209f9c5 100644 --- a/backend/dissemination/api_versions.py +++ b/backend/dissemination/api_versions.py @@ -7,7 +7,7 @@ # These are API versions we want live. live = { - "dissemination": ["api_v1_0_3", "api_v1_1_0", "api_v1_1_1"], + "dissemination": ["api_v1_0_3", "api_v1_1_0"], "support": ["admin_api_v1_1_0"], } @@ -47,20 +47,6 @@ def exec_sql(location, version, filename): curs.execute(sql) -def create_materialized_view(location): - """ - Create or recreate the dissemination_combined materialized view. - We only want this done once on startup, regardless of the API version. - """ - conn = connection(get_conn_string()) - conn.autocommit = True - with conn.cursor() as curs: - path = f"{location}/sql/create_materialized_views.sql" - logger.info("EXEC SQL create_materialized_views.sql") - sql = open(path, "r").read() - curs.execute(sql) - - def create_views(location, version): exec_sql(location, version, "create_views.sql") diff --git a/backend/dissemination/management/commands/create_api_views.py b/backend/dissemination/management/commands/create_api_views.py index 4db39d481e..105b87a780 100644 --- a/backend/dissemination/management/commands/create_api_views.py +++ b/backend/dissemination/management/commands/create_api_views.py @@ -8,7 +8,6 @@ class Command(BaseCommand): """ def handle(self, *args, **kwargs): - api_versions.create_materialized_view("dissemination") api_versions.create_functions("dissemination") api_versions.create_functions("support") api_versions.create_live_views("dissemination") diff --git a/backend/dissemination/management/commands/materialized_views.py b/backend/dissemination/management/commands/materialized_views.py index 11c0b8ab14..67acc95b5d 100644 --- a/backend/dissemination/management/commands/materialized_views.py +++ b/backend/dissemination/management/commands/materialized_views.py @@ -15,10 +15,7 @@ def add_arguments(self, parser): def handle(self, *args, **options): path = "dissemination/sql" if options["create"]: - # Other API views may rely on the materialized view. So, drop and recreate them. - api_versions.drop_live_views("dissemination") api_versions.exec_sql_at_path(path, "create_materialized_views.sql") - api_versions.create_live_views("dissemination") elif options["drop"]: api_versions.exec_sql_at_path(path, "drop_materialized_views.sql") elif options["refresh"]: diff --git a/backend/dissemination/summary_reports.py b/backend/dissemination/summary_reports.py index 7921829134..48e32194fe 100644 --- a/backend/dissemination/summary_reports.py +++ b/backend/dissemination/summary_reports.py @@ -5,11 +5,7 @@ import time import openpyxl as pyxl -from boto3 import client as boto3_client -from botocore.client import ClientError, Config - from django.conf import settings -from fs.memoryfs import MemoryFS from openpyxl.workbook.defined_name import DefinedName from openpyxl.utils import quote_sheetname @@ -606,40 +602,19 @@ def create_workbook(data, protect_sheets=False): return (workbook, t1 - t0) -def persist_workbook(workbook): +def prepare_workbook_for_download(workbook): + t0 = time.time() - s3_client = boto3_client( - service_name="s3", - region_name=settings.AWS_S3_PRIVATE_REGION_NAME, - aws_access_key_id=settings.AWS_PRIVATE_ACCESS_KEY_ID, - aws_secret_access_key=settings.AWS_PRIVATE_SECRET_ACCESS_KEY, - endpoint_url=settings.AWS_S3_PRIVATE_INTERNAL_ENDPOINT, - config=Config(signature_version="s3v4"), - ) + now = datetime.utcnow().strftime("%Y%m%d%H%M%S") + filename = f"fac-summary-report-{now}.xlsx" + + # Save the workbook directly to a BytesIO object + workbook_bytes = io.BytesIO() + workbook.save(workbook_bytes) + workbook_bytes.seek(0) - with MemoryFS() as mem_fs: - now = datetime.utcnow().strftime("%Y%m%d%H%M%S") - filename = f"fac-summary-report-{now}.xlsx" - s3_dir = "temp" - - workbook_write_fp = mem_fs.openbin(filename, mode="w") - workbook.save(workbook_write_fp) - workbook_read_fp = mem_fs.openbin(filename, mode="r") - workbook_read_fp.seek(0) - content = workbook_read_fp.read() - workbook_bytes = io.BytesIO(content) - - try: - s3_client.put_object( - Body=workbook_bytes, - Bucket=settings.AWS_PRIVATE_STORAGE_BUCKET_NAME, - Key=f"{s3_dir}/{filename}", - ) - except ClientError: - logger.warn(f"Unable to put summary report file {filename} in S3!") - raise t1 = time.time() - return (f"temp/{filename}", t1 - t0) + return (filename, workbook_bytes, t1 - t0) def generate_summary_report(report_ids, include_private=False): @@ -651,12 +626,12 @@ def generate_summary_report(report_ids, include_private=False): data = separate_notes_single_fields_from_array_fields(data) (workbook, tcw) = create_workbook(data) insert_dissem_coversheet(workbook, bool(tribal_report_ids), include_private) - (filename, tpw) = persist_workbook(workbook) + (filename, workbook_bytes, tpw) = prepare_workbook_for_download(workbook) t1 = time.time() logger.info( f"SUMMARY_REPORTS generate_summary_report\n\ttotal: {t1-t0} ttri: {ttri} tgrdd: {tgrdd} tcw: {tcw} tpw: {tpw}" ) - return filename + return (filename, workbook_bytes) # Ignore performance profiling for the presub. @@ -667,9 +642,9 @@ def generate_presubmission_report(i2d_data): insert_precert_coversheet(workbook) workbook.security.workbookPassword = str(uuid.uuid4()) workbook.security.lockStructure = True - (filename, _) = persist_workbook(workbook) + (filename, workbook_bytes, _) = prepare_workbook_for_download(workbook) - return filename + return (filename, workbook_bytes) def separate_notes_single_fields_from_array_fields(data): diff --git a/backend/dissemination/test_summary_reports.py b/backend/dissemination/test_summary_reports.py index 00a62e26c9..ea29d6febe 100644 --- a/backend/dissemination/test_summary_reports.py +++ b/backend/dissemination/test_summary_reports.py @@ -25,7 +25,7 @@ def test_generate_summary_report_returns_filename(self): baker.make(FederalAward, report_id=g) self.refresh_materialized_view() - filename = generate_summary_report(report_ids) + filename, _ = generate_summary_report(report_ids) self.assertTrue(filename.startswith, "fac-summary-report-") self.assertTrue(filename.endswith, ".xlsx") diff --git a/backend/dissemination/test_views.py b/backend/dissemination/test_views.py index 22b049057d..252bb5846c 100644 --- a/backend/dissemination/test_views.py +++ b/backend/dissemination/test_views.py @@ -1,3 +1,4 @@ +import io from django.conf import settings from django.contrib.auth import get_user_model from django.test import Client, TestCase @@ -672,8 +673,8 @@ def _mock_filename(self): def _mock_download_url(self): return "http://example.com/gsa-fac-private-s3/temp/some-report-name.xlsx" - @patch("dissemination.summary_reports.persist_workbook") - def test_bad_search_returns_400(self, mock_persist_workbook): + @patch("dissemination.summary_reports.prepare_workbook_for_download") + def test_bad_search_returns_400(self, mock_prepare_workbook_for_download): """ Submitting a form with bad parameters should throw a BadRequest. """ @@ -682,8 +683,8 @@ def test_bad_search_returns_400(self, mock_persist_workbook): ) self.assertEquals(response.status_code, 400) - @patch("dissemination.summary_reports.persist_workbook") - def test_empty_results_returns_404(self, mock_persist_workbook): + @patch("dissemination.summary_reports.prepare_workbook_for_download") + def test_empty_results_returns_404(self, mock_prepare_workbook_for_download): """ Searches with no results should return a 404, not an empty excel file. """ @@ -695,89 +696,162 @@ def test_empty_results_returns_404(self, mock_persist_workbook): ) self.assertEquals(response.status_code, 404) - @patch("dissemination.views.get_download_url") - @patch("dissemination.summary_reports.persist_workbook") - def test_no_permissions_returns_404_on_private( - self, mock_persist_workbook, mock_get_download_url + @patch("dissemination.summary_reports.prepare_workbook_for_download") + def test_authorized_user_with_private_data( + self, mock_prepare_workbook_for_download ): - """ - Non-permissioned users can access private audits through the summary report post. - """ - mock_persist_workbook.return_value = self._mock_filename() - mock_get_download_url.return_value = self._mock_download_url() + """Test that an authorized user can access private data.""" + mock_filename = "mocked-report.xlsx" + mock_workbook_bytes = io.BytesIO(b"fake file content") + mock_prepare_workbook_for_download.return_value = ( + mock_filename, + mock_workbook_bytes, + 1.0, + ) general = self._make_general(is_public=False) baker.make(FederalAward, report_id=general) + baker.make(FindingText, report_id=general) self.refresh_materialized_view() - response = self.anon_client.post(self._summary_report_url(), {}) - mock_persist_workbook.assert_called_once() - self.assertRedirects( - response, - self._mock_download_url(), - status_code=302, - target_status_code=200, - fetch_redirect_response=False, - ) - - @patch("dissemination.views.get_download_url") - @patch("dissemination.summary_reports.persist_workbook") - def test_permissions_returns_file_on_private( - self, mock_persist_workbook, mock_get_download_url + + response = self.perm_client.post(self._summary_report_url(), {}) + self.assertEqual(response.status_code, 200) + self.assertEqual( + response["Content-Type"], + "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", + ) + self.assertIn( + f"attachment; filename={mock_filename}", response["Content-Disposition"] + ) + self.assertEqual(response.content, b"fake file content") + + @patch("dissemination.summary_reports.prepare_workbook_for_download") + def test_unauthorized_user_with_private_data( + self, mock_prepare_workbook_for_download ): - """ - Permissioned users receive a file if there are private results. - """ - mock_persist_workbook.return_value = self._mock_filename() - mock_get_download_url.return_value = self._mock_download_url() + """Test that an unauthorized user can still receive a file, but without private data.""" + mock_filename = "mocked-report.xlsx" + mock_workbook_bytes = io.BytesIO(b"fake file content") + mock_prepare_workbook_for_download.return_value = ( + mock_filename, + mock_workbook_bytes, + 1.0, + ) general = self._make_general(is_public=False) baker.make(FederalAward, report_id=general) + baker.make(FindingText, report_id=general) self.refresh_materialized_view() + + response = self.anon_client.post(self._summary_report_url(), {}) + self.assertEqual(response.status_code, 200) + self.assertEqual( + response["Content-Type"], + "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", + ) + self.assertIn( + f"attachment; filename={mock_filename}", response["Content-Disposition"] + ) + self.assertEqual(response.content, b"fake file content") + + @patch("dissemination.summary_reports.prepare_workbook_for_download") + def test_authorized_user_with_public_data(self, mock_prepare_workbook_for_download): + """Test that an authorized user can access public data.""" + mock_filename = "mocked-report.xlsx" + mock_workbook_bytes = io.BytesIO(b"fake file content") + mock_prepare_workbook_for_download.return_value = ( + mock_filename, + mock_workbook_bytes, + 1.0, + ) + + general = self._make_general(is_public=True) + baker.make(FederalAward, report_id=general) + baker.make(FindingText, report_id=general) + self.refresh_materialized_view() + response = self.perm_client.post(self._summary_report_url(), {}) - mock_persist_workbook.assert_called_once() - self.assertRedirects( - response, - self._mock_download_url(), - status_code=302, - target_status_code=200, - fetch_redirect_response=False, - ) - - @patch("dissemination.views.get_download_url") - @patch("dissemination.summary_reports.persist_workbook") - def test_empty_search_params_returns_file( - self, mock_persist_workbook, mock_get_download_url + self.assertEqual(response.status_code, 200) + self.assertEqual( + response["Content-Type"], + "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", + ) + self.assertIn( + f"attachment; filename={mock_filename}", response["Content-Disposition"] + ) + self.assertEqual(response.content, b"fake file content") + + @patch("dissemination.summary_reports.prepare_workbook_for_download") + def test_unauthorized_user_with_public_data( + self, mock_prepare_workbook_for_download ): + """Test that an unauthorized user can access public data.""" + mock_filename = "mocked-report.xlsx" + mock_workbook_bytes = io.BytesIO(b"fake file content") + mock_prepare_workbook_for_download.return_value = ( + mock_filename, + mock_workbook_bytes, + 1.0, + ) + + general = self._make_general(is_public=True) + baker.make(FederalAward, report_id=general) + baker.make(FindingText, report_id=general) + self.refresh_materialized_view() + + response = self.anon_client.post(self._summary_report_url(), {}) + self.assertEqual(response.status_code, 200) + self.assertEqual( + response["Content-Type"], + "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", + ) + self.assertIn( + f"attachment; filename={mock_filename}", response["Content-Disposition"] + ) + self.assertEqual(response.content, b"fake file content") + + @patch("dissemination.summary_reports.prepare_workbook_for_download") + def test_empty_search_params_returns_file(self, mock_prepare_workbook_for_download): """ File should be generated on empty search parameters ("search all"). """ - mock_persist_workbook.return_value = self._mock_filename() - mock_get_download_url.return_value = self._mock_download_url() - + mock_filename = "mocked-report.xlsx" + mock_workbook_bytes = io.BytesIO(b"fake file content") + mock_prepare_workbook_for_download.return_value = ( + mock_filename, + mock_workbook_bytes, + 1.0, + ) general = self._make_general(is_public=True) baker.make(FederalAward, report_id=general) self.refresh_materialized_view() response = self.anon_client.post(self._summary_report_url(), {}) - mock_persist_workbook.assert_called_once() - self.assertRedirects( - response, - self._mock_download_url(), - status_code=302, - target_status_code=200, - fetch_redirect_response=False, - ) - - @patch("dissemination.views.get_download_url") - @patch("dissemination.summary_reports.persist_workbook") - def test_many_results_returns_file( - self, mock_persist_workbook, mock_get_download_url - ): + + mock_prepare_workbook_for_download.assert_called_once() + + self.assertEqual(response.status_code, 200) + self.assertEqual( + response["Content-Type"], + "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", + ) + self.assertIn( + f"attachment; filename={mock_filename}", response["Content-Disposition"] + ) + self.assertEqual(response.content, b"fake file content") + + @patch("dissemination.summary_reports.prepare_workbook_for_download") + def test_many_results_returns_file(self, mock_prepare_workbook_for_download): """ File should still be generated if there are above SUMMARY_REPORT_DOWNLOAD_LIMIT total results. """ - mock_persist_workbook.return_value = self._mock_filename() - mock_get_download_url.return_value = self._mock_download_url() + mock_filename = "mocked-report.xlsx" + mock_workbook_bytes = io.BytesIO(b"fake file content") + mock_prepare_workbook_for_download.return_value = ( + mock_filename, + mock_workbook_bytes, + 1.0, + ) for i in range(4): general = self._make_general( @@ -789,11 +863,15 @@ def test_many_results_returns_file( with self.settings(SUMMARY_REPORT_DOWNLOAD_LIMIT=2): response = self.anon_client.post(self._summary_report_url(), {}) - mock_persist_workbook.assert_called_once() - self.assertRedirects( - response, - self._mock_download_url(), - status_code=302, - target_status_code=200, - fetch_redirect_response=False, + mock_prepare_workbook_for_download.assert_called_once() + + self.assertEqual(response.status_code, 200) + self.assertEqual( + response["Content-Type"], + "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", + ) + self.assertIn( + f"attachment; filename={mock_filename}", response["Content-Disposition"] ) + + self.assertEqual(response.content, b"fake file content") diff --git a/backend/dissemination/views.py b/backend/dissemination/views.py index d74a596485..49be54a841 100644 --- a/backend/dissemination/views.py +++ b/backend/dissemination/views.py @@ -6,7 +6,7 @@ from django.conf import settings from django.core.exceptions import BadRequest, ValidationError from django.core.paginator import Paginator -from django.http import Http404 +from django.http import Http404, HttpResponse from django.shortcuts import get_object_or_404, redirect, render from django.utils import timezone from django.utils.decorators import method_decorator @@ -478,10 +478,18 @@ def get(self, request, report_id): """ sac = get_object_or_404(General, report_id=report_id) include_private = include_private_results(request) - filename = generate_summary_report([sac.report_id], include_private) - download_url = get_download_url(filename) + filename, workbook_bytes = generate_summary_report( + [sac.report_id], include_private + ) + + # Create an HTTP response with the workbook file for download + response = HttpResponse( + workbook_bytes, + content_type="application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", + ) + response["Content-Disposition"] = f"attachment; filename={filename}" - return redirect(download_url) + return response class MultipleSummaryReportDownloadView(View): @@ -508,10 +516,17 @@ def post(self, request): if len(results) == 0: raise Http404("Cannot generate summary report. No results found.") report_ids = [result.report_id for result in results] - filename = generate_summary_report(report_ids, include_private) - download_url = get_download_url(filename) + filename, workbook_bytes = generate_summary_report( + report_ids, include_private + ) + # Create an HTTP response with the workbook file for download + response = HttpResponse( + workbook_bytes, + content_type="application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", + ) + response["Content-Disposition"] = f"attachment; filename={filename}" - return redirect(download_url) + return response except Http404 as err: logger.info( @@ -520,6 +535,6 @@ def post(self, request): raise Http404 from err except Exception as err: logger.info( - "Enexpected error in MultipleSummaryReportDownloadView post:\n%s", err + "Unexpected error in MultipleSummaryReportDownloadView post:\n%s", err ) raise BadRequest(err) diff --git a/backend/docker-compose-web.yml b/backend/docker-compose-web.yml index f7f9c6017e..98deabef2e 100644 --- a/backend/docker-compose-web.yml +++ b/backend/docker-compose-web.yml @@ -99,7 +99,7 @@ services: PGRST_OPENAPI_SERVER_PROXY_URI: http://127.0.0.1:3000 PGRST_DB_ANON_ROLE: anon # See https://postgrest.org/en/stable/references/api/schemas.html#multiple-schemas for multiple schemas - PGRST_DB_SCHEMAS: "api_v1_0_3, api_v1_1_0, api_v1_1_1, admin_api_v1_1_0" + PGRST_DB_SCHEMAS: "api_v1_0_3, api_v1_1_0, admin_api_v1_1_0" PGRST_JWT_SECRET: ${PGRST_JWT_SECRET:-32_chars_fallback_secret_testing} # Fallback value for testing environments depends_on: db: diff --git a/backend/docker-compose.yml b/backend/docker-compose.yml index ace79bc281..f8ee53cada 100644 --- a/backend/docker-compose.yml +++ b/backend/docker-compose.yml @@ -126,7 +126,7 @@ services: PGRST_OPENAPI_SERVER_PROXY_URI: http://127.0.0.1:3000 PGRST_DB_ANON_ROLE: anon # See https://postgrest.org/en/stable/references/api/schemas.html#multiple-schemas for multiple schemas - PGRST_DB_SCHEMAS: "api_v1_0_3, api_v1_1_0, api_v1_1_1, admin_api_v1_1_0" + PGRST_DB_SCHEMAS: "api_v1_0_3, api_v1_1_0, admin_api_v1_1_0" PGRST_JWT_SECRET: ${PGRST_JWT_SECRET:-32_chars_fallback_secret_testing} # Fallback value for testing environments # Enable this to inspect the DB plans for queries via EXPLAIN PGRST_DB_PLAN_ENABLED: ${PGRST_DB_PLAN_ENABLED:-false} diff --git a/terraform/meta/config.tf b/terraform/meta/config.tf index dd677378c1..ec4b282110 100644 --- a/terraform/meta/config.tf +++ b/terraform/meta/config.tf @@ -18,6 +18,7 @@ locals { # Mirrors @GSA-TTS/FAC-team (developers) # https://github.com/orgs/GSA-TTS/teams/fac-team/members # TODO: Automate updates via GitHub's GraphQL API + "anastasia.gradova@gsa.gov", "bret.mogilefsky@gsa.gov", "james.person@gsa.gov", "matthew.jadud@gsa.gov", diff --git a/terraform/production/production.tf b/terraform/production/production.tf index 187b50e0c6..32f18da3d6 100644 --- a/terraform/production/production.tf +++ b/terraform/production/production.tf @@ -11,7 +11,7 @@ module "production" { postgrest_instances = 4 json_params = jsonencode( { - "storage" : 50, + "storage" : 75, } ) } diff --git a/terraform/shared/modules/env/postgrest.tf b/terraform/shared/modules/env/postgrest.tf index 7cb705c586..0d0c4d7b69 100644 --- a/terraform/shared/modules/env/postgrest.tf +++ b/terraform/shared/modules/env/postgrest.tf @@ -32,7 +32,7 @@ resource "cloudfoundry_app" "postgrest" { environment = { PGRST_DB_URI : cloudfoundry_service_key.postgrest.credentials.uri - PGRST_DB_SCHEMAS : "api_v1_0_3,api_v1_1_0,api_v1_1_1,admin_api_v1_1_0" + PGRST_DB_SCHEMAS : "api_v1_0_3,api_v1_1_0,admin_api_v1_1_0" PGRST_DB_ANON_ROLE : "anon" PGRST_JWT_SECRET : var.pgrst_jwt_secret PGRST_DB_MAX_ROWS : 20000