Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/controllers/api/v2/auth_source_ldaps_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ def show
param :onthefly_register, :bool
param :usergroup_sync, :bool, :desc => N_("sync external user groups on login")
param :tls, :bool
param :cacert, String, :desc => N_("PEM-encoded CA certificate(s)")
param :groups_base, String, :desc => N_("groups base DN")
param :use_netgroups, :bool, :desc => N_("use NIS netgroups instead of posix groups, applicable only when server_type is posix or free_ipa. Deprecated in favor of ldap_group_membership = nis_netgroups"), :deprecated => true
param :ldap_group_membership, AuthSourceLdap::GROUP_MEMBERSHIP_TYPES.keys, :desc => N_("type of group membership to use, applicable only when server_type is posix, free_ipa or netiq. Option rfc4519 is only applicable when server_type is posix.")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ def auth_source_ldap_params_filter
:attr_mail,
:attr_photo,
:base_dn,
:cacert,
:groups_base,
:host,
:ldap_filter,
Expand Down
12 changes: 11 additions & 1 deletion app/models/auth_sources/auth_source_ldap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class AuthSourceLdap < AuthSource
validates :server_type, :presence => true, :inclusion => { :in => SERVER_TYPES.keys.map(&:to_s) }
validates :ldap_group_membership, :presence => true, :inclusion => { :in => :allowed_group_membership_types }, :if => proc { |auth| %w[posix netiq free_ipa].include?(auth.server_type.to_s) }
validate :validate_ldap_filter, :unless => proc { |auth| auth.ldap_filter.blank? }
validate :validate_cacert, :unless => proc { |auth| auth.cacert.blank? }

before_validation :strip_ldap_attributes
before_validation :sanitize_group_membership
Expand Down Expand Up @@ -101,7 +102,8 @@ def to_config(login = nil, password = nil)

def encryption_config
return nil unless tls
{ :method => :simple_tls, :tls_options => { :verify_mode => OpenSSL::SSL::VERIFY_PEER } }
{ :method => :simple_tls, :tls_options => { :verify_mode => OpenSSL::SSL::VERIFY_PEER,
:cert_store => Foreman::Util.ssl_cert_store(cacert) } }
end

def ldap_con(login = nil, password = nil)
Expand Down Expand Up @@ -277,6 +279,14 @@ def validate_ldap_filter
errors.add(:ldap_filter, message)
end

def validate_cacert
Foreman::Util.ssl_cert_store(cacert)
rescue OpenSSL::X509::StoreError => e
message = _("invalid CA certificate")
Foreman::Logging.exception(message, e)
errors.add(:cacert, message)
end

def use_user_login_for_service?
# returns true if account is defined and includes "$login"
(account.present? && account.include?("$login"))
Expand Down
2 changes: 1 addition & 1 deletion app/models/compute_resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ def to_label
end

def connection_options
http_proxy ? {:proxy => http_proxy.full_url, :ssl_cert_store => http_proxy.ssl_cert_store} : {}
http_proxy ? {:proxy => http_proxy.full_url, :ssl_cert_store => Foreman::Util.ssl_cert_store(http_proxy.cacert)} : {}
end

# Override this method to specify provider name
Expand Down
11 changes: 1 addition & 10 deletions app/models/http_proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,23 +41,14 @@ def full_url
uri.to_s
end

def ssl_cert_store
cert_store = OpenSSL::X509::Store.new
cert_store.set_default_paths
if cacert.present?
Foreman::Util.add_ca_bundle_to_store(cacert, cert_store)
end
cert_store
end

def test_connection(url)
RestClient::Request.execute(
method: :head,
url: url,
proxy: full_url,
timeout: 5,
open_timeout: 5,
ssl_cert_store: ssl_cert_store
ssl_cert_store: Foreman::Util.ssl_cert_store(cacert)
)
rescue Excon::Error::Socket => e
e.message
Expand Down
2 changes: 1 addition & 1 deletion app/views/api/v2/auth_source_ldaps/main.json.rabl
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ object @auth_source_ldap
extends "api/v2/auth_source_ldaps/base"

attributes :host, :port, :account, :base_dn, :ldap_filter, :attr_login, :attr_firstname, :attr_lastname,
:attr_mail, :attr_photo, :onthefly_register, :usergroup_sync, :tls, :server_type, :groups_base,
:attr_mail, :attr_photo, :onthefly_register, :usergroup_sync, :tls, :cacert, :server_type, :groups_base,
:ldap_group_membership, :created_at, :updated_at

node :use_netgroups do |auth_source_ldap|
Expand Down
4 changes: 4 additions & 0 deletions app/views/auth_source_ldaps/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
<%= text_f f, :host, :help_inline => link_to_function(_("Test Connection"), "tfm.authSource.testConnection(this, '#{test_connection_auth_source_ldaps_url}')",
:title => _("Test LDAP connectivity"), :id => "test_connection_button", :class =>"btn btn-success") + hidden_spinner('', :id => 'test_connection_indicator').html_safe %>
<%= checkbox_f(f, :tls, {:label => "LDAPS", :onchange => 'tfm.authSource.changeLdapPort(this)'}) %>
<%= textarea_f f, :cacert, :rows => 5,
:control_group_id => 'auth_source_ldap_cacert_group',
:wrapper_class => "form-group#{' hide' unless f.object.tls?}",
:help_block => _("Optional PEM-encoded CA certificate(s) used to verify the LDAP server when LDAPS is enabled, in addition to the system trust store. Leave blank to use the system trust store only.") %>
<%= number_f f, :port, :min => 1, :max => 65535,
:data => { :default_ports => AuthSourceLdap::DEFAULT_PORTS } %>
<%= select_f f, :server_type, AuthSourceLdap::SERVER_TYPES, :first, :last,
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20260716124101_add_ca_cert_to_auth_sources.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddCACertToAuthSources < ActiveRecord::Migration[7.0]
def change
add_column :auth_sources, :cacert, :text
end
end
8 changes: 8 additions & 0 deletions lib/foreman/util.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,13 @@ def self.add_ca_bundle_to_store(ca_bundle, cert_store)
cert_store.add_file(f.path)
end
end

# OpenSSL certificate store with system defaults, optionally extended by a PEM CA bundle.
def self.ssl_cert_store(cacert = nil)
cert_store = OpenSSL::X509::Store.new
cert_store.set_default_paths
add_ca_bundle_to_store(cacert, cert_store) if cacert.present?
cert_store
end
end
end
44 changes: 44 additions & 0 deletions test/controllers/api/v2/auth_source_ldaps_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
class Api::V2::AuthSourceLdapsControllerTest < ActionController::TestCase
valid_attrs = { :name => 'ldap2', :host => 'ldap2', :server_type => 'posix' }

def sample_cacert
File.read(Rails.root.join('test/static_fixtures/certificates/example.com.crt')).chomp
end

test "should get index" do
get :index
assert_response :success
Expand All @@ -18,18 +22,58 @@ class Api::V2::AuthSourceLdapsControllerTest < ActionController::TestCase
assert !show_response.empty?
end

test "should show auth_source_ldap with cacert" do
auth = auth_sources(:one)
auth.update!(:tls => true, :cacert => sample_cacert, :port => 636)
get :show, params: { :id => auth.to_param }
assert_response :success
show_response = ActiveSupport::JSON.decode(@response.body)
assert_equal sample_cacert, show_response['cacert']
end

test "should include cacert in index" do
auth = auth_sources(:one)
auth.update!(:tls => true, :cacert => sample_cacert, :port => 636)
get :index
assert_response :success
results = ActiveSupport::JSON.decode(@response.body)['results']
entry = results.find { |r| r['id'] == auth.id }
assert_equal sample_cacert, entry['cacert']
end

test "should create auth_source_ldap" do
assert_difference('AuthSourceLdap.unscoped.count', 1) do
post :create, params: { :auth_source_ldap => valid_attrs }
end
assert_response :created
end

test "should create auth_source_ldap with cacert" do
attrs = valid_attrs.merge(:tls => true, :port => 636, :cacert => sample_cacert)
assert_difference('AuthSourceLdap.unscoped.count', 1) do
post :create, params: { :auth_source_ldap => attrs }
end
assert_response :created
show_response = ActiveSupport::JSON.decode(@response.body)
assert_equal sample_cacert, show_response['cacert']
assert_equal sample_cacert, AuthSourceLdap.unscoped.find(show_response['id']).cacert
end

