Skip to content

Commit

Permalink
refactor(models): EligibilityVerifier.api_auth_key is a secret field
Browse files Browse the repository at this point in the history
* update definition to use new field
* update migrations
* remove env vars from terraform definitions
  • Loading branch information
thekaveman committed Feb 7, 2024
1 parent e6f4d97 commit 7f153b6
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 16 deletions.
9 changes: 7 additions & 2 deletions benefits/core/migrations/0001_initial.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Generated by Django 5.0.1 on 2024-02-06 03:46
# Generated by Django 5.0.1 on 2024-02-06 18:09

import benefits.core.models
import django.db.models.deletion
Expand Down Expand Up @@ -48,7 +48,12 @@ class Migration(migrations.Migration):
("active", models.BooleanField(default=False)),
("api_url", models.TextField(null=True)),
("api_auth_header", models.TextField(null=True)),
("api_auth_key", models.TextField(null=True)),
(
"api_auth_key_secret_name",
benefits.core.models.SecretValueField(
max_length=127, null=True, validators=[benefits.core.models.SecretNameValidator()]
),
),
("jwe_cek_enc", models.TextField(null=True)),
("jwe_encryption_alg", models.TextField(null=True)),
("jws_signing_alg", models.TextField(null=True)),
Expand Down
4 changes: 2 additions & 2 deletions benefits/core/migrations/0002_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ def load_data(app, *args, **kwargs):
active=os.environ.get("COURTESY_CARD_VERIFIER_ACTIVE", "True").lower() == "true",
api_url=os.environ.get("COURTESY_CARD_VERIFIER_API_URL", "http://server:8000/verify"),
api_auth_header=os.environ.get("COURTESY_CARD_VERIFIER_API_AUTH_HEADER", "X-Server-API-Key"),
api_auth_key=os.environ.get("COURTESY_CARD_VERIFIER_API_AUTH_KEY", "server-auth-token"),
api_auth_key_secret_name="courtesy-card-verifier-api-auth-key",
eligibility_type=mst_courtesy_card_type,
public_key=mst_server_public_key,
jwe_cek_enc=os.environ.get("COURTESY_CARD_VERIFIER_JWE_CEK_ENC", "A256CBC-HS512"),
Expand Down Expand Up @@ -238,7 +238,7 @@ def load_data(app, *args, **kwargs):
active=os.environ.get("MOBILITY_PASS_VERIFIER_ACTIVE", "True").lower() == "true",
api_url=os.environ.get("MOBILITY_PASS_VERIFIER_API_URL", "http://server:8000/verify"),
api_auth_header=os.environ.get("MOBILITY_PASS_VERIFIER_API_AUTH_HEADER", "X-Server-API-Key"),
api_auth_key=os.environ.get("MOBILITY_PASS_VERIFIER_API_AUTH_KEY", "server-auth-token"),
api_auth_key_secret_name="mobility-pass-verifier-api-auth-key",
eligibility_type=sbmtd_mobility_pass_type,
public_key=sbmtd_server_public_key,
jwe_cek_enc=os.environ.get("MOBILITY_PASS_VERIFIER_JWE_CEK_ENC", "A256CBC-HS512"),
Expand Down
11 changes: 8 additions & 3 deletions benefits/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ def __init__(self, *args, **kwargs):
kwargs["max_length"] = 127
# similar to max_length, enforce at the field (form) validation level to not allow blanks
kwargs["blank"] = False
# similar to blank, enforce at the database level that null is not allowed
kwargs["null"] = False
# the default is False, but this is more explicit
kwargs["allow_unicode"] = False
super().__init__(*args, **kwargs)
Expand Down Expand Up @@ -164,7 +162,7 @@ class EligibilityVerifier(models.Model):
active = models.BooleanField(default=False)
api_url = models.TextField(null=True)
api_auth_header = models.TextField(null=True)
api_auth_key = models.TextField(null=True)
api_auth_key_secret_name = SecretValueField(null=True)
eligibility_type = models.ForeignKey(EligibilityType, on_delete=models.PROTECT)
# public key is used to encrypt requests targeted at this Verifier and to verify signed responses from this verifier
public_key = models.ForeignKey(PemData, related_name="+", on_delete=models.PROTECT, null=True)
Expand All @@ -183,6 +181,13 @@ class EligibilityVerifier(models.Model):
def __str__(self):
return self.name

@property
def api_auth_key(self):
if self.api_auth_key_secret_name is not None:
return get_secret_by_name(self.api_auth_key_secret_name)
else:
return None

@property
def public_key_data(self):
"""This Verifier's public key as a string."""
Expand Down
8 changes: 7 additions & 1 deletion tests/pytest/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ def app_request(rf):
return app_request


# autouse this fixture so we never call out to the real secret store
@pytest.fixture(autouse=True)
def mock_models_get_secret_by_name(mocker):
return mocker.patch("benefits.core.models.get_secret_by_name", return_value="secret value!")


@pytest.fixture
def model_PemData():
data = PemData.objects.create(
Expand Down Expand Up @@ -108,7 +114,7 @@ def model_EligibilityVerifier(model_PemData, model_EligibilityType):
active=True,
api_url="https://example.com/verify",
api_auth_header="X-API-AUTH",
api_auth_key="secret-key",
api_auth_key_secret_name="secret-key",
eligibility_type=model_EligibilityType,
public_key=model_PemData,
selection_label_template="eligibility/includes/selection-label.html",
Expand Down
19 changes: 11 additions & 8 deletions tests/pytest/core/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,6 @@ def test_SecretValueField_init():
assert field.description != ""


@pytest.fixture
def mock_get_secret_by_name(mocker):
return mocker.patch("benefits.core.models.get_secret_by_name", return_value="secret value!")


@pytest.mark.django_db
def test_PemData_str(model_PemData):
assert str(model_PemData) == model_PemData.label
Expand Down Expand Up @@ -94,11 +89,11 @@ def test_model_AuthProvider(model_AuthProvider):


@pytest.mark.django_db
def test_model_AuthProvider_client_id(model_AuthProvider, mock_get_secret_by_name):
def test_model_AuthProvider_client_id(model_AuthProvider, mock_models_get_secret_by_name):
secret_value = model_AuthProvider.client_id

mock_get_secret_by_name.assert_called_once_with(model_AuthProvider.client_id_secret_name)
assert secret_value == mock_get_secret_by_name.return_value
mock_models_get_secret_by_name.assert_called_once_with(model_AuthProvider.client_id_secret_name)
assert secret_value == mock_models_get_secret_by_name.return_value


@pytest.mark.django_db
Expand Down Expand Up @@ -266,6 +261,14 @@ def test_EligibilityVerifier_without_AuthProvider(model_EligibilityVerifier):
assert not model_EligibilityVerifier.uses_auth_verification


@pytest.mark.django_db
def test_EligiblityVerifier_api_auth_key(model_EligibilityVerifier, mock_models_get_secret_by_name):
secret_value = model_EligibilityVerifier.api_auth_key

mock_models_get_secret_by_name.assert_called_once_with(model_EligibilityVerifier.api_auth_key_secret_name)
assert secret_value == mock_models_get_secret_by_name.return_value


@pytest.mark.django_db
def test_PaymentProcessor_str(model_PaymentProcessor):
assert str(model_PaymentProcessor) == model_PaymentProcessor.name
Expand Down

0 comments on commit 7f153b6

Please sign in to comment.