Merge branch 'project-existence-leak' into 'master'

lib/api/internal.rb

@@ -16,6 +16,17 @@ module API
       #
       post "/allowed" do
         status 200
+
+        actor = if params[:key_id]
+                  Key.find_by(id: params[:key_id])
+                elsif params[:user_id]
+                  User.find_by(id: params[:user_id])
+                end
+
+        unless actor
+          return Gitlab::GitAccessStatus.new(false, 'No such user or key')
+        end
+
         project_path = params[:project]
 
         # Check for *.wiki repositories.
@@ -32,21 +43,8 @@ module API
 
         project = Project.find_with_namespace(project_path)
 
-        unless project
-          return Gitlab::GitAccessStatus.new(false, 'No such project')
-        end
-
-        actor = if params[:key_id]
-                  Key.find_by(id: params[:key_id])
-                elsif params[:user_id]
-                  User.find_by(id: params[:user_id])
-                end
-
-        unless actor
-          return Gitlab::GitAccessStatus.new(false, 'No such user or key')
-        end
-
-        access.check(
+        if project
+          status = access.check(
             actor,
             params[:action],
             project,
@@ -54,6 +52,13 @@ module API
           )
         end
 
+        if project && status && status.allowed?
+          status
+        else
+          Gitlab::GitAccessStatus.new(false, 'No such project')
+        end
+      end
+
       #
       # Discover user by ssh key
       #

lib/gitlab/backend/grack_auth.rb

@@ -10,8 +10,9 @@ module Grack
       @request = Rack::Request.new(env)
       @auth = Request.new(env)
 
-      # Need this patch due to the rails mount
+      @gitlab_ci = false
 
+      # Need this patch due to the rails mount
       # Need this if under RELATIVE_URL_ROOT
       unless Gitlab.config.gitlab.relative_url_root.empty?
         # If website is mounted using relative_url_root need to remove it first
@@ -22,8 +23,12 @@ module Grack
 
       @env['SCRIPT_NAME'] = ""
 
-      if project
       auth!
+
+      if project && authorized_request?
+        @app.call(env)
+      elsif @user.nil? && !@gitlab_ci
+        unauthorized
       else
         render_not_found
       end
@@ -32,7 +37,8 @@ module Grack
     private
 
     def auth!
-      if @auth.provided?
+      return unless @auth.provided?
+
       return bad_request unless @auth.basic?
 
       # Authentication with username and password
@@ -41,7 +47,8 @@ module Grack
       # Allow authentication for GitLab CI service
       # if valid token passed
       if gitlab_ci_request?(login, password)
-          return @app.call(env)
+        @gitlab_ci = true
+        return
       end
 
       @user = authenticate_user(login, password)
@@ -52,15 +59,8 @@ module Grack
       end
     end
 
-      if authorized_request?
-        @app.call(env)
-      else
-        unauthorized
-      end
-    end
-
     def gitlab_ci_request?(login, password)
-      if login == "gitlab-ci-token" && project.gitlab_ci?
+      if login == "gitlab-ci-token" && project && project.gitlab_ci?
         token = project.gitlab_ci_service.token
 
         if token.present? && token == password && git_cmd == 'git-upload-pack'
@@ -107,6 +107,8 @@ module Grack
     end
 
     def authorized_request?
+      return true if @gitlab_ci
+
       case git_cmd
       when *Gitlab::GitAccess::DOWNLOAD_COMMANDS
         if user
@@ -141,7 +143,9 @@ module Grack
     end
 
     def project
-      @project ||= project_by_path(@request.path_info)
+      return @project if defined?(@project)
+
+      @project = project_by_path(@request.path_info)
     end
 
     def project_by_path(path)

spec/lib/gitlab/backend/grack_auth_spec.rb

+require "spec_helper"
+
+describe Grack::Auth do
+  let(:user)    { create(:user) }
+  let(:project) { create(:project) }
+
+  let(:app)   { lambda { |env| [200, {}, "Success!"] } }
+  let!(:auth) { Grack::Auth.new(app) }
+  let(:env) { 
+    {
+      "rack.input" => "",
+      "REQUEST_METHOD" => "GET",
+      "QUERY_STRING" => "service=git-upload-pack"
+    }
+  }
+  let(:status) { auth.call(env).first }
+
+  describe "#call" do
+    context "when the project doesn't exist" do
+      before do
+        env["PATH_INFO"] = "doesnt/exist.git"
+      end
+
+      context "when no authentication is provided" do
+        it "responds with status 401" do
+          expect(status).to eq(401)
+        end
+      end
+
+      context "when username and password are provided" do
+        context "when authentication fails" do
+          before do
+            env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, "nope")
+          end
+
+          it "responds with status 401" do
+            expect(status).to eq(401)
+          end
+        end
+
+        context "when authentication succeeds" do
+          before do
+            env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password)
+          end
+
+          it "responds with status 404" do
+            expect(status).to eq(404)
+          end
+        end
+      end
+    end
+
+    context "when the project exists" do
+      before do
+        env["PATH_INFO"] = project.path_with_namespace + ".git"
+      end
+
+      context "when the project is public" do
+        before do
+          project.update_attribute(:visibility_level, Project::PUBLIC)
+        end
+
+        it "responds with status 200" do
+          expect(status).to eq(200)
+        end
+      end
+
+      context "when the project is private" do
+        before do
+          project.update_attribute(:visibility_level, Project::PRIVATE)
+        end
+
+        context "when no authentication is provided" do
+          it "responds with status 401" do
+            expect(status).to eq(401)
+          end
+        end
+
+        context "when username and password are provided" do
+          context "when authentication fails" do
+            before do
+              env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, "nope")
+            end
+
+            it "responds with status 401" do
+              expect(status).to eq(401)
+            end
+          end
+
+          context "when authentication succeeds" do
+            before do
+              env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password)
+            end
+
+            context "when the user has access to the project" do
+              before do
+                project.team << [user, :master]
+              end
+
+              context "when the user is blocked" do
+                before do
+                  user.block
+                  project.team << [user, :master]
+                end
+
+                it "responds with status 404" do
+                  expect(status).to eq(404)
+                end
+              end
+
+              context "when the user isn't blocked" do
+                it "responds with status 200" do
+                  expect(status).to eq(200)
+                end
+              end
+            end
+
+            context "when the user doesn't have access to the project" do
+              it "responds with status 404" do
+                expect(status).to eq(404)
+              end
+            end
+          end
+        end
+
+        context "when a gitlab ci token is provided" do
+          let(:token) { "123" }
+
+          before do
+            gitlab_ci_service = project.build_gitlab_ci_service
+            gitlab_ci_service.active = true
+            gitlab_ci_service.token = token
+            gitlab_ci_service.project_url = "http://google.com"
+            gitlab_ci_service.save
+
+            env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials("gitlab-ci-token", token)
+          end
+
+          it "responds with status 200" do
+            expect(status).to eq(200)
+          end
+        end
+      end
+    end
+  end
+end