Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for require ldap-group #232

Closed
wants to merge 13 commits into from
12 changes: 11 additions & 1 deletion manifests/apache/conf.pp
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@
# (string) Determines if other authentication providers are used when a user can be mapped to a DN but the server cannot bind with the credentials
# No default ($::puppetboard::params::ldap_bind_authoritative)
#
# [*ldap_require_group]
# (bool) LDAP group to require on login
# Default to False ($::puppetboard::params::ldap_require_group)
#
# [*$ldap_require_group_dn]
# (string) LDAP group DN for LDAP group
# No default
#
# === Notes:
#
# Make sure you have purge_configs set to false in your apache class!
Expand All @@ -69,7 +77,9 @@
Optional[String] $ldap_bind_dn = undef,
Optional[String] $ldap_bind_password = undef,
Optional[String] $ldap_url = undef,
Optional[String] $ldap_bind_authoritative = undef
Optional[String] $ldap_bind_authoritative = undef,
Boolean $ldap_require_group = $::puppetboard::params::ldap_require_group,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$puppetboard:: vs $::puppetboard.

Optional[String] $ldap_require_group_dn = undef,
) inherits ::puppetboard::params {

$docroot = "${basedir}/puppetboard"
Expand Down
10 changes: 10 additions & 0 deletions manifests/apache/vhost.pp
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,14 @@
# (string) Determines if other authentication providers are used
# when a user can be mapped to a DN but the server cannot bind with the credentials
# No default ($::puppetboard::params::ldap_bind_authoritative)
#
# [*ldap_require_group]
# (bool) LDAP group to require on login
# Default to False ($::puppetboard::params::ldap_require_group)
#
# [*$ldap_require_group_dn]
# (string) LDAP group DN for LDAP group
# No default
class puppetboard::apache::vhost (
String $vhost_name,
String $wsgi_alias = '/',
Expand All @@ -88,6 +96,8 @@
Optional[String] $ldap_bind_password = undef,
Optional[String] $ldap_url = undef,
Optional[String] $ldap_bind_authoritative = undef,
Boolean $ldap_require_group = $::puppetboard::params::ldap_require_group,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the leading colons are not needed here anymore, so
$puppetboard::params...

Optional[String] $ldap_require_group_dn = undef,
Hash $custom_apache_parameters = {},
) inherits ::puppetboard::params {

Expand Down
1 change: 1 addition & 0 deletions manifests/params.pp
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,5 @@
$default_environment = 'production'
$extra_settings = {}
$enable_ldap_auth = false
$ldap_require_group = false
}
3 changes: 3 additions & 0 deletions spec/acceptance/class_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ class { 'puppetboard::apache::conf':
ldap_bind_dn => 'cn=user,dc=puppet,dc=example,dc=com',
ldap_bind_password => 'password',
ldap_url => 'ldap://puppet.example.com',
ldap_require_group => true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi - Yes, I think this is pretty close. But we should test both cases, so one context for ldap auth, and test that the config file does not contain "Require ldap-group".

Then make another conext that sets those parameters explicitly, and test for the expected results.

ldap_require_group_dn => 'cn=admins,=cn=groups,dc=puppet,dc=example,dc=com',
}
EOS

Expand All @@ -138,6 +140,7 @@ class { 'puppetboard::apache::conf':
it { is_expected.to contain 'AuthBasicProvider ldap' }
it { is_expected.to contain 'AuthLDAPBindDN "cn=user,dc=puppet,dc=example,dc=com"' }
it { is_expected.to contain 'AuthLDAPURL "ldap://puppet.example.com"' }
it { is_expected.to contain 'Require ldap-group "cn=admins,=cn=groups,dc=puppet,dc=example,dc=com"' }
end
describe file('/srv/puppetboard/puppetboard/settings.py') do
it { is_expected.to contain "PUPPETDB_KEY = '/var/lib/puppet/ssl/private_keys/test.networkninjas.net.pem'" }
Expand Down
4 changes: 4 additions & 0 deletions templates/apache/conf.erb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ WSGIScriptAlias <%= @wsgi_alias -%> <%= @docroot -%>/wsgi.py
<%- if @ldap_bind_authoritative -%>
AuthLDAPBindAuthoritative <%= @ldap_bind_authoritative -%>
<%- end -%>
<% if @ldap_require_group -%>
Require ldap-group "<%= @ldap_require_group_dn -%>"
<% else %>
Require valid-user
<% end %>
</LocationMatch>
<% end -%>
5 changes: 5 additions & 0 deletions templates/apache/ldap.erb
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,10 @@
<%- if @ldap_bind_authoritative -%>
AuthLDAPBindAuthoritative <%= @ldap_bind_authoritative -%>
<%- end -%>
<% if @ldap_require_group -%>
Require ldap-group "<%= ldap_require_group_dn -%>"
<% else %>
Require valid-user
<% end %>

</LocationMatch>