diff options
| author | Douwe Maan <douwe@selenight.nl> | 2016-03-20 21:03:53 +0100 |
|---|---|---|
| committer | Douwe Maan <douwe@selenight.nl> | 2016-03-20 21:04:07 +0100 |
| commit | 8db1292139cfdac4c29c03b876b68b9e752cf75a (patch) | |
| tree | 2fcf67ada482ecf4ac90f39c858334a62b709618 | |
| parent | 2eb19ea3ea36916bbea72a8ccab3e6d15f602ac9 (diff) | |
| download | gitlab-ce-8db1292139cfdac4c29c03b876b68b9e752cf75a.tar.gz | |
Tweaks, refactoring, and specs
56 files changed, 825 insertions, 1189 deletions
diff --git a/app/assets/stylesheets/framework/mobile.scss b/app/assets/stylesheets/framework/mobile.scss index bf874b9b822..5ea4f9a49db 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/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1f55b18e0b1..3a0eb96a460 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -25,7 +25,6 @@ class ApplicationController < ActionController::Base helper_method :abilities, :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?, :gitorious_import_enabled?, :google_code_import_enabled?, :fogbugz_import_enabled?, :git_import_enabled? - helper_method :repository, :can_collaborate_with_project? rescue_from Encoding::CompatibilityError do |exception| log_exception(exception) @@ -118,47 +117,6 @@ class ApplicationController < ActionController::Base abilities.allowed?(object, action, subject) end - def project - unless @project - namespace = params[:namespace_id] - id = params[:project_id] || params[:id] - - # Redirect from - # localhost/group/project.git - # to - # localhost/group/project - # - if id =~ /\.git\Z/ - redirect_to request.original_url.gsub(/\.git\/?\Z/, '') and return - end - - project_path = "#{namespace}/#{id}" - @project = Project.find_with_namespace(project_path) - - if @project and can?(current_user, :read_project, @project) - if @project.path_with_namespace != project_path - redirect_to request.original_url.gsub(project_path, @project.path_with_namespace) and return - end - @project - elsif current_user.nil? - @project = nil - authenticate_user! - else - @project = nil - render_404 and return - end - end - @project - end - - def repository - @repository ||= project.repository - end - - def authorize_project!(action) - return access_denied! unless can?(current_user, action, project) - end - def access_denied! render "errors/access_denied", layout: "errors", status: 404 end @@ -167,14 +125,6 @@ class ApplicationController < ActionController::Base render "errors/git_not_found.html", layout: "errors", status: 404 end - def method_missing(method_sym, *arguments, &block) - if method_sym.to_s =~ /\Aauthorize_(.*)!\z/ - authorize_project!($1.to_sym) - else - super - end - end - def render_403 head :forbidden end @@ -183,10 +133,6 @@ class ApplicationController < ActionController::Base render file: Rails.root.join("public", "404"), layout: false, status: "404" end - def require_non_empty_project - redirect_to @project if @project.empty_repo? - end - def no_cache_headers response.headers["Cache-Control"] = "no-cache, no-store, max-age=0, must-revalidate" response.headers["Pragma"] = "no-cache" @@ -412,13 +358,6 @@ class ApplicationController < ActionController::Base current_user.nil? && root_path == request.path end - def can_collaborate_with_project?(project = nil) - project ||= @project - - can?(current_user, :push_code, project) || - (current_user && current_user.already_forked?(project)) - end - private def set_default_sort diff --git a/app/controllers/groups/application_controller.rb b/app/controllers/groups/application_controller.rb index 795ce50fe92..949b4a6c25a 100644 --- a/app/controllers/groups/application_controller.rb +++ b/app/controllers/groups/application_controller.rb @@ -1,21 +1,32 @@ class Groups::ApplicationController < ApplicationController layout 'group' + + skip_before_action :authenticate_user! before_action :group private def group - @group ||= Group.find_by(path: params[:group_id]) - end + unless @group + id = params[:group_id] || params[:id] + @group = Group.find_by(path: id) + + unless @group && can?(current_user, :read_group, @group) + @group = nil - def authorize_read_group! - unless @group && can?(current_user, :read_group, @group) - if current_user.nil? - return authenticate_user! - else - return render_404 + if current_user.nil? + authenticate_user! + else + render_404 + end end end + + @group + end + + def group_projects + @projects ||= GroupProjectsFinder.new(group).execute(current_user) end def authorize_admin_group! diff --git a/app/controllers/groups/avatars_controller.rb b/app/controllers/groups/avatars_controller.rb index 76c87366baa..ad2c20b42db 100644 --- a/app/controllers/groups/avatars_controller.rb +++ b/app/controllers/groups/avatars_controller.rb @@ -1,4 +1,6 @@ class Groups::AvatarsController < Groups::ApplicationController + before_action :authorize_admin_group! + def destroy @group.remove_avatar! @group.save diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 0e902c4bb43..d5ef33888c6 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -1,8 +1,5 @@ class Groups::GroupMembersController < Groups::ApplicationController - skip_before_action :authenticate_user!, only: [:index] - # Authorize - before_action :authorize_read_group! before_action :authorize_admin_group_member!, except: [:index, :leave] def index diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb index 0c2a350bc39..0028f072d5b 100644 --- a/app/controllers/groups/milestones_controller.rb +++ b/app/controllers/groups/milestones_controller.rb @@ -1,10 +1,10 @@ class Groups::MilestonesController < Groups::ApplicationController include GlobalMilestones - before_action :projects + before_action :group_projects before_action :milestones, only: [:index] before_action :milestone, only: [:show, :update] - before_action :authorize_group_milestone!, only: [:create, :update] + before_action :authorize_admin_milestones!, only: [:new, :create, :update] def index end @@ -17,7 +17,7 @@ class Groups::MilestonesController < Groups::ApplicationController project_ids = params[:milestone][:project_ids] title = milestone_params[:title] - @group.projects.where(id: project_ids).each do |project| + @projects.where(id: project_ids).each do |project| Milestones::CreateService.new(project, current_user, milestone_params).execute end @@ -37,7 +37,7 @@ class Groups::MilestonesController < Groups::ApplicationController private - def authorize_group_milestone! + def authorize_admin_milestones! return render_404 unless can?(current_user, :admin_milestones, group) end @@ -48,8 +48,4 @@ class Groups::MilestonesController < Groups::ApplicationController def milestone_path(title) group_milestone_path(@group, title.to_slug.to_s, title: title) end - - def projects - @projects ||= @group.projects - end end diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index b635fb150ae..48565f44ffb 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -5,16 +5,15 @@ class GroupsController < Groups::ApplicationController respond_to :html - skip_before_action :authenticate_user!, only: [:index, :show, :issues, :merge_requests] + before_action :authenticate_user!, only: [:new, :create] before_action :group, except: [:index, :new, :create] # Authorize - 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] # Load group projects - before_action :load_projects, except: [:index, :new, :create, :projects, :edit, :update, :autocomplete] + before_action :group_projects, only: [:show, :projects, :activity, :issues, :merge_requests] before_action :event_filter, only: [:activity] layout :determine_layout @@ -39,12 +38,13 @@ class GroupsController < Groups::ApplicationController def show @last_push = current_user.recent_push if current_user + @projects = @projects.includes(:namespace) @projects = filter_projects(@projects) @projects = @projects.sort(@sort = params[:sort]) @projects = @projects.page(params[:page]).per(PER_PAGE) if params[:filter_projects].blank? - @shared_projects = @group.shared_projects + @shared_projects = GroupProjectsFinder.new(group, shared: true).execute(current_user) respond_to do |format| format.html @@ -77,7 +77,7 @@ class GroupsController < Groups::ApplicationController end def projects - @projects = @group.projects.page(params[:page]) + @projects = @projects.sorted_by_activity.page(params[:page]) end def update @@ -96,15 +96,6 @@ class GroupsController < Groups::ApplicationController protected - def group - @group ||= Group.find_by(path: params[:id]) - @group || render_404 - end - - def load_projects - @projects ||= ProjectsFinder.new.execute(current_user, group: group).sorted_by_activity - end - def authorize_create_group! unless can?(current_user, :create_group, nil) return render_404 diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index a326bc58215..276f80f800a 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -1,20 +1,73 @@ class Projects::ApplicationController < ApplicationController - before_action :project - before_action :repository + skip_before_action :authenticate_user! + before_action :project, :repository layout 'project' - def authenticate_user! - # Restrict access to Projects area only - # for non-signed users - if !current_user + helper_method :repository, :can_collaborate_with_project? + + private + + def project + unless @project + namespace = params[:namespace_id] id = params[:project_id] || params[:id] - project_with_namespace = "#{params[:namespace_id]}/#{id}" - @project = Project.find_with_namespace(project_with_namespace) - return if @project && @project.public? + # Redirect from + # localhost/group/project.git + # to + # localhost/group/project + # + if id =~ /\.git\Z/ + redirect_to request.original_url.gsub(/\.git\/?\Z/, '') + return + end + + project_path = "#{namespace}/#{id}" + @project = Project.find_with_namespace(project_path) + + if @project && can?(current_user, :read_project, @project) + if @project.path_with_namespace != project_path + redirect_to request.original_url.gsub(project_path, @project.path_with_namespace) + end + else + @project = nil + + if current_user.nil? + authenticate_user! + else + render_404 + end + end + end + + @project + end + + def repository + @repository ||= project.repository + end + + def can_collaborate_with_project?(project = nil) + project ||= @project + + can?(current_user, :push_code, project) || + (current_user && current_user.already_forked?(project)) + end + + def authorize_project!(action) + return access_denied! unless can?(current_user, action, project) + end + + def method_missing(method_sym, *arguments, &block) + if method_sym.to_s =~ /\Aauthorize_(.*)!\z/ + authorize_project!($1.to_sym) + else + super end + end - super + def require_non_empty_project + redirect_to @project if @project.empty_repo? end def require_branch_head @@ -26,8 +79,6 @@ class Projects::ApplicationController < ApplicationController end end - private - def apply_diff_view_cookie! view = params[:view] || cookies[:diff_view] cookies.permanent[:diff_view] = params[:view] = view if view diff --git a/app/controllers/projects/avatars_controller.rb b/app/controllers/projects/avatars_controller.rb index a6bebc46b06..72921b3aa14 100644 --- a/app/controllers/projects/avatars_controller.rb +++ b/app/controllers/projects/avatars_controller.rb @@ -1,7 +1,7 @@ class Projects::AvatarsController < Projects::ApplicationController include BlobHelper - before_action :project + before_action :authorize_admin_project!, only: [:destroy] def show @blob = @repository.blob_at_branch('master', @project.avatar_in_git) diff --git a/app/controllers/projects/uploads_controller.rb b/app/controllers/projects/uploads_controller.rb index e1fe7ea2114..94c51eeb94d 100644 --- a/app/controllers/projects/uploads_controller.rb +++ b/app/controllers/projects/uploads_controller.rb @@ -1,7 +1,9 @@ class Projects::UploadsController < Projects::ApplicationController - skip_before_action :authenticate_user!, :reject_blocked!, :project, + skip_before_action :reject_blocked!, :project, :repository, if: -> { action_name == 'show' && image? } + before_action :authenticate_user!, only: [:create] + def create link_to_file = ::Projects::UploadService.new(project, params[:file]). execute @@ -26,6 +28,8 @@ class Projects::UploadsController < Projects::ApplicationController send_file uploader.file.path, disposition: disposition end + private + def uploader return @uploader if defined?(@uploader) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index c9930480770..928817ba811 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -1,7 +1,7 @@ -class ProjectsController < ApplicationController +class ProjectsController < Projects::ApplicationController include ExtractsPath - skip_before_action :authenticate_user!, only: [:show, :activity] + before_action :authenticate_user!, except: [:show, :activity] before_action :project, except: [:new, :create] before_action :repository, except: [:new, :create] before_action :assign_ref_vars, :tree, only: [:show], if: :repo_exists? diff --git a/app/finders/contributed_projects_finder.rb b/app/finders/contributed_projects_finder.rb index f8b04dfa2aa..a685719555c 100644 --- a/app/finders/contributed_projects_finder.rb +++ b/app/finders/contributed_projects_finder.rb @@ -1,4 +1,4 @@ -class ContributedProjectsFinder +class ContributedProjectsFinder < UnionFinder def initialize(user) @user = user end @@ -10,27 +10,20 @@ class ContributedProjectsFinder # visible by this user. # # Returns an ActiveRecord::Relation. - def execute(current_user = nil) - if current_user - relation = projects_visible_to_user(current_user) - else - relation = public_projects - end + segments = all_projects(current_user) - relation.includes(:namespace).order_id_desc + find_union(segments, Project).includes(:namespace).order_id_desc end private - 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)]) + def all_projects(current_user) + projects = [] - Project.where("projects.id IN (#{union.to_sql})") - end + projects << @user.contributed_projects.visible_to_user(current_user) if current_user + projects << @user.contributed_projects.public_to_user(current_user) - def public_projects - @user.contributed_projects.public_only + projects end end diff --git a/app/finders/group_projects_finder.rb b/app/finders/group_projects_finder.rb new file mode 100644 index 00000000000..84fe468ae5d --- /dev/null +++ b/app/finders/group_projects_finder.rb @@ -0,0 +1,43 @@ +class GroupProjectsFinder < UnionFinder + def initialize(group, options = {}) + @group = group + @options = options + end + + def execute(current_user = nil) + segments = group_projects(current_user) + + find_union(segments, Project) + end + + private + + def group_projects(current_user) + include_owned = @options.fetch(:owned, true) + include_shared = @options.fetch(:shared, true) + + projects = [] + + if current_user + if @group.users.include?(current_user) + projects << @group.projects if include_owned + projects << @group.shared_projects if include_shared + else + if include_owned + projects << @group.projects.visible_to_user(current_user) + projects << @group.projects.public_to_user(current_user) + end + + if include_shared + projects << @group.shared_projects.visible_to_user(current_user) + projects << @group.shared_projects.public_to_user(current_user) + end + end + else + projects << @group.projects.public_only if include_owned + projects << @group.shared_projects.public_only if include_shared + end + + projects + end +end diff --git a/app/finders/groups_finder.rb b/app/finders/groups_finder.rb index 30698f80231..4e43f42e9e1 100644 --- a/app/finders/groups_finder.rb +++ b/app/finders/groups_finder.rb @@ -1,30 +1,18 @@ -class GroupsFinder +class GroupsFinder < UnionFinder 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 + find_union(segments, Group).order_id_desc end private def all_groups(current_user) - if current_user - user_groups(current_user) - else - [Group.unscoped.public_only] - end - end + groups = [] + + groups << current_user.authorized_groups if current_user + groups << Group.unscoped.public_to_user(current_user) - 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 + groups end end diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 19e8c7a92be..7510f6e9e29 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -81,7 +81,7 @@ class IssuableFinder elsif current_user && params[:authorized_only].presence && !current_user_related? @projects = current_user.authorized_projects.reorder(nil) else - @projects = ProjectsFinder.new.execute(current_user, group: group). + @projects = GroupProjectsFinder.new(group).execute(current_user). reorder(nil) end end @@ -170,7 +170,7 @@ class IssuableFinder end def by_scope(items) - case params[:scope] + case params[:scope] || 'all' when 'created-by-me', 'authored' then items.where(author_id: current_user.id) when 'all' then diff --git a/app/finders/joined_groups_finder.rb b/app/finders/joined_groups_finder.rb index 867eb661682..2a3f0296d37 100644 --- a/app/finders/joined_groups_finder.rb +++ b/app/finders/joined_groups_finder.rb @@ -1,5 +1,4 @@ -#Shows only authorized groups of a user -class JoinedGroupsFinder +class JoinedGroupsFinder < UnionFinder def initialize(user) @user = user end @@ -12,34 +11,19 @@ class JoinedGroupsFinder # # Returns an ActiveRecord::Relation. def execute(current_user = nil) - if current_user - relation = groups_visible_to_user(current_user) - else - relation = public_groups - end + segments = all_groups(current_user) - relation.order_id_desc + find_union(segments, Group).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 = 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})") - end + def all_groups(current_user) + groups = [] - def public_groups - @user.authorized_groups.public_only - end + groups << @user.authorized_groups.visible_to_user(current_user) if current_user + groups << @user.authorized_groups.public_to_user(current_user) - def public_and_internal_groups - @user.authorized_groups.public_and_internal_only + groups end end diff --git a/app/finders/personal_projects_finder.rb b/app/finders/personal_projects_finder.rb index 34f33e2353b..3ad4bd5f066 100644 --- a/app/finders/personal_projects_finder.rb +++ b/app/finders/personal_projects_finder.rb @@ -1,4 +1,4 @@ -class PersonalProjectsFinder +class PersonalProjectsFinder < UnionFinder def initialize(user) @user = user end @@ -11,38 +11,19 @@ class PersonalProjectsFinder # # Returns an ActiveRecord::Relation. def execute(current_user = nil) - if current_user - relation = projects_visible_to_user(current_user) - else - relation = public_projects - end + segments = all_projects(current_user) - relation.includes(:namespace).order_id_desc + find_union(segments, Project).includes(:namespace).order_id_desc end private - def projects_visible_to_user(current_user) - union = Gitlab::SQL::Union.new(projects_for_user_ids(current_user)) + def all_projects(current_user) + projects = [] - Project.where("projects.id IN (#{union.to_sql})") - end - - def public_projects - @user.personal_projects.public_only - end - - 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) + projects << @user.personal_projects.visible_to_user(current_user) if current_user + projects << @user.personal_projects.public_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 + projects end end diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index 3a5fc5b5907..2f0a9659d15 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -1,81 +1,18 @@ -class ProjectsFinder - # Returns all projects, optionally including group projects a user has access - # to. - # - # ## Examples - # - # Retrieving all public projects: - # - # ProjectsFinder.new.execute - # - # Retrieving all public/internal projects and those the given user has access - # to: - # - # ProjectsFinder.new.execute(some_user) - # - # Retrieving all public/internal projects as well as the group's projects the - # user has access to: - # - # ProjectsFinder.new.execute(some_user, group: some_group) - # - # Returns an ActiveRecord::Relation. +class ProjectsFinder < UnionFinder def execute(current_user = nil, options = {}) - group = options[:group] + segments = all_projects(current_user) - if group - segments = group_projects(current_user, group) - else - segments = all_projects(current_user) - end - - if segments.length > 1 - union = Gitlab::SQL::Union.new(segments.map { |s| s.select(:id) }) - - Project.where("projects.id IN (#{union.to_sql})") - else - segments.first - end + find_union(segments, Project) end private - def group_projects(current_user, group) - return [group.projects.public_only] unless current_user - - user_group_projects = [ - group_projects_for_user(current_user, group), - group.shared_projects.visible_to_user(current_user) - ] - if current_user.external? - user_group_projects << group.projects.public_only - else - user_group_projects << group.projects.public_and_internal_only - end - end - def all_projects(current_user) - return [public_projects] unless current_user + projects = [] - if current_user.external? - [current_user.authorized_projects, public_projects] - else - [current_user.authorized_projects, public_and_internal_projects] - end - end - - def group_projects_for_user(current_user, group) - if group.users.include?(current_user) - group.projects - else - group.projects.visible_to_user(current_user) - end - end - - def public_projects - Project.unscoped.public_only - end + projects << current_user.authorized_projects if current_user + projects << Project.unscoped.public_to_user(current_user) - def public_and_internal_projects - Project.unscoped.public_and_internal_only + projects end end diff --git a/app/finders/union_finder.rb b/app/finders/union_finder.rb new file mode 100644 index 00000000000..33cd1a491f3 --- /dev/null +++ b/app/finders/union_finder.rb @@ -0,0 +1,11 @@ +class UnionFinder + def find_union(segments, klass) + if segments.length > 1 + union = Gitlab::SQL::Union.new(segments.map { |s| s.select(:id) }) + + klass.where("#{klass.table_name}.id IN (#{union.to_sql})") + else + segments.first + end + end +end diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index 42e09149bd7..b1f0a765bb9 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -43,8 +43,4 @@ 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 7fa18ba9079..5b1bfb261a5 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -63,6 +63,14 @@ module VisibilityLevelHelper end end + def group_visibility_icon_description(group) + "#{visibility_level_label(group.visibility_level)} - #{group_visibility_level_description(group.visibility_level)}" + end + + def project_visibility_icon_description(project) + "#{visibility_level_label(project.visibility_level)} - #{project_visibility_level_description(project.visibility_level)}" + end + def visibility_level_label(level) Project.visibility_levels.key(level) end diff --git a/app/models/ability.rb b/app/models/ability.rb index 88d7ecf3a16..de9253fcdd8 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -114,6 +114,13 @@ class Ability # Push abilities on the users team role rules.push(*project_team_rules(project.team, user)) + if project.owner == user || + (project.group && project.group.has_owner?(user)) || + user.admin? + + rules.push(*project_owner_rules) + end + if project.public? || (project.internal? && !user.external?) rules.push(*public_project_rules) @@ -121,14 +128,6 @@ class Ability rules << :read_build if project.public_builds? end - if project.owner == user || user.admin? - rules.push(*project_admin_rules) - end - - if project.group && project.group.has_owner?(user) - rules.push(*project_admin_rules) - end - if project.archived? rules -= project_archived_rules end @@ -228,8 +227,8 @@ class Ability ] end - def project_admin_rules - @project_admin_rules ||= project_master_rules + [ + def project_owner_rules + @project_owner_rules ||= project_master_rules + [ :change_namespace, :change_visibility_level, :rename_project, @@ -275,7 +274,7 @@ class Ability rules << :read_group if can_read_group?(user, group) - # Only group masters and group owners can create new projects and change permission level + # Only group masters and group owners can create new projects if group.has_master?(user) || group.has_owner?(user) || user.admin? rules += [ :create_projects, @@ -298,7 +297,7 @@ class Ability def can_read_group?(user, group) user.admin? || group.public? || (group.internal? && !user.external?) || group.users.include?(user) || - ProjectsFinder.new.execute(user, group: group).any? + GroupProjectsFinder.new(group).execute(user).any? end def namespace_abilities(user, namespace) diff --git a/app/models/group.rb b/app/models/group.rb index 17c69af4d1b..900fcd71ff3 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -83,16 +83,9 @@ class Group < Namespace end def visibility_level_allowed_by_projects - unless visibility_level_allowed? - level_name = Gitlab::VisibilityLevel.level_name(visibility_level).downcase - self.errors.add(:visibility_level, "#{level_name} is not allowed since there are projects with higher visibility.") - end - end - - def visibility_level_allowed? projects_visibility = self.projects.pluck(:visibility_level) - allowed_by_projects = projects_visibility.none? { |project_visibility| self.visibility_level < project_visibility } + allowed_by_projects = projects_visibility.all? { |project_visibility| self.visibility_level >= project_visibility } unless allowed_by_projects level_name = Gitlab::VisibilityLevel.level_name(visibility_level).downcase diff --git a/app/models/project.rb b/app/models/project.rb index c1374fc9b3c..ab31a635a82 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -197,7 +197,8 @@ class Project < ActiveRecord::Base validate :avatar_type, if: ->(project) { project.avatar.present? && project.avatar_changed? } validates :avatar, file_size: { maximum: 200.kilobytes.to_i } - validate :visibility_level_allowed_in_group + validate :visibility_level_allowed_by_group + validate :visibility_level_allowed_as_fork add_authentication_token_field :runners_token before_save :ensure_runners_token @@ -441,16 +442,25 @@ class Project < ActiveRecord::Base def check_limit unless creator.can_create_project? or namespace.kind == 'group' - errors[:limit_reached] << ("Your project limit is #{creator.projects_limit} projects! Please contact your administrator to increase it") + self.errors.add(:limit_reached, "Your project limit is #{creator.projects_limit} projects! Please contact your administrator to increase it") end rescue - errors[:base] << ("Can't check your ability to create project") + self.errors.add(:base, "Can't check your ability to create project") end - def visibility_level_allowed_in_group - unless visibility_level_allowed? - self.errors.add(:visibility_level, "#{self.visibility_level} is not allowed in a #{self.group.visibility_level} group.") - end + def visibility_level_allowed_by_group + return if visibility_level_allowed_by_group? + + level_name = Gitlab::VisibilityLevel.level_name(self.visibility_level).downcase + group_level_name = Gitlab::VisibilityLevel.level_name(self.group.visibility_level).downcase + self.errors.add(:visibility_level, "#{level_name} is not allowed in a #{group_level_name} group.") + end + + def visibility_level_allowed_as_fork + return if visibility_level_allowed_as_fork? + + level_name = Gitlab::VisibilityLevel.level_name(self.visibility_level).downcase + self.errors.add(:visibility_level, "#{level_name} is not allowed since the fork source project has lower visibility.") end def to_param @@ -965,22 +975,22 @@ class Project < ActiveRecord::Base issues.opened.count end - def visibility_level_allowed?(level = self.visibility_level) - allowed_by_forks = if forked? && forked_project_link.forked_from_project_id.present? - from_project = eager_load_forked_from_project - Gitlab::VisibilityLevel.allowed_fork_levels(from_project.visibility_level).include?(level) - else - true - end + def visibility_level_allowed_as_fork?(level = self.visibility_level) + return true unless forked? && forked_project_link.forked_from_project_id.present? + + from_project = self.forked_from_project + from_project ||= Project.find(forked_project_link.forked_from_project_id) + Gitlab::VisibilityLevel.allowed_fork_levels(from_project.visibility_level).include?(level) + end - allowed_by_groups = group.present? ? level <= group.visibility_level : true + def visibility_level_allowed_by_group?(level = self.visibility_level) + return true unless group - allowed_by_forks && allowed_by_groups + level <= group.visibility_level end - #Necessary to retrieve many-to-many associations on new forks before validating visibility level - def eager_load_forked_from_project - Project.find(forked_project_link.forked_from_project_id) + def visibility_level_allowed?(level = self.visibility_level) + visibility_level_allowed_as_fork?(level) && visibility_level_allowed_by_group?(level) end def runners_token diff --git a/app/services/base_service.rb b/app/services/base_service.rb index 8563633816c..0d55ba5a981 100644 --- a/app/services/base_service.rb +++ b/app/services/base_service.rb @@ -43,12 +43,9 @@ class BaseService def deny_visibility_level(model, denied_visibility_level = nil) denied_visibility_level ||= model.visibility_level - level_name = Gitlab::VisibilityLevel.level_name(denied_visibility_level) + level_name = Gitlab::VisibilityLevel.level_name(denied_visibility_level).downcase - model.errors.add( - :visibility_level, - "#{level_name} visibility has been restricted by your GitLab administrator" - ) + model.errors.add(:visibility_level, "#{level_name} has been restricted by your GitLab administrator") end private diff --git a/app/services/create_snippet_service.rb b/app/services/create_snippet_service.rb index 101a3df5eee..9884cb96661 100644 --- a/app/services/create_snippet_service.rb +++ b/app/services/create_snippet_service.rb @@ -6,8 +6,7 @@ class CreateSnippetService < BaseService snippet = project.snippets.build(params) end - unless Gitlab::VisibilityLevel.allowed_for?(current_user, - params[:visibility_level]) + unless Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level]) deny_visibility_level(snippet) return snippet end diff --git a/app/services/groups/base_service.rb b/app/services/groups/base_service.rb index 1db81216084..1642115583d 100644 --- a/app/services/groups/base_service.rb +++ b/app/services/groups/base_service.rb @@ -1,20 +1,9 @@ module Groups - class BaseService + class BaseService < BaseService attr_accessor :group, :current_user, :params def initialize(group, user, params = {}) @group, @current_user, @params = group, user, params.dup end - - private - - def visibility_allowed_for_user? - level = group.visibility_level - allowed_by_user = Gitlab::VisibilityLevel.allowed_for?(current_user, level) - - group.errors.add(:visibility_level, "#{level} has been restricted by your GitLab administrator.") unless allowed_by_user - - allowed_by_user - end end end diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb index f605ccca81b..46c2a53e1f6 100644 --- a/app/services/groups/create_service.rb +++ b/app/services/groups/create_service.rb @@ -7,7 +7,10 @@ module Groups def execute @group = Group.new(params) - return @group unless visibility_allowed_for_user? + unless Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level]) + deny_visibility_level(@group) + return @group + end @group.name = @group.path.dup unless @group.name @group.save diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb index 0b0c5a35d37..b70e2e4aaa9 100644 --- a/app/services/groups/update_service.rb +++ b/app/services/groups/update_service.rb @@ -5,9 +5,18 @@ module Groups class UpdateService < Groups::BaseService def execute - group.assign_attributes(params) + # check that user is allowed to set specified visibility_level + new_visibility = params[:visibility_level] + if new_visibility && new_visibility.to_i != group.visibility_level + unless can?(current_user, :change_visibility_level, group) && + Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility) + + deny_visibility_level(group, new_visibility) + return group + end + end - return false unless visibility_allowed_for_user? + group.assign_attributes(params) group.save end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index c4b8420f9f2..501e58c1407 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -10,7 +10,7 @@ module Projects @project = Project.new(params) # Make sure that the user is allowed to use the specified visibility level - unless visibility_level_allowed? + unless Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level]) deny_visibility_level(@project) return @project end @@ -96,9 +96,5 @@ 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/services/projects/update_service.rb b/app/services/projects/update_service.rb index 895e089bea3..941df08995c 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -3,16 +3,13 @@ module Projects def execute # check that user is allowed to set specified visibility_level new_visibility = params[:visibility_level] - if new_visibility - if new_visibility.to_i != project.visibility_level - unless can?(current_user, :change_visibility_level, project) && - Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility) - deny_visibility_level(project, new_visibility) - return project - end + if new_visibility && new_visibility.to_i != project.visibility_level + unless can?(current_user, :change_visibility_level, project) && + Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility) + + deny_visibility_level(project, new_visibility) + return project end - - return false unless visibility_level_allowed?(new_visibility) end new_branch = params[:default_branch] @@ -27,19 +24,5 @@ module Projects end end end - - private - - def visibility_level_allowed?(level) - return true if project.visibility_level_allowed?(level) - - level_name = Gitlab::VisibilityLevel.level_name(level) - project.errors.add( - :visibility_level, - "#{level_name} could not be set as visibility level of this project - parent project settings are more restrictive" - ) - - false - end end end diff --git a/app/services/update_snippet_service.rb b/app/services/update_snippet_service.rb index e9328bb7323..93af8f21972 100644 --- a/app/services/update_snippet_service.rb +++ b/app/services/update_snippet_service.rb @@ -9,7 +9,6 @@ class UpdateSnippetService < BaseService def execute # check that user is allowed to set specified visibility_level new_visibility = params[:visibility_level] - if new_visibility && new_visibility.to_i != snippet.visibility_level unless Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility) deny_visibility_level(snippet, new_visibility) diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml index 222c3e4a40e..5a9fa5d9a4d 100644 --- a/app/views/groups/show.html.haml +++ b/app/views/groups/show.html.haml @@ -17,7 +17,7 @@ .cover-title %h1 = @group.name - %span.visibility-icon.has_tooltip{ data: { container: 'body', placement: 'left' }, title: group_visibility_description(@group) } + %span.visibility-icon.has_tooltip{ data: { container: 'body' }, title: group_visibility_icon_description(@group) } = visibility_level_icon(@group.visibility_level, fw: false) .cover-desc.username @@ -27,34 +27,29 @@ .cover-desc.description = markdown(@group.description, pipeline: :description) -- if can?(current_user, :read_group, @group) - %div{ class: container_class } - .top-area - %ul.nav-links - %li.active - = link_to "#projects", 'data-toggle' => 'tab' do - All Projects - - if @shared_projects.present? - %li - = link_to "#shared", 'data-toggle' => 'tab' do - Shared Projects - .nav-controls - = form_tag request.original_url, method: :get, class: 'project-filter-form', id: 'project-filter-form' do |f| - = search_field_tag :filter_projects, nil, placeholder: 'Filter by name', class: 'projects-list-filter form-control', spellcheck: false - = render 'shared/projects/dropdown' - - if can? current_user, :create_projects, @group - = link_to new_project_path(namespace_id: @group.id), class: 'btn btn-new pull-right' do - = icon('plus') - New Project - - .tab-content - .tab-pane.active#projects - = render "projects", projects: @projects - +%div{ class: container_class } + .top-area + %ul.nav-links + %li.active + = link_to "#projects", 'data-toggle' => 'tab' do + All Projects - if @shared_projects.present? - .tab-pane#shared - = render "shared_projects", projects: @shared_projects - -- else - %p.nav-links.no-top - No projects to show + %li + = link_to "#shared", 'data-toggle' => 'tab' do + Shared Projects + .nav-controls + = form_tag request.original_url, method: :get, class: 'project-filter-form', id: 'project-filter-form' do |f| + = search_field_tag :filter_projects, nil, placeholder: 'Filter by name', class: 'projects-list-filter form-control', spellcheck: false + = render 'shared/projects/dropdown' + - if can? current_user, :create_projects, @group + = link_to new_project_path(namespace_id: @group.id), class: 'btn btn-new pull-right' do + = icon('plus') + New Project + + .tab-content + .tab-pane.active#projects + = render "projects", projects: @projects + + - if @shared_projects.present? + .tab-pane#shared + = render "shared_projects", projects: @shared_projects diff --git a/app/views/layouts/nav/_group.html.haml b/app/views/layouts/nav/_group.html.haml index 59411ae1da1..55940741dc0 100644 --- a/app/views/layouts/nav/_group.html.haml +++ b/app/views/layouts/nav/_group.html.haml @@ -12,40 +12,38 @@ = icon('group fw') %span Group - - if can?(current_user, :read_group, @group) - = nav_link(path: 'groups#activity') do - = link_to activity_group_path(@group), title: 'Activity' do - = icon('dashboard fw') - %span - Activity - - if current_user - = nav_link(controller: [:group, :milestones]) do - = link_to group_milestones_path(@group), title: 'Milestones' do - = icon('clock-o fw') - %span - Milestones - = nav_link(path: 'groups#issues') do - = link_to issues_group_path(@group), title: 'Issues' do - = icon('exclamation-circle fw') - %span - Issues - - if current_user - %span.count= number_with_delimiter(Issue.opened.of_group(@group).count) - = nav_link(path: 'groups#merge_requests') do - = link_to merge_requests_group_path(@group), title: 'Merge Requests' do - = icon('tasks fw') - %span - Merge Requests - - if current_user - %span.count= number_with_delimiter(MergeRequest.opened.of_group(@group).count) - = nav_link(controller: [:group_members]) do - = link_to group_group_members_path(@group), title: 'Members' do - = icon('users fw') + = nav_link(path: 'groups#activity') do + = link_to activity_group_path(@group), title: 'Activity' do + = icon('dashboard fw') + %span + Activity + = nav_link(controller: [:group, :milestones]) do + = link_to group_milestones_path(@group), title: 'Milestones' do + = icon('clock-o fw') + %span + Milestones + = nav_link(path: 'groups#issues') do + = link_to issues_group_path(@group), title: 'Issues' do + = icon('exclamation-circle fw') + %span + Issues + - issues = IssuesFinder.new(current_user, group_id: @group.id, state: 'opened').execute + %span.count= number_with_delimiter(issues.count) + = nav_link(path: 'groups#merge_requests') do + = link_to merge_requests_group_path(@group), title: 'Merge Requests' do + = icon('tasks fw') + %span + Merge Requests + - merge_requests = MergeRequestsFinder.new(current_user, group_id: @group.id, state: 'opened').execute + %span.count= number_with_delimiter(merge_requests.count) + = nav_link(controller: [:group_members]) do + = link_to group_group_members_path(@group), title: 'Members' do + = icon('users fw') + %span + Members + - if can?(current_user, :admin_group, @group) + = nav_link(html_options: { class: "separate-item" }) do + = link_to edit_group_path(@group), title: 'Settings' do + = icon ('cogs fw') %span - Members - - if can?(current_user, :admin_group, @group) - = nav_link(html_options: { class: "separate-item" }) do - = link_to edit_group_path(@group), title: 'Settings' do - = icon ('cogs fw') - %span - Settings + Settings diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml index e8434b5292c..d4bbafbd40f 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -2,21 +2,21 @@ .project-home-panel.cover-block.clearfix{:class => ("empty-project" if empty_repo)} .project-identicon-holder = project_icon(@project, alt: '', class: 'project-avatar avatar s90') - .cover-title#project-home-desc + .cover-title.project-home-desc %h1 = @project.name - %span.visibility-icon.has_tooltip{data: { container: 'body' }, - title: "#{visibility_level_label(@project.visibility_level)} - #{project_visibility_level_description(@project.visibility_level)}"} + %span.visibility-icon.has_tooltip{data: { container: 'body' }, title: project_visibility_icon_description(@project)} = visibility_level_icon(@project.visibility_level, fw: false) - - if @project.description.present? + - if @project.description.present? + .cover-desc.project-home-desc = markdown(@project.description, pipeline: :description) - - if forked_from_project = @project.forked_from_project - %p - Forked from - = link_to project_path(forked_from_project) do - = forked_from_project.namespace.try(:name) + - if forked_from_project = @project.forked_from_project + .cover-desc + Forked from + = link_to project_path(forked_from_project) do + = forked_from_project.namespace.try(:name) .cover-controls - if current_user diff --git a/app/views/shared/groups/_group.html.haml b/app/views/shared/groups/_group.html.haml index 67a59f420f3..03103d46bb9 100644 --- a/app/views/shared/groups/_group.html.haml +++ b/app/views/shared/groups/_group.html.haml @@ -21,7 +21,7 @@ = icon('users') = number_with_delimiter(group.users.count) - %span{title: group_visibility_description(group)} + %span.visibility-icon.has_tooltip{data: { container: 'body', placement: 'left' }, title: group_visibility_icon_description(group)} = visibility_level_icon(group.visibility_level, fw: false) = image_tag group_icon(group), class: "avatar s40 hidden-xs" diff --git a/app/views/shared/projects/_project.html.haml b/app/views/shared/projects/_project.html.haml index 872d2bdf46d..3b987987676 100644 --- a/app/views/shared/projects/_project.html.haml +++ b/app/views/shared/projects/_project.html.haml @@ -27,8 +27,7 @@ %span = icon('star') = project.star_count - %span.visibility-icon.has_tooltip{data: { container: 'body', placement: 'left' }, - title: "#{visibility_level_label(project.visibility_level)} - #{project_visibility_level_description(project.visibility_level)}"} + %span.visibility-icon.has_tooltip{data: { container: 'body', placement: 'left' }, title: project_visibility_icon_description(project)} = visibility_level_icon(project.visibility_level, fw: false) .title 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 d5c78db0aa3..62d96907c8f 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 @@ -5,7 +5,7 @@ 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}") + execute("UPDATE application_settings SET default_group_visibility = #{allowed_visibility_level}") end def down @@ -15,6 +15,7 @@ class AddDefaultGroupVisibilityToApplicationSettings < ActiveRecord::Migration private def allowed_visibility_level + # TODO: Don't use `current_application_settings` allowed_levels = Gitlab::VisibilityLevel.values - current_application_settings.restricted_visibility_levels allowed_levels.max end diff --git a/db/schema.rb b/db/schema.rb index 719fedcdbfb..2aa37cc590c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -590,8 +590,8 @@ ActiveRecord::Schema.define(version: 20160316204731) 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 + t.integer "visibility_level", default: 20, null: false end add_index "namespaces", ["created_at", "id"], name: "index_namespaces_on_created_at_and_id", using: :btree diff --git a/doc/api/groups.md b/doc/api/groups.md index d47e79ba47f..d1b5c9f5f04 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -111,6 +111,7 @@ Parameters: - `name` (required) - The name of the group
- `path` (required) - The path of the group
- `description` (optional) - The group's description
+- `visibility_level` (optional) - The group's visibility. 0 for private, 10 for internal, 20 for public.
## Transfer project to group
diff --git a/lib/api/groups.rb b/lib/api/groups.rb index 1a14d870a4a..c165de21a75 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -31,7 +31,7 @@ module API authorize! :create_group, current_user required_attributes! [:name, :path] - attrs = attributes_for_keys [:name, :path, :description] + attrs = attributes_for_keys [:name, :path, :description, :visibility_level] @group = Group.new(attrs) if @group.save diff --git a/lib/gitlab/visibility_level.rb b/lib/gitlab/visibility_level.rb index 58cd8df5219..0a1f66b0726 100644 --- a/lib/gitlab/visibility_level.rb +++ b/lib/gitlab/visibility_level.rb @@ -11,6 +11,8 @@ module Gitlab included do scope :public_only, -> { where(visibility_level: PUBLIC) } scope :public_and_internal_only, -> { where(visibility_level: [PUBLIC, INTERNAL] ) } + + scope :public_to_user, -> (user) { user && !user.external ? public_and_internal_only : public_only } end PRIVATE = 0 unless const_defined?(:PRIVATE) diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 91db3fd1ee2..938e97298b6 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -20,43 +20,4 @@ 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/controllers/namespaces_controller_spec.rb b/spec/controllers/namespaces_controller_spec.rb index 6350c9c6e48..41ae6063ae0 100644 --- a/spec/controllers/namespaces_controller_spec.rb +++ b/spec/controllers/namespaces_controller_spec.rb @@ -15,12 +15,11 @@ describe NamespacesController do end context "when the namespace belongs to a group" do - let!(:group) { create(:group, visibility_level: Gitlab::VisibilityLevel::PUBLIC) } - let!(:project) { create(:project, namespace: group) } + let!(:group) { create(:group) } - context "when the group has public projects" do + context "when the group is public" do before do - project.update_attribute(:visibility_level, Project::PUBLIC) + group.update_attribute(:visibility_level, Group::PUBLIC) end context "when not signed in" do @@ -44,27 +43,27 @@ describe NamespacesController do end end - context "when the project doesn't have public projects" do + context "when the group is private" do context "when not signed in" do it "does not redirect to the sign in page" do get :show, id: group.path expect(response).not_to redirect_to(new_user_session_path) end end + context "when signed in" do before do sign_in(user) end - context "when the user has access to the project" do + context "when the user has access to the group" do before do - project.team << [user, :master] + group.add_developer(user) end context "when the user is blocked" do before do user.block - project.team << [user, :master] end it "redirects to the sign in page" do @@ -83,11 +82,11 @@ describe NamespacesController do end end - context "when the user doesn't have access to the project" do - it "redirects to the group's page" do + context "when the user doesn't have access to the group" do + it "responds with status 404" do get :show, id: group.path - expect(response).to redirect_to(group_path(group)) + expect(response.status).to eq(404) end end end diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index bf5b13f2645..0947744fc47 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -127,12 +127,10 @@ describe UploadsController do context "when viewing a group avatar" do let!(:group) { create(:group, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) } - let!(:project) { create(:project, namespace: group) } - context "when the group has public projects" do + context "when the group is public" do before do group.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC) - project.update_attribute(:visibility_level, Project::PUBLIC) end context "when not signed in" do @@ -156,7 +154,7 @@ describe UploadsController do end end - context "when the project doesn't have public projects" do + context "when the group is private" do context "when signed in" do before do sign_in(user) @@ -164,13 +162,12 @@ describe UploadsController do context "when the user has access to the project" do before do - project.team << [user, :master] + project.add_developer(user) end context "when the user is blocked" do before do user.block - project.team << [user, :master] end it "redirects to the sign in page" do diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index e54a5a0b72a..ed97b6cb577 100644 --- a/spec/features/projects_spec.rb +++ b/spec/features/projects_spec.rb @@ -12,25 +12,25 @@ feature 'Project', feature: true do it 'parses Markdown' do project.update_attribute(:description, 'This is **my** project') visit path - expect(page).to have_css('.cover-title > p > strong') + expect(page).to have_css('.project-home-desc > p > strong') end it 'passes through html-pipeline' do project.update_attribute(:description, 'This project is the :poop:') visit path - expect(page).to have_css('.cover-title > p > img') + expect(page).to have_css('.project-home-desc > p > img') end it 'sanitizes unwanted tags' do project.update_attribute(:description, "```\ncode\n```") visit path - expect(page).not_to have_css('.cover-title code') + expect(page).not_to have_css('.project-home-desc code') end it 'permits `rel` attribute on links' do project.update_attribute(:description, 'https://google.com/') visit path - expect(page).to have_css('.cover-title a[rel]') + expect(page).to have_css('.project-home-desc a[rel]') end end diff --git a/spec/features/security/group/internal_access_spec.rb b/spec/features/security/group/internal_access_spec.rb index e44d4c32921..d76eb454fe5 100644 --- a/spec/features/security/group/internal_access_spec.rb +++ b/spec/features/security/group/internal_access_spec.rb @@ -1,113 +1,109 @@ require 'rails_helper' -describe 'Internal group access', feature: true do +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 external_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 } - it { is_expected.to be_denied_for :external } + let(:group) { create(:group, :internal) } + let(:project) { create(:project, :internal, group: group) } - end + let(:owner) { create(:user) } + let(:master) { create(:user) } + let(:developer) { create(:user) } + let(:reporter) { create(:user) } + let(:guest) { create(:user) } - 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 + let(:project_guest) { create(:user) } + + before do + group.add_user(owner, Gitlab::Access::OWNER) + group.add_user(master, Gitlab::Access::MASTER) + group.add_user(developer, Gitlab::Access::DEVELOPER) + group.add_user(reporter, Gitlab::Access::REPORTER) + group.add_user(guest, Gitlab::Access::GUEST) + + project.team << [project_guest, :guest] 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 external_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 } - it { is_expected.to be_denied_for :external } + describe "Group should be internal" do + describe '#internal?' do + subject { group.internal? } + it { is_expected.to be_truthy } end + 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 + describe 'GET /groups/:path' do + subject { group_path(group) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_allowed_for project_guest } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :external } + it { is_expected.to be_denied_for :visitor } 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 external_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 } - it { is_expected.to be_denied_for :external } - end + describe 'GET /groups/:path/issues' do + subject { issues_group_path(group) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_allowed_for project_guest } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :external } + it { is_expected.to be_denied_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 + describe 'GET /groups/:path/merge_requests' do + subject { merge_requests_group_path(group) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_allowed_for project_guest } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :external } + it { is_expected.to be_denied_for :visitor } 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 external_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 } - it { is_expected.to be_denied_for :external } - 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 + subject { group_group_members_path(group) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_allowed_for project_guest } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_denied_for :external } + it { is_expected.to be_denied_for :visitor } 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 external_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 } - it { is_expected.to be_denied_for :external } - 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 + subject { edit_group_path(group) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_denied_for master } + it { is_expected.to be_denied_for developer } + it { is_expected.to be_denied_for reporter } + it { is_expected.to be_denied_for guest } + it { is_expected.to be_denied_for project_guest } + 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 end diff --git a/spec/features/security/group/private_access_spec.rb b/spec/features/security/group/private_access_spec.rb index 8d8c61a618f..8ca4a0ac83b 100644 --- a/spec/features/security/group/private_access_spec.rb +++ b/spec/features/security/group/private_access_spec.rb @@ -1,114 +1,109 @@ require 'rails_helper' -describe 'Private group access', feature: true do +describe 'Private Group access', feature: true do include AccessMatchers - include GroupAccessHelper + let(:group) { create(:group, :private) } + let(:project) { create(:project, :private, group: group) } + let(:owner) { create(:user) } + let(:master) { create(:user) } + let(:developer) { create(:user) } + let(:reporter) { create(:user) } + let(:guest) { create(:user) } - 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 external_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 } - it { is_expected.to be_denied_for :external } - end + let(:project_guest) { create(:user) } - 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 + before do + group.add_user(owner, Gitlab::Access::OWNER) + group.add_user(master, Gitlab::Access::MASTER) + group.add_user(developer, Gitlab::Access::DEVELOPER) + group.add_user(reporter, Gitlab::Access::REPORTER) + group.add_user(guest, Gitlab::Access::GUEST) + + project.team << [project_guest, :guest] 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 external_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 } - it { is_expected.to be_denied_for :external } + describe "Group should be private" do + describe '#private?' do + subject { group.private? } + it { is_expected.to be_truthy } end + 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 + describe 'GET /groups/:path' do + subject { group_path(group) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_allowed_for project_guest } + it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } + it { is_expected.to be_denied_for :visitor } 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 external_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 } - it { is_expected.to be_denied_for :external } - end + describe 'GET /groups/:path/issues' do + subject { issues_group_path(group) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_allowed_for project_guest } + it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } + it { is_expected.to be_denied_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 + describe 'GET /groups/:path/merge_requests' do + subject { merge_requests_group_path(group) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_allowed_for project_guest } + it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } + it { is_expected.to be_denied_for :visitor } 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 external_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 } - it { is_expected.to be_denied_for :external } - 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 + subject { group_group_members_path(group) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_allowed_for project_guest } + it { is_expected.to be_denied_for :user } + it { is_expected.to be_denied_for :external } + it { is_expected.to be_denied_for :visitor } 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 external_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 } - it { is_expected.to be_denied_for :external } - 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 + subject { edit_group_path(group) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_denied_for master } + it { is_expected.to be_denied_for developer } + it { is_expected.to be_denied_for reporter } + it { is_expected.to be_denied_for guest } + it { is_expected.to be_denied_for project_guest } + 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 end diff --git a/spec/features/security/group/public_access_spec.rb b/spec/features/security/group/public_access_spec.rb index 5ff982504c5..f556fabb51e 100644 --- a/spec/features/security/group/public_access_spec.rb +++ b/spec/features/security/group/public_access_spec.rb @@ -1,114 +1,109 @@ require 'rails_helper' -describe 'Public group access', feature: true do +describe 'Public Group access', feature: true do include AccessMatchers - include GroupAccessHelper + let(:group) { create(:group, :public) } + let(:project) { create(:project, :public, group: group) } + let(:owner) { create(:user) } + let(:master) { create(:user) } + let(:developer) { create(:user) } + let(:reporter) { create(:user) } + let(:guest) { create(:user) } - 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 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 + let(:project_guest) { create(:user) } - 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 + before do + group.add_user(owner, Gitlab::Access::OWNER) + group.add_user(master, Gitlab::Access::MASTER) + group.add_user(developer, Gitlab::Access::DEVELOPER) + group.add_user(reporter, Gitlab::Access::REPORTER) + group.add_user(guest, Gitlab::Access::GUEST) + + project.team << [project_guest, :guest] 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 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 } + describe "Group should be public" do + describe '#public?' do + subject { group.public? } + it { is_expected.to be_truthy } end + 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 + describe 'GET /groups/:path' do + subject { group_path(group) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_allowed_for project_guest } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :external } + it { is_expected.to be_allowed_for :visitor } 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 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 + describe 'GET /groups/:path/issues' do + subject { issues_group_path(group) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_allowed_for project_guest } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :external } + 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 + describe 'GET /groups/:path/merge_requests' do + subject { merge_requests_group_path(group) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_allowed_for project_guest } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :external } + it { is_expected.to be_allowed_for :visitor } 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 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 - it { is_expected.to be_allowed_for project_group_member(:user) } - it { is_expected.to be_allowed_for :visitor } - end + subject { group_group_members_path(group) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } + it { is_expected.to be_allowed_for reporter } + it { is_expected.to be_allowed_for guest } + it { is_expected.to be_allowed_for project_guest } + it { is_expected.to be_allowed_for :user } + it { is_expected.to be_allowed_for :external } + it { is_expected.to be_allowed_for :visitor } 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 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 - it { is_expected.to be_allowed_for project_group_member(:user) } - it { is_expected.to be_allowed_for :visitor } - end + subject { edit_group_path(group) } + + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } + it { is_expected.to be_denied_for master } + it { is_expected.to be_denied_for developer } + it { is_expected.to be_denied_for reporter } + it { is_expected.to be_denied_for guest } + it { is_expected.to be_denied_for project_guest } + 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 end diff --git a/spec/features/security/group_access_spec.rb b/spec/features/security/group_access_spec.rb deleted file mode 100644 index 55bbeafba33..00000000000 --- a/spec/features/security/group_access_spec.rb +++ /dev/null @@ -1,244 +0,0 @@ -require 'rails_helper' - -describe 'Group access', feature: true do - include AccessMatchers - - def group - @group ||= create(:group, visibility_level: Gitlab::VisibilityLevel::PUBLIC) - end - - def create_project(access_level) - if access_level == :mixed - create(:empty_project, :public, group: group) - create(:empty_project, :internal, group: group) - else - create(:empty_project, access_level, group: group) - end - end - - def group_member(access_level, grp = group()) - level = Object.const_get("Gitlab::Access::#{access_level.upcase}") - - create(:user).tap do |user| - grp.add_user(user, level) - end - end - - describe 'GET /groups/new' do - subject { new_group_path } - - 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 - - describe 'GET /groups/:path' do - subject { group_path(group) } - - context 'with public projects' do - let!(:project) { create_project(:public) } - - 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 } - end - - context 'with mixed projects' do - let!(:project) { create_project(:mixed) } - - 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 } - end - - context 'with internal projects' do - let!(:project) { create_project(:internal) } - - 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 } - end - - context 'with no projects' 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 } - end - end - - describe 'GET /groups/:path/issues' do - subject { issues_group_path(group) } - - context 'with public projects' do - let!(:project) { create_project(:public) } - - 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 } - end - - context 'with mixed projects' do - let!(:project) { create_project(:mixed) } - - 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 } - end - - context 'with internal projects' do - let!(:project) { create_project(:internal) } - - 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 } - end - - context 'with no projects' 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 } - end - end - - describe 'GET /groups/:path/merge_requests' do - subject { merge_requests_group_path(group) } - - context 'with public projects' do - let!(:project) { create_project(:public) } - - 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 } - end - - context 'with mixed projects' do - let!(:project) { create_project(:mixed) } - - 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 } - end - - context 'with internal projects' do - let!(:project) { create_project(:internal) } - - 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 } - end - - context 'with no projects' 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 } - end - end - - describe 'GET /groups/:path/group_members' do - subject { group_group_members_path(group) } - - context 'with public projects' do - let!(:project) { create_project(:public) } - - 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 } - end - - context 'with mixed projects' do - let!(:project) { create_project(:mixed) } - - 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 } - end - - context 'with internal projects' do - let!(:project) { create_project(:internal) } - - 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 } - end - - context 'with no projects' 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 } - end - end - - describe 'GET /groups/:path/edit' do - subject { edit_group_path(group) } - - context 'with public projects' do - let!(:project) { create_project(:public) } - - it { is_expected.to be_allowed_for group_member(:owner) } - it { is_expected.to be_denied_for group_member(:master) } - 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 } - end - - context 'with mixed projects' do - let!(:project) { create_project(:mixed) } - - it { is_expected.to be_allowed_for group_member(:owner) } - it { is_expected.to be_denied_for group_member(:master) } - 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 } - end - - context 'with internal projects' do - let!(:project) { create_project(:internal) } - - it { is_expected.to be_allowed_for group_member(:owner) } - it { is_expected.to be_denied_for group_member(:master) } - 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 } - end - - context 'with no projects' do - it { is_expected.to be_allowed_for group_member(:owner) } - it { is_expected.to be_denied_for group_member(:master) } - 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 } - end - end -end diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb index f88c591d897..79d5bf4cf06 100644 --- a/spec/features/security/project/internal_access_spec.rb +++ b/spec/features/security/project/internal_access_spec.rb @@ -5,25 +5,22 @@ describe "Internal Project Access", feature: true do let(:project) { create(:project, :internal) } - let(:master) { create(:user) } - let(:guest) { create(:user) } - let(:reporter) { create(:user) } - let(:external_team_member) { create(:user, external: true) } + let(:owner) { project.owner } + let(:master) { create(:user) } + let(:developer) { create(:user) } + let(:reporter) { create(:user) } + let(:guest) { create(:user) } before do - # full access project.team << [master, :master] - project.team << [external_team_member, :master] - - # readonly + project.team << [developer, :developer] project.team << [reporter, :reporter] + project.team << [guest, :guest] end describe "Project should be internal" do - subject { project } - describe '#internal?' do - subject { super().internal? } + subject { project.internal? } it { is_expected.to be_truthy } end end @@ -31,78 +28,84 @@ describe "Internal Project Access", feature: true do describe "GET /:project_path" do subject { namespace_project_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/tree/master" do subject { namespace_project_tree_path(project.namespace, project, project.repository.root_ref) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/commits/master" do subject { namespace_project_commits_path(project.namespace, project, project.repository.root_ref, limit: 1) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/commit/:sha" do subject { namespace_project_commit_path(project.namespace, project, project.repository.commit) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/compare" do subject { namespace_project_compare_index_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/project_members" do subject { namespace_project_project_members_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_denied_for developer } it { is_expected.to be_denied_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -110,52 +113,56 @@ describe "Internal Project Access", feature: true do let(:commit) { project.repository.commit } subject { namespace_project_blob_path(project.namespace, project, File.join(commit.id, '.gitignore')) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/edit" do subject { edit_namespace_project_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_denied_for developer } it { is_expected.to be_denied_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/deploy_keys" do subject { namespace_project_deploy_keys_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_denied_for developer } it { is_expected.to be_denied_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/issues" do subject { namespace_project_issues_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -163,65 +170,70 @@ describe "Internal Project Access", feature: true do let(:issue) { create(:issue, project: project) } subject { edit_namespace_project_issue_path(project.namespace, project, issue) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/snippets" do subject { namespace_project_snippets_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/snippets/new" do subject { new_namespace_project_snippet_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/merge_requests" do subject { namespace_project_merge_requests_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/merge_requests/new" do subject { new_namespace_project_merge_request_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_denied_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -233,13 +245,14 @@ describe "Internal Project Access", feature: true do allow_any_instance_of(Project).to receive(:branches).and_return([]) end + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -251,26 +264,28 @@ describe "Internal Project Access", feature: true do allow_any_instance_of(Project).to receive(:tags).and_return([]) end + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/hooks" do subject { namespace_project_hooks_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_denied_for developer } it { is_expected.to be_denied_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end end diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb index 19f287ce7a4..0a89193eb67 100644 --- a/spec/features/security/project/private_access_spec.rb +++ b/spec/features/security/project/private_access_spec.rb @@ -3,27 +3,24 @@ require 'spec_helper' describe "Private Project Access", feature: true do include AccessMatchers - let(:project) { create(:project) } + let(:project) { create(:project, :private) } - let(:master) { create(:user) } - let(:guest) { create(:user) } - let(:reporter) { create(:user) } - let(:external_team_member) { create(:user, external: true) } + let(:owner) { project.owner } + let(:master) { create(:user) } + let(:developer) { create(:user) } + let(:reporter) { create(:user) } + let(:guest) { create(:user) } before do - # full access project.team << [master, :master] - project.team << [external_team_member, :master] - - # readonly + project.team << [developer, :developer] project.team << [reporter, :reporter] + project.team << [guest, :guest] end describe "Project should be private" do - subject { project } - describe '#private?' do - subject { super().private? } + subject { project.private? } it { is_expected.to be_truthy } end end @@ -31,77 +28,84 @@ describe "Private Project Access", feature: true do describe "GET /:project_path" do subject { namespace_project_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } - it { is_expected.to be_denied_for guest } + it { is_expected.to be_allowed_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/tree/master" do subject { namespace_project_tree_path(project.namespace, project, project.repository.root_ref) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/commits/master" do subject { namespace_project_commits_path(project.namespace, project, project.repository.root_ref, limit: 1) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/commit/:sha" do subject { namespace_project_commit_path(project.namespace, project, project.repository.commit) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } - it { is_expected.to be_allowed_for external_team_member } + it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/compare" do subject { namespace_project_compare_index_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/project_members" do subject { namespace_project_project_members_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_denied_for developer } it { is_expected.to be_denied_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -109,52 +113,56 @@ describe "Private Project Access", feature: true do let(:commit) { project.repository.commit } subject { namespace_project_blob_path(project.namespace, project, File.join(commit.id, '.gitignore'))} + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/edit" do subject { edit_namespace_project_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_denied_for developer } it { is_expected.to be_denied_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/deploy_keys" do subject { namespace_project_deploy_keys_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_denied_for developer } it { is_expected.to be_denied_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/issues" do subject { namespace_project_issues_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } - it { is_expected.to be_denied_for guest } + it { is_expected.to be_allowed_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -162,39 +170,42 @@ describe "Private Project Access", feature: true do let(:issue) { create(:issue, project: project) } subject { edit_namespace_project_issue_path(project.namespace, project, issue) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/snippets" do subject { namespace_project_snippets_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } - it { is_expected.to be_denied_for guest } + it { is_expected.to be_allowed_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/merge_requests" do subject { namespace_project_merge_requests_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } - it { is_expected.to be_denied_for guest } + it { is_expected.to be_allowed_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -206,13 +217,14 @@ describe "Private Project Access", feature: true do allow_any_instance_of(Project).to receive(:branches).and_return([]) end + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end @@ -224,26 +236,28 @@ describe "Private Project Access", feature: true do allow_any_instance_of(Project).to receive(:tags).and_return([]) end + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end describe "GET /:project_path/hooks" do subject { namespace_project_hooks_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_denied_for developer } it { is_expected.to be_denied_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } - it { is_expected.to be_allowed_for external_team_member } it { is_expected.to be_denied_for :visitor } end end diff --git a/spec/features/security/project/public_access_spec.rb b/spec/features/security/project/public_access_spec.rb index 4e135076367..40daac89d40 100644 --- a/spec/features/security/project/public_access_spec.rb +++ b/spec/features/security/project/public_access_spec.rb @@ -3,29 +3,24 @@ require 'spec_helper' describe "Public Project Access", feature: true do include AccessMatchers - let(:project) { create(:project) } + let(:project) { create(:project, :public) } - let(:master) { create(:user) } - let(:guest) { create(:user) } - let(:reporter) { create(:user) } + let(:owner) { project.owner } + let(:master) { create(:user) } + let(:developer) { create(:user) } + let(:reporter) { create(:user) } + let(:guest) { create(:user) } before do - # public project - project.visibility_level = Gitlab::VisibilityLevel::PUBLIC - project.save! - - # full access project.team << [master, :master] - - # readonly + project.team << [developer, :developer] project.team << [reporter, :reporter] + project.team << [guest, :guest] end describe "Project should be public" do - subject { project } - describe '#public?' do - subject { super().public? } + subject { project.public? } it { is_expected.to be_truthy } end end @@ -33,9 +28,11 @@ describe "Public Project Access", feature: true do describe "GET /:project_path" do subject { namespace_project_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :external } @@ -45,9 +42,11 @@ describe "Public Project Access", feature: true do describe "GET /:project_path/tree/master" do subject { namespace_project_tree_path(project.namespace, project, project.repository.root_ref) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :external } @@ -57,9 +56,11 @@ describe "Public Project Access", feature: true do describe "GET /:project_path/commits/master" do subject { namespace_project_commits_path(project.namespace, project, project.repository.root_ref, limit: 1) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :external } @@ -69,9 +70,11 @@ describe "Public Project Access", feature: true do describe "GET /:project_path/commit/:sha" do subject { namespace_project_commit_path(project.namespace, project, project.repository.commit) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :external } @@ -81,9 +84,11 @@ describe "Public Project Access", feature: true do describe "GET /:project_path/compare" do subject { namespace_project_compare_index_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :external } @@ -93,9 +98,11 @@ describe "Public Project Access", feature: true do describe "GET /:project_path/project_members" do subject { namespace_project_project_members_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_denied_for developer } it { is_expected.to be_denied_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } @@ -108,9 +115,11 @@ describe "Public Project Access", feature: true do context "when allowed for public" do before { project.update(public_builds: true) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :external } @@ -120,9 +129,11 @@ describe "Public Project Access", feature: true do context "when disallowed for public" do before { project.update(public_builds: false) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } @@ -138,9 +149,11 @@ describe "Public Project Access", feature: true do context "when allowed for public" do before { project.update(public_builds: true) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :external } @@ -150,9 +163,11 @@ describe "Public Project Access", feature: true do context "when disallowed for public" do before { project.update(public_builds: false) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } @@ -165,9 +180,11 @@ describe "Public Project Access", feature: true do subject { namespace_project_blob_path(project.namespace, project, File.join(commit.id, '.gitignore')) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :visitor } @@ -176,9 +193,11 @@ describe "Public Project Access", feature: true do describe "GET /:project_path/edit" do subject { edit_namespace_project_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_denied_for developer } it { is_expected.to be_denied_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } @@ -188,9 +207,11 @@ describe "Public Project Access", feature: true do describe "GET /:project_path/deploy_keys" do subject { namespace_project_deploy_keys_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_denied_for developer } it { is_expected.to be_denied_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } @@ -200,9 +221,11 @@ describe "Public Project Access", feature: true do describe "GET /:project_path/issues" do subject { namespace_project_issues_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :external } @@ -213,9 +236,11 @@ describe "Public Project Access", feature: true do let(:issue) { create(:issue, project: project) } subject { edit_namespace_project_issue_path(project.namespace, project, issue) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } @@ -225,9 +250,11 @@ describe "Public Project Access", feature: true do describe "GET /:project_path/snippets" do subject { namespace_project_snippets_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :external } @@ -237,9 +264,11 @@ describe "Public Project Access", feature: true do describe "GET /:project_path/snippets/new" do subject { new_namespace_project_snippet_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } @@ -249,9 +278,11 @@ describe "Public Project Access", feature: true do describe "GET /:project_path/merge_requests" do subject { namespace_project_merge_requests_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :external } @@ -261,9 +292,11 @@ describe "Public Project Access", feature: true do describe "GET /:project_path/merge_requests/new" do subject { new_namespace_project_merge_request_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_denied_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } @@ -278,9 +311,11 @@ describe "Public Project Access", feature: true do allow_any_instance_of(Project).to receive(:branches).and_return([]) end + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :external } @@ -295,9 +330,11 @@ describe "Public Project Access", feature: true do allow_any_instance_of(Project).to receive(:tags).and_return([]) end + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for guest } it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :external } @@ -307,9 +344,11 @@ describe "Public Project Access", feature: true do describe "GET /:project_path/hooks" do subject { namespace_project_hooks_path(project.namespace, project) } + it { is_expected.to be_allowed_for :admin } + it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for master } + it { is_expected.to be_denied_for developer } it { is_expected.to be_denied_for reporter } - it { is_expected.to be_allowed_for :admin } it { is_expected.to be_denied_for guest } it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for :external } diff --git a/spec/helpers/groups_helper.rb b/spec/helpers/groups_helper_spec.rb index 01ec9e5a07f..4ea90a80a92 100644 --- a/spec/helpers/groups_helper.rb +++ b/spec/helpers/groups_helper_spec.rb @@ -18,19 +18,4 @@ 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/support/group_access_helper.rb b/spec/support/group_access_helper.rb deleted file mode 100644 index c8ed0e406a1..00000000000 --- a/spec/support/group_access_helper.rb +++ /dev/null @@ -1,21 +0,0 @@ -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 - - def external_guest(grp=group()) - create(:user, external: true).tap { |user| grp.add_user(user, Gitlab::Access::GUEST) } - end -end diff --git a/spec/support/matchers/access_matchers.rb b/spec/support/matchers/access_matchers.rb index 4e007c777e3..0497e391860 100644 --- a/spec/support/matchers/access_matchers.rb +++ b/spec/support/matchers/access_matchers.rb @@ -28,7 +28,7 @@ module AccessMatchers if user.kind_of?(User) # User#inspect displays too much information for RSpec's description # messages - "be #{type} for supplied User" + "be #{type} for the specified user" else "be #{type} for #{user}" end |