From 7f797a277b4918a32ca1b59227b005804bfbd982 Mon Sep 17 00:00:00 2001 From: Laura Beaufort Date: Mon, 4 Nov 2024 13:31:53 -0500 Subject: [PATCH 01/24] Update pull_request_template.md ticket and related PRs only --- pull_request_template.md | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/pull_request_template.md b/pull_request_template.md index 13f8c09f2b..9a51b72863 100644 --- a/pull_request_template.md +++ b/pull_request_template.md @@ -1,11 +1,3 @@ -## Summary - -- Issue api# and/or app# +Ticket link: -(Include a summary of proposed changes) - -## How to test - -(Include any information that may be helpful to the reviewer(s). This might include links to sample pages to test or any local environmental setup that is unusual such as environment variable (never credentials), API version to point to, etc) - -- +Related PRs: From 833174fcb5d54dd879fc53c16297443bd4912664 Mon Sep 17 00:00:00 2001 From: Elaine Krauss Date: Tue, 5 Nov 2024 15:51:09 -0500 Subject: [PATCH 02/24] Re-adds the command that runs 'create_committee_view' on server start --- bin/run-api.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/bin/run-api.sh b/bin/run-api.sh index 2aecbf753d..e3ba28d6e2 100755 --- a/bin/run-api.sh +++ b/bin/run-api.sh @@ -2,4 +2,5 @@ cd django-backend # Run migrations and application ./manage.py migrate --no-input --traceback --verbosity 3 && + python manage.py create_committee_views && exec gunicorn --bind 0.0.0.0:8080 fecfiler.wsgi -w 9 From ff70cddc6639c97489e0514ad4a942b56f8b6932 Mon Sep 17 00:00:00 2001 From: toddlees <97105825+toddlees@users.noreply.github.com> Date: Tue, 5 Nov 2024 16:12:14 -0500 Subject: [PATCH 03/24] Update run-api.sh --- bin/run-api.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/bin/run-api.sh b/bin/run-api.sh index e3ba28d6e2..7ebbab4cc9 100755 --- a/bin/run-api.sh +++ b/bin/run-api.sh @@ -4,3 +4,4 @@ cd django-backend ./manage.py migrate --no-input --traceback --verbosity 3 && python manage.py create_committee_views && exec gunicorn --bind 0.0.0.0:8080 fecfiler.wsgi -w 9 +# Runs create_committee_views to update view definition in case it's changed From 3629624ecfd3908fcbf34f8ae0256a6a3e9fd405 Mon Sep 17 00:00:00 2001 From: toddlees Date: Tue, 5 Nov 2024 16:36:51 -0500 Subject: [PATCH 04/24] remove comment --- .circleci/config.yml | 2 +- bin/run-api.sh | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 63274b588b..5afd0425d4 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -239,7 +239,7 @@ workflows: - test filters: branches: - only: /develop|release\/sprint-[\.\d]+|main/ + only: /develop|release\/sprint-[\.\d]+|main|remove-comment/ - docs-build: requires: - test diff --git a/bin/run-api.sh b/bin/run-api.sh index 7ebbab4cc9..e3ba28d6e2 100755 --- a/bin/run-api.sh +++ b/bin/run-api.sh @@ -4,4 +4,3 @@ cd django-backend ./manage.py migrate --no-input --traceback --verbosity 3 && python manage.py create_committee_views && exec gunicorn --bind 0.0.0.0:8080 fecfiler.wsgi -w 9 -# Runs create_committee_views to update view definition in case it's changed From efecadd32791a54ca61ff934485f8ea07eb7ca43 Mon Sep 17 00:00:00 2001 From: toddlees Date: Tue, 5 Nov 2024 16:45:07 -0500 Subject: [PATCH 05/24] undo deploy change --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 5afd0425d4..63274b588b 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -239,7 +239,7 @@ workflows: - test filters: branches: - only: /develop|release\/sprint-[\.\d]+|main|remove-comment/ + only: /develop|release\/sprint-[\.\d]+|main/ - docs-build: requires: - test From c00ffa0f910f76deffae2e4f1d74e12be950b52e Mon Sep 17 00:00:00 2001 From: Elaine Krauss Date: Wed, 6 Nov 2024 10:58:12 -0500 Subject: [PATCH 06/24] Relocates json_schema_to_django_model.py to the validate repo --- .../scripts/json_schema_to_django_model.py | 255 ------------------ sonar-project.properties | 2 +- 2 files changed, 1 insertion(+), 256 deletions(-) delete mode 100644 django-backend/scripts/json_schema_to_django_model.py diff --git a/django-backend/scripts/json_schema_to_django_model.py b/django-backend/scripts/json_schema_to_django_model.py deleted file mode 100644 index b9ad6c5fbe..0000000000 --- a/django-backend/scripts/json_schema_to_django_model.py +++ /dev/null @@ -1,255 +0,0 @@ -"""Converts JSON schema to a workable django model - -This is a utility script to create *starter* django model from -a JSON schema file - -The JSON schema standard can be found here: - -http://json-schema.org/ - -Note: Schema properties prefixed with "fec_" are not part of the JSON schema -standard and are specific to the FEC data. -""" -import json -import argparse -import os - -import structlog - -logger = structlog.get_logger(__name__) - - -def determine_model_name(model_id=None, filename=None): - """ - Get the model name - :param model_id: str, model id - :param filename: str, filename - :return: str, model name - """ - model_name = "" - if model_id: - try: - model_name = model_id.split("/")[-1].replace(".json", "") - except Exception as e: - logger.exception("Unhandled exception {}".format(e)) - - if not model_name and filename: - filename = filename.strip(os.sep) - model_name = filename.split(os.sep)[-1] - model_name = model_name.split(".")[0] - - return model_name.capitalize() or "UnknownModel" - - -def get_required_string( - key_name, required_fields, field_type="string", is_pk_field=False -): - """ - Gets the required portion of model field - :param key_name: - :param required_fields: - :return: str, required model string - """ - if is_pk_field: - return "primary_key=True" - - if key_name in required_fields: - return "null=False, blank=False" - return "null=True, blank=True" - - -def parse_model(json_model): # noqa - - # Make sure not list, but object - if json_model["type"] != "object": - print( - "Model type has to be object to convert to model, got {}".format( - json_model["type"] - ) - ) - - if "oneOf" in json_model: - print("Optional required fields detected: {}".format(json_model["oneOf"])) - - # Default model string - model_str = """ - from django.db import models - from fecfiler.soft_delete.models import SoftDeleteModel - from django.models import json\n - """ - - model_name = determine_model_name(json_model.get("id"), args.filename) - model_str += "class {}(SoftDeleteModel):\n".format(model_name) - model_str += ' """Generated model from json schema"""\n' - print("Model name is {}".format(model_name)) - - if "title" in json_model: - print("Title of model is {}".format(json_model["title"])) - - if "description" in json_model: - print("Description of model is {}".format(json_model["description"])) - - required_fields = [] - if "required" in json_model: - required_fields = json_model["required"] - - for key_name, key_attributes in json_model["properties"].items(): - if key_name.endswith("_id") and key_name != "_id": - print("WARNING: Possible ForeignKey {}".format(key_name)) - - if key_attributes["type"] == "null": - print( - "ERROR: Unsupported type null, skipping for field {}".format(key_name) - ) - - # PK field - is_pk_field = False - if key_name in ["id", "_id"]: - is_pk_field = True - - # If required field - required_str = get_required_string( - key_name, required_fields, key_attributes["type"], is_pk_field - ) - field_str = "" - - # String choice field, enum - if key_attributes["type"] == "string" and "enum" in key_attributes: - if not key_attributes["enum"]: - print( - "ERROR: Missing enum for enum choice field {}, skipping..".format( - key_name - ) - ) - continue - - if len(key_attributes["enum"]) == 1: - print( - "WARNING: enum value with single choice for field {}, choice {}." - "".format(key_name, key_attributes["enum"]) - ) - continue - - # Max length find - max_length = 255 - for choice in key_attributes["enum"]: - if len(choice) > 255: - max_length = len(choice) - - choices = tuple(set(zip(key_attributes["enum"], key_attributes["enum"]))) - - field_str = ( - " {} = models.CharField(choices={}, max_length={}, " - "default='{}', {})\n" - "".format( - key_name, - choices, - max_length, - key_attributes["enum"][0], - required_str, - ) - ) - - # Date time field - elif ( - key_attributes["type"] == "string" - and key_attributes.get("format") == "date-time" - ): - auto_now_add = False - editable = True - if key_name in ["created_on", "modified_on"]: - auto_now_add = True - editable = False - - field_str = ( - " {} = models.DateTimeField(auto_now_add={}, editable={}, {})\n" - "".format(key_name, auto_now_add, editable, required_str) - ) - - elif key_attributes["type"] == "integer": - max_length = key_attributes.get("maxLength") - if max_length is not None: - field_str = " {} = models.IntegerField({}, max_length={})\n".format( - key_name, required_str, max_length - ) - else: - field_str = " {} = models.IntegerField({})\n".format( - key_name, required_str - ) - - elif key_attributes["type"] == "string": - field_str = " {} = models.TextField({})\n".format(key_name, required_str) - - elif key_attributes["type"] == "number": - field_str = " {} = models.IntegerField({})\n".format( - key_name, required_str - ) - - elif key_attributes["type"] == "array": - field_str = " {} = json.JSONField(default=[], {})\n".format( - key_name, required_str - ) - - elif key_attributes["type"] == "object": - field_str = " {} = json.JSONField(default={{}}, {})\n".format( - key_name, required_str - ) - - elif key_attributes["type"] == "boolean": - field_str = " {} = models.BooleanField(default=False, {})\n".format( - key_name, required_str - ) - - model_str += field_str - - # add created and updated fields - model_str += " created = models.DateTimeField(auto_now_add=True)\n" - model_str += " updated = models.DateTimeField(auto_now=True)\n" - model_str += "\n class Meta:\n db_table = '{}s'\n".format( - model_name.lower() - ) - - return model_name, model_str - - -if __name__ == "__main__": - parser = argparse.ArgumentParser() - parser.add_argument("filename") - args = parser.parse_args() - filename = args.filename - - with open(filename) as f: - json_model = json.load(f) - - if ( - json_model["properties"].get("form_type") is not None - and "S" == json_model["properties"]["form_type"]["examples"][0][0] - and "transaction_type_identifier" not in json_model["properties"] - ): - print( - "We've detected that this schema is likely a" - " schedule and yet it has no transaction_type_identifier field." - ) - print("Would you like us to add this field (y/N)?") - choice = input().lower() - add_tti = choice in ["y", "ye", "yes"] - - if add_tti: - json_model["properties"]["transaction_type_identifier"] = { - "title": "TRANSACTION TYPE IDENTIFIER", - "description": "", - "type": "string", - "minLength": 0, - "maxLength": 12, - "pattern": "^[ A-z0-9]{0,12}$", - "examples": ["IK_PAC_REC"], - "fec_form_line": "0", - "fec_type": "A/N-12", - "fec_requiredErrorLevel": "X (error)", - } - - model_name, model_str = parse_model(json_model) - f = open(model_name + ".py", "w") - f.write(model_str) - f.close() - print("Done") diff --git a/sonar-project.properties b/sonar-project.properties index 67ceb88ef6..cd67b38a65 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -12,7 +12,7 @@ sonar.sources=django-backend #sonar.sourceEncoding=UTF-8 # Exclude utility script from coverage -sonar.coverage.exclusions=**/json_schema_to_django_model.py,**/migrations/**,**/settings/*.py +sonar.coverage.exclusions=**/migrations/**,**/settings/*.py sonar.python.coverage.reportPaths=coverage-reports/coverage.xml sonar.python.bandit.reportPaths=bandit.out sonar.python.flake8.reportPaths=flake8.out From 282d506718aa596b0569b05b6a7a7dc7d0c6fb7b Mon Sep 17 00:00:00 2001 From: Elaine Krauss Date: Wed, 6 Nov 2024 12:29:57 -0500 Subject: [PATCH 07/24] Reverts to deleting Committee DB Views and then making new ones --- django-backend/fecfiler/committee_accounts/utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/django-backend/fecfiler/committee_accounts/utils.py b/django-backend/fecfiler/committee_accounts/utils.py index aa18c833e0..d61efc0716 100644 --- a/django-backend/fecfiler/committee_accounts/utils.py +++ b/django-backend/fecfiler/committee_accounts/utils.py @@ -132,7 +132,10 @@ def create_committee_view(committee_uuid): .query.sql_with_params() ) definition = cursor.mogrify(sql, params).decode("utf-8") - cursor.execute(f"CREATE OR REPLACE VIEW {view_name} as {definition}") + cursor.execute( + f"DROP VIEW IF EXISTS {view_name};" + f"CREATE VIEW {view_name} as {definition}" + ) def get_committee_account_data(committee_id): From 6639566884bc7cc3079054f0682af0be21a89521 Mon Sep 17 00:00:00 2001 From: Elaine Krauss Date: Wed, 6 Nov 2024 12:39:54 -0500 Subject: [PATCH 08/24] Adds additional logging --- .../management/commands/create_committee_views.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/django-backend/fecfiler/committee_accounts/management/commands/create_committee_views.py b/django-backend/fecfiler/committee_accounts/management/commands/create_committee_views.py index 7565e88fe7..ca7a11fe6d 100644 --- a/django-backend/fecfiler/committee_accounts/management/commands/create_committee_views.py +++ b/django-backend/fecfiler/committee_accounts/management/commands/create_committee_views.py @@ -8,6 +8,9 @@ class Command(BaseCommand): def handle(self, *args, **options): for committee in CommitteeAccount.objects.all(): + self.stdout.write( + self.style.NOTICE(f"Running create_committee_view on {committee.id}") + ) create_committee_view(committee.id) self.stdout.write( From b37a0cdcc04fa37f525835eb19293c473b0a89e6 Mon Sep 17 00:00:00 2001 From: Elaine Krauss Date: Wed, 6 Nov 2024 12:45:15 -0500 Subject: [PATCH 09/24] Converts logging to Structlog for consistency --- .../management/commands/create_committee_views.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/django-backend/fecfiler/committee_accounts/management/commands/create_committee_views.py b/django-backend/fecfiler/committee_accounts/management/commands/create_committee_views.py index ca7a11fe6d..8382130d5a 100644 --- a/django-backend/fecfiler/committee_accounts/management/commands/create_committee_views.py +++ b/django-backend/fecfiler/committee_accounts/management/commands/create_committee_views.py @@ -2,17 +2,18 @@ from fecfiler.committee_accounts.models import CommitteeAccount from fecfiler.committee_accounts.utils import create_committee_view +import structlog + + +logger = structlog.get_logger(__name__) + class Command(BaseCommand): help = "Create or update committee views" def handle(self, *args, **options): for committee in CommitteeAccount.objects.all(): - self.stdout.write( - self.style.NOTICE(f"Running create_committee_view on {committee.id}") - ) + logger.info(f"Running create_committee_view on {committee.id}") create_committee_view(committee.id) - self.stdout.write( - self.style.SUCCESS("Successfully created/updated committee views") - ) + logger.info("Successfully created/updated committee views") From 0d82856d3d059fe1755cbad3f587bbe450e84798 Mon Sep 17 00:00:00 2001 From: Sasha Dresden Date: Fri, 8 Nov 2024 12:24:23 -0500 Subject: [PATCH 10/24] Update blocking_reports to use list instead of list() --- ...012_alter_transactions_blocking_reports.py | 27 +++++++++++++++++++ .../fecfiler/transactions/models.py | 2 +- 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 django-backend/fecfiler/transactions/migrations/0012_alter_transactions_blocking_reports.py diff --git a/django-backend/fecfiler/transactions/migrations/0012_alter_transactions_blocking_reports.py b/django-backend/fecfiler/transactions/migrations/0012_alter_transactions_blocking_reports.py new file mode 100644 index 0000000000..2fc955acd5 --- /dev/null +++ b/django-backend/fecfiler/transactions/migrations/0012_alter_transactions_blocking_reports.py @@ -0,0 +1,27 @@ +from django.db import migrations, models +from django.contrib.postgres.fields import ArrayField + + +def update_blocking_reports_default(apps, schema_editor): + transaction = apps.get_model("transactions", "Transaction") + transaction._meta.get_field("blocking_reports").default = list + + +class Migration(migrations.Migration): + dependencies = [ + ("transactions", "0011_transaction_can_delete"), + ] + + operations = [ + migrations.AlterField( + model_name="transaction", + name="blocking_reports", + field=ArrayField( + base_field=models.UUIDField(), + blank=False, + default=list, + size=None, + ), + ), + migrations.RunPython(update_blocking_reports_default), + ] diff --git a/django-backend/fecfiler/transactions/models.py b/django-backend/fecfiler/transactions/models.py index 7bd6e78bc3..41243db39c 100644 --- a/django-backend/fecfiler/transactions/models.py +++ b/django-backend/fecfiler/transactions/models.py @@ -149,7 +149,7 @@ def form_type(self, value): ) # report ids of reports that have been submitted # and in doing so have blocked this transaction from being deleted - blocking_reports = ArrayField(models.UUIDField(), blank=False, default=list()) + blocking_reports = ArrayField(models.UUIDField(), blank=False, default=list) objects = TransactionManager() From 149e6bbc58dd0cacc999a0863897b79fbd4f0ff5 Mon Sep 17 00:00:00 2001 From: Elaine Krauss Date: Tue, 12 Nov 2024 15:56:56 -0500 Subject: [PATCH 11/24] Removes unused support for serving static files --- django-backend/fecfiler/settings/base.py | 13 ------------- django-backend/fecfiler/wsgi.py | 3 +-- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/django-backend/fecfiler/settings/base.py b/django-backend/fecfiler/settings/base.py index 1ddd2b5810..cb18f3b348 100644 --- a/django-backend/fecfiler/settings/base.py +++ b/django-backend/fecfiler/settings/base.py @@ -61,7 +61,6 @@ "django.contrib.contenttypes", "django.contrib.sessions", "django.contrib.messages", - "django.contrib.staticfiles", "rest_framework", "drf_spectacular", "corsheaders", @@ -200,18 +199,6 @@ USE_L10N = True USE_TZ = True -# Static files (CSS, JavaScript, Images) -# https://docs.djangoproject.com/en/1.11/howto/static-files/ - -STATIC_URL = "/static/" - -STATICFILES_DIRS = (os.path.join(BASE_DIR, "staticfiles"),) - -STATIC_ROOT = "static" - -# the sub-directories of media and static files -STATICFILES_LOCATION = "static" - REST_FRAMEWORK = { "DEFAULT_PERMISSION_CLASSES": ("rest_framework.permissions.IsAuthenticated",), "DEFAULT_AUTHENTICATION_CLASSES": ( diff --git a/django-backend/fecfiler/wsgi.py b/django-backend/fecfiler/wsgi.py index 477a8462d9..eeabd84270 100644 --- a/django-backend/fecfiler/wsgi.py +++ b/django-backend/fecfiler/wsgi.py @@ -12,6 +12,5 @@ os.environ.setdefault("DJANGO_SETTINGS_MODULE", "fecfiler.settings.production") from django.core.wsgi import get_wsgi_application # noqa -from dj_static import Cling # noqa -application = Cling(get_wsgi_application()) +application = get_wsgi_application() From 0e9c4d33f4a950849782d8dd5c856ec3c01486c1 Mon Sep 17 00:00:00 2001 From: Elaine Krauss Date: Tue, 12 Nov 2024 16:20:55 -0500 Subject: [PATCH 12/24] Removes another unused dependency --- requirements.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/requirements.txt b/requirements.txt index f58e7122b2..aa5e9c34dc 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,7 +5,6 @@ coreapi==2.3.3 coreschema==0.0.4 decorator==5.1.1 dj-database-url==2.2.0 -dj-static==0.0.6 Django==5.1.1 django-cors-headers==4.4.0 django-storages==1.14.4 @@ -21,7 +20,6 @@ MarkupSafe==2.1.5 openapi-codec==1.3.2 psycopg2-binary==2.9.9 retry==0.9.2 -static3==0.7.0 django-otp==1.5.4 zeep==4.2.1 django-structlog[celery]==8.1.0 From a22bcac9a4e5394df15dd288830220aa2f3401d4 Mon Sep 17 00:00:00 2001 From: Elaine Krauss Date: Wed, 13 Nov 2024 11:35:02 -0500 Subject: [PATCH 13/24] Replaces print() calls with logger calls and updates wording for better clarity --- ...rm1m_city_remove_form1m_committee_name_and_more.py | 11 +++++++---- .../fecfiler/transactions/tests/test_manager.py | 1 - .../fecfiler/transactions/tests/test_models.py | 4 ---- django-backend/fecfiler/transactions/views.py | 6 +++--- .../fecfiler/user/migrations/0001_initial.py | 5 ++++- 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/django-backend/fecfiler/reports/migrations/0008_remove_form1m_city_remove_form1m_committee_name_and_more.py b/django-backend/fecfiler/reports/migrations/0008_remove_form1m_city_remove_form1m_committee_name_and_more.py index 4373cbb0be..94fd45d4a1 100644 --- a/django-backend/fecfiler/reports/migrations/0008_remove_form1m_city_remove_form1m_committee_name_and_more.py +++ b/django-backend/fecfiler/reports/migrations/0008_remove_form1m_city_remove_form1m_committee_name_and_more.py @@ -1,6 +1,9 @@ # Generated by Django 4.2.10 on 2024-04-19 19:10 from django.db import migrations, models +import structlog + +logger = structlog.get_logger(__name__) def migrate_committee_data(apps, schema_editor): @@ -20,7 +23,7 @@ def migrate_committee_data(apps, schema_editor): report.zip = form.zip report.save() else: - print("Report not found for ", form) + logger.error(f"F24 Form has no corresponding report! {form}") for form in Form3x.objects.all(): report = Report.objects.filter(form_3x=form).first() @@ -32,7 +35,7 @@ def migrate_committee_data(apps, schema_editor): report.zip = form.zip report.save() else: - print("Report not found for ", form) + logger.error(f"F3X Form has no corresponding report! {form}") for form in Form99.objects.all(): report = Report.objects.filter(form_99=form).first() @@ -45,7 +48,7 @@ def migrate_committee_data(apps, schema_editor): report.zip = form.zip report.save() else: - print("Report not found for ", form) + logger.error(f"F99 Form has no corresponding report! {form}") for form in Form1m.objects.all(): report = Report.objects.filter(form_1m=form).first() @@ -58,7 +61,7 @@ def migrate_committee_data(apps, schema_editor): report.zip = form.zip report.save() else: - print("Report not found for ", form) + logger.error(f"F1M Form has no corresponding report! {form}") class Migration(migrations.Migration): diff --git a/django-backend/fecfiler/transactions/tests/test_manager.py b/django-backend/fecfiler/transactions/tests/test_manager.py index 308a509b36..56b4c98626 100644 --- a/django-backend/fecfiler/transactions/tests/test_manager.py +++ b/django-backend/fecfiler/transactions/tests/test_manager.py @@ -303,7 +303,6 @@ def test_debts(self): self.assertEqual(original_debt_view.incurred_prior, Decimal("0")) self.assertEqual(original_debt_view.payment_prior, Decimal("0")) self.assertEqual(original_debt_view.payment_amount, Decimal("3.50")) - print("DEBT SUCCESS") def test_line_label(self): create_schedule_a( diff --git a/django-backend/fecfiler/transactions/tests/test_models.py b/django-backend/fecfiler/transactions/tests/test_models.py index 3fbe5e2cf1..ac7d3c17b1 100644 --- a/django-backend/fecfiler/transactions/tests/test_models.py +++ b/django-backend/fecfiler/transactions/tests/test_models.py @@ -846,19 +846,15 @@ def test_can_delete_debt(self): m3_report.id ) m3_report.save() - print(f"reports: {m1_report.id}, {m2_report.id}, {m3_report.id}") original_debt.refresh_from_db() # self.assertFalse(original_debt.can_delete) - print(f"blocking reports {original_debt.blocking_reports}") m2_report.upload_submission = None m2_report.save() original_debt.refresh_from_db() - print(f"blocking reports {original_debt.blocking_reports}") self.assertFalse(original_debt.can_delete) m3_report.upload_submission = None m3_report.save() original_debt.refresh_from_db() - print(f"blocking reports {original_debt.blocking_reports}") self.assertTrue(original_debt.can_delete) def set_up_jf_transfer(self): diff --git a/django-backend/fecfiler/transactions/views.py b/django-backend/fecfiler/transactions/views.py index f41106a78b..e525e584d8 100644 --- a/django-backend/fecfiler/transactions/views.py +++ b/django-backend/fecfiler/transactions/views.py @@ -140,7 +140,7 @@ def get_queryset(self): def create(self, request, *args, **kwargs): with db_transaction.atomic(): saved_transaction = self.save_transaction(request.data, request) - print(f"transaction ID: {saved_transaction.id}") + logger.info(f"Created new transaction: {saved_transaction.id}") update_dependent_parent(saved_transaction) return Response(saved_transaction.id) @@ -413,12 +413,12 @@ def get_save_hook(transaction: Transaction): def stringify_queryset(qs): database_uri = os.environ.get("DATABASE_URL") if not database_uri: - print( + logger.error( """Environment variable DATABASE_URL not found. Please check your settings and try again""" ) exit(1) - print("Testing connection...") + logger.info("Testing connection...") conn = psycopg2.connect(database_uri) sql, params = qs.query.sql_with_params() with conn.cursor() as cursor: diff --git a/django-backend/fecfiler/user/migrations/0001_initial.py b/django-backend/fecfiler/user/migrations/0001_initial.py index 0043144b3d..633d44ab96 100644 --- a/django-backend/fecfiler/user/migrations/0001_initial.py +++ b/django-backend/fecfiler/user/migrations/0001_initial.py @@ -5,13 +5,16 @@ import uuid from fecfiler.shared.utilities import get_model_data from django.core.management.utils import get_random_secret_key +import structlog + +logger = structlog.get_logger(__name__) def copy_users(apps, schema_editor): if not apps.is_installed( "authentication" ): # "authentication" not in [app_name for app_name, app in apps.get_models()]: - print("no authentication app") + logger.error("The authentication app isn't installed") return OldAccount = apps.get_model("authentication", "Account") # noqa User = apps.get_model("user", "User") # noqa From 3fd9d099dd143ef1a6b3bb9899a3049f2014a0e2 Mon Sep 17 00:00:00 2001 From: Sasha Dresden Date: Thu, 14 Nov 2024 11:45:49 -0500 Subject: [PATCH 14/24] Update committee and candidate validation to limit to alphanumeric values and then use the sanitized candidate/committee id for building the url --- django-backend/fecfiler/contacts/views.py | 44 +++++++++++++++-------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/django-backend/fecfiler/contacts/views.py b/django-backend/fecfiler/contacts/views.py index 16d66acd08..4d704db0a9 100644 --- a/django-backend/fecfiler/contacts/views.py +++ b/django-backend/fecfiler/contacts/views.py @@ -40,7 +40,19 @@ def validate_and_sanitize_candidate(candidate_id): if candidate_id is None: raise AssertionError("No Candidate ID provided") - return candidate_id + if re.match(r"^[a-zA-Z0-9]+$", candidate_id): + return candidate_id + else: + raise AssertionError("Candidate ID provided invalid") + + +def validate_and_sanitize_committee(committee_id): + if committee_id is None: + raise AssertionError("No Committee ID provided") + if re.match(r"^[a-zA-Z0-9]+$", committee_id): + return committee_id + else: + raise AssertionError("Committee ID provided invalid") class ContactListPagination(pagination.PageNumberPagination): @@ -88,14 +100,13 @@ def candidate(self, request): if not candidate_id: return HttpResponseBadRequest() try: + sanitized_candidate_id = validate_and_sanitize_candidate(candidate_id) headers = {"Content-Type": "application/json"} params = { "api_key": settings.PRODUCTION_OPEN_FEC_API_KEY, "sort": "-two_year_period", } - url = FEC_API_CANDIDATE_ENDPOINT.format( - validate_and_sanitize_candidate(candidate_id) - ) + url = FEC_API_CANDIDATE_ENDPOINT.format(sanitized_candidate_id) response = requests.get(url, headers=headers, params=params).json() results = response["results"] return JsonResponse(results[0] if len(results) > 0 else {}) @@ -107,17 +118,20 @@ def committee(self, request): committee_id = request.query_params.get("committee_id") if not committee_id: return HttpResponseBadRequest() - - headers = {"Content-Type": "application/json"} - response = requests.get( - f"{settings.PRODUCTION_OPEN_FEC_API}committee/{committee_id}/", - params={"api_key": settings.PRODUCTION_OPEN_FEC_API_KEY}, - headers=headers, - ).json() - if response.get("results"): - return JsonResponse(response["results"][0]) - else: - return JsonResponse({}) + try: + sanitized_committee_id = validate_and_sanitize_committee(committee_id) + headers = {"Content-Type": "application/json"} + response = requests.get( + f"{settings.PRODUCTION_OPEN_FEC_API}committee/{sanitized_committee_id}/", + params={"api_key": settings.PRODUCTION_OPEN_FEC_API_KEY}, + headers=headers, + ).json() + if response.get("results"): + return JsonResponse(response["results"][0]) + else: + return JsonResponse({}) + except AssertionError: + return HttpResponseBadRequest() @action(detail=False) def candidate_lookup(self, request): From 879eee053c6d6b7a0bf0470e601f1b9489d554c3 Mon Sep 17 00:00:00 2001 From: Laura Beaufort Date: Thu, 14 Nov 2024 15:09:43 -0500 Subject: [PATCH 15/24] Modify run command - instance 0 runs migrations --- bin/run-api.sh | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/bin/run-api.sh b/bin/run-api.sh index e3ba28d6e2..b958f015f9 100755 --- a/bin/run-api.sh +++ b/bin/run-api.sh @@ -1,6 +1,13 @@ cd django-backend -# Run migrations and application -./manage.py migrate --no-input --traceback --verbosity 3 && - python manage.py create_committee_views && - exec gunicorn --bind 0.0.0.0:8080 fecfiler.wsgi -w 9 +# Only Instance 0 runs migrations and creates views +echo "------ Starting APP ------" +if [ $CF_INSTANCE_INDEX = "0" ]; then + echo "----- Migrating Database -----" + python manage.py migrate --no-input --traceback --verbosity 3 + echo "----- Creating committee views -----" + python manage.py create_committee_views +fi + +# Run application +exec gunicorn --bind 0.0.0.0:8080 fecfiler.wsgi -w 9 From 3d1045a7040b023618868ab6fccd212b8609b9d7 Mon Sep 17 00:00:00 2001 From: Sasha Dresden Date: Fri, 15 Nov 2024 10:28:55 -0500 Subject: [PATCH 16/24] Update committee and candidate validator to ensure exactly 9 characters long --- django-backend/fecfiler/contacts/views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/django-backend/fecfiler/contacts/views.py b/django-backend/fecfiler/contacts/views.py index 4d704db0a9..d38042c593 100644 --- a/django-backend/fecfiler/contacts/views.py +++ b/django-backend/fecfiler/contacts/views.py @@ -40,7 +40,7 @@ def validate_and_sanitize_candidate(candidate_id): if candidate_id is None: raise AssertionError("No Candidate ID provided") - if re.match(r"^[a-zA-Z0-9]+$", candidate_id): + if re.match(r"^[a-zA-Z0-9]{9}$", candidate_id): return candidate_id else: raise AssertionError("Candidate ID provided invalid") @@ -49,7 +49,7 @@ def validate_and_sanitize_candidate(candidate_id): def validate_and_sanitize_committee(committee_id): if committee_id is None: raise AssertionError("No Committee ID provided") - if re.match(r"^[a-zA-Z0-9]+$", committee_id): + if re.match(r"^[a-zA-Z0-9]{9}$", committee_id): return committee_id else: raise AssertionError("Committee ID provided invalid") From d72266af5fb450caa8791927b4cd4fa1c2ace2d9 Mon Sep 17 00:00:00 2001 From: Laura Beaufort Date: Sat, 16 Nov 2024 16:53:28 -0500 Subject: [PATCH 17/24] Add temporary default value to non-nullable fields --- ..._cashonhandyearly_cash_on_hand_and_more.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 django-backend/fecfiler/cash_on_hand/migrations/0002_alter_cashonhandyearly_cash_on_hand_and_more.py diff --git a/django-backend/fecfiler/cash_on_hand/migrations/0002_alter_cashonhandyearly_cash_on_hand_and_more.py b/django-backend/fecfiler/cash_on_hand/migrations/0002_alter_cashonhandyearly_cash_on_hand_and_more.py new file mode 100644 index 0000000000..a1dd1ae26a --- /dev/null +++ b/django-backend/fecfiler/cash_on_hand/migrations/0002_alter_cashonhandyearly_cash_on_hand_and_more.py @@ -0,0 +1,25 @@ +# Generated by Django 5.1.1 on 2024-11-16 21:51 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('cash_on_hand', '0001_initial'), + ] + + operations = [ + migrations.AlterField( + model_name='cashonhandyearly', + name='cash_on_hand', + field=models.DecimalField(decimal_places=2, default=0, max_digits=11), + preserve_default=False, + ), + migrations.AlterField( + model_name='cashonhandyearly', + name='year', + field=models.TextField(default=2020, unique=True), + preserve_default=False, + ), + ] From 98babe80ce3ab904785c49f99def1ffc088756d0 Mon Sep 17 00:00:00 2001 From: Laura Beaufort Date: Tue, 19 Nov 2024 16:27:07 -0500 Subject: [PATCH 18/24] Remove unique constraint on year field --- .../0003_alter_cashonhandyearly_year.py | 18 ++++++++++++++++++ django-backend/fecfiler/cash_on_hand/models.py | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 django-backend/fecfiler/cash_on_hand/migrations/0003_alter_cashonhandyearly_year.py diff --git a/django-backend/fecfiler/cash_on_hand/migrations/0003_alter_cashonhandyearly_year.py b/django-backend/fecfiler/cash_on_hand/migrations/0003_alter_cashonhandyearly_year.py new file mode 100644 index 0000000000..c7a3c29249 --- /dev/null +++ b/django-backend/fecfiler/cash_on_hand/migrations/0003_alter_cashonhandyearly_year.py @@ -0,0 +1,18 @@ +# Generated by Django 5.1.1 on 2024-11-19 21:03 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('cash_on_hand', '0002_alter_cashonhandyearly_cash_on_hand_and_more'), + ] + + operations = [ + migrations.AlterField( + model_name='cashonhandyearly', + name='year', + field=models.TextField(), + ), + ] diff --git a/django-backend/fecfiler/cash_on_hand/models.py b/django-backend/fecfiler/cash_on_hand/models.py index 49f2fe33f1..9eb54e8909 100644 --- a/django-backend/fecfiler/cash_on_hand/models.py +++ b/django-backend/fecfiler/cash_on_hand/models.py @@ -14,7 +14,7 @@ class CashOnHandYearly(CommitteeOwnedModel): unique=True, ) - year = models.TextField(null=False, blank=False, unique=True) + year = models.TextField(null=False, blank=False) cash_on_hand = models.DecimalField( null=False, blank=False, max_digits=11, decimal_places=2 ) From 079c0d4515db625ae6a1dcc9256f8024e1a4c92f Mon Sep 17 00:00:00 2001 From: Laura Beaufort Date: Tue, 19 Nov 2024 21:07:38 -0500 Subject: [PATCH 19/24] Add test for multiple committees overriding the same year --- .../fecfiler/cash_on_hand/tests/test_views.py | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/django-backend/fecfiler/cash_on_hand/tests/test_views.py b/django-backend/fecfiler/cash_on_hand/tests/test_views.py index 823c76ba41..9006d956ed 100644 --- a/django-backend/fecfiler/cash_on_hand/tests/test_views.py +++ b/django-backend/fecfiler/cash_on_hand/tests/test_views.py @@ -63,3 +63,29 @@ def test_set_override(self): committee_account=self.committee, year="2024" ) self.assertEqual(cash_on_hand.cash_on_hand, 2) + self.assertEqual(response.data["cash_on_hand"], "2.00") + + def test_set_override_multiple_cmtes(self): + new_committee = CommitteeAccount.objects.create( + committee_id="C00000000" + ) + + for committee in (self.committee, new_committee): + request = self.factory.post( + "/api/v1/cash_on_hand/year/2024/", {"cash_on_hand": 2} + ) + request.user = self.user + request.session = { + "committee_uuid": str(committee.id), + "committee_id": str(committee.committee_id), + } + force_authenticate(request, self.user) + response = CashOnHandYearlyViewSet.as_view({"post": "cash_on_hand_for_year"})( + request, year="2024" + ) + self.assertEqual(response.status_code, 200) + cash_on_hand = CashOnHandYearly.objects.get( + committee_account=committee, year="2024" + ) + self.assertEqual(cash_on_hand.cash_on_hand, 2) + self.assertEqual(response.data["cash_on_hand"], "2.00") From 12392df95bf84003aad4bccb47d0c8c79f903351 Mon Sep 17 00:00:00 2001 From: Laura Beaufort Date: Wed, 20 Nov 2024 09:19:45 -0500 Subject: [PATCH 20/24] Combine changes into one migration --- ...r_cashonhandyearly_cash_on_hand_and_more.py | 4 ++-- .../0003_alter_cashonhandyearly_year.py | 18 ------------------ 2 files changed, 2 insertions(+), 20 deletions(-) delete mode 100644 django-backend/fecfiler/cash_on_hand/migrations/0003_alter_cashonhandyearly_year.py diff --git a/django-backend/fecfiler/cash_on_hand/migrations/0002_alter_cashonhandyearly_cash_on_hand_and_more.py b/django-backend/fecfiler/cash_on_hand/migrations/0002_alter_cashonhandyearly_cash_on_hand_and_more.py index a1dd1ae26a..0eb4f21e18 100644 --- a/django-backend/fecfiler/cash_on_hand/migrations/0002_alter_cashonhandyearly_cash_on_hand_and_more.py +++ b/django-backend/fecfiler/cash_on_hand/migrations/0002_alter_cashonhandyearly_cash_on_hand_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 5.1.1 on 2024-11-16 21:51 +# Generated by Django 5.1.1 on 2024-11-20 09:18 from django.db import migrations, models @@ -19,7 +19,7 @@ class Migration(migrations.Migration): migrations.AlterField( model_name='cashonhandyearly', name='year', - field=models.TextField(default=2020, unique=True), + field=models.TextField(default=2020), preserve_default=False, ), ] diff --git a/django-backend/fecfiler/cash_on_hand/migrations/0003_alter_cashonhandyearly_year.py b/django-backend/fecfiler/cash_on_hand/migrations/0003_alter_cashonhandyearly_year.py deleted file mode 100644 index c7a3c29249..0000000000 --- a/django-backend/fecfiler/cash_on_hand/migrations/0003_alter_cashonhandyearly_year.py +++ /dev/null @@ -1,18 +0,0 @@ -# Generated by Django 5.1.1 on 2024-11-19 21:03 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('cash_on_hand', '0002_alter_cashonhandyearly_cash_on_hand_and_more'), - ] - - operations = [ - migrations.AlterField( - model_name='cashonhandyearly', - name='year', - field=models.TextField(), - ), - ] From 7a62d9a60569b7dc238f8434367e155ed0b66370 Mon Sep 17 00:00:00 2001 From: Laura Beaufort Date: Wed, 20 Nov 2024 10:32:54 -0500 Subject: [PATCH 21/24] Check for missing migrations --- .circleci/config.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 63274b588b..0504df8520 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -53,7 +53,13 @@ jobs: working_directory: ~/project/db - run: - name: Create/run migrations + name: Check migrations + command: | + python manage.py makemigrations --check + working_directory: ~/project/django-backend/ + + - run: + name: Run migrations command: | python manage.py migrate --no-input --traceback --verbosity 3 working_directory: ~/project/django-backend/ From e4701b8eecd46886a3c0e3bc20f1e6d53b7a6232 Mon Sep 17 00:00:00 2001 From: Laura Beaufort Date: Wed, 20 Nov 2024 10:33:12 -0500 Subject: [PATCH 22/24] Remove unused check --- .circleci/config.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 0504df8520..477d14c68c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -64,12 +64,6 @@ jobs: python manage.py migrate --no-input --traceback --verbosity 3 working_directory: ~/project/django-backend/ - # Only use SonarCloud security checking for now. - # - run: - # name: Bandit security checks - # command: | - # bandit -f json --output bandit.out -ii -ll -r . || echo "Bandit found issues" && cat bandit.out - - run: name: Run tests # Use built-in Django test module From 0581eab3aa3af629bbf0ae5e65c732686f7e653f Mon Sep 17 00:00:00 2001 From: David Heitzer Date: Thu, 21 Nov 2024 11:38:49 -0500 Subject: [PATCH 23/24] 1671 remove old debug login support --- .../fecfiler/authentication/__init__.py | 0 .../authentication/migrations/0001_initial.py | 116 ------------------ .../authentication/migrations/__init__.py | 0 .../fecfiler/authentication/models.py | 99 --------------- .../fecfiler/authentication/test_views.py | 25 ---- .../fecfiler/authentication/urls.py | 12 -- .../fecfiler/authentication/utils.py | 17 --- .../fecfiler/authentication/views.py | 74 ----------- .../middleware/TimeoutMiddleware.py | 0 django-backend/fecfiler/oidc/utils.py | 10 ++ django-backend/fecfiler/oidc/views.py | 2 +- django-backend/fecfiler/settings/base.py | 11 +- .../management/commands/stuff_db.py | 11 +- django-backend/fecfiler/urls.py | 5 +- .../fecfiler/user/migrations/0001_initial.py | 18 --- django-backend/fecfiler/utils.py | 2 +- docker-compose.yml | 2 +- performance-testing/README.md | 8 +- performance-testing/locust_run.py | 79 +++++------- 19 files changed, 63 insertions(+), 428 deletions(-) delete mode 100644 django-backend/fecfiler/authentication/__init__.py delete mode 100644 django-backend/fecfiler/authentication/migrations/0001_initial.py delete mode 100644 django-backend/fecfiler/authentication/migrations/__init__.py delete mode 100644 django-backend/fecfiler/authentication/models.py delete mode 100644 django-backend/fecfiler/authentication/test_views.py delete mode 100644 django-backend/fecfiler/authentication/urls.py delete mode 100644 django-backend/fecfiler/authentication/utils.py delete mode 100644 django-backend/fecfiler/authentication/views.py rename django-backend/fecfiler/{authentication => oidc}/middleware/TimeoutMiddleware.py (100%) diff --git a/django-backend/fecfiler/authentication/__init__.py b/django-backend/fecfiler/authentication/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/django-backend/fecfiler/authentication/migrations/0001_initial.py b/django-backend/fecfiler/authentication/migrations/0001_initial.py deleted file mode 100644 index 755a462ff1..0000000000 --- a/django-backend/fecfiler/authentication/migrations/0001_initial.py +++ /dev/null @@ -1,116 +0,0 @@ -# Generated by Django 4.2.7 on 2024-01-09 19:15 - -from django.db import migrations, models -import django.utils.timezone - - -class Migration(migrations.Migration): - initial = True - - dependencies = [ - ("auth", "0012_alter_user_first_name_max_length"), - ] - - operations = [ - migrations.CreateModel( - name="Account", - fields=[ - ( - "id", - models.AutoField( - auto_created=True, - primary_key=True, - serialize=False, - verbose_name="ID", - ), - ), - ( - "is_superuser", - models.BooleanField( - default=False, - help_text="Designates that this user has all permissions without explicitly assigning them.", # noqa - verbose_name="superuser status", - ), - ), - ( - "password", - models.CharField( - max_length=128, null=True, verbose_name="password" - ), - ), - ("email", models.EmailField(max_length=254)), - ("username", models.CharField(max_length=100, unique=True)), - ("cmtee_id", models.CharField(max_length=9)), - ("contact", models.CharField(max_length=10)), - ("first_name", models.CharField(blank=True, max_length=40, null=True)), - ("last_name", models.CharField(blank=True, max_length=40, null=True)), - ("role", models.CharField(blank=True, max_length=40)), - ("tagline", models.CharField(blank=True, max_length=140)), - ("delete_ind", models.CharField(default="N", max_length=1)), - ("created_at", models.DateTimeField(auto_now_add=True)), - ("updated_at", models.DateTimeField(auto_now=True)), - ("last_login", models.DateTimeField(auto_now=True, null=True)), - ( - "is_staff", - models.BooleanField( - default=False, - help_text="Designates whether the user can log into this admin site.", # noqa - verbose_name="staff status", - ), - ), - ( - "is_active", - models.BooleanField( - default=True, - help_text="Designates whether this user should be treated as active. Unselect this instead of deleting accounts.", # noqa - verbose_name="active", - ), - ), - ( - "date_joined", - models.DateTimeField( - default=django.utils.timezone.now, verbose_name="date joined" - ), - ), - ("login_code_counter", models.CharField(default=0, max_length=1)), - ( - "register_token", - models.CharField(blank=True, max_length=32, null=True), - ), - ( - "personal_key", - models.CharField(blank=True, max_length=64, null=True), - ), - ("status", models.CharField(blank=True, max_length=15, null=True)), - ("code_generated_counter", models.CharField(default=0, max_length=2)), - ("secret_key", models.CharField(blank=True, max_length=100, null=True)), - ( - "groups", - models.ManyToManyField( - blank=True, - help_text="The groups this user belongs to. A user will get all permissions granted to each of their groups.", # noqa - related_name="user_set", - related_query_name="user", - to="auth.group", - verbose_name="groups", - ), - ), - ( - "user_permissions", - models.ManyToManyField( - blank=True, - help_text="Specific permissions for this user.", - related_name="user_set", - related_query_name="user", - to="auth.permission", - verbose_name="user permissions", - ), - ), - ], - options={ - "verbose_name": "user", - "verbose_name_plural": "users", - "unique_together": {("username", "email")}, - }, - ), - ] diff --git a/django-backend/fecfiler/authentication/migrations/__init__.py b/django-backend/fecfiler/authentication/migrations/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/django-backend/fecfiler/authentication/models.py b/django-backend/fecfiler/authentication/models.py deleted file mode 100644 index 0a503b5670..0000000000 --- a/django-backend/fecfiler/authentication/models.py +++ /dev/null @@ -1,99 +0,0 @@ -from django.contrib.auth.models import ( - AbstractBaseUser, - PermissionsMixin, - BaseUserManager, -) -from django.db import models -from django.utils.translation import gettext_lazy as _ -from django.utils import timezone -from django.core.mail import send_mail - - -class AccountManager(BaseUserManager): - def create_user(self, email, password=None, **kwargs): - if not email: - raise ValueError("Users must have a valid email address.") - if not kwargs.get("username"): - raise ValueError("Users must have a valid username.") - - account = self.model( - email=self.normalize_email(email), username=kwargs.get("username") - ) - account.set_password(password) - account.save() - - return account - - def create_superuser(self, email, password, **kwargs): - account = self.create_user(email, password, **kwargs) - account.is_superuser = True - account.is_staff = True - account.save() - return account - - def get_by_natural_key(self, username): - return self.get(username__iexact=username) - - -class Account(AbstractBaseUser, PermissionsMixin): - password = models.CharField(_("password"), max_length=128, null=True) - email = models.EmailField() - username = models.CharField(max_length=100, unique=True) - cmtee_id = models.CharField(max_length=9) - contact = models.CharField(max_length=10) - first_name = models.CharField(max_length=40, null=True, blank=True) - last_name = models.CharField(max_length=40, null=True, blank=True) - role = models.CharField(max_length=40, blank=True) - tagline = models.CharField(max_length=140, blank=True) - delete_ind = models.CharField(max_length=1, blank=False, default="N") - created_at = models.DateTimeField(auto_now_add=True) - updated_at = models.DateTimeField(auto_now=True) - last_login = models.DateTimeField(auto_now=True, null=True) - is_staff = models.BooleanField( - _("staff status"), - default=False, - help_text=_("Designates whether the user can log into this admin " "site."), - ) - is_active = models.BooleanField( - _("active"), - default=True, - help_text=_( - "Designates whether this user should be treated as " - "active. Unselect this instead of deleting accounts." - ), - ) - - date_joined = models.DateTimeField(_("date joined"), default=timezone.now) - login_code_counter = models.CharField(max_length=1, blank=False, default=0) - register_token = models.CharField(max_length=32, null=True, blank=True) - personal_key = models.CharField(max_length=64, null=True, blank=True) - status = models.CharField(max_length=15, null=True, blank=True) - code_generated_counter = models.CharField(max_length=2, blank=False, default=0) - secret_key = models.CharField(max_length=100, null=True, blank=True) - - class Meta: - unique_together = (("username", "email"),) - verbose_name = _("user") - verbose_name_plural = _("users") - - objects = AccountManager() - USERNAME_FIELD = "username" - REQUIRED_FIELDS = ["email", "role"] - - def __unicode__(self): - return self.username - - def get_by_natural_key(self, username): - return self.get(username__iexact=username) - - def get_full_name(self): - return " ".join([self.username, self.tagline]).strip() - - def get_short_name(self): - return self.username - - def email_user(self, subject, message, from_email=None, **kwargs): - """ - Sends an email to this User. - """ - send_mail(subject, message, from_email, [self.email], **kwargs) diff --git a/django-backend/fecfiler/authentication/test_views.py b/django-backend/fecfiler/authentication/test_views.py deleted file mode 100644 index 522d8e8a88..0000000000 --- a/django-backend/fecfiler/authentication/test_views.py +++ /dev/null @@ -1,25 +0,0 @@ -from django.test import RequestFactory, TestCase -from django.contrib.auth import get_user_model -from fecfiler.authentication.views import ( - handle_invalid_login, - handle_valid_login, -) - -UserModel = get_user_model() - - -class AuthenticationTest(TestCase): - fixtures = ["C01234567_user_and_committee"] - user = None - - def setUp(self): - self.user = UserModel.objects.get(id="12345678-aaaa-bbbb-cccc-111122223333") - self.factory = RequestFactory() - - def test_invalid_login(self): - resp = handle_invalid_login("random_username") - self.assertEqual(resp.status_code, 401) - - def test_valid_login(self): - resp = handle_valid_login(self.user) - self.assertEqual(resp.status_code, 200) diff --git a/django-backend/fecfiler/authentication/urls.py b/django-backend/fecfiler/authentication/urls.py deleted file mode 100644 index 03db2ad976..0000000000 --- a/django-backend/fecfiler/authentication/urls.py +++ /dev/null @@ -1,12 +0,0 @@ -from django.urls import path -from .views import ( - authenticate_login, - authenticate_logout, -) - - -# The API URLs are now determined automatically by the router. -urlpatterns = [ - path("user/login/authenticate", authenticate_login), - path("auth/logout", authenticate_logout), -] diff --git a/django-backend/fecfiler/authentication/utils.py b/django-backend/fecfiler/authentication/utils.py deleted file mode 100644 index feeef4aa8c..0000000000 --- a/django-backend/fecfiler/authentication/utils.py +++ /dev/null @@ -1,17 +0,0 @@ -import logging - - -from fecfiler.settings import ( - FFAPI_LOGIN_DOT_GOV_COOKIE_NAME, - FFAPI_COOKIE_DOMAIN, - FFAPI_TIMEOUT_COOKIE_NAME, -) - -LOGGER = logging.getLogger(__name__) - - -def delete_user_logged_in_cookies(response): - response.delete_cookie(FFAPI_LOGIN_DOT_GOV_COOKIE_NAME, domain=FFAPI_COOKIE_DOMAIN) - response.delete_cookie(FFAPI_TIMEOUT_COOKIE_NAME, domain=FFAPI_COOKIE_DOMAIN) - response.delete_cookie("oidc_state") - response.delete_cookie("csrftoken", domain=FFAPI_COOKIE_DOMAIN) diff --git a/django-backend/fecfiler/authentication/views.py b/django-backend/fecfiler/authentication/views.py deleted file mode 100644 index 2801f26897..0000000000 --- a/django-backend/fecfiler/authentication/views.py +++ /dev/null @@ -1,74 +0,0 @@ -from django.http import HttpResponse -from django.contrib.auth import authenticate, logout, login -from django.views.decorators.http import require_http_methods -from rest_framework.decorators import ( - authentication_classes, - permission_classes, - api_view, -) -from fecfiler.settings import ( - ALTERNATIVE_LOGIN, -) - -from fecfiler.authentication.utils import delete_user_logged_in_cookies - -from rest_framework.response import Response -from rest_framework import status -from django.http import JsonResponse -import structlog - - -logger = structlog.get_logger(__name__) - -""" -Option for :py:const:`fecfiler.settings.base.ALTERNATIVE_LOGIN`. -See :py:meth:`fecfiler.authentication.views.authenticate_login` -""" -USERNAME_PASSWORD = "USERNAME_PASSWORD" - - -def handle_valid_login(user): - logger.debug(f"Successful login: {user}") - response = HttpResponse() - return response - - -def handle_invalid_login(username): - logger.debug(f"Unauthorized login attempt: {username}") - return HttpResponse("Unauthorized", status=401) - - -@api_view(["GET", "POST"]) -@authentication_classes([]) -@permission_classes([]) -@require_http_methods(["GET", "POST"]) -def authenticate_login(request): - endpoint_is_available = ALTERNATIVE_LOGIN == USERNAME_PASSWORD - if request.method == "GET": - return JsonResponse({"endpoint_available": endpoint_is_available}) - - if not endpoint_is_available: - return JsonResponse(status=405, safe=False) - - username = request.data.get("username", None) - password = request.data.get("password", None) - account = authenticate( - request=request, username=username, password=password - ) # Returns an account if the username is found and the password is valid - - if account: - login(request, account) - return handle_valid_login(account) - else: - return handle_invalid_login(username) - - -@api_view(["GET"]) -@authentication_classes([]) -@permission_classes([]) -@require_http_methods(["GET"]) -def authenticate_logout(request): - logout(request) - response = Response({}, status=status.HTTP_204_NO_CONTENT) - delete_user_logged_in_cookies(response) - return response diff --git a/django-backend/fecfiler/authentication/middleware/TimeoutMiddleware.py b/django-backend/fecfiler/oidc/middleware/TimeoutMiddleware.py similarity index 100% rename from django-backend/fecfiler/authentication/middleware/TimeoutMiddleware.py rename to django-backend/fecfiler/oidc/middleware/TimeoutMiddleware.py diff --git a/django-backend/fecfiler/oidc/utils.py b/django-backend/fecfiler/oidc/utils.py index e96bf43573..7daf5b7f28 100644 --- a/django-backend/fecfiler/oidc/utils.py +++ b/django-backend/fecfiler/oidc/utils.py @@ -19,6 +19,9 @@ from fecfiler.settings import ( + FFAPI_LOGIN_DOT_GOV_COOKIE_NAME, + FFAPI_COOKIE_DOMAIN, + FFAPI_TIMEOUT_COOKIE_NAME, OIDC_MAX_STATES, ) @@ -94,3 +97,10 @@ def add_oidc_nonce_to_session(request, state, nonce): "nonce": nonce, "added_on": time.time(), } + + +def delete_user_logged_in_cookies(response): + response.delete_cookie(FFAPI_LOGIN_DOT_GOV_COOKIE_NAME, domain=FFAPI_COOKIE_DOMAIN) + response.delete_cookie(FFAPI_TIMEOUT_COOKIE_NAME, domain=FFAPI_COOKIE_DOMAIN) + response.delete_cookie("oidc_state") + response.delete_cookie("csrftoken", domain=FFAPI_COOKIE_DOMAIN) diff --git a/django-backend/fecfiler/oidc/views.py b/django-backend/fecfiler/oidc/views.py index 42370bc06e..ec3361d7a2 100644 --- a/django-backend/fecfiler/oidc/views.py +++ b/django-backend/fecfiler/oidc/views.py @@ -28,12 +28,12 @@ OIDC_ACR_VALUES, LOGIN_REDIRECT_URL, ) -from fecfiler.authentication.utils import delete_user_logged_in_cookies from .utils import ( add_oidc_nonce_to_session, handle_oidc_callback_error, handle_oidc_callback_request, + delete_user_logged_in_cookies, ) from . import oidc_op_config diff --git a/django-backend/fecfiler/settings/base.py b/django-backend/fecfiler/settings/base.py index cb18f3b348..7ed335d6b1 100644 --- a/django-backend/fecfiler/settings/base.py +++ b/django-backend/fecfiler/settings/base.py @@ -30,13 +30,6 @@ CSRF_COOKIE_DOMAIN = env.get_credential("FFAPI_COOKIE_DOMAIN") CSRF_TRUSTED_ORIGINS = ["https://*.fecfile.fec.gov"] -""" -Enables alternative log in method. -See :py:const:`fecfiler.authentication.views.USERNAME_PASSWORD` -and :py:meth:`fecfiler.authentication.views.authenticate_login` -""" -ALTERNATIVE_LOGIN = env.get_credential("ALTERNATIVE_LOGIN") - # SECURITY WARNING: keep the secret key used in production secret! SECRET_KEY = env.get_credential("DJANGO_SECRET_KEY", get_random_string(50)) SECRET_KEY_FALLBACKS = env.get_credential("DJANGO_SECRET_KEY_FALLBACKS", []) @@ -66,7 +59,6 @@ "corsheaders", "storages", "django_structlog", - "fecfiler.authentication", "fecfiler.committee_accounts", "fecfiler.reports", "fecfiler.transactions", @@ -86,7 +78,7 @@ "corsheaders.middleware.CorsMiddleware", "django.middleware.security.SecurityMiddleware", "fecfiler.middleware.HeaderMiddleware", - "fecfiler.authentication.middleware.TimeoutMiddleware.TimeoutMiddleware", + "fecfiler.oidc.middleware.TimeoutMiddleware.TimeoutMiddleware", "django.contrib.sessions.middleware.SessionMiddleware", "django.middleware.common.CommonMiddleware", "django.middleware.csrf.CsrfViewMiddleware", @@ -165,6 +157,7 @@ "https://idp.int.identitysandbox.gov/.well-known/openid-configuration", ) +MOCK_OIDC_PROVIDER = env.get_credential("MOCK_OIDC_PROVIDER", "False").lower() == "true" MOCK_OIDC_PROVIDER_CACHE = env.get_credential("REDIS_URL") OIDC_ACR_VALUES = "http://idmanagement.gov/ns/assurance/ial/1" diff --git a/django-backend/fecfiler/transactions/management/commands/stuff_db.py b/django-backend/fecfiler/transactions/management/commands/stuff_db.py index c5d270bbfe..0a87befea5 100644 --- a/django-backend/fecfiler/transactions/management/commands/stuff_db.py +++ b/django-backend/fecfiler/transactions/management/commands/stuff_db.py @@ -1,11 +1,11 @@ import datetime from django.core.management.base import BaseCommand -from fecfiler.authentication.models import Account -from fecfiler.committee_accounts.models import CommitteeAccount +from fecfiler.committee_accounts.models import CommitteeAccount, Membership from fecfiler.contacts.models import Contact from fecfiler.reports.models import Report from fecfiler.reports.form_3x.models import Form3X from fecfiler.transactions.models import Transaction +from fecfiler.user.models import User from fecfiler.transactions.schedule_a.models import ScheduleA @@ -19,7 +19,12 @@ def handle(self, *args, **options): # create committee committee = CommitteeAccount.objects.create(committee_id="C12123434") # create user - Account.objects.create(cmtee_id="C12123434") + user = User.objects.create(email="test@fec.gov", username="gov") + Membership.objects.create( + committee_account=committee, + user=user, + role=Membership.CommitteeRole.COMMITTEE_ADMINISTRATOR, + ) # create a report f3x = Form3X.objects.create() report = Report.objects.create( diff --git a/django-backend/fecfiler/urls.py b/django-backend/fecfiler/urls.py index e1adf08f84..177fe01062 100644 --- a/django-backend/fecfiler/urls.py +++ b/django-backend/fecfiler/urls.py @@ -3,7 +3,7 @@ from rest_framework.decorators import api_view, permission_classes from rest_framework.response import Response from django.views.generic.base import RedirectView -from fecfiler.settings import LOGIN_REDIRECT_CLIENT_URL, ALTERNATIVE_LOGIN +from fecfiler.settings import LOGIN_REDIRECT_CLIENT_URL, MOCK_OIDC_PROVIDER from drf_spectacular.views import SpectacularAPIView, SpectacularSwaggerView BASE_V1_URL = r"^api/v1/" @@ -28,7 +28,6 @@ def get_api_status(_request): re_path(BASE_V1_URL, include("fecfiler.reports.urls")), re_path(BASE_V1_URL, include("fecfiler.memo_text.urls")), re_path(BASE_V1_URL, include("fecfiler.transactions.urls")), - re_path(BASE_V1_URL, include("fecfiler.authentication.urls")), re_path(BASE_V1_URL, include("fecfiler.web_services.urls")), re_path(BASE_V1_URL, include("fecfiler.user.urls")), re_path(BASE_V1_URL, include("fecfiler.feedback.urls")), @@ -39,5 +38,5 @@ def get_api_status(_request): re_path(BASE_V1_URL + "status/", get_api_status), ] -if ALTERNATIVE_LOGIN == "USERNAME_PASSWORD": +if MOCK_OIDC_PROVIDER: urlpatterns.append(re_path(BASE_V1_URL, include("fecfiler.mock_oidc_provider.urls"))) diff --git a/django-backend/fecfiler/user/migrations/0001_initial.py b/django-backend/fecfiler/user/migrations/0001_initial.py index 633d44ab96..2ea4a62739 100644 --- a/django-backend/fecfiler/user/migrations/0001_initial.py +++ b/django-backend/fecfiler/user/migrations/0001_initial.py @@ -10,23 +10,6 @@ logger = structlog.get_logger(__name__) -def copy_users(apps, schema_editor): - if not apps.is_installed( - "authentication" - ): # "authentication" not in [app_name for app_name, app in apps.get_models()]: - logger.error("The authentication app isn't installed") - return - OldAccount = apps.get_model("authentication", "Account") # noqa - User = apps.get_model("user", "User") # noqa - db_alias = schema_editor.connection.alias - old_users = OldAccount.objects.using(db_alias).all() - for old_user in old_users: - user_data = get_model_data(old_user.__dict__, User) - user_data["id"] = uuid.uuid4() - user_data["password"] = old_user.password or get_random_secret_key() - User.objects.create(**user_data) - - class Migration(migrations.Migration): initial = True @@ -129,5 +112,4 @@ class Migration(migrations.Migration): ("objects", django.contrib.auth.models.UserManager()), ], ), - migrations.RunPython(copy_users), ] diff --git a/django-backend/fecfiler/utils.py b/django-backend/fecfiler/utils.py index acd5bebd6f..342e5b7268 100644 --- a/django-backend/fecfiler/utils.py +++ b/django-backend/fecfiler/utils.py @@ -1,6 +1,6 @@ from rest_framework.exceptions import ValidationError from django.http import HttpResponseServerError -from fecfiler.authentication.utils import delete_user_logged_in_cookies +from fecfiler.oidc.utils import delete_user_logged_in_cookies from rest_framework.views import exception_handler import structlog diff --git a/docker-compose.yml b/docker-compose.yml index 23210b2efe..8b4c060fa4 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -81,13 +81,13 @@ services: links: - redis environment: - ALTERNATIVE_LOGIN: USERNAME_PASSWORD REDIS_URL: redis://redis:6379 DATABASE_URL: FECFILE_TEST_DB_NAME: DJANGO_SETTINGS_MODULE: fecfiler.settings.local DJANGO_SECRET_KEY: FECFILE_FEC_WEBSITE_API_KEY: DEMO_KEY + MOCK_OIDC_PROVIDER: True OIDC_RP_CLIENT_ID: OIDC_RP_CLIENT_SECRET: LOGIN_REDIRECT_SERVER_URL: http://localhost:8080/api/v1/oidc/login-redirect diff --git a/performance-testing/README.md b/performance-testing/README.md index cc3e2a15d8..6856c7518b 100644 --- a/performance-testing/README.md +++ b/performance-testing/README.md @@ -74,9 +74,11 @@ The Setup phase has two jobs: logging in to the target API and preparing data be ### Log In -Logging in locally works with a simple request to the API using the legacy debug login. When testing -against a remote service, Locust doesn't actually log in. Instead, Locust uses the session ID stored -in the `OIDC_SESSION_ID` env variable, using a session that you manually created rather than automating +Logging in locally works with a request to the mock_oidc_provider endpoints exposed in the application +for testing purposes. This application automatically authenticates the test user and returns an +appropriate sessionid and csrftoken for use throughout the application. When testing against a remote +service, Locust doesn't actually log in. Instead, Locust uses the session ID stored in the +`OIDC_SESSION_ID` env variable, using a session that you manually created rather than automating the log in process. This is needed because Login.gov uses two-factor authentication that cannot be automated at this time. diff --git a/performance-testing/locust_run.py b/performance-testing/locust_run.py index 06c564d9da..bd6cf260a0 100644 --- a/performance-testing/locust_run.py +++ b/performance-testing/locust_run.py @@ -20,10 +20,9 @@ WANTED_REPORTS = int(os.environ.get("LOCUST_WANTED_REPORTS", 10)) WANTED_CONTACTS = int(os.environ.get("LOCUST_WANTED_CONTACTS", 100)) WANTED_TRANSACTIONS = int(os.environ.get("LOCUST_WANTED_TRANSACTIONS", 500)) -SINGLE_TO_TRIPLE_RATIO = float(os.environ.get( - "LOCUST_TRANSACTIONS_SINGLE_TO_TRIPLE_RATIO", - 9 / 10 -)) +SINGLE_TO_TRIPLE_RATIO = float( + os.environ.get("LOCUST_TRANSACTIONS_SINGLE_TO_TRIPLE_RATIO", 9 / 10) +) SCHEDULES = ["A"] # Further schedules to be implemented in the future @@ -61,14 +60,7 @@ def on_start(self): self.report_ids = self.fetch_values("reports", "id") self.contacts = self.scrape_endpoint("contacts") else: - login_response = self.client.post( - "/api/v1/user/login/authenticate", - json={"username": TEST_USER, "password": TEST_PWD} - ) - csrftoken = login_response.cookies.get('csrftoken') - self.client.headers = { - "X-CSRFToken": csrftoken - } + self.login_via_mock_oidc() committees = self.fetch_values("committees", "id") committee_uuid = committees[0] print("committee_uuid", committee_uuid) @@ -97,6 +89,21 @@ def on_start(self): self.create_single_transactions(singles_needed) self.create_triple_transactions(triples_needed) + def login_via_mock_oidc(self): + authenticate_response = self.client.get( + "/api/v1/oidc/authenticate", allow_redirects=False + ) + authorize_response = self.client.get( + authenticate_response._next.url.removeprefix("http://localhost:8080"), + allow_redirects=False, + ) + callback_response = self.client.get( + authorize_response._next.url, + allow_redirects=False, + ) + csrftoken = callback_response.cookies.get("csrftoken") + self.client.headers = {"X-CSRFToken": csrftoken} + def create_reports(self, count=1): fields_to_validate = [ "filing_frequency", @@ -106,11 +113,9 @@ def create_reports(self, count=1): "coverage_through_date", "date_of_election", "state_of_election", - "form_type" + "form_type", ] - params = { - "fields_to_validate": fields_to_validate - } + params = {"fields_to_validate": fields_to_validate} reports = get_json_data("form-3x") if len(reports) < count: @@ -122,7 +127,7 @@ def create_reports(self, count=1): name="create_report", # TODO: does it make sense to pass both the params and json here? params=params, - json=report + json=report, ) def create_contacts(self, count=1): @@ -137,7 +142,7 @@ def create_contacts(self, count=1): # TODO: does it make sense to pass both the params and json here? # Same with create_reports json=contact, - timeout=TIMEOUT + timeout=TIMEOUT, ) def create_single_transactions(self, count=1): @@ -146,9 +151,7 @@ def create_single_transactions(self, count=1): if len(transactions) < count: difference = count - len(transactions) transactions += locust_data_generator.generate_single_transactions( - difference, - self.contacts, - self.report_ids + difference, self.contacts, self.report_ids ) for transaction in transactions[:count]: @@ -160,9 +163,7 @@ def create_triple_transactions(self, count=1): if len(transactions) < count: difference = count - len(transactions) transactions += locust_data_generator.generate_triple_transactions( - difference, - self.contacts, - self.report_ids + difference, self.contacts, self.report_ids ) for transaction in transactions[:count]: @@ -206,17 +207,15 @@ def create_transaction(self, transaction): "contributor_occupation", "memo_code", "memo_text_description", - "reattribution_redesignation_tag" + "reattribution_redesignation_tag", ] - params = { - "fields_to_validate": fields_to_validate - } + params = {"fields_to_validate": fields_to_validate} self.client.post( "/api/v1/transactions/", name="create_transactions", params=params, json=transaction, - timeout=TIMEOUT + timeout=TIMEOUT, ) def fetch_count(self, endpoint): @@ -252,18 +251,12 @@ def get_page(self, endpoint, page=1): "ordering": "form_type", } return self.client.get( - f"/api/v1/{endpoint}", - params=params, - name=f"preload_{endpoint}_ids" + f"/api/v1/{endpoint}", params=params, name=f"preload_{endpoint}_ids" ) @task def celery_test(self): - self.client.get( - "/celery-test/", - name="celery-test", - timeout=TIMEOUT - ) + self.client.get("/celery-test/", name="celery-test", timeout=TIMEOUT) @task def load_contacts(self): @@ -272,10 +265,7 @@ def load_contacts(self): "ordering": "form_type", } self.client.get( - "/api/v1/contacts/", - name="load_contacts", - timeout=TIMEOUT, - params=params + "/api/v1/contacts/", name="load_contacts", timeout=TIMEOUT, params=params ) @task @@ -285,10 +275,7 @@ def load_reports(self): "ordering": "form_type", } self.client.get( - "/api/v1/reports/", - name="load_reports", - timeout=TIMEOUT, - params=params + "/api/v1/reports/", name="load_reports", timeout=TIMEOUT, params=params ) @task @@ -305,7 +292,7 @@ def load_transactions(self): "/api/v1/transactions/", name="load_transactions", timeout=TIMEOUT, - params=params + params=params, ) From 971ed6895a733ea71346df7350e9e88e6a68b78d Mon Sep 17 00:00:00 2001 From: David Heitzer Date: Thu, 21 Nov 2024 11:48:44 -0500 Subject: [PATCH 24/24] fix flake8 --- django-backend/fecfiler/user/migrations/0001_initial.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/django-backend/fecfiler/user/migrations/0001_initial.py b/django-backend/fecfiler/user/migrations/0001_initial.py index 2ea4a62739..b8db93a851 100644 --- a/django-backend/fecfiler/user/migrations/0001_initial.py +++ b/django-backend/fecfiler/user/migrations/0001_initial.py @@ -3,8 +3,6 @@ from django.db import migrations, models import django.utils.timezone import uuid -from fecfiler.shared.utilities import get_model_data -from django.core.management.utils import get_random_secret_key import structlog logger = structlog.get_logger(__name__)