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 auth #212

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

LDAP auth #212

wants to merge 1 commit into from

Conversation

meutel
Copy link

@meutel meutel commented Nov 23, 2014

Adds simple LDAP authentification. Uses additional config:
auth_backend: use "LDAP" to active LDAP authentification
ldaphost: LDAP server hostname
ldapport: LDAP server port
ldaprdntpl: user RDN template (%login% replaced by actual login), example: uid=%login%,ou=people,dc=example,dc=org

$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"

@e2jk
Copy link

e2jk commented Nov 23, 2014

Hi meutel, thanks for the pull request.
At this moment we are waiting on @sebsauvage to become active again, and we are working on a temporary community fork where a number of fixes and improvements have already been merged. the idea is to merge the community fork back into Seb's main project, or switch the project structure to a community-based model. In any case, I'd encourage you to submit this pull request against that fork as well: https://github.com/shaarli/Shaarli

Looking through your pull request, I have made a couple of comments inline (in the Files Changed tab). It's the first time I'm using this Github functionality, let me know if you don't see my comments and I'll add them in this discussion. Looks like they are added automatically ;)

This functionality requires the presence of multiple configuration values. To prevent future issues being logged of the type "LDAP doesn't work" that would be due to the user not reading the source code closely enough (which we can't really expect) and missing one of the parameters, I suggest you add these parameters in the GUI configuration part. If you need an example, look at the changes made to tpl/configure.html and $GLOBALS in index.php in this pull request.

Thanks for your work on Shaarli.

@nicofrand
Copy link

Hi there !

Sorry to dig up this old pull request, but has any work been done to make this pull request on the community's repository (I could not find it)?

Would any of you know if this PR could be taken as a base or if the codebase changed too much and this PR's code is now stale?

Thanks in advance, I think it would be really nice to have this (in particular with the YunoHost package of Shaarli if you know YNH).

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.

None yet

3 participants