Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ce into fix/project-import_url

8 files changed, 101 insertions, 43 deletions

diff --git a/app/models/ability.rb b/app/models/ability.rb
index e22da4806e6..fa2345f6faa 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -85,7 +85,7 @@ class Ability
                 subject.group
               end
 
-      if group && group.projects.public_only.any?
+      if group && group.public?
         [:read_group]
       else
         []
@@ -114,6 +114,13 @@ class Ability
         # Push abilities on the users team role
         rules.push(*project_team_rules(project.team, user))
 
+        if project.owner == user ||
+          (project.group && project.group.has_owner?(user)) ||
+          user.admin?
+
+          rules.push(*project_owner_rules)
+        end
+
         if project.public? || (project.internal? && !user.external?)
           rules.push(*public_project_rules)
 
@@ -121,14 +128,6 @@ class Ability
           rules << :read_build if project.public_builds?
         end
 
-        if project.owner == user || user.admin?
-          rules.push(*project_admin_rules)
-        end
-
-        if project.group && project.group.has_owner?(user)
-          rules.push(*project_admin_rules)
-        end
-
         if project.archived?
           rules -= project_archived_rules
         end
@@ -171,7 +170,8 @@ class Ability
         :read_note,
         :create_project,
         :create_issue,
-        :create_note
+        :create_note,
+        :upload_file
       ]
     end
 
@@ -228,14 +228,16 @@ class Ability
       ]
     end
 
