Skip to content

[WIP] Delete ldap forest tech debt #9663

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[WIP] Delete ldap forest tech debt #9663

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a7e5a22
Delete ldap forest tech debt
GilbertCherrie Oct 16, 2025
9d10a7c
Fix specs
GilbertCherrie Oct 16, 2025
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
111 changes: 0 additions & 111 deletions app/controllers/ops_controller/settings.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,117 +35,6 @@ def apply_imports
redirect_to(:action => 'explorer', :no_refresh => true)
end

def forest_get_form_vars
@edit = session[:edit]
@ldap_info = {}
@ldap_info[:mode] = params[:user_proxies_mode] if params[:user_proxies] && params[:user_proxies_mode]
@ldap_info[:ldaphost] = params[:user_proxies][:ldaphost] if params[:user_proxies] && params[:user_proxies][:ldaphost]
@ldap_info[:ldapport] = params[:user_proxies][:ldapport] if params[:user_proxies] && params[:user_proxies][:ldapport]
@ldap_info[:basedn] = params[:user_proxies][:basedn] if params[:user_proxies] && params[:user_proxies][:basedn]
@ldap_info[:bind_dn] = params[:user_proxies][:bind_dn] if params[:user_proxies] && params[:user_proxies][:bind_dn]
@ldap_info[:bind_pwd] = params[:user_proxies][:bind_pwd] if params[:user_proxies] && params[:user_proxies][:bind_pwd]
nil
end

def forest_form_field_changed
assert_privileges("ops_settings")

@edit = session[:edit] # Need to reload @edit so it stays in the session
port = params[:user_proxies_mode] == "ldap" ? "389" : "636"
render :update do |page|
page << javascript_prologue
page << "$('#user_proxies_ldapport').val('#{port}');"
end
end

# AJAX driven routine to select a classification entry
def forest_select
assert_privileges("ops_settings")

forest_get_form_vars
if params[:ldaphost_id] == "new"
render :update do |page|
page << javascript_prologue
page.replace("flash_msg_div", :partial => "layouts/flash_msg")
page << "miqScrollTop();" if @flash_array.present?
page.replace("forest_entries_div", :partial => "ldap_forest_entries", :locals => {:entry => "new", :edit => true})
end
session[:entry] = "new"
else
entry = nil
@edit[:new][:authentication][:user_proxies].each do |f|
entry = f if f[:ldaphost] == params[:ldaphost_id]
end
render :update do |page|
page << javascript_prologue
page.replace("flash_msg_div", :partial => "layouts/flash_msg")
page << "miqScrollTop();" if @flash_array.present?
page.replace("forest_entries_div", :partial => "ldap_forest_entries", :locals => {:entry => entry, :edit => true})
end
session[:entry] = entry
end
end

# AJAX driven routine to delete a classification entry
def forest_delete
assert_privileges("ops_settings")

forest_get_form_vars
idx = nil
@edit[:new][:authentication][:user_proxies].each_with_index do |f, i|
idx = i if f[:ldaphost] == params[:ldaphost_id]
end
@edit[:new][:authentication][:user_proxies].delete_at(idx) unless idx.nil?
@changed = (@edit[:new] != @edit[:current])
render :update do |page|
page << javascript_prologue
page.replace("flash_msg_div", :partial => "layouts/flash_msg")
page << "miqScrollTop();" if @flash_array.present?
page << javascript_for_miq_button_visibility(@changed)
page.replace("forest_entries_div", :partial => "ldap_forest_entries", :locals => {:entry => nil, :edit => false})
end
end

# AJAX driven routine to add/update a classification entry
def forest_accept
assert_privileges("ops_settings")

forest_get_form_vars
no_changes = true
if @ldap_info[:ldaphost] == ""
add_flash(_("LDAP Host is required"), :error)
no_changes = false
elsif @edit[:new][:authentication][:user_proxies].blank? || @edit[:new][:authentication][:user_proxies][0].blank? # if adding forest first time, delete a blank record
@edit[:new][:authentication][:user_proxies].delete_at(0)
else
@edit[:new][:authentication][:user_proxies].each do |f|
# check to make sure ldaphost already doesn't exist and ignore if existing record is being edited.
next unless f[:ldaphost] == @ldap_info[:ldaphost] && session[:entry] == 'new'

