Open alxndrsn opened 5 years ago
@alxndrsn Thanks for posting! We'll take a look as soon as possible.
In the mean time, there are a few ways you can help speed things along:
Please remember: never post in a public forum if you believe you've found a genuine security vulnerability. Instead, disclose it responsibly.
For help with questions about Sails, click here.
@alxndrsn this totally make sense, maybe some kind of function that would run in this scenario and return the desired response body (leaving the headers & status code the same).
If anyone else looking at this issue has anything to add, open to more suggestions/feedback!
Although the framework has great power with its automation, you can provide a workaround with your own controller I presume.
@crh3675 this is true, but inconvenient if it has to be done in many controllers simultaneously.
Policies are processed before controllers, perhaps in there? You can apply a global policy across all controllers which might be another temporary solution
As far as I could work out, you can only have one policy per controller. Also sounds a bit hacky... maybe more intuitive would be an error-handling middleware which filters out the messages I don't like.
You can have multiple policies per controller - we do it all the time- just use an array instead. Or set a wildcard catchall.
module.exports.policies = {
// Default policy is to require authentication
'*': ['isAuthenticated'],
/**
* Main auth controller
*/
AuthController: {
'*': ['isPublic'],
'logout' : ['isPublic'],
'switch': ['isService', 'isAdmin'],
'change': ['isService'],
'validate': ['isPublic']
},
AdminController: ['isPublic'],
HealthController: ['isPublic'],
Not that with multiple policies, you need to enfore the usage of next in the polict
module.exports = function(req, res, next) {
if (//something) {
res.send('bah');
} else {
next();
}
}
You can have multiple policies per controller
@crh3675 this is true. I got confused, because I've tried in the past to enforce policies like "is admin OR manager", but this doesn't seem simple to achieve via policies alone - the combined policies such as
'some-endpoint': ['isAdmin', 'isManager'],
are a conjunction rather than disjunction, so I've ended up with policies like isAdminOrManager
. This gets tedious when there are many different combinations implemented.
@raqem I think there are workarounds implied, but not clearly defined. Options seem to be:
I'd guess that 2 is the less risky option, but I can't confirm that it either works or is reliable. I'll share info here when I've implemented a workaround.
@crh3675 going back to your original point:
Policies are processed before controllers, perhaps in there? You can apply a global policy across all controllers which might be another temporary solution
As there's no setting for machine
to disable these error messages, it's not clear to me how a policy would be able to help in this situation.
I guess it would be easier if the machine actually threw an error that was catchable. I understand a bit more after looking into the machine modularization. Looks like you might be able to create a custom api/responses/badRequest.js
with custom code and catch the E_MISSING_OR_INVALID_PARAMS error code . Seems to not be the most desirable but may provide a workaround.
I am finding a solution for this too, These messages can be leak our api params to outside.
Super simple fix in in responses/badRequest
if (_.isError(data)) {
if (data.code === 'E_MISSING_OR_INVALID_PARAMS') {
if (data.hasOwnProperty('problems')) {
delete data.problems;
}
}
@crh3675 neat. Do you mean in ./node_modules/sails/lib/hooks/responses/defaults/badRequest.js
?
When you create a new Sails application you should have a folder under api/responses of which you can change
When you create a new Sails application you should have a folder under api/responses of which you can change
No, sails will not create response folder automatically. You will need to create the folder also the customResponse file by yourself
So it seems that the solution is to copy /node_modules/sails/lib/hooks/responses/defaults/badRequest.js
into api/responses and customize. It seems my earlier version of Sails 0.12 did auto-create those but 1.0 does not.
Yeah, the solution I choose like this:
define exits property like this:
{
successCreate: {
statusCode: 201,
description: 'Success to create',
responseType: 'responseToClient',
},
invalidInputParam: {
statusCode: 400,
description: 'Bad param input',
responseType: 'responseToClient',
},
notAuthorize: {
statusCode: 401 ,
description: 'Not authorize to access',
responseType: 'responseToClient',
},
notUnique: {
statusCode: 409,
description: 'Not unique',
responseType: 'responseToClient',
},
}
exit.[ exit function name ]()
then the data will automatically send direct to responseToClient function. Just do whatever you want in that file
In production, when required param(s) are omitted from e.g. a POST request, machine's
E_MISSING_OR_INVALID_PARAMS
response will leak information about the API.E.g.:
This error message is generated in the
machine-as-action
module, and there does not appear to be an option to disable this.These detailed messages are very helpful in development, and likely if building a production API to be consumed by third parties.
However, for some projects they represent an unacceptable level of data leakage.
It would be great if there were an option to disable these detailed error messages in production, and instead return a straight
400
error with content simplyBad Request
(text/plain
).