summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2017-04-28 22:06:27 +0000
committerBob Van Landuyt <bob@gitlab.com>2017-05-10 11:09:17 +0200
commit050d92f8177513b74d6c18d75d35a62f5fa6addd (patch)
treedb6caf2ce3c47d1127974384c2c78c11bd7c038f
parentc26e9027d31b0735cea438eaa7bf787bc5b6e3a7 (diff)
downloadgitlab-ce-050d92f8177513b74d6c18d75d35a62f5fa6addd.tar.gz
Merge branch 'snippets-finder-visibility' into 'security'
Refactor snippets finder & dont return internal snippets for external users See merge request !2094
-rw-r--r--app/controllers/dashboard/snippets_controller.rb7
-rw-r--r--app/controllers/explore/snippets_controller.rb2
-rw-r--r--app/controllers/projects/snippets_controller.rb5
-rw-r--r--app/controllers/snippets_controller.rb8
-rw-r--r--app/controllers/users_controller.rb7
-rw-r--r--app/finders/notes_finder.rb2
-rw-r--r--app/finders/snippets_finder.rb102
-rw-r--r--app/models/snippet.rb13
-rw-r--r--app/policies/project_snippet_policy.rb2
-rw-r--r--app/services/search/snippet_service.rb2
-rw-r--r--changelogs/unreleased/snippets-finder-visibility.yml4
-rw-r--r--lib/api/project_snippets.rb3
-rw-r--r--lib/api/snippets.rb4
-rw-r--r--lib/api/v3/project_snippets.rb3
-rw-r--r--lib/api/v3/snippets.rb4
-rw-r--r--spec/controllers/snippets_controller_spec.rb28
-rw-r--r--spec/features/dashboard/snippets_spec.rb47
-rw-r--r--spec/features/projects/snippets_spec.rb24
-rw-r--r--spec/features/snippets/explore_spec.rb25
-rw-r--r--spec/features/users/snippets_spec.rb46
-rw-r--r--spec/finders/snippets_finder_spec.rb125
-rw-r--r--spec/models/snippet_spec.rb40
-rw-r--r--spec/policies/project_snippet_policy_spec.rb80
23 files changed, 399 insertions, 184 deletions
diff --git a/app/controllers/dashboard/snippets_controller.rb b/app/controllers/dashboard/snippets_controller.rb
index bcfdbe14be9..8dd91264451 100644
--- a/app/controllers/dashboard/snippets_controller.rb
+++ b/app/controllers/dashboard/snippets_controller.rb
@@ -1,11 +1,10 @@
class Dashboard::SnippetsController < Dashboard::ApplicationController
def index
- @snippets = SnippetsFinder.new.execute(
+ @snippets = SnippetsFinder.new(
current_user,
- filter: :by_user,
- user: current_user,
+ author: current_user,
scope: params[:scope]
- )
+ ).execute
@snippets = @snippets.page(params[:page])
end
end
diff --git a/app/controllers/explore/snippets_controller.rb b/app/controllers/explore/snippets_controller.rb
index 28760c3f84b..d3f0e033068 100644
--- a/app/controllers/explore/snippets_controller.rb
+++ b/app/controllers/explore/snippets_controller.rb
@@ -1,6 +1,6 @@
class Explore::SnippetsController < Explore::ApplicationController
def index
- @snippets = SnippetsFinder.new.execute(current_user, filter: :all)
+ @snippets = SnippetsFinder.new(current_user).execute
@snippets = @snippets.page(params[:page])
end
end
diff --git a/app/controllers/projects/snippets_controller.rb b/app/controllers/projects/snippets_controller.rb
index 66f913f8f9d..3b2b0d9e502 100644
--- a/app/controllers/projects/snippets_controller.rb
+++ b/app/controllers/projects/snippets_controller.rb
@@ -23,12 +23,11 @@ class Projects::SnippetsController < Projects::ApplicationController
respond_to :html
def index
- @snippets = SnippetsFinder.new.execute(
+ @snippets = SnippetsFinder.new(
current_user,
- filter: :by_project,
project: @project,
scope: params[:scope]
- )
+ ).execute
@snippets = @snippets.page(params[:page])
if @snippets.out_of_range? && @snippets.total_pages != 0
redirect_to namespace_project_snippets_path(page: @snippets.total_pages)
diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb
index 656a365b701..7445f61195d 100644
--- a/app/controllers/snippets_controller.rb
+++ b/app/controllers/snippets_controller.rb
@@ -27,12 +27,8 @@ class SnippetsController < ApplicationController
return render_404 unless @user
- @snippets = SnippetsFinder.new.execute(current_user, {
- filter: :by_user,
- user: @user,
- scope: params[:scope]
- })
- .page(params[:page])
+ @snippets = SnippetsFinder.new(current_user, author: @user, scope: params[:scope])
+ .execute.page(params[:page])
render 'index'
else
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index ca89ed221c6..ba22b2f9d29 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -128,12 +128,11 @@ class UsersController < ApplicationController
end
def load_snippets
- @snippets = SnippetsFinder.new.execute(
+ @snippets = SnippetsFinder.new(
current_user,
- filter: :by_user,
- user: user,
+ author: user,
scope: params[:scope]
- ).page(params[:page])
+ ).execute.page(params[:page])
end
def projects_for_current_user
diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb
index dc6a8ad1f66..02eb983bf55 100644
--- a/app/finders/notes_finder.rb
+++ b/app/finders/notes_finder.rb
@@ -67,7 +67,7 @@ class NotesFinder
when "merge_request"
MergeRequestsFinder.new(@current_user, project_id: @project.id).execute
when "snippet", "project_snippet"
- SnippetsFinder.new.execute(@current_user, filter: :by_project, project: @project)
+ SnippetsFinder.new(@current_user, project: @project).execute
when "personal_snippet"
PersonalSnippet.all
else
diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb
index da6e6e87a6f..c04f61de79c 100644
--- a/app/finders/snippets_finder.rb
+++ b/app/finders/snippets_finder.rb
@@ -1,66 +1,74 @@
-class SnippetsFinder
- def execute(current_user, params = {})
- filter = params[:filter]
- user = params.fetch(:user, current_user)
-
- case filter
- when :all then
- snippets(current_user).fresh
- when :public then
- Snippet.are_public.fresh
- when :by_user then
- by_user(current_user, user, params[:scope])
- when :by_project
- by_project(current_user, params[:project], params[:scope])
- end
+class SnippetsFinder < UnionFinder
+ attr_accessor :current_user, :params
+
+ def initialize(current_user, params = {})
+ @current_user = current_user
+ @params = params
+ end
+
+ def execute
+ items = init_collection
+ items = by_project(items)
+ items = by_author(items)
+ items = by_visibility(items)
+
+ items.fresh
end
private
- def snippets(current_user)
- if current_user
- Snippet.public_and_internal
- else
- # Not authenticated
- #
- # Return only:
- # public snippets
- Snippet.are_public
- end
+ def init_collection
+ items = Snippet.all
+
+ accessible(items)
end
- def by_user(current_user, user, scope)
- snippets = user.snippets.fresh
+ def accessible(items)
+ segments = []
+ segments << items.public_to_user(current_user)
+ segments << authorized_to_user(items) if current_user
- if current_user
- include_private = user == current_user
- by_scope(snippets, scope, include_private)
- else
- snippets.are_public
- end
+ find_union(segments, Snippet)
end
- def by_project(current_user, project, scope)
- snippets = project.snippets.fresh
+ def authorized_to_user(items)
+ items.where(
+ 'author_id = :author_id
+ OR project_id IN (:project_ids)',
+ author_id: current_user.id,
+ project_ids: current_user.authorized_projects.select(:id))
+ end
- if current_user
- include_private = project.team.member?(current_user) || current_user.admin?
- by_scope(snippets, scope, include_private)
- else
- snippets.are_public
- end
+ def by_visibility(items)
+ visibility = params[:visibility] || visibility_from_scope
+
+ return items unless visibility
+
+ items.where(visibility_level: visibility)
+ end
+
+ def by_author(items)
+ return items unless params[:author]
+
+ items.where(author_id: params[:author].id)
+ end
+
+ def by_project(items)
+ return items unless params[:project]
+
+ items.where(project_id: params[:project].id)
end
- def by_scope(snippets, scope = nil, include_private = false)
- case scope.to_s
+ def visibility_from_scope
+ case params[:scope].to_s
when 'are_private'
- include_private ? snippets.are_private : Snippet.none
+ Snippet::PRIVATE
when 'are_internal'
- snippets.are_internal
+ Snippet::INTERNAL
when 'are_public'
- snippets.are_public
+ Snippet::PUBLIC
else
- include_private ? snippets : snippets.public_and_internal
+ nil
end
end
end
diff --git a/app/models/snippet.rb b/app/models/snippet.rb
index abfbefdf9a0..882e2fa0594 100644
--- a/app/models/snippet.rb
+++ b/app/models/snippet.rb
@@ -152,18 +152,5 @@ class Snippet < ActiveRecord::Base
where(table[:content].matches(pattern))
end
-
- def accessible_to(user)
- return are_public unless user.present?
- return all if user.admin?
-
- where(
- 'visibility_level IN (:visibility_levels)
- OR author_id = :author_id
- OR project_id IN (:project_ids)',
- visibility_levels: [Snippet::PUBLIC, Snippet::INTERNAL],
- author_id: user.id,
- project_ids: user.authorized_projects.select(:id))
- end
end
end
diff --git a/app/policies/project_snippet_policy.rb b/app/policies/project_snippet_policy.rb
index 3a96836917e..cf8ff92617f 100644
--- a/app/policies/project_snippet_policy.rb
+++ b/app/policies/project_snippet_policy.rb
@@ -13,7 +13,7 @@ class ProjectSnippetPolicy < BasePolicy
can! :read_project_snippet
end
- if @subject.private? && @subject.project.team.member?(@user)
+ if @subject.project.team.member?(@user)
can! :read_project_snippet
end
end
diff --git a/app/services/search/snippet_service.rb b/app/services/search/snippet_service.rb
index 4f161beea4d..85da0be6fff 100644
--- a/app/services/search/snippet_service.rb
+++ b/app/services/search/snippet_service.rb
@@ -7,7 +7,7 @@ module Search
end
def execute
- snippets = Snippet.accessible_to(current_user)
+ snippets = SnippetsFinder.new(current_user).execute
Gitlab::SnippetSearchResults.new(snippets, params[:search])
end
diff --git a/changelogs/unreleased/snippets-finder-visibility.yml b/changelogs/unreleased/snippets-finder-visibility.yml
new file mode 100644
index 00000000000..fde2262cc8d
--- /dev/null
+++ b/changelogs/unreleased/snippets-finder-visibility.yml
@@ -0,0 +1,4 @@
+---
+title: Refactor snippets finder & dont return internal snippets for external users
+merge_request:
+author:
diff --git a/lib/api/project_snippets.rb b/lib/api/project_snippets.rb
index cfee38a9baf..98bc9c28527 100644
--- a/lib/api/project_snippets.rb
+++ b/lib/api/project_snippets.rb
@@ -17,8 +17,7 @@ module API
end
def snippets_for_current_user
- finder_params = { filter: :by_project, project: user_project }
- SnippetsFinder.new.execute(current_user, finder_params)
+ SnippetsFinder.new(current_user, project: user_project).execute
end
end
diff --git a/lib/api/snippets.rb b/lib/api/snippets.rb
index b93fdc62808..53f5953a8fb 100644
--- a/lib/api/snippets.rb
+++ b/lib/api/snippets.rb
@@ -8,11 +8,11 @@ module API
resource :snippets do
helpers do
def snippets_for_current_user
- SnippetsFinder.new.execute(current_user, filter: :by_user, user: current_user)
+ SnippetsFinder.new(current_user, author: current_user).execute
end
def public_snippets
- SnippetsFinder.new.execute(current_user, filter: :public)
+ SnippetsFinder.new(current_user, visibility: Snippet::PUBLIC).execute
end
end
diff --git a/lib/api/v3/project_snippets.rb b/lib/api/v3/project_snippets.rb
index fc065a22d74..c41fee32610 100644
--- a/lib/api/v3/project_snippets.rb
+++ b/lib/api/v3/project_snippets.rb
@@ -18,8 +18,7 @@ module API
end
def snippets_for_current_user
- finder_params = { filter: :by_project, project: user_project }
- SnippetsFinder.new.execute(current_user, finder_params)
+ SnippetsFinder.new(current_user, project: user_project).execute
end
end
diff --git a/lib/api/v3/snippets.rb b/lib/api/v3/snippets.rb
index 07dac7e9904..0762fc02d70 100644
--- a/lib/api/v3/snippets.rb
+++ b/lib/api/v3/snippets.rb
@@ -8,11 +8,11 @@ module API
resource :snippets do
helpers do
def snippets_for_current_user
- SnippetsFinder.new.execute(current_user, filter: :by_user, user: current_user)
+ SnippetsFinder.new(current_user, author: current_user).execute
end
def public_snippets
- SnippetsFinder.new.execute(current_user, filter: :public)
+ SnippetsFinder.new(current_user, visibility: Snippet::PUBLIC).execute
end
end
diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb
index da46431b700..930415a4778 100644
--- a/spec/controllers/snippets_controller_spec.rb
+++ b/spec/controllers/snippets_controller_spec.rb
@@ -3,6 +3,34 @@ require 'spec_helper'
describe SnippetsController do
let(:user) { create(:user) }
+ describe 'GET #index' do
+ let(:user) { create(:user) }
+
+ context 'when username parameter is present' do
+ it 'renders snippets of a user when username is present' do
+ get :index, username: user.username
+
+ expect(response).to render_template(:index)
+ end
+ end
+
+ context 'when username parameter is not present' do
+ it 'redirects to explore snippets page when user is not logged in' do
+ get :index
+
+ expect(response).to redirect_to(explore_snippets_path)
+ end
+
+ it 'redirects to snippets dashboard page when user is logged in' do
+ sign_in(user)
+
+ get :index
+
+ expect(response).to redirect_to(dashboard_snippets_path)
+ end
+ end
+ end
+
describe 'GET #new' do
context 'when signed in' do
before do
diff --git a/spec/features/dashboard/snippets_spec.rb b/spec/features/dashboard/snippets_spec.rb
index 62937688c22..c6ba118220a 100644
--- a/spec/features/dashboard/snippets_spec.rb
+++ b/spec/features/dashboard/snippets_spec.rb
@@ -12,4 +12,51 @@ describe 'Dashboard snippets', feature: true do
it_behaves_like 'paginated snippets'
end
+
+ context 'filtering by visibility' do
+ let(:user) { create(:user) }
+ let!(:snippets) do
+ [
+ create(:personal_snippet, :public, author: user),
+ create(:personal_snippet, :internal, author: user),
+ create(:personal_snippet, :private, author: user),
+ create(:personal_snippet, :public)
+ ]
+ end
+
+ before do
+ login_as(user)
+
+ visit dashboard_snippets_path
+ end
+
+ it 'contains all snippets of logged user' do
+ expect(page).to have_selector('.snippet-row', count: 3)
+
+ expect(page).to have_content(snippets[0].title)
+ expect(page).to have_content(snippets[1].title)
+ expect(page).to have_content(snippets[2].title)
+ end
+
+ it 'contains all private snippets of logged user when clicking on private' do
+ click_link('Private')
+
+ expect(page).to have_selector('.snippet-row', count: 1)
+ expect(page).to have_content(snippets[2].title)
+ end
+
+ it 'contains all internal snippets of logged user when clicking on internal' do
+ click_link('Internal')
+
+ expect(page).to have_selector('.snippet-row', count: 1)
+ expect(page).to have_content(snippets[1].title)
+ end
+
+ it 'contains all public snippets of logged user when clicking on public' do
+ click_link('Public')
+
+ expect(page).to have_selector('.snippet-row', count: 1)
+ expect(page).to have_content(snippets[0].title)
+ end
+ end
end
diff --git a/spec/features/projects/snippets_spec.rb b/spec/features/projects/snippets_spec.rb
index d37e8ed4699..18689c17fe9 100644
--- a/spec/features/projects/snippets_spec.rb
+++ b/spec/features/projects/snippets_spec.rb
@@ -4,11 +4,27 @@ describe 'Project snippets', feature: true do
context 'when the project has snippets' do
let(:project) { create(:empty_project, :public) }
let!(:snippets) { create_list(:project_snippet, 2, :public, author: project.owner, project: project) }
- before do
- allow(Snippet).to receive(:default_per_page).and_return(1)
- visit namespace_project_snippets_path(project.namespace, project)
+ let!(:other_snippet) { create(:project_snippet) }
+
+ context 'pagination' do
+ before do
+ allow(Snippet).to receive(:default_per_page).and_return(1)
+
+ visit namespace_project_snippets_path(project.namespace, project)
+ end
+
+ it_behaves_like 'paginated snippets'
end
- it_behaves_like 'paginated snippets'
+ context 'list content' do
+ it 'contains all project snippets' do
+ visit namespace_project_snippets_path(project.namespace, project)
+
+ expect(page).to have_selector('.snippet-row', count: 2)
+
+ expect(page).to have_content(snippets[0].title)
+ expect(page).to have_content(snippets[1].title)
+ end
+ end
end
end
diff --git a/spec/features/snippets/explore_spec.rb b/spec/features/snippets/explore_spec.rb
index 10a4597e467..fd097fe2e74 100644
--- a/spec/features/snippets/explore_spec.rb
+++ b/spec/features/snippets/explore_spec.rb
@@ -1,11 +1,11 @@
require 'rails_helper'
feature 'Explore Snippets', feature: true do
- scenario 'User should see snippets that are not private' do
- public_snippet = create(:personal_snippet, :public)
- internal_snippet = create(:personal_snippet, :internal)
- private_snippet = create(:personal_snippet, :private)
+ let!(:public_snippet) { create(:personal_snippet, :public) }
+ let!(:internal_snippet) { create(:personal_snippet, :internal) }
+ let!(:private_snippet) { create(:personal_snippet, :private) }
+ scenario 'User should see snippets that are not private' do
login_as create(:user)
visit explore_snippets_path
@@ -13,4 +13,21 @@ feature 'Explore Snippets', feature: true do
expect(page).to have_content(internal_snippet.title)
expect(page).not_to have_content(private_snippet.title)
end
+
+ scenario 'External user should see only public snippets' do
+ login_as create(:user, :external)
+ visit explore_snippets_path
+
+ expect(page).to have_content(public_snippet.title)
+ expect(page).not_to have_content(internal_snippet.title)
+ expect(page).not_to have_content(private_snippet.title)
+ end
+
+ scenario 'Not authenticated user should see only public snippets' do
+ visit explore_snippets_path
+
+ expect(page).to have_content(public_snippet.title)
+ expect(page).not_to have_content(internal_snippet.title)
+ expect(page).not_to have_content(private_snippet.title)
+ end
end
diff --git a/spec/features/users/snippets_spec.rb b/spec/features/users/snippets_spec.rb
index 1546a06b80c..4efbd672322 100644
--- a/spec/features/users/snippets_spec.rb
+++ b/spec/features/users/snippets_spec.rb
@@ -3,14 +3,46 @@ require 'spec_helper'
describe 'Snippets tab on a user profile', feature: true, js: true do
context 'when the user has snippets' do
let(:user) { create(:user) }
- let!(:snippets) { create_list(:snippet, 2, :public, author: user) }
- before do
- allow(Snippet).to receive(:default_per_page).and_return(1)
- visit user_path(user)
- page.within('.user-profile-nav') { click_link 'Snippets' }
- wait_for_ajax
+
+ context 'pagination' do
+ let!(:snippets) { create_list(:snippet, 2, :public, author: user) }
+
+ before do
+ allow(Snippet).to receive(:default_per_page).and_return(1)
+ visit user_path(user)
+ page.within('.user-profile-nav') { click_link 'Snippets' }
+ wait_for_ajax
+ end
+
+ it_behaves_like 'paginated snippets', remote: true
end
- it_behaves_like 'paginated snippets', remote: true
+ context 'list content' do
+ let!(:public_snippet) { create(:snippet, :public, author: user) }
+ let!(:internal_snippet) { create(:snippet, :internal, author: user) }
+ let!(:private_snippet) { create(:snippet, :private, author: user) }
+ let!(:other_snippet) { create(:snippet, :public) }
+
+ it 'contains only internal and public snippets of a user when a user is logged in' do
+ login_as(:user)
+ visit user_path(user)
+ page.within('.user-profile-nav') { click_link 'Snippets' }
+ wait_for_ajax
+
+ expect(page).to have_selector('.snippet-row', count: 2)
+
+ expect(page).to have_content(public_snippet.title)
+ expect(page).to have_content(internal_snippet.title)
+ end
+
+ it 'contains only public snippets of a user when a user is not logged in' do
+ visit user_path(user)
+ page.within('.user-profile-nav') { click_link 'Snippets' }
+ wait_for_ajax
+
+ expect(page).to have_selector('.snippet-row', count: 1)
+ expect(page).to have_content(public_snippet.title)
+ end
+ end
end
end
diff --git a/spec/finders/snippets_finder_spec.rb b/spec/finders/snippets_finder_spec.rb
index cb6c80d1bd0..9171fb9c4af 100644
--- a/spec/finders/snippets_finder_spec.rb
+++ b/spec/finders/snippets_finder_spec.rb
@@ -8,79 +8,145 @@ describe SnippetsFinder do
let(:project1) { create(:empty_project, :public, group: group) }
let(:project2) { create(:empty_project, :private, group: group) }
- context ':all filter' do
+ 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.execute(user, filter: :all)
- expect(snippets).to include(snippet2, snippet3)
- expect(snippets).not_to include(snippet1)
+ 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.execute(nil, filter: :all)
- expect(snippets).to include(snippet3)
- expect(snippets).not_to include(snippet1, snippet2)
+ 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 = SnippetsFinder.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 = SnippetsFinder.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 = SnippetsFinder.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 ':public filter' do
+ context 'filter by visibility' do
let!(:snippet1) { create(:personal_snippet, :private) }
let!(:snippet2) { create(:personal_snippet, :internal) }
let!(:snippet3) { create(:personal_snippet, :public) }
- it "returns public public snippets" do
- snippets = described_class.new.execute(nil, filter: :public)
+ it "returns public snippets when visibility is PUBLIC" do
+ snippets = SnippetsFinder.new(nil, visibility: Snippet::PUBLIC).execute
expect(snippets).to include(snippet3)
expect(snippets).not_to include(snippet1, snippet2)
end
end
- context ':by_user filter' do
+ context 'filter by scope' do
+ let!(:snippet1) { create(:personal_snippet, :private, author: user) }
+ let!(:snippet2) { create(:personal_snippet, :internal, author: user) }
+ let!(:snippet3) { create(:personal_snippet, :public, author: user) }
+
+ it "returns all snippets for 'all' scope" do
+ snippets = SnippetsFinder.new(user, scope: :all).execute
+
+ expect(snippets).to include(snippet1, snippet2, snippet3)
+ end
+
+ it "returns all snippets for 'are_private' scope" do
+ snippets = SnippetsFinder.new(user, scope: :are_private).execute
+
+ expect(snippets).to include(snippet1)
+ expect(snippets).not_to include(snippet2, snippet3)
+ end
+
+ it "returns all snippets for 'are_interna;' scope" do
+ snippets = SnippetsFinder.new(user, scope: :are_internal).execute
+
+ expect(snippets).to include(snippet2)
+ expect(snippets).not_to include(snippet1, snippet3)
+ end
+
+ it "returns all snippets for 'are_private' scope" do
+ snippets = SnippetsFinder.new(user, scope: :are_public).execute
+
+ expect(snippets).to include(snippet3)
+ expect(snippets).not_to include(snippet1, snippet2)
+ end
+ end
+
+ context 'filter by author' do
let!(:snippet1) { create(:personal_snippet, :private, author: user) }
let!(:snippet2) { create(:personal_snippet, :internal, author: user) }
let!(:snippet3) { create(:personal_snippet, :public, author: user) }
it "returns all public and internal snippets" do
- snippets = described_class.new.execute(user1, filter: :by_user, user: user)
+ snippets = SnippetsFinder.new(user1, author: user).execute
+
expect(snippets).to include(snippet2, snippet3)
expect(snippets).not_to include(snippet1)
end
it "returns internal snippets" do
- snippets = described_class.new.execute(user, filter: :by_user, user: user, scope: "are_internal")
+ snippets = SnippetsFinder.new(user, author: user, visibility: Snippet::INTERNAL).execute
+
expect(snippets).to include(snippet2)
expect(snippets).not_to include(snippet1, snippet3)
end
it "returns private snippets" do
- snippets = described_class.new.execute(user, filter: :by_user, user: user, scope: "are_private")
+ snippets = SnippetsFinder.new(user, author: user, visibility: Snippet::PRIVATE).execute
+
expect(snippets).to include(snippet1)
expect(snippets).not_to include(snippet2, snippet3)
end
it "returns public snippets" do
- snippets = described_class.new.execute(user, filter: :by_user, user: user, scope: "are_public")
+ snippets = SnippetsFinder.new(user, author: user, visibility: Snippet::PUBLIC).execute
+
expect(snippets).to include(snippet3)
expect(snippets).not_to include(snippet1, snippet2)
end
it "returns all snippets" do
- snippets = described_class.new.execute(user, filter: :by_user, user: user)
+ snippets = SnippetsFinder.new(user, author: user).execute
+
expect(snippets).to include(snippet1, snippet2, snippet3)
end
it "returns only public snippets if unauthenticated user" do
- snippets = described_class.new.execute(nil, filter: :by_user, user: user)
+ snippets = SnippetsFinder.new(nil, author: user).execute
+
expect(snippets).to include(snippet3)
expect(snippets).not_to include(snippet2, snippet1)
end
end
- context 'by_project filter' do
+ context 'filter by project' do
before do
@snippet1 = create(:project_snippet, :private, project: project1)
@snippet2 = create(:project_snippet, :internal, project: project1)
@@ -88,43 +154,52 @@ describe SnippetsFinder do
end
it "returns public snippets for unauthorized user" do
- snippets = described_class.new.execute(nil, filter: :by_project, project: project1)
+ snippets = SnippetsFinder.new(nil, project: project1).execute
+
expect(snippets).to include(@snippet3)
expect(snippets).not_to include(@snippet1, @snippet2)
end
it "returns public and internal snippets for non project members" do
- snippets = described_class.new.execute(user, filter: :by_project, project: project1)
+ snippets = SnippetsFinder.new(user, project: project1).execute
+
expect(snippets).to include(@snippet2, @snippet3)
expect(snippets).not_to include(@snippet1)
end
it "returns public snippets for non project members" do
- snippets = described_class.new.execute(user, filter: :by_project, project: project1, scope: "are_public")
+ snippets = SnippetsFinder.new(user, project: project1, visibility: Snippet::PUBLIC).execute
+
expect(snippets).to include(@snippet3)
expect(snippets).not_to include(@snippet1, @snippet2)
end
it "returns internal snippets for non project members" do
- snippets = described_class.new.execute(user, filter: :by_project, project: project1, scope: "are_internal")
+ snippets = SnippetsFinder.new(user, project: project1, visibility: Snippet::INTERNAL).execute
+
expect(snippets).to include(@snippet2)
expect(snippets).not_to include(@snippet1, @snippet3)
end
it "does not return private snippets for non project members" do
- snippets = described_class.new.execute(user, filter: :by_project, project: project1, scope: "are_private")
+ snippets = SnippetsFinder.new(user, project: project1, visibility: Snippet::PRIVATE).execute
+
expect(snippets).not_to include(@snippet1, @snippet2, @snippet3)
end
it "returns all snippets for project members" do
project1.team << [user, :developer]
- snippets = described_class.new.execute(user, filter: :by_project, project: project1)
+
+ snippets = SnippetsFinder.new(user, project: project1).execute
+
expect(snippets).to include(@snippet1, @snippet2, @snippet3)
end
it "returns private snippets for project members" do
project1.team << [user, :developer]
- snippets = described_class.new.execute(user, filter: :by_project, project: project1, scope: "are_private")
+
+ snippets = SnippetsFinder.new(user, project: project1, visibility: Snippet::PRIVATE).execute
+
expect(snippets).to include(@snippet1)
end
end
diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb
index 75b1fc7e216..1e5c96fe593 100644
--- a/spec/models/snippet_spec.rb
+++ b/spec/models/snippet_spec.rb
@@ -131,46 +131,6 @@ describe Snippet, models: true do
end
end
- describe '.accessible_to' do
- let(:author) { create(:author) }
- let(:project) { create(:empty_project) }
-
- let!(:public_snippet) { create(:snippet, :public) }
- let!(:internal_snippet) { create(:snippet, :internal) }
- let!(:private_snippet) { create(:snippet, :private, author: author) }
-
- let!(:project_public_snippet) { create(:snippet, :public, project: project) }
- let!(:project_internal_snippet) { create(:snippet, :internal, project: project) }
- let!(:project_private_snippet) { create(:snippet, :private, project: project) }
-
- it 'returns only public snippets when user is blank' do
- expect(described_class.accessible_to(nil)).to match_array [public_snippet, project_public_snippet]
- end
-
- it 'returns only public, and internal snippets for regular users' do
- user = create(:user)
-
- expect(described_class.accessible_to(user)).to match_array [public_snippet, internal_snippet, project_public_snippet, project_internal_snippet]
- end
-
- it 'returns public, internal snippets and project private snippets for project members' do
- member = create(:user)
- project.team << [member, :developer]
-
- expect(described_class.accessible_to(member)).to match_array [public_snippet, internal_snippet, project_public_snippet, project_internal_snippet, project_private_snippet]
- end
-
- it 'returns private snippets where the user is the author' do
- expect(described_class.accessible_to(author)).to match_array [public_snippet, internal_snippet, private_snippet, project_public_snippet, project_internal_snippet]
- end
-
- it 'returns all snippets when for admins' do
- admin = create(:admin)
-
- expect(described_class.accessible_to(admin)).to match_array [public_snippet, internal_snippet, private_snippet, project_public_snippet, project_internal_snippet, project_private_snippet]
- end
- end
-
describe '#participants' do
let(:project) { create(:empty_project, :public) }
let(:snippet) { create(:snippet, content: 'foo', project: project) }
diff --git a/spec/policies/project_snippet_policy_spec.rb b/spec/policies/project_snippet_policy_spec.rb
index d0758af57dd..e1771b636b8 100644
--- a/spec/policies/project_snippet_policy_spec.rb
+++ b/spec/policies/project_snippet_policy_spec.rb
@@ -1,7 +1,9 @@
require 'spec_helper'
describe ProjectSnippetPolicy, models: true do
- let(:current_user) { create(:user) }
+ let(:regular_user) { create(:user) }
+ let(:external_user) { create(:user, :external) }
+ let(:project) { create(:empty_project) }
let(:author_permissions) do
[
@@ -10,13 +12,15 @@ describe ProjectSnippetPolicy, models: true do
]
end
- subject { described_class.abilities(current_user, project_snippet).to_set }
+ def abilities(user, snippet_visibility)
+ snippet = create(:project_snippet, snippet_visibility, project: project)
- context 'public snippet' do
- let(:project_snippet) { create(:project_snippet, :public) }
+ described_class.abilities(user, snippet).to_set
+ end
+ context 'public snippet' do
context 'no user' do
- let(:current_user) { nil }
+ subject { abilities(nil, :public) }
it do
is_expected.to include(:read_project_snippet)
@@ -25,6 +29,17 @@ describe ProjectSnippetPolicy, models: true do
end
context 'regular user' do
+ subject { abilities(regular_user, :public) }
+
+ it do
+ is_expected.to include(:read_project_snippet)
+ is_expected.not_to include(*author_permissions)
+ end
+ end
+
+ context 'external user' do
+ subject { abilities(external_user, :public) }
+
it do
is_expected.to include(:read_project_snippet)
is_expected.not_to include(*author_permissions)
@@ -33,10 +48,8 @@ describe ProjectSnippetPolicy, models: true do
end
context 'internal snippet' do
- let(:project_snippet) { create(:project_snippet, :internal) }
-
context 'no user' do
- let(:current_user) { nil }
+ subject { abilities(nil, :internal) }
it do
is_expected.not_to include(:read_project_snippet)
@@ -45,6 +58,28 @@ describe ProjectSnippetPolicy, models: true do
end
context 'regular user' do
+ subject { abilities(regular_user, :internal) }
+
+ it do
+ is_expected.to include(:read_project_snippet)
+ is_expected.not_to include(*author_permissions)
+ end
+ end
+
+ context 'external user' do
+ subject { abilities(external_user, :internal) }
+
+ it do
+ is_expected.not_to include(:read_project_snippet)
+ is_expected.not_to include(*author_permissions)
+ end
+ end
+
+ context 'project team member external user' do
+ subject { abilities(external_user, :internal) }
+
+ before { project.team << [external_user, :developer] }
+
it do
is_expected.to include(:read_project_snippet)
is_expected.not_to include(*author_permissions)
@@ -53,10 +88,8 @@ describe ProjectSnippetPolicy, models: true do
end
context 'private snippet' do
- let(:project_snippet) { create(:project_snippet, :private) }
-
context 'no user' do
- let(:current_user) { nil }
+ subject { abilities(nil, :private) }
it do
is_expected.not_to include(:read_project_snippet)
@@ -65,6 +98,8 @@ describe ProjectSnippetPolicy, models: true do
end
context 'regular user' do
+ subject { abilities(regular_user, :private) }
+
it do
is_expected.not_to include(:read_project_snippet)
is_expected.not_to include(*author_permissions)
@@ -72,7 +107,9 @@ describe ProjectSnippetPolicy, models: true do
end
context 'snippet author' do
- let(:project_snippet) { create(:project_snippet, :private, author: current_user) }
+ let(:snippet) { create(:project_snippet, :private, author: regular_user) }
+
+ subject { described_class.abilities(regular_user, snippet).to_set }
it do
is_expected.to include(:read_project_snippet)
@@ -80,8 +117,21 @@ describe ProjectSnippetPolicy, models: true do
end
end
- context 'project team member' do
- before { project_snippet.project.team << [current_user, :developer] }
+ context 'project team member normal user' do
+ subject { abilities(regular_user, :private) }
+
+ before { project.team << [regular_user, :developer] }
+
+ it do
+ is_expected.to include(:read_project_snippet)
+ is_expected.not_to include(*author_permissions)
+ end
+ end
+
+ context 'project team member external user' do
+ subject { abilities(external_user, :private) }
+
+ before { project.team << [external_user, :developer] }
it do
is_expected.to include(:read_project_snippet)
@@ -90,7 +140,7 @@ describe ProjectSnippetPolicy, models: true do
end
context 'admin user' do
- let(:current_user) { create(:admin) }
+ subject { abilities(create(:admin), :private) }
it do
is_expected.to include(:read_project_snippet)