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

Adding basic LDAP package #57

Closed
wants to merge 1 commit into from
Closed

Adding basic LDAP package #57

wants to merge 1 commit into from

Conversation

michael-golfi
Copy link

Attempt at integrating LDAP into your codebase.

Changes

  1. Added LDAP Package
  2. Added basic LDAP package tests (using a public ldap server)

        2. Added ldap tests
@michael-golfi
Copy link
Author

Hi @arekkas,

I would like to use this project, I need LDAP integration though. So I've taken a shot at writing it myself using the suggestions from #28.

I am however, not completely familiar with your codebase and I am unsure what you are expecting through the interface for a non-OAuth provider.

Please let me know,
Michael

@aeneasr
Copy link
Member

aeneasr commented Mar 9, 2016

Very cool! I will look over it in detail during the day (MEZ)

Gesendet mit meinem HTC

----- Reply message -----
Von: "Michael Golfi" notifications@github.com
An: "ory-am/hydra" hydra@noreply.github.com
Cc: "Aeneas" aeneas.rekkas@serlo.org
Betreff: [hydra] Adding basic LDAP package (#57)
Datum: Mi., Mär. 9, 2016 06:47

Hi @arekkas,

I would like to use this project, I need LDAP integration though. So I've taken a shot at writing it myself using the suggestions from #28.

I am however, not completely familiar with your codebase and I am unsure what you are expecting through the interface for a non-OAuth provider.

Please let me know,
Michael


Reply to this email directly or view it on GitHub:
#57 (comment)

func New(id, client, secret, redirectURL string) *ldapconf {
return &ldapconf{
id: id,
host: "ldap.forumsys.com",
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be hardcoded, so other people can use different LDAP servers.

@michael-golfi
Copy link
Author

This is more a proof of concept since I just want to know will this would work with your current codebase to integrate LDAP?

I'll tidy it up and submit another pull request with the config and security changes afterwards.

@michael-golfi
Copy link
Author

Right now from the tests, the LDAP querying works against the public server. However there is no redirect which takes place given this is not an oauth provider. Which is the part I need some clarification for.

This does return a 'user info' object though.

@aeneasr
Copy link
Member

aeneasr commented Mar 9, 2016

Sounds good :) As far as things are looking right now, this should work fine. So you found the right place for everythin! :)

Only thing I'm seeing is that

 func (l *ldapconf) GetAuthenticationURL(state string) string {
    return ""
 }

isn't currently used but is a hard-coded workflow in hydra. So I might have to fix a few things there. GetAuthenticationURL is currently used to redirect a user to the sepcified IdP (identity provider, for example google login). Not sure how that works with LDAP (never used LDAP before).

@aeneasr
Copy link
Member

aeneasr commented Mar 9, 2016

Did you take a look at this: https://auth0.com/docs/connections/enterprise/active-directory ? Would this be required for Hydra as well?

@aeneasr
Copy link
Member

aeneasr commented Mar 9, 2016

One more thing, could you maybe elaborate your use case so I get a better feeling of what you're trying to achieve? :)

@michael-golfi
Copy link
Author

Where I work we use Active Directory but we talk to it using LDAP. I think it eventually could be necessary to implement support for Active Directory in order to get access to some extra features implemented by Microsoft, but so far we have been fine just using LDAP alone (solely for authentication).

At the moment I'm not sure how I would add AD support to this provider. We use LDAP to manage Windows clients with SSO and store organizational data in it, like who reports to who, which groups or departments certain people belong in. So there is already a concept of groups, which could work well with the policies portion of this project. However, I don't think at the moment AD would be really beneficial to authentication alone.

Sample LDAP Workflow

To authenticate a user with LDAP, you use a system account to login in, you query the user subject who you would like to authenticate, you find his full path (eg: "cn=riemann,dc=example,dc=com"). Once that's done, you try to login using that user path and the submitted password. If it doesn't error out then the login is successful.

You could knock out the part of the workflow by setting a base DN as a search base for users and authenticate by having users provide their username and password and attempting the log in portion of the above workflow.

@aeneasr
Copy link
Member

aeneasr commented Mar 9, 2016

To authenticate a user with LDAP, you use a system account to login in, you query the user subject who you would like to authenticate, you find his full path (eg: "cn=riemann,dc=example,dc=com"). Once that's done, you try to login using that user path and the submitted password. If it doesn't error out then the login is successful.

Ok so what we could do would be:

  • hydra.yourorg.com: A running hydra instance
  • sso.yourorg.com: The bridge between hydra and LDAP. So basically "you use a system account to login in, you query the user subject who you would like to authenticate, you find his full path (eg: "cn=riemann,dc=example,dc=com")" plus a user interface
  • ldap.yourorg.com: The LDAP / AD server

Then we could add provider which redirects to sso.yourorg.com, then the authentication against LDAP happens (using custom queries or what-not) by providing a user interface where Peter (for example) enters his credentials and after successful authentication redirects Peter back to Hydra. Hydra then extracts the user info from a SAML Assertion or a signed JWT and issues access tokens accordingly.

Does this make sense? If yes, I have some ideas how we could make that work with Hydra and fix the currently broken login / consent workflow. So this is on my to do list anyhow :)

