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

Async AuthorisationStrategy? #3

Open
avimar opened this issue Dec 2, 2012 · 12 comments
Open

Async AuthorisationStrategy? #3

avimar opened this issue Dec 2, 2012 · 12 comments

Comments

@avimar
Copy link

avimar commented Dec 2, 2012

I wasn't sure where else to ask this.
How do I do an async auth strategy?

e.g. I have a role of editing their own resources -- so I match the credentials to the resource edit request.

But this new case is I want to allow the owner of many sub-accounts to edit the resource. So in that case, I need to do a query to find the owner of that account. (But only if other role-checks fail)

(or maybe I can do this a different way...)

So, is there some way to do async in the AuthorisationStrategy? there's no next(); to not call like in connect middleware...

Thanks!

@ForbesLindesay
Copy link
Owner

At the moment this only supports synchronous strategies. The reason for this is that I wanted to support things like:

app.get('/home', function (req, res, next) {
  if (req.user.is('admin')) {
    res.render('/home/admin');
  } else {
    res.render('home/user');
  }
});

Which wouldn't work with an async API.

It would be cool to have a similar module that did support async, but I don't think it can sensibly be added to this module.

@avimar
Copy link
Author

avimar commented Dec 2, 2012

I see.

This is a bit of a hack.. but how about running a middleware before connect-roles that saves the required information into the req object (and then calls next()). I'll give that a try later today...

@ForbesLindesay
Copy link
Owner

Yes, I should've commented on that, because this inside the authenticationStrategies refers to the request object, you can load anything you need in advance and put it in the request object. That, for example, is how I tend to load the user itself from the database.

The only downside of doing that it that sometimes you'll be running un-necessary database queries, whereas async authentication strategies would let you run them only when needed.

@valerianrossigneux
Copy link

do you have any example of this middleware that would load data into the request object ?

@avimar
Copy link
Author

avimar commented Dec 11, 2013

It's just a middleware that gets run first, something like this:

function checkLogin(req, res, next, hash){
            //do some checks!
            req.isAuthenticated=true;
            req.user={
                isAuthenticated:true
                ,username:username
                };
                next();//run this once all the async things are finished.
            }

@ForbesLindesay
Copy link
Owner

The next version will also support an alternative option whereby all authorisation strategies are allowed to return Promises rather than true/false. This lets them be async, but also means that calls like req.userCan will return a Promise. To enable this, you do:

var ConnectRoles = require('connect-roles');
var roles = new ConnectRoles({async: true});

It's still totally un-tested and un-documented though, so I'm going to leave this open until that gets resolved.

@mike-grayhat
Copy link

It works, thank you!

@JorgeLGonzalez
Copy link

Any new thoughts on the promise-based solution for async authorisation strategy?
Works like a charm for me!

@dskrvk
Copy link

dskrvk commented Apr 15, 2017

Can this be closed now?

@mikeatlas
Copy link

mikeatlas commented Jul 3, 2017

Works for me too.

Note that it needs to only return Promises which either resolve(true) or resolve(false), but not to use reject(...):

permission.use('get stuff', req => (
  new Promise((resolve) => {
    if (userCanDoStuff(req)) { return resolve(true); }
    return resolve(false);
  })));

@mvolkmann
Copy link

Yes, works great for me also! Can the docs be changed to no longer say that the "async" option is experimental? My use case is to query a database to get the roles of the user which is why I needed the callback function of the "use" method to return a Promise. I would guess this is a common scenario. It would be great to show one example of an async function being passed to the "use" method in the docs.

@ForbesLindesay
Copy link
Owner

@mvolkmann Yes, if someone adds a test case to cover it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants