Prevent resource_path request data from overriding nesting

CHANGELOG

@@ -1,3 +1,10 @@
+= 5.0.2
+  * read :resource_path aliases only from request.path_parameters, preventing query and body parameters
+    from removing enclosing resources and bypassing the current user's scoped association.  Route-defined
+    aliases, request.path fallback and memoization are preserved
+  * add regression coverage for query/body overrides, scoped resource lookup and deletion, route-defined
+    aliases and request path memoization
+
 = 5.0.1
   * renamed the internal RequestPathIntrospection#request_path to #rc_request_path: a route like
     `resources :requests` generates a named route helper called request_path, which is included into

lib/resources_controller/request_path_introspection.rb

@@ -13,7 +13,7 @@ module RequestPathIntrospection
     # name lookup - resources_controller would then read the helper's value instead of
     # the path of the current request.  See spec/lib/request_path_introspection_spec.rb.
     def rc_request_path
-      @rc_request_path ||= params[:resource_path] || request.path
+      @rc_request_path ||= request.path_parameters[:resource_path] || request.path
     end
 
     def nesting_request_path

spec/controllers/accounts_controller_spec.rb

@@ -27,9 +27,15 @@ def setup_mocks
     end    
   end
 
-  describe AccountsController, " requesting garbage url" do
-    it "should raise ResourcesController::Specification::NoClassFoundError" do
-      expect { get :show, params: { :resource_path => "/crayzeee" }}.to raise_error(ResourcesController::Specification::NoClassFoundError)
+  describe AccountsController, " requesting a garbage resource_path parameter" do
+    include AccountsControllerSpecHelper
+
+    it "should ignore the parameter and find the current user" do
+      setup_mocks
+      get :show, params: { :resource_path => "/crayzeee" }
+
+      expect(response).to have_http_status(:ok)
+      expect(assigns[:account]).to eq(@current_user)
     end
   end
 

spec/controllers/forums_controller_spec.rb

@@ -150,9 +150,12 @@
     #end
   end
 
-  describe ForumsController, " requesting garbage url" do
-    it "should raise ResourcesController::Specification::NoClassFoundError" do
-      expect { get :index, params: { :resource_path => "/forums/\ncrayzeee" } }.to raise_error(ResourcesController::Specification::NoClassFoundError)
+  describe ForumsController, " requesting a garbage resource_path parameter" do
+    it "should ignore the parameter and find forums" do
+      get :index, params: { :resource_path => "/forums/\ncrayzeee" }
+
+      expect(response).to have_http_status(:ok)
+      expect(assigns[:forums]).to eq(Forum.all)
     end
   end
 

spec/lib/request_path_introspection_spec.rb

@@ -11,18 +11,68 @@ class User < ActiveRecord::Base; end
       @controller = @klass.new
       allow(@controller).to receive(:controller_name).and_return('forums')
       allow(@controller).to receive(:controller_path).and_return('forums')
-      allow(@controller).to receive(:params).and_return({})
-      allow(@controller).to receive(:request).and_return(double('request', :path => '/forums'))
+      @request = ActionDispatch::TestRequest.create
+      @request.path = '/forums'
+      allow(@controller).to receive(:request).and_return(@request)
     end
 
     describe "#rc_request_path" do
       it "should default to request.path" do
         expect(@controller.send(:rc_request_path)).to eq('/forums')
       end
 
