[38706] Make login lookups in auth source sso case-insensitive

https://community.openproject.org/work_packages/38706

app/controllers/concerns/auth_source_sso.rb

@@ -61,11 +61,11 @@ module AuthSourceSSO
   end
 
   def header_name
-    sso_config && sso_config[:header]
+    sso_config && sso_config[:header].to_s
   end
 
   def header_secret
-    sso_config && sso_config[:secret]
+    sso_config && sso_config[:secret].to_s
   end
 
   def header_optional?
@@ -104,7 +104,10 @@ module AuthSourceSSO
   end
 
   def find_user_from_auth_source(login)
-    User.where(login: login).where.not(auth_source_id: nil).first
+    User
+      .by_login(login)
+      .where.not(auth_source_id: nil)
+      .first
   end
 
   def create_user_from_auth_source(login)

app/models/user.rb

@@ -380,15 +380,6 @@ class User < Principal
     pref.comments_in_reverse_order?
   end
 
-  # Find a user account by matching the exact login and then a case-insensitive
-  # version.  Exact matches will be given priority.
-  def self.find_by_login(login)
-    # First look for an exact match
-    user = find_by(login: login)
-    # Fail over to case-insensitive if none was found
-    user || where(["LOWER(login) = ?", login.to_s.downcase]).first
-  end
-
   def self.find_by_rss_key(key)
     return nil unless Setting.feeds_enabled?
 

app/models/users/scopes/find_by_login.rb

@@ -34,8 +34,17 @@ module Users::Scopes
     extend ActiveSupport::Concern
 
     class_methods do
-      def self.find_by_login(login)
-        where(["LOWER(login) = ?", login.to_s.downcase]).first
+      def by_login(login)
+        where(["LOWER(login) = ?", login.to_s.downcase])
+      end
+
+      # Find a user scope by matching the exact login and then a case-insensitive
+      # version. Exact matches will be given priority.
+      def find_by_login(login)
+        # First look for an exact match
+        user = find_by(login: login)
+        # Fail over to case-insensitive if none was found
+        user || by_login(login).first
       end
     end
   end

spec/controllers/concerns/auth_source_sso_spec.rb

@@ -45,12 +45,20 @@ describe MyController, type: :controller do
   let!(:user) { FactoryBot.create :user, login: login, auth_source_id: auth_source.id, last_login_on: 5.days.ago }
   let(:login) { "h.wurst" }
 
+  shared_examples 'should log in the user' do
+    it "logs in given user" do
+      expect(response).to redirect_to my_page_path
+      expect(user.reload.last_login_on).to be_within(10.seconds).of(Time.current)
+      expect(session[:user_id]).to eq user.id
+    end
+  end
+
   shared_examples "auth source sso failure" do
     def attrs(user)
       user.attributes.slice(:login, :mail, :auth_source_id)
     end
 
-    it "should redirect to AccountController#sso to show the error" do
+    it "redirects to AccountController#sso to show the error" do
       expect(response).to redirect_to "/sso"
 
       failure = session[:auth_source_sso_failure]
@@ -94,19 +102,24 @@ describe MyController, type: :controller do
       get :account
     end
 
-    it "should log in given user" do
-      expect(response).to redirect_to my_page_path
-      expect(session[:user_id]).to eq user.id
-    end
+    it_behaves_like 'should log in the user'
 
     context 'when the secret being null' do
       let(:secret) { nil }
 
-      it "should log in given user" do
-        expect(response).to redirect_to my_page_path
-        expect(user.reload.last_login_on).to be_within(10.seconds).of(Time.now)
-        expect(session[:user_id]).to eq user.id
-      end
+      it_behaves_like 'should log in the user'
+    end
+
+    context 'when the secret is a number' do
+      let(:secret) { 42 }
+
+      it_behaves_like 'should log in the user'
+    end
+
+    context 'when the header values does not match the case' do
+      let(:login) { 'H.wUrSt' }
+
+      it_behaves_like 'should log in the user'
     end
 
     context 'when the user is invited' do
@@ -167,7 +180,7 @@ describe MyController, type: :controller do
 
       expect(service).to have_received(:call).with(other_user)
       expect(response).to redirect_to my_page_path
-      expect(user.reload.last_login_on).to be_within(10.seconds).of(Time.now.utc)
+      expect(user.reload.last_login_on).to be_within(10.seconds).of(Time.current)
       expect(session[:user_id]).to eq user.id
       expect(session[:updated_at]).to be > session_update_time
     end