Commit c3e70280 authored by Felipe Artur's avatar Felipe Artur

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
parent bd59e59d
...@@ -383,3 +383,35 @@ table { ...@@ -383,3 +383,35 @@ table {
margin-right: -$gl-padding; margin-right: -$gl-padding;
border-top: 1px solid $border-color; 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;
}
}
...@@ -61,6 +61,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController ...@@ -61,6 +61,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
:session_expire_delay, :session_expire_delay,
:default_project_visibility, :default_project_visibility,
:default_snippet_visibility, :default_snippet_visibility,
:default_group_visibility,
:restricted_signup_domains_raw, :restricted_signup_domains_raw,
:version_check_enabled, :version_check_enabled,
:admin_notification_email, :admin_notification_email,
......
...@@ -79,7 +79,7 @@ class GroupsController < Groups::ApplicationController ...@@ -79,7 +79,7 @@ class GroupsController < Groups::ApplicationController
end end
def update 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." redirect_to edit_group_path(@group), notice: "Group '#{@group.name}' was successfully updated."
else else
render action: "edit" render action: "edit"
......
...@@ -14,7 +14,7 @@ class NamespacesController < ApplicationController ...@@ -14,7 +14,7 @@ class NamespacesController < ApplicationController
if user if user
redirect_to user_path(user) redirect_to user_path(user)
elsif group elsif group && can?(current_user, :read_group, namespace)
redirect_to group_path(group) redirect_to group_path(group)
elsif current_user.nil? elsif current_user.nil?
authenticate_user! authenticate_user!
......
...@@ -3,16 +3,13 @@ class UsersController < ApplicationController ...@@ -3,16 +3,13 @@ class UsersController < ApplicationController
before_action :set_user before_action :set_user
def show def show
<<<<<<< HEAD
=======
@contributed_projects = contributed_projects.joined(@user).reject(&:forked?) @contributed_projects = contributed_projects.joined(@user).reject(&:forked?)
@projects = PersonalProjectsFinder.new(@user).execute(current_user) @projects = PersonalProjectsFinder.new(@user).execute(current_user)
@projects = @projects.page(params[:page]).per(PER_PAGE) @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| respond_to do |format|
format.html format.html
......
#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
...@@ -80,6 +80,10 @@ module VisibilityLevelHelper ...@@ -80,6 +80,10 @@ module VisibilityLevelHelper
current_application_settings.default_snippet_visibility current_application_settings.default_snippet_visibility
end end
def default_group_visibility
current_application_settings.default_group_visibility
end
def skip_level?(form_model, level) def skip_level?(form_model, level)
form_model.is_a?(Project) && form_model.is_a?(Project) &&
!form_model.visibility_level_allowed?(level) !form_model.visibility_level_allowed?(level)
......
...@@ -296,8 +296,7 @@ class Ability ...@@ -296,8 +296,7 @@ class Ability
def can_read_group?(user, group) def can_read_group?(user, group)
is_project_member = ProjectsFinder.new.execute(user, group: group).any? is_project_member = ProjectsFinder.new.execute(user, group: group).any?
internal_group_allowed = group.internal? && user.present? user.admin? || group.public? || group.internal? || group.users.include?(user)
user.admin? || group.users.include?(user) || is_project_member || group.public? || internal_group_allowed
end end
def namespace_abilities(user, namespace) def namespace_abilities(user, namespace)
......
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
# max_attachment_size :integer default(10), not null # max_attachment_size :integer default(10), not null
# default_project_visibility :integer # default_project_visibility :integer
# default_snippet_visibility :integer # default_snippet_visibility :integer
# default_group_visibility :integer
# restricted_signup_domains :text # restricted_signup_domains :text
# user_oauth_applications :boolean default(TRUE) # user_oauth_applications :boolean default(TRUE)
# after_sign_out_path :string(255) # after_sign_out_path :string(255)
......
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
...@@ -21,7 +21,6 @@ class Group < Namespace ...@@ -21,7 +21,6 @@ class Group < Namespace
include Gitlab::ConfigHelper include Gitlab::ConfigHelper
include Gitlab::VisibilityLevel include Gitlab::VisibilityLevel
include Referable include Referable
include SharedScopes
has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember' has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember'
alias_method :members, :group_members alias_method :members, :group_members
......
...@@ -52,7 +52,6 @@ class Project < ActiveRecord::Base ...@@ -52,7 +52,6 @@ class Project < ActiveRecord::Base
include AfterCommitQueue include AfterCommitQueue
include CaseSensitivity include CaseSensitivity
include TokenAuthenticatable include TokenAuthenticatable
include SharedScopes
extend Gitlab::ConfigHelper extend Gitlab::ConfigHelper
...@@ -934,8 +933,10 @@ class Project < ActiveRecord::Base ...@@ -934,8 +933,10 @@ class Project < ActiveRecord::Base
end end
def visibility_level_allowed?(level) def visibility_level_allowed?(level)
return true unless forked? allowed_by_forks = forked? ? Gitlab::VisibilityLevel.allowed_fork_levels(forked_from_project.visibility_level).include?(level.to_i) : true
Gitlab::VisibilityLevel.allowed_fork_levels(forked_from_project.visibility_level).include?(level.to_i) allowed_by_groups = group.present? ? level.to_i <= group.visibility_level : true
allowed_by_forks && allowed_by_groups
end end
def runners_token def runners_token
......
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
#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
...@@ -11,8 +11,8 @@ module Projects ...@@ -11,8 +11,8 @@ module Projects
# Make sure that the user is allowed to use the specified visibility # Make sure that the user is allowed to use the specified visibility
# level # 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) deny_visibility_level(@project)
return @project return @project
end end
......
...@@ -19,6 +19,10 @@ ...@@ -19,6 +19,10 @@
= f.label :default_snippet_visibility, class: 'control-label col-sm-2' = f.label :default_snippet_visibility, class: 'control-label col-sm-2'
.col-sm-10 .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) = 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 .form-group
= f.label :restricted_visibility_levels, class: 'control-label col-sm-2' = f.label :restricted_visibility_levels, class: 'control-label col-sm-2'
.col-sm-10 .col-sm-10
......
...@@ -17,7 +17,7 @@ ...@@ -17,7 +17,7 @@
.col-sm-10 .col-sm-10
= render 'shared/choose_group_avatar_button', f: f = 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 .form-group
.col-sm-offset-2.col-sm-10 .col-sm-offset-2.col-sm-10
......
- @no_container = true - @no_container = true
- unless can?(current_user, :read_group, @group)
- @disable_search_panel = true
= content_for :meta_tags do = content_for :meta_tags do
- if current_user - if current_user
= auto_discovery_link_tag(:atom, group_url(@group, format: :atom, private_token: current_user.private_token), title: "#{@group.name} activity") = auto_discovery_link_tag(:atom, group_url(@group, format: :atom, private_token: current_user.private_token), title: "#{@group.name} activity")
...@@ -17,9 +14,13 @@ ...@@ -17,9 +14,13 @@
.avatar-holder .avatar-holder
= link_to group_icon(@group), target: '_blank' do = link_to group_icon(@group), target: '_blank' do
= image_tag group_icon(@group), class: "avatar group-avatar s90" = image_tag group_icon(@group), class: "avatar group-avatar s90"
.cover-title .group-projects-show-title
%h1
= @group.name = @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 .cover-desc.username
@#{@group.path} @#{@group.path}
...@@ -27,7 +28,6 @@ ...@@ -27,7 +28,6 @@
.cover-desc.description .cover-desc.description
= markdown(@group.description, pipeline: :description) = markdown(@group.description, pipeline: :description)
%ul.nav-links %ul.nav-links
%li.active %li.active
= link_to "#activity", 'data-toggle' => 'tab' do = link_to "#activity", 'data-toggle' => 'tab' do
......
...@@ -7,7 +7,6 @@ ...@@ -7,7 +7,6 @@
.navbar-collapse.collapse .navbar-collapse.collapse
%ul.nav.navbar-nav.pull-right %ul.nav.navbar-nav.pull-right
- unless @disable_search_panel
%li.hidden-sm.hidden-xs %li.hidden-sm.hidden-xs
= render 'layouts/search' = render 'layouts/search'
%li.visible-sm.visible-xs %li.visible-sm.visible-xs
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
.project-home-panel.cover-block.clearfix{:class => ("empty-project" if empty_repo)} .project-home-panel.cover-block.clearfix{:class => ("empty-project" if empty_repo)}
.project-identicon-holder .project-identicon-holder
= project_icon(@project, alt: '', class: 'project-avatar avatar s90') = project_icon(@project, alt: '', class: 'project-avatar avatar s90')
.project-home-desc .group-projects-show-title
%h1 %h1
= @project.name = @project.name
%span.visibility-icon.has_tooltip{data: { container: 'body' }, %span.visibility-icon.has_tooltip{data: { container: 'body' },
......
...@@ -90,6 +90,10 @@ production: &base ...@@ -90,6 +90,10 @@ production: &base
snippets: false snippets: false
builds: true builds: true
## Default group features settings
default_groups_features:
visibility_level: 20
## Webhook settings ## Webhook settings
# Number of seconds to wait for HTTP response after sending webhook HTTP POST request (default: 10) # Number of seconds to wait for HTTP response after sending webhook HTTP POST request (default: 10)
# webhook_timeout: 10 # webhook_timeout: 10
......
class AddDefaultGroupVisibilityToApplicationSettings < ActiveRecord::Migration
def up
add_column :application_settings, :default_group_visibility, :integer
visibility = Settings.gitlab.default_groups_features['visibility_level']
execute("update application_settings set default_group_visibility = #{visibility}")
end
def down
remove_column :application_settings, :default_group_visibility
end
end
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20160301124843) do ActiveRecord::Schema.define(version: 20160308212903) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
...@@ -75,6 +75,7 @@ ActiveRecord::Schema.define(version: 20160301124843) do ...@@ -75,6 +75,7 @@ ActiveRecord::Schema.define(version: 20160301124843) do
t.boolean "akismet_enabled", default: false t.boolean "akismet_enabled", default: false
t.string "akismet_api_key" t.string "akismet_api_key"
t.boolean "email_author_in_body", default: false t.boolean "email_author_in_body", default: false
t.integer "default_group_visibility"
end end
create_table "audit_events", force: :cascade do |t| create_table "audit_events", force: :cascade do |t|
......
...@@ -332,6 +332,7 @@ module API ...@@ -332,6 +332,7 @@ module API
expose :session_expire_delay expose :session_expire_delay
expose :default_project_visibility expose :default_project_visibility
expose :default_snippet_visibility expose :default_snippet_visibility
expose :default_group_visibility
expose :restricted_signup_domains expose :restricted_signup_domains
expose :user_oauth_applications expose :user_oauth_applications
expose :after_sign_out_path expose :after_sign_out_path
......
...@@ -29,6 +29,7 @@ module Gitlab ...@@ -29,6 +29,7 @@ module Gitlab
session_expire_delay: Settings.gitlab['session_expire_delay'], session_expire_delay: Settings.gitlab['session_expire_delay'],
default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'], default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'],
default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'], default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'],
default_group_visibility: Settings.gitlab.default_groups_features['visibility_level'],
restricted_signup_domains: Settings.gitlab['restricted_signup_domains'], restricted_signup_domains: Settings.gitlab['restricted_signup_domains'],
import_sources: ['github','bitbucket','gitlab','gitorious','google_code','fogbugz','git'], import_sources: ['github','bitbucket','gitlab','gitorious','google_code','fogbugz','git'],
shared_runners_enabled: Settings.gitlab_ci['shared_runners_enabled'], shared_runners_enabled: Settings.gitlab_ci['shared_runners_enabled'],
......
...@@ -12,6 +12,13 @@ module Gitlab ...@@ -12,6 +12,13 @@ module Gitlab
PUBLIC = 20 unless const_defined?(:PUBLIC) PUBLIC = 20 unless const_defined?(:PUBLIC)
class << self class << self
def included(base)
base.class_eval do
scope :public_only, -> { where(visibility_level: PUBLIC) }
scope :public_and_internal_only, -> { where(visibility_level: [PUBLIC, INTERNAL] ) }
end
end
def values def values
options.values options.values
end end
......
...@@ -54,6 +54,7 @@ describe GroupsController do ...@@ -54,6 +54,7 @@ describe GroupsController do
let(:group) { create(:group, visibility_level: 20) } let(:group) { create(:group, visibility_level: 20) }
it 'checks if group can be updated' do it 'checks if group can be updated' do
expect_any_instance_of(Groups::UpdateService).to receive(:execute)
expect(controller).to receive(:authorize_admin_group!) expect(controller).to receive(:authorize_admin_group!)
put :update, id: group.path, group: { name: 'test' } put :update, id: group.path, group: { name: 'test' }
end end
......
require 'spec_helper'
describe JoinedGroupsFinder do
describe '#execute' do
let!(:profile_owner) { create(:user) }
let!(:profile_visitor) { create(:user) }
let!(:private_group) { create(:group, visibility_level: Gitlab::VisibilityLevel::PRIVATE) }
let!(:private_group_2) { create(:group, visibility_level: Gitlab::VisibilityLevel::PRIVATE) }
let!(:internal_group) { create(:group, visibility_level: Gitlab::VisibilityLevel::INTERNAL) }
let!(:internal_group_2) { create(:group, visibility_level: Gitlab::VisibilityLevel::INTERNAL) }
let!(:public_group) { create(:group, visibility_level: Gitlab::VisibilityLevel::PUBLIC) }
let!(:public_group_2) { create(:group, visibility_level: Gitlab::VisibilityLevel::PUBLIC) }
let!(:finder) { described_class.new(profile_owner) }
describe 'execute' do
context 'without a user only shows public groups from profile owner' do
before { public_group.add_user(profile_owner, Gitlab::Access::MASTER)}
subject { finder.execute }
it { is_expected.to eq([public_group]) }
end
context 'only shows groups where both users are authorized to see' do
subject { finder.execute(profile_visitor) }
before do
private_group.add_user(profile_owner, Gitlab::Access::MASTER)
private_group.add_user(profile_visitor, Gitlab::Access::DEVELOPER)
internal_group.add_user(profile_owner, Gitlab::Access::MASTER)
public_group.add_user(profile_owner, Gitlab::Access::MASTER)
end
it { is_expected.to eq([public_group, internal_group, private_group]) }
end
context 'shows group if profile visitor is in one of its projects' do
before do
public_group.add_user(profile_owner, Gitlab::Access::MASTER)
private_group.add_user(profile_owner, Gitlab::Access::MASTER)
project = create(:project, :private, group: private_group, name: 'B', path: 'B')
project.team.add_user(profile_visitor, Gitlab::Access::DEVELOPER)
end
subject { finder.execute(profile_visitor) }
it { is_expected.to eq([public_group, private_group]) }
end
end
end
end
...@@ -583,6 +583,21 @@ describe Project, models: true do ...@@ -583,6 +583,21 @@ describe Project, models: true do
it { expect(forked_project.visibility_level_allowed?(Gitlab::VisibilityLevel::PUBLIC)).to be_falsey } it { expect(forked_project.visibility_level_allowed?(Gitlab::VisibilityLevel::PUBLIC)).to be_falsey }
end end
context 'when checking projects from groups' do
let(:private_group) { create(:group, visibility_level: 0) }
let(:internal_group) { create(:group, visibility_level: 10) }
let(:private_project) { create :project, group: private_group, visibility_level: Gitlab::VisibilityLevel::PRIVATE }
let(:internal_project) { create :project, group: internal_group, visibility_level: Gitlab::VisibilityLevel::INTERNAL }
context 'when group is private project can not be internal' do
it { expect(private_project.visibility_level_allowed?(Gitlab::VisibilityLevel::INTERNAL)).to be_falsey }
end
context 'when group is internal project can not be public' do
it { expect(internal_project.visibility_level_allowed?(Gitlab::VisibilityLevel::PUBLIC)).to be_falsey }
end
end
end end
describe '#rename_repo' do describe '#rename_repo' do
......
require 'spec_helper'
describe Groups::UpdateService, services: true do
let!(:user) { create(:user) }
let!(:private_group) { create(:group, visibility_level: Gitlab::VisibilityLevel::PRIVATE) }
let!(:internal_group) { create(:group, visibility_level: Gitlab::VisibilityLevel::INTERNAL) }
let!(:public_group) { create(:group, visibility_level: Gitlab::VisibilityLevel::PUBLIC) }
describe "execute" do
context "project visibility_level validation" do
context "public group with public projects" do
let!(:service) { described_class.new(public_group, user, visibility_level: Gitlab::VisibilityLevel::INTERNAL ) }
before do
public_group.add_user(user, Gitlab::Access::MASTER)
create(:project, :public, group: public_group, name: 'B', path: 'B')
end
it "cant downgrade permission level" do
expect(service.execute).to be_falsy
expect(public_group.errors.count).to eq(1)
end
end
context "internal group with internal project" do
let!(:service) { described_class.new(internal_group, user, visibility_level: Gitlab::VisibilityLevel::PRIVATE ) }
before do
internal_group.add_user(user, Gitlab::Access::MASTER)
create(:project, :internal, group: internal_group, name: 'B', path: 'B')
end
it "cant downgrade permission level" do
expect(service.execute).to be_falsy
expect(internal_group.errors.count).to eq(1)
end
end
end
end
context "unauthorized visibility_level validation" do
let!(:service) { described_class.new(internal_group, user, visibility_level: 99 ) }
before { internal_group.add_user(user, Gitlab::Access::MASTER) }
it "does not change permission level" do
expect(service.execute).to be_falsy
expect(internal_group.errors.count).to eq(1)
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment