-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add additional helper functionality #5
Comments
These are good ideas and I agree they should me merged into this package. For |
It'd be nice if
That way, the default value to return if no second argument is given can still be false (fail closed), but the people who want the "no-op when not under Sandstorm" can add Another option could involve setting a member on the hasSandstormPermission helper for that "default value" up front, and then the test framework can set that to true to say "hey, get out of the way" in one place. |
Keep in mind that this is all client-side, so not security-sensitive. We're just deciding whether or not to display some part of the UI, but a malicious user could of course edit the code to make the UI appear, so the server still needs to validate method calls. |
Ahhh, good point. I assumed this helper function would be used both client and server-side, but upon rereading, I see "UI helper" was clearly stated. :) |
The first point (the automatic subscription to the user info) is no longer necessary as of 0.2: the new The other two points still apply. |
I know how to accomplish all of these tasks, but they seem basic enough that I think they should be rolled into this package. Here's what I've built for a couple Meteor apps that I'm hoping to launch on Sandstorm:
It seems to me that, with the above changes, a Meteor app could potentially be ported to Sandstorm in a way that could work in parallel with the existing app. Pieces of the UI could be selectively displayed/hidden with isInSandstorm, and permissions could either be synced with an existing roles implementation in code that could be kept separate, or checks could be and'd with hasMeteorPermission. If this isn't appropriate for the accounts package, it seems basic enough to warrant addition to a sandstorm-helpers package that is promoted as a way to bring Meteor apps to Sandstorm.
Thanks.
The text was updated successfully, but these errors were encountered: