From ae2d3d3fd92e29b0f663f7a5332a5bd4a4338e03 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 7 Jul 2017 11:48:57 +1000
Subject: [PATCH] Refactoring StripeHelper into service objects

---
 .../admin/enterprises_controller.rb           |  48 ++-----
 .../admin/stripe_accounts_controller.rb       |  33 +++--
 app/controllers/checkout_controller.rb        |  22 +--
 app/helpers/admin/stripe_helper.rb            |  75 ----------
 app/models/spree/ability_decorator.rb         |   4 +
 app/models/spree/gateway/stripe_connect.rb    |  19 +--
 app/models/stripe_account.rb                  |  11 ++
 lib/stripe/account_connector.rb               |  43 ++++++
 lib/stripe/oauth.rb                           |  51 +++++++
 .../admin/enterprises_controller_spec.rb      | 114 ++++++++--------
 .../admin/stripe_accounts_controller_spec.rb  | 129 +++++++++++++-----
 spec/helpers/admin/stripe_helper_spec.rb      |  52 -------
 spec/lib/stripe/account_connector_spec.rb     |  77 +++++++++++
 spec/lib/stripe/oauth_spec.rb                 |  25 ++++
 .../spree/gateway/stripe_connect_spec.rb      |   1 +
 spec/models/stripe_account_spec.rb            |  47 +++++++
 16 files changed, 463 insertions(+), 288 deletions(-)
 delete mode 100644 app/helpers/admin/stripe_helper.rb
 create mode 100644 lib/stripe/account_connector.rb
 create mode 100644 lib/stripe/oauth.rb
 delete mode 100644 spec/helpers/admin/stripe_helper_spec.rb
 create mode 100644 spec/lib/stripe/account_connector_spec.rb
 create mode 100644 spec/lib/stripe/oauth_spec.rb
 create mode 100644 spec/models/stripe_account_spec.rb

diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb
index 672e1ce178..6d548cc602 100644
--- a/app/controllers/admin/enterprises_controller.rb
+++ b/app/controllers/admin/enterprises_controller.rb
@@ -1,4 +1,6 @@
 require 'open_food_network/referer_parser'
+require 'stripe/account_connector'
+require 'stripe/oauth'
 
 module Admin
   class EnterprisesController < ResourceController
@@ -23,7 +25,6 @@ module Admin
     helper 'spree/products'
     include ActionView::Helpers::TextHelper
     include OrderCyclesHelper
-    include Admin::StripeHelper
 
     def index
       respond_to do |format|
@@ -115,48 +116,19 @@ module Admin
     end
 
     def stripe_connect
-      redirect_to authorize_stripe(params[:enterprise_id])
+      redirect_to Stripe::OAuth.authorize_url(params[:enterprise_id])
     end
 
     def stripe_connect_callback
-      if params["code"]
-        state = jwt_decode(params["state"])
-        unless state.keys.include? "enterprise_id"
-          redirect_to '/unauthorized' and return
-        end
-
-        # Get the Enterprise
-        @enterprise = Enterprise.find_by_permalink(state["enterprise_id"])
-        # Get the deets from Stripe
-        response_params = get_stripe_token(params["code"]).params
-
-        # In case of a problem, need to also issue a request to disconnect the account from Stripe
-        if !(spree_current_user.enterprises.include? @enterprise) && !(spree_current_user.admin?)
-          deauthorize_request_for_stripe_id(response_params["stripe_user_id"])
-          redirect_to '/unauthorized' and return
-        end
-
-        stripe_account = StripeAccount.new(stripe_user_id: response_params["stripe_user_id"], stripe_publishable_key: response_params["stripe_publishable_key"], enterprise: @enterprise)
-        if stripe_account.save
-          respond_to do |format|
-            format.html { redirect_to main_app.edit_admin_enterprise_path(@enterprise), notice: "Stripe account connected successfully."}
-            format.json { render json: stripe_account }
-          end
-        else
-          render text: "Failed to save Stripe token", status: 500
-        end
+      connector = Stripe::AccountConnector.new(spree_current_user, params)
+      if connector.create_account
+        flash[:success] = t('admin.controllers.enterprises.stripe_connect_success')
+        redirect_to main_app.edit_admin_enterprise_path(@enterprise)
       else
-        render text: params["error_description"], status: 500
-      end
-    end
-
-    def stripe_disconnect
-      if deauthorize_stripe(params[:account_id])
-        respond_to do |format|
-          format.html { redirect_to main_app.edit_admin_enterprise_path(@enterprise), notice: "Stripe account disconnected."}
-          format.json { render json: "Disconnected" }
-        end
+        render text: t('admin.controllers.enterprises.stripe_connect_fail'), status: 500
       end
+    rescue Stripe::StripeError => e
+      render text: e.message, status: 500
     end
 
     protected
