From 6522d2d10655b86c50bf753516616d98c7d886c7 Mon Sep 17 00:00:00 2001 From: Angela Tran Date: Wed, 31 Jul 2024 20:23:06 +0000 Subject: [PATCH] refactor(model): rename AuthProvider to ClaimsProvider --- benefits/core/admin.py | 4 +- ...0016_rename_authprovider_claimsprovider.py | 52 ++++++++++++ benefits/core/migrations/local_fixtures.json | 6 +- benefits/core/models.py | 6 +- benefits/oauth/client.py | 2 +- docs/configuration/oauth.md | 6 +- tests/pytest/conftest.py | 80 ++++++++++--------- tests/pytest/core/test_models.py | 60 +++++++------- tests/pytest/oauth/test_client.py | 8 +- tests/pytest/oauth/test_views.py | 12 +-- 10 files changed, 146 insertions(+), 90 deletions(-) create mode 100644 benefits/core/migrations/0016_rename_authprovider_claimsprovider.py diff --git a/benefits/core/admin.py b/benefits/core/admin.py index 2f5d2d28e..2e3c3a56b 100644 --- a/benefits/core/admin.py +++ b/benefits/core/admin.py @@ -21,8 +21,8 @@ admin.site.register(models.PemData) -@admin.register(models.AuthProvider) -class AuthProviderAdmin(admin.ModelAdmin): # pragma: no cover +@admin.register(models.ClaimsProvider) +class ClaimsProviderAdmin(admin.ModelAdmin): # pragma: no cover def get_exclude(self, request, obj=None): if not request.user.is_superuser: return ["client_id_secret_name"] diff --git a/benefits/core/migrations/0016_rename_authprovider_claimsprovider.py b/benefits/core/migrations/0016_rename_authprovider_claimsprovider.py new file mode 100644 index 000000000..bf7bf9430 --- /dev/null +++ b/benefits/core/migrations/0016_rename_authprovider_claimsprovider.py @@ -0,0 +1,52 @@ +from django.contrib.auth.management import create_permissions +from django.db import migrations + + +def create_all_permissions(apps, schema_editor): + for app_config in apps.get_app_configs(): + app_config.models_module = True + create_permissions(app_config, apps=apps, verbosity=0) + app_config.models_module = None + + +def delete_auth_provider_permissions(apps, schema_editor): + Permission = apps.get_model("auth", "Permission") + old_permission_names = [ + "Can view auth provider", + "Can change auth provider", + "Can add auth provider", + "Can delete auth provider", + ] + + for name in old_permission_names: + old_permission = Permission.objects.get(name=name) + old_permission.delete() + + +def add_staff_claimsprovider_permissions(apps, schema_editor): + Group = apps.get_model("auth", "Group") + staff_group = Group.objects.get(name="Cal-ITP") + + Permission = apps.get_model("auth", "Permission") + new_permission_names = ["Can view claims provider", "Can change claims provider"] + + for name in new_permission_names: + new_permission = Permission.objects.get(name=name) + staff_group.permissions.add(new_permission) + + +class Migration(migrations.Migration): + + dependencies = [ + ("core", "0015_staff_group_edit_permissions"), + ] + + operations = [ + migrations.RenameModel( + old_name="AuthProvider", + new_name="ClaimsProvider", + ), + migrations.RunPython(create_all_permissions), # this is needed to create the new permissions for the renamed model + migrations.RunPython(delete_auth_provider_permissions), + migrations.RunPython(add_staff_claimsprovider_permissions), + ] diff --git a/benefits/core/migrations/local_fixtures.json b/benefits/core/migrations/local_fixtures.json index 9edf82141..3a2a5a114 100644 --- a/benefits/core/migrations/local_fixtures.json +++ b/benefits/core/migrations/local_fixtures.json @@ -27,7 +27,7 @@ } }, { - "model": "core.authprovider", + "model": "core.claimsprovider", "pk": 1, "fields": { "sign_out_button_template": "core/includes/button--sign-out--login-gov.html", @@ -41,7 +41,7 @@ } }, { - "model": "core.authprovider", + "model": "core.claimsprovider", "pk": 2, "fields": { "sign_out_button_template": "core/includes/button--sign-out--login-gov.html", @@ -55,7 +55,7 @@ } }, { - "model": "core.authprovider", + "model": "core.claimsprovider", "pk": 3, "fields": { "sign_out_button_template": "core/includes/button--sign-out--login-gov.html", diff --git a/benefits/core/models.py b/benefits/core/models.py index 0eb411941..e9a1cded2 100644 --- a/benefits/core/models.py +++ b/benefits/core/models.py @@ -75,8 +75,8 @@ def data(self): return secret_data if secret_data is not None else remote_data -class AuthProvider(models.Model): - """An entity that provides authentication for eligibility verifiers.""" +class ClaimsProvider(models.Model): + """An entity that provides claims for eligibility verification.""" id = models.AutoField(primary_key=True) sign_out_button_template = models.TextField(null=True, blank=True) @@ -179,7 +179,7 @@ class EligibilityVerifier(models.Model): jwe_encryption_alg = models.TextField(null=True, blank=True) # The JWS-compatible signing algorithm jws_signing_alg = models.TextField(null=True, blank=True) - auth_provider = models.ForeignKey(AuthProvider, on_delete=models.PROTECT, null=True, blank=True) + auth_provider = models.ForeignKey(ClaimsProvider, on_delete=models.PROTECT, null=True, blank=True) selection_label_template = models.TextField() start_template = models.TextField(null=True, blank=True) # reference to a form class used by this Verifier, e.g. benefits.app.forms.FormClass diff --git a/benefits/oauth/client.py b/benefits/oauth/client.py index 923b29ab6..596be9ce0 100644 --- a/benefits/oauth/client.py +++ b/benefits/oauth/client.py @@ -41,7 +41,7 @@ def _authorize_params(scheme): def _register_provider(oauth_registry, provider): """ - Register OAuth clients into the given registry, using configuration from AuthProvider model. + Register OAuth clients into the given registry, using configuration from ClaimsProvider model. Adapted from https://stackoverflow.com/a/64174413. """ diff --git a/docs/configuration/oauth.md b/docs/configuration/oauth.md index e9345d16c..4651a2983 100644 --- a/docs/configuration/oauth.md +++ b/docs/configuration/oauth.md @@ -22,9 +22,9 @@ for more details about what features are available. Specifically, from Authlib w ## Django configuration -OAuth settings are configured as instances of the [`AuthProvider` model](../development/models-migrations.md). +OAuth settings are configured as instances of the [`ClaimsProvider` model](../development/models-migrations.md). -The [data migration file](./data.md) contains sample values for an `AuthProvider` configuration. You can set values for a real Open ID Connect provider in environment variables so that they are used instead of the sample values. +The [data migration file](./data.md) contains sample values for an `ClaimsProvider` configuration. You can set values for a real Open ID Connect provider in environment variables so that they are used instead of the sample values. ## Django usage @@ -34,6 +34,6 @@ use in e.g. views. - `oauth` is an `authlib.integrations.django_client.OAuth` instance Consumers call `benefits.oauth.client.create_client(oauth, provider)` with the name of a client to obtain an Authlib client -instance. If that client name has not been registered yet, `_register_provider(oauth_registry, provider)` uses data from the given `AuthProvider` instance to register the client into this instance and returns the client object. +instance. If that client name has not been registered yet, `_register_provider(oauth_registry, provider)` uses data from the given `ClaimsProvider` instance to register the client into this instance and returns the client object. [oauth-client]: https://github.com/cal-itp/benefits/blob/main/benefits/oauth/client.py diff --git a/tests/pytest/conftest.py b/tests/pytest/conftest.py index d47cdb915..3718b49cd 100644 --- a/tests/pytest/conftest.py +++ b/tests/pytest/conftest.py @@ -7,7 +7,7 @@ from pytest_socket import disable_socket from benefits.core import session -from benefits.core.models import AuthProvider, EligibilityType, EligibilityVerifier, PaymentProcessor, PemData, TransitAgency +from benefits.core.models import ClaimsProvider, EligibilityType, EligibilityVerifier, PaymentProcessor, PemData, TransitAgency def pytest_runtest_setup(): @@ -47,8 +47,8 @@ def model_PemData(): @pytest.fixture -def model_AuthProvider(): - auth_provider = AuthProvider.objects.create( +def model_ClaimsProvider(): + auth_provider = ClaimsProvider.objects.create( 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", @@ -60,43 +60,43 @@ def model_AuthProvider(): @pytest.fixture -def model_AuthProvider_with_verification(model_AuthProvider): - model_AuthProvider.scope = "scope" - model_AuthProvider.claim = "claim" - model_AuthProvider.save() +def model_ClaimsProvider_with_verification(model_ClaimsProvider): + model_ClaimsProvider.scope = "scope" + model_ClaimsProvider.claim = "claim" + model_ClaimsProvider.save() - return model_AuthProvider + return model_ClaimsProvider @pytest.fixture -def model_AuthProvider_with_verification_no_sign_out(model_AuthProvider): - model_AuthProvider.scope = "scope" - model_AuthProvider.claim = "claim" - model_AuthProvider.sign_out_button_template = None - model_AuthProvider.sign_out_link_template = None - model_AuthProvider.save() +def model_ClaimsProvider_with_verification_no_sign_out(model_ClaimsProvider): + model_ClaimsProvider.scope = "scope" + model_ClaimsProvider.claim = "claim" + model_ClaimsProvider.sign_out_button_template = None + model_ClaimsProvider.sign_out_link_template = None + model_ClaimsProvider.save() - return model_AuthProvider + return model_ClaimsProvider @pytest.fixture -def model_AuthProvider_without_verification(model_AuthProvider): - model_AuthProvider.scope = None - model_AuthProvider.claim = None - model_AuthProvider.save() +def model_ClaimsProvider_without_verification(model_ClaimsProvider): + model_ClaimsProvider.scope = None + model_ClaimsProvider.claim = None + model_ClaimsProvider.save() - return model_AuthProvider + return model_ClaimsProvider @pytest.fixture -def model_AuthProvider_without_verification_no_sign_out(model_AuthProvider): - model_AuthProvider.scope = None - model_AuthProvider.claim = None - model_AuthProvider.sign_out_button_template = None - model_AuthProvider.sign_out_link_template = None - model_AuthProvider.save() +def model_ClaimsProvider_without_verification_no_sign_out(model_ClaimsProvider): + model_ClaimsProvider.scope = None + model_ClaimsProvider.claim = None + model_ClaimsProvider.sign_out_button_template = None + model_ClaimsProvider.sign_out_link_template = None + model_ClaimsProvider.save() - return model_AuthProvider + return model_ClaimsProvider @pytest.fixture @@ -168,8 +168,10 @@ def model_EligibilityVerifier(model_PemData, model_EligibilityType): @pytest.fixture -def model_EligibilityVerifier_AuthProvider_with_verification(model_AuthProvider_with_verification, model_EligibilityVerifier): - model_EligibilityVerifier.auth_provider = model_AuthProvider_with_verification +def model_EligibilityVerifier_ClaimsProvider_with_verification( + model_ClaimsProvider_with_verification, model_EligibilityVerifier +): + model_EligibilityVerifier.auth_provider = model_ClaimsProvider_with_verification model_EligibilityVerifier.save() return model_EligibilityVerifier @@ -276,23 +278,25 @@ def mocked_session_verifier(mocker, model_EligibilityVerifier): @pytest.fixture -def mocked_session_verifier_oauth(mocker, model_EligibilityVerifier_AuthProvider_with_verification): +def mocked_session_verifier_oauth(mocker, model_EligibilityVerifier_ClaimsProvider_with_verification): return mocker.patch( - "benefits.core.session.verifier", autospec=True, return_value=model_EligibilityVerifier_AuthProvider_with_verification + "benefits.core.session.verifier", + autospec=True, + return_value=model_EligibilityVerifier_ClaimsProvider_with_verification, ) @pytest.fixture def mocked_session_verifier_uses_auth_verification( - model_EligibilityVerifier_AuthProvider_with_verification, mocked_session_verifier_oauth + model_EligibilityVerifier_ClaimsProvider_with_verification, mocked_session_verifier_oauth ): - mock_verifier = model_EligibilityVerifier_AuthProvider_with_verification - mock_verifier.name = model_EligibilityVerifier_AuthProvider_with_verification.name + mock_verifier = model_EligibilityVerifier_ClaimsProvider_with_verification + mock_verifier.name = model_EligibilityVerifier_ClaimsProvider_with_verification.name mock_verifier.auth_provider.sign_out_button_template = ( - model_EligibilityVerifier_AuthProvider_with_verification.auth_provider.sign_out_button_template + model_EligibilityVerifier_ClaimsProvider_with_verification.auth_provider.sign_out_button_template ) mock_verifier.auth_provider.sign_out_link_template = ( - model_EligibilityVerifier_AuthProvider_with_verification.auth_provider.sign_out_link_template + model_EligibilityVerifier_ClaimsProvider_with_verification.auth_provider.sign_out_link_template ) mocked_session_verifier_oauth.return_value = mock_verifier return mocked_session_verifier_oauth @@ -300,10 +304,10 @@ def mocked_session_verifier_uses_auth_verification( @pytest.fixture def mocked_session_verifier_does_not_use_auth_verification( - mocked_session_verifier_uses_auth_verification, model_AuthProvider_without_verification + mocked_session_verifier_uses_auth_verification, model_ClaimsProvider_without_verification ): mocked_verifier = mocked_session_verifier_uses_auth_verification - mocked_verifier.auth_provider = model_AuthProvider_without_verification + mocked_verifier.auth_provider = model_ClaimsProvider_without_verification return mocked_verifier diff --git a/tests/pytest/core/test_models.py b/tests/pytest/core/test_models.py index 63c6ae3e7..c446679b1 100644 --- a/tests/pytest/core/test_models.py +++ b/tests/pytest/core/test_models.py @@ -93,40 +93,40 @@ def test_PemData_data_text_secret_name_and_remote__uses_remote( @pytest.mark.django_db -def test_model_AuthProvider(model_AuthProvider): - assert not model_AuthProvider.supports_claims_verification - assert model_AuthProvider.supports_sign_out - assert str(model_AuthProvider) == model_AuthProvider.client_name +def test_model_ClaimsProvider(model_ClaimsProvider): + assert not model_ClaimsProvider.supports_claims_verification + assert model_ClaimsProvider.supports_sign_out + assert str(model_ClaimsProvider) == model_ClaimsProvider.client_name @pytest.mark.django_db -def test_model_AuthProvider_client_id(model_AuthProvider, mock_models_get_secret_by_name): - secret_value = model_AuthProvider.client_id +def test_model_ClaimsProvider_client_id(model_ClaimsProvider, mock_models_get_secret_by_name): + secret_value = model_ClaimsProvider.client_id - mock_models_get_secret_by_name.assert_called_once_with(model_AuthProvider.client_id_secret_name) + mock_models_get_secret_by_name.assert_called_once_with(model_ClaimsProvider.client_id_secret_name) assert secret_value == mock_models_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 +def test_model_ClaimsProvider_with_verification(model_ClaimsProvider_with_verification): + assert model_ClaimsProvider_with_verification.supports_claims_verification @pytest.mark.django_db -def test_model_AuthProvider_with_verification_no_sign_out(model_AuthProvider_with_verification_no_sign_out): - assert model_AuthProvider_with_verification_no_sign_out.supports_claims_verification - assert not model_AuthProvider_with_verification_no_sign_out.supports_sign_out +def test_model_ClaimsProvider_with_verification_no_sign_out(model_ClaimsProvider_with_verification_no_sign_out): + assert model_ClaimsProvider_with_verification_no_sign_out.supports_claims_verification + assert not model_ClaimsProvider_with_verification_no_sign_out.supports_sign_out @pytest.mark.django_db -def test_model_AuthProvider_without_verification(model_AuthProvider_without_verification): - assert not model_AuthProvider_without_verification.supports_claims_verification +def test_model_ClaimsProvider_without_verification(model_ClaimsProvider_without_verification): + assert not model_ClaimsProvider_without_verification.supports_claims_verification @pytest.mark.django_db -def test_model_AuthProvider_without_verification_no_sign_out(model_AuthProvider_without_verification_no_sign_out): - assert not model_AuthProvider_without_verification_no_sign_out.supports_claims_verification - assert not model_AuthProvider_without_verification_no_sign_out.supports_sign_out +def test_model_ClaimsProvider_without_verification_no_sign_out(model_ClaimsProvider_without_verification_no_sign_out): + assert not model_ClaimsProvider_without_verification_no_sign_out.supports_claims_verification + assert not model_ClaimsProvider_without_verification_no_sign_out.supports_sign_out @pytest.mark.django_db @@ -292,47 +292,47 @@ def test_EligibilityVerifier_by_id_nonmatching(): @pytest.mark.django_db -def test_EligibilityVerifier_with_AuthProvider_with_verification( - model_EligibilityVerifier, model_AuthProvider_with_verification +def test_EligibilityVerifier_with_ClaimsProvider_with_verification( + model_EligibilityVerifier, model_ClaimsProvider_with_verification ): - model_EligibilityVerifier.auth_provider = model_AuthProvider_with_verification + model_EligibilityVerifier.auth_provider = model_ClaimsProvider_with_verification assert model_EligibilityVerifier.is_auth_required assert model_EligibilityVerifier.uses_auth_verification @pytest.mark.django_db -def test_EligibilityVerifier_with_AuthProvider_with_verification_no_sign_out( - model_EligibilityVerifier, model_AuthProvider_with_verification_no_sign_out +def test_EligibilityVerifier_with_ClaimsProvider_with_verification_no_sign_out( + model_EligibilityVerifier, model_ClaimsProvider_with_verification_no_sign_out ): - model_EligibilityVerifier.auth_provider = model_AuthProvider_with_verification_no_sign_out + model_EligibilityVerifier.auth_provider = model_ClaimsProvider_with_verification_no_sign_out assert model_EligibilityVerifier.is_auth_required assert model_EligibilityVerifier.uses_auth_verification @pytest.mark.django_db -def test_EligibilityVerifier_with_AuthProvider_without_verification( - model_EligibilityVerifier, model_AuthProvider_without_verification +def test_EligibilityVerifier_with_ClaimsProvider_without_verification( + model_EligibilityVerifier, model_ClaimsProvider_without_verification ): - model_EligibilityVerifier.auth_provider = model_AuthProvider_without_verification + model_EligibilityVerifier.auth_provider = model_ClaimsProvider_without_verification assert model_EligibilityVerifier.is_auth_required assert not model_EligibilityVerifier.uses_auth_verification @pytest.mark.django_db -def test_EligibilityVerifier_with_AuthProvider_without_verification_no_sign_out( - model_EligibilityVerifier, model_AuthProvider_without_verification_no_sign_out +def test_EligibilityVerifier_with_ClaimsProvider_without_verification_no_sign_out( + model_EligibilityVerifier, model_ClaimsProvider_without_verification_no_sign_out ): - model_EligibilityVerifier.auth_provider = model_AuthProvider_without_verification_no_sign_out + model_EligibilityVerifier.auth_provider = model_ClaimsProvider_without_verification_no_sign_out assert model_EligibilityVerifier.is_auth_required assert not model_EligibilityVerifier.uses_auth_verification @pytest.mark.django_db -def test_EligibilityVerifier_without_AuthProvider(model_EligibilityVerifier): +def test_EligibilityVerifier_without_ClaimsProvider(model_EligibilityVerifier): model_EligibilityVerifier.auth_provider = None assert not model_EligibilityVerifier.is_auth_required diff --git a/tests/pytest/oauth/test_client.py b/tests/pytest/oauth/test_client.py index 3bfe63df8..d6b45c61d 100644 --- a/tests/pytest/oauth/test_client.py +++ b/tests/pytest/oauth/test_client.py @@ -1,6 +1,6 @@ import pytest -from benefits.core.models import AuthProvider +from benefits.core.models import ClaimsProvider from benefits.oauth.client import _client_kwargs, _server_metadata_url, _authorize_params, _register_provider, create_client @@ -40,7 +40,7 @@ def test_authorize_params_no_scheme(): @pytest.mark.django_db def test_register_provider(mocker, mocked_oauth_registry): - mocked_client_provider = mocker.Mock(spec=AuthProvider) + mocked_client_provider = mocker.Mock(spec=ClaimsProvider) mocked_client_provider.client_name = "client_name_1" mocked_client_provider.client_id = "client_id_1" @@ -61,7 +61,7 @@ def test_register_provider(mocker, mocked_oauth_registry): @pytest.mark.django_db def test_create_client_already_registered(mocker, mocked_oauth_registry): - mocked_client_provider = mocker.Mock(spec=AuthProvider) + mocked_client_provider = mocker.Mock(spec=ClaimsProvider) mocked_client_provider.client_name = "client_name_1" mocked_client_provider.client_id = "client_id_1" @@ -73,7 +73,7 @@ def test_create_client_already_registered(mocker, mocked_oauth_registry): @pytest.mark.django_db def test_create_client_already_not_registered_yet(mocker, mocked_oauth_registry): - mocked_client_provider = mocker.Mock(spec=AuthProvider) + mocked_client_provider = mocker.Mock(spec=ClaimsProvider) mocked_client_provider.client_name = "client_name_1" mocked_client_provider.client_id = "client_id_1" diff --git a/tests/pytest/oauth/test_views.py b/tests/pytest/oauth/test_views.py index adb0b5d5b..3f46258c2 100644 --- a/tests/pytest/oauth/test_views.py +++ b/tests/pytest/oauth/test_views.py @@ -52,14 +52,14 @@ def mocked_oauth_client_or_error_redirect__error(mocked_oauth_create_client): @pytest.mark.usefixtures("mocked_session_verifier_uses_auth_verification") def test_oauth_client_or_error_redirect_no_oauth_client( app_request, - model_AuthProvider_with_verification, + model_ClaimsProvider_with_verification, mocked_oauth_create_client, mocked_analytics_module, mocked_sentry_sdk_module, ): mocked_oauth_create_client.return_value = None - result = _oauth_client_or_error_redirect(app_request, model_AuthProvider_with_verification) + result = _oauth_client_or_error_redirect(app_request, model_ClaimsProvider_with_verification) assert result.status_code == 302 assert result.url == reverse(ROUTE_SYSTEM_ERROR) @@ -70,9 +70,9 @@ def test_oauth_client_or_error_redirect_no_oauth_client( @pytest.mark.django_db @pytest.mark.usefixtures("mocked_session_verifier_uses_auth_verification", "mocked_oauth_client_or_error_redirect__error") def test_oauth_client_or_error_redirect_oauth_client_exception( - app_request, model_AuthProvider_with_verification, mocked_analytics_module, mocked_sentry_sdk_module + app_request, model_ClaimsProvider_with_verification, mocked_analytics_module, mocked_sentry_sdk_module ): - result = _oauth_client_or_error_redirect(app_request, model_AuthProvider_with_verification) + result = _oauth_client_or_error_redirect(app_request, model_ClaimsProvider_with_verification) assert result.status_code == 302 assert result.url == reverse(ROUTE_SYSTEM_ERROR) @@ -83,9 +83,9 @@ def test_oauth_client_or_error_redirect_oauth_client_exception( @pytest.mark.django_db @pytest.mark.usefixtures("mocked_session_verifier_uses_auth_verification", "mocked_oauth_create_client") def test_oauth_client_or_error_oauth_client( - app_request, model_AuthProvider_with_verification, mocked_analytics_module, mocked_sentry_sdk_module + app_request, model_ClaimsProvider_with_verification, mocked_analytics_module, mocked_sentry_sdk_module ): - result = _oauth_client_or_error_redirect(app_request, model_AuthProvider_with_verification) + result = _oauth_client_or_error_redirect(app_request, model_ClaimsProvider_with_verification) assert hasattr(result, "authorize_redirect") mocked_analytics_module.error.assert_not_called()