Remove LDAP parts #366

Firstyear · 2022-08-10T03:24:24Z

Start to remove all ldap integration.

The LDAP integration as it exists in Yast is unmaintained and does a lot of things incorrectly. It relies on functionality to bypass password policy in LDAP servers, it requires client side pre-hashing/salting of passwords, it allows clear text storage of pws, it isn't schema aware, it relies on openldap specific functionality and more. Generally it's not in a good shape.

This starts the removal process. There are better tools to manage ldap accounts like ldapvi, dsidm, apache directory studio and more.

Currently this PR does have an error with rake run but the ruby trace has no correlation to the code so I don't know whats wrong :(

INFO: Gem yast-rake-ci not installed, extra tasks not loaded
** Invoke run (first_time)
** Execute run
/sbin/yast2 src/clients/users.rb
rake aborted!
Command failed with status (2): [/sbin/yast2 src/clients/users.rb...]
/usr/lib64/ruby/gems/3.1.0/gems/rake-13.0.6/lib/rake/file_utils.rb:67:in `block in create_shell_runner'
/usr/lib64/ruby/gems/3.1.0/gems/rake-13.0.6/lib/rake/file_utils.rb:57:in `sh'
/usr/lib64/ruby/gems/3.1.0/gems/yast-rake-0.2.47/lib/tasks/run.rake:48:in `block in <top (required)>'
/usr/lib64/ruby/gems/3.1.0/gems/rake-13.0.6/lib/rake/task.rb:281:in `block in execute'
/usr/lib64/ruby/gems/3.1.0/gems/rake-13.0.6/lib/rake/task.rb:281:in `each'
/usr/lib64/ruby/gems/3.1.0/gems/rake-13.0.6/lib/rake/task.rb:281:in `execute'
/usr/lib64/ruby/gems/3.1.0/gems/rake-13.0.6/lib/rake/task.rb:219:in `block in invoke_with_call_chain'
/usr/lib64/ruby/gems/3.1.0/gems/rake-13.0.6/lib/rake/task.rb:199:in `synchronize'
/usr/lib64/ruby/gems/3.1.0/gems/rake-13.0.6/lib/rake/task.rb:199:in `invoke_with_call_chain'
/usr/lib64/ruby/gems/3.1.0/gems/rake-13.0.6/lib/rake/task.rb:188:in `invoke'
/usr/lib64/ruby/gems/3.1.0/gems/rake-13.0.6/lib/rake/application.rb:160:in `invoke_task'
/usr/lib64/ruby/gems/3.1.0/gems/rake-13.0.6/lib/rake/application.rb:116:in `block (2 levels) in top_level'
/usr/lib64/ruby/gems/3.1.0/gems/rake-13.0.6/lib/rake/application.rb:116:in `each'
/usr/lib64/ruby/gems/3.1.0/gems/rake-13.0.6/lib/rake/application.rb:116:in `block in top_level'
/usr/lib64/ruby/gems/3.1.0/gems/rake-13.0.6/lib/rake/application.rb:125:in `run_with_threads'
/usr/lib64/ruby/gems/3.1.0/gems/rake-13.0.6/lib/rake/application.rb:110:in `top_level'
/usr/lib64/ruby/gems/3.1.0/gems/rake-13.0.6/lib/rake/application.rb:83:in `block in run'
/usr/lib64/ruby/gems/3.1.0/gems/rake-13.0.6/lib/rake/application.rb:186:in `standard_exception_handling'
/usr/lib64/ruby/gems/3.1.0/gems/rake-13.0.6/lib/rake/application.rb:80:in `run'
/usr/lib64/ruby/gems/3.1.0/gems/rake-13.0.6/exe/rake:27:in `<top (required)>'
/usr/bin/rake:25:in `load'
/usr/bin/rake:25:in `<main>'
Tasks: TOP => run

dgdavid

Thanks a lot for the PR, much appreciate.

I'd like to test the module manually before going for the approval. It might take me some time because I'm a bit busy today. Hope you don't mind.

Meanwhile, you can bump version and add an entry in the changelog ;)

dgdavid

I managed to invest some time testing this PR today. We're on the way ;) but there are some thing to polish yet.

Users.pm has now a bunch of variables that are used but not previously declared. For example, $substituted, @user_internals, @group_internals or even $password (this one in the sub _hashPassword) We have to either, get ride of the useless code using them or re-declare those that still in use.

The Makefile still referencing files that does not exist anymore

diff --git a/src/Makefile.am b/src/Makefile.am
index 8622773e..3771f491 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -9,7 +9,6 @@ module_DATA = \
  modules/UsersPlugins.pm \
  modules/UsersSimple.pm \
  modules/UsersCache.pm \
-  modules/UsersPluginKerberos.pm \
  modules/UsersPasswd.pm \
  modules/Y2UsersLinux.rb

@@ -25,9 +24,6 @@ client_DATA = \
  clients/users_finish.rb \
  clients/groups.rb \
  clients/users_plugin_quota.rb \
-  clients/users_plugin_ldap_all.rb \
-  clients/users_plugin_ldap_shadowaccount.rb \
-  clients/users_plugin_ldap_passwordpolicy.rb \
  clients/users.rb \
  clients/inst_root_first.rb \
  clients/inst_user_first.rb \
@@ -42,7 +38,6 @@ yncludedir = @yncludedir@/users
ynclude_DATA = \
  include/users/widgets.rb \
  include/users/wizards.rb \
-  include/users/ldap_dialogs.rb \
  include/users/dialogs.rb \
  include/users/complex.rb \
  include/users/routines.rb \

We have to remove these references from there.

