laravel / cashier-stripe

Laravel Cashier provides an expressive, fluent interface to Stripe's subscription billing services.
https://laravel.com/docs/billing
MIT License
2.37k stars 671 forks source link

Allow account ID to be specified (for Stripe Connect) #406

Closed martinbean closed 5 years ago

martinbean commented 7 years ago

I’m using Cashier to develop a hosted CMS. Customers can attach their Stripe account, and then sell event tickets and products via their website’s store.

To facilitate this, I need to charge the Stripe account of the customer rather than my own (platform’s) Stripe account. The request is detailed here: https://stripe.com/docs/recipes/store-builder#charge-connected

Looking at the charge() method in the Billable class, I can see the API key is sent as part of the second parameter to the \Stripe\Charge::create() method:

return StripeCharge::create($options, ['api_key' => $this->getStripeKey()]);

This array needs to be extended to optionally specify an account ID, like this:

return StripeCharge::create($options, [
    'api_key' => $this->getStripeKey(),
    'stripe_account' => 'acct_XXXXX',
]);

I’m not sure how this would look in Cashier’s API. Maybe it could be passed as part of the $options array?

$billable->charge(1000, [
    'stripe_account' => 'acct_XXXXX',
]);

Or add a third option to the charge() method that is passed:

$billable->charge(1000, $options, $headers = [
    'stripe_account' => 'acct_XXXXX',
]);
mlantz commented 7 years ago

I don't think this is needed, the $options look like they get parsed for certain keys such as stripe_account take a look at the Stripe\Util\RequestOptions (Its in the PHP API package stripe/stripe-php) parse method which is used on all calls for the API.

martinbean commented 7 years ago

@mlantz Ah, interesting. Thanks! I’ll give it a try and close this Issue if that is the case.

martinbean commented 7 years ago

@mlantz So I tried specifying stripe_account as a parameter like this:

$charge = $user->charge($amount, [
    'application_fee' => config('services.stripe.application_fee'),
    'source' => $request->input('stripe_token'),
    'stripe_account' => $stripe_account_id,
]);

But I just get the following error:

Received unknown parameter: stripe_account

Looking at that Stripe\Utils\RequestOptions class, it seems to check for the stripe_account key in the $options array, the same array that Cashier specifies the api_key option: https://github.com/stripe/stripe-php/blob/master/lib/Util/RequestOptions.php#L58.

Unfortunately, Cashier doesn’t provide the ability to manipulate this array, so I can’t add a stripe_account key to it.

mlantz commented 7 years ago

Hey @martinbean good catch, I'm cuirous though about full support for the stripe_account lines like this: https://github.com/laravel/cashier/blob/7.0/src/Billable.php#L265 would need to be tweaked as well to include the stripe_account with the api_token, but only include it if its present. thoughts?

martinbean commented 7 years ago

@mlantz I’m wondering if the account could be set in the similar way to the API key. Something like:

Cashier::setStripeAccount('acct_XXXXX');

Which would set a static property. Then in the other Cashier methods, the account could be passed if set:

return StripeCharge::create($options, [
    'api_key' => $this->getStripeKey(),
    'stripe_account' => $this->getStripeAccount(),
]);

I think passing null for the stripe_account key (i.e. if the setStripeAccount() method is never called) will just make Stripe ignore it.

mlantz commented 7 years ago

Can we confirm that? if the key is passed with null Stripe is clever enough to discard it. And I agree with API key style.

mlantz commented 7 years ago

Hey @martinbean just out of curiosity did you have a chance to look into this? I can do some digging this weekend.

lindyhopchris commented 7 years ago

I'm not sure about using in the same way as the API key i.e. Cashier::setStripeAccount('acct_XXXXX'); The API key is consistent per application as the API key is an application key. However the stripe_account key can vary on a per-charge basis. So it would seem sensible for it to be an option for a charge rather than something set statically.

