From 180b6deb4d98f34db8c2c2d1a06760ea0c010630 Mon Sep 17 00:00:00 2001 From: Benjamin ter Kuile Date: Thu, 6 Dec 2012 12:21:38 +0100 Subject: [PATCH] Secure product_categories and add example test for other supplier resources and fix hack attempts included in test --- Gemfile.lock | 2 +- app/controllers/application_controller.rb | 7 +- .../product_categories_controller.rb | 8 +- app/models/product_category.rb | 1 + app/views/dashboard/404.html.slim | 2 + .../suppliers/product_categories_spec.rb | 21 +++ .../supplier_main_board_spec.rb_spec.rb | 1 - .../product_categories_controller_spec.rb | 173 ++++++++++++++++++ 8 files changed, 207 insertions(+), 8 deletions(-) create mode 100644 app/views/dashboard/404.html.slim create mode 100644 spec/acceptance/suppliers/product_categories_spec.rb create mode 100644 spec/controllers/suppliers/product_categories_controller_spec.rb diff --git a/Gemfile.lock b/Gemfile.lock index 5e3e5d51..88f57935 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -9,7 +9,7 @@ GIT GIT remote: git://github.com/bterkuile/simply_stored.git - revision: 04f58b15bb308420f78c66d27ce4d4d3b58183ff + revision: fe749c792e303817f825fa4964936cc68a62b14b specs: simply_stored (1.0.0) activesupport diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 682ac621..829c0df2 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,10 +1,9 @@ class ApplicationController < ActionController::Base before_filter :set_locale layout :layout_by_resource - - protect_from_forgery + rescue_from SimplyStored::RecordNotFound, with: :show_404 private def broadcast_user(uid, event, data = {}) @@ -62,4 +61,8 @@ private "http://#{Rails.env.production? ? 'events.qwaiter.com' : 'localhost'}:9296/faye" end helper_method :event_host + + def show_404 + render 'dashboard/404', layout: true, status: 404 + end end diff --git a/app/controllers/suppliers/product_categories_controller.rb b/app/controllers/suppliers/product_categories_controller.rb index 9e8c10b8..4abad56d 100644 --- a/app/controllers/suppliers/product_categories_controller.rb +++ b/app/controllers/suppliers/product_categories_controller.rb @@ -15,7 +15,7 @@ module Suppliers # GET /product_categories/1 # GET /product_categories/1.json def show - @product_category = ProductCategory.find(params[:id]) + @product_category = ProductCategory.find_by_supplier_id_and_id!(current_supplier.id, params[:id]) respond_to do |format| format.html # show.html.erb @@ -59,7 +59,7 @@ module Suppliers # PUT /product_categories/1 # PUT /product_categories/1.json def update - @product_category = ProductCategory.find(params[:id]) + @product_category = ProductCategory.find_by_supplier_id_and_id!(current_supplier.id, params[:id]) respond_to do |format| if @product_category.update_attributes(params[:product_category]) @@ -75,7 +75,7 @@ module Suppliers # DELETE /product_categories/1 # DELETE /product_categories/1.json def destroy - @product_category = ProductCategory.find(params[:id]) + @product_category = ProductCategory.find_by_supplier_id_and_id!(current_supplier.id, params[:id]) @product_category.destroy respond_to do |format| @@ -87,7 +87,7 @@ module Suppliers # POST /supplier/product_categories/sort # params ~= product_category: ['abc', 'def', 'another id', ...] def sort - @product_categories = ProductCategory.find(params[:product_category]) + @product_categories = ProductCategory.find(params[:product_category]).select{|pc| pc.supplier_id == current_supplier.id} # The select is hack prevention 1.upto(@product_categories.size){|i| @product_categories[i-1].position = i} CouchPotato.database.couchrest_database.bulk_save(@product_categories) render nothing: true diff --git a/app/models/product_category.rb b/app/models/product_category.rb index c1c6b411..d35dec6f 100644 --- a/app/models/product_category.rb +++ b/app/models/product_category.rb @@ -12,6 +12,7 @@ class ProductCategory validates :name, presence: true validates :position, numericality: true validates :supplier_id, presence: true + view :by_supplier_id_and_id, key: [:supplier_id, :_id] def self.for_user(user, options = {}) table = options[:table] diff --git a/app/views/dashboard/404.html.slim b/app/views/dashboard/404.html.slim new file mode 100644 index 00000000..df24bd31 --- /dev/null +++ b/app/views/dashboard/404.html.slim @@ -0,0 +1,2 @@ +h2 404 +p The requested page could not be found diff --git a/spec/acceptance/suppliers/product_categories_spec.rb b/spec/acceptance/suppliers/product_categories_spec.rb new file mode 100644 index 00000000..3e4505d1 --- /dev/null +++ b/spec/acceptance/suppliers/product_categories_spec.rb @@ -0,0 +1,21 @@ +require 'acceptance/acceptance_helper' + +feature 'Supplier product categories spec', %q{ + In order to manage product categories + As a supplier + I want to have control over product categories and associated products +} do + background do + create_supplier 'supplier@qwaiter.com' + end + context "GET #index" do + background do + @product_category = create :product_category, supplier: @supplier + end + scenario "I can see a drag handle having class .handle for sorting the product categories" do + login_supplier_as 'supplier@qwaiter.com' + visit '/supplier/product_categories' + page.should have_selector '.handle' + end + end +end diff --git a/spec/acceptance/suppliers/supplier_main_board_spec.rb_spec.rb b/spec/acceptance/suppliers/supplier_main_board_spec.rb_spec.rb index 62f9d971..77c8fe1a 100644 --- a/spec/acceptance/suppliers/supplier_main_board_spec.rb_spec.rb +++ b/spec/acceptance/suppliers/supplier_main_board_spec.rb_spec.rb @@ -1,6 +1,5 @@ require 'acceptance/acceptance_helper' -@javascript feature 'Supplier main board spec.rb', %q{ In order to manage active list and orders As a supplier diff --git a/spec/controllers/suppliers/product_categories_controller_spec.rb b/spec/controllers/suppliers/product_categories_controller_spec.rb new file mode 100644 index 00000000..f98ce70b --- /dev/null +++ b/spec/controllers/suppliers/product_categories_controller_spec.rb @@ -0,0 +1,173 @@ +# encoding: UTF-8 +require 'spec_helper' + +describe Suppliers::ProductCategoriesController do + before :each do + @supplier = Supplier.find_by_email('supplier@qwaiter.com') || Supplier.create(name: 'Supplier', email: 'supplier@qwaiter.com', password: 'secret') + sign_in @supplier + end + + describe "GET #index" do + it "populates an array of product_categories" do + product_category = create :product_category, supplier: @supplier + get :index + assigns(:product_categories).should eq([product_category]) + end + + it "does not include product_categories from another supplier" do + product_category1 = create :product_category, supplier: @supplier + product_category2 = create :product_category + get :index + assigns(:product_categories).should eq([product_category1]) + end + + it "should render without errors when no objects are present" do + get :index + expect{ render_template :index }.not_to raise_error + end + + it "renders the :index view" do + get :index + response.should render_template :index + end + end + + describe "GET #show" do + it "assigns the requested product_category to @product_category" do + product_category = create :product_category, supplier: @supplier + get :show, id: product_category + assigns(:product_category).should eq(product_category) + end + + it "should not display a product_category of another supplier" do + product_category = create :product_category + get :show, id: product_category + response.status.should == 404 + end + + it "renders the #show view" do + product_category = create :product_category, supplier: @supplier + get :show, id: product_category + response.should render_template :show + end + end + + describe "GET #new" do + it "assigns a new product_category to @product_category" do + get :new + assigns(:product_category).should be_a ProductCategory + end + + it "renders the #show view" do + get :new + response.should render_template :new + end + end + + describe "POST #create" do + context "with valid attributes" do + it "creates a new product_category" do + expect{ + post :create, product_category: attributes_for(:product_category, supplier: @supplier) + }.to change(ProductCategory, :count).by(1) + end + + it "redirects to the new product_category" do + post :create, product_category: attributes_for(:product_category, supplier: @supplier) + response.should redirect_to [:suppliers, ProductCategory.last] + end + + it "should not be possible to create a product category for another supplier" do + supplier2 = create :supplier + post :create, product_category: attributes_for(:product_category, name: 'Trying to hack', supplier: supplier2) + ProductCategory.find_by_name('Trying to hack').supplier_id.should == @supplier.id + end + end + + context "with invalid attributes" do + it "does not save the new product_category" do + expect{ + post :create, product_category: {name: ''} + }.to_not change(ProductCategory, :count) + end + + it "re-renders the new method" do + post :create, product_category: {name: ''} + response.should render_template :new + end + end + end + + describe 'PUT update' do + before :each do + @product_category = create :product_category, supplier: @supplier + end + + context "valid attributes" do + it "located the requested product_category" do + put :update, id: @product_category, product_category: attributes_for(:product_category, supplier: @supplier) + @product_category.reload + assigns(:product_category).should eq(@product_category) + end + + it "changes @product_category's attributes" do + put :update, id: @product_category, product_category: attributes_for(:product_category, name: "ChangedByTest", supplier: @supplier) + @product_category.reload + @product_category.name.should eq("ChangedByTest") + end + + it "redirects to the updated product_category" do + put :update, id: @product_category, product_category: attributes_for(:product_category, supplier: @supplier) + response.should redirect_to [:suppliers, @product_category] + end + it "should not be possible to update a product category to another supplier" do + supplier2 = create :supplier + put :update, id: @product_category, product_category: attributes_for(:product_category, name: "Trying to hack", supplier: supplier2) + ProductCategory.find_by_name('Trying to hack').supplier_id.should == @supplier.id + end + + it "should not be possible to update a product_category of another supplier" do + product_category = create :product_category, name: 'Other supplier product_category' + put :update, id: product_category, product_category: {name: "Trying to hack"} + product_category.reload + product_category.name.should == 'Other supplier product_category' + end + end + + context "invalid attributes" do + it "locates the requested product_category" do + put :update, id: @product_category, product_category: {name: ''} + assigns(:product_category).should eq(@product_category) + end + + it "re-renders the edit method" do + put :update, id: @product_category, product_category: {name: ''} + response.should render_template :edit + end + end + end + + describe 'DELETE destroy' do + before :each do + @product_category = create :product_category, supplier: @supplier + end + + it "deletes the product_category" do + expect{ + delete :destroy, id: @product_category + }.to change(ProductCategory, :count).by(-1) + end + + it "redirects to product_categories#index" do + delete :destroy, id: @product_category + response.should redirect_to [:suppliers, :product_categories] + end + + it "should not be possible to delete a product category of another supplier" do + product_category = create :product_category + expect{ + delete :destroy, id: product_category + }.to_not change(ProductCategory, :count) + end + end +end