From 52052e4df892c6ffa3b04a36d51c7232f47de929 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Thu, 5 Dec 2019 18:51:17 +0100 Subject: [PATCH] Fixes #28436 - Add keycloak support This adds support for keycloak when using the keycloak-httpd-client-install command. A more native integration would rely on the oidc support in puppetlabs-apache, but that's unreleased. It drops the dependency chaining for the passenger package because it was leading to dependency cycles and I don't see a reason for the explicit chaining. --- manifests/config.pp | 3 +++ manifests/config/apache.pp | 28 ++++++++++++++++++++++ manifests/init.pp | 18 ++++++++++++++ manifests/install.pp | 5 ++-- manifests/params.pp | 4 ++++ manifests/settings.pp | 1 + spec/classes/foreman_config_apache_spec.rb | 9 +++++++ spec/classes/foreman_spec.rb | 12 +++++++++- 8 files changed, 76 insertions(+), 4 deletions(-) diff --git a/manifests/config.pp b/manifests/config.pp index 29ce08ff6..5066a46a7 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -119,6 +119,9 @@ passenger_start_timeout => $::foreman::passenger_start_timeout, foreman_url => $::foreman::foreman_url, ipa_authentication => $::foreman::ipa_authentication, + keycloak => $::foreman::keycloak, + keycloak_app_name => $::foreman::keycloak_app_name, + keycloak_realm => $::foreman::keycloak_realm, } contain foreman::config::apache diff --git a/manifests/config/apache.pp b/manifests/config/apache.pp index 6dba8b19b..45c0edc3e 100644 --- a/manifests/config/apache.pp +++ b/manifests/config/apache.pp @@ -119,6 +119,9 @@ Boolean $ipa_authentication = false, Hash[String, Any] $http_vhost_options = {}, Hash[String, Any] $https_vhost_options = {}, + Boolean $keycloak = false, + String[1] $keycloak_app_name = 'foreman-openidc', + String[1] $keycloak_realm = 'ssl-realm', ) { $docroot = "${app_root}/public" @@ -226,6 +229,31 @@ include ::apache::mod::intercept_form_submit include ::apache::mod::lookup_identity include ::apache::mod::auth_kerb + } elsif $keycloak { + # TODO: https://github.com/puppetlabs/puppetlabs-apache/commit/9f7f38ff21036c9a1ce4d669ccaea816941209ca + # adds apache::mod::auth_openidc which allows for proper integration but + # the current release (5.4.0) doesn't include this yet. + include ::apache::mod::authz_user + apache::mod { 'auth_openidc': + package => 'mod_auth_openidc', + } + + # This file is generated by keycloak-httpd-client-install and that manages + # the content. The command would be: + # + # keycloak-httpd-client-install --app-name ${keycloak_app_name} --keycloak-server-url $KEYCLOAK_URL --keycloak-admin-username $KEYCLOAK_USER --keycloak-realm ${keycloak_realm} --keycloak-admin-realm master --keycloak-auth-role root-admin --client-type openidc --client-hostname ${servername} --protected-locations /users/extlogin + # + # If $suburi is used, --location-root should also be passed in + # + # By defining it here we avoid purging it and also tighten the + # permissions so the world can't read its secrets. + # This is functionally equivalent to apache::custom_config without content/source + file { "${apache::confd_dir}/${keycloak_app_name}_oidc_keycloak_${keycloak_realm}.conf": + ensure => file, + owner => 'root', + group => 'root', + mode => '0640', + } } file { "${apache::confd_dir}/${priority}-foreman.d": diff --git a/manifests/init.pp b/manifests/init.pp index 81e95106e..ff19f2734 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -205,6 +205,16 @@ # # $foreman_service_puma_workers:: Number of workers for Puma. Relevant only when Puma service is used and ignored when Passenger is used. # +# === Keycloak parameters: +# +# $keycloak:: Enable Keycloak support. Note this is limited +# to configuring Apache and still relies on manually +# running keycloak-httpd-client-install +# +# $keycloak_app_name:: The app name as passed to keycloak-httpd-client-install +# +# $keycloak_realm:: The realm as passed to keycloak-httpd-client-install +# class foreman ( Stdlib::HTTPUrl $foreman_url = $::foreman::params::foreman_url, Boolean $unattended = $::foreman::params::unattended, @@ -304,6 +314,9 @@ Integer[0] $foreman_service_puma_threads_min = $::foreman::params::foreman_service_puma_threads_min, Integer[0] $foreman_service_puma_threads_max = $::foreman::params::foreman_service_puma_threads_max, Integer[0] $foreman_service_puma_workers = $::foreman::params::foreman_service_puma_workers, + Boolean $keycloak = $::foreman::params::keycloak, + String[1] $keycloak_app_name = $::foreman::params::keycloak_app_name, + String[1] $keycloak_realm = $::foreman::params::keycloak_realm, ) inherits foreman::params { if $db_sslmode == 'UNSET' and $db_root_cert { $db_sslmode_real = 'verify-full' @@ -337,8 +350,13 @@ if $apache { Class['foreman::database'] -> Class['apache::service'] + if $ipa_authentication and $keycloak { + fail("${::hostname}: External authentication via IPA and Keycloak are mutually exclusive.") + } } elsif $ipa_authentication { fail("${::hostname}: External authentication via IPA can only be enabled when Apache is used.") + } elsif $keycloak { + fail("${::hostname}: External authentication via Keycloak can only be enabled when Apache is used.") } # Anchor these separately so as not to break diff --git a/manifests/install.pp b/manifests/install.pp index 57aa15ef8..ac3b92a20 100644 --- a/manifests/install.pp +++ b/manifests/install.pp @@ -13,9 +13,8 @@ if $::foreman::apache and $::foreman::passenger and $::foreman::passenger_ruby_package { package { $::foreman::passenger_ruby_package: - ensure => installed, - require => Class['apache'], - before => Class['apache::service'], + ensure => installed, + before => Class['apache::service'], } } diff --git a/manifests/params.pp b/manifests/params.pp index 7843a833b..da96bee7a 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -93,6 +93,10 @@ $jobs_service_enable = true $jobs_sidekiq_redis_url = undef + # Keycloak + $keycloak = false + $keycloak_app_name = 'foreman-openidc' + $keycloak_realm = 'ssl-realm' $hsts_enabled = true diff --git a/manifests/settings.pp b/manifests/settings.pp index b7ab8980e..9991875ae 100644 --- a/manifests/settings.pp +++ b/manifests/settings.pp @@ -7,6 +7,7 @@ $email_smtp_authentication = $::foreman::email_smtp_authentication, $email_smtp_user_name = $::foreman::email_smtp_user_name, $email_smtp_password = $::foreman::email_smtp_password, + $keycloak = $::foreman::keycloak, ) { unless empty($email_delivery_method) { foreman_config_entry { 'delivery_method': diff --git a/spec/classes/foreman_config_apache_spec.rb b/spec/classes/foreman_config_apache_spec.rb index ae098eb44..d3128a3b7 100644 --- a/spec/classes/foreman_config_apache_spec.rb +++ b/spec/classes/foreman_config_apache_spec.rb @@ -59,6 +59,15 @@ should contain_file('/usr/share/foreman/config/environment.rb').with_owner('foreman') end end + + describe 'with keycloak' do + let(:params) { super().merge(keycloak: true) } + + it { should compile.with_all_deps } + it { should contain_apache__mod('auth_openidc') } + it { should contain_class('apache::mod::authz_user') } + it { should contain_file("#{http_dir}/conf.d/foreman-openidc_oidc_keycloak_ssl-realm.conf") } + end end describe 'with ssl' do diff --git a/spec/classes/foreman_spec.rb b/spec/classes/foreman_spec.rb index 9e0ce2021..02cb0629e 100644 --- a/spec/classes/foreman_spec.rb +++ b/spec/classes/foreman_spec.rb @@ -108,6 +108,7 @@ should contain_class('foreman::config::apache') .with_passenger_ruby(passenger_ruby) + .with_keycloak(false) end it { should contain_apache__vhost('foreman').without_custom_fragment(/Alias/) } @@ -262,11 +263,20 @@ email_smtp_domain: 'example.com', email_smtp_authentication: 'none', email_smtp_user_name: 'root', - email_smtp_password: 'secret' + email_smtp_password: 'secret', + keycloak: true, + keycloak_app_name: 'cloak-app', + keycloak_realm: 'myrealm', } end it { is_expected.to compile.with_all_deps } + it do + is_expected.to contain_class('foreman::config::apache') + .with_keycloak(true) + .with_keycloak_app_name('cloak-app') + .with_keycloak_realm('myrealm') + end end context 'with journald logging' do