Fixes #2851 - Wrong user is used when "X-On-Behalf-Of” header value is an email that starts with digits

app/controllers/application_controller/has_user.rb

@@ -34,15 +34,14 @@ module ApplicationController::HasUser
       raise Exceptions::Forbidden, "Current user has no permission to use 'X-On-Behalf-Of'!"
     end
 
-    # find user for execution based on the header
+    @_user_on_behalf = find_on_behalf_user request.headers['X-On-Behalf-Of'].to_s.downcase.strip
-    %i[id login email].each do |field|
-      @_user_on_behalf = User.find_by(field => request.headers['X-On-Behalf-Of'].to_s.downcase.strip)
-
-      return @_user_on_behalf if @_user_on_behalf
-    end
 
     # no behalf of user found
-    raise Exceptions::Forbidden, "No such user '#{request.headers['X-On-Behalf-Of']}'"
+    if !@_user_on_behalf
+      raise Exceptions::Forbidden, "No such user '#{request.headers['X-On-Behalf-Of']}'"
+    end
+
+    @_user_on_behalf
   end
 
   def current_user_set(user, auth_type = 'session')
@@ -77,4 +76,18 @@ module ApplicationController::HasUser
 
     session[:user_agent] = request.env['HTTP_USER_AGENT']
   end
+
+  # find on behalf user by ID, login or email
+  def find_on_behalf_user(identifier)
+    # ActiveRecord casts string beginning with a numeric characters
+    # to numeric characters by dropping textual bits altogether
+    # thus 123@example.com returns user with ID 123
+    if identifier.match?(%r{^\d+$})
+      user = User.find_by(id: identifier)
+      return user if user
+    end
+
+    # find user for execution based on the header
+    User.where('login = :param OR email = :param', param: identifier).first
+  end
 end

spec/requests/api_auth_on_behalf_of_spec.rb

@@ -241,4 +241,33 @@ RSpec.describe 'Api Auth On Behalf Of', type: :request do
       end
     end
   end
+
+  describe 'user lookup' do
+    it 'does X-On-Behalf-Of auth - user lookup by ID' do
+      authenticated_as(admin, on_behalf_of: customer.id)
+      get '/api/v1/users/me', as: :json
+      expect(json_response.fetch('id')).to be customer.id
+    end
+
+    it 'does X-On-Behalf-Of auth - user lookup by login' do
+      authenticated_as(admin, on_behalf_of: customer.login)
+      get '/api/v1/users/me', as: :json
+      expect(json_response.fetch('id')).to be customer.id
+    end
+
+    it 'does X-On-Behalf-Of auth - user lookup by email' do
+      authenticated_as(admin, on_behalf_of: customer.email)
+      get '/api/v1/users/me', as: :json
+      expect(json_response.fetch('id')).to be customer.id
+    end
+
+    # https://github.com/zammad/zammad/issues/2851
+    it 'does X-On-Behalf-Of auth - user lookup by email even if email starts with a digit' do
+      customer.update! email: "#{agent.id}#{customer.email}"
+
+      authenticated_as(admin, on_behalf_of: customer.email)
+      get '/api/v1/users/me', as: :json
+      expect(json_response.fetch('id')).to be customer.id
+    end
+  end
 end