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

Changes for 7.10.0.rc4

cc @job @douwe

See merge request !1785

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

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'

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)

app/controllers/projects/uploads_controller.rb

 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 redirect_to uploader.url unless uploader.file_storage?
-    uploader.retrieve_from_store!(params[:filename])
-    return not_found! unless uploader.file.exists?
-    disposition = uploader.image? ? 'inline' : 'attachment'
-    send_file uploader.file.path, disposition: disposition
+    return not_found! if uploader.nil? || !uploader.file.exists?
+    disposition = uploader.image? ? 'inline' : 'attachment'
+    send_file uploader.file.path, disposition: disposition
+  end
+  def uploader
+    return @uploader if defined?(@uploader)
+    namespace = params[:namespace_id]
+    id = params[:project_id]
+    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

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'

app/views/projects/labels/_form.html.haml

@@ -16,7 +16,7 @@
     .col-sm-10
       .input-group
         .input-group-addon.label-color-preview  
-        = f.color_field :color, value: "#AA33EE", class: "form-control"
+        = f.color_field :color, class: "form-control"
       .help-block
         Choose any color.
         %br

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.

db/migrate/20150324133047_remove_periods_at_ends_of_usernames.rb

-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

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
     #

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

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