Use guest token to authorize API controller actions

10 files changed, 107 insertions, 220 deletions

diff --git a/app/controllers/solidus_subscriptions/api/v1/base_controller.rb b/app/controllers/solidus_subscriptions/api/v1/base_controller.rb
index 511d361..0058697 100644
--- a/app/controllers/solidus_subscriptions/api/v1/base_controller.rb
+++ b/app/controllers/solidus_subscriptions/api/v1/base_controller.rb
@@ -2,6 +2,9 @@ module SolidusSubscriptions
   module Api
     module V1
       class BaseController < ::Spree::Api::BaseController
+        def subscription_guest_token
+          request.headers['X-Spree-Subscription-Token']
+        end
       end
     end
   end
diff --git a/app/controllers/solidus_subscriptions/api/v1/line_items_controller.rb b/app/controllers/solidus_subscriptions/api/v1/line_items_controller.rb
index 7221b7b..15d35d0 100644
--- a/app/controllers/solidus_subscriptions/api/v1/line_items_controller.rb
+++ b/app/controllers/solidus_subscriptions/api/v1/line_items_controller.rb
@@ -8,7 +8,7 @@ module SolidusSubscriptions
         wrap_parameters :subscription_line_item
 
         def update
-          authorize! :update, @line_item
+          authorize! :update, @line_item, subscription_guest_token
           if @line_item.update(line_item_params)
             render json: @line_item.to_json
           else
@@ -17,11 +17,13 @@ module SolidusSubscriptions
         end
 
         def destroy
-          authorize! :destroy, @line_item
-          return render json: {}, status: :bad_request if @line_item.order.complete?
+          authorize! :destroy, @line_item, subscription_guest_token
 
           @line_item.destroy!
-          @line_item.order.recalculate
+
+          if @line_item.order && !@line_item.order.complete?
+            @line_item.order.recalculate
+          end
 
           render json: @line_item.to_json
         end
diff --git a/app/controllers/solidus_subscriptions/api/v1/subscriptions_controller.rb b/app/controllers/solidus_subscriptions/api/v1/subscriptions_controller.rb
index a28f38f..12b46cb 100644
--- a/app/controllers/solidus_subscriptions/api/v1/subscriptions_controller.rb
+++ b/app/controllers/solidus_subscriptions/api/v1/subscriptions_controller.rb
@@ -35,7 +35,8 @@ module SolidusSubscriptions
         private
 
         def load_subscription
-          @subscription = current_api_user.subscriptions.find(params[:id])
+          @subscription = SolidusSubscriptions::Subscription.find(params[:id])
+          authorize! action_name, @subscription, subscription_guest_token
         end
 
         def subscription_params
diff --git a/app/controllers/spree/admin/subscriptions_controller.rb b/app/controllers/spree/admin/subscriptions_controller.rb
index cde63a0..6c92ace 100644
--- a/app/controllers/spree/admin/subscriptions_controller.rb
+++ b/app/controllers/spree/admin/subscriptions_controller.rb
@@ -6,9 +6,7 @@ module Spree
       skip_before_action :load_resource, only: :index
 
       def index
-        @search = SolidusSubscriptions::Subscription.
-                  accessible_by(current_ability, :index).ransack(params[:q])
-
+        @search = SolidusSubscriptions::Subscription.ransack(params[:q])
         @subscriptions = @search.result(distinct: true).
                          includes(:line_items, :user).
                          page(params[:page]).
diff --git a/lib/solidus_subscriptions/permission_sets/subscription_management.rb b/lib/solidus_subscriptions/permission_sets/subscription_management.rb
index 76c1e94..c669368 100644
--- a/lib/solidus_subscriptions/permission_sets/subscription_management.rb
+++ b/lib/solidus_subscriptions/permission_sets/subscription_management.rb
@@ -4,12 +4,14 @@ module SolidusSubscriptions
   module PermissionSets
     class SubscriptionManagement < ::Spree::PermissionSets::Base
       def activate!
