refactor(models): AuthProvider.client_id is a secret field

* update definition to use new field
* update migrations
* remove env var from terraform definitions

benefits/core/migrations/0001_initial.py

@@ -1,10 +1,12 @@
-# Generated by Django 4.2.4 on 2023-08-16 15:06
+# Generated by Django 5.0.1 on 2024-02-06 03:46
 
-from django.db import migrations, models
+import benefits.core.models
 import django.db.models.deletion
+from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
+
     initial = True
 
     dependencies = []
@@ -17,7 +19,12 @@ class Migration(migrations.Migration):
                 ("sign_out_button_template", models.TextField(null=True)),
                 ("sign_out_link_template", models.TextField(null=True)),
                 ("client_name", models.TextField()),
-                ("client_id", models.TextField()),
+                (
+                    "client_id_secret_name",
+                    benefits.core.models.SecretValueField(
+                        max_length=127, validators=[benefits.core.models.SecretNameValidator()]
+                    ),
+                ),
                 ("authority", models.TextField()),
                 ("scope", models.TextField(null=True)),
                 ("claim", models.TextField(null=True)),

benefits/core/migrations/0002_data.py

@@ -160,7 +160,7 @@ def load_data(app, *args, **kwargs):
         sign_out_button_template="core/includes/button--sign-out--login-gov.html",
         sign_out_link_template="core/includes/link--sign-out--login-gov.html",
         client_name=os.environ.get("SENIOR_AUTH_PROVIDER_CLIENT_NAME", "senior-benefits-oauth-client-name"),
-        client_id=os.environ.get("AUTH_PROVIDER_CLIENT_ID", "benefits-oauth-client-id"),
+        client_id_secret_name="auth-provider-client-id",
         authority=os.environ.get("AUTH_PROVIDER_AUTHORITY", "https://example.com"),
         scope=os.environ.get("SENIOR_AUTH_PROVIDER_SCOPE", "verify:senior"),
         claim=os.environ.get("SENIOR_AUTH_PROVIDER_CLAIM", "senior"),
@@ -171,7 +171,7 @@ def load_data(app, *args, **kwargs):
         sign_out_button_template="core/includes/button--sign-out--login-gov.html",
         sign_out_link_template="core/includes/link--sign-out--login-gov.html",
         client_name=os.environ.get("VETERAN_AUTH_PROVIDER_CLIENT_NAME", "veteran-benefits-oauth-client-name"),
-        client_id=os.environ.get("AUTH_PROVIDER_CLIENT_ID", "benefits-oauth-client-id"),
+        client_id_secret_name="auth-provider-client-id",
         authority=os.environ.get("AUTH_PROVIDER_AUTHORITY", "https://example.com"),
         scope=os.environ.get("VETERAN_AUTH_PROVIDER_SCOPE", "verify:veteran"),
         claim=os.environ.get("VETERAN_AUTH_PROVIDER_CLAIM", "veteran"),

benefits/core/models.py

@@ -13,6 +13,8 @@
 
 import requests
 
+from benefits.secrets import get_secret_by_name
+
 
 logger = logging.getLogger(__name__)
 
@@ -104,7 +106,7 @@ class AuthProvider(models.Model):
     sign_out_button_template = models.TextField(null=True)
     sign_out_link_template = models.TextField(null=True)
     client_name = models.TextField()
-    client_id = models.TextField()
+    client_id_secret_name = SecretValueField()
     authority = models.TextField()
     scope = models.TextField(null=True)
     claim = models.TextField(null=True)
@@ -118,6 +120,10 @@ def supports_claims_verification(self):
     def supports_sign_out(self):
         return bool(self.sign_out_button_template) or bool(self.sign_out_link_template)
 
+    @property
+    def client_id(self):
+        return get_secret_by_name(self.client_id_secret_name)
+
 
 class EligibilityType(models.Model):
     """A single conditional eligibility type."""

terraform/app_service.tf

@@ -110,7 +110,6 @@ resource "azurerm_linux_web_app" "main" {
     "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_CLIENT_ID"                              = "${local.secret_prefix}auth-provider-client-id)"
     "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)"

tests/pytest/conftest.py

@@ -47,7 +47,7 @@ def model_AuthProvider():
         sign_out_button_template="core/includes/button--sign-out--senior.html",
         sign_out_link_template="core/includes/link--sign-out--senior.html",
         client_name="Client",
-        client_id="1234",
+        client_id_secret_name="1234",
         authority="https://example.com",
     )
 

tests/pytest/core/test_models.py

@@ -53,6 +53,11 @@ 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
@@ -88,6 +93,14 @@ def test_model_AuthProvider(model_AuthProvider):
     assert model_AuthProvider.supports_sign_out
 
 
+@pytest.mark.django_db
+def test_model_AuthProvider_client_id(model_AuthProvider, mock_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
+
+
 @pytest.mark.django_db
 def test_model_AuthProvider_with_verification(model_AuthProvider_with_verification):
     assert model_AuthProvider_with_verification.supports_claims_verification