diff --git a/app/controllers/admin/stripe_accounts_controller.rb b/app/controllers/admin/stripe_accounts_controller.rb
index c63ba934c8..e5325e917a 100644
--- a/app/controllers/admin/stripe_accounts_controller.rb
+++ b/app/controllers/admin/stripe_accounts_controller.rb
@@ -1,30 +1,33 @@
 module Admin
   class StripeAccountsController < BaseController
-    include Admin::StripeHelper
     protect_from_forgery except: :destroy_from_webhook
 
     def destroy
-      if deauthorize_stripe(params[:id])
-        respond_to do |format|
-          format.html { redirect_to main_app.edit_admin_enterprise_path(params[:enterprise_id]), notice: "Stripe account disconnected."}
-          format.json { render json: stripe_account }
-        end
+      stripe_account = StripeAccount.find(params[:id])
+      authorize! :destroy, stripe_account
+
+      if stripe_account.deauthorize_and_destroy
+        flash[:success] = "Stripe account disconnected."
       else
-        respond_to do |format|
-          format.html { redirect_to main_app.edit_admin_enterprise_path(params[:enterprise_id]), notice: "Failed to disconnect Stripe."}
-          format.json { render json: stripe_account }
-        end
+        flash[:error] = "Failed to disconnect Stripe."
       end
+
+      redirect_to main_app.edit_admin_enterprise_path(stripe_account.enterprise)
+    rescue ActiveRecord::RecordNotFound
+      flash[:error] = "Failed to disconnect Stripe."
+      redirect_to spree.admin_path
     end
 
     def destroy_from_webhook
-      # Fetch the event again direct from stripe for extra security
-      event = fetch_event_from_stripe(request)
-      if event.type == "account.application.deauthorized"
-        StripeAccount.where(stripe_user_id: event.user_id).map{ |account| account.destroy }
+      # TODO is there a sensible way to confirm this webhook call is actually from Stripe?
+      event = Stripe::Event.construct_from(params)
+      return render nothing: true, status: 400 unless event.type == "account.application.deauthorized"
+
+      destroyed = StripeAccount.where(stripe_user_id: event.user_id).destroy_all
+      if destroyed.any?
         render text: "Account #{event.user_id} deauthorized", status: 200
       else
-        render json: nil, status: 501
+        render nothing: true, status: 400
       end
     end
 
diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb
index 0233e2c383..2f2d5a64ee 100644
--- a/app/controllers/checkout_controller.rb
+++ b/app/controllers/checkout_controller.rb
@@ -208,19 +208,19 @@ class CheckoutController < Spree::CheckoutController
 
   def construct_saved_card_attributes
     existing_card_id = params[:order].delete(:existing_card)
-    if existing_card_id.present?
-      credit_card = Spree::CreditCard.find(existing_card_id)
-      if credit_card.try(:user_id).blank? || credit_card.user_id != spree_current_user.try(:id)
-        raise Spree::Core::GatewayError.new I18n.t(:invalid_credit_card)
-      end
+    return if existing_card_id.blank?
 
-      # Not currently supported but maybe we should add it...?
-      credit_card.verification_value = params[:cvc_confirm] if params[:cvc_confirm].present?
-
-      params[:order][:payments_attributes].first[:source] = credit_card
-      params[:order][:payments_attributes].first[:payment_method_id] = credit_card.payment_method_id
-      params[:order][:payments_attributes].first.delete :source_attributes
+    credit_card = Spree::CreditCard.find(existing_card_id)
+    if credit_card.try(:user_id).blank? || credit_card.user_id != spree_current_user.try(:id)
+      raise Spree::Core::GatewayError, I18n.t(:invalid_credit_card)
     end
+
+    # Not currently supported but maybe we should add it...?
+    credit_card.verification_value = params[:cvc_confirm] if params[:cvc_confirm].present?
+
+    params[:order][:payments_attributes].first[:source] = credit_card
+    params[:order][:payments_attributes].first[:payment_method_id] = credit_card.payment_method_id
+    params[:order][:payments_attributes].first.delete :source_attributes
   end
 
   def rescue_from_spree_gateway_error(error)
