fix case sensitivey of sis_login_id lookups in the API

fixes CNVS-23835

test plan:
 * use any API endpoint that takes a user id (and thus sis_login_id)
 * change the case of the login id (all uppercase, etc.) in the URL
 * it should still find the user

Change-Id: I0821b5d7213142aefa7e26699c26b981845a220b
Reviewed-on: https://gerrit.instructure.com/64586
Tested-by: Jenkins
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>

config/initializers/postgresql_adapter.rb

@@ -1,3 +1,6 @@
+class QuotedValue < String
+end
+
 module PostgreSQLAdapterExtensions
   def readonly?(table = nil, column = nil)
     return @readonly unless @readonly.nil?
@@ -181,6 +184,8 @@ module PostgreSQLAdapterExtensions
   # BigDecimals, Times, etc.) into those representations. Raise
   # ActiveRecord::StatementInvalid for any other non-integer things.
   def quote(value, column = nil)
+    return value if value.is_a?(QuotedValue)
+
     if column && column.type == :integer && !value.respond_to?(:quoted_id)
       case value
         when String, ActiveSupport::Multibyte::Chars, nil, true, false

lib/api.rb

@@ -104,7 +104,10 @@ module Api
         :scope => 'root_account_id' }.freeze,
     'users' =>
       { :lookups => { 'sis_user_id' => 'pseudonyms.sis_user_id',
-                      'sis_login_id' => 'pseudonyms.unique_id',
+                      'sis_login_id' => {
+                          column: 'LOWER(pseudonyms.unique_id)',
+                          transform: ->(id) { QuotedValue.new("LOWER(#{Pseudonym.connection.quote(id)})") }
+                      },
                       'id' => 'users.id',
                       'sis_integration_id' => 'pseudonyms.integration_id',
                       'lti_context_id' => 'users.lti_context_id',
@@ -156,6 +159,10 @@ module Api
 
     column = lookups[sis_column]
     return nil, nil unless column
+    if column.is_a?(Hash)
+      sis_id = column[:transform].call(sis_id)
+      column = column[:column]
+    end
     return column, sis_id
   end
 

spec/lib/api_spec.rb

@@ -54,6 +54,16 @@ describe Api do
       expect(@api.api_find(User, "sis_login_id:sis_user_1@example.com")).to eq @user
     end
 
+    it 'looks for login ids case insensitively' do
+      @user = user_with_pseudonym :username => "sis_user_1@example.com"
+      expect(@api.api_find(User, "sis_login_id:SIS_USER_1@example.com")).to eq @user
+    end
+
+    it 'properly quotes login ids' do
+      @user = user_with_pseudonym :username => "user 'a'"
+      expect(@api.api_find(User, "sis_login_id:user 'a'")).to eq @user
+    end
+
     it 'should not find record from other account' do
       account = Account.create(name: 'new')
       @user = user_with_pseudonym username: "sis_user_1@example.com", account: account
@@ -64,7 +74,7 @@ describe Api do
       account = Account.create(name: 'new')
       @user = user_with_pseudonym username: "sis_user_1@example.com", account: account
       Api.expects(:sis_parse_id).with("root_account:school:sis_login_id:sis_user_1@example.com", anything, anything).
-          returns(['pseudonyms.unique_id', ["sis_user_1@example.com", account]])
+          returns(['LOWER(pseudonyms.unique_id)', [QuotedValue.new("LOWER('sis_user_1@example.com')"), account]])
       expect(@api.api_find(User, "root_account:school:sis_login_id:sis_user_1@example.com")).to eq @user
     end
 
@@ -374,7 +384,7 @@ describe Api do
     end
 
     it 'should handle hex_encoded sis_fields' do
-      expect(Api.sis_parse_id("hex:sis_login_id:7369737573657233406578616d706c652e636f6d", @lookups)).to eq ["pseudonyms.unique_id", "sisuser3@example.com"]
+      expect(Api.sis_parse_id("hex:sis_login_id:7369737573657233406578616d706c652e636f6d", @lookups)).to eq ["LOWER(pseudonyms.unique_id)", "LOWER('sisuser3@example.com')"]
       expect(Api.sis_parse_id("hex:sis_user_id:7369737573657234406578616d706c652e636f6d", @lookups)).to eq ["pseudonyms.sis_user_id", "sisuser4@example.com"]
     end
 
@@ -389,7 +399,7 @@ describe Api do
     end
 
     it 'should handle plain sis_fields' do
-      expect(Api.sis_parse_id("sis_login_id:sisuser3@example.com", @lookups)).to eq ["pseudonyms.unique_id", "sisuser3@example.com"]
+      expect(Api.sis_parse_id("sis_login_id:sisuser3@example.com", @lookups)).to eq ["LOWER(pseudonyms.unique_id)", "LOWER('sisuser3@example.com')"]
       expect(Api.sis_parse_id("sis_user_id:sisuser4", @lookups)).to eq ["pseudonyms.sis_user_id", "sisuser4"]
     end
 
@@ -407,8 +417,8 @@ describe Api do
     it 'should handle surrounding whitespace' do
       expect(Api.sis_parse_id("\t10  ", @lookups)).to eq ["users.id", 10]
       expect(Api.sis_parse_id("\t10\n", @lookups)).to eq ["users.id", 10]
-      expect(Api.sis_parse_id("  hex:sis_login_id:7369737573657233406578616d706c652e636f6d     ", @lookups)).to eq ["pseudonyms.unique_id", "sisuser3@example.com"]
-      expect(Api.sis_parse_id("  sis_login_id:sisuser3@example.com\t", @lookups)).to eq ["pseudonyms.unique_id", "sisuser3@example.com"]
+      expect(Api.sis_parse_id("  hex:sis_login_id:7369737573657233406578616d706c652e636f6d     ", @lookups)).to eq ["LOWER(pseudonyms.unique_id)", "LOWER('sisuser3@example.com')"]
+      expect(Api.sis_parse_id("  sis_login_id:sisuser3@example.com\t", @lookups)).to eq ["LOWER(pseudonyms.unique_id)", "LOWER('sisuser3@example.com')"]
     end
   end
 
@@ -434,7 +444,7 @@ describe Api do
     end
 
     it 'should work with mixed sis id types' do
-      expect(Api.sis_parse_ids([1,2,"sis_user_id:U1",3,"sis_user_id:U2","sis_user_id:U3","sis_login_id:A1"], @lookups)).to eq({ "users.id" => [1, 2, 3], "pseudonyms.sis_user_id" => ["U1", "U2", "U3"], "pseudonyms.unique_id" => ["A1"] })
+      expect(Api.sis_parse_ids([1,2,"sis_user_id:U1",3,"sis_user_id:U2","sis_user_id:U3","sis_login_id:A1"], @lookups)).to eq({ "users.id" => [1, 2, 3], "pseudonyms.sis_user_id" => ["U1", "U2", "U3"], "LOWER(pseudonyms.unique_id)" => ["LOWER('A1')"] })
     end
 
     it 'should skip invalid things' do
@@ -532,7 +542,7 @@ describe Api do
     it 'should correctly capture user lookups' do
       lookups = Api.sis_find_sis_mapping_for_collection(User)[:lookups]
       expect(Api.sis_parse_id("sis_user_id:1", lookups)).to eq ["pseudonyms.sis_user_id", "1"]
-      expect(Api.sis_parse_id("sis_login_id:1", lookups)).to eq ["pseudonyms.unique_id", "1"]
+      expect(Api.sis_parse_id("sis_login_id:1", lookups)).to eq ["LOWER(pseudonyms.unique_id)", "LOWER('1')"]
       expect(Api.sis_parse_id("sis_course_id:1", lookups)).to eq [nil, nil]
       expect(Api.sis_parse_id("sis_term_id:1", lookups)).to eq [nil, nil]
       expect(Api.sis_parse_id("sis_account_id:1", lookups)).to eq [nil, nil]