diff options
author | Felipe Artur <felipefac@gmail.com> | 2016-03-09 03:01:33 +0300 |
---|---|---|
committer | Felipe Artur <felipefac@gmail.com> | 2016-03-10 16:38:36 +0300 |
commit | c3e70280dffe7ee0859ebd73b902d424ca5f809a (patch) | |
tree | 06b83a5ab13d19803332253cf50a941501b29317 /app | |
parent | bd59e59d01c5e845c7f7d451feaa1488670f20de (diff) |
Prevent projects to have higher visibility than groups
Prevent Groups to have smaller visibility than projects
Add default_group_visibility_level to configuration
Code improvements
Diffstat (limited to 'app')
-rw-r--r-- | app/assets/stylesheets/framework/common.scss | 32 | ||||
-rw-r--r-- | app/controllers/admin/application_settings_controller.rb | 1 | ||||
-rw-r--r-- | app/controllers/groups_controller.rb | 2 | ||||
-rw-r--r-- | app/controllers/namespaces_controller.rb | 2 | ||||
-rw-r--r-- | app/controllers/users_controller.rb | 5 | ||||
-rw-r--r-- | app/finders/joined_groups_finder.rb | 45 | ||||
-rw-r--r-- | app/helpers/visibility_level_helper.rb | 4 | ||||
-rw-r--r-- | app/models/ability.rb | 3 | ||||
-rw-r--r-- | app/models/application_setting.rb | 1 | ||||
-rw-r--r-- | app/models/concerns/shared_scopes.rb | 8 | ||||
-rw-r--r-- | app/models/group.rb | 1 | ||||
-rw-r--r-- | app/models/project.rb | 7 | ||||
-rw-r--r-- | app/services/groups/base_service.rb | 9 | ||||
-rw-r--r-- | app/services/groups/update_service.rb | 44 | ||||
-rw-r--r-- | app/services/projects/create_service.rb | 4 | ||||
-rw-r--r-- | app/views/admin/application_settings/_form.html.haml | 4 | ||||
-rw-r--r-- | app/views/groups/new.html.haml | 2 | ||||
-rw-r--r-- | app/views/groups/show.html.haml | 12 | ||||
-rw-r--r-- | app/views/layouts/header/_default.html.haml | 5 | ||||
-rw-r--r-- | app/views/projects/_home_panel.html.haml | 2 |
20 files changed, 160 insertions, 33 deletions
diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index c98e43ad09f..18044b25b87 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -383,3 +383,35 @@ table { margin-right: -$gl-padding; border-top: 1px solid $border-color; } +.message { + border: 1px solid #ccc; + padding: 10px; + color: #333; +} +.message { + border: 1px solid #ccc; + padding: 10px; + color: #333; +} + +.group-projects-show-title{ + h1 { + color: #313236; + margin: 0; + margin-bottom: 6px; + font-size: 23px; + font-weight: normal; + } + + .visibility-icon { + display: inline-block; + margin-left: 5px; + font-size: 18px; + color: $gray; + } + + p { + padding: 0 $gl-padding; + color: #5c5d5e; + } + } diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 04a99d8c84a..ed9f6031389 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -61,6 +61,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :session_expire_delay, :default_project_visibility, :default_snippet_visibility, + :default_group_visibility, :restricted_signup_domains_raw, :version_check_enabled, :admin_notification_email, diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 6532eee1602..54f14e62ead 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -79,7 +79,7 @@ class GroupsController < Groups::ApplicationController end def update - if @group.update_attributes(group_params) + if Groups::UpdateService.new(@group, current_user, group_params).execute redirect_to edit_group_path(@group), notice: "Group '#{@group.name}' was successfully updated." else render action: "edit" diff --git a/app/controllers/namespaces_controller.rb b/app/controllers/namespaces_controller.rb index 282012c60a1..5a94dcb0dbd 100644 --- a/app/controllers/namespaces_controller.rb +++ b/app/controllers/namespaces_controller.rb @@ -14,7 +14,7 @@ class NamespacesController < ApplicationController if user redirect_to user_path(user) - elsif group + elsif group && can?(current_user, :read_group, namespace) redirect_to group_path(group) elsif current_user.nil? authenticate_user! diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index d26a1ce6737..7b32572f822 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -3,16 +3,13 @@ class UsersController < ApplicationController before_action :set_user def show -<<<<<<< HEAD -======= @contributed_projects = contributed_projects.joined(@user).reject(&:forked?) @projects = PersonalProjectsFinder.new(@user).execute(current_user) @projects = @projects.page(params[:page]).per(PER_PAGE) - @groups = @user.groups.order_id_desc + @groups = JoinedGroupsFinder.new(@user).execute(current_user) ->>>>>>> Code improvements respond_to do |format| format.html diff --git a/app/finders/joined_groups_finder.rb b/app/finders/joined_groups_finder.rb new file mode 100644 index 00000000000..131b518563e --- /dev/null +++ b/app/finders/joined_groups_finder.rb @@ -0,0 +1,45 @@ +#Shows only authorized groups of a user +class JoinedGroupsFinder + def initialize(user = nil) + @user = user + end + + # Finds the groups of the source user, optionally limited to those visible to + # the current user. + # + # current_user - If given the groups of "@user" will only include the groups + # "current_user" can also see. + # + # Returns an ActiveRecord::Relation. + def execute(current_user = nil) + if current_user + relation = groups_visible_to_user(current_user) + else + relation = public_groups + end + + relation.order_id_desc + end + + private + + # Returns the groups the user in "current_user" can see. + # + # This list includes all public/internal projects as well as the projects of + # "@user" that "current_user" also has access to. + def groups_visible_to_user(current_user) + base = @user.authorized_groups.visible_to_user(current_user) + extra = public_and_internal_groups + union = Gitlab::SQL::Union.new([base.select(:id), extra.select(:id)]) + + Group.where("namespaces.id IN (#{union.to_sql})") + end + + def public_groups + @user.authorized_groups.public_only + end + + def public_and_internal_groups + @user.authorized_groups.public_and_internal_only + end +end diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb index c47342534a8..930cc883634 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -80,6 +80,10 @@ module VisibilityLevelHelper current_application_settings.default_snippet_visibility end + def default_group_visibility + current_application_settings.default_group_visibility + end + def skip_level?(form_model, level) form_model.is_a?(Project) && !form_model.visibility_level_allowed?(level) diff --git a/app/models/ability.rb b/app/models/ability.rb index ec5587d8fa5..1c9b15069aa 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -296,8 +296,7 @@ class Ability def can_read_group?(user, group) is_project_member = ProjectsFinder.new.execute(user, group: group).any? - internal_group_allowed = group.internal? && user.present? - user.admin? || group.users.include?(user) || is_project_member || group.public? || internal_group_allowed + user.admin? || group.public? || group.internal? || group.users.include?(user) end def namespace_abilities(user, namespace) diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 269056e0e77..c4879598c4e 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -18,6 +18,7 @@ # max_attachment_size :integer default(10), not null # default_project_visibility :integer # default_snippet_visibility :integer +# default_group_visibility :integer # restricted_signup_domains :text # user_oauth_applications :boolean default(TRUE) # after_sign_out_path :string(255) diff --git a/app/models/concerns/shared_scopes.rb b/app/models/concerns/shared_scopes.rb deleted file mode 100644 index f576d2c0821..00000000000 --- a/app/models/concerns/shared_scopes.rb +++ /dev/null @@ -1,8 +0,0 @@ -module SharedScopes - extend ActiveSupport::Concern - - included do - scope :public_only, -> { where(visibility_level: Group::PUBLIC) } - scope :public_and_internal_only, -> { where(visibility_level: [Group::PUBLIC, Group::INTERNAL] ) } - end -end diff --git a/app/models/group.rb b/app/models/group.rb index d1a1817f0fa..02b9a968dcd 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -21,7 +21,6 @@ class Group < Namespace include Gitlab::ConfigHelper include Gitlab::VisibilityLevel include Referable - include SharedScopes has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember' alias_method :members, :group_members diff --git a/app/models/project.rb b/app/models/project.rb index 6e86c7cc883..ae5f6e2417d 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -52,7 +52,6 @@ class Project < ActiveRecord::Base include AfterCommitQueue include CaseSensitivity include TokenAuthenticatable - include SharedScopes extend Gitlab::ConfigHelper @@ -934,8 +933,10 @@ class Project < ActiveRecord::Base end def visibility_level_allowed?(level) - return true unless forked? - Gitlab::VisibilityLevel.allowed_fork_levels(forked_from_project.visibility_level).include?(level.to_i) + allowed_by_forks = forked? ? Gitlab::VisibilityLevel.allowed_fork_levels(forked_from_project.visibility_level).include?(level.to_i) : true + allowed_by_groups = group.present? ? level.to_i <= group.visibility_level : true + + allowed_by_forks && allowed_by_groups end def runners_token diff --git a/app/services/groups/base_service.rb b/app/services/groups/base_service.rb new file mode 100644 index 00000000000..5becd475d3a --- /dev/null +++ b/app/services/groups/base_service.rb @@ -0,0 +1,9 @@ +module Groups + class BaseService + attr_accessor :group, :current_user, :params + + def initialize(group, user, params = {}) + @group, @current_user, @params = group, user, params.dup + end + end +end diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb new file mode 100644 index 00000000000..acb6c529c17 --- /dev/null +++ b/app/services/groups/update_service.rb @@ -0,0 +1,44 @@ +#Checks visibility level permission check before updating a group +#Do not allow to put Group visibility level smaller than its projects +#Do not allow unauthorized permission levels + +module Groups + class UpdateService < Groups::BaseService + def execute + visibility_level_allowed?(params[:visibility_level]) ? group.update_attributes(params) : false + end + + private + + def visibility_level_allowed?(level) + return true unless level.present? + + allowed_by_projects = visibility_by_project(level) + allowed_by_user = visibility_by_user(level) + + allowed_by_projects && allowed_by_user + end + + def visibility_by_project(level) + projects_visibility = group.projects.pluck(:visibility_level) + + allowed_by_projects = !projects_visibility.any?{|project_visibility| level.to_i < project_visibility } + add_error_message("Cannot be changed. There are projects with higher visibility permissions.") unless allowed_by_projects + allowed_by_projects + end + + def visibility_by_user(level) + allowed_by_user = Gitlab::VisibilityLevel.allowed_for?(current_user, level) + add_error_message("You are not authorized to set this permission level.") unless allowed_by_user + allowed_by_user + end + + def add_error_message(message) + level_name = Gitlab::VisibilityLevel.level_name(params[:visibility_level]) + group.errors.add(:visibility_level, message) + end + end +end + + + diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index a6820183bee..522fae79503 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -11,8 +11,8 @@ module Projects # Make sure that the user is allowed to use the specified visibility # level - unless Gitlab::VisibilityLevel.allowed_for?(current_user, - params[:visibility_level]) + + unless Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level]) && @project.visibility_level_allowed?(@project.visibility_level) deny_visibility_level(@project) return @project end diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index b30dfd109ea..0350995d03d 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -19,6 +19,10 @@ = f.label :default_snippet_visibility, class: 'control-label col-sm-2' .col-sm-10 = render('shared/visibility_radios', model_method: :default_snippet_visibility, form: f, selected_level: @application_setting.default_snippet_visibility, form_model: ProjectSnippet.new) + .form-group.group-visibility-level-holder + = f.label :default_group_visibility, class: 'control-label col-sm-2' + .col-sm-10 + = render('shared/visibility_radios', model_method: :default_group_visibility, form: f, selected_level: @application_setting.default_group_visibility, form_model: Group.new) .form-group = f.label :restricted_visibility_levels, class: 'control-label col-sm-2' .col-sm-10 diff --git a/app/views/groups/new.html.haml b/app/views/groups/new.html.haml index 1526ca42634..30ab8aeba13 100644 --- a/app/views/groups/new.html.haml +++ b/app/views/groups/new.html.haml @@ -17,7 +17,7 @@ .col-sm-10 = render 'shared/choose_group_avatar_button', f: f - = render 'shared/visibility_level', f: f, visibility_level: @group.visibility_level, can_change_visibility_level: true, form_model: @group + = render 'shared/visibility_level', f: f, visibility_level: default_group_visibility, can_change_visibility_level: true, form_model: @group .form-group .col-sm-offset-2.col-sm-10 diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml index 6148d8cb3d2..0a8c2da7207 100644 --- a/app/views/groups/show.html.haml +++ b/app/views/groups/show.html.haml @@ -1,8 +1,5 @@ - @no_container = true -- unless can?(current_user, :read_group, @group) - - @disable_search_panel = true - = content_for :meta_tags do - if current_user = auto_discovery_link_tag(:atom, group_url(@group, format: :atom, private_token: current_user.private_token), title: "#{@group.name} activity") @@ -17,8 +14,12 @@ .avatar-holder = link_to group_icon(@group), target: '_blank' do = image_tag group_icon(@group), class: "avatar group-avatar s90" - .cover-title - = @group.name + .group-projects-show-title + %h1 + = @group.name + + %span.visibility-icon.has_tooltip{data: { container: 'body', placement: 'left' }, title: "#{visibility_level_label(@group.visibility_level)} - #{project_visibility_level_description(@group.visibility_level)}"} + = visibility_level_icon(@group.visibility_level, fw: false) .cover-desc.username @#{@group.path} @@ -27,7 +28,6 @@ .cover-desc.description = markdown(@group.description, pipeline: :description) - %ul.nav-links %li.active = link_to "#activity", 'data-toggle' => 'tab' do diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index 77d01a7736c..714da410f56 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -7,9 +7,8 @@ .navbar-collapse.collapse %ul.nav.navbar-nav.pull-right - - unless @disable_search_panel - %li.hidden-sm.hidden-xs - = render 'layouts/search' + %li.hidden-sm.hidden-xs + = render 'layouts/search' %li.visible-sm.visible-xs = link_to search_path, title: 'Search', data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do = icon('search') diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml index b45df44f270..dd16f504c92 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -2,7 +2,7 @@ .project-home-panel.cover-block.clearfix{:class => ("empty-project" if empty_repo)} .project-identicon-holder = project_icon(@project, alt: '', class: 'project-avatar avatar s90') - .project-home-desc + .group-projects-show-title %h1 = @project.name %span.visibility-icon.has_tooltip{data: { container: 'body' }, |