Merge branch 'release/7-8-2' into '7-8-stable'

WIP | Release/7 8 2

See merge request !1629

CHANGELOG

@@ -1,4 +1,19 @@
-v 7.8.0 (unreleased)
+v 7.8.2
+  - Fix service migration issue when upgrading from versions prior to 7.3
+  - Fix setting of the default use project limit via admin UI
+  - Fix showing of already imported projects for GitLab and Gitorious importers
+  - Fix response of push to repository to return "Not found" if user doesn't have access
+  - Fix check if user is allowed to view the file attachment
+  - Fix import check for case sensetive namespaces
+
+v 7.8.1
+  - Fix run of custom post receive hooks
+  - Fix migration that caused issues when upgrading to version 7.8 from versions prior to 7.3
+  - Fix the warning for LDAP users about need to set password
+  - Fix avatars which were not shown for non logged in users
+  - Fix urls for the issues when relative url was enabled
+
+v 7.8.0
   - Fix access control and protection against XSS for note attachments and other uploads.
   - Replace highlight.js with rouge-fork rugments (Stefan Tatschner)
   - Make project search case insensitive (Hannes Rosenögger)
@@ -28,7 +43,7 @@ v 7.8.0 (unreleased)
   - Allow configuring protection of the default branch upon first push (Marco Wessel)
   - Add gitlab.com importer
   - Add an ability to login with gitlab.com
-  - Add a commit calendar to the user profile (Hannes Rosenögger)  
+  - Add a commit calendar to the user profile (Hannes Rosenögger)
   - Submit comment on command-enter
   - Notify all members of a group when that group is mentioned in a comment, for example: `@gitlab-org` or `@sales`.
   - Extend issue clossing pattern to include "Resolve", "Resolves", "Resolved", "Resolving" and "Close"
@@ -43,7 +58,7 @@ v 7.8.0 (unreleased)
   - API: Access groups with their path (Julien Bianchi)
   - Added link to milestone and keeping resource context on smaller viewports for issues and merge requests (Jason Blanchard)
   - Allow notification email to be set separately from primary email.
-  - API: Add support for editing an existing project (Mika Mäenpää and Hannes Rosenögger) 
+  - API: Add support for editing an existing project (Mika Mäenpää and Hannes Rosenögger)
   - Don't have Markdown preview fail for long comments/wiki pages.
   - When test web hook - show error message instead of 500 error page if connection to hook url was reset
   - Added support for firing system hooks on group create/destroy and adding/removing users to group (Boyan Tabakov)

app/controllers/import/base_controller.rb

@@ -3,7 +3,7 @@ class Import::BaseController < ApplicationController
   private
 
   def get_or_create_namespace
-    existing_namespace = Namespace.find_by("path = ? OR name = ?", @target_namespace, @target_namespace)
+    existing_namespace = Namespace.find_by_path_or_name(@target_namespace)
 
     if existing_namespace
       if existing_namespace.owner == current_user

app/controllers/import/gitlab_controller.rb

@@ -16,7 +16,7 @@ class Import::GitlabController < Import::BaseController
     @already_added_projects = current_user.created_projects.where(import_type: "gitlab")
     already_added_projects_names = @already_added_projects.pluck(:import_source)
 
-    @repos.to_a.reject!{ |repo| already_added_projects_names.include? repo["path_with_namespace"] }
+    @repos = @repos.to_a.reject{ |repo| already_added_projects_names.include? repo["path_with_namespace"] }
   end
 
   def jobs

app/controllers/import/gitorious_controller.rb

@@ -15,7 +15,7 @@ class Import::GitoriousController < Import::BaseController
     @already_added_projects = current_user.created_projects.where(import_type: "gitorious")
     already_added_projects_names = @already_added_projects.pluck(:import_source)
 
-    @repos.to_a.reject! { |repo| already_added_projects_names.include? repo.full_name }
+    @repos.reject! { |repo| already_added_projects_names.include? repo.full_name }
   end
 
   def jobs

app/controllers/uploads_controller.rb

@@ -1,5 +1,5 @@
 class UploadsController < ApplicationController
-  skip_before_filter :authenticate_user!, :reject_blocked
+  skip_before_filter :authenticate_user!, :reject_blocked!
   before_filter :authorize_access
 
   def show
@@ -20,7 +20,7 @@ class UploadsController < ApplicationController
 
   def authorize_access
     unless params[:mounted_as] == 'avatar'
-      authenticate_user! && reject_blocked
+      authenticate_user! && reject_blocked!
     end
   end
 end

app/models/namespace.rb

@@ -48,6 +48,11 @@ class Namespace < ActiveRecord::Base
     where('lower(path) = :value', value: path.downcase).first
   end
 
