trailsjs / sails-auth

Passport-based User Authentication system for sails.js applications. Designed to work well with the sails-permissions module.
https://www.npmjs.org/package/sails-auth
MIT License
266 stars 141 forks source link

LinkedIn Passport provider saved as null #154

Closed Garland220 closed 8 years ago

Garland220 commented 8 years ago

I'm using https://github.com/auth0/passport-linkedin-oauth2 LinkedIn passport plugin, and I ran into an issue where provider was being saved as null.

I fixed this by changing a few things in the passport service, but I want to know if this was a bug, or if I should have handled this some other way:

passport.connect = function (req, query, profile, next) {
  var user = { };

  // Use profile.provider or fallback to the query.provider if it is undefined
  // as is the case for OpenID, for example
  var provider = profile.provider || req.param('provider');

  req.session.tokens = query.tokens;

  // Get the authentication provider from the query.
  query.provider = provider;

I moved var provider up, and changed it to fallback to the request param, then set query.provider to the result of var provider. This meant that no matter what query.provider would be set to something. Previously it was undefined when it was being saved in the passport model.

Should I make a pull request for this small change, or was this unnecessary?

EDIT: I added a pull request

155