Meteor-Community-Packages / meteor-roles

Authorization package for Meteor, compatible with built-in accounts packages
http://meteor-community-packages.github.io/meteor-roles/
MIT License
921 stars 168 forks source link

Roles.getUsersInRoles security issue #92

Closed davidsyoung closed 9 years ago

davidsyoung commented 9 years ago

Hi,

Not sure if I'm entirely right with this or not so bare with me. Roles.getUsersInRoles, if used from the server side in a publish function returns user objects WITH the services field.

Maybe it would be a good idea to omit the services field in the method.

Thank you for a great package.

Dave

alanning commented 9 years ago

Hi Dave,

Thanks for the comment. I wouldn't classify this as a security issue since you still control what you publish. But I do think its a great suggestion to be able to limit the fields that are returned by the function call.

Probably the best way to do this is to pass an options parameter as the 3rd param to getUsersInRoles and check for an options.fields object that we can pass on to Mongo.

Quick question, are you using groups?

davidsyoung commented 9 years ago

I agree Adrian, hardly a security issue just thought it wouldn't be a bad idea.

Nope I'm not using groups! I like the options idea.

On Tue, 24 Mar 2015 03:15 Adrian Lanning notifications@github.com wrote:

Hi Dave,

Thanks for the comment. I wouldn't classify this as a security issue since you still control what you publish. But I do think its a great suggestion to be able to limit the fields that are returned by the function call.

Probably the best way to do this is to pass an options parameter as the 3rd param to getUsersInRoles http://alanning.github.io/meteor-roles/classes/Roles.html#method_getUsersInRole and check for an options.fields object that we can pass on to Mongo.

Quick question, are you using groups?

— Reply to this email directly or view it on GitHub https://github.com/alanning/meteor-roles/issues/92#issuecomment-85320820 .

alanning commented 9 years ago

OK, since you aren't using groups, be sure to create an index on the roles field so that your getUsersInRoles function call doesn't do a complete table scan.

davidsyoung commented 9 years ago

Awesome. Thanks for the advice.

On Tue, 24 Mar 2015 12:54 Adrian Lanning notifications@github.com wrote:

OK, since you aren't using groups, be sure to create an index on the roles field so that your getUsersInRoles function call doesn't do a complete table scan.

— Reply to this email directly or view it on GitHub https://github.com/alanning/meteor-roles/issues/92#issuecomment-85484500 .

alanning commented 9 years ago

Just released v1.2.14 which adds an options param to getUsersInRole. The options param is passed directly through to the collection call so you can now limit the fields returned.