-        can :manage, Subscription do |subscription|
-          subscription.user && subscription.user == user
+        can :manage, Subscription do |subscription, guest_token|
+          (subscription.guest_token.present? && subscription.guest_token == guest_token) ||
+            (subscription.user && subscription.user == user)
         end
 
-        can :manage, LineItem do |line_item|
-          line_item.subscription&.user && line_item.subscription.user == user
+        can :manage, LineItem do |line_item, guest_token|
+          (line_item.subscription&.guest_token.present? && line_item.subscription.guest_token == guest_token) ||
+            (line_item.subscription&.user && line_item.subscription.user == user)
         end
       end
     end
diff --git a/spec/controllers/solidus_subscriptions/api/v1/line_items_controller_spec.rb b/spec/controllers/solidus_subscriptions/api/v1/line_items_controller_spec.rb
index bf8ac5f..ab5b828 100644
--- a/spec/controllers/solidus_subscriptions/api/v1/line_items_controller_spec.rb
+++ b/spec/controllers/solidus_subscriptions/api/v1/line_items_controller_spec.rb
@@ -4,7 +4,7 @@ RSpec.describe SolidusSubscriptions::Api::V1::LineItemsController, type: :contro
   routes { SolidusSubscriptions::Engine.routes }
 
   let!(:user) { create(:user) }
-  let(:line) { create :subscription_line_item, order: order }
+  let(:line) { create :subscription_line_item, subscription: subscription }
 
   before { user.generate_spree_api_key! }
 
@@ -20,18 +20,8 @@ RSpec.describe SolidusSubscriptions::Api::V1::LineItemsController, type: :contro
       }
     end
 
-    context 'guest user' do
-      let(:order) { create :order }
-
-      let(:params) do
-        {
-          id: line.id,
-          subscription_line_item: { quantity: 21 },
-          checkout_id: order.number,
-          order_token: order.guest_token,
-          order_id: order.number
-        }
-      end
+    context "when the subscription belongs to the user" do
+      let(:subscription) { create :subscription, user: user }
 
       context "with valid params" do
         let(:json_body) { JSON.parse(subject.body) }
@@ -47,34 +37,7 @@ RSpec.describe SolidusSubscriptions::Api::V1::LineItemsController, type: :contro
         let(:params) do
           {
             id: line.id,
-            subscription_line_item: { interval_length: -1 },
-            checkout_id: order.number,
-            order_token: order.guest_token
-          }
-        end
-
-        it { is_expected.to be_unprocessable }
-      end
-    end
-
-    context "when the order belongs to the user" do
-      let(:order) { create :order, user: user }
-
-      context "with valid params" do
-        let(:json_body) { JSON.parse(subject.body) }
-
-        it { is_expected.to be_successful }
-
-        it "returns the updated record" do
-          expect(json_body["quantity"]).to eq 21
-        end
-      end
-
-      context "with invalid params" do
-        let(:params) do
-          {
-            id: line.id,
-            subscription_line_item: { interval_length: -1 },
+            subscription_line_item: { quantity: -1 },
             token: user.spree_api_key,
             format: :json
           }
@@ -84,8 +47,8 @@ RSpec.describe SolidusSubscriptions::Api::V1::LineItemsController, type: :contro
       end
     end
 
-    context "when the order belongs to someone else" do
-      let(:order) { create :order, user: create(:user) }
+    context "when the subscription belongs to someone else" do
+      let(:subscription) { create :subscription, user: create(:user) }
 
       it { is_expected.to be_unauthorized }
     end
@@ -97,28 +60,15 @@ RSpec.describe SolidusSubscriptions::Api::V1::LineItemsController, type: :contro
     let(:params) {
       {
         id: line.id,
-        order_id: order.id,
         token: user.spree_api_key,
         format: :json
       }
     }
 
-    context "when the order is not ours" do
-      let(:order) { create :order, user: create(:user) }
+    context "when the subscription is not ours" do
+      let(:subscription) { create :subscription, user: create(:user) }
 
       it { is_expected.to be_unauthorized }
     end