-      it " should be params[:resource_path], when set" do
-        @controller.params[:resource_path] = '/foo'
-        expect(@controller.send(:rc_request_path)).to eq('/foo')
+      it "should honor a route-defined resource_path alias" do
+        @request.path = '/'
+        @request.path_parameters = Rails.application.routes.recognize_path('/', :method => :get)
+
+        expect(@controller.send(:rc_request_path)).to eq('/forums')
+      end
+
+      it "should honor a route-defined alias over query parameters" do
+        @request = ActionDispatch::Request.new(Rack::MockRequest.env_for(
+          '/create_forum?resource_path=%2Ftags'
+        ))
+        @request.path_parameters = Rails.application.routes.recognize_path('/create_forum', :method => :get)
+        allow(@controller).to receive(:request).and_return(@request)
+
+        expect(@controller.send(:rc_request_path)).to eq('/forums')
+      end
+
+      it "should ignore query parameters that remove the enclosing resource" do
+        @request = ActionDispatch::Request.new(Rack::MockRequest.env_for(
+          '/account/forums/123.json?resource_path=%2Fforums'
+        ))
+        allow(@controller).to receive(:request).and_return(@request)
+
+        expect(@controller.params[:resource_path]).to eq('/forums')
+        expect(@controller.send(:rc_request_path)).to eq('/account/forums/123.json')
+        expect(@controller.send(:nesting_request_path)).to eq('/account')
+      end
+
+      it "should ignore body parameters that remove the enclosing resource" do
+        @request = ActionDispatch::Request.new(Rack::MockRequest.env_for(
+          '/account/forums/123.json', :method => 'POST', :input => 'resource_path=%2Fforums',
+          'CONTENT_TYPE' => 'application/x-www-form-urlencoded'
+        ))
+        allow(@controller).to receive(:request).and_return(@request)
+
+        expect(@controller.params[:resource_path]).to eq('/forums')
+        expect(@controller.send(:rc_request_path)).to eq('/account/forums/123.json')
+        expect(@controller.send(:nesting_request_path)).to eq('/account')
+      end
+
+      it "should memoize the request path fallback" do
+        expect(@controller.send(:rc_request_path)).to eq('/forums')
+        @request.path = '/tags'
+        expect(@controller.send(:rc_request_path)).to eq('/forums')
+      end
+
+      it "should memoize the route-defined alias" do
+        @request.path_parameters[:resource_path] = '/forums'
+        expect(@controller.send(:rc_request_path)).to eq('/forums')
+        @request.path_parameters[:resource_path] = '/tags'
+        @controller.params[:resource_path] = '/tags'
+        expect(@controller.send(:rc_request_path)).to eq('/forums')
       end
     end
 

spec/requests/request_path_introspection_spec.rb

@@ -0,0 +1,42 @@
+require 'spec_helper'
+
+describe "Resource path scoping", :type => :request do
+  before do
+    @info = Info.create!
+    @account = User.create!(:info => @info)
+    @tag = @info.tags.create!
+    other_info = Info.create!
+    User.create!(:info => other_info)
+    @other_tag = other_info.tags.create!
+    allow_any_instance_of(ApplicationController).to receive(:current_user).and_return(@account)
+  end
+
+  it "should reject a query override that looks up another user's resource" do
+    get "/account/info/tags/#{@other_tag.id}", :params => { :resource_path => '/tags' }
+
+    expect(response).to have_http_status(:not_found)
+  end
+
+  it "should find the current user's resource despite a query override" do
+    get "/account/info/tags/#{@tag.id}", :params => { :resource_path => '/tags' }
+
+    expect(response).to have_http_status(:ok)
+    expect(assigns[:tag]).to eq(@tag)
+    expect(assigns[:account]).to eq(@account)
+  end
+
+  it "should reject a body override that deletes another user's resource" do
+    delete "/account/info/tags/#{@other_tag.id}", :params => { :resource_path => '/tags' }
+
+    expect(response).to have_http_status(:not_found)
+    expect(Tag.exists?(@other_tag.id)).to be true
+  end
+
+  it "should delete the current user's resource despite a body override" do
+    delete "/account/info/tags/#{@tag.id}", :params => { :resource_path => '/tags' }
+
+    expect(response).to redirect_to('/account/info/tags')
+    expect(Tag.exists?(@tag.id)).to be false
+    expect(Tag.exists?(@other_tag.id)).to be true
+  end
+end