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

Support multi tenant secrets #139

Open
christianrolle opened this issue Apr 7, 2021 · 2 comments
Open

Support multi tenant secrets #139

christianrolle opened this issue Apr 7, 2021 · 2 comments

Comments

@christianrolle
Copy link

christianrolle commented Apr 7, 2021

This chapter https://github.com/integrallis/stripe_event#configuration reads like if there is a possibility to configure secrets in a multi tenant environment.
However

return StripeEvent.signing_secrets if StripeEvent.signing_secret
and
attr_reader :signing_secrets

pretty much look like a static interface for the secret configuration.

Is such an interface like:

class StripeTenantSigningSecret
  def call(params)
    StripeAccount.for_tenant(params[:tenant]).pluck :signing_secret
  end
end
StripeEvent.signing_secret = StripeTenantSigningSecret.new

a valid user story for the gem?
Otherwise I see no other chance than overwriting the

@julienbourdeau
Copy link

I'm considering using the gem so I haven't tested it myself yet but from what I understand it depends what we call "multi tenant environment".

In the gem, you'll need to set ALL the signing keys in the configuration

StripeEvent.signing_secrets = [
  ENV['STRIPE_LEGACY_ACCOUNT_SIGNING_SECRET'],
  ENV['STRIPE_ACCOUNT_2_SIGNING_SECRET'],
  ENV['STRIPE_ACCOUNT_3_SIGNING_SECRET'],
]

The gem will test all secrets when receiving a webhook.

possible_secrets.each_with_index do |secret, i|
begin
return Stripe::Webhook.construct_event(payload, signature, secret.to_s)
rescue Stripe::SignatureVerificationError
raise if i == possible_secrets.length - 1
next
end
end

1️⃣ It's great if "multi tenant" means:

  • I use Stripe Connect because I'm a marketplace (what the readme shows)
  • We are one company but we have a few Stripe Accounts for Historical Reasons ™️

2️⃣ It's definitely not suitable if multi tenant means "I have thousands of users and they all use THEIR Stripe accounts". For performance for sure but it could even be a security issue 🤔

My understanding is that you need the second case but this gems only support the first one.

@n-at-han-k
Copy link

Should this be closed? I would assume that this is a non-standard practice.
Stripe provides connect accounts for multitenant applications.

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