sandstorm-io/meteor-accounts-sandstorm

Create a way to only process Sandstorm headers if Sandstorm environment variable is set

paulproteus opened this issue · 4 comments

paulproteus commented

Steps to reproduce:

  • Find some app that has this package installed, but is deployed on appname.meteor.com
  • Send the X-Sandstorm-User-Id header to the app along with other Sandstorm auth headers

Expected behavior:

  • Nothing, since the app is not on Sandstorm.

Actual behavior:

  • The app would create a new account, even though it's not running within Sandstorm, which is very likely not what the person using this Meteor package intended.

Suggested fix:

  • Adjust meteor-accounts-sandstorm so that it checks if (process.env.SANDSTORM === "1") before respecting Sandstorm headers.
  • Bump the major version number to indicate backwards incompatible change.

I can't think of another way to fail-closed, but happy to hear other options. @kentonv looking for your +1 or -1.

(NOTE: Earlier this said === 'Y' but that was a typo! I fixed the code above.)

kentonv commented

We set SANDSTORM to "1", not "Y", but otherwise, sounds good.

Perhaps we can also then enable "highlander" mode where we kill all the other accounts packages.

paulproteus commented

+1 to both of your remarks.

mitar commented

I have made a pull request for this in #20.

mitar commented

I think it is better to go by setting __meteor_runtime_config__.SANDSTORM to true when in Sandstorm based on the environment variable. I am updating my pull request so.

In this way settings is something users can configure, while fact that runtime runs inside Sandstorm is a constant.