From 576e4db9bee89b1bef7272b1fb2d5bdf865a7ed8 Mon Sep 17 00:00:00 2001
From: Rob Harrington <oeoeaio@gmail.com>
Date: Fri, 14 Jul 2017 15:32:06 +1000
Subject: [PATCH] Authorize StripeAccount#status using the account object

Rather than the enterprise
---
 .../admin/stripe_accounts_controller.rb       |  2 +-
 app/models/spree/ability_decorator.rb         |  4 +-
 .../admin/stripe_accounts_controller_spec.rb  | 56 +++++++++----------
 3 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/app/controllers/admin/stripe_accounts_controller.rb b/app/controllers/admin/stripe_accounts_controller.rb
index 383857c2db..937c4f7e8e 100644
--- a/app/controllers/admin/stripe_accounts_controller.rb
+++ b/app/controllers/admin/stripe_accounts_controller.rb
@@ -53,10 +53,10 @@ module Admin
     end
 
     def status
-      authorize! :stripe_account, Enterprise.find_by_id(params[:enterprise_id])
       return render json: { status: :stripe_disabled } unless Spree::Config.stripe_connect_enabled
       stripe_account = StripeAccount.find_by_enterprise_id(params[:enterprise_id])
       return render json: { status: :account_missing } unless stripe_account
+      authorize! :status, stripe_account
 
       begin
         status = Stripe::Account.retrieve(stripe_account.stripe_user_id)
diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb
index 78c994f8a7..ab370f68c2 100644
--- a/app/models/spree/ability_decorator.rb
+++ b/app/models/spree/ability_decorator.rb
@@ -96,7 +96,7 @@ class AbilityDecorator
     can [:welcome, :register], Enterprise do |enterprise|
       enterprise.owner == user
     end
-    can [:manage_payment_methods, :manage_shipping_methods, :manage_enterprise_fees, :stripe_account], Enterprise do |enterprise|
+    can [:manage_payment_methods, :manage_shipping_methods, :manage_enterprise_fees], Enterprise do |enterprise|
       user.enterprises.include? enterprise
     end
 
@@ -122,7 +122,7 @@ 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|
+    can [:status, :destroy], StripeAccount do |stripe_account|
       user.enterprises.include? stripe_account.enterprise
     end
   end
diff --git a/spec/controllers/admin/stripe_accounts_controller_spec.rb b/spec/controllers/admin/stripe_accounts_controller_spec.rb
index 2702d58084..dbe215c6fe 100644
--- a/spec/controllers/admin/stripe_accounts_controller_spec.rb
+++ b/spec/controllers/admin/stripe_accounts_controller_spec.rb
@@ -182,50 +182,50 @@ describe Admin::StripeAccountsController, type: :controller do
       Spree::Config.set(stripe_connect_enabled: false)
     end
 
-    context "where I don't manage the specified enterprise" do
-      let(:user) { create(:user) }
-      let(:enterprise2) { create(:enterprise) }
-      before do
-        user.owned_enterprises << enterprise2
-        params[:enterprise_id] = enterprise.id
-        allow(controller).to receive(:spree_current_user) { user }
-      end
-
-      it "redirects to unauthorized" do
+    context "when Stripe is not enabled" do
+      it "returns with a status of 'stripe_disabled'" do
         spree_get :status, params
-        expect(response).to redirect_to spree.unauthorized_path
+        json_response = JSON.parse(response.body)
+        expect(json_response["status"]).to eq "stripe_disabled"
       end
     end
 
-    context "where I manage the specified enterprise" do
-      before do
-        params[:enterprise_id] = enterprise.id
-        allow(controller).to receive(:spree_current_user) { enterprise.owner }
-      end
+    context "when Stripe is enabled" do
+      before { Spree::Config.set(stripe_connect_enabled: true) }
 
-      context "but Stripe is not enabled" do
-        it "returns with a status of 'stripe_disabled'" do
+      context "but no stripe account is associated with the specified enterprise" do
+        it "returns with a status of 'account_missing'" do
           spree_get :status, params
           json_response = JSON.parse(response.body)
-          expect(json_response["status"]).to eq "stripe_disabled"
+          expect(json_response["status"]).to eq "account_missing"
         end
       end
 
-      context "and Stripe is enabled" do
-        before { Spree::Config.set(stripe_connect_enabled: true) }
+      context "and a stripe account is associated with the specified enterprise" do
+        let!(:account) { create(:stripe_account, stripe_user_id: "acc_123", enterprise: enterprise) }
 
-        context "but it has no associated stripe account" do
-          it "returns with a status of 'account_missing'" do
+        context "but I don't manage the enterprise" do
+          let(:user) { create(:user) }
+          let(:enterprise2) { create(:enterprise) }
+          before do
+            user.owned_enterprises << enterprise2
+            params[:enterprise_id] = enterprise.id
+            allow(controller).to receive(:spree_current_user) { user }
+          end
+
+          it "redirects to unauthorized" do
             spree_get :status, params
-            json_response = JSON.parse(response.body)
-            expect(json_response["status"]).to eq "account_missing"
+            expect(response).to redirect_to spree.unauthorized_path
           end
         end
 
-        context "and it has an associated stripe account" do
-          let!(:account) { create(:stripe_account, stripe_user_id: "acc_123", enterprise: enterprise) }
+        context "and I manage the enterprise" do
+          before do
+            params[:enterprise_id] = enterprise.id
+            allow(controller).to receive(:spree_current_user) { enterprise.owner }
+          end
 
-          context "which has been revoked or does not exist" do
+          context "but access has been revoked or does not exist on stripe's servers" do
             before do
               stub_request(:get, "https://api.stripe.com/v1/accounts/acc_123").to_return(status: 404)
             end