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

Ldap fixes #1068

Closed
wants to merge 3 commits into from
Closed

Ldap fixes #1068

wants to merge 3 commits into from

Conversation

circulon
Copy link

Fixes multiple ldap authentication issues and allows use of a custom port

@Rayne
Copy link
Contributor

Rayne commented Sep 25, 2017

Hello @circulon, this it the wrong repository for contributing patches to the library itself. Please have a look at the fatfree-core repository instead.

@@ -122,7 +122,7 @@ protected function _ldap($id,$pw) {
ldap_set_option($dc,LDAP_OPT_REFERRALS,0) &&
ldap_bind($dc,$this->args['rdn'],$this->args['pw']) &&
($result=ldap_search($dc,$this->args['base_dn'],
$this->args['uid'].'='.$id)) &&
$this->args['uid'].'='.$id, array($this->args['uid']))) &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the short array notation.

@@ -127,7 +127,8 @@ protected function _ldap($id,$pw) {
($info=ldap_get_entries($dc,$result)) &&
@ldap_bind($dc,$info[0]['dn'],$pw) &&
@ldap_close($dc)) {
return $info[0][$this->args['uid']][0]==$id;
$uid = strtolower($this->args['uid']);
return (mb_strtolower($info[0][$uid][0])) == (mb_strtolower($id));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fat-Free Framework tries to keep the "dependencies" as minimal as possible. The pull request introduces a dependency for the mbstring extension.

For the record: The system variable ENCODING defines the mb_internal_encoding when the extension mbstring is available. mbstring is an optional dependency at the moment.

@docwyatt2001
Copy link

This is similar to the changes I was going to submit, with some minor differences. uid doesn't work for Active Directory, so I modified to either use uid if nothing is supplied (for backward compatibility purposes) or an array of attribute. This is also supposed to be best practice for LDAP requests.

It also allowed to check for multiple attributes for the matching values (i.e. user.name or [email protected]) which could be defined. Anything that returns more than 1 differing response, is classed as a failed auth by default.

I do like the port idea... I might pinch that and throw it into mine, before submitting the PR

@Rayne
Copy link
Contributor

Rayne commented Oct 16, 2017

For the record: The PR of @docwyatt2001 is here: f3-factory/fatfree-core#227 👍

@ikkez
Copy link
Collaborator

ikkez commented Oct 26, 2017

Issue f3-factory/fatfree-core#227 was merged and should fix this here as well. Please reopen if there's something left.

@ikkez ikkez closed this Oct 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants