Commit d09f4a2

mo <mo@mokhan.ca>
2019-05-09 21:18:28
add some bounds checking to page params
1 parent ee82c56
Changed files (2)
app
controllers
spec
requests
app/controllers/scim/v2/users_controller.rb
@@ -3,17 +3,15 @@
 module Scim
   module V2
     class UsersController < ::Scim::Controller
-      PAGE_SIZE=25
       rescue_from ActiveRecord::RecordNotFound do |_error|
         @resource_id = params[:id] if params[:id].present?
         render "record_not_found", status: :not_found
       end
 
       def index
-        @page = params.fetch(:startIndex, 1).to_i
-        @page_size = params.fetch(:count, Scim::V2::UsersController::PAGE_SIZE).to_i
+        @page, @page_size = page_params
         @total = User.count
-        @users = @page_size >= 0 ? User.offset(@page - 1).limit(@page_size) : User.none
+        @users = User.offset(@page - 1).limit(@page_size)
         render formats: :scim, status: :ok
       end
 
@@ -46,6 +44,20 @@ module Scim
         params.permit(:schemas, :userName, :locale, :timezone)
       end
 
+      def page_params
+        [
+          page_param(:startIndex, default: 1, bottom: 1, top: 100),
+          page_param(:count, default: 25, bottom: 0, top: 25)
+        ]
+      end
+
+      def page_param(key, default:, bottom: 0, top: 250)
+        actual = params.fetch(key, default).to_i
+        return bottom if actual < bottom
+        return top if actual > top
+        actual
+      end
+
       def repository(container = Spank::IOC)
         container.resolve(:user_repository)
       end
spec/requests/scim/v2/users_spec.rb
@@ -119,7 +119,7 @@ describe '/scim/v2/users' do
       specify { expect(response.body).to be_present }
       specify { expect(json[:schemas]).to match_array([Scim::Kit::V2::Messages::LIST_RESPONSE]) }
       specify { expect(json[:totalResults]).to be(1) }
-      specify { expect(json[:startIndex]).to be(0) }
+      specify { expect(json[:startIndex]).to be(1) }
       specify { expect(json[:itemsPerPage]).to be(25) }
       specify { expect(json[:Resources]).not_to be_empty }
       specify { expect(json[:Resources][0][:id]).to eql(user.to_param) }
@@ -153,14 +153,22 @@ describe '/scim/v2/users' do
         before { get "/scim/v2/users", params: { count: 0 }, headers: headers }
 
         specify { expect(response).to have_http_status(:ok) }
-        specify { expect(response.headers['Content-Type']).to eql('application/scim+json') }
-        specify { expect(response.body).to be_present }
         specify { expect(json[:schemas]).to match_array([Scim::Kit::V2::Messages::LIST_RESPONSE]) }
         specify { expect(json[:totalResults]).to be(users.count + 1) }
         specify { expect(json[:startIndex]).to eql(1) }
         specify { expect(json[:itemsPerPage]).to eql(0) }
         specify { expect(json[:Resources]).to be_empty }
       end
+
+      context "when requesting a count of negative results" do
+        before { get "/scim/v2/users", params: { count: -1 }, headers: headers }
+
+        specify { expect(response).to have_http_status(:ok) }
+        specify { expect(json[:totalResults]).to be(users.count + 1) }
+        specify { expect(json[:startIndex]).to eql(1) }
+        specify { expect(json[:itemsPerPage]).to eql(0) }
+        specify { expect(json[:Resources]).to be_empty }
+      end
     end
 
     xcontext "when fetching specific attributes" do