From f0866b28d384f569345eccb6e64ad93d29a3e570 Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Thu, 25 Aug 2016 20:43:24 -0500 Subject: Fix repo title alignment --- CHANGELOG | 1 + app/assets/stylesheets/framework/header.scss | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index be1063a2330..36c757021d2 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -15,6 +15,7 @@ v 8.12.0 (unreleased) - Added tests for diff notes - Add delimiter to project stars and forks count (ClemMakesApps) - Fix badge count alignment (ClemMakesApps) + - Fix repo title alignment (ClemMakesApps) - Fix branch title trailing space on hover (ClemMakesApps) - Award emoji tooltips containing more than 10 usernames are now truncated !4780 (jlogandavison) - Fix duplicate "me" in award emoji tooltip !5218 (jlogandavison) diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss index 0c607071840..41ffaa38f6c 100644 --- a/app/assets/stylesheets/framework/header.scss +++ b/app/assets/stylesheets/framework/header.scss @@ -94,7 +94,7 @@ header { .side-nav-toggle { position: absolute; left: -10px; - margin: 6px 0; + margin: 7px 0; font-size: 18px; padding: 6px 10px; border: none; -- cgit v1.2.1 From 0fe4cf2b0fdbc33572f11bba1a4426ee05ed7599 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Wed, 24 Aug 2016 20:06:16 -0700 Subject: Fix Sentry not reporting right program for Sidekiq workers Moves program tag into the global configuration since this doesn't change and since Sidekiq workers get a unique context for each event. Closes #21410 --- app/helpers/sentry_helper.rb | 22 ++-------------------- config/initializers/sentry.rb | 1 + lib/gitlab/sentry.rb | 27 +++++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 20 deletions(-) create mode 100644 lib/gitlab/sentry.rb diff --git a/app/helpers/sentry_helper.rb b/app/helpers/sentry_helper.rb index f8cccade15b..3d255df66a0 100644 --- a/app/helpers/sentry_helper.rb +++ b/app/helpers/sentry_helper.rb @@ -1,27 +1,9 @@ module SentryHelper def sentry_enabled? - Rails.env.production? && current_application_settings.sentry_enabled? + Gitlab::Sentry.enabled? end def sentry_context - return unless sentry_enabled? - - if current_user - Raven.user_context( - id: current_user.id, - email: current_user.email, - username: current_user.username, - ) - end - - Raven.tags_context(program: sentry_program_context) - end - - def sentry_program_context - if Sidekiq.server? - 'sidekiq' - else - 'rails' - end + Gitlab::Sentry.context(current_user) end end diff --git a/config/initializers/sentry.rb b/config/initializers/sentry.rb index 74fef7cadfe..5892c1de024 100644 --- a/config/initializers/sentry.rb +++ b/config/initializers/sentry.rb @@ -18,6 +18,7 @@ if Rails.env.production? # Sanitize fields based on those sanitized from Rails. config.sanitize_fields = Rails.application.config.filter_parameters.map(&:to_s) + config.tags = { program: Gitlab::Sentry.program_context } end end end diff --git a/lib/gitlab/sentry.rb b/lib/gitlab/sentry.rb new file mode 100644 index 00000000000..117fc508135 --- /dev/null +++ b/lib/gitlab/sentry.rb @@ -0,0 +1,27 @@ +module Gitlab + module Sentry + def self.enabled? + Rails.env.production? && current_application_settings.sentry_enabled? + end + + def self.context(current_user = nil) + return unless self.enabled? + + if current_user + Raven.user_context( + id: current_user.id, + email: current_user.email, + username: current_user.username, + ) + end + end + + def self.program_context + if Sidekiq.server? + 'sidekiq' + else + 'rails' + end + end + end +end -- cgit v1.2.1 From bfd14f876388f6c9a4006ae270b8a98912415226 Mon Sep 17 00:00:00 2001 From: winniehell Date: Fri, 26 Aug 2016 10:54:17 +0200 Subject: Check for existence of elements under test in application_spec.js (!6051) --- spec/javascripts/application_spec.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/spec/javascripts/application_spec.js b/spec/javascripts/application_spec.js index b48026c3b77..56b98856614 100644 --- a/spec/javascripts/application_spec.js +++ b/spec/javascripts/application_spec.js @@ -13,17 +13,21 @@ gl.utils.preventDisabledButtons(); isClicked = false; $button = $('#test-button'); + expect($button).toExist(); $button.click(function() { return isClicked = true; }); $button.trigger('click'); return expect(isClicked).toBe(false); }); - return it('should be on the same page if a disabled link clicked', function() { - var locationBeforeLinkClick; + + it('should be on the same page if a disabled link clicked', function() { + var locationBeforeLinkClick, $link; locationBeforeLinkClick = window.location.href; gl.utils.preventDisabledButtons(); - $('#test-link').click(); + $link = $('#test-link'); + expect($link).toExist(); + $link.click(); return expect(window.location.href).toBe(locationBeforeLinkClick); }); }); -- cgit v1.2.1 From 32551faeba232b64cb6b5cbf01eff341abe39174 Mon Sep 17 00:00:00 2001 From: Fatih Acet Date: Tue, 30 Aug 2016 08:02:29 +0300 Subject: Minor code refactor for inlining functions. --- app/assets/javascripts/logo.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/logo.js b/app/assets/javascripts/logo.js index e5d4fd44c96..7d8eef1b495 100644 --- a/app/assets/javascripts/logo.js +++ b/app/assets/javascripts/logo.js @@ -1,16 +1,12 @@ (function() { Turbolinks.enableProgressBar(); - start = function() { + $(document).on('page:fetch', function() { $('.tanuki-logo').addClass('animate'); - }; + }); - stop = function() { + $(document).on('page:change', function() { $('.tanuki-logo').removeClass('animate'); - }; - - $(document).on('page:fetch', start); - - $(document).on('page:change', stop); + }); }).call(this); -- cgit v1.2.1 From c1b44cfea4fcba1acbb798d934daddcd8de50354 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 8 Jul 2016 14:01:21 +0100 Subject: Hide group control nav if no options present Closes #19120 --- app/views/layouts/nav/_group.html.haml | 2 +- app/views/layouts/nav/_group_settings.html.haml | 38 ++++++++++++++----------- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/app/views/layouts/nav/_group.html.haml b/app/views/layouts/nav/_group.html.haml index d7d36c84b6c..27ac1760166 100644 --- a/app/views/layouts/nav/_group.html.haml +++ b/app/views/layouts/nav/_group.html.haml @@ -1,5 +1,5 @@ += render 'layouts/nav/group_settings' .scrolling-tabs-container{ class: nav_control_class } - = render 'layouts/nav/group_settings' .fade-left = icon('angle-left') .fade-right diff --git a/app/views/layouts/nav/_group_settings.html.haml b/app/views/layouts/nav/_group_settings.html.haml index bf9a7ecb786..1076ac9270a 100644 --- a/app/views/layouts/nav/_group_settings.html.haml +++ b/app/views/layouts/nav/_group_settings.html.haml @@ -1,22 +1,26 @@ - if current_user + - can_admin_projects = can?(current_user, :admin_group, @group) - can_edit = can?(current_user, :admin_group, @group) - member = @group.members.find_by(user_id: current_user.id) - can_leave = member && can?(current_user, :destroy_group_member, member) - .controls - .dropdown.group-settings-dropdown - %a.dropdown-new.btn.btn-default#group-settings-button{href: '#', 'data-toggle' => 'dropdown'} - = icon('cog') - = icon('caret-down') - %ul.dropdown-menu.dropdown-menu-align-right - = nav_link(path: 'groups#projects') do - = link_to 'Projects', projects_group_path(@group), title: 'Projects' - %li.divider - - if can_edit - %li - = link_to 'Edit Group', edit_group_path(@group) - - if can_leave - %li - = link_to polymorphic_path([:leave, @group, :members]), - data: { confirm: leave_confirmation_message(@group) }, method: :delete, title: 'Leave group' do - Leave Group + - if can_admin_projects || can_edit || can_leave + .controls + .dropdown.group-settings-dropdown + %a.dropdown-new.btn.btn-default#group-settings-button{href: '#', 'data-toggle' => 'dropdown'} + = icon('cog') + = icon('caret-down') + %ul.dropdown-menu.dropdown-menu-align-right + - if can_admin_projects + = nav_link(path: 'groups#projects') do + = link_to 'Projects', projects_group_path(@group), title: 'Projects' + - if can_edit || can_leave + %li.divider + - if can_edit + %li + = link_to 'Edit Group', edit_group_path(@group) + - if can_leave + %li + = link_to polymorphic_path([:leave, @group, :members]), + data: { confirm: leave_confirmation_message(@group) }, method: :delete, title: 'Leave group' do + Leave Group -- cgit v1.2.1 From 45421e1da8953d5faca96f277a142576559f3109 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 30 Aug 2016 12:19:46 +0100 Subject: Updated variable name --- app/views/layouts/nav/_group_settings.html.haml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/layouts/nav/_group_settings.html.haml b/app/views/layouts/nav/_group_settings.html.haml index 1076ac9270a..75275afc0f3 100644 --- a/app/views/layouts/nav/_group_settings.html.haml +++ b/app/views/layouts/nav/_group_settings.html.haml @@ -1,17 +1,17 @@ - if current_user - - can_admin_projects = can?(current_user, :admin_group, @group) + - can_admin_group = can?(current_user, :admin_group, @group) - can_edit = can?(current_user, :admin_group, @group) - member = @group.members.find_by(user_id: current_user.id) - can_leave = member && can?(current_user, :destroy_group_member, member) - - if can_admin_projects || can_edit || can_leave + - if can_admin_group || can_edit || can_leave .controls .dropdown.group-settings-dropdown %a.dropdown-new.btn.btn-default#group-settings-button{href: '#', 'data-toggle' => 'dropdown'} = icon('cog') = icon('caret-down') %ul.dropdown-menu.dropdown-menu-align-right - - if can_admin_projects + - if can_admin_group = nav_link(path: 'groups#projects') do = link_to 'Projects', projects_group_path(@group), title: 'Projects' - if can_edit || can_leave -- cgit v1.2.1 From 7d119bab4816355ce0156f24e0117e6e1d81588f Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 9 Aug 2016 13:56:54 -0700 Subject: re-enable the cyclomatic complexity checker --- app/models/ability.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index c1df4a865f6..fcd7740d79f 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -1,6 +1,5 @@ class Ability class << self - # rubocop: disable Metrics/CyclomaticComplexity def allowed(user, subject) return anonymous_abilities(user, subject) if user.nil? return [] unless user.is_a?(User) -- cgit v1.2.1 From 0f4df86a5e559b9c15f07b43edad829928f59e87 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 23 Aug 2016 17:45:49 -0700 Subject: delete project_security_spec re a conversation with @rspeicher, this spec isn't really testing anything. --- spec/models/project_security_spec.rb | 112 ----------------------------------- 1 file changed, 112 deletions(-) delete mode 100644 spec/models/project_security_spec.rb diff --git a/spec/models/project_security_spec.rb b/spec/models/project_security_spec.rb deleted file mode 100644 index 36379074ea0..00000000000 --- a/spec/models/project_security_spec.rb +++ /dev/null @@ -1,112 +0,0 @@ -require 'spec_helper' - -describe Project, models: true do - describe 'authorization' do - before do - @p1 = create(:project) - - @u1 = create(:user) - @u2 = create(:user) - @u3 = create(:user) - @u4 = @p1.owner - - @abilities = Six.new - @abilities << Ability - end - - let(:guest_actions) { Ability.project_guest_rules } - let(:report_actions) { Ability.project_report_rules } - let(:dev_actions) { Ability.project_dev_rules } - let(:master_actions) { Ability.project_master_rules } - let(:owner_actions) { Ability.project_owner_rules } - - describe "Non member rules" do - it "denies for non-project users any actions" do - owner_actions.each do |action| - expect(@abilities.allowed?(@u1, action, @p1)).to be_falsey - end - end - end - - describe "Guest Rules" do - before do - @p1.project_members.create(project: @p1, user: @u2, access_level: ProjectMember::GUEST) - end - - it "allows for project user any guest actions" do - guest_actions.each do |action| - expect(@abilities.allowed?(@u2, action, @p1)).to be_truthy - end - end - end - - describe "Report Rules" do - before do - @p1.project_members.create(project: @p1, user: @u2, access_level: ProjectMember::REPORTER) - end - - it "allows for project user any report actions" do - report_actions.each do |action| - expect(@abilities.allowed?(@u2, action, @p1)).to be_truthy - end - end - end - - describe "Developer Rules" do - before do - @p1.project_members.create(project: @p1, user: @u2, access_level: ProjectMember::REPORTER) - @p1.project_members.create(project: @p1, user: @u3, access_level: ProjectMember::DEVELOPER) - end - - it "denies for developer master-specific actions" do - [dev_actions - report_actions].each do |action| - expect(@abilities.allowed?(@u2, action, @p1)).to be_falsey - end - end - - it "allows for project user any dev actions" do - dev_actions.each do |action| - expect(@abilities.allowed?(@u3, action, @p1)).to be_truthy - end - end - end - - describe "Master Rules" do - before do - @p1.project_members.create(project: @p1, user: @u2, access_level: ProjectMember::DEVELOPER) - @p1.project_members.create(project: @p1, user: @u3, access_level: ProjectMember::MASTER) - end - - it "denies for developer master-specific actions" do - [master_actions - dev_actions].each do |action| - expect(@abilities.allowed?(@u2, action, @p1)).to be_falsey - end - end - - it "allows for project user any master actions" do - master_actions.each do |action| - expect(@abilities.allowed?(@u3, action, @p1)).to be_truthy - end - end - end - - describe "Owner Rules" do - before do - @p1.project_members.create(project: @p1, user: @u2, access_level: ProjectMember::DEVELOPER) - @p1.project_members.create(project: @p1, user: @u3, access_level: ProjectMember::MASTER) - end - - it "denies for masters admin-specific actions" do - [owner_actions - master_actions].each do |action| - expect(@abilities.allowed?(@u2, action, @p1)).to be_falsey - end - end - - it "allows for project owner any admin actions" do - owner_actions.each do |action| - expect(@abilities.allowed?(@u4, action, @p1)).to be_truthy - end - end - end - end -end -- cgit v1.2.1 From 99ee86206e3e19dd93910a4e7a3a5b6e3a7add9a Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Mon, 8 Aug 2016 10:07:15 -0700 Subject: remove six, and use a Set instead --- Gemfile | 3 --- Gemfile.lock | 2 -- app/models/ability.rb | 25 +++++++++++++++++++------ lib/api/helpers.rb | 6 +----- spec/models/members/project_member_spec.rb | 3 +-- spec/models/note_spec.rb | 3 +-- 6 files changed, 22 insertions(+), 20 deletions(-) diff --git a/Gemfile b/Gemfile index 194379dd687..96841013815 100644 --- a/Gemfile +++ b/Gemfile @@ -97,9 +97,6 @@ gem 'fog-rackspace', '~> 0.1.1' # for aws storage gem 'unf', '~> 0.1.4' -# Authorization -gem 'six', '~> 0.2.0' - # Seed data gem 'seed-fu', '~> 2.3.5' diff --git a/Gemfile.lock b/Gemfile.lock index 0c28975060c..1d0fcfd3c3a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -683,7 +683,6 @@ GEM rack (~> 1.5) rack-protection (~> 1.4) tilt (>= 1.3, < 3) - six (0.2.0) slack-notifier (1.2.1) slop (3.6.0) spinach (0.8.10) @@ -954,7 +953,6 @@ DEPENDENCIES sidekiq-cron (~> 0.4.0) simplecov (= 0.12.0) sinatra (~> 1.4.4) - six (~> 0.2.0) slack-notifier (~> 1.2.0) spinach-rails (~> 0.2.1) spinach-rerun-reporter (~> 0.0.2) diff --git a/app/models/ability.rb b/app/models/ability.rb index fcd7740d79f..622f481a4fc 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -1,7 +1,23 @@ class Ability class << self + + end + + def allowed?(user, action, subject) + allowed(user, subject).include?(action) + end + def allowed(user, subject) - return anonymous_abilities(user, subject) if user.nil? + return uncached_allowed(user, subject) unless RequestStore.active? + + user_key = user ? user.id : 'anonymous' + subject_key = subject ? "#{subject.class.name}/#{subject.id}" : 'global' + key = "/ability/#{user_key}/#{subject_key}" + RequestStore[key] ||= Set.new(uncached_allowed(user, subject)).freeze + end + + def uncached_allowed(user, subject) + return anonymous_abilities(subject) if user.nil? return [] unless user.is_a?(User) return [] if user.blocked? @@ -586,11 +602,8 @@ class Ability end def abilities - @abilities ||= begin - abilities = Six.new - abilities << self - abilities - end + warn 'Ability.abilities is deprecated, use Ability.allowed?(user, action, subject) instead' + self end private diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index da4b1bf9902..1afca5fe2e8 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -409,11 +409,7 @@ module API end def abilities - @abilities ||= begin - abilities = Six.new - abilities << Ability - abilities - end + Ability end def secret_token diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 913d74645a7..c2bf48da44e 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -71,8 +71,7 @@ describe ProjectMember, models: true do describe :import_team do before do - @abilities = Six.new - @abilities << Ability + @abilities = Ability @project_1 = create :project @project_2 = create :project diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 9e8ae07e0b2..f4b9fa270e4 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -85,8 +85,7 @@ describe Note, models: true do @u1 = create(:user) @u2 = create(:user) @u3 = create(:user) - @abilities = Six.new - @abilities << Ability + @abilities = Ability end describe 'read' do -- cgit v1.2.1 From 8702cef27146ab62d44065af3f3d388c7effcedb Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Mon, 8 Aug 2016 14:02:29 -0700 Subject: don't double-cache project_abilites --- app/models/ability.rb | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index 622f481a4fc..595e6be6642 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -181,17 +181,8 @@ class Ability end def project_abilities(user, project) - key = "/user/#{user.id}/project/#{project.id}" - - if RequestStore.active? - RequestStore.store[key] ||= uncached_project_abilities(user, project) - else - uncached_project_abilities(user, project) - end - end - - def uncached_project_abilities(user, project) rules = [] + # Push abilities on the users team role rules.push(*project_team_rules(project.team, user)) @@ -218,7 +209,7 @@ class Ability rules -= project_archived_rules end - (rules - project_disabled_features_rules(project)).uniq + rules - project_disabled_features_rules(project) end def project_team_rules(team, user) -- cgit v1.2.1 From c218dd90dabb0ddff7fab09abbb348fe1c56201b Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 23 Aug 2016 17:29:40 -0700 Subject: make almost everything on Ability private --- app/models/ability.rb | 90 +++++++++++++++++++++++++-------------------------- 1 file changed, 44 insertions(+), 46 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index 595e6be6642..3eb8a5f6e03 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -1,6 +1,48 @@ class Ability class << self + # Given a list of users and a project this method returns the users that can + # read the given project. + def users_that_can_read_project(users, project) + if project.public? + users + else + users.select do |user| + if user.admin? + true + elsif project.internal? && !user.external? + true + elsif project.owner == user + true + elsif project.team.members.include?(user) + true + else + false + end + end + end + end + # Returns an Array of Issues that can be read by the given user. + # + # issues - The issues to reduce down to those readable by the user. + # user - The User for which to check the issues + def issues_readable_by_user(issues, user = nil) + return issues if user && user.admin? + + issues.select { |issue| issue.visible_to_user?(user) } + end + + # TODO: make this private and use the actual abilities stuff for this + def can_edit_note?(user, note) + return false if !note.editable? || !user.present? + return true if note.author == user || user.admin? + + if note.project + max_access_level = note.project.team.max_member_access(user.id) + max_access_level >= Gitlab::Access::MASTER + else + false + end end def allowed?(user, action, subject) @@ -16,6 +58,8 @@ class Ability RequestStore[key] ||= Set.new(uncached_allowed(user, subject)).freeze end + private + def uncached_allowed(user, subject) return anonymous_abilities(subject) if user.nil? return [] unless user.is_a?(User) @@ -44,38 +88,6 @@ class Ability end.concat(global_abilities(user)) end - # Given a list of users and a project this method returns the users that can - # read the given project. - def users_that_can_read_project(users, project) - if project.public? - users - else - users.select do |user| - if user.admin? - true - elsif project.internal? && !user.external? - true - elsif project.owner == user - true - elsif project.team.members.include?(user) - true - else - false - end - end - end - end - - # Returns an Array of Issues that can be read by the given user. - # - # issues - The issues to reduce down to those readable by the user. - # user - The User for which to check the issues - def issues_readable_by_user(issues, user = nil) - return issues if user && user.admin? - - issues.select { |issue| issue.visible_to_user?(user) } - end - # List of possible abilities for anonymous user def anonymous_abilities(user, subject) if subject.is_a?(PersonalSnippet) @@ -420,18 +432,6 @@ class Ability GroupProjectsFinder.new(group).execute(user).any? end - def can_edit_note?(user, note) - return false if !note.editable? || !user.present? - return true if note.author == user || user.admin? - - if note.project - max_access_level = note.project.team.max_member_access(user.id) - max_access_level >= Gitlab::Access::MASTER - else - false - end - end - def namespace_abilities(user, namespace) rules = [] @@ -597,8 +597,6 @@ class Ability self end - private - def restricted_public_level? current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC) end -- cgit v1.2.1 From 5853c96b49010aaf33b85caeb94dfc18873d5656 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Mon, 8 Aug 2016 11:55:13 -0700 Subject: remove Ability.abilities --- app/controllers/application_controller.rb | 8 ++------ app/finders/issuable_finder.rb | 2 +- app/finders/todos_finder.rb | 2 +- app/mailers/base_mailer.rb | 2 +- app/models/ability.rb | 5 ----- app/models/event.rb | 2 +- app/models/merge_request.rb | 2 +- app/models/user.rb | 6 +----- app/services/base_service.rb | 6 +----- lib/api/helpers.rb | 6 +----- lib/banzai/reference_parser/base_parser.rb | 2 +- .../projects/boards/issues_controller_spec.rb | 4 ++-- .../projects/boards/lists_controller_spec.rb | 4 ++-- spec/controllers/projects/boards_controller_spec.rb | 4 ++-- spec/lib/banzai/reference_parser/base_parser_spec.rb | 8 ++++---- spec/lib/banzai/reference_parser/user_parser_spec.rb | 10 +++++----- spec/models/members/project_member_spec.rb | 6 ++---- spec/models/note_spec.rb | 19 +++++++++---------- 18 files changed, 37 insertions(+), 61 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index ebc2a4651ba..bd4ba384b29 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -24,7 +24,7 @@ class ApplicationController < ActionController::Base protect_from_forgery with: :exception - helper_method :abilities, :can?, :current_application_settings + helper_method :can?, :current_application_settings helper_method :import_sources_enabled?, :github_import_enabled?, :github_import_configured?, :gitlab_import_enabled?, :gitlab_import_configured?, :bitbucket_import_enabled?, :bitbucket_import_configured?, :google_code_import_enabled?, :fogbugz_import_enabled?, :git_import_enabled?, :gitlab_project_import_enabled? rescue_from Encoding::CompatibilityError do |exception| @@ -97,12 +97,8 @@ class ApplicationController < ActionController::Base current_application_settings.after_sign_out_path.presence || new_user_session_path end - def abilities - Ability.abilities - end - def can?(object, action, subject) - abilities.allowed?(object, action, subject) + Ability.allowed?(object, action, subject) end def access_denied! diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 33daac0399e..60996b181f2 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -64,7 +64,7 @@ class IssuableFinder if project? @project = Project.find(params[:project_id]) - unless Ability.abilities.allowed?(current_user, :read_project, @project) + unless Ability.allowed?(current_user, :read_project, @project) @project = nil end else diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb index 06b3e8a9502..a93a63bdb9b 100644 --- a/app/finders/todos_finder.rb +++ b/app/finders/todos_finder.rb @@ -83,7 +83,7 @@ class TodosFinder if project? @project = Project.find(params[:project_id]) - unless Ability.abilities.allowed?(current_user, :read_project, @project) + unless Ability.allowed?(current_user, :read_project, @project) @project = nil end else diff --git a/app/mailers/base_mailer.rb b/app/mailers/base_mailer.rb index 8b83bbd93b7..61a574d3dc0 100644 --- a/app/mailers/base_mailer.rb +++ b/app/mailers/base_mailer.rb @@ -9,7 +9,7 @@ class BaseMailer < ActionMailer::Base default reply_to: Proc.new { default_reply_to_address.format } def can? - Ability.abilities.allowed?(current_user, action, subject) + Ability.allowed?(current_user, action, subject) end private diff --git a/app/models/ability.rb b/app/models/ability.rb index 3eb8a5f6e03..891c5ba9276 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -592,11 +592,6 @@ class Ability [:read_user] end - def abilities - warn 'Ability.abilities is deprecated, use Ability.allowed?(user, action, subject) instead' - self - end - def restricted_public_level? current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC) end diff --git a/app/models/event.rb b/app/models/event.rb index fd736d12359..a0b7b0dc2b5 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -65,7 +65,7 @@ class Event < ActiveRecord::Base elsif created_project? true elsif issue? || issue_note? - Ability.abilities.allowed?(user, :read_issue, note? ? note_target : target) + Ability.allowed?(user, :read_issue, note? ? note_target : target) else ((merge_request? || note?) && target.present?) || milestone? end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 62163e74000..57d673a5f25 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -411,7 +411,7 @@ class MergeRequest < ActiveRecord::Base def can_remove_source_branch?(current_user) !source_project.protected_branch?(source_branch) && !source_project.root_ref?(source_branch) && - Ability.abilities.allowed?(current_user, :push_code, source_project) && + Ability.allowed?(current_user, :push_code, source_project) && diff_head_commit == source_branch_head end diff --git a/app/models/user.rb b/app/models/user.rb index ad3cfbc03e4..8f5958333d7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -460,16 +460,12 @@ class User < ActiveRecord::Base can?(:create_group, nil) end - def abilities - Ability.abilities - end - def can_select_namespace? several_namespaces? || admin end def can?(action, subject) - abilities.allowed?(self, action, subject) + Ability.allowed?(self, action, subject) end def first_name diff --git a/app/services/base_service.rb b/app/services/base_service.rb index 0d55ba5a981..0c208150fb8 100644 --- a/app/services/base_service.rb +++ b/app/services/base_service.rb @@ -7,12 +7,8 @@ class BaseService @project, @current_user, @params = project, user, params.dup end - def abilities - Ability.abilities - end - def can?(object, action, subject) - abilities.allowed?(object, action, subject) + Ability.allowed?(object, action, subject) end def notification_service diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 1afca5fe2e8..fdb70af694d 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -148,7 +148,7 @@ module API end def can?(object, action, subject) - abilities.allowed?(object, action, subject) + Ability.allowed?(object, action, subject) end # Checks the occurrences of required attributes, each attribute must be present in the params hash @@ -408,10 +408,6 @@ module API links.join(', ') end - def abilities - Ability - end - def secret_token File.read(Gitlab.config.gitlab_shell.secret_file).chomp end diff --git a/lib/banzai/reference_parser/base_parser.rb b/lib/banzai/reference_parser/base_parser.rb index 6cf218aaa0d..e8e03e4a98f 100644 --- a/lib/banzai/reference_parser/base_parser.rb +++ b/lib/banzai/reference_parser/base_parser.rb @@ -211,7 +211,7 @@ module Banzai end def can?(user, permission, subject) - Ability.abilities.allowed?(user, permission, subject) + Ability.allowed?(user, permission, subject) end def find_projects_for_hash_keys(hash) diff --git a/spec/controllers/projects/boards/issues_controller_spec.rb b/spec/controllers/projects/boards/issues_controller_spec.rb index d0ad5e26dbd..2896636db5a 100644 --- a/spec/controllers/projects/boards/issues_controller_spec.rb +++ b/spec/controllers/projects/boards/issues_controller_spec.rb @@ -41,8 +41,8 @@ describe Projects::Boards::IssuesController do context 'with unauthorized user' do before do - allow(Ability.abilities).to receive(:allowed?).with(user, :read_project, project).and_return(true) - allow(Ability.abilities).to receive(:allowed?).with(user, :read_issue, project).and_return(false) + allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true) + allow(Ability).to receive(:allowed?).with(user, :read_issue, project).and_return(false) end it 'returns a successful 403 response' do diff --git a/spec/controllers/projects/boards/lists_controller_spec.rb b/spec/controllers/projects/boards/lists_controller_spec.rb index 9496636e3cc..af0491bf486 100644 --- a/spec/controllers/projects/boards/lists_controller_spec.rb +++ b/spec/controllers/projects/boards/lists_controller_spec.rb @@ -35,8 +35,8 @@ describe Projects::Boards::ListsController do context 'with unauthorized user' do before do - allow(Ability.abilities).to receive(:allowed?).with(user, :read_project, project).and_return(true) - allow(Ability.abilities).to receive(:allowed?).with(user, :read_list, project).and_return(false) + allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true) + allow(Ability).to receive(:allowed?).with(user, :read_list, project).and_return(false) end it 'returns a successful 403 response' do diff --git a/spec/controllers/projects/boards_controller_spec.rb b/spec/controllers/projects/boards_controller_spec.rb index 75a6d39e82c..6f6e608e1f3 100644 --- a/spec/controllers/projects/boards_controller_spec.rb +++ b/spec/controllers/projects/boards_controller_spec.rb @@ -23,8 +23,8 @@ describe Projects::BoardsController do context 'with unauthorized user' do before do - allow(Ability.abilities).to receive(:allowed?).with(user, :read_project, project).and_return(true) - allow(Ability.abilities).to receive(:allowed?).with(user, :read_board, project).and_return(false) + allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true) + allow(Ability).to receive(:allowed?).with(user, :read_board, project).and_return(false) end it 'returns a successful 404 response' do diff --git a/spec/lib/banzai/reference_parser/base_parser_spec.rb b/spec/lib/banzai/reference_parser/base_parser_spec.rb index ac9c66e2663..9095d2b1345 100644 --- a/spec/lib/banzai/reference_parser/base_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/base_parser_spec.rb @@ -30,7 +30,7 @@ describe Banzai::ReferenceParser::BaseParser, lib: true do it 'returns the nodes if the attribute value equals the current project ID' do link['data-project'] = project.id.to_s - expect(Ability.abilities).not_to receive(:allowed?) + expect(Ability).not_to receive(:allowed?) expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) end @@ -39,7 +39,7 @@ describe Banzai::ReferenceParser::BaseParser, lib: true do link['data-project'] = other_project.id.to_s - expect(Ability.abilities).to receive(:allowed?). + expect(Ability).to receive(:allowed?). with(user, :read_project, other_project). and_return(true) @@ -57,7 +57,7 @@ describe Banzai::ReferenceParser::BaseParser, lib: true do link['data-project'] = other_project.id.to_s - expect(Ability.abilities).to receive(:allowed?). + expect(Ability).to receive(:allowed?). with(user, :read_project, other_project). and_return(false) @@ -221,7 +221,7 @@ describe Banzai::ReferenceParser::BaseParser, lib: true do it 'delegates the permissions check to the Ability class' do user = double(:user) - expect(Ability.abilities).to receive(:allowed?). + expect(Ability).to receive(:allowed?). with(user, :read_project, project) subject.can?(user, :read_project, project) diff --git a/spec/lib/banzai/reference_parser/user_parser_spec.rb b/spec/lib/banzai/reference_parser/user_parser_spec.rb index 9a82891297d..4e7f82a6e09 100644 --- a/spec/lib/banzai/reference_parser/user_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/user_parser_spec.rb @@ -82,7 +82,7 @@ describe Banzai::ReferenceParser::UserParser, lib: true do end it 'returns the nodes if the user can read the group' do - expect(Ability.abilities).to receive(:allowed?). + expect(Ability).to receive(:allowed?). with(user, :read_group, group). and_return(true) @@ -90,7 +90,7 @@ describe Banzai::ReferenceParser::UserParser, lib: true do end it 'returns an empty Array if the user can not read the group' do - expect(Ability.abilities).to receive(:allowed?). + expect(Ability).to receive(:allowed?). with(user, :read_group, group). and_return(false) @@ -103,7 +103,7 @@ describe Banzai::ReferenceParser::UserParser, lib: true do it 'returns the nodes if the attribute value equals the current project ID' do link['data-project'] = project.id.to_s - expect(Ability.abilities).not_to receive(:allowed?) + expect(Ability).not_to receive(:allowed?) expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) end @@ -113,7 +113,7 @@ describe Banzai::ReferenceParser::UserParser, lib: true do link['data-project'] = other_project.id.to_s - expect(Ability.abilities).to receive(:allowed?). + expect(Ability).to receive(:allowed?). with(user, :read_project, other_project). and_return(true) @@ -125,7 +125,7 @@ describe Banzai::ReferenceParser::UserParser, lib: true do link['data-project'] = other_project.id.to_s - expect(Ability.abilities).to receive(:allowed?). + expect(Ability).to receive(:allowed?). with(user, :read_project, other_project). and_return(false) diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index c2bf48da44e..be57957b569 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -71,8 +71,6 @@ describe ProjectMember, models: true do describe :import_team do before do - @abilities = Ability - @project_1 = create :project @project_2 = create :project @@ -91,8 +89,8 @@ describe ProjectMember, models: true do it { expect(@project_2.users).to include(@user_1) } it { expect(@project_2.users).to include(@user_2) } - it { expect(@abilities.allowed?(@user_1, :create_project, @project_2)).to be_truthy } - it { expect(@abilities.allowed?(@user_2, :read_project, @project_2)).to be_truthy } + it { expect(Ability.allowed?(@user_1, :create_project, @project_2)).to be_truthy } + it { expect(Ability.allowed?(@user_2, :read_project, @project_2)).to be_truthy } end describe 'project 1 should not be changed' do diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index f4b9fa270e4..e6b6e7c0634 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -85,7 +85,6 @@ describe Note, models: true do @u1 = create(:user) @u2 = create(:user) @u3 = create(:user) - @abilities = Ability end describe 'read' do @@ -94,9 +93,9 @@ describe Note, models: true do @p2.project_members.create(user: @u3, access_level: ProjectMember::GUEST) end - it { expect(@abilities.allowed?(@u1, :read_note, @p1)).to be_falsey } - it { expect(@abilities.allowed?(@u2, :read_note, @p1)).to be_truthy } - it { expect(@abilities.allowed?(@u3, :read_note, @p1)).to be_falsey } + it { expect(Ability.allowed?(@u1, :read_note, @p1)).to be_falsey } + it { expect(Ability.allowed?(@u2, :read_note, @p1)).to be_truthy } + it { expect(Ability.allowed?(@u3, :read_note, @p1)).to be_falsey } end describe 'write' do @@ -105,9 +104,9 @@ describe Note, models: true do @p2.project_members.create(user: @u3, access_level: ProjectMember::DEVELOPER) end - it { expect(@abilities.allowed?(@u1, :create_note, @p1)).to be_falsey } - it { expect(@abilities.allowed?(@u2, :create_note, @p1)).to be_truthy } - it { expect(@abilities.allowed?(@u3, :create_note, @p1)).to be_falsey } + it { expect(Ability.allowed?(@u1, :create_note, @p1)).to be_falsey } + it { expect(Ability.allowed?(@u2, :create_note, @p1)).to be_truthy } + it { expect(Ability.allowed?(@u3, :create_note, @p1)).to be_falsey } end describe 'admin' do @@ -117,9 +116,9 @@ describe Note, models: true do @p2.project_members.create(user: @u3, access_level: ProjectMember::MASTER) end - it { expect(@abilities.allowed?(@u1, :admin_note, @p1)).to be_falsey } - it { expect(@abilities.allowed?(@u2, :admin_note, @p1)).to be_truthy } - it { expect(@abilities.allowed?(@u3, :admin_note, @p1)).to be_falsey } + it { expect(Ability.allowed?(@u1, :admin_note, @p1)).to be_falsey } + it { expect(Ability.allowed?(@u2, :admin_note, @p1)).to be_truthy } + it { expect(Ability.allowed?(@u3, :admin_note, @p1)).to be_falsey } end end -- cgit v1.2.1 From e208765a92748086cacbc56225e827c8463750a5 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Thu, 11 Aug 2016 15:12:52 -0700 Subject: add policies, and factor out ProjectPolicy --- app/models/ability.rb | 35 +------ app/policies/base_policy.rb | 25 +++++ app/policies/project_policy.rb | 202 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 231 insertions(+), 31 deletions(-) create mode 100644 app/policies/base_policy.rb create mode 100644 app/policies/project_policy.rb diff --git a/app/models/ability.rb b/app/models/ability.rb index 891c5ba9276..4f0ffa09a1f 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -71,7 +71,7 @@ class Ability def abilities_by_subject_class(user:, subject:) case subject when CommitStatus then commit_status_abilities(user, subject) - when Project then project_abilities(user, subject) + when Project then ProjectPolicy.new(user, subject).abilities when Issue then issue_abilities(user, subject) when Note then note_abilities(user, subject) when ProjectSnippet then project_snippet_abilities(user, subject) @@ -85,7 +85,7 @@ class Ability when ExternalIssue, Deployment, Environment then project_abilities(user, subject.project) when Ci::Runner then runner_abilities(user, subject) else [] - end.concat(global_abilities(user)) + end + global_abilities(user) end # List of possible abilities for anonymous user @@ -193,35 +193,8 @@ class Ability end def project_abilities(user, project) - rules = [] - - # Push abilities on the users team role - rules.push(*project_team_rules(project.team, user)) - - owner = user.admin? || - project.owner == user || - (project.group && project.group.has_owner?(user)) - - if owner - rules.push(*project_owner_rules) - end - - if project.public? || (project.internal? && !user.external?) - rules.push(*public_project_rules) - - # Allow to read builds for internal projects - rules << :read_build if project.public_builds? - - unless owner || project.team.member?(user) || project_group_member?(project, user) - rules << :request_access if project.request_access_enabled - end - end - - if project.archived? - rules -= project_archived_rules - end - - rules - project_disabled_features_rules(project) + # temporary patch, deleteme before merge + ProjectPolicy.new(user, project).abilities.to_a end def project_team_rules(team, user) diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb new file mode 100644 index 00000000000..3f52b0b005a --- /dev/null +++ b/app/policies/base_policy.rb @@ -0,0 +1,25 @@ +class BasePolicy + def initialize(user, subject) + @user = user + @subject = subject + end + + def abilities + @can = Set.new + @cannot = Set.new + generate! + @can - @cannot + end + + def generate! + raise 'abstract' + end + + def can!(*rules) + @can.merge(rules) + end + + def cannot!(*rules) + @cannot.merge(rules) + end +end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb new file mode 100644 index 00000000000..1e82070e62a --- /dev/null +++ b/app/policies/project_policy.rb @@ -0,0 +1,202 @@ +class ProjectPolicy < BasePolicy + def project + @subject + end + + def guest_access! + can! :read_project + can! :read_board + can! :read_list + can! :read_wiki + can! :read_issue + can! :read_label + can! :read_milestone + can! :read_project_snippet + can! :read_project_member + can! :read_merge_request + can! :read_note + can! :create_project + can! :create_issue + can! :create_note + can! :upload_file + end + + def reporter_access! + can! :download_code + can! :fork_project + can! :create_project_snippet + can! :update_issue + can! :admin_issue + can! :admin_label + can! :read_commit_status + can! :read_build + can! :read_container_image + can! :read_pipeline + can! :read_environment + can! :read_deployment + end + + def developer_access! + can! :admin_merge_request + can! :update_merge_request + can! :create_commit_status + can! :update_commit_status + can! :create_build + can! :update_build + can! :create_pipeline + can! :update_pipeline + can! :create_merge_request + can! :create_wiki + can! :push_code + can! :create_container_image + can! :update_container_image + can! :create_environment + can! :create_deployment + end + + def master_access! + can! :push_code_to_protected_branches + can! :update_project_snippet + can! :update_environment + can! :update_deployment + can! :admin_milestone + can! :admin_project_snippet + can! :admin_project_member + can! :admin_merge_request + can! :admin_note + can! :admin_wiki + can! :admin_project + can! :admin_commit_status + can! :admin_build + can! :admin_container_image + can! :admin_pipeline + can! :admin_environment + can! :admin_deployment + end + + def public_access! + can! :download_code + can! :fork_project + can! :read_commit_status + can! :read_pipeline + can! :read_container_image + end + + def owner_access! + guest_access! + reporter_access! + developer_access! + master_access! + can! :change_namespace + can! :change_visibility_level + can! :rename_project + can! :remove_project + can! :archive_project + can! :remove_fork_project + can! :destroy_merge_request + can! :destroy_issue + end + + # Push abilities on the users team role + def team_access! + access = project.team.max_member_access(@user.id) + + return if access < Gitlab::Access::GUEST + guest_access! + + return if access < Gitlab::Access::REPORTER + reporter_access! + + return if access < Gitlab::Access::DEVELOPER + developer_access! + + return if access < Gitlab::Access::MASTER + master_access! + end + + def archived_access! + cannot! :create_merge_request + cannot! :push_code + cannot! :push_code_to_protected_branches + cannot! :update_merge_request + cannot! :admin_merge_request + end + + def disabled_features! + unless project.issues_enabled + cannot!(*named_abilities(:issue)) + end + + unless project.merge_requests_enabled + cannot!(*named_abilities(:merge_request)) + end + + unless project.issues_enabled or project.merge_requests_enabled + cannot!(*named_abilities(:label)) + cannot!(*named_abilities(:milestone)) + end + + unless project.snippets_enabled + cannot!(*named_abilities(:project_snippet)) + end + + unless project.wiki_enabled + cannot!(*named_abilities(:wiki)) + end + + unless project.builds_enabled + cannot!(*named_abilities(:build)) + cannot!(*named_abilities(:pipeline)) + cannot!(*named_abilities(:environment)) + cannot!(*named_abilities(:deployment)) + end + + unless project.container_registry_enabled + cannot!(*named_abilities(:container_image)) + end + end + + def generate! + team_access! + + owner = @user.admin? || + project.owner == @user || + (project.group && project.group.has_owner?(@user)) + + owner_access! if owner + + if project.public? || (project.internal? && !@user.external?) + guest_access! + public_access! + + # Allow to read builds for internal projects + can! :read_build if project.public_builds? + + if project.request_access_enabled && + !(owner || project.team.member?(@user) || project_group_member?) + can! :request_access + end + end + + archived_access! if project.archived? + + disabled_features! + end + + def project_group_member? + project.group && + ( + project.group.members.exists?(user_id: @user.id) || + project.group.requesters.exists?(user_id: @user.id) + ) + end + + def named_abilities(name) + [ + :"read_#{name}", + :"create_#{name}", + :"update_#{name}", + :"admin_#{name}" + ] + end +end -- cgit v1.2.1 From 29b1623a3615fb7683702f4de2dfeafca10f9c1c Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 23 Aug 2016 16:19:36 -0700 Subject: add project_policy_spec to replace .project_abilities spec --- spec/models/ability_spec.rb | 64 ------------------------------------ spec/policies/project_policy_spec.rb | 36 ++++++++++++++++++++ 2 files changed, 36 insertions(+), 64 deletions(-) create mode 100644 spec/policies/project_policy_spec.rb diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index c50ca38bdd9..c9e6a334c67 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -171,70 +171,6 @@ describe Ability, lib: true do end end - shared_examples_for ".project_abilities" do |enable_request_store| - before do - RequestStore.begin! if enable_request_store - end - - after do - if enable_request_store - RequestStore.end! - RequestStore.clear! - end - end - - describe '.project_abilities' do - let!(:project) { create(:empty_project, :public) } - let!(:user) { create(:user) } - - it 'returns permissions for admin user' do - admin = create(:admin) - - results = described_class.project_abilities(admin, project) - - expect(results.count).to eq(68) - end - - it 'returns permissions for an owner' do - results = described_class.project_abilities(project.owner, project) - - expect(results.count).to eq(68) - end - - it 'returns permissions for a master' do - project.team << [user, :master] - - results = described_class.project_abilities(user, project) - - expect(results.count).to eq(60) - end - - it 'returns permissions for a developer' do - project.team << [user, :developer] - - results = described_class.project_abilities(user, project) - - expect(results.count).to eq(44) - end - - it 'returns permissions for a guest' do - project.team << [user, :guest] - - results = described_class.project_abilities(user, project) - - expect(results.count).to eq(21) - end - end - end - - describe '.project_abilities with RequestStore' do - it_behaves_like ".project_abilities", true - end - - describe '.project_abilities without RequestStore' do - it_behaves_like ".project_abilities", false - end - describe '.issues_readable_by_user' do context 'with an admin user' do it 'returns all given issues' do diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb new file mode 100644 index 00000000000..eda1cafd65e --- /dev/null +++ b/spec/policies/project_policy_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +describe ProjectPolicy, models: true do + let(:project) { create(:empty_project, :public) } + let(:guest) { create(:user) } + let(:reporter) { create(:user) } + let(:dev) { create(:user) } + let(:master) { create(:user) } + let(:owner) { create(:user) } + let(:admin) { create(:admin) } + + let(:users_ordered_by_permissions) do + [nil, guest, reporter, dev, master, owner, admin] + end + + let(:users_permissions) do + users_ordered_by_permissions.map { |u| Ability.allowed(u, project).size } + end + + before do + project.team << [guest, :guest] + project.team << [master, :master] + project.team << [dev, :developer] + project.team << [reporter, :reporter] + + group = create(:group) + project.project_group_links.create( + group: group, + group_access: Gitlab::Access::MASTER) + group.add_owner(owner) + end + + it 'returns increasing permissions for each level' do + expect(users_permissions).to eq(users_permissions.sort.uniq) + end +end -- cgit v1.2.1 From 1ca9b3354a350b83d1e025b3d46280bc5bb60f2b Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Fri, 12 Aug 2016 11:36:16 -0700 Subject: add support for anonymous abilities --- app/models/ability.rb | 194 ++--------------------------------------- app/policies/base_policy.rb | 26 +++++- app/policies/project_policy.rb | 52 ++++++++--- 3 files changed, 67 insertions(+), 205 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index 4f0ffa09a1f..5d2cbde4c0e 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -71,7 +71,7 @@ class Ability def abilities_by_subject_class(user:, subject:) case subject when CommitStatus then commit_status_abilities(user, subject) - when Project then ProjectPolicy.new(user, subject).abilities + when Project then ProjectPolicy.abilities(user, subject) when Issue then issue_abilities(user, subject) when Note then note_abilities(user, subject) when ProjectSnippet then project_snippet_abilities(user, subject) @@ -96,8 +96,10 @@ class Ability anonymous_project_snippet_abilities(subject) elsif subject.is_a?(CommitStatus) anonymous_commit_status_abilities(subject) - elsif subject.is_a?(Project) || subject.respond_to?(:project) - anonymous_project_abilities(subject) + elsif subject.is_a?(Project) + ProjectPolicy.abilities(nil, subject) + elsif subject.respond_to?(:project) + ProjectPolicy.abilities(nil, subject.project) elsif subject.is_a?(Group) || subject.respond_to?(:group) anonymous_group_abilities(subject) elsif subject.is_a?(User) @@ -194,174 +196,7 @@ class Ability def project_abilities(user, project) # temporary patch, deleteme before merge - ProjectPolicy.new(user, project).abilities.to_a - end - - def project_team_rules(team, user) - # Rules based on role in project - if team.master?(user) - project_master_rules - elsif team.developer?(user) - project_dev_rules - elsif team.reporter?(user) - project_report_rules - elsif team.guest?(user) - project_guest_rules - else - [] - end - end - - def public_project_rules - @public_project_rules ||= project_guest_rules + [ - :download_code, - :fork_project, - :read_commit_status, - :read_pipeline, - :read_container_image - ] - end - - def project_guest_rules - @project_guest_rules ||= [ - :read_project, - :read_wiki, - :read_issue, - :read_board, - :read_list, - :read_label, - :read_milestone, - :read_project_snippet, - :read_project_member, - :read_merge_request, - :read_note, - :create_project, - :create_issue, - :create_note, - :upload_file - ] - end - - def project_report_rules - @project_report_rules ||= project_guest_rules + [ - :download_code, - :fork_project, - :create_project_snippet, - :update_issue, - :admin_issue, - :admin_label, - :admin_list, - :read_commit_status, - :read_build, - :read_container_image, - :read_pipeline, - :read_environment, - :read_deployment - ] - end - - def project_dev_rules - @project_dev_rules ||= project_report_rules + [ - :admin_merge_request, - :update_merge_request, - :create_commit_status, - :update_commit_status, - :create_build, - :update_build, - :create_pipeline, - :update_pipeline, - :create_merge_request, - :create_wiki, - :push_code, - :resolve_note, - :create_container_image, - :update_container_image, - :create_environment, - :create_deployment - ] - end - - def project_archived_rules - @project_archived_rules ||= [ - :create_merge_request, - :push_code, - :push_code_to_protected_branches, - :update_merge_request, - :admin_merge_request - ] - end - - def project_master_rules - @project_master_rules ||= project_dev_rules + [ - :push_code_to_protected_branches, - :update_project_snippet, - :update_environment, - :update_deployment, - :admin_milestone, - :admin_project_snippet, - :admin_project_member, - :admin_merge_request, - :admin_note, - :admin_wiki, - :admin_project, - :admin_commit_status, - :admin_build, - :admin_container_image, - :admin_pipeline, - :admin_environment, - :admin_deployment - ] - end - - def project_owner_rules - @project_owner_rules ||= project_master_rules + [ - :change_namespace, - :change_visibility_level, - :rename_project, - :remove_project, - :archive_project, - :remove_fork_project, - :destroy_merge_request, - :destroy_issue - ] - end - - def project_disabled_features_rules(project) - rules = [] - - unless project.issues_enabled - rules += named_abilities('issue') - end - - unless project.merge_requests_enabled - rules += named_abilities('merge_request') - end - - unless project.issues_enabled or project.merge_requests_enabled - rules += named_abilities('label') - rules += named_abilities('milestone') - end - - unless project.snippets_enabled - rules += named_abilities('project_snippet') - end - - unless project.has_wiki? - rules += named_abilities('wiki') - end - - unless project.builds_enabled - rules += named_abilities('build') - rules += named_abilities('pipeline') - rules += named_abilities('environment') - rules += named_abilities('deployment') - end - - unless project.container_registry_enabled - rules += named_abilities('container_image') - end - - rules + ProjectPolicy.abilities(user, project).to_a end def group_abilities(user, group) @@ -569,15 +404,6 @@ class Ability current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC) end - def named_abilities(name) - [ - :"read_#{name}", - :"create_#{name}", - :"update_#{name}", - :"admin_#{name}" - ] - end - def filter_confidential_issues_abilities(user, issue, rules) return rules if user.admin? || !issue.confidential? @@ -589,13 +415,5 @@ class Ability rules end - - def project_group_member?(project, user) - project.group && - ( - project.group.members.exists?(user_id: user.id) || - project.group.requesters.exists?(user_id: user.id) - ) - end end end diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb index 3f52b0b005a..10ce38329c4 100644 --- a/app/policies/base_policy.rb +++ b/app/policies/base_policy.rb @@ -1,14 +1,21 @@ class BasePolicy + def self.abilities(user, subject) + new(user, subject).abilities + end + + attr_reader :user, :subject def initialize(user, subject) @user = user @subject = subject end def abilities - @can = Set.new - @cannot = Set.new - generate! - @can - @cannot + return anonymous_abilities if @user.nil? + collect_rules { rules } + end + + def anonymous_abilities + collect_rules { anonymous_rules } end def generate! @@ -22,4 +29,15 @@ class BasePolicy def cannot!(*rules) @cannot.merge(rules) end + + private + + def collect_rules(&b) + return Set.new if @subject.nil? + + @can = Set.new + @cannot = Set.new + yield + @can - @cannot + end end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 1e82070e62a..95e8b71c102 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -28,6 +28,7 @@ class ProjectPolicy < BasePolicy can! :update_issue can! :admin_issue can! :admin_label + can! :admin_list can! :read_commit_status can! :read_build can! :read_container_image @@ -48,6 +49,7 @@ class ProjectPolicy < BasePolicy can! :create_merge_request can! :create_wiki can! :push_code + can! :resolve_note can! :create_container_image can! :update_container_image can! :create_environment @@ -98,8 +100,8 @@ class ProjectPolicy < BasePolicy end # Push abilities on the users team role - def team_access! - access = project.team.max_member_access(@user.id) + def team_access!(user) + access = project.team.max_member_access(user.id) return if access < Gitlab::Access::GUEST guest_access! @@ -140,7 +142,7 @@ class ProjectPolicy < BasePolicy cannot!(*named_abilities(:project_snippet)) end - unless project.wiki_enabled + unless project.has_wiki? cannot!(*named_abilities(:wiki)) end @@ -156,16 +158,16 @@ class ProjectPolicy < BasePolicy end end - def generate! - team_access! + def rules + team_access!(user) - owner = @user.admin? || - project.owner == @user || - (project.group && project.group.has_owner?(@user)) + owner = user.admin? || + project.owner == user || + (project.group && project.group.has_owner?(user)) owner_access! if owner - if project.public? || (project.internal? && !@user.external?) + if project.public? || (project.internal? && !user.external?) guest_access! public_access! @@ -173,7 +175,7 @@ class ProjectPolicy < BasePolicy can! :read_build if project.public_builds? if project.request_access_enabled && - !(owner || project.team.member?(@user) || project_group_member?) + !(owner || project.team.member?(user) || project_group_member?(user)) can! :request_access end end @@ -183,11 +185,35 @@ class ProjectPolicy < BasePolicy disabled_features! end - def project_group_member? + def anonymous_rules + return unless project.public? + + can! :read_project + can! :read_board + can! :read_list + can! :read_wiki + can! :read_label + can! :read_milestone + can! :read_project_snippet + can! :read_project_member + can! :read_merge_request + can! :read_note + can! :read_pipeline + can! :read_commit_status + can! :read_container_image + can! :download_code + + # Allow to read builds by anonymous user if guests are allowed + can! :read_build if project.public_builds? + + disabled_features! + end + + def project_group_member?(user) project.group && ( - project.group.members.exists?(user_id: @user.id) || - project.group.requesters.exists?(user_id: @user.id) + project.group.members.exists?(user_id: user.id) || + project.group.requesters.exists?(user_id: user.id) ) end -- cgit v1.2.1 From 4d904bf3521b4600db228c48214f3892e86ac72a Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 16 Aug 2016 11:10:34 -0700 Subject: port issues to Issu{able,e}Policy --- app/models/ability.rb | 6 ++++-- app/policies/base_policy.rb | 12 ++++++++++-- app/policies/issuable_policy.rb | 14 ++++++++++++++ app/policies/issue_policy.rb | 27 +++++++++++++++++++++++++++ app/policies/project_policy.rb | 3 +++ 5 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 app/policies/issuable_policy.rb create mode 100644 app/policies/issue_policy.rb diff --git a/app/models/ability.rb b/app/models/ability.rb index 5d2cbde4c0e..1ea97855e04 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -72,7 +72,7 @@ class Ability case subject when CommitStatus then commit_status_abilities(user, subject) when Project then ProjectPolicy.abilities(user, subject) - when Issue then issue_abilities(user, subject) + when Issue then IssuePolicy.abilities(user, subject) when Note then note_abilities(user, subject) when ProjectSnippet then project_snippet_abilities(user, subject) when PersonalSnippet then personal_snippet_abilities(user, subject) @@ -89,7 +89,7 @@ class Ability end # List of possible abilities for anonymous user - def anonymous_abilities(user, subject) + def anonymous_abilities(subject) if subject.is_a?(PersonalSnippet) anonymous_personal_snippet_abilities(subject) elsif subject.is_a?(ProjectSnippet) @@ -98,6 +98,8 @@ class Ability anonymous_commit_status_abilities(subject) elsif subject.is_a?(Project) ProjectPolicy.abilities(nil, subject) + elsif subject.is_a?(Issue) + IssuePolicy.abilities(nil, subject) elsif subject.respond_to?(:project) ProjectPolicy.abilities(nil, subject.project) elsif subject.is_a?(Group) || subject.respond_to?(:group) diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb index 10ce38329c4..fd5d05a1bd1 100644 --- a/app/policies/base_policy.rb +++ b/app/policies/base_policy.rb @@ -3,6 +3,10 @@ class BasePolicy new(user, subject).abilities end + def self.class_for(subject) + "#{subject.class.name}Policy".constantize + end + attr_reader :user, :subject def initialize(user, subject) @user = user @@ -18,8 +22,12 @@ class BasePolicy collect_rules { anonymous_rules } end - def generate! - raise 'abstract' + def anonymous_rules + rules + end + + def delegate!(new_subject) + @can.merge(BasePolicy.class_for(new_subject).abilities(@user, new_subject)) end def can!(*rules) diff --git a/app/policies/issuable_policy.rb b/app/policies/issuable_policy.rb new file mode 100644 index 00000000000..c253f9a9399 --- /dev/null +++ b/app/policies/issuable_policy.rb @@ -0,0 +1,14 @@ +class IssuablePolicy < BasePolicy + def action_name + @subject.class.name.underscore + end + + def rules + if @user && (@subject.author == @user || @subject.assignee == @user) + can! :"read_#{action_name}" + can! :"update_#{action_name}" + end + + delegate! @subject.project + end +end diff --git a/app/policies/issue_policy.rb b/app/policies/issue_policy.rb new file mode 100644 index 00000000000..08538861364 --- /dev/null +++ b/app/policies/issue_policy.rb @@ -0,0 +1,27 @@ +class IssuePolicy < IssuablePolicy + def issue + @subject + end + + def rules + super + + if @subject.confidential? && !can_read_confidential? + cannot! :read_issue + cannot! :admin_issue + cannot! :update_issue + cannot! :read_issue + end + end + + private + + def can_read_confidential? + return false unless @user + return true if @user.admin? + return true if @subject.author == @user + return true if @subject.assignee == @user + return true if @subject.project.team.member?(@user, Gitlab::Access::REPORTER) + false + end +end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 95e8b71c102..4380b00d962 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -203,6 +203,9 @@ class ProjectPolicy < BasePolicy can! :read_container_image can! :download_code + # NB: may be overridden by IssuePolicy + can! :read_issue + # Allow to read builds by anonymous user if guests are allowed can! :read_build if project.public_builds? -- cgit v1.2.1 From 092861093066f6b474c2dc72de34acf64380a3e6 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 16 Aug 2016 11:32:08 -0700 Subject: add and use MergeRequestPolicy --- app/models/ability.rb | 7 +++++-- app/policies/merge_request_policy.rb | 3 +++ 2 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 app/policies/merge_request_policy.rb diff --git a/app/models/ability.rb b/app/models/ability.rb index 1ea97855e04..b8e3e97b351 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -70,13 +70,14 @@ class Ability def abilities_by_subject_class(user:, subject:) case subject - when CommitStatus then commit_status_abilities(user, subject) when Project then ProjectPolicy.abilities(user, subject) when Issue then IssuePolicy.abilities(user, subject) + when MergeRequest then MergeRequestPolicy.abilities(user, subject) + + when CommitStatus then commit_status_abilities(user, subject) when Note then note_abilities(user, subject) when ProjectSnippet then project_snippet_abilities(user, subject) when PersonalSnippet then personal_snippet_abilities(user, subject) - when MergeRequest then merge_request_abilities(user, subject) when Group then group_abilities(user, subject) when Namespace then namespace_abilities(user, subject) when GroupMember then group_member_abilities(user, subject) @@ -100,6 +101,8 @@ class Ability ProjectPolicy.abilities(nil, subject) elsif subject.is_a?(Issue) IssuePolicy.abilities(nil, subject) + elsif subject.is_a?(MergeRequest) + MergeRequestPolicy.abilities(nil, subject) elsif subject.respond_to?(:project) ProjectPolicy.abilities(nil, subject.project) elsif subject.is_a?(Group) || subject.respond_to?(:group) diff --git a/app/policies/merge_request_policy.rb b/app/policies/merge_request_policy.rb new file mode 100644 index 00000000000..bc3afc626fb --- /dev/null +++ b/app/policies/merge_request_policy.rb @@ -0,0 +1,3 @@ +class MergeRequestPolicy < IssuablePolicy + # pass +end -- cgit v1.2.1 From 16fe6dc7b159a0e6b68a586065de1f95d6acecfa Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 16 Aug 2016 12:05:44 -0700 Subject: port CommitStatus/Build --- app/models/ability.rb | 3 ++- app/policies/base_policy.rb | 4 ++++ app/policies/ci/build_policy.rb | 13 +++++++++++++ app/policies/commit_status_policy.rb | 5 +++++ 4 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 app/policies/ci/build_policy.rb create mode 100644 app/policies/commit_status_policy.rb diff --git a/app/models/ability.rb b/app/models/ability.rb index b8e3e97b351..c89cc9b2e17 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -74,7 +74,8 @@ class Ability when Issue then IssuePolicy.abilities(user, subject) when MergeRequest then MergeRequestPolicy.abilities(user, subject) - when CommitStatus then commit_status_abilities(user, subject) + when Ci::Build then Ci::BuildPolicy.abilities(user, subject) + when CommitStatus then CommitStatus.abilities(user, subject) when Note then note_abilities(user, subject) when ProjectSnippet then project_snippet_abilities(user, subject) when PersonalSnippet then personal_snippet_abilities(user, subject) diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb index fd5d05a1bd1..e1757d97e89 100644 --- a/app/policies/base_policy.rb +++ b/app/policies/base_policy.rb @@ -30,6 +30,10 @@ class BasePolicy @can.merge(BasePolicy.class_for(new_subject).abilities(@user, new_subject)) end + def can?(rule) + @can.include?(rule) && !@cannot.include?(rule) + end + def can!(*rules) @can.merge(rules) end diff --git a/app/policies/ci/build_policy.rb b/app/policies/ci/build_policy.rb new file mode 100644 index 00000000000..2232e231cf8 --- /dev/null +++ b/app/policies/ci/build_policy.rb @@ -0,0 +1,13 @@ +module Ci + class BuildPolicy < CommitStatusPolicy + def rules + super + + # If we can't read build we should also not have that + # ability when looking at this in context of commit_status + %w(read create update admin).each do |rule| + cannot! :"#{rule}_commit_status" unless can? :"#{rule}_build" + end + end + end +end diff --git a/app/policies/commit_status_policy.rb b/app/policies/commit_status_policy.rb new file mode 100644 index 00000000000..593df738328 --- /dev/null +++ b/app/policies/commit_status_policy.rb @@ -0,0 +1,5 @@ +class CommitStatusPolicy < BasePolicy + def rules + delegate! @subject.project + end +end -- cgit v1.2.1 From 3656d3b88a01a50a5eaf66a16b6ac47d3c58352c Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 16 Aug 2016 12:55:44 -0700 Subject: add automatic detection of the policy class --- app/models/ability.rb | 18 +++--------------- app/policies/base_policy.rb | 16 +++++++++++++++- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index c89cc9b2e17..ac5e82c14d2 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -61,6 +61,9 @@ class Ability private def uncached_allowed(user, subject) + policy_class = BasePolicy.class_for(subject) rescue nil + return policy_class.abilities(user, subject) if policy_class + return anonymous_abilities(subject) if user.nil? return [] unless user.is_a?(User) return [] if user.blocked? @@ -70,13 +73,6 @@ class Ability def abilities_by_subject_class(user:, subject:) case subject - when Project then ProjectPolicy.abilities(user, subject) - when Issue then IssuePolicy.abilities(user, subject) - when MergeRequest then MergeRequestPolicy.abilities(user, subject) - - when Ci::Build then Ci::BuildPolicy.abilities(user, subject) - when CommitStatus then CommitStatus.abilities(user, subject) - when Note then note_abilities(user, subject) when ProjectSnippet then project_snippet_abilities(user, subject) when PersonalSnippet then personal_snippet_abilities(user, subject) when Group then group_abilities(user, subject) @@ -96,14 +92,6 @@ class Ability anonymous_personal_snippet_abilities(subject) elsif subject.is_a?(ProjectSnippet) anonymous_project_snippet_abilities(subject) - elsif subject.is_a?(CommitStatus) - anonymous_commit_status_abilities(subject) - elsif subject.is_a?(Project) - ProjectPolicy.abilities(nil, subject) - elsif subject.is_a?(Issue) - IssuePolicy.abilities(nil, subject) - elsif subject.is_a?(MergeRequest) - MergeRequestPolicy.abilities(nil, subject) elsif subject.respond_to?(:project) ProjectPolicy.abilities(nil, subject.project) elsif subject.is_a?(Group) || subject.respond_to?(:group) diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb index e1757d97e89..12f60d8f76e 100644 --- a/app/policies/base_policy.rb +++ b/app/policies/base_policy.rb @@ -4,7 +4,21 @@ class BasePolicy end def self.class_for(subject) - "#{subject.class.name}Policy".constantize + subject.class.ancestors.each do |klass| + next unless klass.name + + begin + policy_class = "#{klass.name}Policy".constantize + + # NB: the < operator here tests whether policy_class + # inherits from BasePolicy + return policy_class if policy_class < BasePolicy + rescue NameError + nil + end + end + + raise "no policy for #{subject.class.name}" end attr_reader :user, :subject -- cgit v1.2.1 From d87c1d550f4870275432698e3cb19033c6855a15 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 16 Aug 2016 13:08:14 -0700 Subject: port notes and project snippets --- app/policies/note_policy.rb | 19 +++++++++++++++++++ app/policies/project_snippet_policy.rb | 20 ++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 app/policies/note_policy.rb create mode 100644 app/policies/project_snippet_policy.rb diff --git a/app/policies/note_policy.rb b/app/policies/note_policy.rb new file mode 100644 index 00000000000..83847466ee2 --- /dev/null +++ b/app/policies/note_policy.rb @@ -0,0 +1,19 @@ +class NotePolicy < BasePolicy + def rules + delegate! @subject.project + + return unless @user + + if @subject.author == @user + can! :read_note + can! :update_note + can! :admin_note + can! :resolve_note + end + + if @subject.for_merge_request? && + @subject.noteable.author == @user + can! :resolve_note + end + end +end diff --git a/app/policies/project_snippet_policy.rb b/app/policies/project_snippet_policy.rb new file mode 100644 index 00000000000..57acccfafd9 --- /dev/null +++ b/app/policies/project_snippet_policy.rb @@ -0,0 +1,20 @@ +class ProjectSnippetPolicy < BasePolicy + def rules + can! :read_project_snippet if @subject.public? + return unless @user + + if @user && @subject.author == @user || @user.admin? + can! :read_project_snippet + can! :update_project_snippet + can! :admin_project_snippet + end + + if @subject.internal? && !@user.external? + can! :read_project_snippet + end + + if @subject.private? && @subject.project.team.member?(@user) + can! :read_project_snippet + end + end +end -- cgit v1.2.1 From 3fdcebfdda31b0cc0f5641489bb4066b1f815df3 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 16 Aug 2016 14:09:44 -0700 Subject: trim dead code --- app/models/ability.rb | 81 --------------------------------------------------- 1 file changed, 81 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index ac5e82c14d2..323597c8888 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -73,7 +73,6 @@ class Ability def abilities_by_subject_class(user:, subject:) case subject - when ProjectSnippet then project_snippet_abilities(user, subject) when PersonalSnippet then personal_snippet_abilities(user, subject) when Group then group_abilities(user, subject) when Namespace then namespace_abilities(user, subject) @@ -140,13 +139,6 @@ class Ability end end - def anonymous_commit_status_abilities(subject) - rules = anonymous_project_abilities(subject.project) - # If subject is Ci::Build which inherits from CommitStatus filter the abilities - rules = filter_build_abilities(rules) if subject.is_a?(Ci::Build) - rules - end - def anonymous_group_abilities(subject) rules = [] @@ -169,14 +161,6 @@ class Ability end end - def anonymous_project_snippet_abilities(snippet) - if snippet.public? - [:read_project_snippet] - else - [] - end - end - def anonymous_user_abilities [:read_user] unless restricted_public_level? end @@ -248,46 +232,6 @@ class Ability rules.flatten end - [:issue, :merge_request].each do |name| - define_method "#{name}_abilities" do |user, subject| - rules = [] - - if subject.author == user || (subject.respond_to?(:assignee) && subject.assignee == user) - rules += [ - :"read_#{name}", - :"update_#{name}", - ] - end - - rules += project_abilities(user, subject.project) - rules = filter_confidential_issues_abilities(user, subject, rules) if subject.is_a?(Issue) - rules - end - end - - def note_abilities(user, note) - rules = [] - - if note.author == user - rules += [ - :read_note, - :update_note, - :admin_note, - :resolve_note - ] - end - - if note.respond_to?(:project) && note.project - rules += project_abilities(user, note.project) - end - - if note.for_merge_request? && note.noteable.author == user - rules << :resolve_note - end - - rules - end - def personal_snippet_abilities(user, snippet) rules = [] @@ -306,24 +250,6 @@ class Ability rules end - def project_snippet_abilities(user, snippet) - rules = [] - - if snippet.author == user || user.admin? - rules += [ - :read_project_snippet, - :update_project_snippet, - :admin_project_snippet - ] - end - - if snippet.public? || (snippet.internal? && !user.external?) || (snippet.private? && snippet.project.team.member?(user)) - rules << :read_project_snippet - end - - rules - end - def group_member_abilities(user, subject) rules = [] target_user = subject.user @@ -362,13 +288,6 @@ class Ability rules end - def commit_status_abilities(user, subject) - rules = project_abilities(user, subject.project) - # If subject is Ci::Build which inherits from CommitStatus filter the abilities - rules = filter_build_abilities(rules) if subject.is_a?(Ci::Build) - rules - end - def filter_build_abilities(rules) # If we can't read build we should also not have that # ability when looking at this in context of commit_status -- cgit v1.2.1 From 4016c5351362a409b9d8bb258e0330089cdb4394 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 16 Aug 2016 15:31:01 -0700 Subject: port personal snippets --- app/models/ability.rb | 33 +-------------------------------- 1 file changed, 1 insertion(+), 32 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index 323597c8888..c5392379b32 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -73,7 +73,6 @@ class Ability def abilities_by_subject_class(user:, subject:) case subject - when PersonalSnippet then personal_snippet_abilities(user, subject) when Group then group_abilities(user, subject) when Namespace then namespace_abilities(user, subject) when GroupMember then group_member_abilities(user, subject) @@ -87,11 +86,7 @@ class Ability # List of possible abilities for anonymous user def anonymous_abilities(subject) - if subject.is_a?(PersonalSnippet) - anonymous_personal_snippet_abilities(subject) - elsif subject.is_a?(ProjectSnippet) - anonymous_project_snippet_abilities(subject) - elsif subject.respond_to?(:project) + if subject.respond_to?(:project) ProjectPolicy.abilities(nil, subject.project) elsif subject.is_a?(Group) || subject.respond_to?(:group) anonymous_group_abilities(subject) @@ -153,14 +148,6 @@ class Ability rules end - def anonymous_personal_snippet_abilities(snippet) - if snippet.public? - [:read_personal_snippet] - else - [] - end - end - def anonymous_user_abilities [:read_user] unless restricted_public_level? end @@ -232,24 +219,6 @@ class Ability rules.flatten end - def personal_snippet_abilities(user, snippet) - rules = [] - - if snippet.author == user - rules += [ - :read_personal_snippet, - :update_personal_snippet, - :admin_personal_snippet - ] - end - - if snippet.public? || (snippet.internal? && !user.external?) - rules << :read_personal_snippet - end - - rules - end - def group_member_abilities(user, subject) rules = [] target_user = subject.user -- cgit v1.2.1 From ccfa032ebc101339c1c0842d0fbeb5b555db9278 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 16 Aug 2016 16:28:47 -0700 Subject: port groups --- app/models/ability.rb | 39 +++----------------------------------- app/policies/group_policy.rb | 45 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 36 deletions(-) create mode 100644 app/policies/group_policy.rb diff --git a/app/models/ability.rb b/app/models/ability.rb index c5392379b32..2360bf3d46c 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -73,7 +73,6 @@ class Ability def abilities_by_subject_class(user:, subject:) case subject - when Group then group_abilities(user, subject) when Namespace then namespace_abilities(user, subject) when GroupMember then group_member_abilities(user, subject) when ProjectMember then project_member_abilities(user, subject) @@ -88,8 +87,8 @@ class Ability def anonymous_abilities(subject) if subject.respond_to?(:project) ProjectPolicy.abilities(nil, subject.project) - elsif subject.is_a?(Group) || subject.respond_to?(:group) - anonymous_group_abilities(subject) + elsif subject.respond_to?(:group) + GroupPolicy.abilities(nil, subject.group) elsif subject.is_a?(User) anonymous_user_abilities else @@ -164,38 +163,6 @@ class Ability ProjectPolicy.abilities(user, project).to_a end - def group_abilities(user, group) - rules = [] - rules << :read_group if can_read_group?(user, group) - - owner = user.admin? || group.has_owner?(user) - master = owner || group.has_master?(user) - - # Only group masters and group owners can create new projects - if master - rules += [ - :create_projects, - :admin_milestones - ] - end - - # Only group owner and administrators can admin group - if owner - rules += [ - :admin_group, - :admin_namespace, - :admin_group_member, - :change_visibility_level - ] - end - - if group.public? || (group.internal? && !user.external?) - rules << :request_access if group.request_access_enabled && group.users.exclude?(user) - end - - rules.flatten - end - def can_read_group?(user, group) return true if user.admin? return true if group.public? @@ -225,7 +192,7 @@ class Ability group = subject.group unless group.last_owner?(target_user) - can_manage = group_abilities(user, group).include?(:admin_group_member) + can_manage = allowed?(user, :admin_group_member, group) if can_manage rules << :update_group_member diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb new file mode 100644 index 00000000000..97ff6233968 --- /dev/null +++ b/app/policies/group_policy.rb @@ -0,0 +1,45 @@ +class GroupPolicy < BasePolicy + def rules + can! :read_group if @subject.public? + return unless @user + + globally_viewable = @subject.public? || (@subject.internal? && !@user.external?) + member = @subject.users.include?(@user) + owner = @user.admin? || @subject.has_owner?(@user) + master = owner || @subject.has_master?(@user) + + can_read = false + can_read ||= globally_viewable + can_read ||= member + can_read ||= @user.admin? + can_read ||= GroupProjectsFinder.new(@subject).execute(@user).any? + can! :read_group if can_read + + # Only group masters and group owners can create new projects + if master + can! :create_projects + can! :admin_milestones + end + + # Only group owner and administrators can admin group + if owner + can! :admin_group + can! :admin_namespace + can! :admin_group_member + can! :change_visibility_level + end + + if globally_viewable && @subject.request_access_enabled && !member + can! :request_access + end + end + + def can_read_group? + return true if @subject.public? + return true if @user.admin? + return true if @subject.internal? && !@user.external? + return true if @subject.users.include?(@user) + + GroupProjectsFinder.new(@subject).execute(@user).any? + end +end -- cgit v1.2.1 From 2944022835d872b472d8691082ef67aa3057d2b4 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 16 Aug 2016 16:29:10 -0700 Subject: trim more dead code --- app/models/ability.rb | 53 +-------------------------------------------------- 1 file changed, 1 insertion(+), 52 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index 2360bf3d46c..794fb1223e3 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -96,57 +96,6 @@ class Ability end end - def anonymous_project_abilities(subject) - project = if subject.is_a?(Project) - subject - else - subject.project - end - - if project && project.public? - rules = [ - :read_project, - :read_board, - :read_list, - :read_wiki, - :read_label, - :read_milestone, - :read_project_snippet, - :read_project_member, - :read_merge_request, - :read_note, - :read_pipeline, - :read_commit_status, - :read_container_image, - :download_code - ] - - # Allow to read builds by anonymous user if guests are allowed - rules << :read_build if project.public_builds? - - # Allow to read issues by anonymous user if issue is not confidential - rules << :read_issue unless subject.is_a?(Issue) && subject.confidential? - - rules - project_disabled_features_rules(project) - else - [] - end - end - - def anonymous_group_abilities(subject) - rules = [] - - group = if subject.is_a?(Group) - subject - else - subject.group - end - - rules << :read_group if group.public? - - rules - end - def anonymous_user_abilities [:read_user] unless restricted_public_level? end @@ -211,7 +160,7 @@ class Ability project = subject.project unless target_user == project.owner - can_manage = project_abilities(user, project).include?(:admin_project_member) + can_manage = allowed?(user, :admin_project_member, project) if can_manage rules << :update_project_member -- cgit v1.2.1 From 9a0ea1350131368b9b723f1a9581bbfffe7c43f8 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 16 Aug 2016 16:29:19 -0700 Subject: factor in global permissions --- app/policies/base_policy.rb | 4 ++-- app/policies/global_policy.rb | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 app/policies/global_policy.rb diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb index 12f60d8f76e..5a5b99c81c8 100644 --- a/app/policies/base_policy.rb +++ b/app/policies/base_policy.rb @@ -4,6 +4,8 @@ class BasePolicy end def self.class_for(subject) + return GlobalPolicy if subject.nil? + subject.class.ancestors.each do |klass| next unless klass.name @@ -59,8 +61,6 @@ class BasePolicy private def collect_rules(&b) - return Set.new if @subject.nil? - @can = Set.new @cannot = Set.new yield diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb new file mode 100644 index 00000000000..94a2906444a --- /dev/null +++ b/app/policies/global_policy.rb @@ -0,0 +1,7 @@ +class GlobalPolicy < BasePolicy + def rules + return unless @user + can! :create_group if @user.can_create_group + can! :read_users_list + end +end -- cgit v1.2.1 From 29059c2e9c7be418d2a99a136934c6d9cca5fccd Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 16 Aug 2016 16:46:35 -0700 Subject: add personal snippets and project members --- app/policies/personal_snippet_policy.rb | 16 ++++++++++++++++ app/policies/project_member_policy.rb | 22 ++++++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 app/policies/personal_snippet_policy.rb create mode 100644 app/policies/project_member_policy.rb diff --git a/app/policies/personal_snippet_policy.rb b/app/policies/personal_snippet_policy.rb new file mode 100644 index 00000000000..46c5aa1a5be --- /dev/null +++ b/app/policies/personal_snippet_policy.rb @@ -0,0 +1,16 @@ +class PersonalSnippetPolicy < BasePolicy + def rules + can! :read_personal_snippet if @subject.public? + return unless @user + + if @subject.author == @user + can! :read_personal_snippet + can! :update_personal_snippet + can! :admin_personal_snippet + end + + if @subject.internal? && !@user.external? + can! :read_personal_snippet + end + end +end diff --git a/app/policies/project_member_policy.rb b/app/policies/project_member_policy.rb new file mode 100644 index 00000000000..1c038dddd4b --- /dev/null +++ b/app/policies/project_member_policy.rb @@ -0,0 +1,22 @@ +class ProjectMemberPolicy < BasePolicy + def rules + # anonymous users have no abilities here + return unless @user + + target_user = @subject.user + project = @subject.project + + return if target_user == project.owner + + can_manage = Ability.allowed?(@user, :admin_project_member, project) + + if can_manage + can! :update_project_member + can! :destroy_project_member + end + + if @user == target_user + can! :destroy_project_member + end + end +end -- cgit v1.2.1 From 5019185edd7718b262eb5ae94f21763f230f0557 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Thu, 18 Aug 2016 09:52:35 -0700 Subject: port runners, namespaces, group/project_members --- app/models/ability.rb | 58 ------------------------------------- app/policies/ci/runner_policy.rb | 13 +++++++++ app/policies/group_member_policy.rb | 19 ++++++++++++ app/policies/namespace_policy.rb | 10 +++++++ 4 files changed, 42 insertions(+), 58 deletions(-) create mode 100644 app/policies/ci/runner_policy.rb create mode 100644 app/policies/group_member_policy.rb create mode 100644 app/policies/namespace_policy.rb diff --git a/app/models/ability.rb b/app/models/ability.rb index 794fb1223e3..7c4210f0706 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -73,12 +73,8 @@ class Ability def abilities_by_subject_class(user:, subject:) case subject - when Namespace then namespace_abilities(user, subject) - when GroupMember then group_member_abilities(user, subject) - when ProjectMember then project_member_abilities(user, subject) when User then user_abilities when ExternalIssue, Deployment, Environment then project_abilities(user, subject.project) - when Ci::Runner then runner_abilities(user, subject) else [] end + global_abilities(user) end @@ -112,48 +108,6 @@ class Ability ProjectPolicy.abilities(user, project).to_a end - def can_read_group?(user, group) - return true if user.admin? - return true if group.public? - return true if group.internal? && !user.external? - return true if group.users.include?(user) - - GroupProjectsFinder.new(group).execute(user).any? - end - - def namespace_abilities(user, namespace) - rules = [] - - # Only namespace owner and administrators can admin it - if namespace.owner == user || user.admin? - rules += [ - :create_projects, - :admin_namespace - ] - end - - rules.flatten - end - - def group_member_abilities(user, subject) - rules = [] - target_user = subject.user - group = subject.group - - unless group.last_owner?(target_user) - can_manage = allowed?(user, :admin_group_member, group) - - if can_manage - rules << :update_group_member - rules << :destroy_group_member - elsif user == target_user - rules << :destroy_group_member - end - end - - rules - end - def project_member_abilities(user, subject) rules = [] target_user = subject.user @@ -182,18 +136,6 @@ class Ability rules end - def runner_abilities(user, runner) - if user.is_admin? - [:assign_runner] - elsif runner.is_shared? || runner.locked? - [] - elsif user.ci_authorized_runners.include?(runner) - [:assign_runner] - else - [] - end - end - def user_abilities [:read_user] end diff --git a/app/policies/ci/runner_policy.rb b/app/policies/ci/runner_policy.rb new file mode 100644 index 00000000000..7edd383530d --- /dev/null +++ b/app/policies/ci/runner_policy.rb @@ -0,0 +1,13 @@ +module Ci + class RunnerPolicy < BasePolicy + def rules + return unless @user + + can! :assign_runner if @user.is_admin? + + return if @subject.is_shared? || @subject.locked? + + can! :assign_runner if @user.ci_authorized_runners.include?(@subject) + end + end +end diff --git a/app/policies/group_member_policy.rb b/app/policies/group_member_policy.rb new file mode 100644 index 00000000000..62335527654 --- /dev/null +++ b/app/policies/group_member_policy.rb @@ -0,0 +1,19 @@ +class GroupMemberPolicy < BasePolicy + def rules + return unless @user + + target_user = @subject.user + group = @subject.group + + return if group.last_owner?(target_user) + + can_manage = Ability.allowed?(@user, :admin_group_member, group) + + if can_manage + can! :update_group_member + can! :destroy_group_member + elsif @user == target_user + can! :destroy_group_member + end + end +end diff --git a/app/policies/namespace_policy.rb b/app/policies/namespace_policy.rb new file mode 100644 index 00000000000..29bb357e00a --- /dev/null +++ b/app/policies/namespace_policy.rb @@ -0,0 +1,10 @@ +class NamespacePolicy < BasePolicy + def rules + return unless @user + + if @subject.owner == @user || @user.admin? + can! :create_projects + can! :admin_namespace + end + end +end -- cgit v1.2.1 From a340829c42617b40696408c3097d6476970e8b87 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Thu, 18 Aug 2016 09:59:17 -0700 Subject: port UserPolicy --- app/models/ability.rb | 11 ----------- app/policies/user_policy.rb | 11 +++++++++++ 2 files changed, 11 insertions(+), 11 deletions(-) create mode 100644 app/policies/user_policy.rb diff --git a/app/models/ability.rb b/app/models/ability.rb index 7c4210f0706..fe171cd1a8b 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -73,7 +73,6 @@ class Ability def abilities_by_subject_class(user:, subject:) case subject - when User then user_abilities when ExternalIssue, Deployment, Environment then project_abilities(user, subject.project) else [] end + global_abilities(user) @@ -85,17 +84,11 @@ class Ability ProjectPolicy.abilities(nil, subject.project) elsif subject.respond_to?(:group) GroupPolicy.abilities(nil, subject.group) - elsif subject.is_a?(User) - anonymous_user_abilities else [] end end - def anonymous_user_abilities - [:read_user] unless restricted_public_level? - end - def global_abilities(user) rules = [] rules << :create_group if user.can_create_group @@ -136,10 +129,6 @@ class Ability rules end - def user_abilities - [:read_user] - end - def restricted_public_level? current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC) end diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb new file mode 100644 index 00000000000..03a2499e263 --- /dev/null +++ b/app/policies/user_policy.rb @@ -0,0 +1,11 @@ +class UserPolicy < BasePolicy + include Gitlab::CurrentSettings + + def rules + can! :read_user if @user || !restricted_public_level? + end + + def restricted_public_level? + current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC) + end +end -- cgit v1.2.1 From 5b7edc74b65f6855d3744ba600f3972c8cbb5894 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Thu, 18 Aug 2016 10:38:25 -0700 Subject: use the cached abilities in #delegate! --- app/policies/base_policy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb index 5a5b99c81c8..6d38e2eaa73 100644 --- a/app/policies/base_policy.rb +++ b/app/policies/base_policy.rb @@ -43,7 +43,7 @@ class BasePolicy end def delegate!(new_subject) - @can.merge(BasePolicy.class_for(new_subject).abilities(@user, new_subject)) + @can.merge(Ability.allowed(@user, new_subject)) end def can?(rule) -- cgit v1.2.1 From 06ba2602c59e5f6627d892ed9fdb2dafade5768b Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Thu, 18 Aug 2016 10:39:49 -0700 Subject: take the dive - only use abilities from Policies --- app/models/ability.rb | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index fe171cd1a8b..b57ada715df 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -61,14 +61,7 @@ class Ability private def uncached_allowed(user, subject) - policy_class = BasePolicy.class_for(subject) rescue nil - return policy_class.abilities(user, subject) if policy_class - - return anonymous_abilities(subject) if user.nil? - return [] unless user.is_a?(User) - return [] if user.blocked? - - abilities_by_subject_class(user: user, subject: subject) + BasePolicy.class_for(subject).abilities(user, subject) end def abilities_by_subject_class(user:, subject:) -- cgit v1.2.1 From 2b26270ab7c489ebde4aac835f8e7307dc7a7441 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Thu, 18 Aug 2016 10:40:39 -0700 Subject: add Deployment, Environment, and ExternalIssue policies --- app/policies/deployment_policy.rb | 5 +++++ app/policies/environment_policy.rb | 5 +++++ app/policies/external_issue_policy.rb | 5 +++++ 3 files changed, 15 insertions(+) create mode 100644 app/policies/deployment_policy.rb create mode 100644 app/policies/environment_policy.rb create mode 100644 app/policies/external_issue_policy.rb diff --git a/app/policies/deployment_policy.rb b/app/policies/deployment_policy.rb new file mode 100644 index 00000000000..163d070ff90 --- /dev/null +++ b/app/policies/deployment_policy.rb @@ -0,0 +1,5 @@ +class DeploymentPolicy < BasePolicy + def rules + delegate! @subject.project + end +end diff --git a/app/policies/environment_policy.rb b/app/policies/environment_policy.rb new file mode 100644 index 00000000000..f4219569161 --- /dev/null +++ b/app/policies/environment_policy.rb @@ -0,0 +1,5 @@ +class EnvironmentPolicy < BasePolicy + def rules + delegate! @subject.project + end +end diff --git a/app/policies/external_issue_policy.rb b/app/policies/external_issue_policy.rb new file mode 100644 index 00000000000..d9e28bd107a --- /dev/null +++ b/app/policies/external_issue_policy.rb @@ -0,0 +1,5 @@ +class ExternalIssuePolicy < BasePolicy + def rules + delegate! @subject.project + end +end -- cgit v1.2.1 From 2bdcef4d672121a387fca6da720d333dda8f7af6 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Thu, 18 Aug 2016 16:12:32 -0700 Subject: use a nil subject when we want to check global abilities --- lib/api/groups.rb | 2 +- lib/api/helpers.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/api/groups.rb b/lib/api/groups.rb index 9d8b8d737a9..f981ec0dbfe 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -30,7 +30,7 @@ module API # Example Request: # POST /groups post do - authorize! :create_group, current_user + authorize! :create_group required_attributes! [:name, :path] attrs = attributes_for_keys [:name, :path, :description, :visibility_level] diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index fdb70af694d..6a20ba95a79 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -129,7 +129,7 @@ module API forbidden! unless current_user.is_admin? end - def authorize!(action, subject) + def authorize!(action, subject = nil) forbidden! unless can?(current_user, action, subject) end -- cgit v1.2.1 From 6070145bebad0a8284b4fe4bb7a1e2b97f03ab1b Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Mon, 22 Aug 2016 17:45:15 -0700 Subject: test if we can :read_group the group, not the namespace --- app/controllers/namespaces_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/namespaces_controller.rb b/app/controllers/namespaces_controller.rb index 5a94dcb0dbd..83eec1bf4a2 100644 --- a/app/controllers/namespaces_controller.rb +++ b/app/controllers/namespaces_controller.rb @@ -14,7 +14,7 @@ class NamespacesController < ApplicationController if user redirect_to user_path(user) - elsif group && can?(current_user, :read_group, namespace) + elsif group && can?(current_user, :read_group, group) redirect_to group_path(group) elsif current_user.nil? authenticate_user! -- cgit v1.2.1 From 35779223a69c22806bbb48d70086c7fb9a23f513 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Mon, 22 Aug 2016 17:45:31 -0700 Subject: special-case blocked users --- app/policies/base_policy.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb index 6d38e2eaa73..a6fd9786ae7 100644 --- a/app/policies/base_policy.rb +++ b/app/policies/base_policy.rb @@ -30,6 +30,7 @@ class BasePolicy end def abilities + return [] if @user && @user.blocked? return anonymous_abilities if @user.nil? collect_rules { rules } end -- cgit v1.2.1 From b3b7fb1fe7b876487b1464aa5779bacec7276742 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 23 Aug 2016 17:56:41 -0700 Subject: remove the rest of the dead code --- app/models/ability.rb | 74 --------------------------------------------------- 1 file changed, 74 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index b57ada715df..8ccbb9bee9c 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -63,79 +63,5 @@ class Ability def uncached_allowed(user, subject) BasePolicy.class_for(subject).abilities(user, subject) end - - def abilities_by_subject_class(user:, subject:) - case subject - when ExternalIssue, Deployment, Environment then project_abilities(user, subject.project) - else [] - end + global_abilities(user) - end - - # List of possible abilities for anonymous user - def anonymous_abilities(subject) - if subject.respond_to?(:project) - ProjectPolicy.abilities(nil, subject.project) - elsif subject.respond_to?(:group) - GroupPolicy.abilities(nil, subject.group) - else - [] - end - end - - def global_abilities(user) - rules = [] - rules << :create_group if user.can_create_group - rules << :read_users_list - rules - end - - def project_abilities(user, project) - # temporary patch, deleteme before merge - ProjectPolicy.abilities(user, project).to_a - end - - def project_member_abilities(user, subject) - rules = [] - target_user = subject.user - project = subject.project - - unless target_user == project.owner - can_manage = allowed?(user, :admin_project_member, project) - - if can_manage - rules << :update_project_member - rules << :destroy_project_member - elsif user == target_user - rules << :destroy_project_member - end - end - - rules - end - - def filter_build_abilities(rules) - # If we can't read build we should also not have that - # ability when looking at this in context of commit_status - %w(read create update admin).each do |rule| - rules.delete(:"#{rule}_commit_status") unless rules.include?(:"#{rule}_build") - end - rules - end - - def restricted_public_level? - current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC) - end - - def filter_confidential_issues_abilities(user, issue, rules) - return rules if user.admin? || !issue.confidential? - - unless issue.author == user || issue.assignee == user || issue.project.team.member?(user, Gitlab::Access::REPORTER) - rules.delete(:admin_issue) - rules.delete(:read_issue) - rules.delete(:update_issue) - end - - rules - end end end -- cgit v1.2.1 From 57def53c84091a56f3a2443d214fe80f2c026d00 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 30 Aug 2016 11:09:21 -0700 Subject: factor out a RuleSet so that `delegate!` retains @cannot --- app/models/ability.rb | 2 +- app/policies/base_policy.rb | 58 ++++++++++++++++++++++++++++++++++++++------- 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index 8ccbb9bee9c..fa8f8bc3a5f 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -55,7 +55,7 @@ class Ability user_key = user ? user.id : 'anonymous' subject_key = subject ? "#{subject.class.name}/#{subject.id}" : 'global' key = "/ability/#{user_key}/#{subject_key}" - RequestStore[key] ||= Set.new(uncached_allowed(user, subject)).freeze + RequestStore[key] ||= uncached_allowed(user, subject).freeze end private diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb index a6fd9786ae7..6a1a7d75ee6 100644 --- a/app/policies/base_policy.rb +++ b/app/policies/base_policy.rb @@ -1,4 +1,47 @@ class BasePolicy + class RuleSet + attr_reader :can_set, :cannot_set + def initialize(can_set, cannot_set) + @can_set = can_set + @cannot_set = cannot_set + end + + def self.empty + new(Set.new, Set.new) + end + + def can?(ability) + @can_set.include?(ability) && !@cannot_set.include?(ability) + end + + def include?(ability) + can?(ability) + end + + def to_set + @can_set - @cannot_set + end + + def merge(other) + @can_set.merge(other.can_set) + @cannot_set.merge(other.cannot_set) + end + + def can!(*abilities) + @can_set.merge(abilities) + end + + def cannot!(*abilities) + @cannot_set.merge(abilities) + end + + def freeze + @can_set.freeze + @cannot_set.freeze + super + end + end + def self.abilities(user, subject) new(user, subject).abilities end @@ -30,7 +73,7 @@ class BasePolicy end def abilities - return [] if @user && @user.blocked? + return RuleSet.empty if @user && @user.blocked? return anonymous_abilities if @user.nil? collect_rules { rules } end @@ -44,27 +87,26 @@ class BasePolicy end def delegate!(new_subject) - @can.merge(Ability.allowed(@user, new_subject)) + @rule_set.merge(Ability.allowed(@user, new_subject)) end def can?(rule) - @can.include?(rule) && !@cannot.include?(rule) + @rule_set.can?(rule) end def can!(*rules) - @can.merge(rules) + @rule_set.can!(*rules) end def cannot!(*rules) - @cannot.merge(rules) + @rule_set.cannot!(*rules) end private def collect_rules(&b) - @can = Set.new - @cannot = Set.new + @rule_set = RuleSet.empty yield - @can - @cannot + @rule_set end end -- cgit v1.2.1 From 71765536d0c29e64eb24ce50da9d5fdfc63f9e78 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 30 Aug 2016 11:10:33 -0700 Subject: move the rules method to the top #cosmetic --- app/policies/project_policy.rb | 54 +++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 4380b00d962..8a1148dece4 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -1,4 +1,31 @@ class ProjectPolicy < BasePolicy + def rules + team_access!(user) + + owner = user.admin? || + project.owner == user || + (project.group && project.group.has_owner?(user)) + + owner_access! if owner + + if project.public? || (project.internal? && !user.external?) + guest_access! + public_access! + + # Allow to read builds for internal projects + can! :read_build if project.public_builds? + + if project.request_access_enabled && + !(owner || project.team.member?(user) || project_group_member?(user)) + can! :request_access + end + end + + archived_access! if project.archived? + + disabled_features! + end + def project @subject end @@ -158,33 +185,6 @@ class ProjectPolicy < BasePolicy end end - def rules - team_access!(user) - - owner = user.admin? || - project.owner == user || - (project.group && project.group.has_owner?(user)) - - owner_access! if owner - - if project.public? || (project.internal? && !user.external?) - guest_access! - public_access! - - # Allow to read builds for internal projects - can! :read_build if project.public_builds? - - if project.request_access_enabled && - !(owner || project.team.member?(user) || project_group_member?(user)) - can! :request_access - end - end - - archived_access! if project.archived? - - disabled_features! - end - def anonymous_rules return unless project.public? -- cgit v1.2.1 From d7bd20099bb9df677cf272a5b211fbe9c330c619 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 30 Aug 2016 11:12:09 -0700 Subject: use a more compact style for access policies --- app/policies/project_policy.rb | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 8a1148dece4..54f5f95cd65 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -130,17 +130,10 @@ class ProjectPolicy < BasePolicy def team_access!(user) access = project.team.max_member_access(user.id) - return if access < Gitlab::Access::GUEST - guest_access! - - return if access < Gitlab::Access::REPORTER - reporter_access! - - return if access < Gitlab::Access::DEVELOPER - developer_access! - - return if access < Gitlab::Access::MASTER - master_access! + guest_access! if access >= Gitlab::Access::GUEST + reporter_access! if access >= Gitlab::Access::REPORTER + developer_access! if access >= Gitlab::Access::DEVELOPER + master_access! if access >= Gitlab::Access::MASTER end def archived_access! -- cgit v1.2.1 From fb2979260998a1b7f35d688ef788d34099322b84 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 30 Aug 2016 11:13:29 -0700 Subject: use || in place of `or` --- app/policies/project_policy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 54f5f95cd65..0e933d00904 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -153,7 +153,7 @@ class ProjectPolicy < BasePolicy cannot!(*named_abilities(:merge_request)) end - unless project.issues_enabled or project.merge_requests_enabled + unless project.issues_enabled || project.merge_requests_enabled cannot!(*named_abilities(:label)) cannot!(*named_abilities(:milestone)) end -- cgit v1.2.1 From b7d300001319e38ddb6e57764d51d5d838689ecd Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 30 Aug 2016 11:14:07 -0700 Subject: line break after guard clause --- app/policies/global_policy.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb index 94a2906444a..3c2fbe6b56b 100644 --- a/app/policies/global_policy.rb +++ b/app/policies/global_policy.rb @@ -1,6 +1,7 @@ class GlobalPolicy < BasePolicy def rules return unless @user + can! :create_group if @user.can_create_group can! :read_users_list end -- cgit v1.2.1 From 482795a90830a4a74b675ef7afc266ca292f6655 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 30 Aug 2016 11:42:23 -0700 Subject: implement RuleSet#size for tests --- app/policies/base_policy.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb index 6a1a7d75ee6..cc82793b716 100644 --- a/app/policies/base_policy.rb +++ b/app/policies/base_policy.rb @@ -6,6 +6,10 @@ class BasePolicy @cannot_set = cannot_set end + def size + to_set.size + end + def self.empty new(Set.new, Set.new) end -- cgit v1.2.1 From 78eabebedc2cb849dd95e5e7e9dff9f1d24f5ebe Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 30 Aug 2016 12:34:28 -0700 Subject: don't use a deprecated api in ability_spec --- spec/models/ability_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index c9e6a334c67..b05510342bc 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -222,12 +222,12 @@ describe Ability, lib: true do describe '.project_disabled_features_rules' do let(:project) { build(:project) } - subject { described_class.project_disabled_features_rules(project) } + subject { described_class.allowed(project.owner, project) } context 'wiki named abilities' do it 'disables wiki abilities if the project has no wiki' do expect(project).to receive(:has_wiki?).and_return(false) - expect(subject).to include(:read_wiki, :create_wiki, :update_wiki, :admin_wiki) + expect(subject).not_to include(:read_wiki, :create_wiki, :update_wiki, :admin_wiki) end end end -- cgit v1.2.1 From bc0a513f624f8e06839b4c3baa5fe10f71284ef2 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 30 Aug 2016 15:55:28 -0700 Subject: s/NB:/NOTE:/ --- app/policies/base_policy.rb | 2 +- app/policies/project_policy.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb index cc82793b716..118c100ca11 100644 --- a/app/policies/base_policy.rb +++ b/app/policies/base_policy.rb @@ -59,7 +59,7 @@ class BasePolicy begin policy_class = "#{klass.name}Policy".constantize - # NB: the < operator here tests whether policy_class + # NOTE: the < operator here tests whether policy_class # inherits from BasePolicy return policy_class if policy_class < BasePolicy rescue NameError diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 0e933d00904..15a9f2f0dca 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -196,7 +196,7 @@ class ProjectPolicy < BasePolicy can! :read_container_image can! :download_code - # NB: may be overridden by IssuePolicy + # NOTE: may be overridden by IssuePolicy can! :read_issue # Allow to read builds by anonymous user if guests are allowed -- cgit v1.2.1 From b105dc791df07bab0d5349c63cb73c7b3ee8212c Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 30 Aug 2016 15:55:37 -0700 Subject: newline before default return --- app/policies/issue_policy.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/policies/issue_policy.rb b/app/policies/issue_policy.rb index 08538861364..bd1811a3c54 100644 --- a/app/policies/issue_policy.rb +++ b/app/policies/issue_policy.rb @@ -22,6 +22,7 @@ class IssuePolicy < IssuablePolicy return true if @subject.author == @user return true if @subject.assignee == @user return true if @subject.project.team.member?(@user, Gitlab::Access::REPORTER) + false end end -- cgit v1.2.1 From 036cc8c27e8340a3eed63444bd3f42f86037f350 Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Fri, 15 Jul 2016 16:21:53 +0200 Subject: API: Expose issue#confidential --- CHANGELOG | 1 + doc/api/issues.md | 32 +++++++++++++------- lib/api/entities.rb | 1 + lib/api/issues.rb | 14 +++++++-- spec/requests/api/issues_spec.rb | 65 +++++++++++++++++++++++++++++++++++++++- 5 files changed, 100 insertions(+), 13 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 4e963702b8e..ec76455e837 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -14,6 +14,7 @@ v 8.12.0 (unreleased) - Add hover color to emoji icon (ClemMakesApps) - Fix branches page dropdown sort alignment (ClemMakesApps) - Add white background for no readme container (ClemMakesApps) + - API: Expose issue confidentiality flag. (Robert Schilling) - Optimistic locking for Issues and Merge Requests (title and description overriding prevention) - Add `wiki_page_events` to project hook APIs (Ben Boeckel) - Remove Gitorious import diff --git a/doc/api/issues.md b/doc/api/issues.md index b194799ccbf..eed0d2fce51 100644 --- a/doc/api/issues.md +++ b/doc/api/issues.md @@ -80,7 +80,8 @@ Example response: "subscribed" : false, "user_notes_count": 1, "due_date": "2016-07-22", - "web_url": "http://example.com/example/example/issues/6" + "web_url": "http://example.com/example/example/issues/6", + "confidential": false } ] ``` @@ -158,7 +159,8 @@ Example response: "subscribed" : false, "user_notes_count": 1, "due_date": null, - "web_url": "http://example.com/example/example/issues/1" + "web_url": "http://example.com/example/example/issues/1", + "confidential": false } ] ``` @@ -238,7 +240,8 @@ Example response: "subscribed" : false, "user_notes_count": 1, "due_date": "2016-07-22", - "web_url": "http://example.com/example/example/issues/1" + "web_url": "http://example.com/example/example/issues/1", + "confidential": false } ] ``` @@ -303,7 +306,8 @@ Example response: "subscribed": false, "user_notes_count": 1, "due_date": null, - "web_url": "http://example.com/example/example/issues/1" + "web_url": "http://example.com/example/example/issues/1", + "confidential": false } ``` @@ -324,6 +328,7 @@ POST /projects/:id/issues | `id` | integer | yes | The ID of a project | | `title` | string | yes | The title of an issue | | `description` | string | no | The description of an issue | +| `confidential` | boolean | no | Set an issue to be confidential. Default is `false`. | | `assignee_id` | integer | no | The ID of a user to assign issue | | `milestone_id` | integer | no | The ID of a milestone to assign issue | | `labels` | string | no | Comma-separated label names for an issue | @@ -362,7 +367,8 @@ Example response: "subscribed" : true, "user_notes_count": 0, "due_date": null, - "web_url": "http://example.com/example/example/issues/14" + "web_url": "http://example.com/example/example/issues/14", + "confidential": false } ``` @@ -385,6 +391,7 @@ PUT /projects/:id/issues/:issue_id | `issue_id` | integer | yes | The ID of a project's issue | | `title` | string | no | The title of an issue | | `description` | string | no | The description of an issue | +| `confidential` | boolean | no | Updates an issue to be confidential | | `assignee_id` | integer | no | The ID of a user to assign the issue to | | `milestone_id` | integer | no | The ID of a milestone to assign the issue to | | `labels` | string | no | Comma-separated label names for an issue | @@ -424,7 +431,8 @@ Example response: "subscribed" : true, "user_notes_count": 0, "due_date": "2016-07-22", - "web_url": "http://example.com/example/example/issues/15" + "web_url": "http://example.com/example/example/issues/15", + "confidential": false } ``` @@ -503,7 +511,8 @@ Example response: "web_url": "https://gitlab.example.com/u/solon.cremin" }, "due_date": null, - "web_url": "http://example.com/example/example/issues/11" + "web_url": "http://example.com/example/example/issues/11", + "confidential": false } ``` @@ -559,7 +568,8 @@ Example response: "web_url": "https://gitlab.example.com/u/solon.cremin" }, "due_date": null, - "web_url": "http://example.com/example/example/issues/11" + "web_url": "http://example.com/example/example/issues/11", + "confidential": false } ``` @@ -616,7 +626,8 @@ Example response: }, "subscribed": false, "due_date": null, - "web_url": "http://example.com/example/example/issues/12" + "web_url": "http://example.com/example/example/issues/12", + "confidential": false } ``` @@ -704,7 +715,8 @@ Example response: "upvotes": 0, "downvotes": 0, "due_date": null, - "web_url": "http://example.com/example/example/issues/110" + "web_url": "http://example.com/example/example/issues/110", + "confidential": false }, "target_url": "https://gitlab.example.com/gitlab-org/gitlab-ci/issues/10", "body": "Vel voluptas atque dicta mollitia adipisci qui at.", diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 4335e3055ef..e3a8ff6de80 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -211,6 +211,7 @@ module API expose :user_notes_count expose :upvotes, :downvotes expose :due_date + expose :confidential expose :web_url do |issue, options| Gitlab::UrlBuilder.build(issue) diff --git a/lib/api/issues.rb b/lib/api/issues.rb index d0bc7243e54..556684187d8 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -140,12 +140,13 @@ module API # labels (optional) - The labels of an issue # created_at (optional) - Date time string, ISO 8601 formatted # due_date (optional) - Date time string in the format YEAR-MONTH-DAY + # confidential (optional) - Boolean parameter if the issue should be confidential # Example Request: # POST /projects/:id/issues post ':id/issues' do required_attributes! [:title] - keys = [:title, :description, :assignee_id, :milestone_id, :due_date] + keys = [:title, :description, :assignee_id, :milestone_id, :due_date, :confidential] keys << :created_at if current_user.admin? || user_project.owner == current_user attrs = attributes_for_keys(keys) @@ -156,6 +157,10 @@ module API attrs[:labels] = params[:labels] if params[:labels] + # Convert and filter out invalid confidential flags + attrs['confidential'] = to_boolean(attrs['confidential']) + attrs.delete('confidential') if attrs['confidential'].nil? + issue = ::Issues::CreateService.new(user_project, current_user, attrs.merge(request: request, api: true)).execute if issue.spam? @@ -182,12 +187,13 @@ module API # state_event (optional) - The state event of an issue (close|reopen) # updated_at (optional) - Date time string, ISO 8601 formatted # due_date (optional) - Date time string in the format YEAR-MONTH-DAY + # confidential (optional) - Boolean parameter if the issue should be confidential # Example Request: # PUT /projects/:id/issues/:issue_id put ':id/issues/:issue_id' do issue = user_project.issues.find(params[:issue_id]) authorize! :update_issue, issue - keys = [:title, :description, :assignee_id, :milestone_id, :state_event, :due_date] + keys = [:title, :description, :assignee_id, :milestone_id, :state_event, :due_date, :confidential] keys << :updated_at if current_user.admin? || user_project.owner == current_user attrs = attributes_for_keys(keys) @@ -198,6 +204,10 @@ module API attrs[:labels] = params[:labels] if params[:labels] + # Convert and filter out invalid confidential flags + attrs['confidential'] = to_boolean(attrs['confidential']) + attrs.delete('confidential') if attrs['confidential'].nil? + issue = ::Issues::UpdateService.new(user_project, current_user, attrs).execute(issue) if issue.valid? diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 3362a88d798..47344a13b5e 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -405,6 +405,7 @@ describe API::API, api: true do expect(json_response['milestone']).to be_a Hash expect(json_response['assignee']).to be_a Hash expect(json_response['author']).to be_a Hash + expect(json_response['confidential']).to be_falsy end it "returns a project issue by id" do @@ -470,13 +471,51 @@ describe API::API, api: true do end describe "POST /projects/:id/issues" do - it "creates a new project issue" do + it 'creates a new project issue' do post api("/projects/#{project.id}/issues", user), title: 'new issue', labels: 'label, label2' + expect(response).to have_http_status(201) expect(json_response['title']).to eq('new issue') expect(json_response['description']).to be_nil expect(json_response['labels']).to eq(['label', 'label2']) + expect(json_response['confidential']).to be_falsy + end + + it 'creates a new confidential project issue' do + post api("/projects/#{project.id}/issues", user), + title: 'new issue', confidential: true + + expect(response).to have_http_status(201) + expect(json_response['title']).to eq('new issue') + expect(json_response['confidential']).to be_truthy + end + + it 'creates a new confidential project issue with a different param' do + post api("/projects/#{project.id}/issues", user), + title: 'new issue', confidential: 'y' + + expect(response).to have_http_status(201) + expect(json_response['title']).to eq('new issue') + expect(json_response['confidential']).to be_truthy + end + + it 'creates a public issue when confidential param is false' do + post api("/projects/#{project.id}/issues", user), + title: 'new issue', confidential: false + + expect(response).to have_http_status(201) + expect(json_response['title']).to eq('new issue') + expect(json_response['confidential']).to be_falsy + end + + it 'creates a public issue when confidential param is invalid' do + post api("/projects/#{project.id}/issues", user), + title: 'new issue', confidential: 'foo' + + expect(response).to have_http_status(201) + expect(json_response['title']).to eq('new issue') + expect(json_response['confidential']).to be_falsy end it "sends notifications for subscribers of newly added labels" do @@ -632,6 +671,30 @@ describe API::API, api: true do expect(response).to have_http_status(200) expect(json_response['title']).to eq('updated title') end + + it 'sets an issue to confidential' do + put api("/projects/#{project.id}/issues/#{issue.id}", user), + confidential: true + + expect(response).to have_http_status(200) + expect(json_response['confidential']).to be_truthy + end + + it 'makes a confidential issue public' do + put api("/projects/#{project.id}/issues/#{confidential_issue.id}", user), + confidential: false + + expect(response).to have_http_status(200) + expect(json_response['confidential']).to be_falsy + end + + it 'does not update a confidential issue with wrong confidential flag' do + put api("/projects/#{project.id}/issues/#{confidential_issue.id}", user), + confidential: 'foo' + + expect(response).to have_http_status(200) + expect(json_response['confidential']).to be_truthy + end end end -- cgit v1.2.1 From 3a575d1479512fc25ec963af0b01f1f24fd64181 Mon Sep 17 00:00:00 2001 From: zzjin Date: Wed, 31 Aug 2016 07:57:56 +0000 Subject: Update toggler_behavior.js to toggle ajax loaded contents like `diffs` page. --- app/assets/javascripts/behaviors/toggler_behavior.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/behaviors/toggler_behavior.js b/app/assets/javascripts/behaviors/toggler_behavior.js index 8ac1ba7665e..5467e3edc69 100644 --- a/app/assets/javascripts/behaviors/toggler_behavior.js +++ b/app/assets/javascripts/behaviors/toggler_behavior.js @@ -1,6 +1,6 @@ (function(w) { $(function() { - $('.js-toggle-button').on('click', function(e) { + $('body').on('click', '.js-toggle-button', function(e) { e.preventDefault(); $(this) .find('.fa') -- cgit v1.2.1 From 97d6f5b6ded829d1f7e792c59ae5eb4b2aae7c70 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 31 Aug 2016 11:41:16 +0100 Subject: Fixed escaping issue with labels filter Closes #15552 --- app/assets/javascripts/gl_dropdown.js | 2 +- app/assets/javascripts/labels_select.js | 2 +- app/views/shared/issuable/_label_dropdown.html.haml | 2 +- spec/features/issues/filter_issues_spec.rb | 10 ++++++++++ 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/gl_dropdown.js b/app/assets/javascripts/gl_dropdown.js index 5a2a8523d9f..77b2082cba0 100644 --- a/app/assets/javascripts/gl_dropdown.js +++ b/app/assets/javascripts/gl_dropdown.js @@ -556,7 +556,7 @@ if (isInput) { field = $(this.el); } else { - field = this.dropdown.parent().find("input[name='" + fieldName + "'][value='" + value + "']"); + field = this.dropdown.parent().find("input[name='" + fieldName + "'][value='" + escape(value) + "']"); } if (el.hasClass(ACTIVE_CLASS)) { el.removeClass(ACTIVE_CLASS); diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js index 565dbeacdb3..bab23ff5ac0 100644 --- a/app/assets/javascripts/labels_select.js +++ b/app/assets/javascripts/labels_select.js @@ -164,7 +164,7 @@ instance.addInput(this.fieldName, label.id); } } - if ($form.find("input[type='hidden'][name='" + ($dropdown.data('fieldName')) + "'][value='" + (this.id(label)) + "']").length) { + if ($form.find("input[type='hidden'][name='" + ($dropdown.data('fieldName')) + "'][value='" + escape(this.id(label)) + "']").length) { selectedClass.push('is-active'); } if ($dropdown.hasClass('js-multiselect') && removesAll) { diff --git a/app/views/shared/issuable/_label_dropdown.html.haml b/app/views/shared/issuable/_label_dropdown.html.haml index d34d28f6736..24a1a616919 100644 --- a/app/views/shared/issuable/_label_dropdown.html.haml +++ b/app/views/shared/issuable/_label_dropdown.html.haml @@ -12,7 +12,7 @@ - if params[:label_name].present? - if params[:label_name].respond_to?('any?') - params[:label_name].each do |label| - = hidden_field_tag "label_name[]", label, id: nil + = hidden_field_tag "label_name[]", u(label), id: nil .dropdown %button.dropdown-menu-toggle.js-label-select.js-multiselect{class: classes.join(' '), type: "button", data: dropdown_data} %span.dropdown-toggle-text diff --git a/spec/features/issues/filter_issues_spec.rb b/spec/features/issues/filter_issues_spec.rb index e262f285868..0e9f814044e 100644 --- a/spec/features/issues/filter_issues_spec.rb +++ b/spec/features/issues/filter_issues_spec.rb @@ -8,6 +8,7 @@ describe 'Filter issues', feature: true do let!(:milestone) { create(:milestone, project: project) } let!(:label) { create(:label, project: project) } let!(:issue1) { create(:issue, project: project) } + let!(:wontfix) { create(:label, project: project, title: "Won't fix") } before do project.team << [user, :master] @@ -107,6 +108,15 @@ describe 'Filter issues', feature: true do end expect(find('.js-label-select .dropdown-toggle-text')).to have_content(label.title) end + + it 'filters by wont fix labels' do + find('.dropdown-menu-labels a', text: label.title).click + page.within '.labels-filter' do + expect(page).to have_content wontfix.title + click_link wontfix.title + end + expect(find('.js-label-select .dropdown-toggle-text')).to have_content(wontfix.title) + end end describe 'Filter issues for assignee and label from issues#index' do -- cgit v1.2.1 From 6c9c33f43e489d9e225bc64342c7d7d0a558b1e7 Mon Sep 17 00:00:00 2001 From: tiagonbotelho Date: Wed, 31 Aug 2016 11:39:35 +0100 Subject: filters tags by name --- CHANGELOG | 1 + app/controllers/projects/tags_controller.rb | 3 ++- app/helpers/tags_helper.rb | 10 ++++++++++ app/views/projects/tags/index.html.haml | 14 ++++++++------ 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 4e963702b8e..d82cf74f322 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.12.0 (unreleased) + - Filter tags by name !6121 - Make push events have equal vertical spacing. - Add two-factor recovery endpoint to internal API !5510 - Remove vendor prefixes for linear-gradient CSS (ClemMakesApps) diff --git a/app/controllers/projects/tags_controller.rb b/app/controllers/projects/tags_controller.rb index 8592579abbd..3e5de2e0d3e 100644 --- a/app/controllers/projects/tags_controller.rb +++ b/app/controllers/projects/tags_controller.rb @@ -1,4 +1,5 @@ class Projects::TagsController < Projects::ApplicationController + include SortingHelper # Authorize before_action :require_non_empty_project before_action :authorize_download_code! @@ -7,7 +8,7 @@ class Projects::TagsController < Projects::ApplicationController def index @sort = params[:sort] || 'name' - @tags = @repository.tags_sorted_by(@sort) + @tags = TagsFinder.new(@repository, params).execute @tags = Kaminari.paginate_array(@tags).page(params[:page]) @releases = project.releases.where(tag: @tags.map(&:name)) diff --git a/app/helpers/tags_helper.rb b/app/helpers/tags_helper.rb index fb85544df2d..c0ec1634cdb 100644 --- a/app/helpers/tags_helper.rb +++ b/app/helpers/tags_helper.rb @@ -3,6 +3,16 @@ module TagsHelper "/tags/#{tag}" end + def filter_tags_path(options = {}) + exist_opts = { + search: params[:search], + sort: params[:sort] + } + + options = exist_opts.merge(options) + namespace_project_tags_path(@project.namespace, @project, @id, options) + end + def tag_list(project) html = '' project.tag_list.each do |tag| diff --git a/app/views/projects/tags/index.html.haml b/app/views/projects/tags/index.html.haml index 368231e73fe..31a023f24cf 100644 --- a/app/views/projects/tags/index.html.haml +++ b/app/views/projects/tags/index.html.haml @@ -8,21 +8,23 @@ Tags give the ability to mark specific points in history as being important .nav-controls - - if can? current_user, :push_code, @project - = link_to new_namespace_project_tag_path(@project.namespace, @project), class: 'btn btn-create new-tag-btn' do - New tag + = form_tag(filter_tags_path, method: :get) do + = search_field_tag :search, params[:search], { placeholder: 'Filter by tag name', id: 'tag-search', class: 'form-control search-text-input input-short', spellcheck: false } .dropdown.inline %button.dropdown-toggle.btn{ type: 'button', data: { toggle: 'dropdown'} } %span.light= @sort.humanize %b.caret %ul.dropdown-menu.dropdown-menu-align-right %li - = link_to namespace_project_tags_path(sort: nil) do + = link_to filter_tags_path(sort: nil) do Name - = link_to namespace_project_tags_path(sort: sort_value_recently_updated) do + = link_to filter_tags_path(sort: sort_value_recently_updated) do = sort_title_recently_updated - = link_to namespace_project_tags_path(sort: sort_value_oldest_updated) do + = link_to filter_tags_path(sort: sort_value_oldest_updated) do = sort_title_oldest_updated + - if can? current_user, :push_code, @project + = link_to new_namespace_project_tag_path(@project.namespace, @project), class: 'btn btn-create new-tag-btn' do + New tag .tags - if @tags.any? -- cgit v1.2.1 From 39d789eaeeffb2667d95d05da76d3a7b72ae3b13 Mon Sep 17 00:00:00 2001 From: Vitaly Baev Date: Wed, 31 Aug 2016 15:11:36 +0300 Subject: Fixed invisible scroll controlls on build page on iPhone --- CHANGELOG | 1 + app/assets/stylesheets/pages/builds.scss | 1 + 2 files changed, 2 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 7941a29d4ed..6a4fdad8755 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -50,6 +50,7 @@ v 8.12.0 (unreleased) - Adds response mime type to transaction metric action when it's not HTML - Fix hover leading space bug in pipeline graph !5980 - User can edit closed MR with deleted fork (Katarzyna Kobierska Ula Budziszewska) !5496 + - Fixed invisible scroll controls on build page on iPhone v 8.11.4 (unreleased) - Fix broken gitlab:backup:restore because of bad permissions on repo storage !6098 (Dirk Hörner) diff --git a/app/assets/stylesheets/pages/builds.scss b/app/assets/stylesheets/pages/builds.scss index 8c33e7d9a2e..cee198691c2 100644 --- a/app/assets/stylesheets/pages/builds.scss +++ b/app/assets/stylesheets/pages/builds.scss @@ -36,6 +36,7 @@ &.affix { right: 30px; bottom: 15px; + z-index: 1; @media (min-width: $screen-md-min) { right: 26%; -- cgit v1.2.1 From 52ab33a3fa1bd31e8e01ea8b9c89d8c08dbeff82 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Wed, 31 Aug 2016 15:04:51 +0100 Subject: Expire commit view partial after a day We rarely use Russian-doll caching in views, and when we do, it's typically with a naturally-invalidating key. In the case of the commit partial, though, the author lookup isn't part of the cache key - because we're not going to use the state of the users table - and so a new email address can take up to two weeks to show against the commits list. Limiting this to a day still caches the partial for a healthy amount of time, without as bad a worst case scenario. --- CHANGELOG | 1 + app/views/projects/commits/_commit.html.haml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 7941a29d4ed..f36ab3b2b21 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -30,6 +30,7 @@ v 8.12.0 (unreleased) - Added tests for diff notes - Add a button to download latest successful artifacts for branches and tags !5142 - Remove redundant pipeline tooltips (ClemMakesApps) + - Expire commit info views after one day, instead of two weeks, to allow for user email updates - Add delimiter to project stars and forks count (ClemMakesApps) - Fix badge count alignment (ClemMakesApps) - Fix branch title trailing space on hover (ClemMakesApps) diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index fd888f41b1e..389477d0927 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -7,7 +7,7 @@ - cache_key = [project.path_with_namespace, commit.id, current_application_settings, note_count] - cache_key.push(commit.status) if commit.status -= cache(cache_key) do += cache(cache_key, expires_in: 1.day) do %li.commit.js-toggle-container{ id: "commit-#{commit.short_id}" } = author_avatar(commit, size: 36) -- cgit v1.2.1 From fd4efde5aa0693c04cc2679b550b271ea40eea39 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 29 Aug 2016 16:47:31 +0200 Subject: Block concurrent pipeline processings --- CHANGELOG | 4 +--- app/services/ci/process_pipeline_service.rb | 14 ++++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 7941a29d4ed..d1e5c65ac31 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -54,13 +54,11 @@ v 8.12.0 (unreleased) v 8.11.4 (unreleased) - Fix broken gitlab:backup:restore because of bad permissions on repo storage !6098 (Dirk Hörner) - Creating an issue through our API now emails label subscribers !5720 + - Block concurrent updates for Pipeline - Fix resolving conflicts on forks - Fix diff commenting on merge requests created prior to 8.10 - -v 8.11.4 (unreleased) - Fix issue boards leak private label names and descriptions -v 8.11.3 (unreleased) v 8.11.3 - Do not enforce using hash with hidden key in CI configuration. !6079 - Allow system info page to handle case where info is unavailable diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb index f049ed628db..de48a50774e 100644 --- a/app/services/ci/process_pipeline_service.rb +++ b/app/services/ci/process_pipeline_service.rb @@ -10,13 +10,15 @@ module Ci create_builds! end - new_builds = - stage_indexes_of_created_builds.map do |index| - process_stage(index) - end + @pipeline.with_lock do + new_builds = + stage_indexes_of_created_builds.map do |index| + process_stage(index) + end - # Return a flag if a when builds got enqueued - new_builds.flatten.any? + # Return a flag if a when builds got enqueued + new_builds.flatten.any? + end end private -- cgit v1.2.1 From e18d034b20c8b0f1a1900e75c9bc0bce51c75438 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Wed, 31 Aug 2016 15:23:31 +0000 Subject: Fix grammar (those issue -> those issues) --- PROCESS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PROCESS.md b/PROCESS.md index 8e1a3f7360f..8af660fbdd1 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -50,7 +50,7 @@ etc.). The most important thing is making sure valid issues receive feedback from the development team. Therefore the priority is mentioning developers that can help -on those issue. Please select someone with relevant experience from +on those issues. Please select someone with relevant experience from [GitLab core team][core-team]. If there is nobody mentioned with that expertise look in the commit history for the affected files to find someone. Avoid mentioning the lead developer, this is the person that is least likely to give a -- cgit v1.2.1 From 632899826bbb4d50f8c003b2c1771fa17d89d022 Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Tue, 30 Aug 2016 14:20:31 -0500 Subject: Fix bug where pagination is still displayed despite all todos marked as done --- CHANGELOG | 1 + app/assets/javascripts/todos.js | 2 +- app/views/dashboard/todos/index.html.haml | 2 +- features/steps/dashboard/todos.rb | 1 + spec/features/todos/todos_spec.rb | 14 ++++++++++++++ 5 files changed, 18 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 18efe057299..c1d989f1739 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -10,6 +10,7 @@ v 8.12.0 (unreleased) - Reduce contributions calendar data payload (ClemMakesApps) - Add `web_url` field to issue, merge request, and snippet API objects (Ben Boeckel) - Set path for all JavaScript cookies to honor GitLab's subdirectory setting !5627 (Mike Greiling) + - Fix bug where pagination is still displayed despite all todos marked as done (ClemMakesApps) - Shorten task status phrase (ClemMakesApps) - Add hover color to emoji icon (ClemMakesApps) - Fix branches page dropdown sort alignment (ClemMakesApps) diff --git a/app/assets/javascripts/todos.js b/app/assets/javascripts/todos.js index 6e677fa8cc6..06605320a35 100644 --- a/app/assets/javascripts/todos.js +++ b/app/assets/javascripts/todos.js @@ -66,7 +66,7 @@ success: (function(_this) { return function(data) { $this.remove(); - $('.js-todos-list').remove(); + $('.prepend-top-default').html('
You\'re all done!
'); return _this.updateBadges(data); }; })(this) diff --git a/app/views/dashboard/todos/index.html.haml b/app/views/dashboard/todos/index.html.haml index d320d3bcc1e..6bcc37042ea 100644 --- a/app/views/dashboard/todos/index.html.haml +++ b/app/views/dashboard/todos/index.html.haml @@ -66,7 +66,7 @@ - if @todos.any? .js-todos-options{ data: {per_page: @todos.limit_value, current_page: @todos.current_page, total_pages: @todos.total_pages} } - @todos.group_by(&:project).each do |group| - .panel.panel-default.panel-small.js-todos-list + .panel.panel-default.panel-small - project = group[0] .panel-heading = link_to project.name_with_namespace, namespace_project_path(project.namespace, project) diff --git a/features/steps/dashboard/todos.rb b/features/steps/dashboard/todos.rb index 60152d3da55..0607086c166 100644 --- a/features/steps/dashboard/todos.rb +++ b/features/steps/dashboard/todos.rb @@ -54,6 +54,7 @@ class Spinach::Features::DashboardTodos < Spinach::FeatureSteps page.within('.todos-pending-count') { expect(page).to have_content '0' } expect(page).to have_content 'To do 0' expect(page).to have_content 'Done 4' + expect(page).to have_content "You're all done!" expect(page).not_to have_link project.name_with_namespace should_not_see_todo "John Doe assigned you merge request #{merge_request.to_reference}" should_not_see_todo "John Doe mentioned you on issue #{issue.to_reference}" diff --git a/spec/features/todos/todos_spec.rb b/spec/features/todos/todos_spec.rb index 32544f3f538..fc555a74f30 100644 --- a/spec/features/todos/todos_spec.rb +++ b/spec/features/todos/todos_spec.rb @@ -118,6 +118,20 @@ describe 'Dashboard Todos', feature: true do expect(page).to have_css("#todo_#{Todo.first.id}") end end + + describe 'mark all as done', js: true do + before do + visit dashboard_todos_path + click_link('Mark all as done') + end + + it 'shows "All done" message!' do + within('.todos-pending-count') { expect(page).to have_content '0' } + expect(page).to have_content 'To do 0' + expect(page).to have_content "You're all done!" + expect(page).not_to have_selector('.gl-pagination') + end + end end context 'User has a Todo in a project pending deletion' do -- cgit v1.2.1 From 636dbc85e24600849de9dd09c4854edf4adfe807 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Wed, 31 Aug 2016 17:40:54 +0200 Subject: The ID of a project can be also a string [ci skip] --- doc/api/commits.md | 14 +++++++------- doc/api/projects.md | 8 ++++---- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/doc/api/commits.md b/doc/api/commits.md index 55d0de7afd9..682151d4b1d 100644 --- a/doc/api/commits.md +++ b/doc/api/commits.md @@ -10,7 +10,7 @@ GET /projects/:id/repository/commits | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `id` | integer | yes | The ID of a project or NAMESPACE/PROJECT_NAME owned by the authenticated user +| `id` | integer/string | yes | The ID of a project or NAMESPACE/PROJECT_NAME owned by the authenticated user | `ref_name` | string | no | The name of a repository branch or tag or if not given the default branch | | `since` | string | no | Only commits after or in this date will be returned in ISO 8601 format YYYY-MM-DDTHH:MM:SSZ | | `until` | string | no | Only commits before or in this date will be returned in ISO 8601 format YYYY-MM-DDTHH:MM:SSZ | @@ -58,7 +58,7 @@ Parameters: | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `id` | integer | yes | The ID of a project or NAMESPACE/PROJECT_NAME owned by the authenticated user +| `id` | integer/string | yes | The ID of a project or NAMESPACE/PROJECT_NAME owned by the authenticated user | `sha` | string | yes | The commit hash or name of a repository branch or tag | ```bash @@ -102,7 +102,7 @@ Parameters: | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `id` | integer | yes | The ID of a project or NAMESPACE/PROJECT_NAME owned by the authenticated user +| `id` | integer/string | yes | The ID of a project or NAMESPACE/PROJECT_NAME owned by the authenticated user | `sha` | string | yes | The commit hash or name of a repository branch or tag | ```bash @@ -138,7 +138,7 @@ Parameters: | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `id` | integer | yes | The ID of a project or NAMESPACE/PROJECT_NAME owned by the authenticated user +| `id` | integer/string | yes | The ID of a project or NAMESPACE/PROJECT_NAME owned by the authenticated user | `sha` | string | yes | The commit hash or name of a repository branch or tag | ```bash @@ -187,7 +187,7 @@ POST /projects/:id/repository/commits/:sha/comments | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `id` | integer | yes | The ID of a project or NAMESPACE/PROJECT_NAME owned by the authenticated user +| `id` | integer/string | yes | The ID of a project or NAMESPACE/PROJECT_NAME owned by the authenticated user | `sha` | string | yes | The commit SHA or name of a repository branch or tag | | `note` | string | yes | The text of the comment | | `path` | string | no | The file path relative to the repository | @@ -232,7 +232,7 @@ GET /projects/:id/repository/commits/:sha/statuses | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `id` | integer | yes | The ID of a project or NAMESPACE/PROJECT_NAME owned by the authenticated user +| `id` | integer/string | yes | The ID of a project or NAMESPACE/PROJECT_NAME owned by the authenticated user | `sha` | string | yes | The commit SHA | `ref_name`| string | no | The name of a repository branch or tag or, if not given, the default branch | `stage` | string | no | Filter by [build stage](../ci/yaml/README.md#stages), e.g., `test` @@ -306,7 +306,7 @@ POST /projects/:id/statuses/:sha | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `id` | integer | yes | The ID of a project or NAMESPACE/PROJECT_NAME owned by the authenticated user +| `id` | integer/string | yes | The ID of a project or NAMESPACE/PROJECT_NAME owned by the authenticated user | `sha` | string | yes | The commit SHA | `state` | string | yes | The state of the status. Can be one of the following: `pending`, `running`, `success`, `failed`, `canceled` | `ref` | string | no | The `ref` (branch or tag) to which the status refers diff --git a/doc/api/projects.md b/doc/api/projects.md index 0d5aa61aa74..a62aaee14d7 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -535,7 +535,7 @@ POST /projects/:id/star | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `id` | integer | yes | The ID of the project or NAMESPACE/PROJECT_NAME | +| `id` | integer/string | yes | The ID of the project or NAMESPACE/PROJECT_NAME | ```bash curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/5/star" @@ -602,7 +602,7 @@ DELETE /projects/:id/star | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `id` | integer | yes | The ID of the project or NAMESPACE/PROJECT_NAME | +| `id` | integer/string | yes | The ID of the project or NAMESPACE/PROJECT_NAME | ```bash curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/5/star" @@ -673,7 +673,7 @@ POST /projects/:id/archive | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `id` | integer | yes | The ID of the project or NAMESPACE/PROJECT_NAME | +| `id` | integer/string | yes | The ID of the project or NAMESPACE/PROJECT_NAME | ```bash curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/archive" @@ -760,7 +760,7 @@ POST /projects/:id/unarchive | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `id` | integer | yes | The ID of the project or NAMESPACE/PROJECT_NAME | +| `id` | integer/string | yes | The ID of the project or NAMESPACE/PROJECT_NAME | ```bash curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/unarchive" -- cgit v1.2.1 From 52daddc0b7166fc50949db042f4f05488f0b9eae Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 30 Aug 2016 16:20:56 -0300 Subject: Use updated_at as the last updated date when importing from GitHub --- lib/gitlab/github_import/issue_formatter.rb | 6 +--- lib/gitlab/github_import/milestone_formatter.rb | 36 ++++------------------ lib/gitlab/github_import/pull_request_formatter.rb | 11 +------ .../gitlab/github_import/issue_formatter_spec.rb | 5 ++- .../github_import/milestone_formatter_spec.rb | 5 ++- .../github_import/pull_request_formatter_spec.rb | 7 ++--- 6 files changed, 15 insertions(+), 55 deletions(-) diff --git a/lib/gitlab/github_import/issue_formatter.rb b/lib/gitlab/github_import/issue_formatter.rb index 835ec858b35..07edbe37a13 100644 --- a/lib/gitlab/github_import/issue_formatter.rb +++ b/lib/gitlab/github_import/issue_formatter.rb @@ -12,7 +12,7 @@ module Gitlab author_id: author_id, assignee_id: assignee_id, created_at: raw_data.created_at, - updated_at: updated_at + updated_at: raw_data.updated_at } end @@ -69,10 +69,6 @@ module Gitlab def state raw_data.state == 'closed' ? 'closed' : 'opened' end - - def updated_at - state == 'closed' ? raw_data.closed_at : raw_data.updated_at - end end end end diff --git a/lib/gitlab/github_import/milestone_formatter.rb b/lib/gitlab/github_import/milestone_formatter.rb index 53d4b3102d1..b2fa524cf5b 100644 --- a/lib/gitlab/github_import/milestone_formatter.rb +++ b/lib/gitlab/github_import/milestone_formatter.rb @@ -3,14 +3,14 @@ module Gitlab class MilestoneFormatter < BaseFormatter def attributes { - iid: number, + iid: raw_data.number, project: project, - title: title, - description: description, - due_date: due_date, + title: raw_data.title, + description: raw_data.description, + due_date: raw_data.due_on, state: state, - created_at: created_at, - updated_at: updated_at + created_at: raw_data.created_at, + updated_at: raw_data.updated_at } end @@ -20,33 +20,9 @@ module Gitlab private - def number - raw_data.number - end - - def title - raw_data.title - end - - def description - raw_data.description - end - - def due_date - raw_data.due_on - end - def state raw_data.state == 'closed' ? 'closed' : 'active' end - - def created_at - raw_data.created_at - end - - def updated_at - state == 'closed' ? raw_data.closed_at : raw_data.updated_at - end end end end diff --git a/lib/gitlab/github_import/pull_request_formatter.rb b/lib/gitlab/github_import/pull_request_formatter.rb index 04aa3664f64..d9d436d7490 100644 --- a/lib/gitlab/github_import/pull_request_formatter.rb +++ b/lib/gitlab/github_import/pull_request_formatter.rb @@ -20,7 +20,7 @@ module Gitlab author_id: author_id, assignee_id: assignee_id, created_at: raw_data.created_at, - updated_at: updated_at + updated_at: raw_data.updated_at } end @@ -103,15 +103,6 @@ module Gitlab 'opened' end end - - def updated_at - case state - when 'merged' then raw_data.merged_at - when 'closed' then raw_data.closed_at - else - raw_data.updated_at - end - end end end end diff --git a/spec/lib/gitlab/github_import/issue_formatter_spec.rb b/spec/lib/gitlab/github_import/issue_formatter_spec.rb index 0e7ffbe9b8e..d60c4111e99 100644 --- a/spec/lib/gitlab/github_import/issue_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/issue_formatter_spec.rb @@ -48,8 +48,7 @@ describe Gitlab::GithubImport::IssueFormatter, lib: true do end context 'when issue is closed' do - let(:closed_at) { DateTime.strptime('2011-01-28T19:01:12Z') } - let(:raw_data) { double(base_data.merge(state: 'closed', closed_at: closed_at)) } + let(:raw_data) { double(base_data.merge(state: 'closed')) } it 'returns formatted attributes' do expected = { @@ -62,7 +61,7 @@ describe Gitlab::GithubImport::IssueFormatter, lib: true do author_id: project.creator_id, assignee_id: nil, created_at: created_at, - updated_at: closed_at + updated_at: updated_at } expect(issue.attributes).to eq(expected) diff --git a/spec/lib/gitlab/github_import/milestone_formatter_spec.rb b/spec/lib/gitlab/github_import/milestone_formatter_spec.rb index 5a421e50581..09337c99a07 100644 --- a/spec/lib/gitlab/github_import/milestone_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/milestone_formatter_spec.rb @@ -40,8 +40,7 @@ describe Gitlab::GithubImport::MilestoneFormatter, lib: true do end context 'when milestone is closed' do - let(:closed_at) { DateTime.strptime('2011-01-28T19:01:12Z') } - let(:raw_data) { double(base_data.merge(state: 'closed', closed_at: closed_at)) } + let(:raw_data) { double(base_data.merge(state: 'closed')) } it 'returns formatted attributes' do expected = { @@ -52,7 +51,7 @@ describe Gitlab::GithubImport::MilestoneFormatter, lib: true do state: 'closed', due_date: nil, created_at: created_at, - updated_at: closed_at + updated_at: updated_at } expect(formatter.attributes).to eq(expected) diff --git a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb index b667abf063d..edfc6ad81c6 100644 --- a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb @@ -62,8 +62,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do end context 'when pull request is closed' do - let(:closed_at) { DateTime.strptime('2011-01-28T19:01:12Z') } - let(:raw_data) { double(base_data.merge(state: 'closed', closed_at: closed_at)) } + let(:raw_data) { double(base_data.merge(state: 'closed')) } it 'returns formatted attributes' do expected = { @@ -81,7 +80,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do author_id: project.creator_id, assignee_id: nil, created_at: created_at, - updated_at: closed_at + updated_at: updated_at } expect(pull_request.attributes).to eq(expected) @@ -108,7 +107,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do author_id: project.creator_id, assignee_id: nil, created_at: created_at, - updated_at: merged_at + updated_at: updated_at } expect(pull_request.attributes).to eq(expected) -- cgit v1.2.1 From 80c4f1093747797cafaa832130e9781e3774722e Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 30 Aug 2016 16:25:28 -0300 Subject: Don't touch Issue/Merge Request when importing GitHub comments --- lib/gitlab/github_import/importer.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb index 02ffb43d89b..1b2a5eb8f52 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -152,12 +152,14 @@ module Gitlab end def create_comments(issuable, comments) - comments.each do |raw| - begin - comment = CommentFormatter.new(project, raw) - issuable.notes.create!(comment.attributes) - rescue => e - errors << { type: :comment, url: Gitlab::UrlSanitizer.sanitize(raw.url), errors: e.message } + ActiveRecord::Base.no_touching do + comments.each do |raw| + begin + comment = CommentFormatter.new(project, raw) + issuable.notes.create!(comment.attributes) + rescue => e + errors << { type: :comment, url: Gitlab::UrlSanitizer.sanitize(raw.url), errors: e.message } + end end end end -- cgit v1.2.1 From 9ef743d1e4b669f15f51ca440b1172856e1d7fff Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 30 Aug 2016 16:34:20 -0300 Subject: Update CHANGELOG --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 08562b1a5a0..65cbcd6059a 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -57,6 +57,7 @@ v 8.12.0 (unreleased) v 8.11.4 (unreleased) - Fix broken gitlab:backup:restore because of bad permissions on repo storage !6098 (Dirk Hörner) + - Fix sorting issues by "last updated" doesn't work after import from GitHub - Creating an issue through our API now emails label subscribers !5720 - Block concurrent updates for Pipeline - Fix resolving conflicts on forks -- cgit v1.2.1 From eba024366bdaa2d2645857fcb404ad0468b782fa Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Thu, 18 Aug 2016 11:18:25 -0500 Subject: Remove redundant js-timeago-pending from user activity log --- CHANGELOG | 1 + app/assets/javascripts/activities.js | 2 +- app/views/events/_event.html.haml | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 08562b1a5a0..40a7237333f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -30,6 +30,7 @@ v 8.12.0 (unreleased) - Add last commit time to repo view (ClemMakesApps) - Added project specific enable/disable setting for LFS !5997 - Don't expose a user's token in the `/api/v3/user` API (!6047) + - Remove redundant js-timeago-pending from user activity log (ClemMakesApps) - Added tests for diff notes - Add a button to download latest successful artifacts for branches and tags !5142 - Remove redundant pipeline tooltips (ClemMakesApps) diff --git a/app/assets/javascripts/activities.js b/app/assets/javascripts/activities.js index 5ea6086ab77..d5e11e22be5 100644 --- a/app/assets/javascripts/activities.js +++ b/app/assets/javascripts/activities.js @@ -12,7 +12,7 @@ } Activities.prototype.updateTooltips = function() { - return gl.utils.localTimeAgo($('.js-timeago', '#activity')); + return gl.utils.localTimeAgo($('.js-timeago', '.content_list')); }; Activities.prototype.reloadActivities = function() { diff --git a/app/views/events/_event.html.haml b/app/views/events/_event.html.haml index 5c318cd3b8b..31fdcc5e21b 100644 --- a/app/views/events/_event.html.haml +++ b/app/views/events/_event.html.haml @@ -1,7 +1,7 @@ - if event.visible_to_user?(current_user) .event-item{ class: event_row_class(event) } .event-item-timestamp - #{time_ago_with_tooltip(event.created_at)} + #{time_ago_with_tooltip(event.created_at, skip_js: true)} = cache [event, current_application_settings, "v2.2"] do = author_avatar(event, size: 40) -- cgit v1.2.1 From 97b69862ad50936ad0700b23c485a38b637469a7 Mon Sep 17 00:00:00 2001 From: tiagonbotelho Date: Wed, 31 Aug 2016 12:35:23 +0100 Subject: add specs for tags finder --- app/controllers/projects/tags_controller.rb | 5 +- app/finders/tags_finder.rb | 29 +++++++++++ app/views/projects/tags/index.html.haml | 5 +- spec/finders/tags_finder_spec.rb | 79 +++++++++++++++++++++++++++++ 4 files changed, 115 insertions(+), 3 deletions(-) create mode 100644 app/finders/tags_finder.rb create mode 100644 spec/finders/tags_finder_spec.rb diff --git a/app/controllers/projects/tags_controller.rb b/app/controllers/projects/tags_controller.rb index 3e5de2e0d3e..6ea8ee62bc5 100644 --- a/app/controllers/projects/tags_controller.rb +++ b/app/controllers/projects/tags_controller.rb @@ -1,5 +1,6 @@ class Projects::TagsController < Projects::ApplicationController include SortingHelper + # Authorize before_action :require_non_empty_project before_action :authorize_download_code! @@ -7,7 +8,9 @@ class Projects::TagsController < Projects::ApplicationController before_action :authorize_admin_project!, only: [:destroy] def index - @sort = params[:sort] || 'name' + params[:sort] = params[:sort].presence || 'name' + + @sort = params[:sort] @tags = TagsFinder.new(@repository, params).execute @tags = Kaminari.paginate_array(@tags).page(params[:page]) diff --git a/app/finders/tags_finder.rb b/app/finders/tags_finder.rb new file mode 100644 index 00000000000..b474f0805dc --- /dev/null +++ b/app/finders/tags_finder.rb @@ -0,0 +1,29 @@ +class TagsFinder + def initialize(repository, params) + @repository = repository + @params = params + end + + def execute + tags = @repository.tags_sorted_by(sort) + filter_by_name(tags) + end + + private + + def sort + @params[:sort].presence + end + + def search + @params[:search].presence + end + + def filter_by_name(tags) + if search + tags.select { |tag| tag.name.include?(search) } + else + tags + end + end +end diff --git a/app/views/projects/tags/index.html.haml b/app/views/projects/tags/index.html.haml index 31a023f24cf..6adbe9351dc 100644 --- a/app/views/projects/tags/index.html.haml +++ b/app/views/projects/tags/index.html.haml @@ -12,7 +12,8 @@ = search_field_tag :search, params[:search], { placeholder: 'Filter by tag name', id: 'tag-search', class: 'form-control search-text-input input-short', spellcheck: false } .dropdown.inline %button.dropdown-toggle.btn{ type: 'button', data: { toggle: 'dropdown'} } - %span.light= @sort.humanize + %span.light + = @sort.humanize %b.caret %ul.dropdown-menu.dropdown-menu-align-right %li @@ -22,7 +23,7 @@ = sort_title_recently_updated = link_to filter_tags_path(sort: sort_value_oldest_updated) do = sort_title_oldest_updated - - if can? current_user, :push_code, @project + - if can?(current_user, :push_code, @project) = link_to new_namespace_project_tag_path(@project.namespace, @project), class: 'btn btn-create new-tag-btn' do New tag diff --git a/spec/finders/tags_finder_spec.rb b/spec/finders/tags_finder_spec.rb new file mode 100644 index 00000000000..2ac810e478a --- /dev/null +++ b/spec/finders/tags_finder_spec.rb @@ -0,0 +1,79 @@ +require 'spec_helper' + +describe TagsFinder do + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:repository) { project.repository } + + describe '#execute' do + context 'sort only' do + it 'sorts by name' do + tags_finder = described_class.new(repository, {}) + + result = tags_finder.execute + + expect(result.first.name).to eq("v1.0.0") + end + + it 'sorts by recently_updated' do + tags_finder = described_class.new(repository, { sort: 'updated_desc' }) + + result = tags_finder.execute + recently_updated_tag = repository.tags.max do |a, b| + repository.commit(a.target).committed_date <=> repository.commit(b.target).committed_date + end + + expect(result.first.name).to eq(recently_updated_tag.name) + end + + it 'sorts by last_updated' do + tags_finder = described_class.new(repository, { sort: 'updated_asc' }) + + result = tags_finder.execute + + expect(result.first.name).to eq('v1.0.0') + end + end + + context 'filter only' do + it 'filters tags by name' do + tags_finder = described_class.new(repository, { search: '1.0.0' }) + + result = tags_finder.execute + + expect(result.first.name).to eq('v1.0.0') + expect(result.count).to eq(1) + end + + it 'does not find any tags with that name' do + tags_finder = described_class.new(repository, { search: 'hey' }) + + result = tags_finder.execute + + expect(result.count).to eq(0) + end + end + + context 'filter and sort' do + it 'filters tags by name and sorts by recently_updated' do + params = { sort: 'updated_desc', search: 'v1' } + tags_finder = described_class.new(repository, params) + + result = tags_finder.execute + + expect(result.first.name).to eq('v1.1.0') + expect(result.count).to eq(2) + end + + it 'filters tags by name and sorts by last_updated' do + params = { sort: 'updated_asc', search: 'v1' } + tags_finder = described_class.new(repository, params) + + result = tags_finder.execute + + expect(result.first.name).to eq('v1.0.0') + expect(result.count).to eq(2) + end + end + end +end -- cgit v1.2.1 From 6a58af3a4a1c3122d57238505898f61c40bf7b54 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 25 Aug 2016 18:34:01 -0500 Subject: Add BroadcastMessage API implementation --- CHANGELOG | 1 + lib/api/api.rb | 1 + lib/api/broadcast_messages.rb | 99 +++++++++++++++ lib/api/entities.rb | 5 + spec/requests/api/broadcast_messages_spec.rb | 180 +++++++++++++++++++++++++++ 5 files changed, 286 insertions(+) create mode 100644 lib/api/broadcast_messages.rb create mode 100644 spec/requests/api/broadcast_messages_spec.rb diff --git a/CHANGELOG b/CHANGELOG index 2b727491760..e3935038517 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -22,6 +22,7 @@ v 8.12.0 (unreleased) - Remove Gitorious import - Fix inconsistent background color for filter input field (ClemMakesApps) - Add Sentry logging to API calls + - Add BroadcastMessage API - Automatically expand hidden discussions when accessed by a permalink !5585 (Mike Greiling) - Remove unused mixins (ClemMakesApps) - Add search to all issue board lists diff --git a/lib/api/api.rb b/lib/api/api.rb index 4602e627fdb..e14464c1b0d 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -31,6 +31,7 @@ module API mount ::API::AccessRequests mount ::API::AwardEmoji mount ::API::Branches + mount ::API::BroadcastMessages mount ::API::Builds mount ::API::CommitStatuses mount ::API::Commits diff --git a/lib/api/broadcast_messages.rb b/lib/api/broadcast_messages.rb new file mode 100644 index 00000000000..fb2a4148011 --- /dev/null +++ b/lib/api/broadcast_messages.rb @@ -0,0 +1,99 @@ +module API + class BroadcastMessages < Grape::API + before { authenticate! } + before { authenticated_as_admin! } + + resource :broadcast_messages do + helpers do + def find_message + BroadcastMessage.find(params[:id]) + end + end + + desc 'Get all broadcast messages' do + detail 'This feature was introduced in GitLab 8.12.' + success Entities::BroadcastMessage + end + params do + optional :page, type: Integer, desc: 'Current page number' + optional :per_page, type: Integer, desc: 'Number of messages per page' + end + get do + messages = BroadcastMessage.all + + present paginate(messages), with: Entities::BroadcastMessage + end + + desc 'Create a broadcast message' do + detail 'This feature was introduced in GitLab 8.12.' + success Entities::BroadcastMessage + end + params do + requires :message, type: String, desc: 'Message to display' + optional :starts_at, type: DateTime, desc: 'Starting time', default: -> { Time.zone.now } + optional :ends_at, type: DateTime, desc: 'Ending time', default: -> { 1.hour.from_now } + optional :color, type: String, desc: 'Background color' + optional :font, type: String, desc: 'Foreground color' + end + post do + create_params = declared(params, include_missing: false).to_h + message = BroadcastMessage.create(create_params) + + if message.persisted? + present message, with: Entities::BroadcastMessage + else + render_validation_error!(message) + end + end + + desc 'Get a specific broadcast message' do + detail 'This feature was introduced in GitLab 8.12.' + success Entities::BroadcastMessage + end + params do + requires :id, type: Integer, desc: 'Broadcast message ID' + end + get ':id' do + message = find_message + + present message, with: Entities::BroadcastMessage + end + + desc 'Update a broadcast message' do + detail 'This feature was introduced in GitLab 8.12.' + success Entities::BroadcastMessage + end + params do + requires :id, type: Integer, desc: 'Broadcast message ID' + optional :message, type: String, desc: 'Message to display' + optional :starts_at, type: DateTime, desc: 'Starting time' + optional :ends_at, type: DateTime, desc: 'Ending time' + optional :color, type: String, desc: 'Background color' + optional :font, type: String, desc: 'Foreground color' + end + put ':id' do + message = find_message + update_params = declared(params, include_missing: false).to_h + + if message.update(update_params) + present message, with: Entities::BroadcastMessage + else + render_validation_error!(message) + end + end + + desc 'Delete a broadcast message' do + detail 'This feature was introduced in GitLab 8.12.' + success Entities::BroadcastMessage + end + params do + requires :id, type: Integer, desc: 'Broadcast message ID' + end + delete ':id' do + message = find_message + + present message.destroy, with: Entities::BroadcastMessage + end + end + end +end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index e3a8ff6de80..fe7468dd681 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -575,5 +575,10 @@ module API class Template < Grape::Entity expose :name, :content end + + class BroadcastMessage < Grape::Entity + expose :id, :message, :starts_at, :ends_at, :color, :font + expose :active?, as: :active + end end end diff --git a/spec/requests/api/broadcast_messages_spec.rb b/spec/requests/api/broadcast_messages_spec.rb new file mode 100644 index 00000000000..7c9078b2864 --- /dev/null +++ b/spec/requests/api/broadcast_messages_spec.rb @@ -0,0 +1,180 @@ +require 'spec_helper' + +describe API::BroadcastMessages, api: true do + include ApiHelpers + + let(:user) { create(:user) } + let(:admin) { create(:admin) } + + describe 'GET /broadcast_messages' do + it 'returns a 401 for anonymous users' do + get api('/broadcast_messages') + + expect(response).to have_http_status(401) + end + + it 'returns a 403 for users' do + get api('/broadcast_messages', user) + + expect(response).to have_http_status(403) + end + + it 'returns an Array of BroadcastMessages for admins' do + create(:broadcast_message) + + get api('/broadcast_messages', admin) + + expect(response).to have_http_status(200) + expect(json_response).to be_kind_of(Array) + expect(json_response.first.keys) + .to match_array(%w(id message starts_at ends_at color font active)) + end + end + + describe 'GET /broadcast_messages/:id' do + let!(:message) { create(:broadcast_message) } + + it 'returns a 401 for anonymous users' do + get api("/broadcast_messages/#{message.id}") + + expect(response).to have_http_status(401) + end + + it 'returns a 403 for users' do + get api("/broadcast_messages/#{message.id}", user) + + expect(response).to have_http_status(403) + end + + it 'returns the specified message for admins' do + get api("/broadcast_messages/#{message.id}", admin) + + expect(response).to have_http_status(200) + expect(json_response['id']).to eq message.id + expect(json_response.keys) + .to match_array(%w(id message starts_at ends_at color font active)) + end + end + + describe 'POST /broadcast_messages' do + it 'returns a 401 for anonymous users' do + post api('/broadcast_messages'), attributes_for(:broadcast_message) + + expect(response).to have_http_status(401) + end + + it 'returns a 403 for users' do + post api('/broadcast_messages', user), attributes_for(:broadcast_message) + + expect(response).to have_http_status(403) + end + + context 'as an admin' do + it 'requires the `message` parameter' do + attrs = attributes_for(:broadcast_message) + attrs.delete(:message) + + post api('/broadcast_messages', admin), attrs + + expect(response).to have_http_status(400) + expect(json_response['error']).to eq 'message is missing' + end + + it 'defines sane default start and end times' do + time = Time.zone.parse('2016-07-02 10:11:12') + travel_to(time) do + post api('/broadcast_messages', admin), message: 'Test message' + + expect(response).to have_http_status(201) + expect(json_response['starts_at']).to eq '2016-07-02T10:11:12.000Z' + expect(json_response['ends_at']).to eq '2016-07-02T11:11:12.000Z' + end + end + + it 'accepts a custom background and foreground color' do + attrs = attributes_for(:broadcast_message, color: '#000000', font: '#cecece') + + post api('/broadcast_messages', admin), attrs + + expect(response).to have_http_status(201) + expect(json_response['color']).to eq attrs[:color] + expect(json_response['font']).to eq attrs[:font] + end + end + end + + describe 'PUT /broadcast_messages/:id' do + let!(:message) { create(:broadcast_message) } + + it 'returns a 401 for anonymous users' do + put api("/broadcast_messages/#{message.id}"), + attributes_for(:broadcast_message) + + expect(response).to have_http_status(401) + end + + it 'returns a 403 for users' do + put api("/broadcast_messages/#{message.id}", user), + attributes_for(:broadcast_message) + + expect(response).to have_http_status(403) + end + + context 'as an admin' do + it 'accepts new background and foreground colors' do + attrs = { color: '#000000', font: '#cecece' } + + put api("/broadcast_messages/#{message.id}", admin), attrs + + expect(response).to have_http_status(200) + expect(json_response['color']).to eq attrs[:color] + expect(json_response['font']).to eq attrs[:font] + end + + it 'accepts new start and end times' do + time = Time.zone.parse('2016-07-02 10:11:12') + travel_to(time) do + attrs = { starts_at: Time.zone.now, ends_at: 3.hours.from_now } + + put api("/broadcast_messages/#{message.id}", admin), attrs + + expect(response).to have_http_status(200) + expect(json_response['starts_at']).to eq '2016-07-02T10:11:12.000Z' + expect(json_response['ends_at']).to eq '2016-07-02T13:11:12.000Z' + end + end + + it 'accepts a new message' do + attrs = { message: 'new message' } + + put api("/broadcast_messages/#{message.id}", admin), attrs + + expect(response).to have_http_status(200) + expect { message.reload }.to change { message.message }.to('new message') + end + end + end + + describe 'DELETE /broadcast_messages/:id' do + let!(:message) { create(:broadcast_message) } + + it 'returns a 401 for anonymous users' do + delete api("/broadcast_messages/#{message.id}"), + attributes_for(:broadcast_message) + + expect(response).to have_http_status(401) + end + + it 'returns a 403 for users' do + delete api("/broadcast_messages/#{message.id}", user), + attributes_for(:broadcast_message) + + expect(response).to have_http_status(403) + end + + it 'deletes the broadcast message for admins' do + expect { delete api("/broadcast_messages/#{message.id}", admin) } + .to change { BroadcastMessage.count }.by(-1) + end + end +end -- cgit v1.2.1 From 9d4af1b14068aa77848b982372ddf9e11567ae5f Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 30 Aug 2016 16:47:44 -0300 Subject: Add BroadcastMessage API documentation --- doc/api/broadcast_messages.md | 158 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 doc/api/broadcast_messages.md diff --git a/doc/api/broadcast_messages.md b/doc/api/broadcast_messages.md new file mode 100644 index 00000000000..c3a9207a3ae --- /dev/null +++ b/doc/api/broadcast_messages.md @@ -0,0 +1,158 @@ +# Broadcast Messages + +> **Note:** This feature was introduced in GitLab 8.12. + +The broadcast message API is only accessible to administrators. All requests by +guests will respond with `401 Unauthorized`, and all requests by normal users +will respond with `403 Forbidden`. + +## Get all broadcast messages + +``` +GET /broadcast_messages +``` + +```bash +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/broadcast_messages +``` + +Example response: + +```json +[ + { + "message":"Example broadcast message", + "starts_at":"2016-08-24T23:21:16.078Z", + "ends_at":"2016-08-26T23:21:16.080Z", + "color":"#E75E40", + "font":"#FFFFFF", + "id":1, + "active": false + } +] +``` + +## Get a specific broadcast message + +``` +GET /broadcast_messages/:id +``` + +| Attribute | Type | Required | Description | +| ----------- | -------- | -------- | ------------------------- | +| `id` | integer | yes | Broadcast message ID | + +```bash +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/broadcast_messages/1 +``` + +Example response: + +```json +{ + "message":"Deploy in progress", + "starts_at":"2016-08-24T23:21:16.078Z", + "ends_at":"2016-08-26T23:21:16.080Z", + "color":"#cecece", + "font":"#FFFFFF", + "id":1, + "active":false +} +``` + +## Create a broadcast message + +Responds with `400 Bad request` when the `message` parameter is missing or the +`color` or `font` values are invalid, and `201 Created` when the broadcast +message was successfully created. + +``` +POST /broadcast_messages +``` + +| Attribute | Type | Required | Description | +| ----------- | -------- | -------- | ---------------------------------------------------- | +| `message` | string | yes | Message to display | +| `starts_at` | datetime | no | Starting time (defaults to current time) | +| `ends_at` | datetime | no | Ending time (defaults to one hour from current time) | +| `color` | string | no | Background color hex code | +| `font` | string | no | Foreground color hex code | + +```bash +curl --data "message=Deploy in progress&color=#cecece" --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/broadcast_messages +``` + +Example response: + +```json +{ + "message":"Deploy in progress", + "starts_at":"2016-08-26T00:41:35.060Z", + "ends_at":"2016-08-26T01:41:35.060Z", + "color":"#cecece", + "font":"#FFFFFF", + "id":1, + "active": true +} +``` + +## Update a broadcast message + +``` +PUT /broadcast_messages/:id +``` + +| Attribute | Type | Required | Description | +| ----------- | -------- | -------- | ------------------------- | +| `id` | integer | yes | Broadcast message ID | +| `message` | string | no | Message to display | +| `starts_at` | datetime | no | Starting time | +| `ends_at` | datetime | no | Ending time | +| `color` | string | no | Background color hex code | +| `font` | string | no | Foreground color hex code | + +```bash +curl --request PUT --data "message=Update message&color=#000" --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/broadcast_messages/1 +``` + +Example response: + +```json +{ + "message":"Update message", + "starts_at":"2016-08-26T00:41:35.060Z", + "ends_at":"2016-08-26T01:41:35.060Z", + "color":"#000", + "font":"#FFFFFF", + "id":1, + "active": true +} +``` + +## Delete a broadcast message + +``` +DELETE /broadcast_messages/:id +``` + +| Attribute | Type | Required | Description | +| ----------- | -------- | -------- | ------------------------- | +| `id` | integer | yes | Broadcast message ID | + +```bash +curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/broadcast_messages/1 +``` + +Example response: + +```json +{ + "message":"Update message", + "starts_at":"2016-08-26T00:41:35.060Z", + "ends_at":"2016-08-26T01:41:35.060Z", + "color":"#000", + "font":"#FFFFFF", + "id":1, + "active": true +} +``` -- cgit v1.2.1 From 9b376e772edda214e189752c13d831bae7f1088d Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Thu, 25 Aug 2016 11:16:28 -0300 Subject: GitHub importer use default project visibility for non-private projects --- lib/gitlab/github_import/project_creator.rb | 2 +- .../gitlab/github_import/project_creator_spec.rb | 54 ++++++++++++++++------ 2 files changed, 41 insertions(+), 15 deletions(-) diff --git a/lib/gitlab/github_import/project_creator.rb b/lib/gitlab/github_import/project_creator.rb index f4221003db5..8237fe104d6 100644 --- a/lib/gitlab/github_import/project_creator.rb +++ b/lib/gitlab/github_import/project_creator.rb @@ -17,7 +17,7 @@ module Gitlab path: repo.name, description: repo.description, namespace_id: namespace.id, - visibility_level: repo.private ? Gitlab::VisibilityLevel::PRIVATE : Gitlab::VisibilityLevel::PUBLIC, + visibility_level: repo.private ? Gitlab::VisibilityLevel::PRIVATE : ApplicationSetting.current.default_project_visibility, import_type: "github", import_source: repo.full_name, import_url: repo.clone_url.sub("https://", "https://#{@session_data[:github_access_token]}@"), diff --git a/spec/lib/gitlab/github_import/project_creator_spec.rb b/spec/lib/gitlab/github_import/project_creator_spec.rb index 0f363b8b0aa..014ee462e5c 100644 --- a/spec/lib/gitlab/github_import/project_creator_spec.rb +++ b/spec/lib/gitlab/github_import/project_creator_spec.rb @@ -2,33 +2,59 @@ require 'spec_helper' describe Gitlab::GithubImport::ProjectCreator, lib: true do let(:user) { create(:user) } + let(:namespace) { create(:group, owner: user) } + let(:repo) do OpenStruct.new( login: 'vim', name: 'vim', - private: true, full_name: 'asd/vim', - clone_url: "https://gitlab.com/asd/vim.git", - owner: OpenStruct.new(login: "john") + clone_url: 'https://gitlab.com/asd/vim.git' ) end - let(:namespace) { create(:group, owner: user) } - let(:token) { "asdffg" } - let(:access_params) { { github_access_token: token } } + + subject(:service) { described_class.new(repo, namespace, user, github_access_token: 'asdffg') } before do namespace.add_owner(user) + allow_any_instance_of(Project).to receive(:add_import_job) end - it 'creates project' do - allow_any_instance_of(Project).to receive(:add_import_job) + describe '#execute' do + it 'creates a project' do + expect { service.execute }.to change(Project, :count).by(1) + end + + it 'handle GitHub credentials' do + project = service.execute + + expect(project.import_url).to eq('https://asdffg@gitlab.com/asd/vim.git') + expect(project.safe_import_url).to eq('https://*****@gitlab.com/asd/vim.git') + expect(project.import_data.credentials).to eq(user: 'asdffg', password: nil) + end + + context 'when Github project is private' do + it 'sets project visibility to private' do + repo.private = true + + project = service.execute + + expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) + end + end + + context 'when Github project is public' do + before do + allow_any_instance_of(ApplicationSetting).to receive(:default_project_visibility).and_return(Gitlab::VisibilityLevel::INTERNAL) + end + + it 'sets project visibility to the default project visibility' do + repo.private = false - project_creator = Gitlab::GithubImport::ProjectCreator.new(repo, namespace, user, access_params) - project = project_creator.execute + project = service.execute - expect(project.import_url).to eq("https://asdffg@gitlab.com/asd/vim.git") - expect(project.safe_import_url).to eq("https://*****@gitlab.com/asd/vim.git") - expect(project.import_data.credentials).to eq(user: "asdffg", password: nil) - expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) + expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL) + end + end end end -- cgit v1.2.1 From a41b5e5bdc3ec5e8c44db5824665593799c530b6 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 31 Aug 2016 18:29:00 -0300 Subject: Update CHANGELOG --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 13ec1bb885f..9837b2edb9d 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -63,6 +63,7 @@ v 8.12.0 (unreleased) v 8.11.4 (unreleased) - Fix broken gitlab:backup:restore because of bad permissions on repo storage !6098 (Dirk Hörner) - Fix sorting issues by "last updated" doesn't work after import from GitHub + - GitHub importer use default project visibility for non-private projects - Creating an issue through our API now emails label subscribers !5720 - Block concurrent updates for Pipeline - Fix resolving conflicts on forks -- cgit v1.2.1 From 51c9cea86835427af983c30ddec7753949cb0bff Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Thu, 1 Sep 2016 09:42:18 +0200 Subject: Add MR Documentation description template --- .gitlab/merge_request_templates/Documentation.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 .gitlab/merge_request_templates/Documentation.md diff --git a/.gitlab/merge_request_templates/Documentation.md b/.gitlab/merge_request_templates/Documentation.md new file mode 100644 index 00000000000..d2a1eb56423 --- /dev/null +++ b/.gitlab/merge_request_templates/Documentation.md @@ -0,0 +1,14 @@ +See the general Documentation guidelines http://docs.gitlab.com/ce/development/doc_styleguide.html. + +## What does this MR do? + +(briefly describe what this MR is about) + +## Moving docs to a new location? + +See the guidelines: http://docs.gitlab.com/ce/development/doc_styleguide.html#changing-document-location + +- [ ] Make sure the old link is not removed and has its contents replaced with a link to the new location. +- [ ] Make sure internal links pointing to the document in question are not broken. +- [ ] Search and replace any links referring to old docs in GitLab Rails app, specifically under the `app/views/` directory. +- [ ] If working on CE, submit an MR to EE with the changes as well. -- cgit v1.2.1