From 0a7f7161198feaa9a4cae7c16669a0e6187aed33 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Thu, 17 Mar 2016 19:42:46 -0300 Subject: [PATCH] Code fixes --- app/assets/stylesheets/framework/blocks.scss | 18 +++++++++++ app/assets/stylesheets/framework/common.scss | 22 -------------- app/controllers/groups_controller.rb | 4 +-- app/finders/contributed_projects_finder.rb | 7 ++--- app/finders/groups_finder.rb | 10 ++++++- app/finders/joined_groups_finder.rb | 4 +-- app/finders/personal_projects_finder.rb | 17 +++++++---- app/models/ability.rb | 7 ++--- app/services/groups/base_service.rb | 16 ++++++++-- app/services/groups/create_service.rb | 19 ++++++------ app/services/groups/update_service.rb | 19 +----------- app/views/groups/show.html.haml | 10 +------ config/gitlab.yml.example | 4 --- ...roup_visibility_to_application_settings.rb | 12 ++------ lib/gitlab/current_settings.rb | 1 - lib/gitlab/visibility_level.rb | 13 ++++---- .../security/group/internal_access_spec.rb | 21 +++++++++---- .../security/group/private_access_spec.rb | 30 ++++++++++++------- .../security/group/public_access_spec.rb | 10 +++++++ spec/finders/groups_finder_spec.rb | 9 +++++- spec/finders/joined_groups_finder_spec.rb | 19 ++++++++++++ spec/finders/personal_projects_finder_spec.rb | 15 +++++++++- spec/services/groups/create_service_spec.rb | 12 ++++---- spec/support/group_access_helper.rb | 4 +++ 24 files changed, 178 insertions(+), 125 deletions(-) diff --git a/app/assets/stylesheets/framework/blocks.scss b/app/assets/stylesheets/framework/blocks.scss index d20b77ffae9..31084872367 100644 --- a/app/assets/stylesheets/framework/blocks.scss +++ b/app/assets/stylesheets/framework/blocks.scss @@ -115,6 +115,24 @@ color: #4c4e54; font-size: 23px; line-height: 1.1; + + h1 { + color: #313236; + margin-bottom: 6px; + font-size: 23px; + } + + .visibility-icon { + display: inline-block; + margin-left: 5px; + font-size: 18px; + color: $gray; + } + + p { + padding: 0 $gl-padding; + color: #5c5d5e; + } } .cover-desc { diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index f4608cd80bb..ff551f151f1 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -385,25 +385,3 @@ table { margin-right: -$gl-padding; border-top: 1px solid $border-color; } - -.cover-title{ - h1 { - color: #313236; - margin: 0; - margin-bottom: 6px; - font-size: 23px; - font-weight: normal; - } - - .visibility-icon { - display: inline-block; - margin-left: 5px; - font-size: 18px; - color: $gray; - } - - p { - padding: 0 $gl-padding; - color: #5c5d5e; - } -} diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 8243946c852..ba2057eb2c8 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -28,9 +28,9 @@ class GroupsController < Groups::ApplicationController end def create - @group = Group.new(group_params) + @group = Groups::CreateService.new(current_user, group_params).execute - if Groups::CreateService.new(@group, current_user, group_params).execute + if @group.persisted? redirect_to @group, notice: "Group '#{@group.name}' was successfully created." else render action: "new" diff --git a/app/finders/contributed_projects_finder.rb b/app/finders/contributed_projects_finder.rb index 4f7fe1c748b..f8b04dfa2aa 100644 --- a/app/finders/contributed_projects_finder.rb +++ b/app/finders/contributed_projects_finder.rb @@ -10,8 +10,9 @@ class ContributedProjectsFinder # visible by this user. # # Returns an ActiveRecord::Relation. + def execute(current_user = nil) - if current_user && !current_user.external? + if current_user relation = projects_visible_to_user(current_user) else relation = public_projects @@ -24,9 +25,7 @@ class ContributedProjectsFinder def projects_visible_to_user(current_user) authorized = @user.contributed_projects.visible_to_user(current_user) - - union = Gitlab::SQL::Union. - new([authorized.select(:id), public_projects.select(:id)]) + union = Gitlab::SQL::Union.new([authorized.select(:id), public_projects.select(:id)]) Project.where("projects.id IN (#{union.to_sql})") end diff --git a/app/finders/groups_finder.rb b/app/finders/groups_finder.rb index ce62f5e762f..30698f80231 100644 --- a/app/finders/groups_finder.rb +++ b/app/finders/groups_finder.rb @@ -14,9 +14,17 @@ class GroupsFinder def all_groups(current_user) if current_user - [current_user.authorized_groups, Group.unscoped.public_and_internal_only] + user_groups(current_user) else [Group.unscoped.public_only] end end + + def user_groups(current_user) + if current_user.external? + [current_user.authorized_groups, Group.unscoped.public_only] + else + [current_user.authorized_groups, Group.unscoped.public_and_internal_only] + end + end end diff --git a/app/finders/joined_groups_finder.rb b/app/finders/joined_groups_finder.rb index ff744689e3d..867eb661682 100644 --- a/app/finders/joined_groups_finder.rb +++ b/app/finders/joined_groups_finder.rb @@ -12,7 +12,7 @@ class JoinedGroupsFinder # # Returns an ActiveRecord::Relation. def execute(current_user = nil) - if current_user && !current_user.external? + if current_user relation = groups_visible_to_user(current_user) else relation = public_groups @@ -29,7 +29,7 @@ class JoinedGroupsFinder # "@user" that "current_user" also has access to. def groups_visible_to_user(current_user) base = @user.authorized_groups.visible_to_user(current_user) - extra = public_and_internal_groups + extra = current_user.external? ? public_groups : public_and_internal_groups union = Gitlab::SQL::Union.new([base.select(:id), extra.select(:id)]) Group.where("namespaces.id IN (#{union.to_sql})") diff --git a/app/finders/personal_projects_finder.rb b/app/finders/personal_projects_finder.rb index 0e2d915da54..34f33e2353b 100644 --- a/app/finders/personal_projects_finder.rb +++ b/app/finders/personal_projects_finder.rb @@ -11,7 +11,7 @@ class PersonalProjectsFinder # # Returns an ActiveRecord::Relation. def execute(current_user = nil) - if current_user && !current_user.external? + if current_user relation = projects_visible_to_user(current_user) else relation = public_projects @@ -23,10 +23,7 @@ class PersonalProjectsFinder private def projects_visible_to_user(current_user) - authorized = @user.personal_projects.visible_to_user(current_user) - - union = Gitlab::SQL::Union. - new([authorized.select(:id), public_and_internal_projects.select(:id)]) + union = Gitlab::SQL::Union.new(projects_for_user_ids(current_user)) Project.where("projects.id IN (#{union.to_sql})") end @@ -38,4 +35,14 @@ class PersonalProjectsFinder def public_and_internal_projects @user.personal_projects.public_and_internal_only end + + def projects_for_user_ids(current_user) + authorized = @user.personal_projects.visible_to_user(current_user) + + if current_user.external? + [authorized.select(:id), public_projects.select(:id)] + else + [authorized.select(:id), public_and_internal_projects.select(:id)] + end + end end diff --git a/app/models/ability.rb b/app/models/ability.rb index 134ae440c9c..ffcf05dcd33 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -295,11 +295,8 @@ class Ability end def can_read_group?(user, group) - if user.external? - group.public? || ProjectsFinder.new.execute(user, group: group).any? - else - user.admin? || group.public? || group.internal? || group.users.include?(user) || ProjectsFinder.new.execute(user, group: group).any? - end + user.admin? || group.public? || (group.internal? && !user.external?) || group.users.include?(user) || + ProjectsFinder.new.execute(user, group: group).any? end def namespace_abilities(user, namespace) diff --git a/app/services/groups/base_service.rb b/app/services/groups/base_service.rb index 644ec7c013e..053b6a05281 100644 --- a/app/services/groups/base_service.rb +++ b/app/services/groups/base_service.rb @@ -6,8 +6,20 @@ module Groups @group, @current_user, @params = group, user, params.dup end - def add_error_message(message) - group.errors.add(:visibility_level, message) + private + + def visibility_allowed_for_user?(level) + allowed_by_user = Gitlab::VisibilityLevel.allowed_for?(current_user, level) + @group.errors.add(:visibility_level, "You are not authorized to set this permission level.") unless allowed_by_user + allowed_by_user + end + + def visibility_allowed_for_project?(level) + projects_visibility = group.projects.pluck(:visibility_level) + + allowed_by_projects = !projects_visibility.any? { |project_visibility| level.to_i < project_visibility } + @group.errors.add(:visibility_level, "Cannot be changed. There are projects with higher visibility permissions.") unless allowed_by_projects + allowed_by_projects end end end diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb index e2875aafb94..38742369d82 100644 --- a/app/services/groups/create_service.rb +++ b/app/services/groups/create_service.rb @@ -1,17 +1,16 @@ module Groups class CreateService < Groups::BaseService - def execute - return false unless visibility_level_allowed?(params[:visibility_level]) - @group.name = @group.path.dup unless @group.name - @group.save(params) && @group.add_owner(current_user) + def initialize(user, params = {}) + @current_user, @params = user, params.dup + @group = Group.new(@params) end - private - - def visibility_level_allowed?(level) - allowed = Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level]) - add_error_message("Visibility level restricted by admin.") unless allowed - allowed + def execute + return @group unless visibility_allowed_for_user?(@params[:visibility_level]) + @group.name = @group.path.dup unless @group.name + @group.save + @group.add_owner(@current_user) + @group end end end diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb index a7382c1e07c..b910e0fde98 100644 --- a/app/services/groups/update_service.rb +++ b/app/services/groups/update_service.rb @@ -14,24 +14,7 @@ module Groups def visibility_level_allowed?(level) return true unless level.present? - allowed_by_projects = visibility_by_project(level) - allowed_by_user = visibility_by_user(level) - - allowed_by_projects && allowed_by_user - end - - def visibility_by_project(level) - projects_visibility = group.projects.pluck(:visibility_level) - - allowed_by_projects = !projects_visibility.any?{ |project_visibility| level.to_i < project_visibility } - add_error_message("Cannot be changed. There are projects with higher visibility permissions.") unless allowed_by_projects - allowed_by_projects - end - - def visibility_by_user(level) - allowed_by_user = Gitlab::VisibilityLevel.allowed_for?(current_user, level) - add_error_message("You are not authorized to set this permission level.") unless allowed_by_user - allowed_by_user + visibility_allowed_for_project?(level) && visibility_allowed_for_user?(level) end end end diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml index 2653a03059f..4be117667db 100644 --- a/app/views/groups/show.html.haml +++ b/app/views/groups/show.html.haml @@ -18,7 +18,7 @@ %h1 = @group.name - %span.visibility-icon.has_tooltip{data: { container: 'body', placement: 'left' }, title: "#{visibility_level_label(@group.visibility_level)} - #{group_visibility_description(@group)}"} + %span.visibility-icon.has_tooltip{data: { container: 'body', placement: 'left' }, title: "#{group_visibility_description(@group)}"} = visibility_level_icon(@group.visibility_level, fw: false) .cover-desc.username @@ -28,14 +28,6 @@ .cover-desc.description = markdown(@group.description, pipeline: :description) - %ul.nav-links - %li.active - = link_to "#activity", 'data-toggle' => 'tab' do - Activity - %li - = link_to "#projects", 'data-toggle' => 'tab' do - Projects - - if can?(current_user, :read_group, @group) %div{ class: container_class } .top-area diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index fc808e228de..05f127d622a 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -90,10 +90,6 @@ production: &base snippets: false builds: true - ## Default group features settings - default_groups_features: - visibility_level: 20 - ## Webhook settings # Number of seconds to wait for HTTP response after sending webhook HTTP POST request (default: 10) # webhook_timeout: 10 diff --git a/db/migrate/20160308212903_add_default_group_visibility_to_application_settings.rb b/db/migrate/20160308212903_add_default_group_visibility_to_application_settings.rb index b71322376fa..d5c78db0aa3 100644 --- a/db/migrate/20160308212903_add_default_group_visibility_to_application_settings.rb +++ b/db/migrate/20160308212903_add_default_group_visibility_to_application_settings.rb @@ -13,15 +13,9 @@ class AddDefaultGroupVisibilityToApplicationSettings < ActiveRecord::Migration end private - def allowed_visibility_level - default_visibility = Settings.gitlab.default_groups_features['visibility_level'] - restricted_levels = current_application_settings.restricted_visibility_levels - return default_visibility unless restricted_levels.present? - if restricted_levels.include?(default_visibility) - Gitlab::VisibilityLevel.values.select{ |vis_level| vis_level unless restricted_levels.include?(vis_level) }.last - else - default_visibility - end + def allowed_visibility_level + allowed_levels = Gitlab::VisibilityLevel.values - current_application_settings.restricted_visibility_levels + allowed_levels.max end end diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index a50d8e3d5a8..761b63e98f6 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -29,7 +29,6 @@ module Gitlab session_expire_delay: Settings.gitlab['session_expire_delay'], default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'], default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'], - default_group_visibility: Settings.gitlab.default_groups_features['visibility_level'], restricted_signup_domains: Settings.gitlab['restricted_signup_domains'], import_sources: ['github','bitbucket','gitlab','gitorious','google_code','fogbugz','git'], shared_runners_enabled: Settings.gitlab_ci['shared_runners_enabled'], diff --git a/lib/gitlab/visibility_level.rb b/lib/gitlab/visibility_level.rb index f6e0dd6afc0..f193fb282ab 100644 --- a/lib/gitlab/visibility_level.rb +++ b/lib/gitlab/visibility_level.rb @@ -6,19 +6,18 @@ module Gitlab module VisibilityLevel extend CurrentSettings + extend ActiveSupport::Concern + + included do + scope :public_only, -> { where(visibility_level: PUBLIC) } + scope :public_and_internal_only, -> { where(visibility_level: [PUBLIC, INTERNAL] ) } + end PRIVATE = 0 unless const_defined?(:PRIVATE) INTERNAL = 10 unless const_defined?(:INTERNAL) PUBLIC = 20 unless const_defined?(:PUBLIC) class << self - def included(base) - base.class_eval do - scope :public_only, -> { where(visibility_level: PUBLIC) } - scope :public_and_internal_only, -> { where(visibility_level: [PUBLIC, INTERNAL] ) } - end - end - def values options.values end diff --git a/spec/features/security/group/internal_access_spec.rb b/spec/features/security/group/internal_access_spec.rb index 4e781c23ee0..e44d4c32921 100644 --- a/spec/features/security/group/internal_access_spec.rb +++ b/spec/features/security/group/internal_access_spec.rb @@ -12,9 +12,12 @@ describe 'Internal group access', feature: true do it { is_expected.to be_allowed_for group_member(:master) } it { is_expected.to be_allowed_for group_member(:reporter) } it { is_expected.to be_allowed_for group_member(:guest) } + it { is_expected.to be_allowed_for external_guest } it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for :user } - it { is_expected.to_not be_allowed_for :visitor } + it { is_expected.to be_denied_for :visitor } + it { is_expected.to be_denied_for :external } + end context "when user in group project" do @@ -31,9 +34,11 @@ describe 'Internal group access', feature: true do it { is_expected.to be_allowed_for group_member(:master) } it { is_expected.to be_allowed_for group_member(:reporter) } it { is_expected.to be_allowed_for group_member(:guest) } + it { is_expected.to be_allowed_for external_guest } it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for :user } - it { is_expected.to_not be_allowed_for :visitor } + it { is_expected.to be_denied_for :visitor } + it { is_expected.to be_denied_for :external } end context "when user in group project" do @@ -50,9 +55,11 @@ describe 'Internal group access', feature: true do it { is_expected.to be_allowed_for group_member(:master) } it { is_expected.to be_allowed_for group_member(:reporter) } it { is_expected.to be_allowed_for group_member(:guest) } + it { is_expected.to be_allowed_for external_guest } it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for :user } - it { is_expected.to_not be_allowed_for :visitor } + it { is_expected.to be_denied_for :visitor } + it { is_expected.to be_denied_for :external } end context "when user in group project" do @@ -70,9 +77,11 @@ describe 'Internal group access', feature: true do it { is_expected.to be_allowed_for group_member(:master) } it { is_expected.to be_allowed_for group_member(:reporter) } it { is_expected.to be_allowed_for group_member(:guest) } + it { is_expected.to be_allowed_for external_guest } it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for :user } - it { is_expected.to_not be_allowed_for :visitor } + it { is_expected.to be_denied_for :visitor } + it { is_expected.to be_denied_for :external } end context "when user in group project" do @@ -89,9 +98,11 @@ describe 'Internal group access', feature: true do it { is_expected.to be_allowed_for group_member(:master) } it { is_expected.to be_allowed_for group_member(:reporter) } it { is_expected.to be_allowed_for group_member(:guest) } + it { is_expected.to be_allowed_for external_guest } it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for :user } - it { is_expected.to_not be_allowed_for :visitor } + it { is_expected.to be_denied_for :visitor } + it { is_expected.to be_denied_for :external } end context "when user in group project" do diff --git a/spec/features/security/group/private_access_spec.rb b/spec/features/security/group/private_access_spec.rb index 0d01310b449..8d8c61a618f 100644 --- a/spec/features/security/group/private_access_spec.rb +++ b/spec/features/security/group/private_access_spec.rb @@ -14,9 +14,11 @@ describe 'Private group access', feature: true do it { is_expected.to be_allowed_for group_member(:master) } it { is_expected.to be_allowed_for group_member(:reporter) } it { is_expected.to be_allowed_for group_member(:guest) } + it { is_expected.to be_allowed_for external_guest } it { is_expected.to be_allowed_for :admin } - it { is_expected.to_not be_allowed_for :user } - it { is_expected.to_not be_allowed_for :visitor } + it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :visitor } + it { is_expected.to be_denied_for :external } end context "when user in group project" do @@ -33,9 +35,11 @@ describe 'Private group access', feature: true do it { is_expected.to be_allowed_for group_member(:master) } it { is_expected.to be_allowed_for group_member(:reporter) } it { is_expected.to be_allowed_for group_member(:guest) } + it { is_expected.to be_allowed_for external_guest } it { is_expected.to be_allowed_for :admin } - it { is_expected.to_not be_allowed_for :user } - it { is_expected.to_not be_allowed_for :visitor } + it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :visitor } + it { is_expected.to be_denied_for :external } end context "when user in group project" do @@ -52,9 +56,11 @@ describe 'Private group access', feature: true do it { is_expected.to be_allowed_for group_member(:master) } it { is_expected.to be_allowed_for group_member(:reporter) } it { is_expected.to be_allowed_for group_member(:guest) } + it { is_expected.to be_allowed_for external_guest } it { is_expected.to be_allowed_for :admin } - it { is_expected.to_not be_allowed_for :user } - it { is_expected.to_not be_allowed_for :visitor } + it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :visitor } + it { is_expected.to be_denied_for :external } end context "when user in group project" do @@ -72,9 +78,11 @@ describe 'Private group access', feature: true do it { is_expected.to be_allowed_for group_member(:master) } it { is_expected.to be_allowed_for group_member(:reporter) } it { is_expected.to be_allowed_for group_member(:guest) } + it { is_expected.to be_allowed_for external_guest } it { is_expected.to be_allowed_for :admin } - it { is_expected.to_not be_allowed_for :user } - it { is_expected.to_not be_allowed_for :visitor } + it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :visitor } + it { is_expected.to be_denied_for :external } end context "when user in group project" do @@ -91,9 +99,11 @@ describe 'Private group access', feature: true do it { is_expected.to be_allowed_for group_member(:master) } it { is_expected.to be_allowed_for group_member(:reporter) } it { is_expected.to be_allowed_for group_member(:guest) } + it { is_expected.to be_allowed_for external_guest } it { is_expected.to be_allowed_for :admin } - it { is_expected.to_not be_allowed_for :user } - it { is_expected.to_not be_allowed_for :visitor } + it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :visitor } + it { is_expected.to be_denied_for :external } end context "when user in group project" do diff --git a/spec/features/security/group/public_access_spec.rb b/spec/features/security/group/public_access_spec.rb index 75d208f2949..5ff982504c5 100644 --- a/spec/features/security/group/public_access_spec.rb +++ b/spec/features/security/group/public_access_spec.rb @@ -14,9 +14,11 @@ describe 'Public group access', feature: true do it { is_expected.to be_allowed_for group_member(:master) } it { is_expected.to be_allowed_for group_member(:reporter) } it { is_expected.to be_allowed_for group_member(:guest) } + it { is_expected.to be_allowed_for external_guest } it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :visitor } + it { is_expected.to be_allowed_for :external } end context "when user in group project" do @@ -33,9 +35,11 @@ describe 'Public group access', feature: true do it { is_expected.to be_allowed_for group_member(:master) } it { is_expected.to be_allowed_for group_member(:reporter) } it { is_expected.to be_allowed_for group_member(:guest) } + it { is_expected.to be_allowed_for external_guest } it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :visitor } + it { is_expected.to be_allowed_for :external } end context "when user in group project" do @@ -52,9 +56,11 @@ describe 'Public group access', feature: true do it { is_expected.to be_allowed_for group_member(:master) } it { is_expected.to be_allowed_for group_member(:reporter) } it { is_expected.to be_allowed_for group_member(:guest) } + it { is_expected.to be_allowed_for external_guest } it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :visitor } + it { is_expected.to be_allowed_for :external } end context "when user in group project" do @@ -72,9 +78,11 @@ describe 'Public group access', feature: true do it { is_expected.to be_allowed_for group_member(:master) } it { is_expected.to be_allowed_for group_member(:reporter) } it { is_expected.to be_allowed_for group_member(:guest) } + it { is_expected.to be_allowed_for external_guest } it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :visitor } + it { is_expected.to be_allowed_for :external } end context "when user in group project" do @@ -91,9 +99,11 @@ describe 'Public group access', feature: true do it { is_expected.to be_allowed_for group_member(:master) } it { is_expected.to be_allowed_for group_member(:reporter) } it { is_expected.to be_allowed_for group_member(:guest) } + it { is_expected.to be_allowed_for external_guest } it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :visitor } + it { is_expected.to be_allowed_for :external } end context "when user in group project" do diff --git a/spec/finders/groups_finder_spec.rb b/spec/finders/groups_finder_spec.rb index ed24954af7a..d0fd7af8cc3 100644 --- a/spec/finders/groups_finder_spec.rb +++ b/spec/finders/groups_finder_spec.rb @@ -18,7 +18,14 @@ describe GroupsFinder do describe 'with a user' do subject { finder.execute(user) } - it { is_expected.to eq([public_group, internal_group]) } + context 'normal user' do + it { is_expected.to eq([public_group, internal_group]) } + end + + context 'external user' do + before { user.update_attribute(external: true) } + it { is_expected.to eq([public_group]) } + end end end end diff --git a/spec/finders/joined_groups_finder_spec.rb b/spec/finders/joined_groups_finder_spec.rb index e2f6c593638..7b6fc837e5f 100644 --- a/spec/finders/joined_groups_finder_spec.rb +++ b/spec/finders/joined_groups_finder_spec.rb @@ -46,6 +46,25 @@ describe JoinedGroupsFinder do it { is_expected.to eq([public_group, private_group]) } end + + context 'external users' do + before do + profile_visitor.update_attributes(external: true) + public_group.add_user(profile_owner, Gitlab::Access::MASTER) + internal_group.add_user(profile_owner, Gitlab::Access::MASTER) + end + + subject { finder.execute(profile_visitor) } + + it "doest not show internal groups if not member" do + expect(subject).to eq([public_group]) + end + + it "shows internal groups if authorized" do + internal_group.add_user(profile_visitor, Gitlab::Access::MASTER) + expect(subject).to eq([public_group, internal_group]) + end + end end end end diff --git a/spec/finders/personal_projects_finder_spec.rb b/spec/finders/personal_projects_finder_spec.rb index 38817add456..8758f61903c 100644 --- a/spec/finders/personal_projects_finder_spec.rb +++ b/spec/finders/personal_projects_finder_spec.rb @@ -16,6 +16,11 @@ describe PersonalProjectsFinder do path: 'B') end + let!(:internal_project) do + create(:project, :internal, namespace: source_user.namespace, name: 'c', + path: 'C') + end + before do private_project.team << [current_user, Gitlab::Access::DEVELOPER] end @@ -29,6 +34,14 @@ describe PersonalProjectsFinder do describe 'with a current user' do subject { finder.execute(current_user) } - it { is_expected.to eq([private_project, public_project]) } + context 'normal user' do + it { is_expected.to eq([internal_project, private_project, public_project]) } + end + + context 'external' do + before { current_user.update_attributes(external: true) } + + it { is_expected.to eq([private_project, public_project]) } + end end end diff --git a/spec/services/groups/create_service_spec.rb b/spec/services/groups/create_service_spec.rb index 7dbc5297978..b938a2f0c05 100644 --- a/spec/services/groups/create_service_spec.rb +++ b/spec/services/groups/create_service_spec.rb @@ -1,22 +1,20 @@ require 'spec_helper' describe Groups::CreateService, services: true do - let!(:user) { create(:user) } - let!(:private_group) { create(:group, visibility_level: Gitlab::VisibilityLevel::PRIVATE) } - let!(:internal_group) { create(:group, visibility_level: Gitlab::VisibilityLevel::INTERNAL) } - let!(:public_group) { create(:group, visibility_level: Gitlab::VisibilityLevel::PUBLIC) } + let!(:user) { create(:user) } + let!(:group_params) { { path: "group_path", visibility_level: Gitlab::VisibilityLevel::PUBLIC } } describe "execute" do - let!(:service) { described_class.new(public_group, user, visibility_level: Gitlab::VisibilityLevel::PUBLIC ) } + let!(:service) { described_class.new(user, group_params ) } subject { service.execute } context "create groups without restricted visibility level" do - it { is_expected.to be_truthy } + it { is_expected.to be_persisted } end context "cannot create group with restricted visibility level" do before { allow(current_application_settings).to receive(:restricted_visibility_levels).and_return([Gitlab::VisibilityLevel::PUBLIC]) } - it { is_expected.to be_falsy } + it { is_expected.to_not be_persisted } end end end diff --git a/spec/support/group_access_helper.rb b/spec/support/group_access_helper.rb index a1a8fb2bd72..c8ed0e406a1 100644 --- a/spec/support/group_access_helper.rb +++ b/spec/support/group_access_helper.rb @@ -14,4 +14,8 @@ module GroupAccessHelper create(:user).tap { |user| grp.add_user(user, level) } end + + def external_guest(grp=group()) + create(:user, external: true).tap { |user| grp.add_user(user, Gitlab::Access::GUEST) } + end end