From f07fb7d2799881fe422e6cf1c9c4811047c1239b Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Wed, 9 Feb 2022 21:06:28 -0500 Subject: [PATCH] LDAP support: include BC support for the YAML based loading Includes a refactoring of the configuration loader. --- doc/manual/src/configuration.md | 49 ++++- flake.nix | 1 + src/lib/Hydra.pm | 4 +- src/lib/Hydra/Config.pm | 158 ++++++++++++++ src/lib/Hydra/Controller/User.pm | 12 +- src/lib/Hydra/Helper/Nix.pm | 19 -- src/script/hydra-eval-jobset | 1 + src/script/hydra-notify | 1 + src/script/hydra-s3-backup-collect-garbage | 1 + src/script/hydra-send-stats | 1 + src/script/hydra-update-gc-roots | 1 + t/Hydra/Config/hydra-notify.t | 3 +- t/Hydra/Config/include.t | 7 +- t/Hydra/Config/ldap_role_map.t | 242 +++++++++++++++++++++ t/Hydra/Config/statsd.t | 3 +- t/Hydra/Controller/User/ldap.t | 13 +- 16 files changed, 475 insertions(+), 41 deletions(-) create mode 100644 t/Hydra/Config/ldap_role_map.t diff --git a/doc/manual/src/configuration.md b/doc/manual/src/configuration.md index fe1a8402..1a4db163 100644 --- a/doc/manual/src/configuration.md +++ b/doc/manual/src/configuration.md @@ -105,11 +105,11 @@ in the hydra configuration file, as below: Using LDAP as authentication backend (optional) ----------------------------------------------- -Instead of using Hydra\'s built-in user management you can optionally +Instead of using Hydra's built-in user management you can optionally use LDAP to manage roles and users. This is configured by defining the `` block in the configuration file. -In this block it\'s possible to configure the authentication plugin in the +In this block it's possible to configure the authentication plugin in the `` block, all options are directly passed to `Catalyst::Authentication ::Store::LDAP`. The documentation for the available settings can be found [here] (https://metacpan.org/pod/Catalyst::Authentication::Store::LDAP#CONFIGURATION-OPTIONS). @@ -135,7 +135,6 @@ Example configuration: ldap_server = localhost timeout = 30 - debug = 2 binddn = "cn=root,dc=example" bindpw = notapassword @@ -164,14 +163,52 @@ Example configuration: # Make all users in the hydra_admin group Hydra admins hydra_admin = admin - # Allow all users in the dev group to restart jobs + # Allow all users in the dev group to restart jobs and cancel builds dev = restart-jobs + dev = cancel-builds ``` -This example configuration also enables the (very verbose) LDAP debug logging -by setting `config.ldap_server_options.debug`. +### Debugging LDAP + +Set the `debug` parameter under `ldap.config.ldap_server_options.debug`: + +``` + + + + + debug = 2 + + + + +``` + +### Legacy LDAP Configuration + +Hydra used to load the LDAP configuration from a YAML file in the +`HYDRA_LDAP_CONFIG` environment variable. This behavior is deperecated +and will be removed. + +When Hydra uses the deprecated YAML file, Hydra applies the following +default role mapping: + +``` + + + hydra_admin = admin + hydra_bump-to-front = bump-to-front + hydra_cancel-build = cancel-build + hydra_create-projects = create-projects + hydra_restart-jobs = restart-jobs + + +``` + +Note that configuring both the LDAP parameters in the hydra.conf and via +the environment variable is a fatal error. Embedding Extra HTML -------------------- diff --git a/flake.nix b/flake.nix index 736cbd53..56cb2960 100644 --- a/flake.nix +++ b/flake.nix @@ -521,6 +521,7 @@ TextDiff TextTable UUID4Tiny + YAML XMLSimple ]; }; diff --git a/src/lib/Hydra.pm b/src/lib/Hydra.pm index 47ada081..910212ce 100644 --- a/src/lib/Hydra.pm +++ b/src/lib/Hydra.pm @@ -6,7 +6,7 @@ use parent 'Catalyst'; use Moose; use Hydra::Plugin; use Hydra::Model::DB; -use Hydra::Helper::Nix qw(getHydraConfig); +use Hydra::Config qw(getLDAPConfigAmbient); use Catalyst::Runtime '5.70'; use Catalyst qw/ConfigLoader Static::Simple @@ -43,7 +43,7 @@ __PACKAGE__->config( role_field => "role", }, }, - ldap => Hydra::Helper::Nix::getHydraConfig->{'ldap'}->{'config'} + ldap => getLDAPConfigAmbient()->{'config'} }, 'Plugin::ConfigLoader' => { driver => { diff --git a/src/lib/Hydra/Config.pm b/src/lib/Hydra/Config.pm index bb991822..d96292fe 100644 --- a/src/lib/Hydra/Config.pm +++ b/src/lib/Hydra/Config.pm @@ -2,7 +2,165 @@ package Hydra::Config; use strict; use warnings; +use Config::General; +use List::SomeUtils qw(none); +use YAML qw(LoadFile); + +our @ISA = qw(Exporter); +our @EXPORT = qw( + getHydraConfig + getLDAPConfig + getLDAPConfigAmbient +); our %configGeneralOpts = (-UseApacheInclude => 1, -IncludeAgain => 1, -IncludeRelative => 1); +my $hydraConfigCache; + +sub getHydraConfig { + return $hydraConfigCache if defined $hydraConfigCache; + + my $conf; + + if ($ENV{"HYDRA_CONFIG"}) { + $conf = $ENV{"HYDRA_CONFIG"}; + } else { + require Hydra::Model::DB; + $conf = Hydra::Model::DB::getHydraPath() . "/hydra.conf" + }; + + if (-f $conf) { + $hydraConfigCache = loadConfig($conf); + } else { + $hydraConfigCache = {}; + } + + return $hydraConfigCache; +} + +sub loadConfig { + my ($sourceFile) = @_; + + my %opts = (%configGeneralOpts, -ConfigFile => $sourceFile); + + return { Config::General->new(%opts)->getall }; +} + +sub is_ldap_in_legacy_mode { + my ($config, %env) = @_; + + my $legacy_defined = defined $env{"HYDRA_LDAP_CONFIG"}; + + if (defined $config->{"ldap"}) { + if ($legacy_defined) { + die "The legacy environment variable HYDRA_LDAP_CONFIG is set, but config is also specified in hydra.conf. Please unset the environment variable."; + } + + return 0; + } elsif ($legacy_defined) { + warn "Hydra is configured to use LDAP via the HYDRA_LDAP_CONFIG, a deprecated method. Please see the docs about configuring LDAP in the hydra.conf."; + return 1; + } else { + return 0; + } +} + +sub getLDAPConfigAmbient { + return getLDAPConfig(getHydraConfig(), %ENV); +} + +sub getLDAPConfig { + my ($config, %env) = @_; + + my $ldap_config; + + if (is_ldap_in_legacy_mode($config, %env)) { + $ldap_config = get_legacy_ldap_config($env{"HYDRA_LDAP_CONFIG"}); + } else { + $ldap_config = $config->{"ldap"}; + } + + $ldap_config->{"role_mapping"} = normalize_ldap_role_mappings($ldap_config->{"role_mapping"}); + + return $ldap_config; +} + +sub get_legacy_ldap_config { + my ($ldap_yaml_file) = @_; + + return { + config => LoadFile($ldap_yaml_file), + role_mapping => { + "hydra_admin" => [ "admin" ], + "hydra_bump-to-front" => [ "bump-to-front" ], + "hydra_cancel-build" => [ "cancel-build" ], + "hydra_create-projects" => [ "create-projects" ], + "hydra_restart-jobs" => [ "restart-jobs" ], + }, + }; +} + +sub normalize_ldap_role_mappings { + my ($input_map) = @_; + + my $mapping = {}; + + my @errors; + + for my $group (keys %{$input_map}) { + my $input = $input_map->{$group}; + + if (ref $input eq "ARRAY") { + $mapping->{$group} = $input; + } elsif (ref $input eq "") { + $mapping->{$group} = [ $input ]; + } else { + push @errors, "On group '$group': the value is of type ${\ref $input}. Only strings and lists are acceptable."; + $mapping->{$group} = [ ]; + } + + eval { + validate_roles($mapping->{$group}); + }; + if ($@) { + push @errors, "On group '$group': $@"; + } + } + + if (@errors) { + die join "\n", @errors; + } + + return $mapping; +} + +sub validate_roles { + my ($roles) = @_; + + my @invalid; + my $valid = valid_roles(); + + for my $role (@$roles) { + if (none { $_ eq $role } @$valid) { + push @invalid, "'$role'"; + } + } + + if (@invalid) { + die "Invalid roles: ${\join ', ', @invalid}. Valid roles are: ${\join ', ', @$valid}."; + } + + return 1; +} + +sub valid_roles { + return [ + "admin", + "bump-to-front", + "cancel-build", + "create-projects", + "restart-jobs", + ]; +} + 1; diff --git a/src/lib/Hydra/Controller/User.pm b/src/lib/Hydra/Controller/User.pm index 08b2c91b..2a8affae 100644 --- a/src/lib/Hydra/Controller/User.pm +++ b/src/lib/Hydra/Controller/User.pm @@ -7,6 +7,7 @@ use base 'Hydra::Base::Controller::REST'; use File::Slurper qw(read_text); use Crypt::RandPasswd; use Digest::SHA1 qw(sha1_hex); +use Hydra::Config qw(getLDAPConfigAmbient); use Hydra::Helper::Nix; use Hydra::Helper::CatalystUtils; use Hydra::Helper::Email; @@ -56,12 +57,10 @@ sub logout_POST { sub doLDAPLogin { my ($self, $c, $username) = @_; - my $user = $c->find_user({ username => $username }); my $LDAPUser = $c->find_user({ username => $username }, 'ldap'); my @LDAPRoles = $LDAPUser->roles; - my %ldap_config = %{Hydra::Helper::Nix::getHydraConfig->{'ldap'}}; - my %role_mapping = $ldap_config{'role_mapping'} ? %{$ldap_config{'role_mapping'}} : (); + my $role_mapping = getLDAPConfigAmbient()->{"role_mapping"}; if (!$user) { $c->model('DB::Users')->create( @@ -82,8 +81,11 @@ sub doLDAPLogin { } $user->userroles->delete; foreach my $ldap_role (@LDAPRoles) { - if (%role_mapping{$ldap_role}) { - $user->userroles->create({ role => $role_mapping{$ldap_role} }); + if (defined($role_mapping->{$ldap_role})) { + my $roles = $role_mapping->{$ldap_role}; + for my $mapped_role (@$roles) { + $user->userroles->create({ role => $mapped_role }); + } } } $c->set_authenticated($user); diff --git a/src/lib/Hydra/Helper/Nix.pm b/src/lib/Hydra/Helper/Nix.pm index 796d9844..514fb439 100644 --- a/src/lib/Hydra/Helper/Nix.pm +++ b/src/lib/Hydra/Helper/Nix.pm @@ -5,7 +5,6 @@ use warnings; use Exporter; use File::Path; use File::Basename; -use Config::General; use Hydra::Config; use Hydra::Helper::CatalystUtils; use Hydra::Model::DB; @@ -49,24 +48,6 @@ sub getHydraHome { return $dir; } - -my $hydraConfig; - -sub getHydraConfig { - return $hydraConfig if defined $hydraConfig; - my $conf = $ENV{"HYDRA_CONFIG"} || (Hydra::Model::DB::getHydraPath . "/hydra.conf"); - my %opts = (%Hydra::Config::configGeneralOpts, -ConfigFile => $conf); - if (-f $conf) { - my %h = Config::General->new(%opts)->getall; - - $hydraConfig = \%h; - } else { - $hydraConfig = {}; - } - return $hydraConfig; -} - - # Return hash of statsd configuration of the following shape: # ( # host => string, diff --git a/src/script/hydra-eval-jobset b/src/script/hydra-eval-jobset index 450c5f50..108c59c8 100755 --- a/src/script/hydra-eval-jobset +++ b/src/script/hydra-eval-jobset @@ -8,6 +8,7 @@ use Data::Dump qw(dump); use Digest::SHA qw(sha256_hex); use Encode; use File::Slurper qw(read_text); +use Hydra::Config; use Hydra::Helper::AddBuilds; use Hydra::Helper::CatalystUtils; use Hydra::Helper::Email; diff --git a/src/script/hydra-notify b/src/script/hydra-notify index 3b8ffe6d..1e666bf7 100755 --- a/src/script/hydra-notify +++ b/src/script/hydra-notify @@ -6,6 +6,7 @@ use utf8; use Getopt::Long; use Time::HiRes qw( gettimeofday tv_interval ); use HTTP::Server::PSGI; +use Hydra::Config; use Hydra::Event; use Hydra::Event::BuildFinished; use Hydra::Helper::AddBuilds; diff --git a/src/script/hydra-s3-backup-collect-garbage b/src/script/hydra-s3-backup-collect-garbage index 23b50c2f..bd8da2db 100755 --- a/src/script/hydra-s3-backup-collect-garbage +++ b/src/script/hydra-s3-backup-collect-garbage @@ -9,6 +9,7 @@ use Net::Amazon::S3; use Net::Amazon::S3::Client; use Nix::Config; use Nix::Store; +use Hydra::Config; use Hydra::Model::DB; use Hydra::Helper::Nix; diff --git a/src/script/hydra-send-stats b/src/script/hydra-send-stats index 596c622a..d8286c01 100755 --- a/src/script/hydra-send-stats +++ b/src/script/hydra-send-stats @@ -3,6 +3,7 @@ use strict; use warnings; use utf8; +use Hydra::Config; use Hydra::Helper::Nix; use Net::Statsd; use File::Slurper qw(read_text); diff --git a/src/script/hydra-update-gc-roots b/src/script/hydra-update-gc-roots index 91d6393b..fbb90488 100755 --- a/src/script/hydra-update-gc-roots +++ b/src/script/hydra-update-gc-roots @@ -6,6 +6,7 @@ use File::Path; use File::stat; use File::Basename; use Nix::Store; +use Hydra::Config; use Hydra::Schema; use Hydra::Helper::Nix; use Hydra::Model::DB; diff --git a/t/Hydra/Config/hydra-notify.t b/t/Hydra/Config/hydra-notify.t index fb050324..272f63b6 100644 --- a/t/Hydra/Config/hydra-notify.t +++ b/t/Hydra/Config/hydra-notify.t @@ -1,6 +1,7 @@ use strict; use warnings; use Setup; +use Hydra::Config; my %ctx = test_init(hydra_config => q| @@ -14,7 +15,7 @@ my %ctx = test_init(hydra_config => q| require Hydra::Helper::Nix; use Test2::V0; -is(Hydra::Helper::Nix::getHydraNotifyPrometheusConfig(Hydra::Helper::Nix::getHydraConfig()), { +is(Hydra::Helper::Nix::getHydraNotifyPrometheusConfig(getHydraConfig()), { 'listen_address' => "127.0.0.1", 'port' => 9199 }, "Reading specific configuration from the hydra.conf works"); diff --git a/t/Hydra/Config/include.t b/t/Hydra/Config/include.t index e161902e..fe2dd1ed 100644 --- a/t/Hydra/Config/include.t +++ b/t/Hydra/Config/include.t @@ -1,6 +1,8 @@ use strict; use warnings; use Setup; +use Hydra::Config; +use Test2::V0; my %ctx = test_init( use_external_destination_store => 0, @@ -17,10 +19,7 @@ write_file($ctx{'tmpdir'} . "/bar.conf", q| bar = baz |); -require Hydra::Helper::Nix; -use Test2::V0; - -is(Hydra::Helper::Nix::getHydraConfig(), { +is(getHydraConfig(), { foo => { bar => "baz" } }, "Nested includes work."); diff --git a/t/Hydra/Config/ldap_role_map.t b/t/Hydra/Config/ldap_role_map.t new file mode 100644 index 00000000..e56dd619 --- /dev/null +++ b/t/Hydra/Config/ldap_role_map.t @@ -0,0 +1,242 @@ + +use strict; +use warnings; +use Setup; +use Hydra::Config; +use Test2::V0; + +my $tmpdir = File::Temp->newdir(); +my $cfgfile = "$tmpdir/conf"; +my $scratchCfgFile = "$tmpdir/hydra.scratch.conf"; + +my $ldapInHydraConfFile = "$tmpdir/hydra.empty.conf"; +write_file($ldapInHydraConfFile, < + + + class = Password + + + + hydra_admin = admin + hydra_one_group_many_roles = create-projects + hydra_one_group_many_roles = cancel-build + + +CONF +my $ldapInHydraConf = Hydra::Config::loadConfig($ldapInHydraConfFile); + +my $emptyHydraConfFile = "$tmpdir/hydra.empty.conf"; +write_file($emptyHydraConfFile, ""); +my $emptyHydraConf = Hydra::Config::loadConfig($emptyHydraConfFile); + +my $ldapYamlFile = "$tmpdir/ldap.yaml"; +write_file($ldapYamlFile, < sub { + subtest "No ldap section and an env var gets us legacy data" => sub { + like( + warning { + is( + Hydra::Config::getLDAPConfig( + $emptyHydraConf, + ( HYDRA_LDAP_CONFIG => $ldapYamlFile ) + ), + { + config => { + credential => { + class => "Password", + }, + }, + role_mapping => { + "hydra_admin" => [ "admin" ], + "hydra_bump-to-front" => [ "bump-to-front" ], + "hydra_cancel-build" => [ "cancel-build" ], + "hydra_create-projects" => [ "create-projects" ], + "hydra_restart-jobs" => [ "restart-jobs" ], + } + }, + "The empty file and set env var make legacy mode active." + ); + }, + qr/configured to use LDAP via the HYDRA_LDAP_CONFIG/, + "Having the environment variable set warns." + ); + }; + + subtest "An ldap section and no env var gets us normalized data" => sub { + is( + warns { + is( + Hydra::Config::getLDAPConfig( + $ldapInHydraConf, + () + ), + { + config => { + credential => { + class => "Password", + }, + }, + role_mapping => { + "hydra_admin" => [ "admin" ], + "hydra_one_group_many_roles" => [ "create-projects", "cancel-build" ], + } + }, + "The empty file and set env var make legacy mode active." + ); + }, + 0, + "No warnings are issued for non-legacy LDAP support." + ); + }; +}; + + +subtest "is_ldap_in_legacy_mode" => sub { + subtest "With the environment variable set and an empty hydra.conf" => sub { + like( + warning { + is( + Hydra::Config::is_ldap_in_legacy_mode( + $emptyHydraConf, + ( HYDRA_LDAP_CONFIG => $ldapYamlFile ) + ), + 1, + "The empty file and set env var make legacy mode active." + ); + }, + qr/configured to use LDAP via the HYDRA_LDAP_CONFIG/, + "Having the environment variable set warns." + ); + }; + + subtest "With the environment variable set and LDAP specified in hydra.conf" => sub { + like( + dies { + Hydra::Config::is_ldap_in_legacy_mode( + $ldapInHydraConf, + ( HYDRA_LDAP_CONFIG => $ldapYamlFile ) + ); + }, + qr/HYDRA_LDAP_CONFIG is set, but config is also specified in hydra\.conf/, + "Having the environment variable set dies to avoid misconfiguration." + ); + }; + + subtest "Without the environment variable set and an empty hydra.conf" => sub { + is( + warns { + is( + Hydra::Config::is_ldap_in_legacy_mode( + $emptyHydraConf, + () + ), + 0, + "The empty file and unset env var means non-legacy." + ); + }, + 0, + "We should receive zero warnings." + ); + }; + + subtest "Without the environment variable set and LDAP specified in hydra.conf" => sub { + is( + warns { + is( + Hydra::Config::is_ldap_in_legacy_mode( + $ldapInHydraConf, + () + ), + 0, + "The empty file and unset env var means non-legacy." + ); + }, + 0, + "We should receive zero warnings." + ); + }; +}; + +subtest "get_legacy_ldap_config" => sub { + is( + Hydra::Config::get_legacy_ldap_config($ldapYamlFile), + { + config => { + credential => { + class => "Password", + }, + }, + role_mapping => { + "hydra_admin" => [ "admin" ], + "hydra_bump-to-front" => [ "bump-to-front" ], + "hydra_cancel-build" => [ "cancel-build" ], + "hydra_create-projects" => [ "create-projects" ], + "hydra_restart-jobs" => [ "restart-jobs" ], + } + }, + "Legacy, default role maps are applied." + ); +}; + +subtest "validate_roles" => sub { + ok(Hydra::Config::validate_roles([]), "An empty list is valid"); + ok(Hydra::Config::validate_roles(Hydra::Config::valid_roles()), "All current roles are valid."); + like( + dies { Hydra::Config::validate_roles([""]) }, + qr/Invalid roles: ''./, + "Invalid roles are failing" + ); + like( + dies { Hydra::Config::validate_roles(["foo", "bar"]) }, + qr/Invalid roles: 'foo', 'bar'./, + "All the invalid roles are present in the error" + ); +}; + +subtest "normalize_ldap_role_mappings" => sub { + is( + Hydra::Config::normalize_ldap_role_mappings({}), + {}, + "An empty input map is an empty output map." + ); + + is( + Hydra::Config::normalize_ldap_role_mappings({ + hydra_admin => "admin", + hydra_one_group_many_roles => [ "create-projects", "bump-to-front" ], + }), + { + hydra_admin => [ "admin" ], + hydra_one_group_many_roles => [ "create-projects", "bump-to-front" ], + }, + "Lists and plain strings normalize to lists" + ); + + like( + dies{ + Hydra::Config::normalize_ldap_role_mappings({ + "group" => "invalid-role", + }), + }, + qr/Invalid roles.*invalid-role/, + "Invalid roles fail to normalize." + ); + + + like( + dies{ + Hydra::Config::normalize_ldap_role_mappings({ + "group" => { "nested" => "data" }, + }), + }, + qr/On group 'group':.* Only strings/, + "Invalid nesting fail to normalize." + ); +}; + +done_testing; diff --git a/t/Hydra/Config/statsd.t b/t/Hydra/Config/statsd.t index c50e8d99..ba23a28a 100644 --- a/t/Hydra/Config/statsd.t +++ b/t/Hydra/Config/statsd.t @@ -1,6 +1,7 @@ use strict; use warnings; use Setup; +use Hydra::Config; my %ctx = test_init(hydra_config => q| @@ -12,7 +13,7 @@ my %ctx = test_init(hydra_config => q| require Hydra::Helper::Nix; use Test2::V0; -is(Hydra::Helper::Nix::getStatsdConfig(Hydra::Helper::Nix::getHydraConfig()), { +is(Hydra::Helper::Nix::getStatsdConfig(getHydraConfig()), { 'host' => "foo.bar", 'port' => 18125 }, "Reading specific configuration from the hydra.conf works"); diff --git a/t/Hydra/Controller/User/ldap.t b/t/Hydra/Controller/User/ldap.t index caa3433c..19e7825a 100644 --- a/t/Hydra/Controller/User/ldap.t +++ b/t/Hydra/Controller/User/ldap.t @@ -13,10 +13,12 @@ my $users = { admin => $ldap->add_user("admin_user"), not_admin => $ldap->add_user("not_admin_user"), many_roles => $ldap->add_user("many_roles"), + many_roles_one_group => $ldap->add_user("many_roles_one_group"), }; $ldap->add_group("hydra_admin", $users->{"admin"}->{"username"}); $ldap->add_group("hydra-admin", $users->{"not_admin"}->{"username"}); +$ldap->add_group("hydra_one_group_many_roles", $users->{"many_roles_one_group"}->{"username"}); $ldap->add_group("hydra_create-projects", $users->{"many_roles"}->{"username"}); $ldap->add_group("hydra_restart-jobs", $users->{"many_roles"}->{"username"}); @@ -69,6 +71,10 @@ my $ctx = test_context( hydra_cancel-build = cancel-build hydra_bump-to-front = bump-to-front hydra_restart-jobs = restart-jobs + + hydra_one_group_many_roles = create-projects + hydra_one_group_many_roles = cancel-build + hydra_one_group_many_roles = bump-to-front CFG @@ -79,9 +85,10 @@ Catalyst::Test->import('Hydra'); subtest "Valid login attempts" => sub { my %users_to_roles = ( unrelated => [], - admin => ["admin"], - not_admin => [], - many_roles => [ "create-projects", "restart-jobs", "bump-to-front", "cancel-build" ], + admin => ["admin"], + not_admin => [], + many_roles => [ "create-projects", "restart-jobs", "bump-to-front", "cancel-build" ], + many_roles_one_group => [ "create-projects", "bump-to-front", "cancel-build" ], ); for my $username (keys %users_to_roles) { my $user = $users->{$username};