Merge branch '7-10-rc4' into '7-10-stable'

Changes for 7.10.0.rc4

cc @job @douwe

See merge request !1785

11 files changed, 286 insertions, 108 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 5de75d368e2..1aab904f117 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -12,6 +12,9 @@ v 7.11.0 (unreleased)
   -
 
 v 7.10.0 (unreleased)
+  - Ignore submodules that are defined in .gitmodules but are checked in as directories.
+  - Allow projects to be imported from Google Code.
+  - Remove access control for uploaded images to fix broken images in emails (Hannes Rosenögger)
   - Allow users to be invited by email to join a group or project.
   - Don't crash when project repository doesn't exist.
   - Add config var to block auto-created LDAP users.
@@ -84,7 +87,6 @@ v 7.10.0 (unreleased)
   - Fix admin user projects lists.
   - Don't leak private group existence by redirecting from namespace controller to group controller.
   - Ability to skip some items from backup (database, respositories or uploads)
-  - Fix "Hello @username." references not working by no longer allowing usernames to end in period.
   - Archive repositories in background worker.
   - Import GitHub, Bitbucket or GitLab.com projects owned by authenticated user into current namespace.
   - Project labels are now available over the API under the "tag_list" field (Cristian Medina) 
@@ -101,6 +103,12 @@ v 7.10.0 (unreleased)
   - Remove truncation from issue titles on milestone page (Jason Blanchard)
   - Fix stuck Merge Request merging events from old installations (Ben Bodenmiller)
   - Fix merge request comments on files with multiple commits
+  - Fix Resource Owner Password Authentication Flow
+  
+v 7.9.4
+  - Security: Fix project import URL regex to prevent arbitary local repos from being imported
+  - Fixed issue where only 25 commits would load in file listings
+  - Fix LDAP identities  after config update
 
 v 7.9.3
   - Contains no changes
diff --git a/Gemfile b/Gemfile
index 3f262655117..1c2a9002204 100644
--- a/Gemfile
+++ b/Gemfile
@@ -39,7 +39,7 @@ gem "browser"
 
 # Extracting information from a git repository
 # Provide access to Gitlab::Git library
-gem "gitlab_git", '~> 7.1.9'
+gem "gitlab_git", '~> 7.1.10'
 
 # Ruby/Rack Git Smart-HTTP Server Handler
 gem 'gitlab-grack', '~> 2.0.0.rc2', require: 'grack'
diff --git a/Gemfile.lock b/Gemfile.lock
index bfe626521e7..360b1abcf56 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -212,7 +212,7 @@ GEM
       mime-types (~> 1.19)
     gitlab_emoji (0.1.0)
       gemojione (~> 2.0)
-    gitlab_git (7.1.9)
+    gitlab_git (7.1.10)
       activesupport (~> 4.0)
       charlock_holmes (~> 0.6)
       gitlab-linguist (~> 3.0)
@@ -703,7 +703,7 @@ DEPENDENCIES
   gitlab-grack (~> 2.0.0.rc2)
   gitlab-linguist (~> 3.0.1)
   gitlab_emoji (~> 0.1)
-  gitlab_git (~> 7.1.9)
+  gitlab_git (~> 7.1.10)
   gitlab_meta (= 7.0)
   gitlab_omniauth-ldap (= 1.2.1)
   gollum-lib (~> 4.0.2)
diff --git a/app/controllers/projects/uploads_controller.rb b/app/controllers/projects/uploads_controller.rb
index 9020e86c44e..276dced8656 100644
--- a/app/controllers/projects/uploads_controller.rb
+++ b/app/controllers/projects/uploads_controller.rb
@@ -1,7 +1,11 @@
 class Projects::UploadsController < Projects::ApplicationController
   layout 'project'
 
-  before_filter :project
+  # We want to skip these filters for only the `show` action if `image?` is true, 
+  # but `skip_before_filter` doesn't work with both `only` and `if`, so we accomplish the same like this.
+  skipped_filters = [:authenticate_user!, :reject_blocked!, :project, :repository]
+  skip_before_filter  *skipped_filters, only: [:show]
+  before_filter       *skipped_filters, only: [:show], unless: :image?
 
   def create
     link_to_file = ::Projects::UploadService.new(project, params[:file]).
@@ -21,15 +25,32 @@ class Projects::UploadsController < Projects::ApplicationController
   end
 
   def show
-    uploader = FileUploader.new(project, params[:secret])
+    return not_found! if uploader.nil? || !uploader.file.exists?
 
-    return redirect_to uploader.url unless uploader.file_storage?
+    disposition = uploader.image? ? 'inline' : 'attachment'
+    send_file uploader.file.path, disposition: disposition
+  end
 
