From 6fd80b110f7d4426a557a9acf2eb93f1caffccfa Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Mon, 9 Dec 2024 15:31:40 +0100 Subject: [PATCH 1/4] :sparkles: [#125] Reserve identifier field for setup-configuration on model --- mozilla_django_oidc_db/models.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mozilla_django_oidc_db/models.py b/mozilla_django_oidc_db/models.py index e347a54..88ff404 100644 --- a/mozilla_django_oidc_db/models.py +++ b/mozilla_django_oidc_db/models.py @@ -244,6 +244,9 @@ class OpenIDConnectConfig(OpenIDConnectConfigBase): Configuration for authentication/authorization via OpenID connect """ + # reserve this for when it will be added in the future + identifier: ClassVar[str] = "admin-oidc" + username_claim = ClaimField( verbose_name=_("username claim"), default=ClaimFieldDefault("sub"), From b12ac43661c1546ae59783bfe07a3001489ab104 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Mon, 9 Dec 2024 15:32:33 +0100 Subject: [PATCH 2/4] :recycle: [#125] Modify setup-config format to accept list of configs to make the format more future proof, since we will move from SingletonModel to regular Models for the config --- .../setup_configuration/models.py | 11 +++- .../setup_configuration/steps.py | 62 +++++++++++-------- 2 files changed, 44 insertions(+), 29 deletions(-) diff --git a/mozilla_django_oidc_db/setup_configuration/models.py b/mozilla_django_oidc_db/setup_configuration/models.py index 84f71f9..75c0652 100644 --- a/mozilla_django_oidc_db/setup_configuration/models.py +++ b/mozilla_django_oidc_db/setup_configuration/models.py @@ -2,7 +2,7 @@ from django_setup_configuration.fields import DjangoModelRef from django_setup_configuration.models import ConfigurationModel -from pydantic import AnyUrl, Discriminator, Tag +from pydantic import AnyUrl, Discriminator, Field, Tag from typing_extensions import Annotated from mozilla_django_oidc_db.models import OpenIDConnectConfig @@ -46,7 +46,10 @@ def get_endpoint_endpoint_model(endpoint_data): ] -class AdminOIDCConfigurationModel(ConfigurationModel): +class AdminOIDCConfigurationModelItem(ConfigurationModel): + # Currently unused because we use a SingletonModel, but this will be relevant in the + # future + identifier: str = Field(description="a unique identifier for this configuration") # Change default to True enabled: bool = DjangoModelRef(OpenIDConnectConfig, "enabled", default=True) @@ -89,3 +92,7 @@ class Meta: "make_users_staff", ] } + + +class AdminOIDCConfigurationModel(ConfigurationModel): + items: list[AdminOIDCConfigurationModelItem] = Field(default_factory=list) diff --git a/mozilla_django_oidc_db/setup_configuration/steps.py b/mozilla_django_oidc_db/setup_configuration/steps.py index 02f9d9e..9146308 100644 --- a/mozilla_django_oidc_db/setup_configuration/steps.py +++ b/mozilla_django_oidc_db/setup_configuration/steps.py @@ -21,43 +21,51 @@ class AdminOIDCConfigurationStep(BaseConfigurationStep[AdminOIDCConfigurationMod enable_setting = "oidc_db_config_enable" def execute(self, model: AdminOIDCConfigurationModel) -> None: + if len(model.items) != 1: + raise ConfigurationRunFailed( + "You must specify exactly one OIDC configuration" + ) + + config_model = model.items[0] all_settings = { - "enabled": model.enabled, - "oidc_rp_client_id": model.oidc_rp_client_id, - "oidc_rp_client_secret": model.oidc_rp_client_secret, - "oidc_rp_sign_algo": model.oidc_rp_sign_algo, - "oidc_rp_scopes_list": model.oidc_rp_scopes_list, - "oidc_op_jwks_endpoint": model.oidc_op_jwks_endpoint, - "oidc_token_use_basic_auth": model.oidc_token_use_basic_auth, - "oidc_rp_idp_sign_key": model.oidc_rp_idp_sign_key, - "oidc_op_logout_endpoint": model.oidc_op_logout_endpoint, - "oidc_use_nonce": model.oidc_use_nonce, - "oidc_nonce_size": model.oidc_nonce_size, - "oidc_state_size": model.oidc_state_size, - "oidc_keycloak_idp_hint": model.oidc_keycloak_idp_hint, - "userinfo_claims_source": model.userinfo_claims_source, - "username_claim": model.username_claim, - "claim_mapping": model.claim_mapping, - "groups_claim": model.groups_claim, - "sync_groups": model.sync_groups, - "sync_groups_glob_pattern": model.sync_groups_glob_pattern, - "make_users_staff": model.make_users_staff, - "superuser_group_names": model.superuser_group_names, + "enabled": config_model.enabled, + "oidc_rp_client_id": config_model.oidc_rp_client_id, + "oidc_rp_client_secret": config_model.oidc_rp_client_secret, + "oidc_rp_sign_algo": config_model.oidc_rp_sign_algo, + "oidc_rp_scopes_list": config_model.oidc_rp_scopes_list, + "oidc_op_jwks_endpoint": config_model.oidc_op_jwks_endpoint, + "oidc_token_use_basic_auth": config_model.oidc_token_use_basic_auth, + "oidc_rp_idp_sign_key": config_model.oidc_rp_idp_sign_key, + "oidc_op_logout_endpoint": config_model.oidc_op_logout_endpoint, + "oidc_use_nonce": config_model.oidc_use_nonce, + "oidc_nonce_size": config_model.oidc_nonce_size, + "oidc_state_size": config_model.oidc_state_size, + "oidc_keycloak_idp_hint": config_model.oidc_keycloak_idp_hint, + "userinfo_claims_source": config_model.userinfo_claims_source, + "username_claim": config_model.username_claim, + "claim_mapping": config_model.claim_mapping, + "groups_claim": config_model.groups_claim, + "sync_groups": config_model.sync_groups, + "sync_groups_glob_pattern": config_model.sync_groups_glob_pattern, + "make_users_staff": config_model.make_users_staff, + "superuser_group_names": config_model.superuser_group_names, "default_groups": get_groups_by_name( - model.default_groups, model.sync_groups_glob_pattern, model.sync_groups + config_model.default_groups, + config_model.sync_groups_glob_pattern, + config_model.sync_groups, ), } - if isinstance(model.endpoint_config, OIDCDiscoveryEndpoint): + if isinstance(config_model.endpoint_config, OIDCDiscoveryEndpoint): all_settings.update( - oidc_op_discovery_endpoint=model.endpoint_config.oidc_op_discovery_endpoint, + oidc_op_discovery_endpoint=config_model.endpoint_config.oidc_op_discovery_endpoint, ) else: all_settings.update( - oidc_op_authorization_endpoint=model.endpoint_config.oidc_op_authorization_endpoint, - oidc_op_token_endpoint=model.endpoint_config.oidc_op_token_endpoint, - oidc_op_user_endpoint=model.endpoint_config.oidc_op_user_endpoint, + oidc_op_authorization_endpoint=config_model.endpoint_config.oidc_op_authorization_endpoint, + oidc_op_token_endpoint=config_model.endpoint_config.oidc_op_token_endpoint, + oidc_op_user_endpoint=config_model.endpoint_config.oidc_op_user_endpoint, ) form = OpenIDConnectConfigForm( From 21a5dffb3de0e49fac70f4ce9c330910c081f86f Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Mon, 9 Dec 2024 15:32:52 +0100 Subject: [PATCH 3/4] :white_check_mark: [#125] Add/update tests for changed setup config format --- tests/setupconfig/conftest.py | 10 +++ tests/setupconfig/files/defaults.yml | 14 ++-- tests/setupconfig/files/discovery.yml | 10 +-- tests/setupconfig/files/full_setup.yml | 70 ++++++++++--------- .../setupconfig/files/missing_identifier.yml | 9 +++ tests/setupconfig/files/multiple_configs.yml | 17 +++++ tests/setupconfig/files/no_sync_groups.yml | 22 +++--- tests/setupconfig/files/sync_groups.yml | 28 ++++---- tests/setupconfig/test_steps.py | 43 ++++++++++-- 9 files changed, 150 insertions(+), 73 deletions(-) create mode 100644 tests/setupconfig/files/missing_identifier.yml create mode 100644 tests/setupconfig/files/multiple_configs.yml diff --git a/tests/setupconfig/conftest.py b/tests/setupconfig/conftest.py index bf80e86..9a16f26 100644 --- a/tests/setupconfig/conftest.py +++ b/tests/setupconfig/conftest.py @@ -43,6 +43,16 @@ def sync_groups_config_yml(): return "tests/setupconfig/files/sync_groups.yml" +@pytest.fixture() +def multiple_configs_yml(): + return "tests/setupconfig/files/multiple_configs.yml" + + +@pytest.fixture() +def missing_identifier_yml(): + return "tests/setupconfig/files/missing_identifier.yml" + + @pytest.fixture def set_config_to_non_default_values(): """ diff --git a/tests/setupconfig/files/defaults.yml b/tests/setupconfig/files/defaults.yml index 889c392..18fb301 100644 --- a/tests/setupconfig/files/defaults.yml +++ b/tests/setupconfig/files/defaults.yml @@ -1,8 +1,10 @@ oidc_db_config_enable: True oidc_db_config_admin_auth: - oidc_rp_client_id: client-id - oidc_rp_client_secret: secret - endpoint_config: - oidc_op_authorization_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/auth - oidc_op_token_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/token - oidc_op_user_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/userinfo + items: + - identifier: admin-oidc + oidc_rp_client_id: client-id + oidc_rp_client_secret: secret + endpoint_config: + oidc_op_authorization_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/auth + oidc_op_token_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/token + oidc_op_user_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/userinfo diff --git a/tests/setupconfig/files/discovery.yml b/tests/setupconfig/files/discovery.yml index be8b924..0f62ce9 100644 --- a/tests/setupconfig/files/discovery.yml +++ b/tests/setupconfig/files/discovery.yml @@ -1,6 +1,8 @@ oidc_db_config_enable: True oidc_db_config_admin_auth: - oidc_rp_client_id: testid - oidc_rp_client_secret: 7DB3KUAAizYCcmZufpHRVOcD0TOkNO3I - endpoint_config: - oidc_op_discovery_endpoint: http://localhost:8080/realms/test/ + items: + - identifier: admin-oidc + oidc_rp_client_id: testid + oidc_rp_client_secret: 7DB3KUAAizYCcmZufpHRVOcD0TOkNO3I + endpoint_config: + oidc_op_discovery_endpoint: http://localhost:8080/realms/test/ diff --git a/tests/setupconfig/files/full_setup.yml b/tests/setupconfig/files/full_setup.yml index 0a4a63f..dbdd1e6 100644 --- a/tests/setupconfig/files/full_setup.yml +++ b/tests/setupconfig/files/full_setup.yml @@ -1,37 +1,39 @@ oidc_db_config_enable: True oidc_db_config_admin_auth: - enabled: False - oidc_rp_client_id: client-id - oidc_rp_client_secret: secret - oidc_rp_scopes_list: - - open_id - - email - - profile - - extra_scope - oidc_rp_sign_algo: RS256 - oidc_rp_idp_sign_key: key - oidc_op_jwks_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/certs - endpoint_config: - oidc_op_authorization_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/auth - oidc_op_token_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/token - oidc_op_user_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/userinfo - username_claim: - - claim_name - groups_claim: - - groups_claim_name - claim_mapping: - first_name: - - given_name - sync_groups: false - sync_groups_glob_pattern: local.groups.* - default_groups: - - local.groups.Admins - - local.groups.Read-only - make_users_staff: true - superuser_group_names: - - superuser - oidc_use_nonce: false - oidc_nonce_size: 48 - oidc_state_size: 48 - userinfo_claims_source: id_token + items: + - identifier: admin-oidc + enabled: False + oidc_rp_client_id: client-id + oidc_rp_client_secret: secret + oidc_rp_scopes_list: + - open_id + - email + - profile + - extra_scope + oidc_rp_sign_algo: RS256 + oidc_rp_idp_sign_key: key + oidc_op_jwks_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/certs + endpoint_config: + oidc_op_authorization_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/auth + oidc_op_token_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/token + oidc_op_user_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/userinfo + username_claim: + - claim_name + groups_claim: + - groups_claim_name + claim_mapping: + first_name: + - given_name + sync_groups: false + sync_groups_glob_pattern: local.groups.* + default_groups: + - local.groups.Admins + - local.groups.Read-only + make_users_staff: true + superuser_group_names: + - superuser + oidc_use_nonce: false + oidc_nonce_size: 48 + oidc_state_size: 48 + userinfo_claims_source: id_token diff --git a/tests/setupconfig/files/missing_identifier.yml b/tests/setupconfig/files/missing_identifier.yml new file mode 100644 index 0000000..c3c51f3 --- /dev/null +++ b/tests/setupconfig/files/missing_identifier.yml @@ -0,0 +1,9 @@ +oidc_db_config_enable: True +oidc_db_config_admin_auth: + items: + - oidc_rp_client_id: client-id + oidc_rp_client_secret: secret + endpoint_config: + oidc_op_authorization_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/auth + oidc_op_token_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/token + oidc_op_user_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/userinfo diff --git a/tests/setupconfig/files/multiple_configs.yml b/tests/setupconfig/files/multiple_configs.yml new file mode 100644 index 0000000..a3b0a09 --- /dev/null +++ b/tests/setupconfig/files/multiple_configs.yml @@ -0,0 +1,17 @@ +oidc_db_config_enable: True +oidc_db_config_admin_auth: + items: + - identifier: admin-oidc + oidc_rp_client_id: client-id + oidc_rp_client_secret: secret + endpoint_config: + oidc_op_authorization_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/auth + oidc_op_token_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/token + oidc_op_user_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/userinfo + - identifier: admin-oidc + oidc_rp_client_id: client-id2 + oidc_rp_client_secret: secret2 + endpoint_config: + oidc_op_authorization_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/auth + oidc_op_token_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/token + oidc_op_user_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/userinfo diff --git a/tests/setupconfig/files/no_sync_groups.yml b/tests/setupconfig/files/no_sync_groups.yml index 590e699..e67f658 100644 --- a/tests/setupconfig/files/no_sync_groups.yml +++ b/tests/setupconfig/files/no_sync_groups.yml @@ -1,12 +1,14 @@ oidc_db_config_enable: True oidc_db_config_admin_auth: - oidc_rp_client_id: client-id - oidc_rp_client_secret: secret - endpoint_config: - oidc_op_authorization_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/auth - oidc_op_token_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/token - oidc_op_user_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/userinfo - sync_groups: false - default_groups: - - SuperAdmins - - NormalUsers + items: + - identifier: admin-oidc + oidc_rp_client_id: client-id + oidc_rp_client_secret: secret + endpoint_config: + oidc_op_authorization_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/auth + oidc_op_token_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/token + oidc_op_user_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/userinfo + sync_groups: false + default_groups: + - SuperAdmins + - NormalUsers diff --git a/tests/setupconfig/files/sync_groups.yml b/tests/setupconfig/files/sync_groups.yml index 291eb6a..66f9f1a 100644 --- a/tests/setupconfig/files/sync_groups.yml +++ b/tests/setupconfig/files/sync_groups.yml @@ -1,15 +1,17 @@ oidc_db_config_enable: True oidc_db_config_admin_auth: - oidc_rp_client_id: client-id - oidc_rp_client_secret: secret - endpoint_config: - oidc_op_authorization_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/auth - oidc_op_token_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/token - oidc_op_user_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/userinfo - sync_groups: true - sync_groups_glob_pattern: local.groups.* - default_groups: - - local.groups.SuperAdmins - - local.WeirdAdmins - - local.groups.NormalUsers - - local.WeirdUsers + items: + - identifier: admin-oidc + oidc_rp_client_id: client-id + oidc_rp_client_secret: secret + endpoint_config: + oidc_op_authorization_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/auth + oidc_op_token_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/token + oidc_op_user_endpoint: http://localhost:8080/realms/test/protocol/openid-connect/userinfo + sync_groups: true + sync_groups_glob_pattern: local.groups.* + default_groups: + - local.groups.SuperAdmins + - local.WeirdAdmins + - local.groups.NormalUsers + - local.WeirdUsers diff --git a/tests/setupconfig/test_steps.py b/tests/setupconfig/test_steps.py index 9943c1f..538a78e 100644 --- a/tests/setupconfig/test_steps.py +++ b/tests/setupconfig/test_steps.py @@ -2,7 +2,10 @@ import pytest import requests -from django_setup_configuration.exceptions import ConfigurationRunFailed +from django_setup_configuration.exceptions import ( + ConfigurationRunFailed, + PrerequisiteFailed, +) from django_setup_configuration.test_utils import execute_single_step from mozilla_django_oidc_db.models import ( @@ -244,6 +247,30 @@ def test_configure_discovery_failure( assert config.oidc_op_discovery_endpoint == "" +@pytest.mark.django_db +def test_configure_fails_with_multiple_configs(multiple_configs_yml): + with pytest.raises(ConfigurationRunFailed) as excinfo: + execute_single_step( + AdminOIDCConfigurationStep, yaml_source=multiple_configs_yml + ) + assert str(excinfo.value) == "You must specify exactly one OIDC configuration" + + config = OpenIDConnectConfig.get_solo() + assert not config.enabled + + +@pytest.mark.django_db +def test_configure_fails_without_identifier(missing_identifier_yml): + with pytest.raises(PrerequisiteFailed) as excinfo: + execute_single_step( + AdminOIDCConfigurationStep, yaml_source=missing_identifier_yml + ) + assert "oidc_db_config_admin_auth.items.0.identifier" in str(excinfo.value) + + config = OpenIDConnectConfig.get_solo() + assert not config.enabled + + @pytest.mark.django_db def test_sync_groups_is_false(no_sync_groups_config_yml): # create groups so they can be found @@ -253,8 +280,10 @@ def test_sync_groups_is_false(no_sync_groups_config_yml): AdminOIDCConfigurationStep, yaml_source=no_sync_groups_config_yml ) - assert not result.config_model.sync_groups - assert result.config_model.default_groups == ["SuperAdmins", "NormalUsers"] + config_model = result.config_model.items[0] + + assert not config_model.sync_groups + assert config_model.default_groups == ["SuperAdmins", "NormalUsers"] config = OpenIDConnectConfig.get_solo() assert config.default_groups.all().count() == 1 @@ -271,14 +300,16 @@ def test_sync_groups_is_true(sync_groups_config_yml): AdminOIDCConfigurationStep, yaml_source=sync_groups_config_yml ) - assert result.config_model.sync_groups - assert result.config_model.default_groups == [ + config_model = result.config_model.items[0] + + assert config_model.sync_groups + assert config_model.default_groups == [ "local.groups.SuperAdmins", "local.WeirdAdmins", "local.groups.NormalUsers", "local.WeirdUsers", ] - assert result.config_model.sync_groups_glob_pattern == "local.groups.*" + assert config_model.sync_groups_glob_pattern == "local.groups.*" config = OpenIDConnectConfig.get_solo() assert config.default_groups.all().count() == 3 From c3a65775e08095a6058867f66ac62cfb802ffb29 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Mon, 9 Dec 2024 16:03:37 +0100 Subject: [PATCH 4/4] :memo: [#125] Update setup config format in docs --- docs/setup_configuration.rst | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/setup_configuration.rst b/docs/setup_configuration.rst index be63a14..1bfdecb 100644 --- a/docs/setup_configuration.rst +++ b/docs/setup_configuration.rst @@ -46,10 +46,12 @@ Example: *setup_config.yml* ... oidc_db_config_enable: True oidc_db_config_admin_auth: - oidc_rp_client_id: client-id - oidc_rp_client_secret: secret - endpoint_config: - oidc_op_discovery_endpoint: https://keycloak.local/protocol/openid-connect/ + items: + - identifier: admin-oidc + oidc_rp_client_id: client-id + oidc_rp_client_secret: secret + endpoint_config: + oidc_op_discovery_endpoint: https://keycloak.local/protocol/openid-connect/ ... This is file is then used with the setup configuration command setup the OIDC admin: @@ -65,6 +67,7 @@ Required Fields: """""""""""""""" +* ``identifier``: a unique identifier for this configuration. * ``oidc_rp_client_id``: OpenID Connect client ID from the OIDC Provider. * ``oidc_rp_client_secret``: OpenID Connect secret from the OIDC Provider. * ``endpoint_config``: Dictionary containing endpoint information