228417 - editparams doesn't check for NET::LDAP module when setting loginmethod to LDAP

Status: RESOLVED WORKSFORME

(Bugzilla :: Administration, task)

Description

[Tony Malandain]

User-Agent:       Mozilla/5.0 (X11; U; Linux i586) Gecko/20031119 Galeon/1.3.7
Build Identifier: Mozilla/5.0 (X11; U; Linux i586) Gecko/20031119 Galeon/1.3.7

The checksetup script doesn't detect if the NET::LDAP module is already
installed or not. This lead to a failure when the administrator tries to setup
authentication through LDAP.

Reproducible: Always

Steps to Reproduce:
1. Make sure Net::LDAP is not already installed.
2. Run checksetup.pl


Actual Results:  
checksetup doesn't warn the user that Net::LDAP needs to be installed in order
to have LDAP authentication support.

Expected Results:  
checksetup should add a warning in the optional module section explaining that
Net::LDAP must be installed in order to use LDAP authentication.

Comment 1

[Dave Miller [:justdave]]

If you set the loginmethod param in editparams to LDAP then checksetup will
check for it.

The option section was getting a bit cluttered if you didn't install the
options. (and editparams will check for it when you attempt to change the param)

Comment 2

[Dave Miller [:justdave]]

ok, editparams crashes when you try to set the param instead of giving you a
nice error.  That's my recommendation for fixing this though is to make
editparams give you a nice error instead.

Comment 3

[Dave Miller [:justdave]]

crasher -> critical

confirming.

Comment 4

[Dave Miller [:justdave]]

ok, chatting about this in IRC, I think we do need to get this into checksetup,
too, but we need to start finding ways to disable the checks for optional stuff
in checksetup.pl, because you get a full page of garbage now if you don't
install the optional stuff.  checksetup.pl is meant to be run every time you
upgrade or make local changes, and on many sites that happens frequently. :)

Comment 5

[Dave Miller [:justdave]]

OK, short term for 2.18 we need to get doeditparams.cgi to refuse to allow
loginmethod to be set to LDAP if Net::LDAP isn't installed.

Comment 6

[Joel Peshkin]

defparams looks like it already does this.  Is that not working?

Comment 7

[Bradley Baetz (:bbaetz)]

I'm sure that I amde sure that this still workd when I redid the auth stuff....

Comment 8

[Dave Miller [:justdave]]

hmm, yep, it's in an "eval" block, which means it shouldn't crash, and there is
an error check immediately afterwards.  Not a blocker then.

Comment 9

[Max Kanat-Alexander]

This bug has not been touched by its owner in over six months, even though it is
targeted to 2.20, for which the freeze is 10 days away. Unsetting the target
milestone, on the assumption that nobody is actually working on it or has any
plans to soon.

If you are the owner, and you plan to work on the bug, please give it a real
target milestone. If you are the owner, and you do *not* plan to work on it,
please reassign it to [e-mail] or a .bugs component owner. If you are
*anybody*, and you get this comment, and *you* plan to work on the bug, please
reassign it to yourself if you have the ability.

Comment 10

[Dave Miller [:justdave]]

Reassigning bugs that I'm not actively working on to the default component owner
in order to try to make some sanity out of my personal buglist.  This doesn't
mean the bug isn't being dealt with, just that I'm not the one doing it.  If you
are dealing with this bug, please assign it to yourself.

Comment 11

[Frédéric Buclin]

doeditparams.cgi has been merged with editparams.cgi. updating the summary
accordingly.

Comment 12

[Max Kanat-Alexander]

It checks it nowadays, in check_user_verify_class.