diff options
50 files changed, 875 insertions, 106 deletions
diff --git a/Gemfile.lock b/Gemfile.lock index f4f5649eb75..9c785c903c0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1049,6 +1049,3 @@ DEPENDENCIES web-console (~> 2.0) webmock (~> 1.21.0) wikicloth (= 0.8.1) - -BUNDLED WITH - 1.11.2 diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index ff551f151f1..f4608cd80bb 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -385,3 +385,25 @@ 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/assets/stylesheets/framework/mobile.scss b/app/assets/stylesheets/framework/mobile.scss index 3bfac2ad9b5..d088228fe4c 100644 --- a/app/assets/stylesheets/framework/mobile.scss +++ b/app/assets/stylesheets/framework/mobile.scss @@ -48,7 +48,7 @@ display: block; } - .project-home-desc { + #project-home-desc { font-size: 21px; } diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index b1b76edfb32..1a7b0c1e278 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -61,28 +61,6 @@ } } - .project-home-desc { - 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; - } - } - .project-repo-buttons { margin-top: 20px; margin-bottom: 0px; diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 04a99d8c84a..ed9f6031389 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -61,6 +61,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :session_expire_delay, :default_project_visibility, :default_snippet_visibility, + :default_group_visibility, :restricted_signup_domains_raw, :version_check_enabled, :admin_notification_email, diff --git a/app/controllers/explore/groups_controller.rb b/app/controllers/explore/groups_controller.rb index a9bf4321f73..9575a87ee41 100644 --- a/app/controllers/explore/groups_controller.rb +++ b/app/controllers/explore/groups_controller.rb @@ -1,6 +1,6 @@ class Explore::GroupsController < Explore::ApplicationController def index - @groups = Group.order_id_desc + @groups = GroupsFinder.new.execute(current_user) @groups = @groups.search(params[:search]) if params[:search].present? @groups = @groups.sort(@sort = params[:sort]) @groups = @groups.page(params[:page]).per(PER_PAGE) diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 06c5c8be9a5..8243946c852 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -9,7 +9,7 @@ class GroupsController < Groups::ApplicationController before_action :group, except: [:index, :new, :create] # Authorize - before_action :authorize_read_group!, except: [:index, :show, :new, :create, :autocomplete] + before_action :authorize_read_group!, except: [:index, :new, :create] before_action :authorize_admin_group!, only: [:edit, :update, :destroy, :projects] before_action :authorize_create_group!, only: [:new, :create] @@ -29,10 +29,8 @@ class GroupsController < Groups::ApplicationController def create @group = Group.new(group_params) - @group.name = @group.path.dup unless @group.name - if @group.save - @group.add_owner(current_user) + if Groups::CreateService.new(@group, current_user, group_params).execute redirect_to @group, notice: "Group '#{@group.name}' was successfully created." else render action: "new" @@ -83,7 +81,7 @@ class GroupsController < Groups::ApplicationController end def update - if @group.update_attributes(group_params) + if Groups::UpdateService.new(@group, current_user, group_params).execute redirect_to edit_group_path(@group), notice: "Group '#{@group.name}' was successfully updated." else render action: "edit" @@ -109,7 +107,7 @@ class GroupsController < Groups::ApplicationController # Dont allow unauthorized access to group def authorize_read_group! - unless @group and (@projects.present? or can?(current_user, :read_group, @group)) + unless can?(current_user, :read_group, @group) if current_user.nil? return authenticate_user! else @@ -135,7 +133,7 @@ class GroupsController < Groups::ApplicationController end def group_params - params.require(:group).permit(:name, :description, :path, :avatar, :public, :share_with_group_lock) + params.require(:group).permit(:name, :description, :path, :avatar, :public, :visibility_level, :share_with_group_lock) end def load_events diff --git a/app/controllers/namespaces_controller.rb b/app/controllers/namespaces_controller.rb index 282012c60a1..5a94dcb0dbd 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 + elsif group && can?(current_user, :read_group, namespace) redirect_to group_path(group) elsif current_user.nil? authenticate_user! diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index e10c633690f..481d00d6aae 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -108,7 +108,7 @@ class UsersController < ApplicationController end def load_groups - @groups = @user.groups.order_id_desc + @groups = JoinedGroupsFinder.new(@user).execute(current_user) end def projects_for_current_user diff --git a/app/finders/groups_finder.rb b/app/finders/groups_finder.rb new file mode 100644 index 00000000000..ce62f5e762f --- /dev/null +++ b/app/finders/groups_finder.rb @@ -0,0 +1,22 @@ +class GroupsFinder + def execute(current_user = nil) + segments = all_groups(current_user) + + if segments.length > 1 + union = Gitlab::SQL::Union.new(segments.map { |s| s.select(:id) }) + Group.where("namespaces.id IN (#{union.to_sql})").order_id_desc + else + segments.first + end + end + + private + + def all_groups(current_user) + if current_user + [current_user.authorized_groups, Group.unscoped.public_and_internal_only] + else + [Group.unscoped.public_only] + end + end +end diff --git a/app/finders/joined_groups_finder.rb b/app/finders/joined_groups_finder.rb new file mode 100644 index 00000000000..fbdf492c965 --- /dev/null +++ b/app/finders/joined_groups_finder.rb @@ -0,0 +1,45 @@ +#Shows only authorized groups of a user +class JoinedGroupsFinder + def initialize(user) + @user = user + end + + # Finds the groups of the source user, optionally limited to those visible to + # the current user. + # + # current_user - If given the groups of "@user" will only include the groups + # "current_user" can also see. + # + # Returns an ActiveRecord::Relation. + def execute(current_user = nil) + if current_user + relation = groups_visible_to_user(current_user) + else + relation = public_groups + end + + relation.order_id_desc + end + + private + + # Returns the groups the user in "current_user" can see. + # + # This list includes all public/internal projects as well as the projects of + # "@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 + union = Gitlab::SQL::Union.new([base.select(:id), extra.select(:id)]) + + Group.where("namespaces.id IN (#{union.to_sql})") + end + + def public_groups + @user.authorized_groups.public_only + end + + def public_and_internal_groups + @user.authorized_groups.public_and_internal_only + end +end diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index 1d36969cd62..42e09149bd7 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -19,6 +19,10 @@ module GroupsHelper end end + def can_change_group_visibility_level?(group) + can?(current_user, :change_visibility_level, group) + end + def group_icon(group) if group.is_a?(String) group = Group.find_by(path: group) @@ -39,4 +43,8 @@ module GroupsHelper full_title end end + + def group_visibility_description(group) + "#{visibility_level_label(group.visibility_level)} - #{group_visibility_level_description(group.visibility_level)}" + end end diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb index 71d33b445c2..930cc883634 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -19,6 +19,8 @@ module VisibilityLevelHelper case form_model when Project project_visibility_level_description(level) + when Group + group_visibility_level_description(level) when Snippet snippet_visibility_level_description(level, form_model) end @@ -35,6 +37,17 @@ module VisibilityLevelHelper end end + def group_visibility_level_description(level) + case level + when Gitlab::VisibilityLevel::PRIVATE + "The group can be accessed only by members." + when Gitlab::VisibilityLevel::INTERNAL + "The group can be accessed by any logged user." + when Gitlab::VisibilityLevel::PUBLIC + "The group can be accessed without any authentication." + end + end + def snippet_visibility_level_description(level, snippet = nil) case level when Gitlab::VisibilityLevel::PRIVATE @@ -67,6 +80,10 @@ module VisibilityLevelHelper current_application_settings.default_snippet_visibility end + def default_group_visibility + current_application_settings.default_group_visibility + end + def skip_level?(form_model, level) form_model.is_a?(Project) && !form_model.visibility_level_allowed?(level) diff --git a/app/models/ability.rb b/app/models/ability.rb index ccac08b7d3f..455ea7bcc69 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -83,7 +83,7 @@ class Ability subject.group end - if group && group.projects.public_only.any? + if group && group.public? [:read_group] else [] @@ -271,11 +271,9 @@ class Ability def group_abilities(user, group) rules = [] - if user.admin? || group.users.include?(user) || ProjectsFinder.new.execute(user, group: group).any? - rules << :read_group - end + rules << :read_group if can_read_group?(user, group) - # Only group masters and group owners can create new projects in group + # Only group masters and group owners can create new projects and change permission level if group.has_master?(user) || group.has_owner?(user) || user.admin? rules += [ :create_projects, @@ -288,13 +286,19 @@ class Ability rules += [ :admin_group, :admin_namespace, - :admin_group_member + :admin_group_member, + :change_visibility_level ] end rules.flatten end + def can_read_group?(user, group) + user.admin? || group.public? || group.internal? || group.users.include?(user) || + ProjectsFinder.new.execute(user, group: group).any? + end + def namespace_abilities(user, namespace) rules = [] diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 269056e0e77..c4879598c4e 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -18,6 +18,7 @@ # max_attachment_size :integer default(10), not null # default_project_visibility :integer # default_snippet_visibility :integer +# default_group_visibility :integer # restricted_signup_domains :text # user_oauth_applications :boolean default(TRUE) # after_sign_out_path :string(255) diff --git a/app/models/group.rb b/app/models/group.rb index 9919ca112dc..b094a65e3d6 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -2,15 +2,16 @@ # # Table name: namespaces # -# id :integer not null, primary key -# name :string(255) not null -# path :string(255) not null -# owner_id :integer -# created_at :datetime -# updated_at :datetime -# type :string(255) -# description :string(255) default(""), not null -# avatar :string(255) +# id :integer not null, primary key +# name :string(255) not null +# path :string(255) not null +# owner_id :integer +# visibility_level :integer default(20), not null +# created_at :key => "value", datetime +# updated_at :datetime +# type :string(255) +# description :string(255) default(""), not null +# avatar :string(255) # require 'carrierwave/orm/activerecord' @@ -18,6 +19,7 @@ require 'file_size_validator' class Group < Namespace include Gitlab::ConfigHelper + include Gitlab::VisibilityLevel include Referable has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember' @@ -74,6 +76,10 @@ class Group < Namespace name end + def visibility_level_field + visibility_level + end + def avatar_url(size = nil) if avatar.present? [gitlab_config.url, avatar.url].join diff --git a/app/models/project.rb b/app/models/project.rb index ab4913e99a8..2828385a5f6 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -215,8 +215,6 @@ class Project < ActiveRecord::Base scope :in_group_namespace, -> { joins(:group) } scope :personal, ->(user) { where(namespace_id: user.namespace_id) } scope :joined, ->(user) { where('namespace_id != ?', user.namespace_id) } - scope :public_only, -> { where(visibility_level: Project::PUBLIC) } - scope :public_and_internal_only, -> { where(visibility_level: Project.public_and_internal_levels) } scope :non_archived, -> { where(archived: false) } scope :for_milestones, ->(ids) { joins(:milestones).where('milestones.id' => ids).distinct } @@ -964,8 +962,10 @@ class Project < ActiveRecord::Base end def visibility_level_allowed?(level) - return true unless forked? - Gitlab::VisibilityLevel.allowed_fork_levels(forked_from_project.visibility_level).include?(level.to_i) + allowed_by_forks = forked? ? Gitlab::VisibilityLevel.allowed_fork_levels(forked_from_project.visibility_level).include?(level.to_i) : true + allowed_by_groups = group.present? ? level.to_i <= group.visibility_level : true + + allowed_by_forks && allowed_by_groups end def runners_token diff --git a/app/services/groups/base_service.rb b/app/services/groups/base_service.rb new file mode 100644 index 00000000000..644ec7c013e --- /dev/null +++ b/app/services/groups/base_service.rb @@ -0,0 +1,13 @@ +module Groups + class BaseService + attr_accessor :group, :current_user, :params + + def initialize(group, user, params = {}) + @group, @current_user, @params = group, user, params.dup + end + + def add_error_message(message) + group.errors.add(:visibility_level, message) + end + end +end diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb new file mode 100644 index 00000000000..e2875aafb94 --- /dev/null +++ b/app/services/groups/create_service.rb @@ -0,0 +1,17 @@ +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) + 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 + end + end +end diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb new file mode 100644 index 00000000000..a7382c1e07c --- /dev/null +++ b/app/services/groups/update_service.rb @@ -0,0 +1,37 @@ +#Checks visibility level permission check before updating a group +#Do not allow to put Group visibility level smaller than its projects +#Do not allow unauthorized permission levels + +module Groups + class UpdateService < Groups::BaseService + def execute + return false unless visibility_level_allowed?(params[:visibility_level]) + group.update_attributes(params) + end + + private + + 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 + end + end +end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index a6820183bee..4c121106bda 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -11,8 +11,8 @@ module Projects # Make sure that the user is allowed to use the specified visibility # level - unless Gitlab::VisibilityLevel.allowed_for?(current_user, - params[:visibility_level]) + + unless visibility_level_allowed? deny_visibility_level(@project) return @project end @@ -100,5 +100,9 @@ module Projects @project.import_start if @project.import? end + + def visibility_level_allowed? + Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level]) && @project.visibility_level_allowed?(@project.visibility_level) + end end end diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index b30dfd109ea..0350995d03d 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -19,6 +19,10 @@ = f.label :default_snippet_visibility, class: 'control-label col-sm-2' .col-sm-10 = render('shared/visibility_radios', model_method: :default_snippet_visibility, form: f, selected_level: @application_setting.default_snippet_visibility, form_model: ProjectSnippet.new) + .form-group.group-visibility-level-holder + = f.label :default_group_visibility, class: 'control-label col-sm-2' + .col-sm-10 + = render('shared/visibility_radios', model_method: :default_group_visibility, form: f, selected_level: @application_setting.default_group_visibility, form_model: Group.new) .form-group = f.label :restricted_visibility_levels, class: 'control-label col-sm-2' .col-sm-10 diff --git a/app/views/groups/edit.html.haml b/app/views/groups/edit.html.haml index 83936d39b16..ea5a0358392 100644 --- a/app/views/groups/edit.html.haml +++ b/app/views/groups/edit.html.haml @@ -23,6 +23,8 @@ %hr = link_to 'Remove avatar', group_avatar_path(@group.to_param), data: { confirm: "Group avatar will be removed. Are you sure?"}, method: :delete, class: "btn btn-remove btn-sm remove-avatar" + = render 'shared/visibility_level', f: f, visibility_level: @group.visibility_level, can_change_visibility_level: can_change_group_visibility_level?(@group), form_model: @group + .form-group %hr = f.label :share_with_group_lock, class: 'control-label' do @@ -32,6 +34,7 @@ = f.check_box :share_with_group_lock %span.descr Prevent sharing a project with another group within this group + .form-actions = f.submit 'Save group', class: "btn btn-save" diff --git a/app/views/groups/new.html.haml b/app/views/groups/new.html.haml index 4bc31cabea6..30ab8aeba13 100644 --- a/app/views/groups/new.html.haml +++ b/app/views/groups/new.html.haml @@ -17,6 +17,8 @@ .col-sm-10 = render 'shared/choose_group_avatar_button', f: f + = render 'shared/visibility_level', f: f, visibility_level: default_group_visibility, can_change_visibility_level: true, form_model: @group + .form-group .col-sm-offset-2.col-sm-10 = render 'shared/group_tips' diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml index 23a34ac36dd..2653a03059f 100644 --- a/app/views/groups/show.html.haml +++ b/app/views/groups/show.html.haml @@ -1,8 +1,5 @@ - @no_container = true -- unless can?(current_user, :read_group, @group) - - @disable_search_panel = true - = content_for :meta_tags do - if current_user = auto_discovery_link_tag(:atom, group_url(@group, format: :atom, private_token: current_user.private_token), title: "#{@group.name} activity") @@ -18,7 +15,11 @@ = link_to group_icon(@group), target: '_blank' do = image_tag group_icon(@group), class: "avatar group-avatar s90" .cover-title - = @group.name + %h1 + = @group.name + + %span.visibility-icon.has_tooltip{data: { container: 'body', placement: 'left' }, title: "#{visibility_level_label(@group.visibility_level)} - #{group_visibility_description(@group)}"} + = visibility_level_icon(@group.visibility_level, fw: false) .cover-desc.username @#{@group.path} @@ -27,6 +28,14 @@ .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/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index 77d01a7736c..714da410f56 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -7,9 +7,8 @@ .navbar-collapse.collapse %ul.nav.navbar-nav.pull-right - - unless @disable_search_panel - %li.hidden-sm.hidden-xs - = render 'layouts/search' + %li.hidden-sm.hidden-xs + = render 'layouts/search' %li.visible-sm.visible-xs = link_to search_path, title: 'Search', data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do = icon('search') diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml index b45df44f270..e8434b5292c 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -2,7 +2,7 @@ .project-home-panel.cover-block.clearfix{:class => ("empty-project" if empty_repo)} .project-identicon-holder = project_icon(@project, alt: '', class: 'project-avatar avatar s90') - .project-home-desc + .cover-title#project-home-desc %h1 = @project.name %span.visibility-icon.has_tooltip{data: { container: 'body' }, diff --git a/app/views/shared/_group_tips.html.haml b/app/views/shared/_group_tips.html.haml index e5cf783beb7..46e4340511a 100644 --- a/app/views/shared/_group_tips.html.haml +++ b/app/views/shared/_group_tips.html.haml @@ -1,6 +1,5 @@ %ul %li A group is a collection of several projects - %li Groups are private by default %li Members of a group may only view projects they have permission to access %li Group project URLs are prefixed with the group namespace %li Existing projects may be moved into a group diff --git a/app/views/shared/groups/_group.html.haml b/app/views/shared/groups/_group.html.haml index fb9a8db0889..82eeb2088c8 100644 --- a/app/views/shared/groups/_group.html.haml +++ b/app/views/shared/groups/_group.html.haml @@ -21,6 +21,9 @@ = icon('users') = number_with_delimiter(group.users.count) + %span{title: group_visibility_description(group)} + = visibility_level_icon(group.visibility_level, fw: false) + = image_tag group_icon(group), class: "avatar s40 hidden-xs" = link_to group, class: 'group-name title' do = group.name diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 05f127d622a..fc808e228de 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -90,6 +90,10 @@ 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/20160301124843_add_visibility_level_to_groups.rb b/db/migrate/20160301124843_add_visibility_level_to_groups.rb new file mode 100644 index 00000000000..cef553981e7 --- /dev/null +++ b/db/migrate/20160301124843_add_visibility_level_to_groups.rb @@ -0,0 +1,6 @@ +class AddVisibilityLevelToGroups < ActiveRecord::Migration + def change + #All groups public by default + add_column :namespaces, :visibility_level, :integer, null: false, default: 20 + end +end 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 new file mode 100644 index 00000000000..b71322376fa --- /dev/null +++ b/db/migrate/20160308212903_add_default_group_visibility_to_application_settings.rb @@ -0,0 +1,27 @@ +#Create visibility level field on DB +#Sets default_visibility_level to value on settings if not restricted +#If value is restricted takes higher visibility level allowed + +class AddDefaultGroupVisibilityToApplicationSettings < ActiveRecord::Migration + def up + add_column :application_settings, :default_group_visibility, :integer + execute("update application_settings set default_group_visibility = #{allowed_visibility_level}") + end + + def down + remove_column :application_settings, :default_group_visibility + 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 + end +end diff --git a/db/schema.rb b/db/schema.rb index b29b56eeb0b..f5e3e5bc861 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -77,6 +77,7 @@ ActiveRecord::Schema.define(version: 20160314143402) do t.boolean "akismet_enabled", default: false t.string "akismet_api_key" t.boolean "email_author_in_body", default: false + t.integer "default_group_visibility" end create_table "audit_events", force: :cascade do |t| @@ -586,6 +587,7 @@ ActiveRecord::Schema.define(version: 20160314143402) do t.string "type" t.string "description", default: "", null: false t.string "avatar" + t.integer "visibility_level", default: 0, null: false t.boolean "share_with_group_lock", default: false end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 71197205f34..20565e368dd 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -340,6 +340,7 @@ module API expose :session_expire_delay expose :default_project_visibility expose :default_snippet_visibility + expose :default_group_visibility expose :restricted_signup_domains expose :user_oauth_applications expose :after_sign_out_path diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index 761b63e98f6..a50d8e3d5a8 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -29,6 +29,7 @@ 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 3160a3c7582..f6e0dd6afc0 100644 --- a/lib/gitlab/visibility_level.rb +++ b/lib/gitlab/visibility_level.rb @@ -12,6 +12,13 @@ module Gitlab 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/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 938e97298b6..91db3fd1ee2 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -20,4 +20,43 @@ describe GroupsController do end end end + + describe 'GET show' do + let(:group) { create(:group, visibility_level: 20) } + + it 'checks if group can be read' do + expect(controller).to receive(:authorize_read_group!) + get :show, id: group.path + end + end + + describe 'POST create' do + before { sign_in(create(:user)) } + + it 'checks if group can be created' do + expect(controller).to receive(:authorize_create_group!) + post :create, { group: { name: "any params" } } + end + end + + describe 'DELETE destroy' do + before { sign_in(create(:user)) } + let(:group) { create(:group, visibility_level: 20) } + + it 'checks if group can be deleted' do + expect(controller).to receive(:authorize_admin_group!) + delete :destroy, id: group.path + end + end + + describe 'PUT update' do + before { sign_in(create(:user)) } + let(:group) { create(:group, visibility_level: 20) } + + it 'checks if group can be updated' do + expect_any_instance_of(Groups::UpdateService).to receive(:execute) + expect(controller).to receive(:authorize_admin_group!) + put :update, id: group.path, group: { name: 'test' } + end + end end diff --git a/spec/features/security/group/internal_access_spec.rb b/spec/features/security/group/internal_access_spec.rb new file mode 100644 index 00000000000..4e781c23ee0 --- /dev/null +++ b/spec/features/security/group/internal_access_spec.rb @@ -0,0 +1,102 @@ +require 'rails_helper' + +describe 'Internal group access', feature: true do + include AccessMatchers + include GroupAccessHelper + + describe 'GET /groups/:path' do + subject { group_path(group(Gitlab::VisibilityLevel::INTERNAL)) } + + context "when user not in group project" do + it { is_expected.to be_allowed_for group_member(:owner) } + 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 :admin } + it { is_expected.to be_allowed_for :user } + it { is_expected.to_not be_allowed_for :visitor } + end + + context "when user in group project" do + it { is_expected.to be_allowed_for project_group_member(:user) } + it { is_expected.to_not be_allowed_for :visitor } + end + end + + describe 'GET /groups/:path/issues' do + subject { issues_group_path(group(Gitlab::VisibilityLevel::INTERNAL)) } + + context "when user not in group project" do + it { is_expected.to be_allowed_for group_member(:owner) } + 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 :admin } + it { is_expected.to be_allowed_for :user } + it { is_expected.to_not be_allowed_for :visitor } + end + + context "when user in group project" do + it { is_expected.to be_allowed_for project_group_member(:user) } + it { is_expected.to_not be_allowed_for :visitor } + end + end + + describe 'GET /groups/:path/merge_requests' do + subject { issues_group_path(group(Gitlab::VisibilityLevel::INTERNAL)) } + + context "when user not in group project" do + it { is_expected.to be_allowed_for group_member(:owner) } + 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 :admin } + it { is_expected.to be_allowed_for :user } + it { is_expected.to_not be_allowed_for :visitor } + end + + context "when user in group project" do + it { is_expected.to be_allowed_for project_group_member(:user) } + it { is_expected.to_not be_allowed_for :visitor } + end + end + + + describe 'GET /groups/:path/group_members' do + subject { issues_group_path(group(Gitlab::VisibilityLevel::INTERNAL)) } + + context "when user not in group project" do + it { is_expected.to be_allowed_for group_member(:owner) } + 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 :admin } + it { is_expected.to be_allowed_for :user } + it { is_expected.to_not be_allowed_for :visitor } + end + + context "when user in group project" do + it { is_expected.to be_allowed_for project_group_member(:user) } + it { is_expected.to_not be_allowed_for :visitor } + end + end + + describe 'GET /groups/:path/edit' do + subject { issues_group_path(group(Gitlab::VisibilityLevel::INTERNAL)) } + + context "when user not in group project" do + it { is_expected.to be_allowed_for group_member(:owner) } + 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 :admin } + it { is_expected.to be_allowed_for :user } + it { is_expected.to_not be_allowed_for :visitor } + end + + context "when user in group project" do + it { is_expected.to be_allowed_for project_group_member(:user) } + it { is_expected.to_not be_allowed_for :visitor } + end + end +end diff --git a/spec/features/security/group/private_access_spec.rb b/spec/features/security/group/private_access_spec.rb new file mode 100644 index 00000000000..0d01310b449 --- /dev/null +++ b/spec/features/security/group/private_access_spec.rb @@ -0,0 +1,104 @@ +require 'rails_helper' + +describe 'Private group access', feature: true do + include AccessMatchers + include GroupAccessHelper + + + + describe 'GET /groups/:path' do + subject { group_path(group(Gitlab::VisibilityLevel::PRIVATE)) } + + context "when user not in group project" do + it { is_expected.to be_allowed_for group_member(:owner) } + 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 :admin } + it { is_expected.to_not be_allowed_for :user } + it { is_expected.to_not be_allowed_for :visitor } + end + + context "when user in group project" do + it { is_expected.to be_allowed_for project_group_member(:user) } + it { is_expected.to_not be_allowed_for :visitor } + end + end + + describe 'GET /groups/:path/issues' do + subject { issues_group_path(group(Gitlab::VisibilityLevel::PRIVATE)) } + + context "when user not in group project" do + it { is_expected.to be_allowed_for group_member(:owner) } + 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 :admin } + it { is_expected.to_not be_allowed_for :user } + it { is_expected.to_not be_allowed_for :visitor } + end + + context "when user in group project" do + it { is_expected.to be_allowed_for project_group_member(:user) } + it { is_expected.to_not be_allowed_for :visitor } + end + end + + describe 'GET /groups/:path/merge_requests' do + subject { issues_group_path(group(Gitlab::VisibilityLevel::PRIVATE)) } + + context "when user not in group project" do + it { is_expected.to be_allowed_for group_member(:owner) } + 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 :admin } + it { is_expected.to_not be_allowed_for :user } + it { is_expected.to_not be_allowed_for :visitor } + end + + context "when user in group project" do + it { is_expected.to be_allowed_for project_group_member(:user) } + it { is_expected.to_not be_allowed_for :visitor } + end + end + + + describe 'GET /groups/:path/group_members' do + subject { issues_group_path(group(Gitlab::VisibilityLevel::PRIVATE)) } + + context "when user not in group project" do + it { is_expected.to be_allowed_for group_member(:owner) } + 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 :admin } + it { is_expected.to_not be_allowed_for :user } + it { is_expected.to_not be_allowed_for :visitor } + end + + context "when user in group project" do + it { is_expected.to be_allowed_for project_group_member(:user) } + it { is_expected.to_not be_allowed_for :visitor } + end + end + + describe 'GET /groups/:path/edit' do + subject { issues_group_path(group(Gitlab::VisibilityLevel::PRIVATE)) } + + context "when user not in group project" do + it { is_expected.to be_allowed_for group_member(:owner) } + 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 :admin } + it { is_expected.to_not be_allowed_for :user } + it { is_expected.to_not be_allowed_for :visitor } + end + + context "when user in group project" do + it { is_expected.to be_allowed_for project_group_member(:user) } + it { is_expected.to_not be_allowed_for :visitor } + end + end +end diff --git a/spec/features/security/group/public_access_spec.rb b/spec/features/security/group/public_access_spec.rb new file mode 100644 index 00000000000..75d208f2949 --- /dev/null +++ b/spec/features/security/group/public_access_spec.rb @@ -0,0 +1,104 @@ +require 'rails_helper' + +describe 'Public group access', feature: true do + include AccessMatchers + include GroupAccessHelper + + + + describe 'GET /groups/:path' do + subject { group_path(group(Gitlab::VisibilityLevel::PUBLIC)) } + + context "when user not in group project" do + it { is_expected.to be_allowed_for group_member(:owner) } + 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 :admin } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :visitor } + end + + context "when user in group project" do + it { is_expected.to be_allowed_for project_group_member(:user) } + it { is_expected.to be_allowed_for :visitor } + end + end + + describe 'GET /groups/:path/issues' do + subject { issues_group_path(group(Gitlab::VisibilityLevel::PUBLIC)) } + + context "when user not in group project" do + it { is_expected.to be_allowed_for group_member(:owner) } + 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 :admin } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :visitor } + end + + context "when user in group project" do + it { is_expected.to be_allowed_for project_group_member(:user) } + it { is_expected.to be_allowed_for :visitor } + end + end + + describe 'GET /groups/:path/merge_requests' do + subject { issues_group_path(group(Gitlab::VisibilityLevel::PUBLIC)) } + + context "when user not in group project" do + it { is_expected.to be_allowed_for group_member(:owner) } + 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 :admin } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :visitor } + end + + context "when user in group project" do + it { is_expected.to be_allowed_for project_group_member(:user) } + it { is_expected.to be_allowed_for :visitor } + end + end + + + describe 'GET /groups/:path/group_members' do + subject { issues_group_path(group(Gitlab::VisibilityLevel::PUBLIC)) } + + context "when user not in group project" do + it { is_expected.to be_allowed_for group_member(:owner) } + 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 :admin } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :visitor } + end + + context "when user in group project" do + it { is_expected.to be_allowed_for project_group_member(:user) } + it { is_expected.to be_allowed_for :visitor } + end + end + + describe 'GET /groups/:path/edit' do + subject { issues_group_path(group(Gitlab::VisibilityLevel::PUBLIC)) } + + context "when user not in group project" do + it { is_expected.to be_allowed_for group_member(:owner) } + 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 :admin } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :visitor } + end + + context "when user in group project" do + it { is_expected.to be_allowed_for project_group_member(:user) } + it { is_expected.to be_allowed_for :visitor } + end + end +end diff --git a/spec/features/security/group_access_spec.rb b/spec/features/security/group_access_spec.rb index 65f8073c693..0194581dfd1 100644 --- a/spec/features/security/group_access_spec.rb +++ b/spec/features/security/group_access_spec.rb @@ -43,8 +43,6 @@ describe 'Group access', feature: true do 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 :admin } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_allowed_for :visitor } end context 'with mixed projects' do @@ -55,8 +53,6 @@ describe 'Group access', feature: true do 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 :admin } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_allowed_for :visitor } end context 'with internal projects' do @@ -67,8 +63,6 @@ describe 'Group access', feature: true do 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 :admin } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_allowed_for :visitor } end context 'with no projects' do @@ -77,8 +71,6 @@ describe 'Group access', feature: true do 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 :admin } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_allowed_for :visitor } end end @@ -93,8 +85,6 @@ describe 'Group access', feature: true do 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 :admin } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_allowed_for :visitor } end context 'with mixed projects' do @@ -105,8 +95,6 @@ describe 'Group access', feature: true do 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 :admin } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_allowed_for :visitor } end context 'with internal projects' do @@ -117,8 +105,6 @@ describe 'Group access', feature: true do 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 :admin } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_denied_for :visitor } end context 'with no projects' do @@ -127,8 +113,6 @@ describe 'Group access', feature: true do 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 :admin } - it { is_expected.to be_denied_for :user } - it { is_expected.to be_denied_for :visitor } end end @@ -143,8 +127,6 @@ describe 'Group access', feature: true do 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 :admin } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_allowed_for :visitor } end context 'with mixed projects' do @@ -155,8 +137,6 @@ describe 'Group access', feature: true do 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 :admin } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_allowed_for :visitor } end context 'with internal projects' do @@ -167,8 +147,6 @@ describe 'Group access', feature: true do 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 :admin } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_denied_for :visitor } end context 'with no projects' do @@ -177,8 +155,6 @@ describe 'Group access', feature: true do 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 :admin } - it { is_expected.to be_denied_for :user } - it { is_expected.to be_denied_for :visitor } end end @@ -193,8 +169,6 @@ describe 'Group access', feature: true do 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 :admin } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_allowed_for :visitor } end context 'with mixed projects' do @@ -205,8 +179,6 @@ describe 'Group access', feature: true do 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 :admin } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_allowed_for :visitor } end context 'with internal projects' do @@ -217,8 +189,6 @@ describe 'Group access', feature: true do 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 :admin } - it { is_expected.to be_allowed_for :user } - it { is_expected.to be_denied_for :visitor } end context 'with no projects' do @@ -227,8 +197,6 @@ describe 'Group access', feature: true do 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 :admin } - it { is_expected.to be_denied_for :user } - it { is_expected.to be_denied_for :visitor } end end @@ -243,8 +211,6 @@ describe 'Group access', feature: true do it { is_expected.to be_denied_for group_member(:reporter) } it { is_expected.to be_denied_for group_member(:guest) } it { is_expected.to be_allowed_for :admin } - it { is_expected.to be_denied_for :user } - it { is_expected.to be_denied_for :visitor } end context 'with mixed projects' do @@ -255,8 +221,6 @@ describe 'Group access', feature: true do it { is_expected.to be_denied_for group_member(:reporter) } it { is_expected.to be_denied_for group_member(:guest) } it { is_expected.to be_allowed_for :admin } - it { is_expected.to be_denied_for :user } - it { is_expected.to be_denied_for :visitor } end context 'with internal projects' do @@ -267,8 +231,6 @@ describe 'Group access', feature: true do it { is_expected.to be_denied_for group_member(:reporter) } it { is_expected.to be_denied_for group_member(:guest) } it { is_expected.to be_allowed_for :admin } - it { is_expected.to be_denied_for :user } - it { is_expected.to be_denied_for :visitor } end context 'with no projects' do @@ -277,8 +239,6 @@ describe 'Group access', feature: true do it { is_expected.to be_denied_for group_member(:reporter) } it { is_expected.to be_denied_for group_member(:guest) } it { is_expected.to be_allowed_for :admin } - it { is_expected.to be_denied_for :user } - it { is_expected.to be_denied_for :visitor } end end end diff --git a/spec/finders/groups_finder_spec.rb b/spec/finders/groups_finder_spec.rb new file mode 100644 index 00000000000..ed24954af7a --- /dev/null +++ b/spec/finders/groups_finder_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +describe GroupsFinder do + describe '#execute' do + let(:user) { create(:user) } + let!(:private_group) { create(:group, visibility_level: 0) } + let!(:internal_group) { create(:group, visibility_level: 10) } + let!(:public_group) { create(:group, visibility_level: 20) } + let(:finder) { described_class.new } + + describe 'execute' do + describe 'without a user' do + subject { finder.execute } + + it { is_expected.to eq([public_group]) } + end + + describe 'with a user' do + subject { finder.execute(user) } + + it { is_expected.to eq([public_group, internal_group]) } + end + end + end +end diff --git a/spec/finders/joined_groups_finder_spec.rb b/spec/finders/joined_groups_finder_spec.rb new file mode 100644 index 00000000000..e2f6c593638 --- /dev/null +++ b/spec/finders/joined_groups_finder_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +describe JoinedGroupsFinder do + describe '#execute' do + let!(:profile_owner) { create(:user) } + let!(:profile_visitor) { create(:user) } + + let!(:private_group) { create(:group, visibility_level: Gitlab::VisibilityLevel::PRIVATE) } + let!(:private_group_2) { create(:group, visibility_level: Gitlab::VisibilityLevel::PRIVATE) } + let!(:internal_group) { create(:group, visibility_level: Gitlab::VisibilityLevel::INTERNAL) } + let!(:internal_group_2) { create(:group, visibility_level: Gitlab::VisibilityLevel::INTERNAL) } + let!(:public_group) { create(:group, visibility_level: Gitlab::VisibilityLevel::PUBLIC) } + let!(:public_group_2) { create(:group, visibility_level: Gitlab::VisibilityLevel::PUBLIC) } + let!(:finder) { described_class.new(profile_owner) } + + describe 'execute' do + context 'without a user only shows public groups from profile owner' do + before { public_group.add_user(profile_owner, Gitlab::Access::MASTER)} + subject { finder.execute } + + it { is_expected.to eq([public_group]) } + end + + context 'only shows groups where both users are authorized to see' do + subject { finder.execute(profile_visitor) } + + before do + private_group.add_user(profile_owner, Gitlab::Access::MASTER) + private_group.add_user(profile_visitor, Gitlab::Access::DEVELOPER) + internal_group.add_user(profile_owner, Gitlab::Access::MASTER) + public_group.add_user(profile_owner, Gitlab::Access::MASTER) + end + + it { is_expected.to eq([public_group, internal_group, private_group]) } + end + + context 'shows group if profile visitor is in one of its projects' do + before do + public_group.add_user(profile_owner, Gitlab::Access::MASTER) + private_group.add_user(profile_owner, Gitlab::Access::MASTER) + project = create(:project, :private, group: private_group, name: 'B', path: 'B') + project.team.add_user(profile_visitor, Gitlab::Access::DEVELOPER) + end + + subject { finder.execute(profile_visitor) } + + it { is_expected.to eq([public_group, private_group]) } + end + end + end +end diff --git a/spec/helpers/groups_helper.rb b/spec/helpers/groups_helper.rb index 4ea90a80a92..01ec9e5a07f 100644 --- a/spec/helpers/groups_helper.rb +++ b/spec/helpers/groups_helper.rb @@ -18,4 +18,19 @@ describe GroupsHelper do expect(group_icon(group.path)).to match('group_avatar.png') end end + + describe 'permissions' do + let(:group) { create(:group) } + let!(:user) { create(:user) } + + before do + allow(self).to receive(:current_user).and_return(user) + allow(self).to receive(:can?) { true } + end + + it 'checks user ability to change permissions' do + expect(self).to receive(:can?).with(user, :change_visibility_level, group) + can_change_group_visibility_level?(group) + end + end end diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb index cd7596a763d..ef60ef75fbe 100644 --- a/spec/helpers/visibility_level_helper_spec.rb +++ b/spec/helpers/visibility_level_helper_spec.rb @@ -8,6 +8,7 @@ describe VisibilityLevelHelper do end let(:project) { build(:project) } + let(:group) { build(:group) } let(:personal_snippet) { build(:personal_snippet) } let(:project_snippet) { build(:project_snippet) } @@ -19,6 +20,13 @@ describe VisibilityLevelHelper do end end + context 'used with a Group' do + it 'delegates groups to #group_visibility_level_description' do + expect(visibility_level_description(Gitlab::VisibilityLevel::PRIVATE, group)) + .to match /group/i + end + end + context 'called with a Snippet' do it 'delegates snippets to #snippet_visibility_level_description' do expect(visibility_level_description(Gitlab::VisibilityLevel::INTERNAL, project_snippet)) diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index c9245fc9535..135d298e10f 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -56,6 +56,24 @@ describe Group, models: true do end end + describe 'scopes' do + let!(:private_group) { create(:group, visibility_level: 0) } + let!(:internal_group) { create(:group, visibility_level: 10) } + let!(:public_group) { create(:group, visibility_level: 20) } + + describe 'public_only' do + subject { described_class.public_only } + + it{ is_expected.to eq([public_group]) } + end + + describe 'public_and_internal_only' do + subject { described_class.public_and_internal_only } + + it{ is_expected.to eq([public_group, internal_group]) } + end + end + describe '#to_reference' do it 'returns a String reference to the object' do expect(group.to_reference).to eq "@#{group.name}" diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b8b9a455b83..7fd3726c6ad 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -720,4 +720,20 @@ describe Project, models: true do expect(described_class.search_by_title('KITTENS')).to eq([project]) end end + + context 'when checking projects from groups' do + let(:private_group) { create(:group, visibility_level: 0) } + let(:internal_group) { create(:group, visibility_level: 10) } + + let(:private_project) { create :project, group: private_group, visibility_level: Gitlab::VisibilityLevel::PRIVATE } + let(:internal_project) { create :project, group: internal_group, visibility_level: Gitlab::VisibilityLevel::INTERNAL } + + context 'when group is private project can not be internal' do + it { expect(private_project.visibility_level_allowed?(Gitlab::VisibilityLevel::INTERNAL)).to be_falsey } + end + + context 'when group is internal project can not be public' do + it { expect(internal_project.visibility_level_allowed?(Gitlab::VisibilityLevel::PUBLIC)).to be_falsey } + end + end end diff --git a/spec/services/groups/create_service_spec.rb b/spec/services/groups/create_service_spec.rb new file mode 100644 index 00000000000..7dbc5297978 --- /dev/null +++ b/spec/services/groups/create_service_spec.rb @@ -0,0 +1,22 @@ +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) } + + describe "execute" do + let!(:service) { described_class.new(public_group, user, visibility_level: Gitlab::VisibilityLevel::PUBLIC ) } + subject { service.execute } + + context "create groups without restricted visibility level" do + it { is_expected.to be_truthy } + 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 } + end + end +end diff --git a/spec/services/groups/update_service_spec.rb b/spec/services/groups/update_service_spec.rb new file mode 100644 index 00000000000..c759e32342d --- /dev/null +++ b/spec/services/groups/update_service_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +describe Groups::UpdateService, 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) } + + describe "execute" do + context "project visibility_level validation" do + + context "public group with public projects" do + let!(:service) { described_class.new(public_group, user, visibility_level: Gitlab::VisibilityLevel::INTERNAL ) } + + before do + public_group.add_user(user, Gitlab::Access::MASTER) + create(:project, :public, group: public_group, name: 'B', path: 'B') + end + + it "cant downgrade permission level" do + expect(service.execute).to be_falsy + expect(public_group.errors.count).to eq(1) + end + end + + context "internal group with internal project" do + let!(:service) { described_class.new(internal_group, user, visibility_level: Gitlab::VisibilityLevel::PRIVATE ) } + + before do + internal_group.add_user(user, Gitlab::Access::MASTER) + create(:project, :internal, group: internal_group, name: 'B', path: 'B') + end + + it "cant downgrade permission level" do + expect(service.execute).to be_falsy + expect(internal_group.errors.count).to eq(1) + end + end + end + end + + context "unauthorized visibility_level validation" do + let!(:service) { described_class.new(internal_group, user, visibility_level: 99 ) } + before { internal_group.add_user(user, Gitlab::Access::MASTER) } + + it "does not change permission level" do + expect(service.execute).to be_falsy + expect(internal_group.errors.count).to eq(1) + end + end +end diff --git a/spec/support/group_access_helper.rb b/spec/support/group_access_helper.rb new file mode 100644 index 00000000000..a1a8fb2bd72 --- /dev/null +++ b/spec/support/group_access_helper.rb @@ -0,0 +1,17 @@ +module GroupAccessHelper + def group(visibility_level=0) + @group ||= create(:group, visibility_level: visibility_level) + end + + def project_group_member(access_level) + project = create(:project, visibility_level: group.visibility_level, group: group, name: 'B', path: 'B') + + create(:user).tap { |user| project.team.add_user(user, Gitlab::Access::DEVELOPER) } + end + + def group_member(access_level, grp=group()) + level = Object.const_get("Gitlab::Access::#{access_level.upcase}") + + create(:user).tap { |user| grp.add_user(user, level) } + end +end |