Make sure anonymous user with proper permissions can access data

[asydorchuk]

The current implementation will throw exception when listing or accessing dashboards for the anonymous user that belongs to Public role and has proper permissions, because id and roles properties are not defined for flask AnonymousUserMixin.

[landscape-bot]

Code quality remained the same when pulling 33f04dd on asydorchuk:master into 77e4d4b on airbnb:master.

[coveralls]

Coverage increased (+0.4%) to 81.353% when pulling 33f04ddcbfffdd9f8358e65c3ae75f42280963bd on asydorchuk:master into 77e4d4b on airbnb:master.

[mistercrunch]

get_user_roles

[asydorchuk]

Done.

[mistercrunch]

This is great overall! I like the idea but I'm more concerned about testing public not having access to slices and dashboards. Maybe the tests should give access to only one dataset and confirm that only that dataset is accessible.

We may want to add a configuration flag PUBLIC_ROLE_LIKE_GAMMA (False by default) which would give the same access level to public as gamma does (adding missing the permissions only, not removing any), then it would be for admins to add specific datasets to the public role to allow for specific slices and dashboards.

[landscape-bot]

Code quality remained the same when pulling d33accb on asydorchuk:master into 77e4d4b on airbnb:master.

[mistercrunch]

Let me know if you want to take on PUBLIC_ROLE_LIKE_GAMMA as part of this PR, we could save it for another PR if you prefer.

[coveralls]

Coverage increased (+0.4%) to 81.353% when pulling d33accb66fe41d26d5f376c54088a18ca19eb45b on asydorchuk:master into 77e4d4b on airbnb:master.

[asydorchuk]

I agree about testing public not having access to the specific slices or dashboards. Going to add more tests.

Regarding PUBLIC_ROLE_LIKE_GAMMA flag, let's have it as a separate pull request.

[landscape-bot]

Code quality remained the same when pulling 8c00ab1 on asydorchuk:master into 77e4d4b on airbnb:master.

[coveralls]

Coverage increased (+0.4%) to 81.369% when pulling 8c00ab15c1a389c80878b057835081775261d23a on asydorchuk:master into 77e4d4b on airbnb:master.

[landscape-bot]

Code quality remained the same when pulling 2c602e7 on asydorchuk:master into 88c9516 on airbnb:master.

[coveralls]

Coverage increased (+0.4%) to 81.269% when pulling 2c602e7 on asydorchuk:master into 88c9516 on airbnb:master.

[asydorchuk]

@mistercrunch please have another look. In order to make the tests work, I needed to replace lazy_gettext with gettext method for flashing translations. The issue and behavior I was getting otherwise is described here: pallets/flask#812

[landscape-bot]

Code quality remained the same when pulling 31c072d on asydorchuk:master into 88c9516 on airbnb:master.

[coveralls]

Coverage increased (+0.5%) to 81.343% when pulling 31c072d on asydorchuk:master into 88c9516 on airbnb:master.

[mistercrunch]

Ok this is good! I'm planning on adding support for role definition as part of the configurations in the near future.

[asydorchuk]

Thanks! Config role definitions sounds like a great addition. We use puppet in our organization and that would simplify caravel setup on service machines. I am going to prepare a separate pull request for PUBLIC_ROLE_LIKE_GAMMA flag.