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 additional directories options for LDAP Auth #1443

Merged

Conversation

zivis
Copy link
Contributor

@zivis zivis commented Apr 25, 2016

  • AuthLDAPURL as auth_ldap_url
  • AUTHLDAPBindDN as auth_ldap_bind_dn
  • AuthLDAPBindPassword as auth_ldap_bind_password

@hunner
Copy link
Contributor

hunner commented Apr 25, 2016

Thanks @zivis ! Could you also document these new directories values? There is a section in the readme for them.

@zivis zivis force-pushed the additional_AuthLDAP__directories_options branch from ef99153 to 1325843 Compare April 26, 2016 09:47
@zivis
Copy link
Contributor Author

zivis commented Apr 26, 2016

I've updated the README and squashed the update of the documentation into the commit.
Thanks for the hint

@antaflos
Copy link
Contributor

antaflos commented May 4, 2016

Oh, 👍 from me! Can you also add support for AuthLDAPGroupAttribute and AuthLDAPGroupAttributeIsDN while you're at it? That'd be awesome :)

* AuthLDAPURL as auth_ldap_url
* AUTHLDAPBindDN as auth_ldap_bind_dn
* AuthLDAPBindPassword as auth_ldap_bind_password
* AuthLDAPGroupAttribute as auth_ldap_group_attribute
* AuthLDAPGroupAttributeIsDN as auth_ldap_group_attribute_is_dn

add documentation in README for additional directories options LDAP Auth
@zivis zivis force-pushed the additional_AuthLDAP__directories_options branch from 1325843 to 00db1f9 Compare May 9, 2016 15:49
@zivis
Copy link
Contributor Author

zivis commented May 9, 2016

I also added support for AuthLDAPGroupAttribute and AuthLDAPGroupAttributeIsDN.

additional question:
I would like to fail the compilation when "AuthBasicProvider ldap" is used but the class ::apache:mod:authnz_ldap is not in catalogue. Should i do this in this PR/Commit or should i open a new one?

@antaflos
Copy link
Contributor

@zivis I think such a failsafe is a separate feature and would belong in a separate PR since it does not really pertain to the changes you introduced here.

@tphoney
Copy link
Contributor

tphoney commented Jun 17, 2016

@sathieu thanks for the work, i agree with @antaflos the extra functionality you were suggesting, belongs in a separate PR.

@tphoney tphoney merged commit 355702b into puppetlabs:master Jun 17, 2016
@sathieu
Copy link
Contributor

sathieu commented Jun 17, 2016

@tphoney you mean thanks @zivis?

@tphoney
Copy link
Contributor

tphoney commented Jun 17, 2016

@zivis apologies, thanks for your work.
@sathieu thanks anyway

@zivis zivis deleted the additional_AuthLDAP__directories_options branch August 5, 2016 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants