integrallis / stripe_event

Stripe webhook integration for Rails applications.
https://rubygems.org/gems/stripe_event
MIT License
844 stars 104 forks source link

Stripe Connect #65

Closed MarkMurphy closed 8 years ago

MarkMurphy commented 8 years ago

With Stripe Connect you have to configure a separate endpoint (in the Stripe Dashboard Webhook Settings) to receive notifications related to your connected stripe accounts. Stripe Connect events themselves don't look any different than regular Account events and as such makes them indistinguishable from each other. I'd like to see some examples of how this library might be used to handle both Account events (webhook events on your own account) and Connect events (webhook events on connected accounts).

MarkMurphy commented 8 years ago

I'd be happy to submit a pull request for this if we can decide on a solution here. My initial thoughts would be to implement some sort of custom namespace option at the route level which would be passed to StripeEvent.instrument as a second argument eg.

module StripeEvent
  class WebhookController < ActionController::Base
    # ...

    def event
      StripeEvent.instrument(params, event_namespace)
      # If `event_namespace` was equal to "connect":
      # => stripe_event.connect.account.updated
      head :ok
    rescue StripeEvent::UnauthorizedError => e
      log_error(e)
      head :unauthorized
    end

   # ...
  end
end

Where or how event_namespace is provided is to be determined. It could be:

Example via Engine.routes

# stripe_event/config/routes.rb
StripeEvent::Engine.routes.draw do
  root to: 'webhook#event', via: :post

  # Set StripeEvent.webhook_namespaces in config/initializers/stripe_event.rb
  StripeEvent.webhook_namespaces.each do |namespace|
    post '/:namespace', to: 'webhook#event', constraints: { namespace: namespace }
  end

end

# config/initializers/stripe_event.rb
StripeEvent.configure do |events|
  events.webhook_namespaces = ["connect"]
  # events.register_namespace("connect")
  # ...
end
MarkMurphy commented 8 years ago

Another option for the built in routing solution might be to just add a connect route to the engine routes:

 # stripe_event/config/routes.rb
StripeEvent::Engine.routes.draw do
  root to: 'webhook#event', via: :post
  post '/connect', to: 'webhook#event', namespace: 'connect'
end
MarkMurphy commented 8 years ago

If you guys are looking for another maintainer I'd be happy to lend a hand.

rmm5t commented 8 years ago

I'd like to see some examples of how this library might be used to handle both Account events (webhook events on your own account) and Connect events (webhook events on connected accounts).

@MarkMurphy Sorry for the delayed response here, but distinguishing between the two is relatively straight forward.

As the Stripe API Event Object Docs detail:

When using Connect, you can also receive notifications of events that occur in connected accounts. For these events, there will be an additional user_id attribute in the received Event object.

While, I'm not a fan that Stripe calls this a user_id (instead of an account_id), you can use this param in your custom EventRetriever to determine if your webhooks are coming directly from your Account or via a Stripe Connected Account. You can even pass extra params to your subscribed events (note the new connected boolean param below).

class EventRetriever
  def call(params)
    api_key = find_api_key(params[:user_id])
    event = Stripe::Event.retrieve(params[:id], api_key)
    { type: event[:type], event: event, params: params, connected: params[:user_id].present? }
  end

  def find_api_key(account_id = nil)
    # lookup api_key for the account or return the main api_key if no account_id
  end
end

StripeEvent.event_retriever = EventRetriever.new

Something similar was discussed #51 and #30. Please give those issues a read too.

In other words, you would configure Stripe to send direct account and stripe connect webhooks to the same endpoint.

Maybe I'm not fully understanding the complexity of your particular use-case, but I don't think namespacing these two types of events apart is necessary.

I actually have an application that does almost exactly this; however, I do something slightly different. I actually have my main account eat it's own dog food and become a stripe-connected account on it's own application. i.e. Most events that I care about for the main account come in as stripe-connected webhooks anyway, so everything behaves the same. Of course, my use-cases allow for this quite nicely, and each application is different.

MarkMurphy commented 8 years ago

Oh nice, I didn't see the user_id parameter. Thanks

MarkMurphy commented 8 years ago

Probably worth adding a note and an example of this in the README.md

rmm5t commented 8 years ago

Probably worth adding a note and an example of this in the README.md

Which part? There is a note in the README about :user_id and a custom EventRetriever.

However, I'd be open to a pull-request to improve the README if you think clarification would help there.

MarkMurphy commented 8 years ago

Ah, right again. Yeah, I guess more specifically I was looking for a mention of Connect or Stripe Connect and because I was unaware of the user_id parameter I think just skipped over that section because I was already aware of the customizability of the event retriever method.

MarkMurphy commented 8 years ago

Our use case is a bit different. Our connected accounts are all managed accounts so no api keys needed, other than our own. So basically, this is what we're after:

class StripeEventRetriever
  def self.call(params)
    event = Stripe::Event.retrieve(params[:id])

    # Events that occur in connected accounts contain a `user_id` parameter.  
    if params[:user_id].present?
      # Munge the event type property so that we can listen for events specifcly 
      # from stripe connect. eg. type: "account.updated" # => type: "connect.account.updated"
      return event.merge(type: "connect.#{event[:type]}")
    end

    event
  end
end
MarkMurphy commented 8 years ago

I actually have an application that does almost exactly this; however, I do something slightly different. I actually have my main account eat it's own dog food and become a stripe-connected account on it's own application. i.e. Most events that I care about for the main account come in as stripe-connected webhooks anyway, so everything behaves the same. Of course, my use-cases allow for this quite nicely, and each application is different.

On second thought, I prefer that approach better. Thanks for the idea.

rmm5t commented 8 years ago

On second thought, I prefer that approach better. Thanks for the idea.

:beers: Sure thing.