diff --git a/app/helpers/admin/stripe_helper.rb b/app/helpers/admin/stripe_helper.rb
deleted file mode 100644
index 4fdc16355c..0000000000
--- a/app/helpers/admin/stripe_helper.rb
+++ /dev/null
@@ -1,75 +0,0 @@
-# require File.join(Rails.root, '/lib/oauth2/strategy/deauthorize')
-# require File.join(Rails.root, '/lib/oauth2/client')
-# require 'oauth2'
-module Admin
-  module StripeHelper
-
-    class << self
-      attr_accessor :client, :options
-    end
-
-    @options = {
-      :site => 'https://connect.stripe.com',
-      :authorize_url => '/oauth/authorize',
-      :deauthorize_url => '/oauth/deauthorize',
-      :token_url => '/oauth/token'
-    }
-
-    @client = OAuth2::Client.new(
-      ENV['STRIPE_CLIENT_ID'],
-      ENV['STRIPE_INSTANCE_SECRET_KEY'],
-      options
-    )
-
-    def get_stripe_token(code, options={})
-      StripeHelper.client.auth_code.get_token(code, options)
-    end
-
-    def authorize_stripe(enterprise_id, options={})
-      options = options.merge({enterprise_id: enterprise_id})
-      jwt = jwt_encode options
-      # State param will be passed back after auth
-      StripeHelper.client.auth_code.authorize_url(state: jwt, scope: 'read_write')
-    end
-
-    def deauthorize_stripe(account_id)
-      stripe_account = StripeAccount.find(account_id)
-      if stripe_account
-        # If the account is only connected to one Enterprise, make a request to remove it on the Stripe side
-        if StripeAccount.where(stripe_user_id: stripe_account.stripe_user_id).size == 1
-          response = deauthorize_request_for_stripe_id(stripe_account.stripe_user_id)
-          if response # Response from OAuth2 only returned if successful
-            stripe_account.destroy
-          end
-        else
-          stripe_account.destroy
-        end
-      end
-    end
-
-    def fetch_event_from_stripe(request)
-      event_json = JSON.parse(request.body.read)
-      # If the application has been deauthorised, we are no longer authorised to retrieve events for that account
-      # Left here in case it's useful for other webhooks
-      unless event_json["type"] == "account.application.deauthorized"
-        acct_param = event_json["user_id"] ? {"Stripe-Account" => event_json["user_id"]} : nil
-        Stripe::Event.retrieve(event_json["id"],acct_param)
-      else
-        Stripe::Event.construct_from(event_json)
-      end
-    end
-
-    def deauthorize_request_for_stripe_id(id)
-      StripeHelper.client.deauthorize(id).deauthorize_request
-    end
-
-    private
-    def jwt_encode payload
-      JWT.encode(payload, Openfoodnetwork::Application.config.secret_token, 'HS256')
-    end
-
-    def jwt_decode token
-      JWT.decode(token, Openfoodnetwork::Application.config.secret_token, true, algorithm: 'HS256')[0] # only returns the original payload
-    end
-  end
-end
diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb
index a56f233434..78c994f8a7 100644
--- a/app/models/spree/ability_decorator.rb
+++ b/app/models/spree/ability_decorator.rb
@@ -121,6 +121,10 @@ class AbilityDecorator
 
     # Not sure how to restrict these methods to a non-resource Controller
     can [:stripe_connect, :stripe_connect_callback, :stripe_disconnect], :all
+
+    can [:destroy], StripeAccount do |stripe_account|
+      user.enterprises.include? stripe_account.enterprise
+    end
   end
 
   def add_product_management_abilities(user)
diff --git a/app/models/spree/gateway/stripe_connect.rb b/app/models/spree/gateway/stripe_connect.rb
index 89318142f7..66e0e88cf5 100644
--- a/app/models/spree/gateway/stripe_connect.rb
+++ b/app/models/spree/gateway/stripe_connect.rb
@@ -123,16 +123,17 @@ module Spree
     end
 
     def token_from_card_profile_ids(creditcard)
-      if token_or_card_id = creditcard.gateway_payment_profile_id
-        if customer = creditcard.gateway_customer_profile_id
-          # Assume the gateway_payment_profile_id is a Stripe card_id
-          # So generate a new token, using the customer_id and card_id
-          return tokenize_instance_customer_card(customer, token_or_card_id)
-        end
+      token_or_card_id = creditcard.gateway_payment_profile_id
+      customer = creditcard.gateway_customer_profile_id
 
-        # Assume the gateway_payment_profile_id is a token generated by StripeJS
-        return token_or_card_id
-      end
+      return nil if token_or_card_id.blank?
+
+      # Assume the gateway_payment_profile_id is a token generated by StripeJS
+      return token_or_card_id if customer.blank?
+
+      # Assume the gateway_payment_profile_id is a Stripe card_id
+      # So generate a new token, using the customer_id and card_id
+      tokenize_instance_customer_card(customer, token_or_card_id)
     end
 
     def tokenize_instance_customer_card(customer, card)
diff --git a/app/models/stripe_account.rb b/app/models/stripe_account.rb
index dd263a6b37..8c727533f0 100644
--- a/app/models/stripe_account.rb
+++ b/app/models/stripe_account.rb
@@ -2,4 +2,15 @@ class StripeAccount < ActiveRecord::Base
   belongs_to :enterprise
   validates_presence_of :stripe_user_id, :stripe_publishable_key
   validates_uniqueness_of :enterprise_id
+
+  def deauthorize_and_destroy
+    accounts = StripeAccount.where(stripe_user_id: stripe_user_id)
+
+    # Only deauthorize the user if it is not linked to multiple accounts
+    if accounts.count > 1 || Stripe::OAuth.deauthorize(stripe_user_id)
+      destroy
+    else
+      false
+    end
+  end
 end
diff --git a/lib/stripe/account_connector.rb b/lib/stripe/account_connector.rb
new file mode 100644
index 0000000000..c48f38d117
--- /dev/null
+++ b/lib/stripe/account_connector.rb
@@ -0,0 +1,43 @@
+# Encapsulation of logic used to handle the response from Stripe following an
+# attempt to connect an account to the instance using the OAuth Connection Flow
+# https://stripe.com/docs/connect/standard-accounts#oauth-flow
+
+module Stripe
+  class AccountConnector
+    attr_reader :oauth_response, :enterprise, :user, :params
+
+    def initialize(user, params)
+      @user = user
+      @params = params
+
+      raise StripeError, params["error_description"] unless params["code"]
+      raise CanCan::AccessDenied unless state.keys.include? "enterprise_id"
+
+      # Request an access token based on the code provided
+      @oauth_response = OAuth.request_access_token(params["code"])
+
+      # Find the Enterprise
+      @enterprise = Enterprise.find_by_permalink(state["enterprise_id"])
+
+      return if user.enterprises.include?(enterprise) || user.admin?
+
+      # Local authorisation issue, so request disconnection from Stripe
+      OAuth.deauthorize(oauth_response["stripe_user_id"])
+      raise CanCan::AccessDenied
+    end
+
+    def create_account
+      StripeAccount.create(
+        stripe_user_id: oauth_response["stripe_user_id"],
+        stripe_publishable_key: oauth_response["stripe_publishable_key"],
+        enterprise: enterprise
+      )
+    end
+
+    private
+
+    def state
+      OAuth.send(:jwt_decode, params["state"])
+    end
+  end
+end
diff --git a/lib/stripe/oauth.rb b/lib/stripe/oauth.rb
new file mode 100644
index 0000000000..105e8cdd3d
--- /dev/null
+++ b/lib/stripe/oauth.rb
@@ -0,0 +1,51 @@
+module Stripe
+  class OAuth
+    class << self
+      attr_accessor :client, :options
+    end
+
+    @options = {
+      :site => 'https://connect.stripe.com',
+      :authorize_url => '/oauth/authorize',
+      :deauthorize_url => '/oauth/deauthorize',
+      :token_url => '/oauth/token'
+    }
+
+    @client = OAuth2::Client.new(
+      ENV['STRIPE_CLIENT_ID'],
+      ENV['STRIPE_INSTANCE_SECRET_KEY'],
+      options
+    )
+
+    def self.authorize_url(enterprise_id, options = {})
+      options[:enterprise_id] = enterprise_id
+      jwt = jwt_encode(options)
+      # State param will be passed back after auth
+      client.auth_code.authorize_url(state: jwt, scope: 'read_write')
+    end
+
+    def self.request_access_token(auth_code)
+      # Fetch and return the account details from Stripe
+      client.auth_code.get_token(auth_code).params
+    end
+
+    def self.deauthorize(stripe_user_id)
+      client.deauthorize(stripe_user_id).deauthorize_request
+    end
+
+    private
+
+    def self.secret_token
+      Openfoodnetwork::Application.config.secret_token
+    end
+
+    def self.jwt_encode(payload)
+      JWT.encode(payload, secret_token, 'HS256')
+    end
+
+    def self.jwt_decode(token)
+      # Returns the original payload
+      JWT.decode(token, secret_token, true, algorithm: 'HS256')[0]
+    end
+  end
+end
diff --git a/spec/controllers/admin/enterprises_controller_spec.rb b/spec/controllers/admin/enterprises_controller_spec.rb
index 836f3e0de0..c51fbddd4f 100644
--- a/spec/controllers/admin/enterprises_controller_spec.rb
+++ b/spec/controllers/admin/enterprises_controller_spec.rb
@@ -144,67 +144,71 @@ module Admin
           expect(distributor.users).to_not include user
         end
 
-        describe "stripe connect" do
-          it "redirects to Stripe" do
-            controller.stub spree_current_user: distributor_manager
-            Admin::StripeHelper.client.stub id: "abc"
+        describe "#stripe_connect" do
+          before do
+            allow(controller).to receive(:spree_current_user) { distributor_manager }
+            allow(::Stripe::OAuth).to receive(:authorize_url) { "some_url" }
+          end
+
+          it "redirects to Stripe Authorization url constructed OAuth" do
             spree_get :stripe_connect
