Skip to content
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

Open
ndarilek opened this issue Jul 27, 2015 · 5 comments
Open

Add additional helper functionality #5

ndarilek opened this issue Jul 27, 2015 · 5 comments

Comments

@ndarilek
Copy link
Contributor

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:

  • A "user" subscription, subscribed to globally on startup, that pushes services.sandstorm.permissions of the current user to the client.
  • An isInSandstorm UI helper that returns true if services.sandstorm != null for the current user. This could be substituted for a more appropriate check if one were to exist.
  • A hasSandstormPermission UI helper that takes a single permission. Perhaps it could also take an array. If the user isn't in Sandstorm I return true. The idea is that hasSandstormPermission can be paired with any app-native RBAC, and that if the user isn't in Sandstorm, it should be a no-op. I want to build my app taking permissions into account, be able to test it thoroughly outside of Sandstorm, then load it into the dev environment near the end to ensure that the permissions work. While making this helper return false might seem more secure in a "deny by default" sense, I worry that it might cause developers to either not check permissions or to remove tests when developing, rather than leaving tests in and punting their implementation to Sandstorm. IOW, if this helper returned false by default, I might get annoyed at not being able to test a piece of functionality and remove the check, or I might forget to add one when building out a feature.

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.

@kentonv
Copy link
Member

kentonv commented Aug 1, 2015

These are good ideas and I agree they should me merged into this package.

For isInSandstorm, I think the best thing might be to make this part of Meteor.settings.public. We could easily pass that setting automatically in the Sandstorm packaging without affecting meteor dev mode.

@zarvox
Copy link

zarvox commented Aug 1, 2015

It'd be nice if hasSandstormPermission failed closed, but the ecological argument for it being minimally-painful is compelling. Maybe we could make hasSandstormPermission() take two arguments: the permission, and the default value to return if this app is not running under Sandstorm:

hasSandstormPermission("write", false)

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 true as the second argument and get the behavior you describe.

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.

@kentonv
Copy link
Member

kentonv commented Aug 1, 2015

It'd be nice if hasSandstormPermission failed closed

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.

@zarvox
Copy link

zarvox commented Aug 1, 2015

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. :)

@kentonv
Copy link
Member

kentonv commented Mar 3, 2016

The first point (the automatic subscription to the user info) is no longer necessary as of 0.2: the new Meteor.sandstormUser() method will return all Sandstorm info without any subscription.

The other two points still apply.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants