diff options
author | Stan Hu <stanhu@gmail.com> | 2018-02-09 23:42:02 +0300 |
---|---|---|
committer | Ian Baum <ibaum@gitlab.com> | 2018-02-10 00:51:51 +0300 |
commit | 328eb762a9eb82ae59d8e63657af21256e126419 (patch) | |
tree | f2e5bc0a93d9f2cb4852dae6e2cc97e7993f6f9d | |
parent | ef2698915bfac9048343bb78290269c70e7d84dd (diff) |
Merge branch 'rs-pick-security' into 'master'
Pick 10.4.3 fixes into master
See merge request gitlab-org/gitlab-ce!17040
30 files changed, 651 insertions, 130 deletions
diff --git a/app/assets/javascripts/render_mermaid.js b/app/assets/javascripts/render_mermaid.js index 31c7a772cf4..d4f18955bd2 100644 --- a/app/assets/javascripts/render_mermaid.js +++ b/app/assets/javascripts/render_mermaid.js @@ -30,6 +30,9 @@ export default function renderMermaid($els) { $els.each((i, el) => { const source = el.textContent; + // Remove any extra spans added by the backend syntax highlighting. + Object.assign(el, { textContent: source }); + mermaid.init(undefined, el, (id) => { const svg = document.getElementById(id); diff --git a/app/controllers/import/base_controller.rb b/app/controllers/import/base_controller.rb index 9de0297ecfd..c84fc2d305d 100644 --- a/app/controllers/import/base_controller.rb +++ b/app/controllers/import/base_controller.rb @@ -2,26 +2,16 @@ class Import::BaseController < ApplicationController private def find_or_create_namespace(names, owner) - return current_user.namespace if names == owner - return current_user.namespace unless current_user.can_create_group? - names = params[:target_namespace].presence || names - full_path_namespace = Namespace.find_by_full_path(names) - return full_path_namespace if full_path_namespace + return current_user.namespace if names == owner + + group = Groups::NestedCreateService.new(current_user, group_path: names).execute - names.split('/').inject(nil) do |parent, name| - begin - namespace = Group.create!(name: name, - path: name, - owner: current_user, - parent: parent) - namespace.add_owner(current_user) + group.errors.any? ? current_user.namespace : group + rescue => e + Gitlab::AppLogger.error(e) - namespace - rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid - Namespace.where(parent: parent).find_by_path_or_name(name) - end - end + current_user.namespace end end diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb index 4450766485f..33359fa1efb 100644 --- a/app/finders/snippets_finder.rb +++ b/app/finders/snippets_finder.rb @@ -1,14 +1,28 @@ +# 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 - attr_accessor :current_user, :params + include Gitlab::Allowable + attr_accessor :current_user, :params, :project def initialize(current_user, params = {}) @current_user = current_user @params = params + @project = params[:project] end def execute items = init_collection - items = by_project(items) items = by_author(items) items = by_visibility(items) @@ -18,25 +32,42 @@ class SnippetsFinder < UnionFinder private 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 - def accessible(items) - segments = [] - segments << items.public_to_user(current_user) - segments << authorized_to_user(items) if current_user + def authorized_snippets + Snippet.where(feature_available_projects.or(not_project_related)).public_or_visible_to_user(current_user) + end - find_union(segments, Snippet.includes(:author)) + 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 - 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)) + def not_project_related + table[:project_id].eq(nil) + end + + def table + Snippet.arel_table end def by_visibility(items) @@ -53,12 +84,6 @@ class SnippetsFinder < UnionFinder 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 visibility_from_scope case params[:scope].to_s when 'are_private' diff --git a/app/models/project.rb b/app/models/project.rb index 0590cc1c720..3893b1818f3 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1589,8 +1589,11 @@ class Project < ActiveRecord::Base end def protected_for?(ref) - ProtectedBranch.protected?(self, ref) || + if repository.branch_exists?(ref) + ProtectedBranch.protected?(self, ref) + elsif repository.tag_exists?(ref) ProtectedTag.protected?(self, ref) + end end def deployment_variables diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 7c8716f8c18..a58c208279e 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -74,6 +74,27 @@ class Snippet < ActiveRecord::Base @link_reference_pattern ||= super("snippets", /(?<snippet>\d+)/) 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) reference = "#{self.class.reference_prefix}#{id}" diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 1dd8f0a25a9..61a7bf02675 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -119,7 +119,6 @@ class ProjectPolicy < BasePolicy enable :create_note enable :upload_file enable :read_cycle_analytics - enable :read_project_snippet end rule { can?(:reporter_access) }.policy do diff --git a/app/services/groups/nested_create_service.rb b/app/services/groups/nested_create_service.rb index d6f08fc3cce..5c337a9faa5 100644 --- a/app/services/groups/nested_create_service.rb +++ b/app/services/groups/nested_create_service.rb @@ -11,8 +11,8 @@ module Groups def execute return nil unless group_path - if group = Group.find_by_full_path(group_path) - return group + if namespace = namespace_or_group(group_path) + return namespace end if group_path.include?('/') && !Group.supports_nested_groups? @@ -40,10 +40,14 @@ module Groups ) new_params[:visibility_level] ||= Gitlab::CurrentSettings.current_application_settings.default_group_visibility - last_group = Group.find_by_full_path(partial_path) || Groups::CreateService.new(current_user, new_params).execute + last_group = namespace_or_group(partial_path) || Groups::CreateService.new(current_user, new_params).execute end last_group end + + def namespace_or_group(group_path) + Namespace.find_by_full_path(group_path) + end end end diff --git a/lib/api/todos.rb b/lib/api/todos.rb index ffccfebe752..c6dbcf84e3a 100644 --- a/lib/api/todos.rb +++ b/lib/api/todos.rb @@ -60,7 +60,7 @@ module API end post ':id/mark_as_done' do TodoService.new.mark_todos_as_done_by_ids(params[:id], current_user) - todo = Todo.find(params[:id]) + todo = current_user.todos.find(params[:id]) present todo, with: Entities::Todo, current_user: current_user end diff --git a/lib/api/v3/todos.rb b/lib/api/v3/todos.rb index 2f2cf259987..3e2c61f6dbd 100644 --- a/lib/api/v3/todos.rb +++ b/lib/api/v3/todos.rb @@ -12,7 +12,7 @@ module API end delete ':id' do TodoService.new.mark_todos_as_done_by_ids(params[:id], current_user) - todo = Todo.find(params[:id]) + todo = current_user.todos.find(params[:id]) present todo, with: ::API::Entities::Todo, current_user: current_user end diff --git a/lib/banzai/filter/syntax_highlight_filter.rb b/lib/banzai/filter/syntax_highlight_filter.rb index a79a0154846..0ac7e231b5b 100644 --- a/lib/banzai/filter/syntax_highlight_filter.rb +++ b/lib/banzai/filter/syntax_highlight_filter.rb @@ -14,23 +14,33 @@ module Banzai end def highlight_node(node) - code = node.text css_classes = 'code highlight js-syntax-highlight' - language = node.attr('lang') + lang = node.attr('lang') + retried = false - if use_rouge?(language) - lexer = lexer_for(language) + if use_rouge?(lang) + lexer = lexer_for(lang) language = lexer.tag + else + lexer = Rouge::Lexers::PlainText.new + language = lang + end + + begin + code = Rouge::Formatters::HTMLGitlab.format(lex(lexer, node.text), tag: language) + css_classes << " #{language}" if language + rescue + # Gracefully handle syntax highlighter bugs/errors to ensure users can + # still access an issue/comment/etc. First, retry with the plain text + # filter. If that fails, then just skip this entirely, but that would + # be a pretty bad upstream bug. + return if retried - begin - code = Rouge::Formatters::HTMLGitlab.format(lex(lexer, code), tag: language) - css_classes << " #{language}" - rescue - # Gracefully handle syntax highlighter bugs/errors to ensure - # users can still access an issue/comment/etc. + language = nil + lexer = Rouge::Lexers::PlainText.new + retried = true - language = nil - end + retry end highlighted = %(<pre class="#{css_classes}" lang="#{language}" v-pre="true"><code>#{code}</code></pre>) diff --git a/spec/controllers/import/bitbucket_controller_spec.rb b/spec/controllers/import/bitbucket_controller_spec.rb index e8707760a5a..f5e75ff3fdb 100644 --- a/spec/controllers/import/bitbucket_controller_spec.rb +++ b/spec/controllers/import/bitbucket_controller_spec.rb @@ -200,7 +200,7 @@ describe Import::BitbucketController do end end - context 'user has chosen an existing nested namespace and name for the project' do + context 'user has chosen an existing nested namespace and name for the project', :postgresql do let(:parent_namespace) { create(:group, name: 'foo', owner: user) } let(:nested_namespace) { create(:group, name: 'bar', parent: parent_namespace) } let(:test_name) { 'test_name' } @@ -218,7 +218,7 @@ describe Import::BitbucketController do end end - context 'user has chosen a non-existent nested namespaces and name for the project' do + context 'user has chosen a non-existent nested namespaces and name for the project', :postgresql do let(:test_name) { 'test_name' } it 'takes the selected namespace and name' do @@ -249,10 +249,14 @@ describe Import::BitbucketController do end end - context 'user has chosen existent and non-existent nested namespaces and name for the project' do + context 'user has chosen existent and non-existent nested namespaces and name for the project', :postgresql do let(:test_name) { 'test_name' } let!(:parent_namespace) { create(:group, name: 'foo', owner: user) } + before do + parent_namespace.add_owner(user) + end + it 'takes the selected namespace and name' do expect(Gitlab::BitbucketImport::ProjectCreator) .to receive(:new).with(bitbucket_repo, test_name, kind_of(Namespace), user, access_params) diff --git a/spec/controllers/import/gitlab_controller_spec.rb b/spec/controllers/import/gitlab_controller_spec.rb index faf1e6f63ea..3bfb34d8d2a 100644 --- a/spec/controllers/import/gitlab_controller_spec.rb +++ b/spec/controllers/import/gitlab_controller_spec.rb @@ -174,7 +174,7 @@ describe Import::GitlabController do end end - context 'user has chosen an existing nested namespace for the project' do + context 'user has chosen an existing nested namespace for the project', :postgresql do let(:parent_namespace) { create(:group, name: 'foo', owner: user) } let(:nested_namespace) { create(:group, name: 'bar', parent: parent_namespace) } @@ -191,7 +191,7 @@ describe Import::GitlabController do end end - context 'user has chosen a non-existent nested namespaces for the project' do + context 'user has chosen a non-existent nested namespaces for the project', :postgresql do let(:test_name) { 'test_name' } it 'takes the selected namespace and name' do @@ -222,10 +222,14 @@ describe Import::GitlabController do end end - context 'user has chosen existent and non-existent nested namespaces and name for the project' do + context 'user has chosen existent and non-existent nested namespaces and name for the project', :postgresql do let(:test_name) { 'test_name' } let!(:parent_namespace) { create(:group, name: 'foo', owner: user) } + before do + parent_namespace.add_owner(user) + end + it 'takes the selected namespace and name' do expect(Gitlab::GitlabImport::ProjectCreator) .to receive(:new).with(gitlab_repo, kind_of(Namespace), user, access_params) diff --git a/spec/features/copy_as_gfm_spec.rb b/spec/features/markdown/copy_as_gfm_spec.rb index f82ed6300cc..f82ed6300cc 100644 --- a/spec/features/copy_as_gfm_spec.rb +++ b/spec/features/markdown/copy_as_gfm_spec.rb diff --git a/spec/features/gitlab_flavored_markdown_spec.rb b/spec/features/markdown/gitlab_flavored_markdown_spec.rb index 3c2186b3598..3c2186b3598 100644 --- a/spec/features/gitlab_flavored_markdown_spec.rb +++ b/spec/features/markdown/gitlab_flavored_markdown_spec.rb diff --git a/spec/features/markdown_spec.rb b/spec/features/markdown/markdown_spec.rb index f13d78d24e3..f13d78d24e3 100644 --- a/spec/features/markdown_spec.rb +++ b/spec/features/markdown/markdown_spec.rb diff --git a/spec/features/markdown/math_spec.rb b/spec/features/markdown/math_spec.rb new file mode 100644 index 00000000000..6a23d6b78ab --- /dev/null +++ b/spec/features/markdown/math_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe 'Math rendering', :js do + it 'renders inline and display math correctly' do + description = <<~MATH + This math is inline $`a^2+b^2=c^2`$. + + This is on a separate line + ```math + a^2+b^2=c^2 + ``` + MATH + + project = create(:project, :public) + issue = create(:issue, project: project, description: description) + + visit project_issue_path(project, issue) + + expect(page).to have_selector('.katex .mord.mathit', text: 'b') + expect(page).to have_selector('.katex-display .mord.mathit', text: 'b') + end +end diff --git a/spec/features/markdown/mermaid_spec.rb b/spec/features/markdown/mermaid_spec.rb new file mode 100644 index 00000000000..a25d701ee35 --- /dev/null +++ b/spec/features/markdown/mermaid_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +describe 'Mermaid rendering', :js do + it 'renders Mermaid diagrams correctly' do + description = <<~MERMAID + ```mermaid + graph TD; + A-->B; + A-->C; + B-->D; + C-->D; + ``` + MERMAID + + project = create(:project, :public) + issue = create(:issue, project: project, description: description) + + visit project_issue_path(project, issue) + + %w[A B C D].each do |label| + expect(page).to have_selector('svg foreignObject', text: label) + end + end +end diff --git a/spec/finders/snippets_finder_spec.rb b/spec/finders/snippets_finder_spec.rb index 0a018d2b417..54a07eccaba 100644 --- a/spec/finders/snippets_finder_spec.rb +++ b/spec/finders/snippets_finder_spec.rb @@ -1,57 +1,8 @@ require 'spec_helper' describe SnippetsFinder do - let(:user) { create :user } - let(:user1) { create :user } - 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 + include Gitlab::Allowable + using RSpec::Parameterized::TableSyntax context 'filter by visibility' do let!(:snippet1) { create(:personal_snippet, :private) } @@ -67,6 +18,7 @@ describe SnippetsFinder do end context 'filter by scope' do + let(:user) { create :user } let!(:snippet1) { create(:personal_snippet, :private, author: user) } let!(:snippet2) { create(:personal_snippet, :internal, author: user) } let!(:snippet3) { create(:personal_snippet, :public, author: user) } @@ -84,7 +36,7 @@ describe SnippetsFinder do expect(snippets).not_to include(snippet2, snippet3) 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 expect(snippets).to include(snippet2) @@ -100,6 +52,8 @@ describe SnippetsFinder do end context 'filter by author' do + let(:user) { create :user } + let(:user1) { create :user } let!(:snippet1) { create(:personal_snippet, :private, author: user) } let!(:snippet2) { create(:personal_snippet, :internal, author: user) } let!(:snippet3) { create(:personal_snippet, :public, author: user) } @@ -147,6 +101,10 @@ describe SnippetsFinder do end context 'filter by project' do + let(:user) { create :user } + let(:group) { create :group, :public } + let(:project1) { create(:project, :public, group: group) } + before do @snippet1 = create(:project_snippet, :private, project: project1) @snippet2 = create(:project_snippet, :internal, project: project1) @@ -203,4 +161,9 @@ describe SnippetsFinder do expect(snippets).to include(@snippet1) 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 diff --git a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb index 9f2efa05a01..ef52c572898 100644 --- a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb +++ b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb @@ -3,35 +3,86 @@ require 'spec_helper' describe Banzai::Filter::SyntaxHighlightFilter do include FilterSpecHelper + shared_examples "XSS prevention" do |lang| + it "escapes HTML tags" do + # This is how a script tag inside a code block is presented to this filter + # after Markdown rendering. + result = filter(%{<pre lang="#{lang}"><code><script>alert(1)</script></code></pre>}) + + expect(result.to_html).not_to include("<script>alert(1)</script>") + expect(result.to_html).to include("alert(1)") + end + end + context "when no language is specified" do it "highlights as plaintext" do result = filter('<pre><code>def fun end</code></pre>') + expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight plaintext" lang="plaintext" v-pre="true"><code><span id="LC1" class="line" lang="plaintext">def fun end</span></code></pre>') end + + include_examples "XSS prevention", "" end context "when a valid language is specified" do it "highlights as that language" do result = filter('<pre><code lang="ruby">def fun end</code></pre>') + expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight ruby" lang="ruby" v-pre="true"><code><span id="LC1" class="line" lang="ruby"><span class="k">def</span> <span class="nf">fun</span> <span class="k">end</span></span></code></pre>') end + + include_examples "XSS prevention", "ruby" end context "when an invalid language is specified" do it "highlights as plaintext" do result = filter('<pre><code lang="gnuplot">This is a test</code></pre>') + expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight plaintext" lang="plaintext" v-pre="true"><code><span id="LC1" class="line" lang="plaintext">This is a test</span></code></pre>') end + + include_examples "XSS prevention", "gnuplot" end - context "when Rouge formatting fails" do + context "languages that should be passed through" do + %w(math mermaid plantuml).each do |lang| + context "when #{lang} is specified" do + it "highlights as plaintext but with the correct language attribute and class" do + result = filter(%{<pre><code lang="#{lang}">This is a test</code></pre>}) + + expect(result.to_html).to eq(%{<pre class="code highlight js-syntax-highlight #{lang}" lang="#{lang}" v-pre="true"><code><span id="LC1" class="line" lang="#{lang}">This is a test</span></code></pre>}) + end + + include_examples "XSS prevention", lang + end + end + end + + context "when Rouge lexing fails" do before do - allow_any_instance_of(Rouge::Formatter).to receive(:format).and_raise(StandardError) + allow_any_instance_of(Rouge::Lexers::Ruby).to receive(:stream_tokens).and_raise(StandardError) end it "highlights as plaintext" do result = filter('<pre><code lang="ruby">This is a test</code></pre>') - expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight" lang="" v-pre="true"><code>This is a test</code></pre>') + + expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight" lang="" v-pre="true"><code><span id="LC1" class="line" lang="">This is a test</span></code></pre>') + end + + include_examples "XSS prevention", "ruby" + end + + context "when Rouge lexing fails after a retry" do + before do + allow_any_instance_of(Rouge::Lexers::PlainText).to receive(:stream_tokens).and_raise(StandardError) + end + + it "does not add highlighting classes" do + result = filter('<pre><code>This is a test</code></pre>') + + expect(result.to_html).to eq('<pre><code>This is a test</code></pre>') end + + include_examples "XSS prevention", "ruby" end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 0b3d5c6a0bd..3e8b9ea472a 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1589,7 +1589,7 @@ describe Ci::Build do context 'when the branch is protected' do before do - create(:protected_branch, project: build.project, name: build.ref) + allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true) end it { is_expected.to include(protected_variable) } @@ -1597,7 +1597,7 @@ describe Ci::Build do context 'when the tag is protected' do before do - create(:protected_tag, project: build.project, name: build.ref) + allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true) end it { is_expected.to include(protected_variable) } @@ -1634,7 +1634,7 @@ describe Ci::Build do context 'when the branch is protected' do before do - create(:protected_branch, project: build.project, name: build.ref) + allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true) end it { is_expected.to include(protected_variable) } @@ -1642,7 +1642,7 @@ describe Ci::Build do context 'when the tag is protected' do before do - create(:protected_tag, project: build.project, name: build.ref) + allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true) end it { is_expected.to include(protected_variable) } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 338fb314ee9..4f16b73ef38 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -549,7 +549,7 @@ describe Group do context 'when the ref is a protected branch' do before do - create(:protected_branch, name: 'ref', project: project) + allow(project).to receive(:protected_for?).with('ref').and_return(true) end it_behaves_like 'ref is protected' @@ -557,7 +557,7 @@ describe Group do context 'when the ref is a protected tag' do before do - create(:protected_tag, name: 'ref', project: project) + allow(project).to receive(:protected_for?).with('ref').and_return(true) end it_behaves_like 'ref is protected' @@ -571,6 +571,10 @@ describe Group do let(:variable_child_2) { create(:ci_group_variable, group: group_child_2) } let(:variable_child_3) { create(:ci_group_variable, group: group_child_3) } + before do + allow(project).to receive(:protected_for?).with('ref').and_return(true) + end + it 'returns all variables belong to the group and parent groups' do expected_array1 = [protected_variable, secret_variable] expected_array2 = [variable_child, variable_child_2, variable_child_3] diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index c6ca038a2ba..7f54192954e 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2092,7 +2092,7 @@ describe Project do context 'when the ref is a protected branch' do before do - create(:protected_branch, name: 'ref', project: project) + allow(project).to receive(:protected_for?).with('ref').and_return(true) end it_behaves_like 'ref is protected' @@ -2100,7 +2100,7 @@ describe Project do context 'when the ref is a protected tag' do before do - create(:protected_tag, name: 'ref', project: project) + allow(project).to receive(:protected_for?).with('ref').and_return(true) end it_behaves_like 'ref is protected' @@ -2125,6 +2125,8 @@ describe Project do context 'when the ref is a protected branch' do before do + allow(project).to receive(:repository).and_call_original + allow(project).to receive_message_chain(:repository, :branch_exists?).and_return(true) create(:protected_branch, name: 'ref', project: project) end @@ -2135,6 +2137,8 @@ describe Project do context 'when the ref is a protected tag' do before do + allow(project).to receive_message_chain(:repository, :branch_exists?).and_return(false) + allow(project).to receive_message_chain(:repository, :tag_exists?).and_return(true) create(:protected_tag, name: 'ref', project: project) end diff --git a/spec/policies/personal_snippet_policy_spec.rb b/spec/policies/personal_snippet_policy_spec.rb index b70c8646a3d..50bb0899eba 100644 --- a/spec/policies/personal_snippet_policy_spec.rb +++ b/spec/policies/personal_snippet_policy_spec.rb @@ -1,5 +1,6 @@ require 'spec_helper' +# Snippet visibility scenarios are included in more details in spec/support/snippet_visibility.rb describe PersonalSnippetPolicy do let(:regular_user) { create(:user) } let(:external_user) { create(:user, :external) } diff --git a/spec/policies/project_snippet_policy_spec.rb b/spec/policies/project_snippet_policy_spec.rb index cdba1b09fc1..4d32e06b553 100644 --- a/spec/policies/project_snippet_policy_spec.rb +++ b/spec/policies/project_snippet_policy_spec.rb @@ -1,5 +1,6 @@ require 'spec_helper' +# Snippet visibility scenarios are included in more details in spec/support/snippet_visibility.rb describe ProjectSnippetPolicy do let(:regular_user) { create(:user) } let(:external_user) { create(:user, :external) } diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index 74198c8eb4f..b3e253befc6 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -32,6 +32,27 @@ describe API::Snippets do expect(json_response).to be_an Array expect(json_response.size).to eq(0) 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 describe 'GET /snippets/public' do diff --git a/spec/requests/api/todos_spec.rb b/spec/requests/api/todos_spec.rb index fb3a33cadff..2ee8d150dc8 100644 --- a/spec/requests/api/todos_spec.rb +++ b/spec/requests/api/todos_spec.rb @@ -129,6 +129,12 @@ describe API::Todos do post api("/todos/#{pending_1.id}/mark_as_done", john_doe) end + + it 'returns 404 if the todo does not belong to the current user' do + post api("/todos/#{pending_1.id}/mark_as_done", author_1) + + expect(response.status).to eq(404) + end end end diff --git a/spec/requests/api/v3/todos_spec.rb b/spec/requests/api/v3/todos_spec.rb index 53fd962272a..ea648e3917f 100644 --- a/spec/requests/api/v3/todos_spec.rb +++ b/spec/requests/api/v3/todos_spec.rb @@ -38,6 +38,12 @@ describe API::V3::Todos do delete v3_api("/todos/#{pending_1.id}", john_doe) end + + it 'returns 404 if the todo does not belong to the current user' do + delete v3_api("/todos/#{pending_1.id}", author_1) + + expect(response.status).to eq(404) + end end end diff --git a/spec/services/search/snippet_service_spec.rb b/spec/services/search/snippet_service_spec.rb index bc7885b03d9..8ad162ad66e 100644 --- a/spec/services/search/snippet_service_spec.rb +++ b/spec/services/search/snippet_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Search::SnippetService do let(:author) { create(:author) } - let(:project) { create(:project) } + let(:project) { create(:project, :public) } let!(:public_snippet) { create(:snippet, :public, content: 'password: XXX') } let!(:internal_snippet) { create(:snippet, :internal, content: 'password: XXX') } diff --git a/spec/support/controllers/githubish_import_controller_shared_examples.rb b/spec/support/controllers/githubish_import_controller_shared_examples.rb index a0839eefe6c..bf2c52b927f 100644 --- a/spec/support/controllers/githubish_import_controller_shared_examples.rb +++ b/spec/support/controllers/githubish_import_controller_shared_examples.rb @@ -235,7 +235,7 @@ shared_examples 'a GitHub-ish import controller: POST create' do end end - context 'user has chosen an existing nested namespace and name for the project' do + context 'user has chosen an existing nested namespace and name for the project', :postgresql do let(:parent_namespace) { create(:group, name: 'foo', owner: user) } let(:nested_namespace) { create(:group, name: 'bar', parent: parent_namespace) } let(:test_name) { 'test_name' } @@ -253,7 +253,7 @@ shared_examples 'a GitHub-ish import controller: POST create' do end end - context 'user has chosen a non-existent nested namespaces and name for the project' do + context 'user has chosen a non-existent nested namespaces and name for the project', :postgresql do let(:test_name) { 'test_name' } it 'takes the selected namespace and name' do @@ -284,10 +284,14 @@ shared_examples 'a GitHub-ish import controller: POST create' do end end - context 'user has chosen existent and non-existent nested namespaces and name for the project' do + context 'user has chosen existent and non-existent nested namespaces and name for the project', :postgresql do let(:test_name) { 'test_name' } let!(:parent_namespace) { create(:group, name: 'foo', owner: user) } + before do + parent_namespace.add_owner(user) + end + it 'takes the selected namespace and name' do expect(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, test_name, kind_of(Namespace), user, access_params, type: provider) @@ -304,6 +308,53 @@ shared_examples 'a GitHub-ish import controller: POST create' do expect { post :create, { target_namespace: 'foo/foobar/bar', new_name: test_name, format: :js } } .to change { Namespace.count }.by(2) end + + it 'does not create a new namespace under the user namespace' do + expect(Gitlab::LegacyGithubImport::ProjectCreator) + .to receive(:new).with(provider_repo, test_name, user.namespace, user, access_params, type: provider) + .and_return(double(execute: build_stubbed(:project))) + + expect { post :create, { target_namespace: "#{user.namespace_path}/test_group", new_name: test_name, format: :js } } + .not_to change { Namespace.count } + end + end + + context 'user cannot create a subgroup inside a group is not a member of' do + let(:test_name) { 'test_name' } + let!(:parent_namespace) { create(:group, name: 'foo') } + + it 'does not take the selected namespace and name' do + expect(Gitlab::LegacyGithubImport::ProjectCreator) + .to receive(:new).with(provider_repo, test_name, user.namespace, user, access_params, type: provider) + .and_return(double(execute: build_stubbed(:project))) + + post :create, { target_namespace: 'foo/foobar/bar', new_name: test_name, format: :js } + end + + it 'does not create the namespaces' do + allow(Gitlab::LegacyGithubImport::ProjectCreator) + .to receive(:new).with(provider_repo, test_name, kind_of(Namespace), user, access_params, type: provider) + .and_return(double(execute: build_stubbed(:project))) + + expect { post :create, { target_namespace: 'foo/foobar/bar', new_name: test_name, format: :js } } + .not_to change { Namespace.count } + end + end + + context 'user can use a group without having permissions to create a group' do + let(:test_name) { 'test_name' } + let!(:group) { create(:group, name: 'foo') } + + it 'takes the selected namespace and name' do + group.add_owner(user) + user.update!(can_create_group: false) + + expect(Gitlab::LegacyGithubImport::ProjectCreator) + .to receive(:new).with(provider_repo, test_name, group, user, access_params, type: provider) + .and_return(double(execute: build_stubbed(:project))) + + post :create, { target_namespace: 'foo', new_name: test_name, format: :js } + end end end end diff --git a/spec/support/snippet_visibility.rb b/spec/support/snippet_visibility.rb new file mode 100644 index 00000000000..1cb904823d2 --- /dev/null +++ b/spec/support/snippet_visibility.rb @@ -0,0 +1,304 @@ +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 |