Make CI/Oauth/rate limiting reusable

app/controllers/projects/git_http_controller.rb

@@ -41,82 +41,20 @@ class Projects::GitHttpController < Projects::ApplicationController
     return if project && project.public? && upload_pack?
 
     authenticate_or_request_with_http_basic do |login, password|
-      return @ci = true if valid_ci_request?(login, password)
+      user, type = Gitlab::Auth.find(login, password, project: project, ip: request.ip)
 
-      @user = Gitlab::Auth.new.find(login, password)
-      @user ||= oauth_access_token_check(login, password)
-      rate_limit_ip!(login, @user)
-    end
-  end
-
-  def ensure_project_found!
-    render_not_found if project.blank?
-  end
-
-  def valid_ci_request?(login, password)
-    matched_login = /(?<service>^[a-zA-Z]*-ci)-token$/.match(login)
-
-    unless project && matched_login.present? && upload_pack?
-      return false
-    end
-
-    underscored_service = matched_login['service'].underscore
-
-    if underscored_service == 'gitlab_ci'
-      project && project.valid_build_token?(password)
-    elsif Service.available_services_names.include?(underscored_service)
-      # We treat underscored_service as a trusted input because it is included
-      # in the Service.available_services_names whitelist.
-      service_method = "#{underscored_service}_service"
-      service = project.send(service_method)
-
-      service && service.activated? && service.valid_token?(password)
-    end
-  end
-
-  def oauth_access_token_check(login, password)
-    if login == "oauth2" && upload_pack? && password.present?
-      token = Doorkeeper::AccessToken.by_token(password)
-      token && token.accessible? && User.find_by(id: token.resource_owner_id)
-    end
-  end
-
-  def rate_limit_ip!(login, user)
-    # If the user authenticated successfully, we reset the auth failure count
-    # from Rack::Attack for that IP. A client may attempt to authenticate
-    # with a username and blank password first, and only after it receives
-    # a 401 error does it present a password. Resetting the count prevents
-    # false positives from occurring.
-    #
-    # Otherwise, we let Rack::Attack know there was a failed authentication
-    # attempt from this IP. This information is stored in the Rails cache
-    # (Redis) and will be used by the Rack::Attack middleware to decide
-    # whether to block requests from this IP.
-
-    config = Gitlab.config.rack_attack.git_basic_auth
-    return user unless config.enabled
-
-    if user
-      # A successful login will reset the auth failure count from this IP
-      Rack::Attack::Allow2Ban.reset(request.ip, config)
+      if (type == :ci) && upload_pack?
+        @ci = true
+      elsif (type == :oauth) && !upload_pack?
+        @user = nil
       else
-      banned = Rack::Attack::Allow2Ban.filter(request.ip, config) do
-        # Unless the IP is whitelisted, return true so that Allow2Ban
-        # increments the counter (stored in Rails.cache) for the IP
-        if config.ip_whitelist.include?(request.ip)
-          false
-        else
-          true
-        end
+        @user = user
       end
-
-      if banned
-        Rails.logger.info "IP #{request.ip} failed to login " \
-          "as #{login} but has been temporarily banned from Git auth"
     end
   end
 
-    user
+  def ensure_project_found!
+    render_not_found if project.blank?
   end
 
   def project

config/initializers/doorkeeper.rb

@@ -12,7 +12,7 @@ Doorkeeper.configure do
   end
 
   resource_owner_from_credentials do |routes|
-    Gitlab::Auth.new.find(params[:username], params[:password])
+    Gitlab::Auth.find_by_master_or_ldap(params[:username], params[:password])
   end
 
   # If you want to restrict access to the web interface for adding oauth authorized applications, you need to declare the block below.

lib/api/session.rb

@@ -11,8 +11,12 @@ module API
     # Example Request:
     #  POST /session
     post "/session" do
-      auth = Gitlab::Auth.new
-      user = auth.find(params[:email] || params[:login], params[:password])
+      user, _ = Gitlab::Auth.find(
+        params[:email] || params[:login],
+        params[:password],
+        project: nil,
+        ip: request.ip
+      )
 
       return unauthorized! unless user
       present user, with: Entities::UserLogin

lib/gitlab/auth.rb

 module Gitlab
   class Auth
-    def find(login, password)
+    class << self
+      def find(login, password, project:, ip:)
+        raise "Must provide an IP for rate limiting" if ip.nil?
+
+        user, type = nil, nil
+
+        if valid_ci_request?(login, password, project)
+          type = :ci
+        elsif user = find_by_master_or_ldap(login, password)
+          type = :master_or_ldap
+        elsif user = oauth_access_token_check(login, password)
+          type = :oauth
+        end
+
+        rate_limit!(ip, success: !!user || (type == :ci), login: login)
+        [user, type]
+      end
+
+      def find_by_master_or_ldap(login, password)
         user = User.by_login(login)
 
         # If no user is found, or it's an LDAP server, try LDAP.
@@ -14,5 +32,70 @@ module Gitlab
           user if user.valid_password?(password)
         end
       end