-
-    context "when the order is finalised" do
-      let(:order) { create :completed_order_with_totals, user: user }
-
-      it { is_expected.to be_bad_request }
-    end
-
-    context "when the order is ours and incomplete" do
-      let(:order) { create :order, user: user }
-
-      it { is_expected.to be_successful }
-    end
   end
 end
diff --git a/spec/controllers/solidus_subscriptions/api/v1/subscriptions_controller_spec.rb b/spec/controllers/solidus_subscriptions/api/v1/subscriptions_controller_spec.rb
index 90dd089..2c9007f 100644
--- a/spec/controllers/solidus_subscriptions/api/v1/subscriptions_controller_spec.rb
+++ b/spec/controllers/solidus_subscriptions/api/v1/subscriptions_controller_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-RSpec.describe SolidusSubscriptions::Api::V1::SubscriptionsController, type: :controller do
+RSpec.describe SolidusSubscriptions::Api::V1::SubscriptionsController do
   routes { SolidusSubscriptions::Engine.routes }
 
   let!(:user) { create :user }
@@ -24,7 +24,7 @@ RSpec.describe SolidusSubscriptions::Api::V1::SubscriptionsController, type: :co
     context "when the subscription belongs to someone else" do
       let!(:subscription) { create :subscription, user: create(:user) }
 
-      it { is_expected.to be_not_found }
+      it { is_expected.to be_unauthorized }
     end
 
     context 'when the subscription is canceled' do
@@ -99,7 +99,7 @@ RSpec.describe SolidusSubscriptions::Api::V1::SubscriptionsController, type: :co
     context 'when the subscription belongs to someone else' do
       let!(:subscription) { create :subscription, :with_line_item, user: create(:user) }
 
-      it { is_expected.to be_not_found }
+      it { is_expected.to be_unauthorized }
     end
   end
 
diff --git a/spec/lib/solidus_subscriptions/ability_spec.rb b/spec/lib/solidus_subscriptions/ability_spec.rb
deleted file mode 100644
index 07d48ab..0000000
--- a/spec/lib/solidus_subscriptions/ability_spec.rb
+++ /dev/null
@@ -1,70 +0,0 @@
-require 'spec_helper'
-require "cancan/matchers"
-
-RSpec.describe SolidusSubscriptions::Ability do
-  subject { described_class.new user }
-
-  context 'when the user is a default customer' do
-    let(:user) { create :user }
-
-    context 'owns the order' do
-      let(:order) { create :order }
-      let(:line_item) do
-        create :subscription_line_item, order: order
-      end
-
-      it { is_expected.to be_able_to :index, line_item, order }
-      it { is_expected.to be_able_to :show, line_item, order }
-      it { is_expected.to be_able_to :create, line_item, order }
-      it { is_expected.to be_able_to :update, line_item, order }
-      it { is_expected.to be_able_to :destroy, line_item, order }
-    end
-
-    context 'doesnt own the order' do
-      let(:order) { create :order }
-      let(:another_order) { create :order }
-
-      let(:line_item) do
-        create :subscription_line_item, order: order
-      end
-
-      it { is_expected.not_to be_able_to :index, line_item, another_order }
-      it { is_expected.not_to be_able_to :show, line_item, another_order }
-      it { is_expected.not_to be_able_to :create, line_item, another_order }
-      it { is_expected.not_to be_able_to :update, line_item, another_order }
-      it { is_expected.not_to be_able_to :destroy, line_item, another_order }
-    end
-
-    context 'the user owns a subscription' do
-      let(:subscription) { create :subscription, user: user }
-
-      it { is_expected.to be_able_to :index, subscription }
-      it { is_expected.to be_able_to :show, subscription }
-      it { is_expected.to be_able_to :create, subscription }
-      it { is_expected.to be_able_to :update, subscription }
-      it { is_expected.to be_able_to :destroy, subscription }
-      it { is_expected.to be_able_to :skip, subscription }
-      it { is_expected.to be_able_to :cancel, subscription }
-    end
-
-    context 'the doesnt own a subscription' do
-      let(:another_user) { create :user }
-      let(:subscription) { create :subscription, user: another_user }
-
-      it { is_expected.not_to be_able_to :index, subscription }
-      it { is_expected.not_to be_able_to :show, subscription }
-      it { is_expected.not_to be_able_to :create, subscription }
-      it { is_expected.not_to be_able_to :update, subscription }
-      it { is_expected.not_to be_able_to :destroy, subscription }
-      it { is_expected.not_to be_able_to :skip, subscription }
-      it { is_expected.not_to be_able_to :cancel, subscription }
-    end
-  end
-
-  context 'the user is an admin' do
-    let(:user) { create :admin_user }
-
-    it { is_expected.to be_able_to :manage, SolidusSubscriptions::Subscription }
-    it { is_expected.to be_able_to :manage, SolidusSubscriptions::LineItem }
-  end
-end
diff --git a/spec/lib/solidus_subscriptions/permission_sets/subscription_management_spec.rb b/spec/lib/solidus_subscriptions/permission_sets/subscription_management_spec.rb
index e3c3c66..7079c8a 100644
--- a/spec/lib/solidus_subscriptions/permission_sets/subscription_management_spec.rb
+++ b/spec/lib/solidus_subscriptions/permission_sets/subscription_management_spec.rb
@@ -1,49 +1,95 @@
 # frozen_string_literal: true
 
 RSpec.describe SolidusSubscriptions::PermissionSets::SubscriptionManagement do
