Merge branch 'jej/scim-without-filter' into 'master'

SCIM GET /Users supports requests without a filter

Closes #35209

See merge request gitlab-org/gitlab!19421

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\"")