deitch
commented
9 years ago This is interesting.
I have two concerns with this solution.
- What if the 2 items are not integers? IDs could be strings (I personally like using UUIDs or shortened versions of them, but to each their own).
- What if the
param()method you provided isn't available?
The second problem can be solved by making cansecurity entirely tested and functioning under restify. I would like to get that done.
The first is the more pertinent question. req.param() will always return a string, since that is what every framework treats query params and path parts as. Are you saying that when the validation is done, and it returns the user object, the ID field is an integer?
I like the idea of parseInt, but since there is no way for the framework to know if we are dealing with int or string, maybe the other way around? Instead of parseInt, we should convert both of them to strings, to be safe? After all, a string a = "abc"; a.toString(); will still give "abc", while a=25; a.toString(); will give "25", so it is safe.
socketwiz
When I use the api restrictToSelfOrRoles I end up here:
https://github.com/deitch/cansecurity/blob/master/lib/authorization.js#L20
and the comparison fails for me because on the left I have an integer and on the right I have a string. The string contains the integer value but since its not converted to an integer the test fails. I took the liberty of creating this PR to address it but this might be one of the differences between an express app and a restify app. For example, restify doesn't seem to have a param method on the req object. My first inclination was to fix this line such that if the param method was not available, fallback to the params (note the S) object which is available in restify. Then I came up with what I thought might be a better option, which was to simply add a param method to the restify req through middleware, so thats what I did. I basically "borrowed" the param method from express and glued it into restify:
https://gist.github.com/socketwiz/9c142302c1b8fc7fceb6
That leaves me with the problem I've addressed here. This PR fixes my problem, but I'm not sure what implications it would have on the project as a whole. Comments? Ideas?