diff --git a/app/helpers/members_helper.rb b/app/helpers/members_helper.rb
index c70cd19b587a50ced7651c19cee33251ad4678e9..ec106418f2dc4f69b051688ab30ab1dcd8fa59d4 100644
--- a/app/helpers/members_helper.rb
+++ b/app/helpers/members_helper.rb
@@ -12,17 +12,6 @@ module MembersHelper
can?(current_user, action_member_permission(:admin, member), member.source)
end
- def can_see_request_access_button?(source)
- source_parent = source.respond_to?(:group) && source.group
-
- return false if source_parent && source.group.members.exists?(user_id: current_user.id)
- return false if source_parent && source.group.requesters.exists?(user_id: current_user.id)
- return false if source.members.exists?(user_id: current_user.id)
- return true if source.requesters.exists?(user_id: current_user.id)
-
- true
- end
-
def remove_member_message(member, user: nil)
user = current_user if defined?(current_user)
diff --git a/app/models/ability.rb b/app/models/ability.rb
index ba1f2ae40752c3558b68471bbdb9759192d68e59..eeb0ceba08113b9d385ee63cbfb8b3cf30dc3dc9 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -157,10 +157,11 @@ 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?
+ owner = user.admin? ||
+ project.owner == user ||
+ (project.group && project.group.has_owner?(user))
+ if owner
rules.push(*project_owner_rules)
end
@@ -169,6 +170,10 @@ class Ability
# Allow to read builds for internal projects
rules << :read_build if project.public_builds?
+
+ unless owner || project.team.member?(user) || project_group_member?(project, user)
+ rules << :request_access
+ end
end
if project.archived?
@@ -345,8 +350,11 @@ class Ability
rules = []
rules << :read_group if can_read_group?(user, group)
+ owner = user.admin? || group.has_owner?(user)
+ master = owner || group.has_master?(user)
+
# Only group masters and group owners can create new projects
- if group.has_master?(user) || group.has_owner?(user) || user.admin?
+ if master
rules += [
:create_projects,
:admin_milestones
@@ -354,7 +362,7 @@ class Ability
end
# Only group owner and administrators can admin group
- if group.has_owner?(user) || user.admin?
+ if owner
rules += [
:admin_group,
:admin_namespace,
@@ -363,6 +371,10 @@ class Ability
]
end
+ if group.public? || (group.internal? && !user.external?)
+ rules << :request_access unless group.users.include?(user)
+ end
+
rules.flatten
end
@@ -564,5 +576,13 @@ class Ability
rules
end
+
+ def project_group_member?(project, user)
+ project.group &&
+ (
+ project.group.members.exists?(user_id: user.id) ||
+ project.group.requesters.exists?(user_id: user.id)
+ )
+ end
end
end
diff --git a/app/views/shared/members/_access_request_buttons.html.haml b/app/views/shared/members/_access_request_buttons.html.haml
index 35dcdccc92122cd0ec16fd3c16159a34015bdd2d..eff914398bb0251a5e84de11e81a25c557bf6d6d 100644
--- a/app/views/shared/members/_access_request_buttons.html.haml
+++ b/app/views/shared/members/_access_request_buttons.html.haml
@@ -1,4 +1,4 @@
-- if can_see_request_access_button?(source)
+- if can?(current_user, :request_access, source)
- if requester = source.requesters.find_by(user_id: current_user.id)
= link_to 'Withdraw Access Request', polymorphic_path([:leave, source, :members]),
method: :delete,
diff --git a/db/migrate/20160705163108_remove_requesters_that_are_owners.rb b/db/migrate/20160705163108_remove_requesters_that_are_owners.rb
new file mode 100644
index 0000000000000000000000000000000000000000..1fca230c019498d820f37f6f01f2c828395ae935
--- /dev/null
+++ b/db/migrate/20160705163108_remove_requesters_that_are_owners.rb
@@ -0,0 +1,40 @@
+class RemoveRequestersThatAreOwners < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ def up
+ # Delete requesters that are owner of their projects and actually requested
+ # access to it
+ execute <<-SQL
+ DELETE FROM members
+ WHERE members.source_type = 'Project'
+ AND members.type = 'ProjectMember'
+ AND members.requested_at IS NOT NULL
+ AND members.user_id = (
+ SELECT namespaces.owner_id
+ FROM namespaces
+ JOIN projects ON namespaces.id = projects.namespace_id
+ WHERE namespaces.type IS NULL
+ AND projects.id = members.source_id
+ AND namespaces.owner_id = members.user_id);
+ SQL
+
+ # Delete requesters that are owner of their project's group and actually requested
+ # access to it
+ execute <<-SQL
+ DELETE FROM members
+ WHERE members.source_type = 'Project'
+ AND members.type = 'ProjectMember'
+ AND members.requested_at IS NOT NULL
+ AND members.user_id = (
+ SELECT namespaces.owner_id
+ FROM namespaces
+ JOIN projects ON namespaces.id = projects.namespace_id
+ WHERE namespaces.type = 'Group'
+ AND projects.id = members.source_id
+ AND namespaces.owner_id = members.user_id);
+ SQL
+ end
+
+ def down
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 5b9ed985facaff324a562d3418f2b7a4e87edab3..c1e88c1ed7ed2c1cc35f6e2fd980b651f5ad6112 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20160703180340) do
+ActiveRecord::Schema.define(version: 20160705163108) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
diff --git a/spec/features/groups/members/member_cannot_request_access_to_his_project_spec.rb b/spec/features/groups/members/member_cannot_request_access_to_his_project_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..37c433cc09a886abb55a695c667edb582fef3998
--- /dev/null
+++ b/spec/features/groups/members/member_cannot_request_access_to_his_project_spec.rb
@@ -0,0 +1,16 @@
+require 'spec_helper'
+
+feature 'Groups > Members > Member cannot request access to his project', feature: true do
+ let(:member) { create(:user) }
+ let(:group) { create(:group) }
+
+ background do
+ group.add_developer(member)
+ login_as(member)
+ visit group_path(group)
+ end
+
+ scenario 'member does not see the request access button' do
+ expect(page).not_to have_content 'Request Access'
+ end
+end
diff --git a/spec/features/projects/members/group_member_cannot_request_access_to_his_group_project_spec.rb b/spec/features/projects/members/group_member_cannot_request_access_to_his_group_project_spec.rb
index 4d5d656f00c3be9a7b494a0303baa8290393c100..ff9b60078066a98d6bb4e23469d20e40c964fd91 100644
--- a/spec/features/projects/members/group_member_cannot_request_access_to_his_group_project_spec.rb
+++ b/spec/features/projects/members/group_member_cannot_request_access_to_his_group_project_spec.rb
@@ -5,9 +5,6 @@ feature 'Projects > Members > Group member cannot request access to his group pr
let(:group) { create(:group) }
let(:project) { create(:project, namespace: group) }
- background do
- end
-
scenario 'owner does not see the request access button' do
group.add_owner(user)
login_and_visit_project_page(user)
diff --git a/spec/features/projects/members/member_cannot_request_access_to_his_project_spec.rb b/spec/features/projects/members/member_cannot_request_access_to_his_project_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..9564347e7335c1785559b9cd8a371a6e037728e4
--- /dev/null
+++ b/spec/features/projects/members/member_cannot_request_access_to_his_project_spec.rb
@@ -0,0 +1,16 @@
+require 'spec_helper'
+
+feature 'Projects > Members > Member cannot request access to his project', feature: true do
+ let(:member) { create(:user) }
+ let(:project) { create(:project) }
+
+ background do
+ project.team << [member, :developer]
+ login_as(member)
+ visit namespace_project_path(project.namespace, project)
+ end
+
+ scenario 'member does not see the request access button' do
+ expect(page).not_to have_content 'Request Access'
+ end
+end
diff --git a/spec/features/projects/members/owner_cannot_request_access_to_his_project_spec.rb b/spec/features/projects/members/owner_cannot_request_access_to_his_project_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..0e54c4fdf20b4c7bdcdf0df8dc6365942a8a9ecc
--- /dev/null
+++ b/spec/features/projects/members/owner_cannot_request_access_to_his_project_spec.rb
@@ -0,0 +1,16 @@
+require 'spec_helper'
+
+feature 'Projects > Members > Owner cannot request access to his project', feature: true do
+ let(:owner) { create(:user) }
+ let(:project) { create(:project) }
+
+ background do
+ project.team << [owner, :owner]
+ login_as(owner)
+ visit namespace_project_path(project.namespace, project)
+ end
+
+ scenario 'owner does not see the request access button' do
+ expect(page).not_to have_content 'Request Access'
+ end
+end
diff --git a/spec/helpers/members_helper_spec.rb b/spec/helpers/members_helper_spec.rb
index 7b2155e9a4e35b36a1dd43f9981f018d6efbb408..f75fdb739f6891b0f547928043cc2210e19f30d0 100644
--- a/spec/helpers/members_helper_spec.rb
+++ b/spec/helpers/members_helper_spec.rb
@@ -57,72 +57,6 @@ describe MembersHelper do
end
end
- describe '#can_see_request_access_button?' do
- let(:user) { create(:user) }
- let(:group) { create(:group, :public) }
- let(:project) { create(:project, :public, group: group) }
-
- before do
- allow(helper).to receive(:current_user).and_return(user)
- end
-
- context 'source is a group' do
- context 'current_user is not a member' do
- it 'returns true' do
- expect(helper.can_see_request_access_button?(group)).to be_truthy
- end
- end
-
- context 'current_user is a member' do
- it 'returns false' do
- group.add_owner(user)
-
- expect(helper.can_see_request_access_button?(group)).to be_falsy
- end
- end
-
- context 'current_user is a requester' do
- it 'returns true' do
- group.request_access(user)
-
- expect(helper.can_see_request_access_button?(group)).to be_truthy
- end
- end
- end
-
- context 'source is a project' do
- context 'current_user is not a member' do
- it 'returns true' do
- expect(helper.can_see_request_access_button?(project)).to be_truthy
- end
- end
-
- context 'current_user is a group member' do
- it 'returns false' do
- group.add_owner(user)
-
- expect(helper.can_see_request_access_button?(project)).to be_falsy
- end
- end
-
- context 'current_user is a group requester' do
- it 'returns false' do
- group.request_access(user)
-
- expect(helper.can_see_request_access_button?(project)).to be_falsy
- end
- end
-
- context 'current_user is a member' do
- it 'returns false' do
- project.team << [user, :master]
-
- expect(helper.can_see_request_access_button?(project)).to be_falsy
- end
- end
- end
- end
-
describe '#remove_member_message' do
let(:requester) { build(:user) }
let(:project) { create(:project) }