The module fails when trying to write changes to the system

Internal error. Please report a bug report with logs.
Run save_y2logs to get complete logs.

Caller: /usr/share/YaST2/include/users/complex.rb:84:in `WriteDialog'

Details: undefined method `LDAPModified' for Yast::Users:Module

if Users.LDAPModified && (Ldap.anonymous || Ldap.bind_pass == nil)
^^^^^^^^^^^^^

Start the Ruby debugger now and debug the issue? (Experts only!)

Which is probably a matter of adapting the #WriteDialog method.

src/modules/Users.pm

dgdavid · 2022-08-23T13:01:24Z

This step should be removed too:

yast-users/src/modules/Users.pm

Lines 4161 to 4164 in 074bb76

    
           # progress stage label 
        
           __("Write LDAP users, groups and settings"), 
        
           # progress stage label 
        
           __("Write local users, groups and settings"),

Firstyear · 2022-08-24T00:51:49Z

Yeah, there is a lot to remove still, but once I hit the case where I couldn't proceed, I stopped "removing" since I could no longer test and I didn't want to cause more damage. So any help would be great :)

dgdavid · 2022-08-24T07:03:25Z

Yeah, there is a lot to remove still, but once I hit the case where I couldn't proceed, I stopped "removing" since I could no longer test and I didn't want to cause more damage. So any help would be great :)

Ok, I'll jump in. Let's see how further I can go.

Related to commit 50e8c4c

coveralls · 2022-08-24T08:24:07Z

Pull Request Test Coverage Report for Build 2925671381

0 of 11 (0.0%) changed or added relevant lines in 4 files are covered.
12 unchanged lines in 4 files lost coverage.
Overall coverage increased (+4.8%) to 64.835%

Changes Missing Coverage	Changed/Added Lines	%
src/include/users/wizards.rb	1	0.0%
src/include/users/helps.rb	2	0.0%
src/include/users/widgets.rb	2	0.0%
src/include/users/dialogs.rb	6	0.0%

Files with Coverage Reduction	New Missed Lines	%
src/include/users/complex.rb	1	43.48%
src/include/users/wizards.rb	3	29.27%
src/include/users/dialogs.rb	4	10.95%
src/include/users/widgets.rb	4	13.33%

Totals
Change from base Build 2904522885:	4.8%
Covered Lines:	3269
Relevant Lines:	5042

💛 - Coveralls

Otherwise, Perl will complain with 'Bareword "true" not allowed while "strict subs"'

dgdavid · 2022-08-24T08:58:29Z

I manage to get CI green, but still changes to do. Specially in the documentation, where none LDAP reference has been removed yet.

ancorgs · 2022-08-25T09:26:33Z

There are quite some documents that mention the LDAP functionality. I guess they should also get updated to explicitly mention the functionality was there but was intentionally removed:

dgdavid reviewed Aug 10, 2022

View reviewed changes

This comment was marked as outdated.

Sign in to view

dgdavid mentioned this pull request Aug 23, 2022

Do not crash if reading active LSM modules returns nil yast/yast-security#129

Merged

dgdavid requested changes Aug 23, 2022

View reviewed changes

This comment was marked as outdated.

Sign in to view

dgdavid reviewed Aug 23, 2022

View reviewed changes

src/modules/Users.pm Show resolved Hide resolved

dgdavid reviewed Aug 23, 2022

View reviewed changes

src/modules/Users.pm Show resolved Hide resolved

dgdavid reviewed Aug 23, 2022

View reviewed changes

src/modules/Users.pm Show resolved Hide resolved

src/modules/Users.pm Show resolved Hide resolved

Firstyear and others added 4 commits August 24, 2022 09:16

Remove LDAP parts

50e8c4c

Update Makefile file

a45ba1b

More LDAP removals

1a182fa

Related to commit 50e8c4c

Bump version and update changelog

3d6791d

dgdavid force-pushed the 20220810-remove-ldap branch from 2b168e4 to 3d6791d Compare August 24, 2022 08:21

dgdavid added 3 commits August 24, 2022 09:39

Additional LDAP removals

5a0e4d4

Do not use bareword true

5a89fe5

Otherwise, Perl will complain with 'Bareword "true" not allowed while "strict subs"'

Fix member_attr initialization in USERS.pm

ec7bbd1

dgdavid requested review from jreidinger and joseivanlopez August 24, 2022 08:58

dgdavid force-pushed the 20220810-remove-ldap branch from 5daa2f7 to 67acebc Compare August 25, 2022 09:59

Update documentation

67acebc

Update README

e30fe3d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove LDAP parts #366

Remove LDAP parts #366

Firstyear commented Aug 10, 2022

dgdavid left a comment

This comment was marked as outdated.

This comment was marked as outdated.

dgdavid left a comment

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

dgdavid commented Aug 23, 2022

Firstyear commented Aug 24, 2022

dgdavid commented Aug 24, 2022

coveralls commented Aug 24, 2022 •

edited

Loading

dgdavid commented Aug 24, 2022

ancorgs commented Aug 25, 2022

Remove LDAP parts #366

Are you sure you want to change the base?

Remove LDAP parts #366

Conversation

Firstyear commented Aug 10, 2022

dgdavid left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

dgdavid left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

dgdavid commented Aug 23, 2022

Firstyear commented Aug 24, 2022

dgdavid commented Aug 24, 2022

coveralls commented Aug 24, 2022 • edited Loading

Pull Request Test Coverage Report for Build 2925671381

💛 - Coveralls

dgdavid commented Aug 24, 2022

ancorgs commented Aug 25, 2022

coveralls commented Aug 24, 2022 •

edited

Loading