-            ['https://connect.stripe.com/oauth/authorize',
-              'response_type=code',
-              'state=',
-              'client_id=abc'].each{|element| response.location.should match element}
+            expect(response).to redirect_to "some_url"
+          end
+        end
+
+        context "#stripe_connect_callback" do
+          let(:params) { { id: distributor.permalink } }
+          before { allow(controller).to receive(:spree_current_user) { distributor_manager } }
+
+          context "when the connector raises a StripeError" do
+            before do
+              allow(Stripe::AccountConnector).to receive(:new).and_raise Stripe::StripeError, "some error"
+            end
+
+            it "returns a 500 error" do
+              spree_get :stripe_connect_callback, params
+              response.status.should be 500
+            end
           end
 
-          it "returns 500 on callback if the response code is not provided" do
-            controller.stub spree_current_user: distributor_manager
-            spree_get :stripe_connect_callback
-            response.status.should be 500
+          context "when the connector raises an AccessDenied error" do
+            before do
+              allow(Stripe::AccountConnector).to receive(:new).and_raise CanCan::AccessDenied, "some error"
+            end
+
+            it "redirects to unauthorized" do
+              spree_get :stripe_connect_callback, params
+              expect(response).to redirect_to spree.unauthorized_path
+            end
           end
 
-          it "redirects to login with the query params in case of session problems" do
-            pending("Difficult to reproduce: sometimes get logged out of OFN during Stripe connect
-            redirect/callback process. After logging in again, it doesn't redirect to the callback URL.")
-            controller.stub spree_current_user: nil
-            params = {this: "that"}
-            spree_get :stripe_connect_callback, params
-            # This is the idea - but even if generated correctly not sure it actually works since the redirect
-            # is ultimately handled in Angular, which presumably doesn't know which controller to
-            # use for the action?
-            response.should redirect_to root_path(anchor: "login?after_login=/?action=stripe_connect&this=that")
+          context "when initializing the connector does not raise an error" do
+            let(:connector) { double(:connector) }
+
+            before do
+              allow(Stripe::AccountConnector).to receive(:new) { connector }
+            end
+
+            context "when the connector succeeds in creating a new stripe account record" do
+              before { allow(connector).to receive(:create_account) { true } }
+
+              it "redirects to the enterprise edit path" do
+                spree_get :stripe_connect_callback, params
+                expect(flash[:success]).to eq I18n.t('admin.controllers.enterprises.stripe_connect_success')
+                expect(response).to redirect_to edit_admin_enterprise_path(distributor)
+              end
+            end
+
+            context "when the connector succeeds in creating a new stripe account record" do
+              before { allow(connector).to receive(:create_account) { false } }
+
+              it "renders a failure message" do
+                spree_get :stripe_connect_callback, params
+                expect(response.body).to eq I18n.t('admin.controllers.enterprises.stripe_connect_fail')
+                expect(response.status).to be 500
+              end
+            end
           end
-
-          it "redirects to unauthorized if the callback state param is invalid" do
-             controller.stub spree_current_user: distributor_manager
-             payload = {junk: "Ssfs"}
-             params = {state: JWT.encode(payload, Openfoodnetwork::Application.config.secret_token),
-                        code: "code"}
-             spree_get :stripe_connect_callback, params
-             response.should redirect_to '/unauthorized'
-          end
-
-          # TODO: This should probably also include managers/coordinators as well as owners?
-          it "makes a request to cancel the Stripe connection if the user does not own the enterprise" do
-            controller.stub spree_current_user: distributor_manager
-            controller.stub(:deauthorize_request_for_stripe_id)
-            controller.stub_chain(:get_stripe_token, :params).and_return({stripe_user_id: "xyz123", stripe_publishable_key: "abc456"}.to_json)
-            payload = {enterprise_id: supplier.permalink} # Request is not for the current user's Enterprise
-            params = {state: JWT.encode(payload, Openfoodnetwork::Application.config.secret_token),
-                        code: "code"}
-            spree_get :stripe_connect_callback, params
-
-            controller.should have_received(:deauthorize_request_for_stripe_id)
-          end
-
-          it "makes a new Stripe Account from the callback params" do
-            controller.stub spree_current_user: distributor_manager
-            controller.stub_chain(:get_stripe_token, :params).and_return({stripe_user_id: "xyz123", stripe_publishable_key: "abc456"}.to_json)
-            payload = {enterprise_id: distributor.permalink}
-            params = {state: JWT.encode(payload, Openfoodnetwork::Application.config.secret_token),
-                        code: "code"}
-            expect{spree_get :stripe_connect_callback, params}.to change{StripeAccount.all.length}.by 1
-            StripeAccount.last.enterprise_id.should eq distributor.id
-          end
-
         end
 
 
diff --git a/spec/controllers/admin/stripe_accounts_controller_spec.rb b/spec/controllers/admin/stripe_accounts_controller_spec.rb
index 415baac673..047885041f 100644
--- a/spec/controllers/admin/stripe_accounts_controller_spec.rb
+++ b/spec/controllers/admin/stripe_accounts_controller_spec.rb
@@ -3,40 +3,103 @@ require 'spec_helper'
 describe Admin::StripeAccountsController, type: :controller do
 
   describe "destroy_from_webhook" do
+    let!(:stripe_account) { create(:stripe_account, stripe_user_id: "webhook_id") }
+    let(:params) do
+      {
+        "format"=> "json",
+        "id" => "evt_123",
+        "object" => "event",
+        "data" => { "object" => { "id" => "ca_9B" } },
+        "type" => "account.application.deauthorized",
+        "user_id" => "webhook_id"
+      }
+    end
+
     it "deletes Stripe accounts in response to a webhook" do
-      # https://stripe.com/docs/api#retrieve_event
-      allow(controller).to receive(:fetch_event_from_stripe)
-        .and_return(Stripe::Event.construct_from({"id"=>"evt_wrfwg4323fw",
-                     "object"=>"event",
-                     "api_version"=>nil,
-                     "created"=>1484870684,
-                     "data"=>
-                      {"object"=>
-                        {"id"=>"application_id",
-                         "object"=>"application",
-                         "name"=>"Open Food Network UK"}},
-                     "livemode"=>false,
-                     "pending_webhooks"=>1,
-                     "request"=>nil,
-                     "type"=>"account.application.deauthorized",
-                     "user_id"=>"webhook_id"}))
-      account = create(:stripe_account, stripe_user_id: "webhook_id")
-      expect(Stripe::Event).not_to receive(:retrieve) # should not retrieve direct for a deauth event
-      post 'destroy_from_webhook', {"id"=>"evt_wrfwg4323fw",
-                                     "object"=>"event",
-                                     "api_version"=>nil,
-                                     "created"=>1484870684,
-                                     "data"=>
-                                      {"object"=>
-                                        {"id"=>"ca_9ByaSyyyXj5O73DWisU0KLluf0870Vro",
-                                         "object"=>"application",
-                                         "name"=>"Open Food Network UK"}},
-                                     "livemode"=>false,
-                                     "pending_webhooks"=>1,
-                                     "request"=>nil,
-                                     "type"=>"account.application.deauthorized",
-                                     "user_id"=>"webhook_id"}
-      expect(StripeAccount.all).not_to include account
+      post 'destroy_from_webhook', params
+      expect(response.status).to eq 200
+      expect(response.body).to eq "Account webhook_id deauthorized"
+      expect(StripeAccount.all).not_to include stripe_account
+    end
+
+    context "when the user_id on the event does not match any known accounts" do
+      before do
+        params["user_id"] = "webhook_id1"
+      end
+
+      it "does nothing" do
+        post 'destroy_from_webhook', params
+        expect(response.status).to eq 400
+        expect(StripeAccount.all).to include stripe_account
+      end
+    end
+
+    context "when the event is not a deauthorize event" do
+      before do
+        params["type"] = "account.application.authorized"
+      end
+
+      it "does nothing" do
+        post 'destroy_from_webhook', params
+        expect(response.status).to eq 400
+        expect(StripeAccount.all).to include stripe_account
+      end
+    end
+  end
+
+  describe "destroy" do
+    let(:enterprise) { create(:distributor_enterprise) }
+    let(:params) { { format: :json, id: "some_id"  } }
+
+    context "when the specified stripe account doesn't exist" do
+      it "raises an error?" do
+        spree_delete :destroy, params
+      end
+    end
+
+    context "when the specified stripe account exists" do
+      let(:stripe_account) { create(:stripe_account, enterprise: enterprise) }
+
+      before do
+        # So that we can stub #deauthorize_and_destroy
+        allow(StripeAccount).to receive(:find) { stripe_account }
+        params[:id] = stripe_account.id
+      end
+
+      context "when I don't manage the enterprise linked to the stripe account" do
+        let(:some_user) { create(:user) }
+
+        before { allow(controller).to receive(:spree_current_user) { some_user } }
+
+        it "redirects to unauthorized" do
+          spree_delete :destroy, params
+          expect(response).to redirect_to spree.unauthorized_path
+        end
+      end
+
+      context "when I manage the enterprise linked to the stripe account" do
+        before { allow(controller).to receive(:spree_current_user) { enterprise.owner } }
+
+        context "and the attempt to deauthorize_and_destroy succeeds" do
+          before { allow(stripe_account).to receive(:deauthorize_and_destroy) { stripe_account } }
+
+          it "redirects to unauthorized" do
+            spree_delete :destroy, params
+            expect(response).to redirect_to edit_admin_enterprise_path(enterprise)
+            expect(flash[:success]).to eq "Stripe account disconnected."
+          end
+        end
+
+        context "and the attempt to deauthorize_and_destroy fails" do
+          before { allow(stripe_account).to receive(:deauthorize_and_destroy) { false } }
+
+          it "redirects to unauthorized" do
+            spree_delete :destroy, params
+            expect(response).to redirect_to edit_admin_enterprise_path(enterprise)
+            expect(flash[:error]).to eq "Failed to disconnect Stripe."
+          end
+        end
+      end
     end
   end
 
diff --git a/spec/helpers/admin/stripe_helper_spec.rb b/spec/helpers/admin/stripe_helper_spec.rb
deleted file mode 100644
index c15ca4a59f..0000000000
--- a/spec/helpers/admin/stripe_helper_spec.rb
+++ /dev/null
@@ -1,52 +0,0 @@
-require 'spec_helper'
-
-describe Admin::StripeHelper do
-  let!(:enterprise) { create(:enterprise) }
-  let!(:enterprise2) { create(:enterprise) }
-  let!(:stripe_account) { create(:stripe_account, enterprise: enterprise) }
-
-  it "calls the Stripe API to get a token" do
-    expect(Admin::StripeHelper.client.auth_code).to receive(:get_token).with("abc", {})
-    helper.get_stripe_token("abc")
-  end
-
-  it "calls the Stripe API for authorization with read_write permission, passing appropriate JWT in the state param" do
-    expect(Admin::StripeHelper.client.auth_code).to receive(:authorize_url).with({
-      state: JWT.encode({enterprise_id: "enterprise-permalink"}, Openfoodnetwork::Application.config.secret_token, 'HS256'),
-      scope: "read_write"
-    })
-    helper.authorize_stripe("enterprise-permalink")
-  end
-
-  context "Disconnecting an account" do
-    it "doesn't destroy the database record if the Stripe API disconnect failed" do
-      Admin::StripeHelper.client
-        .deauthorize(stripe_account.stripe_user_id)
-        .stub(:deauthorize_request)
-        .and_return(nil)
-
-      deauthorize_stripe(stripe_account.id)
-      expect(StripeAccount.all).to include(stripe_account)
-    end
-
-    it "destroys the record if the Stripe API disconnect succeeds" do
-      Admin::StripeHelper.client
-        .deauthorize(stripe_account.stripe_user_id)
-        .stub(:deauthorize_request)
-        .and_return("something truthy")
-
-      deauthorize_stripe(stripe_account.id)
-      expect(StripeAccount.all).not_to include(stripe_account)
-    end
-
-    it "Doesn't make a Stripe API disconnection request if the account is also associated with another Enterprise" do
-      another_stripe_account = create(:stripe_account, enterprise: enterprise2, stripe_user_id: stripe_account.stripe_user_id)
-      expect(Admin::StripeHelper.client.deauthorize(stripe_account.stripe_user_id)).not_to receive(:deauthorize_request)
-      deauthorize_stripe(stripe_account.id)
-    end
-
-    it "encodes and decodes JWT" do
-      jwt_decode(jwt_encode({test: "string"})).should eq({"test" => "string"})
-    end
-  end
-end
diff --git a/spec/lib/stripe/account_connector_spec.rb b/spec/lib/stripe/account_connector_spec.rb
new file mode 100644
index 0000000000..51893bf806
--- /dev/null
+++ b/spec/lib/stripe/account_connector_spec.rb
@@ -0,0 +1,77 @@
+require 'spec_helper'
+require 'stripe/account_connector'
+require 'stripe/oauth'
+
+module Stripe
+  describe AccountConnector do
+    describe "initialization" do
+      let(:user) { create(:user) }
+      let(:enterprise) { create(:enterprise) }
+      let(:payload) { { "junk" => "Ssfs" } }
+      let(:state) { JWT.encode(payload, Openfoodnetwork::Application.config.secret_token) }
+      let(:params) { { "state" => state } }
+
+      context "when params have no 'code' key" do
+        it "raises a StripeError" do
+           expect{ AccountConnector.new(user, params) }.to raise_error StripeError
+        end
+      end
+
+      context "when params have a 'code' key" do
+        before { params["code"] = 'code' }
+
+        context "and the decoded state param doesn't contain an 'enterprise_id' key" do
+          it "raises an AccessDenied error" do
+            expect{ AccountConnector.new(user, params) }.to raise_error CanCan::AccessDenied
+          end
+        end
+
+        context "and the decoded state param contains an 'enterprise_id' key" do
+          let(:payload) { { enterprise_id: enterprise.permalink } }
+          let(:access_token) { { "stripe_user_id" => "some_user_id", "stripe_publishable_key" => "some_key" } }
+
+          before do
+            expect(OAuth).to receive(:request_access_token) { access_token }
+          end
+
+          context "but the user doesn't manage own or manage the corresponding enterprise" do
+            it "makes a request to cancel the Stripe connection and raises an error" do
+              expect(OAuth).to receive(:deauthorize).with("some_user_id")
+              expect{ AccountConnector.new(user, params) }.to raise_error CanCan::AccessDenied
+            end
+          end
+
+          context "and the user manages the corresponding enterprise" do
+            before do
+              user.enterprise_roles.create(enterprise: enterprise)
+            end
+
+            it "raises no errors" do
+              expect(OAuth).to_not receive(:deauthorize)
+              AccountConnector.new(user, params)
+            end
+
+            it "allows creations of a new Stripe Account from the callback params" do
+              connector = AccountConnector.new(user, params)
+              expect{connector.create_account}.to change(StripeAccount, :count).by(1)
+            end
+          end
+
+          context "and the user owns the corresponding enterprise" do
+            let(:user) { enterprise.owner }
+
+            it "raises no errors" do
+              expect(OAuth).to_not receive(:deauthorize)
+              AccountConnector.new(user, params)
+            end
+
+            it "allows creations of a new Stripe Account from the callback params" do
+              connector = AccountConnector.new(user, params)
+              expect{connector.create_account}.to change(StripeAccount, :count).by(1)
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/stripe/oauth_spec.rb b/spec/lib/stripe/oauth_spec.rb
new file mode 100644
index 0000000000..6f46f138cb
--- /dev/null
+++ b/spec/lib/stripe/oauth_spec.rb
@@ -0,0 +1,25 @@
+require 'spec_helper'
+require 'stripe/oauth'
+
+module Stripe
+  describe OAuth do
+    describe "contructing an authorization url" do
+      let(:enterprise_id) { "ent_id" }
+
+      before do
+        allow(OAuth.client).to receive(:id) { 'abc' }
+      end
+
+      it "builds a url with all of the necessary params" do
+        url = OAuth.authorize_url(enterprise_id)
+        uri = URI.parse(url)
+        params = CGI::parse(uri.query)
+        expect(params.keys).to include 'client_id', 'response_type', 'state', 'scope'
+        expect(params["state"]).to eq [OAuth.jwt_encode(enterprise_id: enterprise_id)]
+        expect(uri.scheme).to eq 'https'
+        expect(uri.host).to eq 'connect.stripe.com'
+        expect(uri.path).to eq '/oauth/authorize'
+      end
+    end
+  end
+end
diff --git a/spec/models/spree/gateway/stripe_connect_spec.rb b/spec/models/spree/gateway/stripe_connect_spec.rb
index 56cd7e6e41..d2cc2310ea 100644
--- a/spec/models/spree/gateway/stripe_connect_spec.rb
+++ b/spec/models/spree/gateway/stripe_connect_spec.rb
@@ -43,6 +43,7 @@ describe Spree::Gateway::StripeConnect, type: :model do
 
     context "when the credit card provided does not have a gateway_payment_profile_id" do
       before { allow(creditcard).to receive(:gateway_payment_profile_id) { nil } }
+      before { allow(creditcard).to receive(:gateway_customer_profile_id) { "customer_id123" } }
 
       it "returns nil....?" do
         result = subject.send(:token_from_card_profile_ids, creditcard)
diff --git a/spec/models/stripe_account_spec.rb b/spec/models/stripe_account_spec.rb
new file mode 100644
index 0000000000..dd81c202b1
--- /dev/null
+++ b/spec/models/stripe_account_spec.rb
@@ -0,0 +1,47 @@
+require 'spec_helper'
+require 'stripe/oauth'
+
+describe StripeAccount do
+  describe "deauthorize_and_destroy" do
+    let!(:enterprise) { create(:enterprise) }
+    let!(:enterprise2) { create(:enterprise) }
+    let!(:stripe_account) { create(:stripe_account, enterprise: enterprise) }
+
+    context "when the Stripe API disconnect fails" do
+      before do
+        Stripe::OAuth.client
+          .deauthorize(stripe_account.stripe_user_id)
+          .stub(:deauthorize_request)
+          .and_return(nil)
+      end
+
+      it "doesn't destroy the record" do
+        stripe_account.deauthorize_and_destroy
+        expect(StripeAccount.all).to include(stripe_account)
+      end
+    end
+
+    context "when the Stripe API disconnect succeeds" do
+      before do
+        Stripe::OAuth.client
+          .deauthorize(stripe_account.stripe_user_id)
+          .stub(:deauthorize_request)
+          .and_return("something truthy")
+      end
+
+      it "destroys the record" do
+        stripe_account.deauthorize_and_destroy
+        expect(StripeAccount.all).not_to include(stripe_account)
+      end
+    end
+
+    context "if the account is also associated with another Enterprise" do
+      let!(:another_stripe_account) { create(:stripe_account, enterprise: enterprise2, stripe_user_id: stripe_account.stripe_user_id) }
+
+      it "Doesn't make a Stripe API disconnection request " do
+        expect(Stripe::OAuth.client.deauthorize(stripe_account.stripe_user_id)).not_to receive(:deauthorize_request)
+        stripe_account.deauthorize_and_destroy
+      end
+    end
+  end
+end