@aeneasr
Copy link
Member

aeneasr commented Mar 9, 2016

Oh and feel free to ask if I am unclear or context is missing or the message doesn't get through. Happy to answer any questions and also happy to see people that want to use Hydra!

@michael-golfi
Copy link
Author

I think your proposal makes sense but wouldn't starting a new sso server defeat the purpose of integrating LDAP? Would it make sense to launch some sort of authentication endpoint on Hydra?

@aeneasr
Copy link
Member

aeneasr commented Mar 9, 2016

I think your proposal makes sense but wouldn't starting a new sso server defeat the purpose of integrating LDAP?

Maybe SSO was the wrong wording. What I mean is a ldap frontend which offers an user interface (enter your username and password) and verifies the user input by querying the LDAP server. So it is not an SSO service per se, it just feels like one to the user who logs in and never notices that LDAP is being used.

Would it make sense to launch some sort of authentication endpoint on Hydra?

I want to avoid a real authentication endpoint on Hydra because it would require some sort of user interface - and I want to keep that away from Hydra as it is always hard to customize those and adds a lot of complexity. Right now, you can however use the Resource Owner Password Credentials grant which is specified in OAuth2 and allows OAuth2 clients to log users in using their username and password credentials. But that's a little bit off topic, so don't worry about this part too much ;)

@michael-golfi
Copy link
Author

Alright so in a nutshell:

This would require a separate auth server (using LDAP) that exposes an API to authenticate users given a username and password. This server would have a simple frontend that would accept the username and password and post to the backend.

Hydra would redirect users to this frontend, then the frontend would redirect the users back to hydra with some user info and then would take the user info and redirect to the 'final destination app'.

Do I understand this correctly?

@aeneasr
Copy link
Member

aeneasr commented Mar 9, 2016

Yes that's it exactly. I was planning on implementing that flow anyways, because there is no UI in hydra and we need a login interface :)

@aeneasr
Copy link
Member

aeneasr commented Mar 9, 2016

The above works by the way already for signing up users :)

@michael-golfi
Copy link
Author

I had started to do something similar to this several months ago using the osin oauth library. I had created a login page using the text/template library. It's a bit messy and I was in the process of changing the storage backend to Postgres (from MongoDb), it has a working LDAP signin. I don't think I have a lot of time to support it though between school and work. Feel free to check it out and grab the login interface if you'd like.

It's hosted here.

@aeneasr
Copy link
Member

aeneasr commented Mar 9, 2016

Cool I will check that out tomorrow!

Another option could be to implement an LDAP Adapter of the account storage. You still need an user interface though :)

@michael-golfi
Copy link
Author

That sounds like a good option too. Making a simple UI for login isn't such a big deal for me, I can add that if you'd like.

@michael-golfi
Copy link
Author

@arekkas

Regarding implementing an LDAP adapter for account storage, where should I start? Do you still want to continue this way?

EDIT: I was taking a look at Postgres Storage. Creating one that uses LDAP backend seems like a really nice way to proceed from a design perspective since this won't require the use of multiple account storage backends.

@aeneasr
Copy link
Member

aeneasr commented Mar 10, 2016

If you implement the store you still need an UI. Also hydra currently uses BCrypt for password hashing (mandatory) so there might be some trouble there as well. I think that a third party service is the faster and smarter way to go (and more flexible).

@aeneasr
Copy link
Member

aeneasr commented Mar 11, 2016

I had a little time to think about this, and I think we should take the road of letting users connect to multiple user databases. One could be Postgres, another LDAP, another MySQL (and so on). This is going to be some work because we would need to be able to

  • add & remove user databases with a command like hydra connect ldap://...
  • specifying hash options (bcrypt, sha, ...)
  • maybe do other custom stuff

or more abstract: the connectors should be able to handle common configuration set ups.

What do you think @michael-golfi

ping @leetal

@aeneasr
Copy link
Member

aeneasr commented Mar 11, 2016

Ok so the idea would be:

  • idp.yourdomain.org: implements a HTTP interface that allows for user management (create, login, ...). It is not important if the service uses LDAP or Postgres or what not.
  • login.yourdomain.org: a front for logging people in (user interface) and getting their consent (if oauth2 token is requested). Can use idp.yourdomain.org for user data lookup
  • hydra.yourdomain.org: uses idp.yourdomain.org to request user data for processing

@aeneasr
Copy link
Member

aeneasr commented Mar 11, 2016

Hydra would include packages (hydra-host, hydra-idp) and I would probably bootstrap an exemplary hydra-login front end using e.g. react.

@michael-golfi
Copy link
Author

I think using LDAP as a database is a good idea. IMO bigger companies will want to reuse their infrastructure and not recreate anything if possible. LDAP and Active Directory is also used as a primary data source for the most up to date user information. I cannot speak on behalf of having multiple data sources for user information since I have never personally seen this, except in the case when departments would want to enhance user data with custom permissions. They would have a local (eg: Postgres) and remote (eg: LDAP) in this scenario.