-  it 'is allowed to manage their subscriptions' do
-    user = create(:user)
-    subscription = create(:subscription, user: user)
+  context 'when the user is authenticated' do
+    it 'is allowed to manage their subscriptions' do
+      user = create(:user)
+      subscription = create(:subscription, user: user)
 
-    ability = Spree::Ability.new(user)
-    permission_set = described_class.new(ability)
-    permission_set.activate!
+      ability = Spree::Ability.new(user)
+      permission_set = described_class.new(ability)
+      permission_set.activate!
 
-    expect(ability).to be_able_to(:manage, subscription)
-  end
+      expect(ability).to be_able_to(:manage, subscription)
+    end
 
-  it "is allowed to manage someone else's subscriptions" do
-    user = create(:user)
-    subscription = create(:subscription)
+    it "is allowed to manage someone else's subscriptions" do
+      user = create(:user)
+      subscription = create(:subscription)
 
-    ability = Spree::Ability.new(user)
-    permission_set = described_class.new(ability)
-    permission_set.activate!
+      ability = Spree::Ability.new(user)
+      permission_set = described_class.new(ability)
+      permission_set.activate!
 
-    expect(ability).not_to be_able_to(:manage, subscription)
-  end
+      expect(ability).not_to be_able_to(:manage, subscription)
+    end
+
+    it 'is allowed to manage line items on their subscriptions' do
+      user = create(:user)
+      subscription = create(:subscription, user: user)
+      line_item = create(:subscription_line_item, subscription: subscription)
+
+      ability = Spree::Ability.new(user)
+      permission_set = described_class.new(ability)
+      permission_set.activate!
 
-  it 'is allowed to manage line items on their subscriptions' do
-    user = create(:user)
-    subscription = create(:subscription, user: user)
-    line_item = create(:subscription_line_item, subscription: subscription)
+      expect(ability).to be_able_to(:manage, line_item)
+    end
 
-    ability = Spree::Ability.new(user)
-    permission_set = described_class.new(ability)
-    permission_set.activate!
+    it "is not allowed to manage line items on someone else's subscriptions" do
+      user = create(:user)
+      subscription = create(:subscription)
+      line_item = create(:subscription_line_item, subscription: subscription)
 
-    expect(ability).to be_able_to(:manage, line_item)
+      ability = Spree::Ability.new(user)
+      permission_set = described_class.new(ability)
+      permission_set.activate!
+
+      expect(ability).not_to be_able_to(:manage, line_item)
+    end
   end
 
