search

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

Fix for userIsInRole helper: Check to handle when `userRoles` is null. #168

Closed shwaydogg closed 8 years ago

shwaydogg commented 8 years ago

Certain race conditions can cause userRoles to be null and an exception to be thrown. Even while wrapping with currentUser helper:

{{#if currentUser}} {{#if isInRole 'coach'}} ....

typeof(null) returns "object" so the check on the previous line is not enough to avoid a js exception: 'object' === typeof userRoles.

@alanning I'll be at Devshop tomorrow if there's anything to discuss :)

mitar commented 8 years ago

This is fixed in v2.0 branch with a different check: https://github.com/alanning/meteor-roles/blob/v2.0/roles/roles_common.js#L851

mitar commented 8 years ago

But thanks for reporting. Please do test v2.0 to see if it still does not work for you.

mitar commented 8 years ago

BTW, I am not sure which race conditions can make roles null? They can make it so that the field does not exist, or that it does. But not that it is null? Is this something specific to your code? Anyway, in v2.0 we have user.roles || [] which will take care of this as well.

shwaydogg commented 8 years ago

Thanks for the reply Mitar! Just realized it might end up being null do a transforms not catching up. I'm using the jagi:astronomy and it will set fields to null.

Adrian did mention you were moving to 2.0, but as this is a bugfix and not a feature additions it might still make sense to add in the 1.0 line for those who don't upgrade. Up to you guys of course.

mitar commented 8 years ago

You are right. Can you then do else if (userRoles && 'object' === typeof userRoles) { instead there?

mitar commented 8 years ago

Thanks.