refactor(secrets): enforce name validation in helper function

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()]
                     ),
                 ),
             ],

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

benefits/secrets.py

@@ -1,23 +1,52 @@
 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__)
 
 
 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()
 

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

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,14 +15,67 @@ def mock_DefaultAzureCredential(mocker):
 
 @pytest.fixture
 def secret_name():
-    return "the secret name"
+    return "the-secret-name"
 
 
 @pytest.fixture
 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