I've started building the LDAP connector. I was following the interface you have for storage as a 'contract' between the services, is that alright? For the frontend in this case I'm going to most likely use a cut-down bootstrap with minimal dependencies (this is already done).

Have you considered using a config manager such as viper to set up deployments with presets?

@aeneasr
Copy link
Member

aeneasr commented Mar 12, 2016

I think using LDAP as a database is a good idea. IMO bigger companies will want to reuse their infrastructure and not recreate anything if possible. LDAP and Active Directory is also used as a primary data source for the most up to date user information. I cannot speak on behalf of having multiple data sources for user information since I have never personally seen this, except in the case when departments would want to enhance user data with custom permissions. They would have a local (eg: Postgres) and remote (eg: LDAP) in this scenario.

Agreed! Just need to find a good way to tackle the different configuration scenarios. I've never used LDAP but from what I know the queries can look quite different depending on the use case (e.g. query against department, company, ...).

I've started building the LDAP connector. I was following the interface you have for storage as a 'contract' between the services, is that alright? For the frontend in this case I'm going to most likely use a cut-down bootstrap with minimal dependencies (this is already done).

Sounds good. I still have the concerns that I raised in the lines above. For implementation help you can also take a look at CoreOS/Dex LDAP connector.

Have you considered using a config manager such as viper to set up deployments with presets?

Not yet, but it looks like a really useful addition. I would really like to have an CLI which let's me do:

hydra account connection add ldap://...

For this, we would need to write a small configuration persistence utility and could use e.g. viper unmarshalling to retrieve it. Also the CLI needs refactoring.

I want to go a little bit more into detail with the IDP idea:

Let's say I want to connect to my enterprise LDAP and use Hydra for OAuth2 token issuance. I would add my LDAP connector endpoint to hydra:

hydra account connection add https://ldap-connector.yourdomain.org

The endpoint https://ldap-connector.yourdomain.org would implement an HTTP interface, for example:

GET /accounts - get all accounts
GET /accounts/<id> - get an account by id
GET /accounts?username=<..> - get an account by the username (username could be email as well)
...

Hydra could offer different exemplary (or default) implementations for this http interface, including hydra-connector-ldap, hydra-connector-postgres, and so on. This would be similar to the naming currently existing in the cli directory.

That way, we would separate concerns and let people easily extend or modify an existing connector without having to clone all of hydra. Connectors are also likely to receive less and smaller updates so keeping those maintained would be much easier.

What do you think of this approach? It would take time to refactor that stuff, also I'm working on implementing fosite in hydra as well.

@michael-golfi
Copy link
Author

Sounds good. I still have the concerns that I raised in the lines above. For implementation help you can also take a look at CoreOS/Dex LDAP connector.

Thanks I wasn't aware of this library I'll definitely take a closer look!

For this, we would need to write a small configuration persistence utility and could use e.g. viper unmarshalling to retrieve it. Also the CLI needs refactoring.

Kubernetes uses cobra (from the viper guys) for CLI, it works pretty well for them. I've never used it but looked really closely at it a while ago.

That way, we would separate concerns and let people easily extend or modify an existing connector without having to clone all of hydra. Connectors are also likely to receive less and smaller updates so keeping those maintained would be much easier.

I agree this would be a really nice way to separate concerns, and for others (myself included) to add their own connectors.

(Sorry I accidentally closed, I'm on mobile)

@aeneasr
Copy link
Member

aeneasr commented Mar 12, 2016

I agree this would be a really nice way to separate concerns, and add their own connectors.

Glad you like the approach :) It will take a little bit of time for me to prepare / implement those things however.

@aeneasr aeneasr mentioned this pull request Mar 22, 2016
30 tasks
@aeneasr
Copy link
Member

aeneasr commented Mar 22, 2016

The approach we discussed will be tackled with #62

@aeneasr aeneasr added the stale Feedback from one or more authors is required to proceed. label Mar 22, 2016
@michael-golfi
Copy link
Author

I'm going to work on this but I'm going to be a bit stalled by work and school so it might take me a bit. I'll try and contribute some time as I can.

@aeneasr
Copy link
Member

aeneasr commented Mar 22, 2016

No worries, it will take me some time to get the architecture where I want it to be.

@aeneasr
Copy link
Member

aeneasr commented May 14, 2016

Superseded by #62

@aeneasr aeneasr closed this May 14, 2016
@rdeusser
Copy link

rdeusser commented Jun 3, 2016

@arekkas Does an LDAP connector still need to be written?

@aeneasr
Copy link
Member

aeneasr commented Jun 3, 2016

@IAmTheMuffinMan hydra itself does not know connectors. Your login endpoint needs to be able to handle LDAP. Hydra authenticates users using JWT. Here's an example how this works in real-life.

There is currently no official LDAP login endpoint - so yes, a hydra compatible LDAP endpoints needs to be written :)

@aeneasr aeneasr removed the stale Feedback from one or more authors is required to proceed. label Jun 4, 2016
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.

3 participants