From 7a3b83c72f33f3fc3b977fe399a0a8b37017d9b9 Mon Sep 17 00:00:00 2001 From: Kegan Maher Date: Tue, 6 Feb 2024 19:50:25 +0000 Subject: [PATCH] refactor(models): PemData.data could come from a secret field PemData favors the secret, but fallback to a remote URL this is to allow for simpler turn-key local development * update definition to use new field * update migrations * remove env vars from terraform definitions * move default keys from data migration to files --- benefits/core/migrations/0001_initial.py | 7 ++- benefits/core/migrations/0002_data.py | 72 +++++------------------- benefits/core/models.py | 29 ++++++---- keys/README.md | 5 ++ keys/client.key | 27 +++++++++ keys/client.pub | 9 +++ terraform/app_service.tf | 11 ---- tests/pytest/conftest.py | 4 +- tests/pytest/core/test_models.py | 61 ++++++++++++++++---- 9 files changed, 129 insertions(+), 96 deletions(-) create mode 100644 keys/README.md create mode 100644 keys/client.key create mode 100644 keys/client.pub diff --git a/benefits/core/migrations/0001_initial.py b/benefits/core/migrations/0001_initial.py index a3f3c630a2..db5f9936da 100644 --- a/benefits/core/migrations/0001_initial.py +++ b/benefits/core/migrations/0001_initial.py @@ -92,8 +92,13 @@ class Migration(migrations.Migration): fields=[ ("id", models.AutoField(primary_key=True, serialize=False)), ("label", models.TextField()), - ("text", models.TextField(null=True)), ("remote_url", models.TextField(null=True)), + ( + "text_secret_name", + benefits.core.models.SecretValueField( + max_length=127, null=True, validators=[benefits.core.models.SecretNameValidator()] + ), + ), ], ), migrations.CreateModel( diff --git a/benefits/core/migrations/0002_data.py b/benefits/core/migrations/0002_data.py index b509e3d11f..1c20657935 100644 --- a/benefits/core/migrations/0002_data.py +++ b/benefits/core/migrations/0002_data.py @@ -51,106 +51,60 @@ def load_data(app, *args, **kwargs): ), ) - default_client_private_key = """ ------BEGIN RSA PRIVATE KEY----- -MIIEpAIBAAKCAQEA1pt0ZoOuPEVPJJS+5r884zcjZLkZZ2GcPwr79XOLDbOi46on -Ca79kjRnhS0VUK96SwUPS0z9J5mDA5LSNL2RoxFb5QGaevnJY828NupzTNdUd0sY -JK3kRjKUggHWuB55hwJcH/Dx7I3DNH4NL68UAlK+VjwJkfYPrhq/bl5z8ZiurvBa -5C1mDxhFpcTZlCfxQoas7D1d+uPACF6mEMbQNd3RaIaSREO50NvNywXIIt/OmCiR -qI7JtOcn4eyh1I4j9WtlbMhRJLfwPMAgY5epTsWcURmhVofF2wVoFbib3JGCfA7t -z/gmP5YoEKnf/cumKmF3e9LrZb8zwm7bTHUViwIDAQABAoIBAQCIv0XMjNvZS9DC -XoXGQtVpcxj6dXfaiDgnc7hZDubsNCr3JtT5NqgdIYdVNQUABNDIPNEiCkzFjuwM -uuF2+dRzM/x6UCs/cSsCjXYBCCOwMwV/fjpEJQnwMQqwTLulVsXZYYeSUtXVBf/8 -0tVULRty34apLFhsyX30UtboXQdESfpmm5ZsqsZJlYljw+M7JxRMneQclI19y/ya -hPWlfhLB9OffVEJXGaWx1NSYnKoCMKqE/+4krROr6V62xXaNyX6WtU6XiT7C6R5A -PBxfhmoeFdVCF6a+Qq0v2fKThYoZnV4sn2q2An9YPfynFYnlgzdfnAFSejsqxQd0 -fxYLOtMBAoGBAP1jxjHDJngZ1N+ymw9MIpRgr3HeuMP5phiSTbY2tu9lPzQd+TMX -fhr1bQh2Fd/vU0u7X0yPnTWtUrLlCdGnWPpXivx95GNGgUUIk2HStFdrRx+f2Qvk -G8vtLgmSbjQ26UiHzxi9Wa0a41PWIA3TixkcFrS2X29Qc4yd6pVHmicfAoGBANjR -Z8aaDkSKLkq5Nk1T7I0E1+mtPoH1tPV/FJClXjJrvfDuYHBeOyUpipZddnZuPGWA -IW2tFIsMgJQtgpvgs52NFI7pQGJRUPK/fTG+Ycocxo78TkLr/RIj8Kj5brXsbZ9P -3/WBX5GAISTSp1ab8xVgK/Tm07hGupKVqnY2lCAVAoGAIql0YjhE2ecGtLcU+Qm8 -LTnwpg4GjmBnNTNGSCfB7IuYEsQK489R49Qw3xhwM5rkdRajmbCHm+Eiz+/+4NwY -kt5I1/NMu7vYUR40MwyEuPSm3Q+bvEGu/71pL8wFIUVlshNJ5CN60fA8qqo+5kVK -4Ntzy7Kq6WpC9Dhh75vE3ZcCgYEAty99uXtxsJD6+aEwcvcENkUwUztPQ6ggAwci -je9Z/cmwCj6s9mN3HzfQ4qgGrZsHpk4ycCK655xhilBFOIQJ3YRUKUaDYk4H0YDe -Osf6gTP8wtQDH2GZSNlavLk5w7UFDYQD2b47y4fw+NaOEYvjPl0p5lmb6ebAPZb8 -FbKZRd0CgYBC1HTbA+zMEqDdY4MWJJLC6jZsjdxOGhzjrCtWcIWEGMDF7oDDEoix -W3j2hwm4C6vaNkH9XX1dr5+q6gq8vJQdbYoExl22BGMiNbfI3+sLRk0zBYL//W6c -tSREgR4EjosqQfbkceLJ2JT1wuNjInI0eR9H3cRugvlDTeWtbdJ5qA== ------END RSA PRIVATE KEY----- -""" - client_private_key = PemData.objects.create( - text=os.environ.get("CLIENT_PRIVATE_KEY", default_client_private_key), + remote_url="https://raw.githubusercontent.com/cal-itp/benefits/dev/keys/client.key", + text_secret_name="client-private-key", label="Benefits client private key", ) - default_client_public_key = """ ------BEGIN PUBLIC KEY----- -MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA1pt0ZoOuPEVPJJS+5r88 -4zcjZLkZZ2GcPwr79XOLDbOi46onCa79kjRnhS0VUK96SwUPS0z9J5mDA5LSNL2R -oxFb5QGaevnJY828NupzTNdUd0sYJK3kRjKUggHWuB55hwJcH/Dx7I3DNH4NL68U -AlK+VjwJkfYPrhq/bl5z8ZiurvBa5C1mDxhFpcTZlCfxQoas7D1d+uPACF6mEMbQ -Nd3RaIaSREO50NvNywXIIt/OmCiRqI7JtOcn4eyh1I4j9WtlbMhRJLfwPMAgY5ep -TsWcURmhVofF2wVoFbib3JGCfA7tz/gmP5YoEKnf/cumKmF3e9LrZb8zwm7bTHUV -iwIDAQAB ------END PUBLIC KEY----- -""" - client_public_key = PemData.objects.create( - text=os.environ.get("CLIENT_PUBLIC_KEY", default_client_public_key), + remote_url="https://raw.githubusercontent.com/cal-itp/benefits/dev/keys/client.pub", + text_secret_name="client-public-key", label="Benefits client public key", ) - dummy_cert_text = """ ------BEGIN CERTIFICATE----- -PEM DATA ------END CERTIFICATE----- -""" - mst_payment_processor_client_cert = PemData.objects.create( - text=os.environ.get("MST_PAYMENT_PROCESSOR_CLIENT_CERT", dummy_cert_text), + text_secret_name="mst-payment-processor-client-cert", label="MST payment processor client certificate", ) mst_payment_processor_client_cert_private_key = PemData.objects.create( - text=os.environ.get("MST_PAYMENT_PROCESSOR_CLIENT_CERT_PRIVATE_KEY", client_private_key.text), + text_secret_name="mst-payment-processor-client-cert-private-key", label="MST payment processor client certificate private key", ) mst_payment_processor_client_cert_root_ca = PemData.objects.create( - text=os.environ.get("MST_PAYMENT_PROCESSOR_CLIENT_CERT_ROOT_CA", dummy_cert_text), + text_secret_name="mst-payment-processor-client-cert-root-ca", label="MST payment processor client certificate root CA", ) sacrt_payment_processor_client_cert = PemData.objects.create( - text=os.environ.get("SACRT_PAYMENT_PROCESSOR_CLIENT_CERT", dummy_cert_text), + text_secret_name="sacrt-payment-processor-client-cert", label="SacRT payment processor client certificate", ) sacrt_payment_processor_client_cert_private_key = PemData.objects.create( - text=os.environ.get("SACRT_PAYMENT_PROCESSOR_CLIENT_CERT_PRIVATE_KEY", client_private_key.text), + text_secret_name="sacrt-payment-processor-client-cert-private-key", label="SacRT payment processor client certificate private key", ) sacrt_payment_processor_client_cert_root_ca = PemData.objects.create( - text=os.environ.get("SACRT_PAYMENT_PROCESSOR_CLIENT_CERT_ROOT_CA", dummy_cert_text), + text_secret_name="sacrt-payment-processor-client-cert-root-ca", label="SacRT payment processor client certificate root CA", ) sbmtd_payment_processor_client_cert = PemData.objects.create( - text=os.environ.get("SBMTD_PAYMENT_PROCESSOR_CLIENT_CERT", dummy_cert_text), + text_secret_name="sbmtd-payment-processor-client-cert", label="SBMTD payment processor client certificate", ) sbmtd_payment_processor_client_cert_private_key = PemData.objects.create( - text=os.environ.get("SBMTD_PAYMENT_PROCESSOR_CLIENT_CERT_PRIVATE_KEY", client_private_key.text), + text_secret_name="sbmtd-payment-processor-client-cert-private-key", label="SBMTD payment processor client certificate private key", ) sbmtd_payment_processor_client_cert_root_ca = PemData.objects.create( - text=os.environ.get("SBMTD_PAYMENT_PROCESSOR_CLIENT_CERT_ROOT_CA", dummy_cert_text), + text_secret_name="sbmtd-payment-processor-client-cert-root-ca", label="SBMTD payment processor client certificate root CA", ) diff --git a/benefits/core/models.py b/benefits/core/models.py index 070b8ccb70..2280e55a1b 100644 --- a/benefits/core/models.py +++ b/benefits/core/models.py @@ -2,6 +2,7 @@ The core application: Common model definitions. """ +from functools import cached_property import importlib import logging import re @@ -78,23 +79,31 @@ class PemData(models.Model): id = models.AutoField(primary_key=True) # Human description of the PEM data label = models.TextField() - # The data in utf-8 encoded PEM text format - text = models.TextField(null=True) + # The name of a secret with data in utf-8 encoded PEM text format + text_secret_name = SecretValueField(null=True) # Public URL hosting the utf-8 encoded PEM text remote_url = models.TextField(null=True) def __str__(self): return self.label - @property + @cached_property def data(self): - if self.text: - return self.text - elif self.remote_url: - self.text = requests.get(self.remote_url, timeout=settings.REQUESTS_TIMEOUT).text - - self.save() - return self.text + """ + Attempts to get data from `remote_url` or `text_secret_name`, with the latter taking precendence if both are defined. + """ + remote_data = None + secret_data = None + + if self.remote_url: + remote_data = requests.get(self.remote_url, timeout=settings.REQUESTS_TIMEOUT).text + if self.text_secret_name: + try: + secret_data = get_secret_by_name(self.text_secret_name) + except Exception: + secret_data = None + + return secret_data if secret_data is not None else remote_data class AuthProvider(models.Model): diff --git a/keys/README.md b/keys/README.md new file mode 100644 index 0000000000..4bc0033ae1 --- /dev/null +++ b/keys/README.md @@ -0,0 +1,5 @@ +# keys + +_These keys are just samples_. They cannot be used for production systems. + +See more at diff --git a/keys/client.key b/keys/client.key new file mode 100644 index 0000000000..99016ca4df --- /dev/null +++ b/keys/client.key @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEpAIBAAKCAQEA1pt0ZoOuPEVPJJS+5r884zcjZLkZZ2GcPwr79XOLDbOi46on +Ca79kjRnhS0VUK96SwUPS0z9J5mDA5LSNL2RoxFb5QGaevnJY828NupzTNdUd0sY +JK3kRjKUggHWuB55hwJcH/Dx7I3DNH4NL68UAlK+VjwJkfYPrhq/bl5z8ZiurvBa +5C1mDxhFpcTZlCfxQoas7D1d+uPACF6mEMbQNd3RaIaSREO50NvNywXIIt/OmCiR +qI7JtOcn4eyh1I4j9WtlbMhRJLfwPMAgY5epTsWcURmhVofF2wVoFbib3JGCfA7t +z/gmP5YoEKnf/cumKmF3e9LrZb8zwm7bTHUViwIDAQABAoIBAQCIv0XMjNvZS9DC +XoXGQtVpcxj6dXfaiDgnc7hZDubsNCr3JtT5NqgdIYdVNQUABNDIPNEiCkzFjuwM +uuF2+dRzM/x6UCs/cSsCjXYBCCOwMwV/fjpEJQnwMQqwTLulVsXZYYeSUtXVBf/8 +0tVULRty34apLFhsyX30UtboXQdESfpmm5ZsqsZJlYljw+M7JxRMneQclI19y/ya +hPWlfhLB9OffVEJXGaWx1NSYnKoCMKqE/+4krROr6V62xXaNyX6WtU6XiT7C6R5A +PBxfhmoeFdVCF6a+Qq0v2fKThYoZnV4sn2q2An9YPfynFYnlgzdfnAFSejsqxQd0 +fxYLOtMBAoGBAP1jxjHDJngZ1N+ymw9MIpRgr3HeuMP5phiSTbY2tu9lPzQd+TMX +fhr1bQh2Fd/vU0u7X0yPnTWtUrLlCdGnWPpXivx95GNGgUUIk2HStFdrRx+f2Qvk +G8vtLgmSbjQ26UiHzxi9Wa0a41PWIA3TixkcFrS2X29Qc4yd6pVHmicfAoGBANjR +Z8aaDkSKLkq5Nk1T7I0E1+mtPoH1tPV/FJClXjJrvfDuYHBeOyUpipZddnZuPGWA +IW2tFIsMgJQtgpvgs52NFI7pQGJRUPK/fTG+Ycocxo78TkLr/RIj8Kj5brXsbZ9P +3/WBX5GAISTSp1ab8xVgK/Tm07hGupKVqnY2lCAVAoGAIql0YjhE2ecGtLcU+Qm8 +LTnwpg4GjmBnNTNGSCfB7IuYEsQK489R49Qw3xhwM5rkdRajmbCHm+Eiz+/+4NwY +kt5I1/NMu7vYUR40MwyEuPSm3Q+bvEGu/71pL8wFIUVlshNJ5CN60fA8qqo+5kVK +4Ntzy7Kq6WpC9Dhh75vE3ZcCgYEAty99uXtxsJD6+aEwcvcENkUwUztPQ6ggAwci +je9Z/cmwCj6s9mN3HzfQ4qgGrZsHpk4ycCK655xhilBFOIQJ3YRUKUaDYk4H0YDe +Osf6gTP8wtQDH2GZSNlavLk5w7UFDYQD2b47y4fw+NaOEYvjPl0p5lmb6ebAPZb8 +FbKZRd0CgYBC1HTbA+zMEqDdY4MWJJLC6jZsjdxOGhzjrCtWcIWEGMDF7oDDEoix +W3j2hwm4C6vaNkH9XX1dr5+q6gq8vJQdbYoExl22BGMiNbfI3+sLRk0zBYL//W6c +tSREgR4EjosqQfbkceLJ2JT1wuNjInI0eR9H3cRugvlDTeWtbdJ5qA== +-----END RSA PRIVATE KEY----- diff --git a/keys/client.pub b/keys/client.pub new file mode 100644 index 0000000000..8e94353e95 --- /dev/null +++ b/keys/client.pub @@ -0,0 +1,9 @@ +-----BEGIN PUBLIC KEY----- +MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA1pt0ZoOuPEVPJJS+5r88 +4zcjZLkZZ2GcPwr79XOLDbOi46onCa79kjRnhS0VUK96SwUPS0z9J5mDA5LSNL2R +oxFb5QGaevnJY828NupzTNdUd0sYJK3kRjKUggHWuB55hwJcH/Dx7I3DNH4NL68U +AlK+VjwJkfYPrhq/bl5z8ZiurvBa5C1mDxhFpcTZlCfxQoas7D1d+uPACF6mEMbQ +Nd3RaIaSREO50NvNywXIIt/OmCiRqI7JtOcn4eyh1I4j9WtlbMhRJLfwPMAgY5ep +TsWcURmhVofF2wVoFbib3JGCfA7tz/gmP5YoEKnf/cumKmF3e9LrZb8zwm7bTHUV +iwIDAQAB +-----END PUBLIC KEY----- diff --git a/terraform/app_service.tf b/terraform/app_service.tf index 9434657bc8..573a373ce6 100644 --- a/terraform/app_service.tf +++ b/terraform/app_service.tf @@ -97,19 +97,8 @@ resource "azurerm_linux_web_app" "main" { "SACRT_SENIOR_GROUP_ID" = "${local.secret_prefix}sacrt-senior-group-id)" "SBMTD_SENIOR_GROUP_ID" = "${local.secret_prefix}sbmtd-senior-group-id)", "SBMTD_MOBILITY_PASS_GROUP_ID" = "${local.secret_prefix}sbmtd-mobility-pass-group-id)" - "CLIENT_PRIVATE_KEY" = "${local.secret_prefix}client-private-key)" - "CLIENT_PUBLIC_KEY" = "${local.secret_prefix}client-public-key)" "MST_SERVER_PUBLIC_KEY_URL" = "${local.secret_prefix}mst-server-public-key-url)" "SBMTD_SERVER_PUBLIC_KEY_URL" = "${local.secret_prefix}sbmtd-server-public-key-url)" - "MST_PAYMENT_PROCESSOR_CLIENT_CERT" = "${local.secret_prefix}mst-payment-processor-client-cert)" - "MST_PAYMENT_PROCESSOR_CLIENT_CERT_PRIVATE_KEY" = "${local.secret_prefix}mst-payment-processor-client-cert-private-key)" - "MST_PAYMENT_PROCESSOR_CLIENT_CERT_ROOT_CA" = "${local.secret_prefix}mst-payment-processor-client-cert-root-ca)" - "SACRT_PAYMENT_PROCESSOR_CLIENT_CERT" = "${local.secret_prefix}sacrt-payment-processor-client-cert)" - "SACRT_PAYMENT_PROCESSOR_CLIENT_CERT_PRIVATE_KEY" = "${local.secret_prefix}sacrt-payment-processor-client-cert-private-key)" - "SACRT_PAYMENT_PROCESSOR_CLIENT_CERT_ROOT_CA" = "${local.secret_prefix}sacrt-payment-processor-client-cert-root-ca)" - "SBMTD_PAYMENT_PROCESSOR_CLIENT_CERT" = "${local.secret_prefix}sbmtd-payment-processor-client-cert)" - "SBMTD_PAYMENT_PROCESSOR_CLIENT_CERT_PRIVATE_KEY" = "${local.secret_prefix}sbmtd-payment-processor-client-cert-private-key)" - "SBMTD_PAYMENT_PROCESSOR_CLIENT_CERT_ROOT_CA" = "${local.secret_prefix}sbmtd-payment-processor-client-cert-root-ca)" "AUTH_PROVIDER_AUTHORITY" = "${local.secret_prefix}auth-provider-authority)" "SENIOR_AUTH_PROVIDER_CLIENT_NAME" = "${local.secret_prefix}senior-auth-provider-client-name)" "SENIOR_AUTH_PROVIDER_SCOPE" = "${local.secret_prefix}senior-auth-provider-scope)" diff --git a/tests/pytest/conftest.py b/tests/pytest/conftest.py index 920aaa9e3f..cc62540bc1 100644 --- a/tests/pytest/conftest.py +++ b/tests/pytest/conftest.py @@ -40,9 +40,7 @@ def mock_models_get_secret_by_name(mocker): @pytest.fixture def model_PemData(): - data = PemData.objects.create( - text="-----BEGIN PUBLIC KEY-----\nPEM DATA\n-----END PUBLIC KEY-----\n", label="Test public key" - ) + data = PemData.objects.create(text_secret_name="pem-secret-data", label="Test public key") return data diff --git a/tests/pytest/core/test_models.py b/tests/pytest/core/test_models.py index 8f959ddef5..ee83452136 100644 --- a/tests/pytest/core/test_models.py +++ b/tests/pytest/core/test_models.py @@ -6,6 +6,12 @@ from benefits.core.models import SecretNameValidator, SecretValueField, EligibilityType, EligibilityVerifier, TransitAgency +@pytest.fixture +def mock_requests_get_pem_data(mocker): + # intercept and spy on the GET request + return mocker.patch("benefits.core.models.requests.get", return_value=mocker.Mock(text="PEM text")) + + @pytest.mark.parametrize( "secret_name", [ @@ -59,27 +65,58 @@ def test_PemData_str(model_PemData): @pytest.mark.django_db -def test_PemData_data_text(model_PemData): - assert model_PemData.text - assert model_PemData.data == model_PemData.text +def test_PemData_data_text_secret_name(model_PemData, mock_models_get_secret_by_name): + # a secret name and not remote URL, should use secret value + + data = model_PemData.data + + mock_models_get_secret_by_name.assert_called_once_with(model_PemData.text_secret_name) + assert data == mock_models_get_secret_by_name.return_value @pytest.mark.django_db -def test_PemData_data_remote(model_PemData, mocker): - model_PemData.text = None +def test_PemData_data_remote(model_PemData, mock_requests_get_pem_data): + # a remote URL and no secret name, should use remote value + + model_PemData.text_secret_name = None model_PemData.remote_url = "http://localhost/publickey" - # intercept and spy on the GET request - requests_spy = mocker.patch("benefits.core.models.requests.get", return_value=mocker.Mock(text="PEM text")) + assert not model_PemData.text_secret_name + + data = model_PemData.data + + mock_requests_get_pem_data.assert_called_once_with(model_PemData.remote_url, timeout=settings.REQUESTS_TIMEOUT) + assert data == mock_requests_get_pem_data.return_value.text + - assert not model_PemData.text +@pytest.mark.django_db +def test_PemData_data_text_secret_name_and_remote__uses_text_secret( + model_PemData, mock_models_get_secret_by_name, mock_requests_get_pem_data +): + # a remote URL and the secret value is not None, should use the secret value + + model_PemData.remote_url = "http://localhost/publickey" + + data = model_PemData.data + + mock_models_get_secret_by_name.assert_called_once_with(model_PemData.text_secret_name) + mock_requests_get_pem_data.assert_called_once_with(model_PemData.remote_url, timeout=settings.REQUESTS_TIMEOUT) + assert data == mock_models_get_secret_by_name.return_value + + +@pytest.mark.django_db +def test_PemData_data_text_secret_name_and_remote__uses_remote( + model_PemData, mock_models_get_secret_by_name, mock_requests_get_pem_data +): + # a remote URL and the secret value is None, should use remote value + model_PemData.remote_url = "http://localhost/publickey" + mock_models_get_secret_by_name.return_value = None data = model_PemData.data - assert model_PemData.text - assert data == "PEM text" - assert data == model_PemData.text - requests_spy.assert_called_once_with(model_PemData.remote_url, timeout=settings.REQUESTS_TIMEOUT) + mock_models_get_secret_by_name.assert_called_once_with(model_PemData.text_secret_name) + mock_requests_get_pem_data.assert_called_once_with(model_PemData.remote_url, timeout=settings.REQUESTS_TIMEOUT) + assert data == mock_requests_get_pem_data.return_value.text @pytest.mark.django_db