-    uploader.retrieve_from_store!(params[:filename])
+  def uploader
+    return @uploader if defined?(@uploader)
 
-    return not_found! unless uploader.file.exists?
+    namespace = params[:namespace_id]
+    id = params[:project_id]
 
-    disposition = uploader.image? ? 'inline' : 'attachment'
-    send_file uploader.file.path, disposition: disposition
+    file_project = Project.find_with_namespace("#{namespace}/#{id}")
+
+    if file_project.nil?
+      @uploader = nil 
+      return
+    end
+
+    @uploader = FileUploader.new(file_project, params[:secret])
+    @uploader.retrieve_from_store!(params[:filename])
+
+    @uploader
+  end
+
+  def image?
+    uploader && uploader.file.exists? && uploader.image?
   end
 end
diff --git a/app/models/label.rb b/app/models/label.rb
index 9d7099c5652..1f22ed23d42 100644
--- a/app/models/label.rb
+++ b/app/models/label.rb
@@ -13,6 +13,8 @@
 class Label < ActiveRecord::Base
   DEFAULT_COLOR = '#428BCA'
 
+  default_value_for :color, DEFAULT_COLOR
+
   belongs_to :project
   has_many :label_links, dependent: :destroy
   has_many :issues, through: :label_links, source: :target, source_type: 'Issue'
diff --git a/app/views/projects/labels/_form.html.haml b/app/views/projects/labels/_form.html.haml
index ad993db6c0b..261d52dedc1 100644
--- a/app/views/projects/labels/_form.html.haml
+++ b/app/views/projects/labels/_form.html.haml
@@ -16,7 +16,7 @@
     .col-sm-10
       .input-group
         .input-group-addon.label-color-preview &nbsp;
-        = f.color_field :color, value: "#AA33EE", class: "form-control"
+        = f.color_field :color, class: "form-control"
       .help-block
         Choose any color.
         %br
diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb
index 9da7ebf4290..d422acb31d6 100644
--- a/config/initializers/doorkeeper.rb
+++ b/config/initializers/doorkeeper.rb
@@ -11,7 +11,7 @@ Doorkeeper.configure do
   end
 
   resource_owner_from_credentials do |routes|
-    u = User.find_by(email: params[:username])
+    u = User.find_by(email: params[:username]) || User.find_by(username: params[:username])
     u if u && u.valid_password?(params[:password])
   end
 
@@ -83,7 +83,7 @@ Doorkeeper.configure do
   #
   # If not specified, Doorkeeper enables all the four grant flows.
   #
-  # grant_flows %w(authorization_code implicit password client_credentials)
+  grant_flows %w(authorization_code password client_credentials)
 
   # Under some circumstances you might want to have applications auto-approved,
   # so that the user skips the authorization step.
diff --git a/db/migrate/20150324133047_remove_periods_at_ends_of_usernames.rb b/db/migrate/20150324133047_remove_periods_at_ends_of_usernames.rb
deleted file mode 100644
index dc38b0eceb7..00000000000
--- a/db/migrate/20150324133047_remove_periods_at_ends_of_usernames.rb
+++ /dev/null
@@ -1,76 +0,0 @@
-class RemovePeriodsAtEndsOfUsernames < ActiveRecord::Migration
-  include Gitlab::ShellAdapter
-
-  class Namespace < ActiveRecord::Base
-    class << self
-      def find_by_path_or_name(path)
-        find_by("lower(path) = :path OR lower(name) = :path", path: path.downcase)
-      end
-
-      def clean_path(path)
-        path = path.dup
-        path.gsub!(/@.*\z/,             "")
-        path.gsub!(/\.git\z/,           "")
-        path.gsub!(/\A-+/,              "")
-        path.gsub!(/\.+\z/,             "")
-        path.gsub!(/[^a-zA-Z0-9_\-\.]/, "")
-
-        counter = 0
-        base = path
-        while Namespace.find_by_path_or_name(path)
-          counter += 1
-          path = "#{base}#{counter}"
-        end
-
-        path
-      end
-    end
-  end
-
-  def up
-    changed_paths = {}
-
-    select_all("SELECT id, username FROM users WHERE username LIKE '%.'").each do |user|
-      username_was = user["username"]
-      username = Namespace.clean_path(username_was)
-      changed_paths[username_was] = username
-
-      username = quote_string(username)
-      execute "UPDATE users SET username = '#{username}' WHERE id = #{user["id"]}"
-      execute "UPDATE namespaces SET path = '#{username}', name = '#{username}' WHERE type IS NULL AND owner_id = #{user["id"]}"
-    end
-
-    select_all("SELECT id, path FROM namespaces WHERE type = 'Group' AND path LIKE '%.'").each do |group|
-      path_was = group["path"]
-      path = Namespace.clean_path(path_was)
-      changed_paths[path_was] = path
-
-      path = quote_string(path)
-      execute "UPDATE namespaces SET path = '#{path}' WHERE id = #{group["id"]}"
-    end
-
-    changed_paths.each do |path_was, path|
-      if gitlab_shell.mv_namespace(path_was, path)
-        # If repositories moved successfully we need to remove old satellites
-        # and send update instructions to users.
-        # However we cannot allow rollback since we moved namespace dir
-        # So we basically we mute exceptions in next actions
-        begin
-          gitlab_shell.rm_satellites(path_was)
-          # We cannot send update instructions since models and mailers
-          # can't safely be used from migrations as they may be written for 
-          # later versions of the database.
-          # send_update_instructions
-        rescue
-          # Returning false does not rollback after_* transaction but gives
-          # us information about failing some of tasks
-          false
-        end
-      else
-        # if we cannot move namespace directory we should rollback
-        # db changes in order to prevent out of sync between db and fs
-        raise Exception.new('namespace directory cannot be moved')
-      end
-    end
-  end
-end
diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb
index 8073417a16a..47c456d8dc7 100644
--- a/lib/gitlab/markdown.rb
+++ b/lib/gitlab/markdown.rb
@@ -68,23 +68,8 @@ module Gitlab
       @options      = options
       @html_options = html_options
 
