Commit 8d5a55ef authored by Douwe Maan's avatar Douwe Maan Committed by Rémy Coutable

Merge branch 'fix-14607' into 'master'

Ensure private project snippets are not viewable by unauthorized people

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

See merge request !1946
parent 7b1fa197
...@@ -16,6 +16,8 @@ v 8.6.2 ...@@ -16,6 +16,8 @@ v 8.6.2
- Fix an issue with width of project select dropdown. !3386 - Fix an issue with width of project select dropdown. !3386
- Fix error 500 with cancel button on issuable edit form. !3392 - Fix error 500 with cancel button on issuable edit form. !3392
- Fix background when editing a highlighted note. !3423 - Fix background when editing a highlighted note. !3423
- Remove tabstop from the WIP toggle links. !3426
- Ensure private project snippets are not viewable by unauthorized people.
v 8.6.2 (unreleased) v 8.6.2 (unreleased)
- Comments on confidential issues don't show up in activity feed to non-members - Comments on confidential issues don't show up in activity feed to non-members
......
...@@ -3,7 +3,7 @@ class Projects::SnippetsController < Projects::ApplicationController ...@@ -3,7 +3,7 @@ class Projects::SnippetsController < Projects::ApplicationController
before_action :snippet, only: [:show, :edit, :destroy, :update, :raw] before_action :snippet, only: [:show, :edit, :destroy, :update, :raw]
# Allow read any snippet # Allow read any snippet
before_action :authorize_read_project_snippet! before_action :authorize_read_project_snippet!, except: [:new, :create, :index]
# Allow write(create) snippet # Allow write(create) snippet
before_action :authorize_create_project_snippet!, only: [:new, :create] before_action :authorize_create_project_snippet!, only: [:new, :create]
...@@ -81,6 +81,10 @@ class Projects::SnippetsController < Projects::ApplicationController ...@@ -81,6 +81,10 @@ class Projects::SnippetsController < Projects::ApplicationController
@snippet ||= @project.snippets.find(params[:id]) @snippet ||= @project.snippets.find(params[:id])
end end
def authorize_read_project_snippet!
return render_404 unless can?(current_user, :read_project_snippet, @snippet)
end
def authorize_update_project_snippet! def authorize_update_project_snippet!
return render_404 unless can?(current_user, :update_project_snippet, @snippet) return render_404 unless can?(current_user, :update_project_snippet, @snippet)
end end
......
...@@ -27,6 +27,8 @@ class Ability ...@@ -27,6 +27,8 @@ class Ability
case true case true
when subject.is_a?(PersonalSnippet) when subject.is_a?(PersonalSnippet)
anonymous_personal_snippet_abilities(subject) anonymous_personal_snippet_abilities(subject)
when subject.is_a?(ProjectSnippet)
anonymous_project_snippet_abilities(subject)
when subject.is_a?(CommitStatus) when subject.is_a?(CommitStatus)
anonymous_commit_status_abilities(subject) anonymous_commit_status_abilities(subject)
when subject.is_a?(Project) || subject.respond_to?(:project) when subject.is_a?(Project) || subject.respond_to?(:project)
...@@ -100,6 +102,14 @@ class Ability ...@@ -100,6 +102,14 @@ class Ability
end end
end end
def anonymous_project_snippet_abilities(snippet)
if snippet.public?
[:read_project_snippet]
else
[]
end
end
def global_abilities(user) def global_abilities(user)
rules = [] rules = []
rules << :create_group if user.can_create_group rules << :create_group if user.can_create_group
...@@ -338,24 +348,22 @@ class Ability ...@@ -338,24 +348,22 @@ class Ability
end end
end end
[:note, :project_snippet].each do |name| def note_abilities(user, note)
define_method "#{name}_abilities" do |user, subject| rules = []
rules = []
if subject.author == user
rules += [
:"read_#{name}",
:"update_#{name}",
:"admin_#{name}"
]
end
if subject.respond_to?(:project) && subject.project if note.author == user
rules += project_abilities(user, subject.project) rules += [
end :read_note,
:update_note,
:admin_note
]
end
rules if note.respond_to?(:project) && note.project
rules += project_abilities(user, note.project)
end end
rules
end end
def personal_snippet_abilities(user, snippet) def personal_snippet_abilities(user, snippet)
...@@ -376,6 +384,24 @@ class Ability ...@@ -376,6 +384,24 @@ class Ability
rules rules
end end
def project_snippet_abilities(user, snippet)
rules = []
if snippet.author == user || user.admin?
rules += [
:read_project_snippet,
:update_project_snippet,
:admin_project_snippet
]
end
if snippet.public? || (snippet.internal? && !user.external?) || (snippet.private? && snippet.project.team.member?(user))
rules << :read_project_snippet
end
rules
end
def group_member_abilities(user, subject) def group_member_abilities(user, subject)
rules = [] rules = []
target_user = subject.user target_user = subject.user
......
require 'spec_helper'
describe Projects::SnippetsController do
let(:project) { create(:project_empty_repo, :public, snippets_enabled: true) }
let(:user) { create(:user) }
let(:user2) { create(:user) }
before do
project.team << [user, :master]
project.team << [user2, :master]
end
describe 'GET #index' do
context 'when the project snippet is private' do
let!(:project_snippet) { create(:project_snippet, :private, project: project, author: user) }
context 'when anonymous' do
it 'does not include the private snippet' do
get :index, namespace_id: project.namespace.path, project_id: project.path
expect(assigns(:snippets)).not_to include(project_snippet)
expect(response.status).to eq(200)
end
end
context 'when signed in as the author' do
before { sign_in(user) }
it 'renders the snippet' do
get :index, namespace_id: project.namespace.path, project_id: project.path
expect(assigns(:snippets)).to include(project_snippet)
expect(response.status).to eq(200)
end
end
context 'when signed in as a project member' do
before { sign_in(user2) }
it 'renders the snippet' do
get :index, namespace_id: project.namespace.path, project_id: project.path
expect(assigns(:snippets)).to include(project_snippet)
expect(response.status).to eq(200)
end
end
end
end
%w[show raw].each do |action|
describe "GET ##{action}" do
context 'when the project snippet is private' do
let(:project_snippet) { create(:project_snippet, :private, project: project, author: user) }
context 'when anonymous' do
it 'responds with status 404' do
get action, namespace_id: project.namespace.path, project_id: project.path, id: project_snippet.to_param
expect(response.status).to eq(404)
end
end
context 'when signed in as the author' do
before { sign_in(user) }
it 'renders the snippet' do
get action, namespace_id: project.namespace.path, project_id: project.path, id: project_snippet.to_param
expect(assigns(:snippet)).to eq(project_snippet)
expect(response.status).to eq(200)
end
end
context 'when signed in as a project member' do
before { sign_in(user2) }
it 'renders the snippet' do
get action, namespace_id: project.namespace.path, project_id: project.path, id: project_snippet.to_param
expect(assigns(:snippet)).to eq(project_snippet)
expect(response.status).to eq(200)
end
end
end
context 'when the project snippet does not exist' do
context 'when anonymous' do
it 'responds with status 404' do
get action, namespace_id: project.namespace.path, project_id: project.path, id: 42
expect(response.status).to eq(404)
end
end
context 'when signed in' do
before { sign_in(user) }
it 'responds with status 404' do
get action, namespace_id: project.namespace.path, project_id: project.path, id: 42
expect(response.status).to eq(404)
end
end
end
end
end
end
require 'spec_helper'
describe "Internal Project Snippets Access", feature: true do
include AccessMatchers
let(:project) { create(:project, :internal) }
let(:owner) { project.owner }
let(:master) { create(:user) }
let(:developer) { create(:user) }
let(:reporter) { create(:user) }
let(:guest) { create(:user) }
let(:internal_snippet) { create(:project_snippet, :internal, project: project, author: owner) }
let(:private_snippet) { create(:project_snippet, :private, project: project, author: owner) }
before do
project.team << [master, :master]
project.team << [developer, :developer]
project.team << [reporter, :reporter]
project.team << [guest, :guest]
end
describe "GET /:project_path/snippets" do
subject { namespace_project_snippets_path(project.namespace, project) }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for developer }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_allowed_for :user }
it { is_expected.to be_denied_for :external }
it { is_expected.to be_denied_for :visitor }
end
describe "GET /:project_path/snippets/new" do
subject { new_namespace_project_snippet_path(project.namespace, project) }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for developer }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_denied_for guest }
it { is_expected.to be_denied_for :user }
it { is_expected.to be_denied_for :external }
it { is_expected.to be_denied_for :visitor }
end
describe "GET /:project_path/snippets/:id for an internal snippet" do
subject { namespace_project_snippet_path(project.namespace, project, internal_snippet) }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for developer }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_allowed_for :user }
it { is_expected.to be_denied_for :external }
it { is_expected.to be_denied_for :visitor }
end
describe "GET /:project_path/snippets/:id for a private snippet" do
subject { namespace_project_snippet_path(project.namespace, project, private_snippet) }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for developer }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_denied_for :user }
it { is_expected.to be_denied_for :external }
it { is_expected.to be_denied_for :visitor }
end
end
require 'spec_helper'
describe "Private Project Snippets Access", feature: true do
include AccessMatchers
let(:project) { create(:project, :private) }
let(:owner) { project.owner }
let(:master) { create(:user) }
let(:developer) { create(:user) }
let(:reporter) { create(:user) }
let(:guest) { create(:user) }
let(:private_snippet) { create(:project_snippet, :private, project: project, author: owner) }
before do
project.team << [master, :master]
project.team << [developer, :developer]
project.team << [reporter, :reporter]
project.team << [guest, :guest]
end
describe "GET /:project_path/snippets" do
subject { namespace_project_snippets_path(project.namespace, project) }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for developer }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_denied_for :user }
it { is_expected.to be_denied_for :external }
it { is_expected.to be_denied_for :visitor }
end
describe "GET /:project_path/snippets/new" do
subject { new_namespace_project_snippet_path(project.namespace, project) }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for developer }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_denied_for guest }
it { is_expected.to be_denied_for :user }
it { is_expected.to be_denied_for :external }
it { is_expected.to be_denied_for :visitor }
end
describe "GET /:project_path/snippets/:id for a private snippet" do
subject { namespace_project_snippet_path(project.namespace, project, private_snippet) }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for developer }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_denied_for :user }
it { is_expected.to be_denied_for :external }
it { is_expected.to be_denied_for :visitor }
end
end
require 'spec_helper'
describe "Public Project Snippets Access", feature: true do
include AccessMatchers
let(:project) { create(:project, :public) }
let(:owner) { project.owner }
let(:master) { create(:user) }
let(:developer) { create(:user) }
let(:reporter) { create(:user) }
let(:guest) { create(:user) }
let(:public_snippet) { create(:project_snippet, :public, project: project, author: owner) }
let(:internal_snippet) { create(:project_snippet, :internal, project: project, author: owner) }
let(:private_snippet) { create(:project_snippet, :private, project: project, author: owner) }
before do
project.team << [master, :master]
project.team << [developer, :developer]
project.team << [reporter, :reporter]
project.team << [guest, :guest]
end
describe "GET /:project_path/snippets" do
subject { namespace_project_snippets_path(project.namespace, project) }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for developer }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_allowed_for :user }
it { is_expected.to be_allowed_for :external }
it { is_expected.to be_allowed_for :visitor }
end
describe "GET /:project_path/snippets/new" do
subject { new_namespace_project_snippet_path(project.namespace, project) }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for developer }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_denied_for guest }
it { is_expected.to be_denied_for :user }
it { is_expected.to be_denied_for :external }
it { is_expected.to be_denied_for :visitor }
end
describe "GET /:project_path/snippets/:id for a public snippet" do
subject { namespace_project_snippet_path(project.namespace, project, public_snippet) }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for developer }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_allowed_for :user }
it { is_expected.to be_allowed_for :external }
it { is_expected.to be_allowed_for :visitor }
end
describe "GET /:project_path/snippets/:id for an internal snippet" do
subject { namespace_project_snippet_path(project.namespace, project, internal_snippet) }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for developer }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_allowed_for :user }
it { is_expected.to be_denied_for :external }
it { is_expected.to be_denied_for :visitor }
end
describe "GET /:project_path/snippets/:id for a private snippet" do
subject { namespace_project_snippet_path(project.namespace, project, private_snippet) }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for developer }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_denied_for :user }
it { is_expected.to be_denied_for :external }
it { is_expected.to be_denied_for :visitor }
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