Skip to content

LDAP auth #212

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

LDAP auth #212

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 37 additions & 3 deletions index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,17 +308,51 @@ function fillSessionInfo() {
// Check that user/password is correct.
Copy link

Choose a reason for hiding this comment

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

Please update this comment, as now it doesn't only mean checking that the username and password are correct, but can mean verifying the LDAP login is valid. Suggested change:
"Check that the username/password combination or LDAP credentials are valid"

function check_auth($login,$password)
{
$hash = sha1($password.$login.$GLOBALS['salt']);
if ($login==$GLOBALS['login'] && $hash==$GLOBALS['hash'])
$success = False;
if ('LDAP'==$GLOBALS['config']['auth_backend'])
{
// use LDAP authentification.
Copy link

Choose a reason for hiding this comment

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

Typo: "authentification " -> "authentication"

// Needs global configuration;
// auth_backend = LDAP
// ldaphost: LDAP hostname
// ldapport: LDAP port
// ldaprdntpl: user RDN template (%login% replaced by actual login)
$success = check_auth_ldap($login, $password);
}
else
{
$hash = sha1($password.$login.$GLOBALS['salt']);
$success = ($login==$GLOBALS['login'] && $hash==$GLOBALS['hash']);
}
if ($success)
{ // Login/password is correct.
Copy link

Choose a reason for hiding this comment

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

Please update this line again, as now it doesn't only mean that the username and password are correct, but can mean that the LDAP login is valid. Suggested change:
"Username/password combination is valid, or LDAP authentication was successful"

fillSessionInfo();
fillSessionInfo();
logm('Login successful');
return True;
}
logm('Login failed for user '.$login);
return False;
}

// Check user/password with ldap server.
Copy link

Choose a reason for hiding this comment

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

Replace "user" by "username".
Replace "ldap server" by "LDAP server"

function check_auth_ldap($login,$password)
{
// check invalid login/password
Copy link

Choose a reason for hiding this comment

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

Replace "login" by "username".

if (empty($login) or empty($password) or preg_match('/[^a-zA-Z]/',$login)) {
Copy link

Choose a reason for hiding this comment

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

Just a general quesion (I'm not familiar with the LDAP specs): why the preg_match('/[^a-zA-Z]/',$login)?

logm('Invalid login or password');
return False;
}
$userrdn = str_replace("%login%", $login, $GLOBALS['config']['ldaprdntpl']);
$ldapconn = ldap_connect($GLOBALS['config']['ldaphost'], $GLOBALS['config']['ldapport']);
ldap_set_option($ldapconn, LDAP_OPT_PROTOCOL_VERSION, 3);
if ($ldapconn)
Copy link

Choose a reason for hiding this comment

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

Please rewrite the if/else statement like this:

    if {
        return (ldap_bind($ldapconn, $userrdn, $password));
    } else {
        logm('LDAP connection failed');
    }

{
return (ldap_bind($ldapconn, $userrdn, $password));
}
else logm('LDAP connection failed');
return False;
}

// Returns true if the user is logged in.
function isLoggedIn()
{
Expand Down