-      # Extract pre blocks so they are not altered
-      # from http://github.github.com/github-flavored-markdown/
-      text.gsub!(%r{<pre>.*?</pre>|<code>.*?</code>}m) { |match| extract_piece(match) }
-      # Extract links with probably parsable hrefs
-      text.gsub!(%r{<a.*?>.*?</a>}m) { |match| extract_piece(match) }
-      # Extract images with probably parsable src
-      text.gsub!(%r{<img.*?>}m) { |match| extract_piece(match) }
-
       # TODO: add popups with additional information
 
-      text = parse(text, project)
-
-      # Insert pre block extractions
-      text.gsub!(/\{gfm-extraction-(\h{32})\}/) do
-        insert_piece($1)
-      end
-
       # Used markdown pipelines in GitLab:
       # GitlabEmojiFilter - performs emoji replacement.
       # SanitizationFilter - remove unsafe HTML tags and attributes
@@ -129,6 +114,21 @@ module Gitlab
 
       text = result[:output].to_html(save_with: save_options)
 
+      # Extract pre blocks so they are not altered
+      # from http://github.github.com/github-flavored-markdown/
+      text.gsub!(%r{<pre>.*?</pre>|<code>.*?</code>}m) { |match| extract_piece(match) }
+      # Extract links with probably parsable hrefs
+      text.gsub!(%r{<a.*?>.*?</a>}m) { |match| extract_piece(match) }
+      # Extract images with probably parsable src
+      text.gsub!(%r{<img.*?>}m) { |match| extract_piece(match) }
+
+      text = parse(text, project)
+
+      # Insert pre block extractions
+      text.gsub!(/\{gfm-extraction-(\h{32})\}/) do
+        insert_piece($1)
+      end
+
       if options[:parse_tasks]
         text = parse_tasks(text)
       end
@@ -150,7 +150,7 @@ module Gitlab
       @extractions[id]
     end
 
-    # Private: Parses text for references and emoji
+    # Private: Parses text for references
     #
     # text - Text to parse
     #
diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb
index 0571574aa4f..9aeed5e6939 100644
--- a/lib/gitlab/regex.rb
+++ b/lib/gitlab/regex.rb
@@ -2,7 +2,7 @@ module Gitlab
   module Regex
     extend self
 
-    NAMESPACE_REGEX_STR = '(?:[a-zA-Z0-9_\.][a-zA-Z0-9_\-\.]*[a-zA-Z0-9_\-]|[a-zA-Z0-9_])'.freeze
+    NAMESPACE_REGEX_STR = '(?:[a-zA-Z0-9_\.][a-zA-Z0-9_\-\.]*)'.freeze
 
     def namespace_regex
       @namespace_regex ||= /\A#{NAMESPACE_REGEX_STR}\z/.freeze
@@ -10,7 +10,7 @@ module Gitlab
 
     def namespace_regex_message
       "can contain only letters, digits, '_', '-' and '.'. " \
-      "Cannot start with '-' or end in '.'." \
+      "Cannot start with '-'." \
     end
 
 
diff --git a/spec/controllers/projects/uploads_controller_spec.rb b/spec/controllers/projects/uploads_controller_spec.rb
index 029f48b2d7a..f51abfedae5 100644
--- a/spec/controllers/projects/uploads_controller_spec.rb
+++ b/spec/controllers/projects/uploads_controller_spec.rb
@@ -54,4 +54,227 @@ describe Projects::UploadsController do
       end
     end
   end
+
+  describe "GET #show" do
+    let(:go) do
+      get :show,
+        namespace_id: project.namespace.to_param,
+        project_id:   project.to_param,
+        secret:       "123456",
+        filename:     "image.jpg"
+    end
+
+    context "when the project is public" do
+      before do
+        project.update_attribute(:visibility_level, Project::PUBLIC)
+      end
+
+      context "when not signed in" do
+        context "when the file exists" do
+          before do
+            allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
+            allow(jpg).to receive(:exists?).and_return(true)
+          end
+
+          it "responds with status 200" do
+            go
+
+            expect(response.status).to eq(200)
+          end
+        end
+
+        context "when the file doesn't exist" do
+          it "responds with status 404" do
+            go
+
+            expect(response.status).to eq(404)
+          end
+        end
+      end
+
+      context "when signed in" do
+        before do
+          sign_in(user)
+        end
+
+        context "when the file exists" do
+          before do
+            allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
+            allow(jpg).to receive(:exists?).and_return(true)
+          end
+
+          it "responds with status 200" do
+            go
+
+            expect(response.status).to eq(200)
+          end
+        end
+
+        context "when the file doesn't exist" do
+          it "responds with status 404" do
+            go
+
+            expect(response.status).to eq(404)
+          end
+        end
+      end
+    end
+
+    context "when the project is private" do
+      before do
+        project.update_attribute(:visibility_level, Project::PRIVATE)
+      end
+
+      context "when not signed in" do
+        context "when the file exists" do
+          before do
+            allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
+            allow(jpg).to receive(:exists?).and_return(true)
+          end
+
+          context "when the file is an image" do
+            before do
+              allow_any_instance_of(FileUploader).to receive(:image?).and_return(true)
+            end
+
+            it "responds with status 200" do
+              go
+
+              expect(response.status).to eq(200)
+            end
+          end
+
+          context "when the file is not an image" do
+            it "redirects to the sign in page" do
+              go
+
+              expect(response).to redirect_to(new_user_session_path)
+            end
+          end
+        end
+
+        context "when the file doesn't exist" do
+          it "redirects to the sign in page" do
+            go
+
+            expect(response).to redirect_to(new_user_session_path)
+          end
+        end
+      end
+
+      context "when signed in" do
+        before do
+          sign_in(user)
+        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
+
+            context "when the file exists" do
+              before do
+                allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
+                allow(jpg).to receive(:exists?).and_return(true)
+              end
+
+              context "when the file is an image" do
+                before do
+                  allow_any_instance_of(FileUploader).to receive(:image?).and_return(true)
+                end
+
+                it "responds with status 200" do
+                  go
+
+                  expect(response.status).to eq(200)
+                end
+              end
+
+              context "when the file is not an image" do
+                it "redirects to the sign in page" do
+                  go
+
+                  expect(response).to redirect_to(new_user_session_path)
+                end
+              end
+            end
+
+            context "when the file doesn't exist" do
+              it "redirects to the sign in page" do
+                go
+
+                expect(response).to redirect_to(new_user_session_path)
+              end
+            end
+          end
+
+          context "when the user isn't blocked" do
+            context "when the file exists" do
+              before do
+                allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
+                allow(jpg).to receive(:exists?).and_return(true)
+              end
+
+              it "responds with status 200" do
+                go
+
+                expect(response.status).to eq(200)
+              end
+            end
+
+            context "when the file doesn't exist" do
+              it "responds with status 404" do
+                go
+
+                expect(response.status).to eq(404)
+              end
+            end
+          end
+        end
+
+        context "when the user doesn't have access to the project" do
+          context "when the file exists" do
+            before do
+              allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
+              allow(jpg).to receive(:exists?).and_return(true)
+            end
+
+            context "when the file is an image" do
+              before do
+                allow_any_instance_of(FileUploader).to receive(:image?).and_return(true)
+              end
+
+              it "responds with status 200" do
+                go
+
+                expect(response.status).to eq(200)
+              end
+            end
+
+            context "when the file is not an image" do
+              it "responds with status 404" do
+                go
+
+                expect(response.status).to eq(404)
+              end
+            end
+          end
+
+          context "when the file doesn't exist" do
+            it "responds with status 404" do
+              go
+
+              expect(response.status).to eq(404)
+            end
+          end
+        end
+      end
+    end
+  end
 end