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

Clean up LDAP Connector #483

Merged
merged 1 commit into from
Jun 28, 2016
Merged

Clean up LDAP Connector #483

merged 1 commit into from
Jun 28, 2016

Conversation

ericchiang
Copy link
Contributor

@ericchiang ericchiang commented Jun 21, 2016

The first commit cleans up the LDAP connector a bit. A follow up one will add the groups implementation.

Cleanup includes:

  • Remove some unlikely to be used fields to help configurability.
    • Combined "serverHost" and "serverPort" into "host"
    • Remove "timeout" (just default to 30 seconds).
    • Remove "maxIdleConn" will add it back if users feel the need
      to control the number of cached connections.
    • Remove "trustedEmailProvider" (just always trust).
    • Remove "skipCertVerification" you can't make this connector
      ingore TLS errors.
  • Fix configs that don't search before bind (fixes LDAP connector doesn't work without searchBeforeAuth #481).
  • Add more examples to Documentation
  • Refactor LDAPPool Acquire() and Put() into a Do() function which
    always does the flow correctly.
  • Added more comments and renamed some functions.

Edit:

  • Moved methods on LDAPIdentityProvider to the LDAPConnector

@ericchiang ericchiang force-pushed the ldap-groups branch 5 times, most recently from b35c661 to 16b3ee0 Compare June 22, 2016 00:45
@ericchiang
Copy link
Contributor Author

Groups implementation added. Need to figure out how to have this work with refresh tokens (hopefully without too much refactoring).

@ericchiang ericchiang force-pushed the ldap-groups branch 2 times, most recently from b51a22c to c629cfa Compare June 23, 2016 17:18
@ericchiang ericchiang changed the title Add LDAP connector groups implementation Clean up LDAP Connector Jun 23, 2016
@ericchiang
Copy link
Contributor Author

This PR has been changed to just include the cleanups to the LDAP connector.

New docs here: https://github.com/ericchiang/dex/blob/c629cfaf1d6298fe0f532f80e48a4c852c86cdac/Documentation/connectors-configuration.md#ldap-connector

Ready for review.

@ericchiang
Copy link
Contributor Author

Woops, left some docs unfinished.

@sym3tri sym3tri self-assigned this Jun 23, 2016
@ericchiang
Copy link
Contributor Author

Ready for review

(&(objectClass=person)(uid=janedoe))
```

If the search finds an entry
Copy link

Choose a reason for hiding this comment

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

dangling sentence fragment

@sym3tri
Copy link

sym3tri commented Jun 28, 2016

found a few minor things, otherwise LGTM

@ericchiang
Copy link
Contributor Author

👍 updated

* Remove some unlikely to be used fields to help configurability.
  * Combined "serverHost" and "serverPort" into "host"
  * Remove "timeout" (just default to 30 seconds).
  * Remove "maxIdleConn" will add it back if users feel the need
    to control the number of cached connections.
  * Remove "trustedEmailProvider" (just always trust).
  * Remove "skipCertVerification" you can't make this connector
    ingore TLS errors.
* Fix configs that don't search before bind (previously broken).
* Add more examples to Documentation
* Refactor LDAPPool Acquire() and Put() into a Do() function which
  always does the flow correctly.
* Added more comments and renamed some functions.
* Moved methods on LDAPIdentityProvider to the LDAPConnector
@ericchiang ericchiang merged commit b5d2b7e into dexidp:master Jun 28, 2016
@ericchiang ericchiang deleted the ldap-groups branch November 22, 2016 20:06
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.

LDAP connector doesn't work without searchBeforeAuth
2 participants