test "should update auth_source_ldap" do
put :update, params: { :id => auth_sources(:one).to_param, :auth_source_ldap => valid_attrs }
assert_response :success
end

test "should update auth_source_ldap cacert" do
auth = auth_sources(:one)
put :update, params: { :id => auth.to_param,
:auth_source_ldap => { :tls => true, :port => 636, :cacert => sample_cacert } }
assert_response :success
show_response = ActiveSupport::JSON.decode(@response.body)
assert_equal sample_cacert, show_response['cacert']
assert_equal sample_cacert, auth.reload.cacert
end

test "should destroy auth_source_ldap" do
assert_difference('AuthSourceLdap.unscoped.count', -1) do
auth = auth_sources(:one)
Expand Down
5 changes: 5 additions & 0 deletions test/controllers/concerns/parameters/auth_source_ldap_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,9 @@ class AuthSourceLdapParametersTest < ActiveSupport::TestCase
params = ActionController::Parameters.new(:auth_source_ldap => {:type => AuthSourceHidden.name})
refute_includes self.class.auth_source_ldap_params_filter.filter_params(params, context), 'type'
end

test "permits cacert" do
params = ActionController::Parameters.new(:auth_source_ldap => {:cacert => "-----BEGIN CERTIFICATE-----\n...\n-----END CERTIFICATE-----"})
assert_includes self.class.auth_source_ldap_params_filter.filter_params(params, context), 'cacert'
end
end
14 changes: 14 additions & 0 deletions test/models/auth_sources/auth_source_ldap_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ def setup
should allow_value(' ').for(:ldap_filter)
should allow_value('key=value').for(:ldap_filter)
should allow_value("key=#{'a' * 256}").for(:ldap_filter)
should_not validate_presence_of(:cacert)
should_not allow_value('not a certificate').for(:cacert)
should_not allow_value("-----BEGIN CERTIFICATE-----\nYQo=\n-----END CERTIFICATE-----").for(:cacert)
should allow_value('').for(:cacert)
should allow_value(nil).for(:cacert)
should allow_value(File.read(Rails.root.join('test/static_fixtures/certificates/example.com.crt'))).for(:cacert)
should validate_length_of(:name).is_at_most(60)
should validate_length_of(:host).is_at_most(60)
should validate_length_of(:account_password).is_at_most(69)
Expand Down Expand Up @@ -208,6 +214,14 @@ def setup
assert_equal OpenSSL::SSL::VERIFY_PEER, conf[:encryption][:tls_options][:verify_mode]
end

test '#to_config includes cert_store when tls and cacert are set' do
cacert = File.read(Rails.root.join('test/static_fixtures/certificates/example.com.crt'))
ldap = FactoryBot.build_stubbed(:auth_source_ldap, :tls => true, :cacert => cacert)
conf = ldap.to_config('user', 'pass')
assert_kind_of OpenSSL::X509::Store, conf[:encryption][:tls_options][:cert_store]
assert_equal OpenSSL::SSL::VERIFY_PEER, conf[:encryption][:tls_options][:verify_mode]
end

test '#ldap_con does not cache connections with user auth' do
ldap = FactoryBot.build_stubbed(:auth_source_ldap, :account => 'DOMAIN/$login')
refute_equal ldap.ldap_con('user', 'pass'), ldap.ldap_con('user', 'pass')
Expand Down
17 changes: 17 additions & 0 deletions test/unit/foreman/util_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,21 @@ class UtilTest < ActiveSupport::TestCase
test 'secure_encryption_key should match AS default key length' do
assert_equal ActiveSupport::MessageEncryptor.key_len, secure_encryption_key.length
end

test 'ssl_cert_store returns a store with system defaults' do
store = Foreman::Util.ssl_cert_store
assert_instance_of OpenSSL::X509::Store, store
end

test 'ssl_cert_store adds cacert when present' do
cacert = File.read(Rails.root.join('test/static_fixtures/certificates/example.com.crt'))
Foreman::Util.expects(:add_ca_bundle_to_store).with(cacert, instance_of(OpenSSL::X509::Store))
Foreman::Util.ssl_cert_store(cacert)
end

