Commit 5e9e5692 authored by Douwe Maan's avatar Douwe Maan Committed by Robert Speicher

Merge branch...

Merge branch 'security-10-4-25223-snippets-finder-doesnt-obey-feature-visibility' into 'security-10-4'

[Port for security-10-4]: Makes SnippetFinder ensure feature visibility
parent 721fab66
# Snippets Finder
#
# Used to filter Snippets collections by a set of params
#
# Arguments.
#
# current_user - The current user, nil also can be used.
# params:
# visibility (integer) - Individual snippet visibility: Public(20), internal(10) or private(0).
# project (Project) - Project related.
# author (User) - Author related.
#
# params are optional
class SnippetsFinder < UnionFinder class SnippetsFinder < UnionFinder
attr_accessor :current_user, :params include Gitlab::Allowable
attr_accessor :current_user, :params, :project
def initialize(current_user, params = {}) def initialize(current_user, params = {})
@current_user = current_user @current_user = current_user
@params = params @params = params
@project = params[:project]
end end
def execute def execute
items = init_collection items = init_collection
items = by_project(items)
items = by_author(items) items = by_author(items)
items = by_visibility(items) items = by_visibility(items)
...@@ -18,25 +32,42 @@ class SnippetsFinder < UnionFinder ...@@ -18,25 +32,42 @@ class SnippetsFinder < UnionFinder
private private
def init_collection def init_collection
items = Snippet.all if project.present?
authorized_snippets_from_project
else
authorized_snippets
end
end
accessible(items) def authorized_snippets_from_project
if can?(current_user, :read_project_snippet, project)
if project.team.member?(current_user)
project.snippets
else
project.snippets.public_to_user(current_user)
end
else
Snippet.none
end
end end
def accessible(items) def authorized_snippets
segments = [] Snippet.where(feature_available_projects.or(not_project_related)).public_or_visible_to_user(current_user)
segments << items.public_to_user(current_user) end
segments << authorized_to_user(items) if current_user
def feature_available_projects
projects = Project.public_or_visible_to_user(current_user)
.with_feature_available_for_user(:snippets, current_user).select(:id)
arel_query = Arel::Nodes::SqlLiteral.new(projects.to_sql)
table[:project_id].in(arel_query)
end
find_union(segments, Snippet.includes(:author)) def not_project_related
table[:project_id].eq(nil)
end end
def authorized_to_user(items) def table
items.where( Snippet.arel_table
'author_id = :author_id
OR project_id IN (:project_ids)',
author_id: current_user.id,
project_ids: current_user.authorized_projects.select(:id))
end end
def by_visibility(items) def by_visibility(items)
...@@ -53,12 +84,6 @@ class SnippetsFinder < UnionFinder ...@@ -53,12 +84,6 @@ class SnippetsFinder < UnionFinder
items.where(author_id: params[:author].id) items.where(author_id: params[:author].id)
end end
def by_project(items)
return items unless params[:project]
items.where(project_id: params[:project].id)
end
def visibility_from_scope def visibility_from_scope
case params[:scope].to_s case params[:scope].to_s
when 'are_private' when 'are_private'
......
...@@ -74,6 +74,27 @@ class Snippet < ActiveRecord::Base ...@@ -74,6 +74,27 @@ class Snippet < ActiveRecord::Base
@link_reference_pattern ||= super("snippets", /(?<snippet>\d+)/) @link_reference_pattern ||= super("snippets", /(?<snippet>\d+)/)
end end
# Returns a collection of snippets that are either public or visible to the
# logged in user.
#
# This method does not verify the user actually has the access to the project
# the snippet is in, so it should be only used on a relation that's already scoped
# for project access
def self.public_or_visible_to_user(user = nil)
if user
authorized = user
.project_authorizations
.select(1)
.where('project_authorizations.project_id = snippets.project_id')
levels = Gitlab::VisibilityLevel.levels_for_user(user)
where('EXISTS (?) OR snippets.visibility_level IN (?) or snippets.author_id = (?)', authorized, levels, user.id)
else
public_to_user
end
end
def to_reference(from = nil, full: false) def to_reference(from = nil, full: false)
reference = "#{self.class.reference_prefix}#{id}" reference = "#{self.class.reference_prefix}#{id}"
......
...@@ -119,7 +119,6 @@ class ProjectPolicy < BasePolicy ...@@ -119,7 +119,6 @@ class ProjectPolicy < BasePolicy
enable :create_note enable :create_note
enable :upload_file enable :upload_file
enable :read_cycle_analytics enable :read_cycle_analytics
enable :read_project_snippet
end end
rule { can?(:reporter_access) }.policy do rule { can?(:reporter_access) }.policy do
......
require 'spec_helper' require 'spec_helper'
describe SnippetsFinder do describe SnippetsFinder do
let(:user) { create :user } include Gitlab::Allowable
let(:user1) { create :user } using RSpec::Parameterized::TableSyntax
let(:group) { create :group, :public }
let(:project1) { create(:project, :public, group: group) }
let(:project2) { create(:project, :private, group: group) }
context 'all snippets visible to a user' do
let!(:snippet1) { create(:personal_snippet, :private) }
let!(:snippet2) { create(:personal_snippet, :internal) }
let!(:snippet3) { create(:personal_snippet, :public) }
let!(:project_snippet1) { create(:project_snippet, :private) }
let!(:project_snippet2) { create(:project_snippet, :internal) }
let!(:project_snippet3) { create(:project_snippet, :public) }
it "returns all private and internal snippets" do
snippets = described_class.new(user, scope: :all).execute
expect(snippets).to include(snippet2, snippet3, project_snippet2, project_snippet3)
expect(snippets).not_to include(snippet1, project_snippet1)
end
it "returns all public snippets" do
snippets = described_class.new(nil, scope: :all).execute
expect(snippets).to include(snippet3, project_snippet3)
expect(snippets).not_to include(snippet1, snippet2, project_snippet1, project_snippet2)
end
it "returns all public and internal snippets for normal user" do
snippets = described_class.new(user).execute
expect(snippets).to include(snippet2, snippet3, project_snippet2, project_snippet3)
expect(snippets).not_to include(snippet1, project_snippet1)
end
it "returns all public snippets for non authorized user" do
snippets = described_class.new(nil).execute
expect(snippets).to include(snippet3, project_snippet3)
expect(snippets).not_to include(snippet1, snippet2, project_snippet1, project_snippet2)
end
it "returns all public and authored snippets for external user" do
external_user = create(:user, :external)
authored_snippet = create(:personal_snippet, :internal, author: external_user)
snippets = described_class.new(external_user).execute
expect(snippets).to include(snippet3, project_snippet3, authored_snippet)
expect(snippets).not_to include(snippet1, snippet2, project_snippet1, project_snippet2)
end
end
context 'filter by visibility' do context 'filter by visibility' do
let!(:snippet1) { create(:personal_snippet, :private) } let!(:snippet1) { create(:personal_snippet, :private) }
...@@ -67,6 +18,7 @@ describe SnippetsFinder do ...@@ -67,6 +18,7 @@ describe SnippetsFinder do
end end
context 'filter by scope' do context 'filter by scope' do
let(:user) { create :user }
let!(:snippet1) { create(:personal_snippet, :private, author: user) } let!(:snippet1) { create(:personal_snippet, :private, author: user) }
let!(:snippet2) { create(:personal_snippet, :internal, author: user) } let!(:snippet2) { create(:personal_snippet, :internal, author: user) }
let!(:snippet3) { create(:personal_snippet, :public, author: user) } let!(:snippet3) { create(:personal_snippet, :public, author: user) }
...@@ -84,7 +36,7 @@ describe SnippetsFinder do ...@@ -84,7 +36,7 @@ describe SnippetsFinder do
expect(snippets).not_to include(snippet2, snippet3) expect(snippets).not_to include(snippet2, snippet3)
end end
it "returns all snippets for 'are_interna;' scope" do it "returns all snippets for 'are_internal' scope" do
snippets = described_class.new(user, scope: :are_internal).execute snippets = described_class.new(user, scope: :are_internal).execute
expect(snippets).to include(snippet2) expect(snippets).to include(snippet2)
...@@ -100,6 +52,8 @@ describe SnippetsFinder do ...@@ -100,6 +52,8 @@ describe SnippetsFinder do
end end
context 'filter by author' do context 'filter by author' do
let(:user) { create :user }
let(:user1) { create :user }
let!(:snippet1) { create(:personal_snippet, :private, author: user) } let!(:snippet1) { create(:personal_snippet, :private, author: user) }
let!(:snippet2) { create(:personal_snippet, :internal, author: user) } let!(:snippet2) { create(:personal_snippet, :internal, author: user) }
let!(:snippet3) { create(:personal_snippet, :public, author: user) } let!(:snippet3) { create(:personal_snippet, :public, author: user) }
...@@ -147,6 +101,10 @@ describe SnippetsFinder do ...@@ -147,6 +101,10 @@ describe SnippetsFinder do
end end
context 'filter by project' do context 'filter by project' do
let(:user) { create :user }
let(:group) { create :group, :public }
let(:project1) { create(:project, :public, group: group) }
before do before do
@snippet1 = create(:project_snippet, :private, project: project1) @snippet1 = create(:project_snippet, :private, project: project1)
@snippet2 = create(:project_snippet, :internal, project: project1) @snippet2 = create(:project_snippet, :internal, project: project1)
...@@ -203,4 +161,9 @@ describe SnippetsFinder do ...@@ -203,4 +161,9 @@ describe SnippetsFinder do
expect(snippets).to include(@snippet1) expect(snippets).to include(@snippet1)
end end
end end
describe "#execute" do
# Snippet visibility scenarios are included in more details in spec/support/snippet_visibility.rb
include_examples 'snippet visibility', described_class
end
end end
require 'spec_helper' require 'spec_helper'
# Snippet visibility scenarios are included in more details in spec/support/snippet_visibility.rb
describe PersonalSnippetPolicy do describe PersonalSnippetPolicy do
let(:regular_user) { create(:user) } let(:regular_user) { create(:user) }
let(:external_user) { create(:user, :external) } let(:external_user) { create(:user, :external) }
......
require 'spec_helper' require 'spec_helper'
# Snippet visibility scenarios are included in more details in spec/support/snippet_visibility.rb
describe ProjectSnippetPolicy do describe ProjectSnippetPolicy do
let(:regular_user) { create(:user) } let(:regular_user) { create(:user) }
let(:external_user) { create(:user, :external) } let(:external_user) { create(:user, :external) }
......
...@@ -32,6 +32,27 @@ describe API::Snippets do ...@@ -32,6 +32,27 @@ describe API::Snippets do
expect(json_response).to be_an Array expect(json_response).to be_an Array
expect(json_response.size).to eq(0) expect(json_response.size).to eq(0)
end end
it 'returns 404 for non-authenticated' do
create(:personal_snippet, :internal)
get api("/snippets/")
expect(response).to have_gitlab_http_status(401)
end
it 'does not return snippets related to a project with disable feature visibility' do
project = create(:project)
create(:project_member, project: project, user: user)
public_snippet = create(:personal_snippet, :public, author: user, project: project)
project.project_feature.update_attribute(:snippets_access_level, 0)
get api("/snippets/", user)
json_response.each do |snippet|
expect(snippet["id"]).not_to eq(public_snippet.id)
end
end
end end
describe 'GET /snippets/public' do describe 'GET /snippets/public' do
......
...@@ -2,7 +2,7 @@ require 'spec_helper' ...@@ -2,7 +2,7 @@ require 'spec_helper'
describe Search::SnippetService do describe Search::SnippetService do
let(:author) { create(:author) } let(:author) { create(:author) }
let(:project) { create(:project) } let(:project) { create(:project, :public) }
let!(:public_snippet) { create(:snippet, :public, content: 'password: XXX') } let!(:public_snippet) { create(:snippet, :public, content: 'password: XXX') }
let!(:internal_snippet) { create(:snippet, :internal, content: 'password: XXX') } let!(:internal_snippet) { create(:snippet, :internal, content: 'password: XXX') }
......
RSpec.shared_examples 'snippet visibility' do
let!(:author) { create(:user) }
let!(:member) { create(:user) }
let!(:external) { create(:user, :external) }
let!(:snippet_type_visibilities) do
{
public: Snippet::PUBLIC,
internal: Snippet::INTERNAL,
private: Snippet::PRIVATE
}
end
context "For project snippets" do
let!(:users) do
{
unauthenticated: nil,
external: external,
non_member: create(:user),
member: member,
author: author
}
end
let!(:project_type_visibilities) do
{
public: Gitlab::VisibilityLevel::PUBLIC,
internal: Gitlab::VisibilityLevel::INTERNAL,
private: Gitlab::VisibilityLevel::PRIVATE
}
end
let(:project_feature_visibilities) do
{
enabled: ProjectFeature::ENABLED,
private: ProjectFeature::PRIVATE,
disabled: ProjectFeature::DISABLED
}
end
where(:project_type, :feature_visibility, :user_type, :snippet_type, :outcome) do
[
# Public projects
[:public, :enabled, :unauthenticated, :public, true],
[:public, :enabled, :unauthenticated, :internal, false],
[:public, :enabled, :unauthenticated, :private, false],
[:public, :enabled, :external, :public, true],
[:public, :enabled, :external, :internal, false],
[:public, :enabled, :external, :private, false],
[:public, :enabled, :non_member, :public, true],
[:public, :enabled, :non_member, :internal, true],
[:public, :enabled, :non_member, :private, false],
[:public, :enabled, :member, :public, true],
[:public, :enabled, :member, :internal, true],
[:public, :enabled, :member, :private, true],
[:public, :enabled, :author, :public, true],
[:public, :enabled, :author, :internal, true],
[:public, :enabled, :author, :private, true],
[:public, :private, :unauthenticated, :public, false],
[:public, :private, :unauthenticated, :internal, false],
[:public, :private, :unauthenticated, :private, false],
[:public, :private, :external, :public, false],
[:public, :private, :external, :internal, false],
[:public, :private, :external, :private, false],
[:public, :private, :non_member, :public, false],
[:public, :private, :non_member, :internal, false],
[:public, :private, :non_member, :private, false],
[:public, :private, :member, :public, true],
[:public, :private, :member, :internal, true],
[:public, :private, :member, :private, true],
[:public, :private, :author, :public, true],
[:public, :private, :author, :internal, true],
[:public, :private, :author, :private, true],
[:public, :disabled, :unauthenticated, :public, false],
[:public, :disabled, :unauthenticated, :internal, false],
[:public, :disabled, :unauthenticated, :private, false],
[:public, :disabled, :external, :public, false],
[:public, :disabled, :external, :internal, false],
[:public, :disabled, :external, :private, false],
[:public, :disabled, :non_member, :public, false],
[:public, :disabled, :non_member, :internal, false],
[:public, :disabled, :non_member, :private, false],
[:public, :disabled, :member, :public, false],
[:public, :disabled, :member, :internal, false],
[:public, :disabled, :member, :private, false],
[:public, :disabled, :author, :public, false],
[:public, :disabled, :author, :internal, false],
[:public, :disabled, :author, :private, false],
# Internal projects
[:internal, :enabled, :unauthenticated, :public, false],
[:internal, :enabled, :unauthenticated, :internal, false],
[:internal, :enabled, :unauthenticated, :private, false],
[:internal, :enabled, :external, :public, false],
[:internal, :enabled, :external, :internal, false],
[:internal, :enabled, :external, :private, false],
[:internal, :enabled, :non_member, :public, true],
[:internal, :enabled, :non_member, :internal, true],
[:internal, :enabled, :non_member, :private, false],
[:internal, :enabled, :member, :public, true],
[:internal, :enabled, :member, :internal, true],
[:internal, :enabled, :member, :private, true],
[:internal, :enabled, :author, :public, true],
[:internal, :enabled, :author, :internal, true],
[:internal, :enabled, :author, :private, true],
[:internal, :private, :unauthenticated, :public, false],
[:internal, :private, :unauthenticated, :internal, false],
[:internal, :private, :unauthenticated, :private, false],
[:internal, :private, :external, :public, false],
[:internal, :private, :external, :internal, false],
[:internal, :private, :external, :private, false],
[:internal, :private, :non_member, :public, false],
[:internal, :private, :non_member, :internal, false],
[:internal, :private, :non_member, :private, false],
[:internal, :private, :member, :public, true],
[:internal, :private, :member, :internal, true],
[:internal, :private, :member, :private, true],
[:internal, :private, :author, :public, true],
[:internal, :private, :author, :internal, true],
[:internal, :private, :author, :private, true],
[:internal, :disabled, :unauthenticated, :public, false],
[:internal, :disabled, :unauthenticated, :internal, false],
[:internal, :disabled, :unauthenticated, :private, false],
[:internal, :disabled, :external, :public, false],
[:internal, :disabled, :external, :internal, false],
[:internal, :disabled, :external, :private, false],
[:internal, :disabled, :non_member, :public, false],
[:internal, :disabled, :non_member, :internal, false],
[:internal, :disabled, :non_member, :private, false],
[:internal, :disabled, :member, :public, false],
[:internal, :disabled, :member, :internal, false],
[:internal, :disabled, :member, :private, false],
[:internal, :disabled, :author, :public, false],
[:internal, :disabled, :author, :internal, false],
[:internal, :disabled, :author, :private, false],
# Private projects
[:private, :enabled, :unauthenticated, :public, false],
[:private, :enabled, :unauthenticated, :internal, false],
[:private, :enabled, :unauthenticated, :private, false],
[:private, :enabled, :external, :public, true],
[:private, :enabled, :external, :internal, true],
[:private, :enabled, :external, :private, true],
[:private, :enabled, :non_member, :public, false],
[:private, :enabled, :non_member, :internal, false],
[:private, :enabled, :non_member, :private, false],
[:private, :enabled, :member, :public, true],
[:private, :enabled, :member, :internal, true],
[:private, :enabled, :member, :private, true],
[:private, :enabled, :author, :public, true],
[:private, :enabled, :author, :internal, true],
[:private, :enabled, :author, :private, true],
[:private, :private, :unauthenticated, :public, false],
[:private, :private, :unauthenticated, :internal, false],
[:private, :private, :unauthenticated, :private, false],
[:private, :private, :external, :public, true],
[:private, :private, :external, :internal, true],
[:private, :private, :external, :private, true],
[:private, :private, :non_member, :public, false],
[:private, :private, :non_member, :internal, false],
[:private, :private, :non_member, :private, false],
[:private, :private, :member, :public, true],
[:private, :private, :member, :internal, true],
[:private, :private, :member, :private, true],
[:private, :private, :author, :public, true],
[:private, :private, :author, :internal, true],
[:private, :private, :author, :private, true],
[:private, :disabled, :unauthenticated, :public, false],
[:private, :disabled, :unauthenticated, :internal, false],
[:private, :disabled, :unauthenticated, :private, false],
[:private, :disabled, :external, :public, false],
[:private, :disabled, :external, :internal, false],
[:private, :disabled, :external, :private, false],
[:private, :disabled, :non_member, :public, false],
[:private, :disabled, :non_member, :internal, false],
[:private, :disabled, :non_member, :private, false],
[:private, :disabled, :member, :public, false],
[:private, :disabled, :member, :internal, false],
[:private, :disabled, :member, :private, false],
[:private, :disabled, :author, :public, false],
[:private, :disabled, :author, :internal, false],
[:private, :disabled, :author, :private, false]
]
end
with_them do
let!(:project) { create(:project, visibility_level: project_type_visibilities[project_type]) }
let!(:project_feature) { project.project_feature.update_column(:snippets_access_level, project_feature_visibilities[feature_visibility]) }
let!(:user) { users[user_type] }
let!(:snippet) { create(:project_snippet, visibility_level: snippet_type_visibilities[snippet_type], project: project, author: author) }
let!(:members) do
project.add_developer(author)
project.add_developer(member)
project.add_developer(external) if project.private?
end
context "For #{params[:project_type]} project and #{params[:user_type]} users" do
it 'should agree with the read_project_snippet policy' do
expect(can?(user, :read_project_snippet, snippet)).to eq(outcome)
end
it 'should return proper outcome' do
results = described_class.new(user, project: project).execute
expect(results.include?(snippet)).to eq(outcome)
end
end
context "Without a given project and #{params[:user_type]} users" do
it 'should return proper outcome' do
results = described_class.new(user).execute
expect(results.include?(snippet)).to eq(outcome)
end
end
end
end
context 'For personal snippets' do
let!(:users) do
{
unauthenticated: nil,
external: external,
non_member: create(:user),
author: author
}
end
where(:snippet_visibility, :user_type, :outcome) do
[
[:public, :unauthenticated, true],
[:public, :external, true],
[:public, :non_member, true],
[:public, :author, true],
[:internal, :unauthenticated, false],
[:internal, :external, false],
[:internal, :non_member, true],
[:internal, :author, true],
[:private, :unauthenticated, false],
[:private, :external, false],
[:private, :non_member, false],
[:private, :author, true]
]
end
with_them do
let!(:user) { users[user_type] }
let!(:snippet) { create(:personal_snippet, visibility_level: snippet_type_visibilities[snippet_visibility], author: author) }
context "For personal and #{params[:snippet_visibility]} snippets with #{params[:user_type]} user" do
it 'should agree with read_personal_snippet policy' do
expect(can?(user, :read_personal_snippet, snippet)).to eq(outcome)
end
it 'should return proper outcome' do
results = described_class.new(user).execute
expect(results.include?(snippet)).to eq(outcome)
end
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