+
+      private
+
+      def valid_ci_request?(login, password, project)
+        matched_login = /(?<service>^[a-zA-Z]*-ci)-token$/.match(login)
+
+        return false unless project && matched_login.present?
+
+        underscored_service = matched_login['service'].underscore
+
+        if underscored_service == 'gitlab_ci'
+          project && project.valid_build_token?(password)
+        elsif Service.available_services_names.include?(underscored_service)
+          # We treat underscored_service as a trusted input because it is included
+          # in the Service.available_services_names whitelist.
+          service_method = "#{underscored_service}_service"
+          service = project.send(service_method)
+
+          service && service.activated? && service.valid_token?(password)
+        end
+      end
+
+      def oauth_access_token_check(login, password)
+        if login == "oauth2" && password.present?
+          token = Doorkeeper::AccessToken.by_token(password)
+          token && token.accessible? && User.find_by(id: token.resource_owner_id)
+        end
+      end
+
+      def rate_limit!(ip, success:, login:)
+        # If the user authenticated successfully, we reset the auth failure count
+        # from Rack::Attack for that IP. A client may attempt to authenticate
+        # with a username and blank password first, and only after it receives
+        # a 401 error does it present a password. Resetting the count prevents
+        # false positives from occurring.
+        #
+        # Otherwise, we let Rack::Attack know there was a failed authentication
+        # attempt from this IP. This information is stored in the Rails cache
+        # (Redis) and will be used by the Rack::Attack middleware to decide
+        # whether to block requests from this IP.
+
+        config = Gitlab.config.rack_attack.git_basic_auth
+        return unless config.enabled
+
+        if success
+          # A successful login will reset the auth failure count from this IP
+          Rack::Attack::Allow2Ban.reset(ip, config)
+        else
+          banned = Rack::Attack::Allow2Ban.filter(ip, config) do
+            # Unless the IP is whitelisted, return true so that Allow2Ban
+            # increments the counter (stored in Rails.cache) for the IP
+            if config.ip_whitelist.include?(ip)
+              false
+            else
+              true
+            end
+          end
+
+          if banned
+            Rails.logger.info "IP #{ip} failed to login " \
+              "as #{login} but has been temporarily banned from Git auth"
+          end
+        end
+      end
+    end
   end
 end

lib/gitlab/backend/grack_auth.rb

@@ -95,7 +95,7 @@ module Grack
     end
 
     def authenticate_user(login, password)
-      user = Gitlab::Auth.new.find(login, password)
+      user, _ = Gitlab::Auth.new.find_by_master_or_ldap(login, password)
 
       unless user
         user = oauth_access_token_check(login, password)

spec/lib/gitlab/auth_spec.rb

 require 'spec_helper'
 
 describe Gitlab::Auth, lib: true do
-  let(:gl_auth) { Gitlab::Auth.new }
+  let(:gl_auth) { described_class }
 
-  describe :find do
+  describe 'find' do
+    it 'recognizes CI' do
+      token = '123'
+      project = create(:empty_project)
+      project.update_attributes(runners_token: token, builds_enabled: true)
+      ip = 'ip'
+
+      expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: 'gitlab-ci-token')
+      expect(gl_auth.find('gitlab-ci-token', token, project: project, ip: ip)).to eq([nil, :ci])
+    end
+
+    it 'recognizes master passwords' do
+      user = create(:user, password: 'password')
+      ip = 'ip'
+
+      expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username)
+      expect(gl_auth.find(user.username, 'password', project: nil, ip: ip)).to eq([user, :master_or_ldap])
+    end
+
+    it 'recognizes OAuth tokens' do
+      user = create(:user)
+      application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user)
+      token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id)
+      ip = 'ip'
+
+      expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: 'oauth2')
+      expect(gl_auth.find("oauth2", token.token, project: nil, ip: ip)).to eq([user, :oauth])
+    end
+
+    it 'returns double nil for invalid credentials' do
+      login = 'foo'
+      ip = 'ip'
+
+      expect(gl_auth).to receive(:rate_limit!).with(ip, success: false, login: login)
+      expect(gl_auth.find(login, 'bar', project: nil, ip: ip)).to eq ([nil, nil])
+    end
+  end
+
+  describe 'find_by_master_or_ldap' do
     let!(:user) do
       create(:user,
         username: username,
@@ -14,25 +52,25 @@ describe Gitlab::Auth, lib: true do
     let(:password) { 'my-secret' }
 
     it "should find user by valid login/password" do
-      expect( gl_auth.find(username, password) ).to eql user
+      expect( gl_auth.find_by_master_or_ldap(username, password) ).to eql user
     end
 
     it 'should find user by valid email/password with case-insensitive email' do
-      expect(gl_auth.find(user.email.upcase, password)).to eql user
+      expect(gl_auth.find_by_master_or_ldap(user.email.upcase, password)).to eql user
     end
 
     it 'should find user by valid username/password with case-insensitive username' do
-      expect(gl_auth.find(username.upcase, password)).to eql user
+      expect(gl_auth.find_by_master_or_ldap(username.upcase, password)).to eql user
     end
 
     it "should not find user with invalid password" do
       password = 'wrong'
-      expect( gl_auth.find(username, password) ).not_to eql user
+      expect( gl_auth.find_by_master_or_ldap(username, password) ).not_to eql user
     end
 
     it "should not find user with invalid login" do
       user = 'wrong'
-      expect( gl_auth.find(username, password) ).not_to eql user
+      expect( gl_auth.find_by_master_or_ldap(username, password) ).not_to eql user
     end
 
     context "with ldap enabled" do
@@ -43,13 +81,13 @@ describe Gitlab::Auth, lib: true do
       it "tries to autheticate with db before ldap" do
         expect(Gitlab::LDAP::Authentication).not_to receive(:login)
 
-        gl_auth.find(username, password)
+        gl_auth.find_by_master_or_ldap(username, password)
       end
 
       it "uses ldap as fallback to for authentication" do
         expect(Gitlab::LDAP::Authentication).to receive(:login)
 
-        gl_auth.find('ldap_user', 'password')
+        gl_auth.find_by_master_or_ldap('ldap_user', 'password')
       end
     end
   end