-    def project_admin_rules
-      @project_admin_rules ||= project_master_rules + [
+    def project_owner_rules
+      @project_owner_rules ||= project_master_rules + [
         :change_namespace,
         :change_visibility_level,
         :rename_project,
         :remove_project,
         :archive_project,
-        :remove_fork_project
+        :remove_fork_project,
+        :destroy_merge_request,
+        :destroy_issue
       ]
     end
 
@@ -273,11 +275,9 @@ class Ability
     def group_abilities(user, group)
       rules = []
 
-      if user.admin? || group.users.include?(user) || ProjectsFinder.new.execute(user, group: group).any?
-        rules << :read_group
-      end
+      rules << :read_group if can_read_group?(user, group)
 
-      # Only group masters and group owners can create new projects in group
+      # Only group masters and group owners can create new projects
       if group.has_master?(user) || group.has_owner?(user) || user.admin?
         rules += [
           :create_projects,
@@ -290,13 +290,23 @@ class Ability
         rules += [
           :admin_group,
           :admin_namespace,
-          :admin_group_member
+          :admin_group_member,
+          :change_visibility_level
         ]
       end
 
       rules.flatten
     end
 
+    def can_read_group?(user, group)
+      return true if user.admin?
+      return true if group.public?
+      return true if group.internal? && !user.external?
+      return true if group.users.include?(user)
+
+      GroupProjectsFinder.new(group).execute(user).any?
+    end
+
     def namespace_abilities(user, namespace)
       rules = []
 
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 269056e0e77..c4879598c4e 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -18,6 +18,7 @@
 #  max_attachment_size               :integer          default(10), not null
 #  default_project_visibility        :integer
 #  default_snippet_visibility        :integer
+#  default_group_visibility          :integer
 #  restricted_signup_domains         :text
 #  user_oauth_applications           :boolean          default(TRUE)
 #  after_sign_out_path               :string(255)
diff --git a/app/models/concerns/internal_id.rb b/app/models/concerns/internal_id.rb
index 821ed54fb98..51288094ef1 100644
--- a/app/models/concerns/internal_id.rb
+++ b/app/models/concerns/internal_id.rb
@@ -7,7 +7,10 @@ module InternalId
   end
 
   def set_iid
-    max_iid = project.send(self.class.name.tableize).maximum(:iid)
+    records = project.send(self.class.name.tableize)
+    records = records.with_deleted if self.paranoid?
+    max_iid = records.maximum(:iid)
+
     self.iid = max_iid.to_i + 1
   end
 
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 9ab72652190..476e1ce7af0 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -58,6 +58,8 @@ module Issuable
     attr_mentionable :description, cache: true
     participant :author, :assignee, :notes_with_associations
     strip_attributes :title
+
+    acts_as_paranoid
   end
 
   module ClassMethods
diff --git a/app/models/group.rb b/app/models/group.rb
index 9919ca112dc..b332601c59b 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -2,15 +2,16 @@
 #
 # Table name: namespaces
 #
-#  id          :integer          not null, primary key
-#  name        :string(255)      not null
-#  path        :string(255)      not null
-#  owner_id    :integer
-#  created_at  :datetime
-#  updated_at  :datetime
-#  type        :string(255)
-#  description :string(255)      default(""), not null
-#  avatar      :string(255)
+#  id                     :integer          not null, primary key
+#  name                   :string(255)      not null
+#  path                   :string(255)      not null
+#  owner_id               :integer
+#  visibility_level       :integer          default(20), not null
+#  created_at             :datetime
+#  updated_at             :datetime
+#  type                   :string(255)
+#  description            :string(255)      default(""), not null
+#  avatar                 :string(255)
 #
 
 require 'carrierwave/orm/activerecord'
@@ -18,6 +19,7 @@ require 'file_size_validator'
 
 class Group < Namespace
   include Gitlab::ConfigHelper
+  include Gitlab::VisibilityLevel
   include Referable
 
   has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember'
@@ -27,6 +29,8 @@ class Group < Namespace
   has_many :shared_projects, through: :project_group_links, source: :project
 
   validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? }
+  validate :visibility_level_allowed_by_projects
+
   validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
 
   mount_uploader :avatar, AvatarUploader
@@ -74,6 +78,21 @@ class Group < Namespace
     name
   end
 
+  def visibility_level_field
+    visibility_level
+  end
+
+  def visibility_level_allowed_by_projects
+    allowed_by_projects = self.projects.where('visibility_level > ?', self.visibility_level).none?
+
+    unless allowed_by_projects
+      level_name = Gitlab::VisibilityLevel.level_name(visibility_level).downcase
+      self.errors.add(:visibility_level, "#{level_name} is not allowed since there are projects with higher visibility.")
+    end
+
+    allowed_by_projects
+  end
+
   def avatar_url(size = nil)
     if avatar.present?
       [gitlab_config.url, avatar.url].join
diff --git a/app/models/issue.rb b/app/models/issue.rb
index ddb51ad5775..f32db59ac9f 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -36,9 +36,6 @@ class Issue < ActiveRecord::Base
 
   validates :project, presence: true
 
-  scope :of_group,
-    ->(group) { where(project_id: group.projects.select(:id).reorder(nil)) }
-
   scope :cared, ->(user) { where(assignee_id: user) }
   scope :open_for, ->(user) { opened.assigned_to(user) }
   scope :in_projects, ->(project_ids) { where(project_id: project_ids) }
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index a015a9ef394..ef48207f956 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -131,7 +131,6 @@ class MergeRequest < ActiveRecord::Base
   validate :validate_branches
   validate :validate_fork
 
-  scope :of_group, ->(group) { where("source_project_id in (:group_project_ids) OR target_project_id in (:group_project_ids)", group_project_ids: group.projects.select(:id).reorder(nil)) }
   scope :by_branch, ->(branch_name) { where("(source_branch LIKE :branch) OR (target_branch LIKE :branch)", branch: branch_name) }
   scope :cared, ->(user) { where('assignee_id = :user OR author_id = :user', user: user.id) }
   scope :by_milestone, ->(milestone) { where(milestone_id: milestone) }
diff --git a/app/models/project.rb b/app/models/project.rb
index 6d3cf3e3f1b..c4287d314ea 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -73,7 +73,7 @@ class Project < ActiveRecord::Base
     update_column(:last_activity_at, self.created_at)
   end
 
-  # update visibility_levet of forks
+  # update visibility_level of forks
   after_update :update_forks_visibility_level
   def update_forks_visibility_level
     return unless visibility_level < visibility_level_was
@@ -197,6 +197,8 @@ class Project < ActiveRecord::Base
   validate :avatar_type,
     if: ->(project) { project.avatar.present? && project.avatar_changed? }
   validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
+  validate :visibility_level_allowed_by_group
+  validate :visibility_level_allowed_as_fork
 
   add_authentication_token_field :runners_token
   before_save :ensure_runners_token
@@ -215,8 +217,6 @@ class Project < ActiveRecord::Base
   scope :in_group_namespace, -> { joins(:group) }
   scope :personal, ->(user) { where(namespace_id: user.namespace_id) }
   scope :joined, ->(user) { where('namespace_id != ?', user.namespace_id) }
-  scope :public_only, -> { where(visibility_level: Project::PUBLIC) }
-  scope :public_and_internal_only, -> { where(visibility_level: Project.public_and_internal_levels) }
   scope :non_archived, -> { where(archived: false) }
   scope :for_milestones, ->(ids) { joins(:milestones).where('milestones.id' => ids).distinct }
 
@@ -246,10 +246,6 @@ class Project < ActiveRecord::Base
   end
 
   class << self
-    def public_and_internal_levels
-      [Project::PUBLIC, Project::INTERNAL]
-    end
-
     def abandoned
       where('projects.last_activity_at < ?', 6.months.ago)
     end
@@ -464,10 +460,25 @@ class Project < ActiveRecord::Base
 
   def check_limit
     unless creator.can_create_project? or namespace.kind == 'group'
-      errors[:limit_reached] << ("Your project limit is #{creator.projects_limit} projects! Please contact your administrator to increase it")
+      self.errors.add(:limit_reached, "Your project limit is #{creator.projects_limit} projects! Please contact your administrator to increase it")
     end
   rescue
-    errors[:base] << ("Can't check your ability to create project")
+    self.errors.add(:base, "Can't check your ability to create project")
+  end
+
+  def visibility_level_allowed_by_group
+    return if visibility_level_allowed_by_group?
+
+    level_name = Gitlab::VisibilityLevel.level_name(self.visibility_level).downcase
+    group_level_name = Gitlab::VisibilityLevel.level_name(self.group.visibility_level).downcase
+    self.errors.add(:visibility_level, "#{level_name} is not allowed in a #{group_level_name} group.")
+  end
+
+  def visibility_level_allowed_as_fork
+    return if visibility_level_allowed_as_fork?
+
+    level_name = Gitlab::VisibilityLevel.level_name(self.visibility_level).downcase
+    self.errors.add(:visibility_level, "#{level_name} is not allowed since the fork source project has lower visibility.")
   end
 
   def to_param
@@ -983,9 +994,25 @@ class Project < ActiveRecord::Base
     issues.opened.count
   end
 
-  def visibility_level_allowed?(level)
+  def visibility_level_allowed_as_fork?(level = self.visibility_level)
     return true unless forked?
-    Gitlab::VisibilityLevel.allowed_fork_levels(forked_from_project.visibility_level).include?(level.to_i)
+
+    # self.forked_from_project will be nil before the project is saved, so
+    # we need to go through the relation
+    original_project = forked_project_link.forked_from_project
+    return true unless original_project
+
+    level <= original_project.visibility_level
+  end
+
+  def visibility_level_allowed_by_group?(level = self.visibility_level)
+    return true unless group
+
+    level <= group.visibility_level
+  end
+
+  def visibility_level_allowed?(level = self.visibility_level)
+    visibility_level_allowed_as_fork?(level) && visibility_level_allowed_by_group?(level)
   end
 
   def runners_token