no_changes = false
add_flash(_("LDAP Host should be unique"), :error)
break
end
end
if no_changes
if session[:entry] == "new"
@edit[:new][:authentication][:user_proxies].push(@ldap_info)
else
@edit[:new][:authentication][:user_proxies].each_with_index do |f, i|
@edit[:new][:authentication][:user_proxies][i] = @ldap_info if f[:ldaphost] == session[:entry][:ldaphost]
end
end
end
@changed = (@edit[:new] != @edit[:current])
render :update do |page|
page << javascript_prologue
page << javascript_for_miq_button_visibility(@changed)
page.replace("flash_msg_div", :partial => "layouts/flash_msg")
page << "miqScrollTop();" if @flash_array.present?
page.replace("forest_entries_div", :partial => "ldap_forest_entries", :locals => {:entry => nil, :edit => false}) if no_changes
end
end

def region_edit
assert_privileges("region_edit")

Expand Down
90 changes: 0 additions & 90 deletions app/views/ops/_ldap_forest_entries.html.haml
View file
Open in desktop

This file was deleted.

4 changes: 0 additions & 4 deletions config/routes.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2446,10 +2446,6 @@
diagnostics_tree_select
explorer
fetch_target_ids
forest_accept
forest_delete
forest_form_field_changed
forest_select
help_menu_form_field_changed
label_tag_mapping_delete
label_tag_mapping_edit
Expand Down
15 changes: 0 additions & 15 deletions spec/controllers/ops_controller/settings/common_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,21 +152,6 @@
:active_tab => 'settings_authentication')
controller.x_node = "svr-#{miq_server.id}"
end

it "sets ldap_role to false to make forest entries div hidden" do
controller.params = {:id => 'authentication',
:authentication_mode => 'database'}
controller.send(:settings_get_form_vars)
expect(assigns(:edit)[:new][:authentication][:ldap_role]).to eq(false)
end

it "resets ldap_role to it's original state so forest entries div can be displayed" do
session[:edit][:new][:authentication][:mode] = 'database'
controller.params = {:id => 'authentication',
:authentication_mode => 'ldap'}
controller.send(:settings_get_form_vars)
expect(assigns(:edit)[:new][:authentication][:ldap_role]).to eq(true)
end
end

describe "#pglogical_save_subscriptions" do
Expand Down
59 changes: 0 additions & 59 deletions spec/controllers/ops_settings_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,65 +136,6 @@
allow(controller).to receive(:data_for_breadcrumbs).and_return({})
end
end

context '#forest_accept' do
let(:user) { FactoryBot.create(:user, :features => %w(ops_settings)) }
before do
login_as user
end

context 'adding an LDAP Trusted Forest' do
before do
EvmSpecHelper.create_guid_miq_server_zone
@user_proxies = {:ldaphost => 'ldap.manageiq1.org',
:ldapport => '389',
:mode => 'ldap',
:basedn => 'cn=groups,cn=accounts,dc=miq',
:bind_dn => 'uid=admin,cn=users,cn=accounts,dc=miq,dc=e',
:bind_pwd => '******'}
@vmdb = ::Settings.to_hash
expect(controller).to receive(:render)
end

after(:each) { expect(response.status).to eq(200) }

it 'is a new record' do
session[:edit] = {:current => @vmdb, :new => {:authentication => {:user_proxies => []}}}
session[:entry] = 'new'
controller.send(:forest_accept)
end

it 'is an existing record' do
controller.params = {:user_proxies_mode => '', :user_proxies => @user_proxies}
session[:edit] = {:current => @vmdb, :new => {:authentication => {:user_proxies => [@user_proxies]}}}
session[:entry] = @user_proxies
controller.send(:forest_accept)
end

it 'LDAP Host exists' do
@user_proxies[:ldaphost] = ''
controller.params = {:user_proxies_mode => '', :user_proxies => @user_proxies}
session[:edit] = {:current => @vmdb, :new => {:authentication => {:user_proxies => [@user_proxies]}}}
session[:entry] = @user_proxies

controller.send(:forest_accept)

flash_messages = controller.instance_variable_get(:@flash_array)
expect(flash_messages.first).to eq(:message => 'LDAP Host is required', :level => :error)
end

it 'LDAP Host is unique' do
controller.params = {:user_proxies_mode => '', :user_proxies => @user_proxies}
session[:edit] = {:current => @vmdb, :new => {:authentication => {:user_proxies => [@user_proxies, @user_proxies]}}}
session[:entry] = 'new'

controller.send(:forest_accept)

flash_messages = controller.instance_variable_get(:@flash_array)
expect(flash_messages.first).to eq(:message => 'LDAP Host should be unique', :level => :error)
end
end
end
end

context "replace_right_cell" do
Expand Down
4 changes: 0 additions & 4 deletions spec/routing/ops_routing_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@
diagnostics_server_list
diagnostics_tree_select
explorer
forest_accept
forest_delete
forest_form_field_changed
forest_select
label_tag_mapping_delete
label_tag_mapping_edit
label_tag_mapping_update
Expand Down