Commit 1532d202 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'fix/link-group-permissions' into 'master'

Check permissions when sharing project with group

## Summary

Unprivileged user was able to share project with group he didn't have access to, and therefore gain partial access to that group, which opened possibilities for further actions like listing private projects in that group.

See https://gitlab.com/gitlab-org/gitlab-ce/issues/15330

## Fix

This change introduces additional check for group read access.

## Further work

We can think about preventing such problems in the future (this is quite common problem) by moving permissions checks to another layer of abstraction (TBD).

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/15330

See merge request !1949
parents 51b777fa b9e13c24
...@@ -91,6 +91,9 @@ v 8.7.0 (unreleased) ...@@ -91,6 +91,9 @@ v 8.7.0 (unreleased)
- Execute system web hooks on push to the project - Execute system web hooks on push to the project
- Allow enable/disable push events for system hooks - Allow enable/disable push events for system hooks
v 8.6.7 (unreleased)
- Fix vulnerability that made it possible to enumerate private projects belonging to group
v 8.6.6 v 8.6.6
- Expire the exists cache before deletion to ensure project dir actually exists (Stan Hu). !3413 - Expire the exists cache before deletion to ensure project dir actually exists (Stan Hu). !3413
- Fix error on language detection when repository has no HEAD (e.g., master branch) (Jeroen Bobbeldijk). !3654 - Fix error on language detection when repository has no HEAD (e.g., master branch) (Jeroen Bobbeldijk). !3654
......
...@@ -7,10 +7,12 @@ class Projects::GroupLinksController < Projects::ApplicationController ...@@ -7,10 +7,12 @@ class Projects::GroupLinksController < Projects::ApplicationController
end end
def create def create
link = project.project_group_links.new group = Group.find(params[:link_group_id])
link.group_id = params[:link_group_id] return render_404 unless can?(current_user, :read_group, group)
link.group_access = params[:link_group_access]
link.save project.project_group_links.create(
group: group, group_access: params[:link_group_access]
)
redirect_to namespace_project_group_links_path(project.namespace, project) redirect_to namespace_project_group_links_path(project.namespace, project)
end end
......
require 'spec_helper'
describe Projects::GroupLinksController do
let(:project) { create(:project, :private) }
let(:group) { create(:group, :private) }
let(:user) { create(:user) }
before do
project.team << [user, :master]
sign_in(user)
end
describe '#create' do
shared_context 'link project to group' do
before do
post(:create, namespace_id: project.namespace.to_param,
project_id: project.to_param,
link_group_id: group.id,
link_group_access: ProjectGroupLink.default_access)
end
end
context 'when user has access to group he want to link project to' do
before { group.add_developer(user) }
include_context 'link project to group'
it 'links project with selected group' do
expect(group.shared_projects).to include project
end
it 'redirects to project group links page'do
expect(response).to redirect_to(
namespace_project_group_links_path(project.namespace, project)
)
end
end
context 'when user doers not have access to group he want to link to' do
include_context 'link project to group'
it 'renders 404' do
expect(response.status).to eq 404
end
it 'does not share project with that group' do
expect(group.shared_projects).to_not include project
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment