536888 - Fix phonebook (and other internal apps) - ldap issues

Status: RESOLVED FIXED

(Webtools Graveyard :: Phonebook, defect)

Description

[Aravind Gottipati [:aravind]]

It appears that the phonebook currently used the binduser to query and fetch user information (for displaying in the phonebook).  This is incorrect and will break when I tighten the binduser access privileges.  The binduser is meant to be used only to fetch the users full dn (and lookup any group information as needed).

The phone book (and any other apps that use ldap) should be changed to bind as a regular user (with the supplied credentials) and do any other lookups (picture, im, etc) and fetch information using the users dn.

Comment 1

[Michael Morgan [:morgamic]]

We coded it that way because LDAP was set up wrong.  Now we're a dependency for doing it right.  OK.

Comment 2

[Michael Morgan [:morgamic]]

What other internal apps do you need fixed, Aravind?

Comment 3

[Michael Morgan [:morgamic]]

Attachment: v1, moves crap out of config and adds application bind

- moves functions out of config.php into functions.php
- changes ldapconn bind to use app-defined dn/pass
- moves config.php to config-dist.php so we don't check in passwords

Comment 4

[Michael Morgan [:morgamic]]

Wilson - can you look at that patch and see if that flies?  Aravind, if you want us to patch this we need to have a bind dn and password for the application.  Could you create that and email the info to us at some point please?

Comment 5

[Aravind Gottipati [:aravind]]

(In reply to comment #1)
> We coded it that way because LDAP was set up wrong.  Now we're a dependency for
> doing it right.  OK.

Please explain to me how "you coded it that way because LDAP was set up wrong".  What specifically in our LDAP setup is causing you to use the bind DN to query user information instead of the user's DN?

> What other internal apps do you need fixed, Aravind?

I meant that as a general check for applications like the PTO app, etc.. that use LDAP.  If you have an app that uses LDAP, please make sure it adheres to the general guideline above.

> Your request to get the bind dn and password

I am okay with sharing that information with you, but it shouldn't be needed to build the app.  You can always use your credentials when you develop it, place it in a config file and I will change the config file as needed (like we do with all our other apps).

Comment 6

[Wilson Lee [:kourge]]

Since I'm not sure if I'm reading this correctly, let me try to paraphrase what I've read so far:
The application currently binds using user-supplied credentials to do anything. This is wrong. It should bind anonymously as much as possible and only bind with user credentials when more privileged operations such as editing entries are going to be performed.

If that is the case here, I'm all for this change. If not, then I'm not exactly sure what should be changed. (The way LDAP is handled in the phonebook was inherited from the old codebase.)

Comment 7

[Aravind Gottipati [:aravind]]

(In reply to comment #6)
> Since I'm not sure if I'm reading this correctly, let me try to paraphrase what
> I've read so far:
> The application currently binds using user-supplied credentials to do anything.
> This is wrong. 

Oh no.. That's exactly what it should do!  You should be using the binduser credentials to get the user dn from LDAP and from that point on, use that user DN for any further queries.

> If that is the case here, I'm all for this change. If not, then I'm not exactly
> sure what should be changed. (The way LDAP is handled in the phonebook was
> inherited from the old codebase.)

The old phonebook did it right, but I wasn't sure how the new phone book handled it.  When I tightened the privileges in LDAP last quarter, I broke the phone book, I assumed that this was because you were using the binduser credentials for everything.

In the first comment, I said "It appears that the phonebook currently used the binduser to query and fetch user information (for displaying in the phonebook)."  If you are telling me that this is not the case, then I will go dig some more into LDAP settings and figure out whats causing the behavior.

Comment 8

[Wilson Lee [:kourge]]

I'm sure the phonebook currently binds using user-supplied credentials and nothing else. There was one thing I did change: the old phonebook would first try to bind anonymously. If that fails, it immediately gives up. If that succeeds, it would then try to look up a DN using the user-supplied email address. It would then bind again using the resulting DN and the user-supplied password.

I changed it to thus: given an email address, the phonebook would use heuristics to determine what the DN should be. (This is done without using LDAP.) It then binds with the heuristically determined DN and the user-supplied password.

Comment 9

[Michael Morgan [:morgamic]]

(In reply to comment #5)
> Please explain to me how "you coded it that way because LDAP was set up wrong".
>  What specifically in our LDAP setup is causing you to use the bind DN to query
> user information instead of the user's DN?
I misunderstood the question.  Ignore that comment.  It's why I shouldn't comment on bugs after 8pm.

In the past when I've worked on these apps we used application-specific logins to control what the app could do and didn't rely on user accounts at all aside from authentication checks to see if they were allowed to login at all.  What the app could/couldn't see was tied to a unique account set up for the app with certain privs.

Anyway, if we could make the LDAP change and test the phonebook we could see the logs so we can understand what's breaking.  Should have done this in the first place since it's much easier than making assumptions.

Comment 10

[Wilson Lee [:kourge]]

Is LDAP binding no longer an issue for the phonebook? Have the binduser privileges been tightened yet? If that's the case I'm marking this bug as resolved.

Comment 11

[Fred Wenzel [:wenzel]]

Confidently closing this fixed for now. I believe the phonebook is working fine now, as the user who's logged in.