Skip to content
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.

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

Add ldap bind user #45

Open
t0xicCode opened this issue Sep 6, 2014 · 13 comments
Open

Add ldap bind user #45

t0xicCode opened this issue Sep 6, 2014 · 13 comments

Comments

@t0xicCode
Copy link

It is considered good practice to have ldap authentication libraries first bind using a service account, then run a search for the user and finally try to bind (and thus authenticate) using the provided credentials.

As an added bonus, that would enable supports for multi-level user trees, which the current implementation does not support.

@pmjones
Copy link
Member

pmjones commented Sep 6, 2014

I am all for that. If you have examples in code, or a PR, I'd be happy to a review.

@t0xicCode
Copy link
Author

Unfortunately I do not have any example readily available. I do know that
the Drupal ldap module supports this use case.

@pmjones
Copy link
Member

pmjones commented Sep 6, 2014

Link to the Drupal LDAP module? Every little bit you help out helps this go faster. :-)

@t0xicCode
Copy link
Author

I'm on mobile right now, but I'll add some details as soon as I get to a
computer 😃

@harikt
Copy link
Member

harikt commented Sep 7, 2014

@t0xicCode is it simple ldap or ldap ?

http://cgit.drupalcode.org/simple_ldap/tree/?h=7.x-2.x
http://cgit.drupalcode.org/ldap/tree/?h=8.x-2.x

May be good if you can point to the right file though.

Thanks

@t0xicCode
Copy link
Author

@harikt ldap. It's located in the function at http://cgit.drupalcode.org/ldap/tree/ldap_authentication/ldap_authentication.inc#n532.

Line 562 does the initial bind with the service account credentials, it then maps or searches for the given username, and finally, at line 659 actually authenticates with the given password and the mapped ldap user.

The bind function that is called at multiple places uses the stored service account information if it's passed NULL for its parameters.

@harikt
Copy link
Member

harikt commented Sep 7, 2014

Thank you @t0xicCode

@enygma
Copy link
Contributor

enygma commented Oct 15, 2014

There's also some trickiness here that some LDAP servers require you to set options for the bind to even work right:

ldap_set_option($ldap, LDAP_OPT_REFERRALS, 0);
ldap_set_option($ldap, LDAP_OPT_PROTOCOL_VERSION, 3);

I'm not sure this breaks it for other connection types, but I had to use this for a MS-based domain handler.

Also, I noticed in the VerifierInterface, it requires the second parameter (the hashed value). In this case, it doesn't make sense to have to generate something for that as the plain-text password is just sent to the LDAP server for validation.

@harikt
Copy link
Member

harikt commented Oct 15, 2014

Also, I noticed in the VerifierInterface, it requires the second parameter (the hashed value). In this case, it doesn't make sense to have to generate something for that as the plain-text password is just sent to the LDAP server for validation.

Hm, in that case the verifier don't need to do the verification . And even though there need a PlainText verifier.

@harikt
Copy link
Member

harikt commented Oct 15, 2014

Sorry forgot to add . Which always returns true.

<?php
namespace Aura\Auth\Verifier;

class PlainTextVerifier implements VerifierInterface
{
    public function verify($plaintext, $hashvalue, array $extra = array())
    {
        return $plaintext === $hashvalue;
    }
}

@enygma
Copy link
Contributor

enygma commented Oct 15, 2014

So you're saying not having a verifier for LDAP at all? Based on the others it kind of seems like that's the point (to abstract that out behind the generic verify method).

@harikt
Copy link
Member

harikt commented Oct 15, 2014

@enygma I was looking at http://php.net/manual/en/function.ldap-compare.php and it seems to me we need to do something like as shown in example. The above example I mentioned checking plain and hash one is wrong.

@enygma
Copy link
Contributor

enygma commented Oct 15, 2014

Yeah, the tricky part on that is whether or not the password is returned and how to set up that base DN information. Also, the compare will only work if there's a service account as was mentioned in the initial comment here. If you're using the ldap_bind method for testing the login, you wouldn't even be able to run the compare if the login was incorrect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants