tommybananas / finale

Create flexible REST endpoints and controllers from Sequelize models in your Express app
187 stars 36 forks source link

Authentication #17

Closed lfernando-silva closed 6 years ago

lfernando-silva commented 6 years ago

Hey, good work at this library. I'm using it and it works well.

About authentication, I realized the params of .auth hook are different of passport params, as I couldnt make

//myHooksAsMiddleware.js
module.exports = {
    create: {
          auth: myCustomPassportAuth
    },
    list: {},
    update: {},
    delete: {},
    read: {}
};

because the 3rd param of myCustomPassportAuth middleware is a next, but finale-rest .auth has a 3rd param that is an object that contains the next function. So I had to change to

//myHooksAsMiddleware.js
module.exports = {
    create: {
          auth: (req, res, context) => myCustomPassportAuth(req, res, context.continue)
    },
    list: {},
    update: {},
    delete: {},
    read: {}
};

Does have some different aproach to do this authentication?

tommybananas commented 6 years ago

At first glance I would probably take the passport auth out of the finale controller (before finale routes) and do the authentication check inside the auth milestone if possible.

 auth: (req, res, context) => req.user ? context.continue : context.stop

Also, if you're only passing in context.continue, how would you throw an error? The milestones really aren't set up to be middleware themselves, nor do they have a reason to be.

tommybananas commented 6 years ago

Here is an example taken straight out of the docs for using non-middleware passport to accomplish this. You could also just use the auth milestone.

users.list.fetch.before(function(req, res, context) {
    passport.authenticate('bearer', function(err, user, info) {
        if(err) {
            res.status(500);
            return context.stop();
        }

        if(user) {
            context.continue();
        } else {
            context.error(new ForbiddenError());
        }
    });
});
lfernando-silva commented 6 years ago

Ok, is a good way to handle the problem. I will follow this. Thanks!