From 516a09e53b2076728e7eac448cb8c11db3df394b Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Tue, 5 Mar 2024 15:58:15 +0900 Subject: [PATCH] federation: Validate values by data types Change-Id: I9192ea2c6edb6344b6b25a2012b246cfc67b5557 --- manifests/federation/mellon.pp | 9 +--- manifests/federation/openidc.pp | 79 ++++++++++++------------------ manifests/federation/shibboleth.pp | 11 ++--- templates/openidc.conf.erb | 2 +- 4 files changed, 37 insertions(+), 64 deletions(-) diff --git a/manifests/federation/mellon.pp b/manifests/federation/mellon.pp index 036c528a7..83cb6d3de 100644 --- a/manifests/federation/mellon.pp +++ b/manifests/federation/mellon.pp @@ -35,8 +35,8 @@ class keystone::federation::mellon ( $methods, $idp_name, $protocol_name, - $template_order = 331, - Boolean $enable_websso = false, + Integer[330, 999] $template_order = 331, + Boolean $enable_websso = false, ) { include apache @@ -48,11 +48,6 @@ class keystone::federation::mellon ( fail('The keystone::wsgi::apache class should be included in the catalog') } - # Note: if puppet-apache modify these values, this needs to be updated - if $template_order <= 330 or $template_order >= 999 { - fail('The template order should be greater than 330 and less than 999.') - } - if ('external' in $methods ) { fail("The external method should be dropped to avoid any interference with some \ Apache + Mellon SP setups, where a REMOTE_USER env variable is always set, even as an empty value.") diff --git a/manifests/federation/openidc.pp b/manifests/federation/openidc.pp index 3fdf663c9..7ff8d9210 100644 --- a/manifests/federation/openidc.pp +++ b/manifests/federation/openidc.pp @@ -135,26 +135,26 @@ class keystone::federation::openidc ( $openidc_provider_metadata_url, $openidc_client_id, $openidc_client_secret, - $openidc_crypto_passphrase = 'openstack', - $openidc_response_type = 'id_token', - $openidc_response_mode = undef, - $openidc_cache_type = undef, - $openidc_cache_shm_max = undef, - $openidc_cache_shm_entry_size = undef, - $openidc_cache_dir = undef, - $openidc_cache_clean_interval = undef, - $openidc_claim_delimiter = undef, - Boolean $openidc_enable_oauth = false, - $openidc_introspection_endpoint = undef, - $openidc_verify_jwks_uri = undef, - $openidc_verify_method = 'introspection', - $openidc_pass_userinfo_as = undef, - $openidc_pass_claim_as = undef, - $memcached_servers = undef, - $redis_server = undef, - $redis_password = undef, - $remote_id_attribute = $facts['os_service_default'], - $template_order = 331, + $openidc_crypto_passphrase = 'openstack', + $openidc_response_type = 'id_token', + $openidc_response_mode = undef, + $openidc_cache_type = undef, + $openidc_cache_shm_max = undef, + $openidc_cache_shm_entry_size = undef, + $openidc_cache_dir = undef, + $openidc_cache_clean_interval = undef, + $openidc_claim_delimiter = undef, + Boolean $openidc_enable_oauth = false, + $openidc_introspection_endpoint = undef, + $openidc_verify_jwks_uri = undef, + Enum['introspection', 'jwks'] $openidc_verify_method = 'introspection', + Optional[Enum['claims', 'json', 'jwt']] $openidc_pass_userinfo_as = undef, + Optional[Enum['none', 'environment', 'headers', 'both']] $openidc_pass_claim_as = undef, + $memcached_servers = undef, + $redis_server = undef, + $redis_password = undef, + $remote_id_attribute = $facts['os_service_default'], + Integer[330, 999] $template_order = 331, ) { include apache @@ -167,32 +167,20 @@ class keystone::federation::openidc ( fail('The keystone::wsgi::apache class should be included in the catalog') } - if !($openidc_verify_method in ['introspection', 'jwks']) { - fail('Unsupported token verification method.' + - ' Must be one of "introspection" or "jwks"') - } - - if ($openidc_verify_method == 'introspection') { - if $openidc_enable_oauth and !$openidc_introspection_endpoint { - fail('You must set openidc_introspection_endpoint when enabling oauth support' + + case $openidc_verify_method { + 'introspection': { + if $openidc_enable_oauth and !$openidc_introspection_endpoint { + fail( + 'You must set openidc_introspection_endpoint when enabling oauth support' + ' and introspection.') + } } - } elsif ($openidc_verify_method == 'jwks') { - if $openidc_enable_oauth and !$openidc_verify_jwks_uri { - fail('You must set openidc_verify_jwks_uri when enabling oauth support' + + default: { # jwks + if $openidc_enable_oauth and !$openidc_verify_jwks_uri { + fail( + 'You must set openidc_verify_jwks_uri when enabling oauth support' + ' and local signature verification using a JWKS URL') - } - } - - if $openidc_pass_userinfo_as != undef { - if !($openidc_pass_userinfo_as in ['claims', 'json', 'jwt']) { - fail('Unsupported OIDCPassUserInfoAs. Must be one of: claims, json or jwt') - } - } - - if $openidc_pass_claim_as != undef { - if !($openidc_pass_claim_as in ['none', 'environment', 'headers', 'both']) { - fail('Unsupported OIDCPassClaimAs. Must be one of: none, environment, headers, both') + } } } @@ -202,11 +190,6 @@ class keystone::federation::openidc ( $memcached_servers_real = undef } - # Note: if puppet-apache modify these values, this needs to be updated - if $template_order <= 330 or $template_order >= 999 { - fail('The template order should be greater than 330 and less than 999.') - } - if ('external' in $methods ) { fail('The external method should be dropped to avoid any interference with openid.') } diff --git a/manifests/federation/shibboleth.pp b/manifests/federation/shibboleth.pp index 6a0b3cdce..8e81da419 100644 --- a/manifests/federation/shibboleth.pp +++ b/manifests/federation/shibboleth.pp @@ -45,9 +45,9 @@ # class keystone::federation::shibboleth ( $methods, - Boolean $suppress_warning = false, - $template_order = 331, - $yum_repo_name = 'shibboleth', + Boolean $suppress_warning = false, + Integer[330, 999] $template_order = 331, + String[1] $yum_repo_name = 'shibboleth', ) { include apache @@ -57,11 +57,6 @@ class keystone::federation::shibboleth ( fail('The keystone::wsgi::apache class should be included in the catalog') } - # Note: if puppet-apache modify these values, this needs to be updated - if $template_order <= 330 or $template_order >= 999 { - fail('The template order should be greater than 330 and less than 999.') - } - if ('external' in $methods ) { fail("The external method should be dropped to avoid any interference with some \ Apache + Shibboleth SP setups, where a REMOTE_USER env variable is always set, even as an empty value.") diff --git a/templates/openidc.conf.erb b/templates/openidc.conf.erb index 1d3e11781..67f17bc0d 100644 --- a/templates/openidc.conf.erb +++ b/templates/openidc.conf.erb @@ -50,7 +50,7 @@ OIDCOAuthClientID "<%= scope['keystone::federation::openidc::openidc_client_id']-%>" OIDCOAuthClientSecret "<%= scope['keystone::federation::openidc::openidc_client_secret']-%>" OIDCOAuthIntrospectionEndpoint "<%= scope['keystone::federation::openidc::openidc_introspection_endpoint']-%>" - <%- elsif scope['keystone::federation::openidc::openidc_verify_method'] == 'jwks' -%> + <%- else -%> OIDCOAuthVerifyJwksUri "<%= scope['keystone::federation::openidc::openidc_verify_jwks_uri']-%>" <%- end -%>