+  # Case insensetive search for namespace by path or name
+  def self.find_by_path_or_name(path)
+    find_by("lower(path) = :path OR lower(name) = :path", path: path.downcase)
+  end
+
   def self.search(query)
     where("name LIKE :query OR path LIKE :query", query: "%#{query}%")
   end

app/models/user.rb

@@ -56,14 +56,13 @@ class User < ActiveRecord::Base
   include Gitlab::ConfigHelper
   include TokenAuthenticatable
   extend Gitlab::ConfigHelper
-  extend Gitlab::CurrentSettings
+  include Gitlab::CurrentSettings
 
   default_value_for :admin, false
   default_value_for :can_create_group, gitlab_config.default_can_create_group
   default_value_for :can_create_team, false
   default_value_for :hide_no_ssh_key, false
   default_value_for :hide_no_password, false
-  default_value_for :projects_limit, current_application_settings.default_projects_limit
   default_value_for :theme_id, gitlab_config.default_theme
 
   devise :database_authenticatable, :lockable, :async,
@@ -142,6 +141,7 @@ class User < ActiveRecord::Base
 
   before_save :ensure_authentication_token
   after_save :ensure_namespace_correct
+  after_initialize :set_projects_limit
   after_create :post_create_hook
   after_destroy :post_destroy_hook
 
@@ -468,6 +468,13 @@ class User < ActiveRecord::Base
     end
   end
 
+  def set_projects_limit
+    connection_default_value_defined = new_record? && !projects_limit_changed?
+    return unless self.projects_limit.nil? || connection_default_value_defined
+
+    self.projects_limit = current_application_settings.default_projects_limit
+  end
+
   def requires_ldap_check?
     if !Gitlab.config.ldap.enabled
       false

db/migrate/20141006143943_move_slack_service_to_webhook.rb

@@ -10,7 +10,7 @@ class MoveSlackServiceToWebhook < ActiveRecord::Migration
         slack_service.properties.delete('subdomain')
         # Room is configured on the Slack side
         slack_service.properties.delete('room')
-        slack_service.save
+        slack_service.save(validate: false)
       end
     end
   end

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,26 +43,20 @@ module API
 
         project = Project.find_with_namespace(project_path)
 
-        unless project
-          return Gitlab::GitAccessStatus.new(false, 'No such project')
+        if project
+          status = access.check(
+            actor,
+            params[:action],
+            project,
+            params[:changes]
+          )
         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')
+        if project && status && status.allowed?
+          status
+        else
+          Gitlab::GitAccessStatus.new(false, 'No such project')
         end
-
-        access.check(
-          actor,
-          params[:action],
-          project,
-          params[:changes]
-        )
       end
 
       #

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!
+      auth!
+
+      if project && authorized_request?
+        @app.call(env)
+      elsif @user.nil? && !@gitlab_ci
+        unauthorized
       else
         render_not_found
       end
@@ -32,35 +37,30 @@ module Grack
     private
 
     def auth!
-      if @auth.provided?
-        return bad_request unless @auth.basic?
+      return unless @auth.provided?
 
-        # Authentication with username and password
-        login, password = @auth.credentials
+      return bad_request unless @auth.basic?
 
-        # Allow authentication for GitLab CI service
-        # if valid token passed
-        if gitlab_ci_request?(login, password)
-          return @app.call(env)
-        end
+      # Authentication with username and password
+      login, password = @auth.credentials
 
-        @user = authenticate_user(login, password)
-
-        if @user
-          Gitlab::ShellEnv.set_env(@user)
-          @env['REMOTE_USER'] = @auth.username
-        end
+      # Allow authentication for GitLab CI service
+      # if valid token passed
+      if gitlab_ci_request?(login, password)
+        @gitlab_ci = true
+        return
       end
 
-      if authorized_request?
-        @app.call(env)
-      else
-        unauthorized
+      @user = authenticate_user(login, password)
+
+      if @user
+        Gitlab::ShellEnv.set_env(@user)
+        @env['REMOTE_USER'] = @auth.username
       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

@@ -0,0 +1,146 @@
+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

spec/models/namespace_spec.rb

@@ -75,4 +75,14 @@ describe Namespace do
       expect(namespace.rm_dir).to be_truthy
     end
   end
+
+  describe :find_by_path_or_name do
+    before do
+      @namespace = create(:namespace, name: 'WoW', path: 'woW')
+    end
+
+    it { expect(Namespace.find_by_path_or_name('wow')).to eq(@namespace) }
+    it { expect(Namespace.find_by_path_or_name('WOW')).to eq(@namespace) }
+    it { expect(Namespace.find_by_path_or_name('unknown')).to eq(nil) }
+  end
 end