patrickpissurno / fastify-esso

The easiest authentication plugin for Fastify, with built-in support for single sign-on (SSO)
https://npm.im/fastify-esso
MIT License
52 stars 6 forks source link

fastify.requireAuthentication() #14

Closed tehla closed 1 year ago

tehla commented 2 years ago

https://github.com/patrickpissurno/fastify-esso/blob/6d4e04ac5ab0dcc32338947717d5698d86090b62/lib/index.js#L152

Hello

I'm a bit confused with the fastify plugin pattern (and with JS as well sometimes), but why do you need to inject the server instance into "requireAuthentication" ? You seem to already have this instance injected above : https://github.com/patrickpissurno/fastify-esso/blob/6d4e04ac5ab0dcc32338947717d5698d86090b62/lib/index.js#L98

So I'm not sure, but is there a way to call fastify.requireAuthentication() from a method defined with no param at all ?

tehla commented 2 years ago

Could the full example work like this ? (with auth route defined otherwhere the plugin registration )

// register our private routes
fastify.register(() => {
    fastify.requireAuthentication()
    fastify.get('/order-drink', async (req, reply) => {
        return { message: 'Hello ' + req.auth.user + '. Here is your drink!' };
    });
});
patrickpissurno commented 1 year ago

Honestly, I don't know if it would be possible to improve usability by not requiring you to pass fastify into the requireAuthentication function.

The reason you need to pass it right now is that even tho it appears that requireAuthentication is a method of fastify, it is currently not bound to it, so the this variable is not set, and thus if you don't pass it like that (again even tho it looks redundant), it wouldn't be possible to access fastify from inside it to do what needs to be done.

But from a practical perspective, don't worry about that. Just call fastify.requireAuthentication(fastify) and it will work fine.

Cheers!