test 'ssl_cert_store skips cacert when blank' do
Foreman::Util.expects(:add_ca_bundle_to_store).never
Foreman::Util.ssl_cert_store(nil)
Foreman::Util.ssl_cert_store('')
end
end
9 changes: 8 additions & 1 deletion webpack/assets/javascripts/foreman_auth_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@

export function changeLdapPort(item) {
const port = $('#auth_source_ldap_port');
const tlsEnabled = $(item).is(':checked');

if (port.length > 0) {
const value = parseInt(port.val(), 10);
Expand All @@ -47,7 +48,7 @@
defaultPorts.ldap != null &&
defaultPorts.ldaps != null
) {
if ($(item).is(':checked')) {
if (tlsEnabled) {
if (value === defaultPorts.ldap) {
port.val(defaultPorts.ldaps);
}
Expand All @@ -56,6 +57,8 @@
}
}
}

$('#auth_source_ldap_cacert_group').toggle(tlsEnabled);

Check warning on line 61 in webpack/assets/javascripts/foreman_auth_source.js

View workflow job for this annotation

GitHub Actions / test (13, 3.3, 22)

You have a misspelled word: cacert on String

Check warning on line 61 in webpack/assets/javascripts/foreman_auth_source.js

View workflow job for this annotation

GitHub Actions / test (13, 3.0, 22)

You have a misspelled word: cacert on String
}

function updateLdapAccountHelp(selectedType) {
Expand Down Expand Up @@ -108,5 +111,9 @@
$(document).ready(() => {
if (window.location.pathname.match(/auth_source_ldaps/i)) {
changeLdapServerType();
const tlsCheckbox = $('#auth_source_ldap_tls');

Check warning on line 114 in webpack/assets/javascripts/foreman_auth_source.js

View workflow job for this annotation

GitHub Actions / test (13, 3.3, 22)

You have a misspelled word: tls on String

Check warning on line 114 in webpack/assets/javascripts/foreman_auth_source.js

View workflow job for this annotation

GitHub Actions / test (13, 3.0, 22)

You have a misspelled word: tls on String
if (tlsCheckbox.length > 0) {
changeLdapPort(tlsCheckbox);
}
}
});
24 changes: 22 additions & 2 deletions webpack/assets/javascripts/foreman_auth_source.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,37 @@ jest.unmock('jquery');
jest.unmock('./foreman_auth_source');

describe('AuthSourceLDAP tests', () => {
it('change LDAP port', () => {
beforeEach(() => {
document.body.innerHTML = `
<input min="1" max="65535" data-default-ports="{&quot;ldap&quot;:389,&quot;ldaps&quot;:636}" class="form-control " type="number" value="389" name="auth_source_ldap[port]" id="auth_source_ldap_port">
<div id="auth_source_ldap_cacert_group" class="form-group hide" style="display: none;">
<textarea name="auth_source_ldap[cacert]" id="auth_source_ldap_cacert"></textarea>
</div>
`;
});

it('change LDAP port', () => {
const notChecked =
'<input type="checkbox" name="auth_source_ldap[tls]" value="0""> ';
'<input type="checkbox" name="auth_source_ldap[tls]" value="0"> ';
const checked =
'<input type="checkbox" name="auth_source_ldap[tls]" value="1" checked="checked"> ';
changeLdapPort(checked);
expect($('#auth_source_ldap_port').val()).toEqual('636');
changeLdapPort(notChecked);
expect($('#auth_source_ldap_port').val()).toEqual('389');
});

it('toggles CA certificate field with LDAPS', () => {
const notChecked =
'<input type="checkbox" name="auth_source_ldap[tls]" value="0"> ';
const checked =
'<input type="checkbox" name="auth_source_ldap[tls]" value="1" checked="checked"> ';
const cacertGroup = document.getElementById('auth_source_ldap_cacert_group');

changeLdapPort(checked);
expect(cacertGroup.style.display).not.toBe('none');

changeLdapPort(notChecked);
expect(cacertGroup.style.display).toBe('none');
});
});
Loading