-  it "is not allowed to manage line items on someone else's subscriptions" do
-    user = create(:user)
-    subscription = create(:subscription)
-    line_item = create(:subscription_line_item, subscription: subscription)
+  context 'when the user provides a guest token' do
+    it 'is allowed to manage their subscriptions' do
+      subscription = create(:subscription)
+
+      ability = Spree::Ability.new(nil)
+      permission_set = described_class.new(ability)
+      permission_set.activate!
+
+      expect(ability).to be_able_to(:manage, subscription, subscription.guest_token)
+    end
+
+    it "is allowed to manage someone else's subscriptions" do
+      subscription = create(:subscription)
+
+      ability = Spree::Ability.new(nil)
+      permission_set = described_class.new(ability)
+      permission_set.activate!
+
+      expect(ability).not_to be_able_to(:manage, subscription, 'invalid')
+    end
+
+    it 'is allowed to manage line items on their subscriptions' do
+      subscription = create(:subscription)
+      line_item = create(:subscription_line_item, subscription: subscription)
+
+      ability = Spree::Ability.new(nil)
+      permission_set = described_class.new(ability)
+      permission_set.activate!
+
+      expect(ability).to be_able_to(:manage, line_item, subscription.guest_token)
+    end
+
+    it "is not allowed to manage line items on someone else's subscriptions" do
+      subscription = create(:subscription)
+      line_item = create(:subscription_line_item, subscription: subscription)
 
-    ability = Spree::Ability.new(user)
-    permission_set = described_class.new(ability)
-    permission_set.activate!
+      ability = Spree::Ability.new(nil)
+      permission_set = described_class.new(ability)
+      permission_set.activate!
 
-    expect(ability).not_to be_able_to(:manage, line_item)
+      expect(ability).not_to be_able_to(:manage, line_item, 'invalid')
+    end
   end
 end
diff --git a/spec/requests/solidus_subscriptions/api/v1/subscriptions_spec.rb b/spec/requests/solidus_subscriptions/api/v1/subscriptions_spec.rb
deleted file mode 100644
index bab7b23..0000000
--- a/spec/requests/solidus_subscriptions/api/v1/subscriptions_spec.rb
+++ /dev/null
@@ -1,45 +0,0 @@
-require 'spec_helper'
-
-RSpec.describe "Subscription endpoints", type: :request do
-  let(:json_resp) { JSON.parse(response.body) }
-  let(:user) { create :user }
-
-  before { user.generate_spree_api_key! }
-
-  describe "#cancel" do
-    let(:subscription) do
-      create :subscription, :with_line_item, actionable_date: (Date.current + 1.month), user: user
-    end
-
-    it "returns the canceled record", :aggregate_failures do
-      post solidus_subscriptions.cancel_api_v1_subscription_path(subscription), params: { token: user.spree_api_key }
-      expect(json_resp["state"]).to eq "canceled"
-      expect(json_resp["actionable_date"]).to be_nil
-    end
-
-    context 'when the miniumum notice has been past' do
-      let(:subscription) do
-        create :subscription, :with_line_item, actionable_date: Date.current, user: user
-      end
-
-      it "returns the record pending cancellation", :aggregate_failures do
-        post solidus_subscriptions.cancel_api_v1_subscription_path(subscription), params: { token: user.spree_api_key }
-        expect(json_resp["state"]).to eq "pending_cancellation"
-      end
-    end
-  end
-
-  describe "#skip" do
-    let(:subscription) { create :subscription, :with_line_item, actionable_date: 1.day.from_now, user: user }
-    let(:expected_date) { "2016-10-27T00:00:00.000Z" }
-
-    before { Timecop.freeze(Date.parse("2016-09-26")) }
-
-    after  { Timecop.return }
-
-    it "returns the updated record", :aggregate_failures do
-      post solidus_subscriptions.skip_api_v1_subscription_path(subscription), params: { token: user.spree_api_key }
-      expect(json_resp["actionable_date"]).to eq expected_date
-    end
-  end
-end