strongloop / loopback-example-passport

LoopBack example for facebook login
Other
185 stars 134 forks source link

ldap configuration #12

Closed dennyabuti closed 10 years ago

dennyabuti commented 10 years ago

I am trying to configure passport for authentication with an ldap provider using passport-ldapauth "ldap": { "provider": "ldap", "authScheme":"ldap", "module": "passport-ldapauth", "authPath": "/auth/ldap", "server":{ "url": "ldap://server:10389/o=org,dc=domain,dc=com", "adminDn": "uid=admin,ou=system", "adminPassword": "pwd", "searchBase": "ou=users,o=org,dc=domain,dc=com", "searchFilter": "(cn={{username}})" } }

Everything seems to be working fine until the point where done(err, user, authInfo) is called inside loginCallback(req, done) defined in passport-configurator.js.

TypeError: undefined is not a function

Where is the done closure supposed to be defined?

Thanks for your help!

raymondfeng commented 10 years ago

It seems that LDAP strategy expects a callback function with (req, user, done). The req argument is provided if passReqToCallback is true. Can you check to make sure method signatures are matching?

dennyabuti commented 10 years ago

You are right!

The callback provided by the default auth strategy constructor was expecting 5 parameter whereas passport-ldapauth was calling it with only 3 parameters of which req was the first.

I modified passport-ldap auth strategy from return this.verify(req, user, verify.call(this)); to return this.verify(req, null, null, user, verify.call(this));

The actual implementation has other details but that was the missing nugget.

Thank you so much!

raymondfeng commented 10 years ago

I'm closing the issue now. Please reopen if any code change is required on this repo.

crandmck commented 10 years ago

@raymondfeng Should this be documented? If so, please reopen, change label to doc, and assign to me. I notice that providers.json.template doesn't have an LDAP entry.

mmt050 commented 9 years ago

I see in the code of passport-configurator that all strategies are expected to match the callback signature:

function (req, accessToken, refreshToken, profile, done) {

The passport-ldapauth strategy doesn't support this, and neither does passport-ldap. I checked and some of the existing strategies listed on passport's site use the following signature for their callbacks (local, openId):

 self.success(user, info);

or if they support request callbacks:

self._verify(req, result.email, done);

Who should make a modification to conform to the other? Loopback's passport-configurator, or should all libraries migrate to the oauth standard signature?

OwenBrotherwood commented 9 years ago

@crandmck @raymondfeng I think something should be stated in the docs as it does not seem that LDAP works "out of the box" despite it being mentioned in https://strongloop.com/strongblog/authenticate-loopback-google-facebook-github/

FYI Current Community work for LDAP, as far as I know:

Strongloop may be supporting LDAP for it's own customers: I am free wheeling on freely available code

Strongloop should probably deceide it's buisness needs concerning support of LDAP to allow MS-AD Authentication/Authorization or, as Microsoft has support for OAuth, deceide that until a customer needs it, to not support LDAP to allow MS-AD. As for other use cases with OpenLDAP, again Strongloop needs to deceide: quite often, if one can get OpenLDAP running, one can also get an OAuth running. The problem is that company policies and availability of OAuth may not allow this route for possible customers.

The comment about signature is "important": I am no node programmer/ passport exert, but I could se some hurdles. Current passport strategy is based on using passport-ldapauth, I gave up on passport-windowsauth (I am not a node programmer)

crandmck commented 9 years ago

@raymondfeng This seems reasonable to me. Please confirm if you think I should update the docs as stated above.

OwenBrotherwood commented 9 years ago

@crandmck LDAP has been merged LDAP https://github.com/strongloop/loopback-component-passport/commit/ae998bba2fd8205d3df7dd9f46de25d73061d588

Example of how to use LDAP MS-AD: May be needed. The merge allows the possibility for using: let us see if there comes an issue from the users. Prepare use cases for the use of internal authentication/authorization where loopback is used where the focus is not on creation of app/api/gui but on the centralised (security department) type of aproach for role management.

Signarture of passport: I have another use case that allows me to investigate signature and if it is possible to use Microsoft centric method(s) for passport. It could be important to ensure Microsoft's passport solution as a Strongloop strategic/archetectural priority https://github.com/AzureAD/passport-azure-ad

crandmck commented 9 years ago

Thanks @OwenBrotherwood I would like to add this to the docs, but I really need and example of how to configure it; something along the lines of what we have now for FaceBook and Google.

Can you provide such an example?

OwenBrotherwood commented 9 years ago

@crandmck Pierre has tested for LDAP OpenLDAP : config https://github.com/strongloop/loopback-component-passport/pull/44#issuecomment-91611286

I will provide a config for LDAP MS AD (Microsoft Active Directory) that shows general configuration type values with some "getting started" comments.

OwenBrotherwood commented 9 years ago

@crandmck The doc should indicate some form of early support

https://github.com/strongloop/loopback-component-passport/issues/58 The callback works in the context ms-ad LDAP "works for me"

"ms-ad": {
    "provider": "ms-ad",
    "authScheme":"ldap",
    "module": "passport-ldapauth",
    "authPath": "/auth/msad",
    "successRedirect": "/auth/account",
    "failureRedirect": "/msad",
    "failureFlash": true,
    "session": true,
    "LdapAttributeForLogin": "mail",
    "LdapAttributeForUsername": "mail",
    "LdapAttributeForMail": "mail",
    "server":{
      "url": "ldap://ldap.example.org:389/dc=example,dc=org",
      "bindDn": "bindUsername",
      "bindCredentials": "bindPassword",
      "searchBase": "ou=people,dc=example,dc=org",
      "searchAttributes": ["cn", "mail", "uid", "givenname"],
      "searchFilter": "(&(objectcategory=person)(objectclass=user)(|(samaccountname={{username}})(mail={{username}})))"
    }
  },
``
crandmck commented 9 years ago

OK, I added this to http://docs.strongloop.com/pages/viewpage.action?pageId=3836277#Third-partylogin(Passport)-Configuringthird-partyproviders. Please take a look.

OwenBrotherwood commented 9 years ago

@crandmck Looks good. Let us see what issues there are in order to pin down key problems for doc.

My current workload is:

OwenBrotherwood commented 9 years ago

Comments on LDAP MS-AD (for anyone finding this pull request and semi-doc)

OwenBrotherwood commented 9 years ago

Comments on passport to an alternative to accessing MS-AD instead of via LDAP: ADSF (for anyone finding this pull request and is wondering how to integrate to MS-AD It is hoped to show this at some time, but currently causing me headaches https://github.com/AzureAD/passport-azure-ad/issues/25