SCIM GET /Users supports requests without a filter

Allows all SCIM identities to be returned, paginated.

SCIM clients will sometimes send a request without a filter, using
pagination to limit results instead. Previously we would raise an error
when a filter wasn't provided, meaning we were unable to process those
requests.

Our initial approach was an MVC for Azure support, which required filter
by id. Filters are optional however, and we need to conform to the spec
more closely for Okta comaptibility.

See: https://tools.ietf.org/html/rfc7644#section-3.4.2.2

doc/api/scim.md

@@ -21,7 +21,7 @@ Parameters:
 
 | Attribute | Type    | Required | Description                                                                                                                             |
 |:----------|:--------|:---------|:----------------------------------------------------------------------------------------------------------------------------------------|
-| `filter`   | string  | yes     | A [filter](#available-filters) expression. |
+| `filter`   | string  | no     | A [filter](#available-filters) expression. |
 | `group_path` | string | yes    | Full path to the group. |
 | `startIndex` | integer | no    | The 1-based index indicating where to start returning results from. A value of less than one will be interpreted as 1. |
 | `count` | integer | no    | Desired maximum number of query results. |

ee/app/finders/scim_finder.rb

@@ -3,14 +3,40 @@
 class ScimFinder
   attr_reader :saml_provider
 
+  UnsupportedFilter = Class.new(StandardError)
+
   def initialize(group)
     @saml_provider = group&.saml_provider
   end
 
   def search(params)
     return Identity.none unless saml_provider&.enabled?
+    return saml_provider.identities if unfiltered?(params)
+
+    filter_identities(params)
+  end
+
+  private
+
+  def unfiltered?(params)
+    params[:filter].blank?
+  end
 
+  def filter_identities(params)
     parser = EE::Gitlab::Scim::ParamsParser.new(params)
+
+    if eq_filter_on_extern_uid?(parser)
+      by_extern_uid(parser)
+    else
+      raise UnsupportedFilter
+    end
+  end
+
+  def eq_filter_on_extern_uid?(parser)
+    parser.filter_operator == :eq && parser.filter_params[:extern_uid].present?
+  end
+
+  def by_extern_uid(parser)
     Identity.where_group_saml_uid(saml_provider, parser.filter_params[:extern_uid])
   end
 end

ee/changelogs/unreleased/jej-scim-without-filter.yml

+---
+title: SCIM GET /Users supports requests without a filter
+merge_request: 19421
+author:
+type: changed

ee/lib/api/scim.rb

@@ -95,8 +95,6 @@ module API
           detail 'This feature was introduced in GitLab 11.10.'
         end
         get do
-          scim_error!(message: 'Missing filter params') unless params[:filter]
-
           group = find_and_authenticate_group!(params[:group])
 
           results = ScimFinder.new(group).search(params)
@@ -106,6 +104,8 @@ module API
 
           result_set = { resources: response_page, total_results: results.count, items_per_page: per_page(params[:count]), start_index: params[:startIndex] }
           present result_set, with: ::EE::Gitlab::Scim::Users
+        rescue ScimFinder::UnsupportedFilter
+          scim_error!(message: 'Unsupported Filter')
         end
 
         desc 'Get a SAML user' do

ee/spec/finders/scim_finder_spec.rb

@@ -30,6 +30,7 @@ describe ScimFinder do
 
       context 'with an eq filter' do
         let!(:identity) { create(:group_saml_identity, saml_provider: saml_provider) }
+        let!(:other_identity) { create(:group_saml_identity, saml_provider: saml_provider) }
 
         it 'allows identity lookup by id/externalId' do
           expect(finder.search(filter: "id eq #{identity.extern_uid}")).to be_a ActiveRecord::Relation
@@ -37,6 +38,20 @@ describe ScimFinder do
           expect(finder.search(filter: "externalId eq #{identity.extern_uid}").first).to eq identity
         end
       end
+
+      it 'returns all related identities if there is no filter' do
+        create_list(:group_saml_identity, 2, saml_provider: saml_provider)
+
+        expect(finder.search({}).count).to eq 2
+      end
+
+      it 'raises an error if the filter is unsupported' do
+        expect { finder.search(filter: 'id ne 1').count }.to raise_error(ScimFinder::UnsupportedFilter)
+      end
+
+      it 'raises an error if the attribute path is unsupported' do
+        expect { finder.search(filter: 'userName eq "name"').count }.to raise_error(ScimFinder::UnsupportedFilter)
+      end
     end
   end
 end

ee/spec/requests/api/scim_spec.rb

@@ -23,13 +23,21 @@ describe API::Scim do
         end
       end
 
-      it 'responds with an error if there is no filter' do
+      it 'responds with paginated users when there is no filter' do
         get scim_api("scim/v2/groups/#{group.full_path}/Users")
 
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response['Resources']).not_to be_empty
+        expect(json_response['totalResults']).to eq(Identity.count)
+      end
+
+      it 'responds with an error for unsupported filters' do
+        get scim_api("scim/v2/groups/#{group.full_path}/Users?filter=id ne \"#{identity.extern_uid}\"")
+
         expect(response).to have_gitlab_http_status(412)
       end
 
-      context 'existing user' do
+      context 'existing user matches filter' do
         it 'responds with 200' do
           get scim_api("scim/v2/groups/#{group.full_path}/Users?filter=id eq \"#{identity.extern_uid}\"")
 
@@ -47,7 +55,7 @@ describe API::Scim do
         end
       end
 
-      context 'no user' do
+      context 'no user matches filter' do
         it 'responds with 200' do
           get scim_api("scim/v2/groups/#{group.full_path}/Users?filter=id eq \"nonexistent\"")