Closed alexeyismirnov closed 10 years ago
That's awesome, thanks! I'll add a link to this in the docs.
I also plan to add some features to Synth to make this even easier :)
On Tue, Jun 17, 2014 at 2:19 AM, Alexey Smirnov notifications@github.com wrote:
Just noticed that "User authentication" section is missing in the tutorial.. So, I created a small example about this topic. Take a look at the following repo:
Authentication example for SynthJS https://github.com/alexeyismirnov/synth-auth-example.
Feel free to merge it if you like it :-)
— Reply to this email directly or view it on GitHub https://github.com/JonAbrams/synth/issues/39.
Thanks for the example @alexeyismirnov.
@JonAbrams, what are you plans to make this easier?
I am about to build a tiny resource sharing server with user/group restrictions. And I have decided to go with synth. So if I can anyway contribute to those plans it would be great!
Otherwise, I will just go with @alexeyismirnov approach for now.
My plan was to add a middleware mechanism to Synth that pays attention to config options that can be set on API endpoints.
Basically, you can choose to whitelist (or blacklist) various API endpoints to require authentication. If an endpoint is determined to require auth, then an attempt is made to call up the user from the db in the authentication middleware, using the supplied cookie, and attached to the request.
So far I've added the setting of the config variable functionality (see here and here for a couple simple examples), but I've yet to document it. After that, the next thing is to figure out an elegant way to declare middleware.
Am I on the right track? Would native middleware support be a good thing?
Ideas are welcome :)
I like the idea of the config element on the resource it self. That keeps it simple and clean.
In my case auth/non-auth is not enough i need more than one level of auth (admin and non-admin)
I am not quite sure how to implement the middleware part. But I really like the 'policies' concept from sails.js.
We could combine it with the config idea, i.e.:
exports.post = function (config) {
config.policies = ['isAuthenticated','isAdmin'];
return function (req, res) {
// create new product
return { success: true };
};
};
and then have policies defined in back/policies
. I think we could also separate this in a middleware module, I just don't know exactly how one would do that.
I think my idea for combining configs and middleware should handle your policy requirements just fine.
Ok good.
the next thing is to figure out an elegant way to declare middleware.
Here is my thoughts on it.
First thing would be where to put the middle-ware, I think it should reside in back/middleware
.
But how do we load the middleware? It would be great to do it automatically load it like the resources, but this might need a way to handle the ordering of middleware.
Should any middleware access the entire config object? I actually think that it would be nice to register a middleware component to a certain config member. Such that the middleware is only called if the resource/route has that config member.
Example:
back/resources/product/new.js
exports.post = function (config) {
config.auth = true;
config.validation = {
schema: {
name: {
required: true,
type: string
}
}
};
return function (req, res) {
// create new product
return { success: true };
};
};
back/middleware/auth/index.js (could also be back/middleware/auth.js )
// this is auth middleware
function auth (req, res, next) {
// config is attached to req
if (req.auth === true) {
// authenticate user
req.authenticated = true;
}
next();
}
module.exports = auth;
back/middleware/validation/index.js (could also be back/middleware/validation.js )
// this is validation middleware
function validate (req, res, next) {
var schema = req.validation.schema;
for (var item in schema) {
// validation code
}
next();
}
module.exports = validate;
Updated with more complex example
I agree that middleware should be declared in back/middleware
, and it should be done as 'automatically' as possible.
Should any middleware access the entire config object?
For simplicity, for now, I think the answer is yes.
Reworking your example a bit, with the assumption we can now use promises: (back/middleware/auth.js)
exports.auth = function (config, req, res, next) {
if (req.cookies.userToken) {
return getUser(req.cookies.userToken).then(function (user) {
if (!user && config.authRequired) throw "User not found"; // You could also put the throw in getUser
req.user = user;
});
} else if (config.authRequired) {
// authentication required, but no credentials given
throw "Authentication Required";
} else {
// No auth required, no credentials given, nothing to do!
next();
}
}
As for the ordering, how about if each middleware has a name, and can optionally define its dependencies, that are then injected before the usual req
, res
, and next
params?
e.g.
back/middleware/auth.js
:
exports.user = function (req, res, next) {
if (req.cookies.userToken) {
return getUser(req.cookies.userToken).then(function (user) {
if (user) {
return req.user = user;
} else {
res.clearCookie('userToken');
}
};
}
}
exports.authRequired = function (config, user, req, res, next) {
if (config.authRequired && !user) throw "Auth required";
next();
}
With this scheme, all public functions declared in back/middleware/**
are executed before a request is handled, but they're done in order for dependencies to be met.
It could even output the order for middleware using the synth routes
command.
I don't know how hard this would be to implement though, hopefully I can use a DI (Dependency Injection) library or something :)
I really like the idea of the dependency into to the middelware. But if we go there, why not add this directly to the request handlers?
On the other hand, I somehow feel that the dependency injection style, might be too complex from the synth user point of view. Not sure if this is the case though.
Regarding your example:
In the user
-middleware you return the user (and don't call next()
).
Whereas in the authRequired
you call next.
Maybe the dependency injection does not fit so well with the next
-style chaining.
If we go for the dependency injection style, we should probably not use next
in the synth-middleware.
But wrap all the synth-middleware in an express-middleware that call next once its all done.
Alternatively, we could something like this instead:
back/middleware/auth
:
exports.user = {
middleware: function (config, req, res, next) {
if (req.cookies.userToken) {
getUser(req.cookies.userToken).then(function (user) {
if (user) {
req.user = user;
} else {
res.clearCookie('userToken');
}
next();
};
}
} else {
next();
}
}
};
exports.authRequired = {
dependencies: 'user',
middleware: function (config, req, res, next) {
if (config.authRequired && !req.user) throw "Auth required";
next();
}
}
exports.isAdmin = {
dependencies: ['user','authRequired'],
middleware: function (config, req, res, next) {
if (config.adminRequired && !req.user.isAdmin) throw "User is not admin";
next();
}
}
This way it should be easy to resolve the list of dependencies on server start and give an error on unresolvable dependencies.
I really like the idea of the dependency into to the middelware. But if we go there, why not add this directly to the request handlers?
Ha, I thought the same thing shortly after writing my above post. Indeed it'd be awesome, for example:
exports.post = function (db, user, req, res) {
if (!user || !user.isAdmin()) throw new AuthRequired();
return db.insert({
user_id: user.id,
message: req.body.message
});
};
In the user-middleware you return the user (and don't call next()). Whereas in the authRequired you call next.
Agreed, next()
shouldn't be needed in a promise friendly framework like Synth. Either the middleware returns a promise, in which case that promise needs to be fulfilled before moving on, or it returns data that is then immediately passed on, just like in the endpoints.
If we go for the dependency injection style, we should probably not use next in the synth-middleware. But wrap all the synth-middleware in an express-middleware that call next once its all done.
Or perhaps do the opposite, register Express Middleware inside a Synth middleware, e.g.
back/middleware/express.js
var bodyParser = require('body-parser');
// `app` is a built-in middleware/service that sends the Express app
// `initial` is a special middleware/service name that always gets called once, on load, used for setting up non-Synth middleware
exports.initial = function (app) {
app.use(bodyParser.json());
app.use(cookieParser('my secret token'));
};
Alternatively, we could something like this instead This way it should be easy to resolve the list of dependencies on server start and give an error on unresolvable dependencies.
While it might be a bit easier to implement dependency support like that, it's more verbose, and therefore not very Synthy :)
I still like the idea of using Angular style dependency injection. Compared to your alternative, the Angular way also makes the result of the middleware calls available. See the user and db example above. At this point, it's not just a middleware, it's also a service!
Added bonus: Since this is all back-end, non-minified JS code, we don't have to worry about annotating the dependencies :smiley:
I really like where this is going, and I agree that my alternative was not very Synthy :)
I still like the idea of using Angular style dependency injection. Compared to your alternative, the Angular way also makes the result of the middleware calls available. See the user and db example above. At this point, it's not just a middleware, it's also a service!
Also I never really liked the term middleware, so we should maybe consider calling it services.
Or perhaps do the opposite, register Express Middleware inside a Synth middleware, e.g. I like this idea, but I'm not sure I like the idea of having
initial
as a special 'service', what ifapp
was a special dependency? Not sure this is any better...
Another benefit from this is that we don't need to call all middleware as in a standard express application, by having these dependencies it suffices to call the services a given endpoint is dependent on. Eg. we could have bodyParser
as a service, and only call it when we need it. One could probably think of a better example.
back/service/bodyParser.js
:
exports.bodyParser = require('bodyParser').json();
back/service/user.js
:
exports.user = function (bodyParser, req, res) {
if (req.cookies.userToken) {
return getUser(req.cookies.userToken).then(function (user) {
if (user) {
return user;
} else {
res.clearCookie('userToken');
}
};
}
return null;
}
Furthermore, I believe we can on server start compute the dependencies such that, we can execute the independend dependencies in parallel. I.e we should be able to generate the actual response handler as a promise chain(tree).
I just realized that we probably need to wrap express middleware in promises to make it "next-free"
back/service/bodyParser.js
:
exports.bodyParser = Promise.promisify(require('bodyParser').json());
How does this look?
back/services/bodyParser.js
var bodyParser = Promise.promisify(require('bodyParser').json());
exports.params = function (req, res) {
// Combines the three sources of parameters into one, and return them in one object
// Similar to how Rails and various other frameworks do it
return bodyParser(req, res).then(function () {
return _.merge({}, req.query, req.params, req.body);
});
};
back/resources/order/new.js
exports.post = function (params, user, db) {
return db.insert({
product: params.product_id, // from req.params
user_id: user.id,
shipping_instructions: params.shippingInstructions // from req.body
});
};
Look Ma, no req/res needed!
Also I never really liked the term middleware, so we should maybe consider calling it services.
I agree, especially if they're not automatically called for every endpoint, and only when needed. It would make it more clear to have different names.
Look Ma, no req/res needed!
Still laughing...
How does this look?
This is so cool! So, req
and res
are just two special dependencies that are fulfilled by express.
This also means that there is almost no difference between the definition of a service and an endpoint. The only difference is the way that the path and name are important for the route and http-type for endpoint.
Yup! req and res are just built-in services, like app.
Does this mean that 'config' isn't needed anymore?
This is very cool, hopefully I can get some time to implement it this weekend.
On Fri, Aug 1, 2014 at 11:41 AM, Mikael Møller notifications@github.com wrote:
Look Ma, no req/res needed!
Still laughing...
How does this look?
This is so cool! So, req and res are just two special dependencies that are fulfilled by express.
This also means that there is almost no difference between the definition of a service and an endpoint. The only difference is the way that the path and name are important for the route and http-type for endpoint.
— Reply to this email directly or view it on GitHub https://github.com/JonAbrams/synth/issues/39#issuecomment-50919994.
Does this mean that 'config' isn't needed anymore?
I think so, I cannot think of a case where we need that. The only thing I could think of is if we need to have two very similar services, that depends on certain config. But I cannot come up with something that would need that, where you could just do a parameterised function and then define two services.
This is very cool, hopefully I can get some time to implement it this weekend.
That sounds great. I don't have any time this weekend, but if you get something done I would be ready to play with it on monday.
I am really looking forward to this! Let know what I can do.
I looked into existing DI solutions and only found Angular's. It's not documented, seems to be rapidly changing, depends on ES6, and it doesn't appear to work the way I need it to (with Angular, all services are singletons, which doesn't fit the above use cases).
Therefore, I've created a new DI module for node called Synth-DI!
So far, not much code, it's mostly just a readme. Feel free to provide feedback on it over there.
So far, not much code, it's mostly just a readme. Feel free to provide feedback on it over there.
I see that this is no longer true, great! I will try to play around with it today. I thought we needed some kind of topological sort to do this, but may our use of promises will allow us to just reuse the result.
Again, GREAT work!
Yup, good old recursion and dynamic programming to the rescue, no sorting needed.
There's still a risk of cycles occurring but those will just raise a "call stack exceeded" error.
On Mon, Aug 4, 2014 at 12:59 PM, Mikael Møller notifications@github.com wrote:
So far, not much code, it's mostly just a readme. Feel free to provide feedback on it over there. I see that this is no longer true, great! I will try to play around with it today. I thought we needed some kind of topological sort to do this, but may our use of promises will allow us to just reuse the result.
Again, GREAT work!
Reply to this email directly or view it on GitHub: https://github.com/JonAbrams/synth/issues/39#issuecomment-51109463
Just noticed that "User authentication" section is missing in the tutorial.. So, I created a small example about this topic. Take a look at the following repo:
Authentication example for SynthJS.
Feel free to merge it if you like it :-)