For example, I have a multi-tenant application where a tenant can attach multiple Stripe accounts, and then there is logic at the point the charge is created as to which Stripe account will be used.

Surely this:

$billable->charge(1000, [
    'stripe_account' => 'acct_XXXXX',
]);

Can get translated to this:

return StripeCharge::create($options, [
    'api_key' => $this->getStripeKey(),
    'stripe_account' => 'acct_XXXXX',
]);

I.e. stripe_account is pulled out of $options and passed into the second parameter of StripeCharge::create()? That would be a sensible mapping of cashier options to how the Stripe library requires the parameters?

mlantz commented 7 years ago

Agreed - my only concern is that refunds, invoices etc will need this stripe_account as well which means further changes - I'm happy to put in a PR with this, I was just looking to confirm some stuff. I'll see what I can do tonight.

fwartner commented 7 years ago

Any updates on this?

kiwo12345 commented 7 years ago

I wonder the same, any updates?

mlantz commented 7 years ago

I've got a bunch on my plat at this minute but this is on my radar still and I'd like to move forward on it, but it'll be a bit yet before I can take it on. I'm happy to see others interested though @fwartner @kiwo12345

ahalimkara commented 6 years ago

I think there should be a way to be able to set api options, not only account id, to benefit form most of Stripe api. So I added setStripeApiOptions function to Billable class and changed static getStripeKey function to instance method as getStripeApiOptions (as I saw this function always called as instance method). You can see all changes at this commit.

Also added two new functions withParams, withOptions to SubscriptionBuilder class to be able to send custom parameters when creating subscriptions #410 So you might create subscriptions as:

$user->setStripeApiOptions(['stripe_account' => 'acct_id', 'api_key' => 'api_key'])
     ->newSubscription($plan->name, $plan->id)
     ->withParams(['application_fee_percent' => $APPLICATION_FEE_PERCENT])
     ->withOptions(['stripe_account' => $this->getStripeAccountId()])
     ->create($token);

It is a backward compatible change except getStripeKey function's signature is changed (but it can be leaved as old signature) Am I missing something? Or any side effects?

Btw, there is a composer package which i used for tests here.

zagreusinoz commented 6 years ago

Just checking in on this issue because I'm getting ready to develop my own application that would use this functionality. Is this something that we can look forward to soon or should I build it myself separately?

RTC1 commented 6 years ago

Did this one go anywhere?

Hesesses commented 6 years ago

Anyone implemented this?

MACscr commented 6 years ago

Anyone get this working? Would really like to know if cashier works with stripe connect.

keithbrink commented 6 years ago

Just added a PR with a possible implementation of this, let me know what you think: #519 .

driesvints commented 5 years ago

The PR for Stripe Connect functionality was rejected so closing this because atm we have no plans of supporting it.

martinbean commented 5 years ago

@driesvints Disappointing given it’s a genuine use case. I’ve had to roll my own implementation over using Cashier because of this limitation.

driesvints commented 5 years ago

@martinbean I understand. Unfortunately there's a lot on our plate and we have to make decisions on what we want to focus. Adding support for this also means us maintaining it. For a big feature like this which we don't use ourselves this means another large chunk of responsibility.

Cashier is open source software which means you are allowed to fork, extend and re-release it as you see fit. You are totally free to release your own version of Cashier with full support for this and can maintain yourself.

I'm keeping a list for me personally with stuff I want to add in Cashier perhaps one day and I'm putting this on the list. Who knows, perhaps it'll be added some day. Just not at this moment.

Thanks to everyone who requested this. Hope you all understand.

connecteev commented 4 years ago

@martinbean @driesvints just curious, it's been ~ 2 yrs since this was closed. Does cashier support Stripe connect now?

driesvints commented 4 years ago

No, sorry. Look, if this is really something so many people then feel free to fork the package and do a release which has Connect support. At the moment we don't want to put that maintenance burdon on ourselves.