Problem when using with Passport

[mrlucmorin]

I'm getting some undesired side effect when using connect-roles in conjuntion with Passport.

It seems that when connect-roles defines the user object in attachHelpers(), it causes passport to wrongly detect user as being authenticated.

Passport uses the following code to check for user authentication:

req.isAuthenticated = function() {
  var property = 'user';
  if (this._passport && this._passport.instance._userProperty) {
    property = this._passport.instance._userProperty;
  }

  return (this[property]) ? true : false;
};

So, checking for "this[property]" always return true after connect-roles has called attachHelpers():

function attachHelpers(req, obj) {
  var oldUser = req.user;
  obj.user = req.user || Object.create(defaultUser);
  if(oldUser){
    obj.user.isAuthenticated = true;
  }else{
    obj.user.isAuthenticated = false;
  }
  if(obj.user){
    obj.user.is = tester(req,'is');
    obj.user.can = tester(req,'can');
  }
}

Is there any way to get both libraries to collaborate ?

Thanks

[mrlucmorin]

Ok, I've just found out about the initialize options in Passport where you can specify the property name to be used instead of defaulting to "user":

app.use(passport.initialize({ userProperty: 'currentUser' }));

So now Passport will check the "currentUser" property instead of "user", which allows it to play nice with connect-roles. Too bad this is not properly documented.

I think it would be a good idea to add this option to connect-roles, it would allow it to play nice with other libaries as well.

Cheers

[mrlucmorin]

Upon further testing, it appears that changing the userProperty option in Passport is not enough to eliminate unwanted side effect.

Refering to my first post, and discussing the issue with the creator of Passport, I feel that the attachHelpers method should not define req.user if it doesn't exist.

If we look at it from a standpoint of authentication vs. authorization, it doesn't really make sense to create a dummy user if we're not really yet authenticated.

What I suggest is the addition of an option to Connect-roles that will allow users of an authentication library to short-circuit the creation of the "dummy" user.

For now I have modified attachHelpers as follows, and after testing it seems to work nice with the default values of Passport:

function attachHelpers(req, obj) {
if(obj.user){
obj.user.is = tester(req,'is');
obj.user.can = tester(req,'can');
}
}

What do you think of this issue ?

[facultymatt]

+1 i'm having the same issue.

[facultymatt]

I think I fixed this issue in my project by moving app.use(user) AFTER app.use(router) is called.

Not sure why but this seems to correct passport user / connect-roles user conflicts.

Please let me know if this works for your situation.

[akinsella]

Hi, I have same issue.

User should not be created if user is not authenticated. It is a requirement from passport-js as you said.
Maybe, a custom extension based on your code should be created for better with passport-js -> passport-roles !

Anyway, if you can remove use of user variable when not authenticated it would help for integration with passport-js :)

[tylor]

Same problem, trying to use with Passport. I tried using a modified attachHelpers like @mrlucmorin suggested:

function attachHelpers(req, obj) {
  if(obj.user){
    obj.user.is = tester(req,'is');
    obj.user.can = tester(req,'can');
  }
}

However, this means that is/can don't get exposed in templates. Something like this for the middleware seems to do the trick:

var exports = module.exports = function middleware(req, res, next) {
  if (req.user) {
    req.user.is = tester(req,'is');
    req.user.can = tester(req,'can');
    if (res.locals) {
      res.locals.user = req.user;
    }
  }
  next();
};

[YourDeveloperFriend]

+1 I have a similar problem.

[mrlucmorin]

@tylor those should still be attached to res.locals

If you look at https://github.com/ForbesLindesay/connect-roles/blob/master/index.js#L16-L17 you can see that attachHelpers is being called twice, once with the req object, and if res.locals is defines, then it gets called with it too

[jdiamond]

@mrlucmorin Isn't this only a problem when the connect-roles middleware runs before the passport middleware?

[mrlucmorin]

@jdiamond It could be, but I haven't played with this in more than a month now :-)

[ForbesLindesay]

This will be resolved in the next version