diff --git a/benefits/core/migrations/0001_initial.py b/benefits/core/migrations/0001_initial.py index db5f9936da..5eaa52e692 100644 --- a/benefits/core/migrations/0001_initial.py +++ b/benefits/core/migrations/0001_initial.py @@ -1,6 +1,7 @@ # Generated by Django 5.0.1 on 2024-02-06 18:09 import benefits.core.models +import benefits.secrets import django.db.models.deletion from django.db import migrations, models @@ -21,9 +22,7 @@ class Migration(migrations.Migration): ("client_name", models.TextField()), ( "client_id_secret_name", - benefits.core.models.SecretValueField( - max_length=127, validators=[benefits.core.models.SecretNameValidator()] - ), + benefits.core.models.SecretValueField(max_length=127, validators=[benefits.secrets.SecretNameValidator()]), ), ("authority", models.TextField()), ("scope", models.TextField(null=True)), @@ -51,7 +50,7 @@ class Migration(migrations.Migration): ( "api_auth_key_secret_name", benefits.core.models.SecretValueField( - max_length=127, null=True, validators=[benefits.core.models.SecretNameValidator()] + max_length=127, null=True, validators=[benefits.secrets.SecretNameValidator()] ), ), ("jwe_cek_enc", models.TextField(null=True)), @@ -96,7 +95,7 @@ class Migration(migrations.Migration): ( "text_secret_name", benefits.core.models.SecretValueField( - max_length=127, null=True, validators=[benefits.core.models.SecretNameValidator()] + max_length=127, null=True, validators=[benefits.secrets.SecretNameValidator()] ), ), ], diff --git a/benefits/core/models.py b/benefits/core/models.py index 2280e55a1b..7f51244472 100644 --- a/benefits/core/models.py +++ b/benefits/core/models.py @@ -5,44 +5,19 @@ from functools import cached_property import importlib import logging -import re -from django.core.validators import RegexValidator from django.conf import settings from django.db import models from django.urls import reverse import requests -from benefits.secrets import get_secret_by_name +from benefits.secrets import NAME_VALIDATOR, get_secret_by_name logger = logging.getLogger(__name__) -class SecretNameValidator(RegexValidator): - """RegexValidator that validates a secret name. - - Azure KeyVault currently enforces the following rules: - - * The value must be between 1 and 127 characters long. - * Secret names can only contain alphanumeric characters and dashes. - - Read more about Azure KeyVault naming rules: - https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules#microsoftkeyvault - - Read more about Django validators: - https://docs.djangoproject.com/en/5.0/ref/validators/#module-django.core.validators - """ - - def __init__(self, *args, **kwargs): - kwargs["regex"] = re.compile(r"^[-a-zA-Z0-9]{1,127}$", re.ASCII) - kwargs["message"] = ( - "Enter a valid secret name of between 1-127 alphanumeric ASCII characters and the hyphen character only." - ) - super().__init__(*args, **kwargs) - - class SecretValueField(models.SlugField): """Field that handles retrieving a value from a secret store. @@ -51,8 +26,6 @@ class SecretValueField(models.SlugField): The secret value itself MUST NEVER be stored in this field. """ - NAME_VALIDATOR = SecretNameValidator() - description = """Field that handles retrieving a value from a secret store. The field value is the name of the secret to be retrieved. Must be between 1-127 alphanumeric ASCII characters or hyphen @@ -62,7 +35,7 @@ class SecretValueField(models.SlugField): """ def __init__(self, *args, **kwargs): - kwargs["validators"] = [self.NAME_VALIDATOR] + kwargs["validators"] = [NAME_VALIDATOR] # although the validator also checks for a max length of 127 # this setting enforces the length at the database column level as well kwargs["max_length"] = 127 diff --git a/benefits/secrets.py b/benefits/secrets.py index 293b6d9092..0d1668d5e1 100644 --- a/benefits/secrets.py +++ b/benefits/secrets.py @@ -1,11 +1,13 @@ import logging import os +import re import sys from azure.core.exceptions import ClientAuthenticationError from azure.identity import DefaultAzureCredential from azure.keyvault.secrets import SecretClient from django.conf import settings +from django.core.validators import RegexValidator logger = logging.getLogger(__name__) @@ -13,11 +15,38 @@ KEY_VAULT_URL = "https://kv-cdt-pub-calitp-{env}-001.vault.azure.net/" +class SecretNameValidator(RegexValidator): + """RegexValidator that validates a secret name. + + Azure KeyVault currently enforces the following rules: + + * The value must be between 1 and 127 characters long. + * Secret names can only contain alphanumeric characters and dashes. + + Read more about Azure KeyVault naming rules: + https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules#microsoftkeyvault + + Read more about Django validators: + https://docs.djangoproject.com/en/5.0/ref/validators/#module-django.core.validators + """ + + def __init__(self, *args, **kwargs): + kwargs["regex"] = re.compile(r"^[-a-zA-Z0-9]{1,127}$", re.ASCII) + kwargs["message"] = ( + "Enter a valid secret name of between 1-127 alphanumeric ASCII characters and the hyphen character only." + ) + super().__init__(*args, **kwargs) + + +NAME_VALIDATOR = SecretNameValidator() + + def get_secret_by_name(secret_name, client=None): """Read a value from the secret store, currently Azure KeyVault. When `settings.RUNTIME_ENVIRONMENT() == "local"`, reads from the environment instead. """ + NAME_VALIDATOR(secret_name) runtime_env = settings.RUNTIME_ENVIRONMENT() diff --git a/tests/pytest/core/test_models.py b/tests/pytest/core/test_models.py index ee83452136..8637fde509 100644 --- a/tests/pytest/core/test_models.py +++ b/tests/pytest/core/test_models.py @@ -1,9 +1,9 @@ from django.conf import settings -from django.core.exceptions import ValidationError import pytest -from benefits.core.models import SecretNameValidator, SecretValueField, EligibilityType, EligibilityVerifier, TransitAgency +from benefits.core.models import SecretValueField, EligibilityType, EligibilityVerifier, TransitAgency +import benefits.secrets @pytest.fixture @@ -12,45 +12,10 @@ def mock_requests_get_pem_data(mocker): return mocker.patch("benefits.core.models.requests.get", return_value=mocker.Mock(text="PEM text")) -@pytest.mark.parametrize( - "secret_name", - [ - "a", - "1", - "one", - "one-two-three", - "1-2-3", - "this-is-a-really-long-secret-name-in-fact-it-is-the-absolute-maximum-length-of-127-characters-to-be-exact-and-now-it-has-enough", # noqa: E501 - ], -) -def test_SecretNameValidator_valid(secret_name): - validator = SecretNameValidator() - - # a successful validation does not raise an Exception and returns None - assert validator(secret_name) is None - - -@pytest.mark.parametrize( - "secret_name", - [ - "", - "!", - "underscores_not_allowed", - "this-is-a-really-long-secret-name-in-fact-it-much-much-longer-than-the-absolute-maximum-length-of-127-characters-and-now-it-has-enough-to-be-too-long", # noqa: E501 - ], -) -def test_SecretNameValidator_invalid(secret_name): - validator = SecretNameValidator() - - # an unsuccessful validation raises django.core.exceptions.ValidationError - with pytest.raises(ValidationError): - validator(secret_name) - - def test_SecretValueField_init(): field = SecretValueField() - assert SecretValueField.NAME_VALIDATOR in field.validators + assert benefits.secrets.NAME_VALIDATOR in field.validators assert field.max_length == 127 assert field.blank is False assert field.null is False diff --git a/tests/pytest/test_secrets.py b/tests/pytest/test_secrets.py index 1029538b0e..0d43415301 100644 --- a/tests/pytest/test_secrets.py +++ b/tests/pytest/test_secrets.py @@ -1,7 +1,8 @@ -import pytest from azure.core.exceptions import ClientAuthenticationError +from django.core.exceptions import ValidationError +import pytest -from benefits.secrets import KEY_VAULT_URL, get_secret_by_name +from benefits.secrets import KEY_VAULT_URL, SecretNameValidator, NAME_VALIDATOR, get_secret_by_name @pytest.fixture(autouse=True) @@ -14,7 +15,7 @@ def mock_DefaultAzureCredential(mocker): @pytest.fixture def secret_name(): - return "the secret name" + return "the-secret-name" @pytest.fixture @@ -22,6 +23,59 @@ def secret_value(): return "the secret value" +@pytest.mark.parametrize( + "secret_name", + [ + "a", + "1", + "one", + "one-two-three", + "1-2-3", + "this-is-a-really-long-secret-name-in-fact-it-is-the-absolute-maximum-length-of-127-characters-to-be-exact-and-now-it-has-enough", # noqa: E501 + ], +) +def test_SecretNameValidator_valid(secret_name): + validator = SecretNameValidator() + + # a successful validation does not raise an Exception and returns None + assert validator(secret_name) is None + assert NAME_VALIDATOR(secret_name) is None + + +@pytest.mark.parametrize( + "secret_name", + [ + "", + "!", + "underscores_not_allowed", + "this-is-a-really-long-secret-name-in-fact-it-much-much-longer-than-the-absolute-maximum-length-of-127-characters-and-now-it-has-enough-to-be-too-long", # noqa: E501 + ], +) +def test_SecretNameValidator_invalid(secret_name): + validator = SecretNameValidator() + + # an unsuccessful validation raises django.core.exceptions.ValidationError + with pytest.raises(ValidationError): + validator(secret_name) + + with pytest.raises(ValidationError): + NAME_VALIDATOR(secret_name) + + +@pytest.mark.parametrize( + "secret_name", + [ + "", + "!", + "underscores_not_allowed", + "this-is-a-really-long-secret-name-in-fact-it-much-much-longer-than-the-absolute-maximum-length-of-127-characters-and-now-it-has-enough-to-be-too-long", # noqa: E501 + ], +) +def test_get_secret_by_name__invalid_name(secret_name): + with pytest.raises(ValidationError): + get_secret_by_name(secret_name) + + @pytest.mark.parametrize("runtime_env", ["dev", "test", "prod"]) def test_get_secret_by_name__with_client__returns_secret_value(mocker, runtime_env, settings, secret_name, secret_value): settings.RUNTIME_ENVIRONMENT = lambda: runtime_env