From b7cc9bb53125f05af6a119cc5b5799a6c1146add Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Wed, 30 Sep 2015 14:55:07 +0300 Subject: [PATCH] Abiliy to disable 'Share with Group' feature --- CHANGELOG-EE | 1 + app/controllers/groups_controller.rb | 2 +- app/models/project.rb | 4 +++ app/models/project_team.rb | 4 +-- app/views/groups/edit.html.haml | 11 ++++++- .../layouts/nav/_project_settings.html.haml | 11 ++++--- .../projects/project_members/index.html.haml | 2 +- .../20150930110012_add_group_share_lock.rb | 5 +++ db/schema.rb | 11 ++++--- doc/api/groups.md | 18 +++++++++++ lib/api/groups.rb | 32 +++++++++++++++++-- lib/api/projects.rb | 4 +++ spec/models/project_spec.rb | 13 ++++++++ spec/models/project_team_spec.rb | 25 +++++++++++++++ spec/requests/api/groups_spec.rb | 18 +++++++++++ spec/requests/api/projects_spec.rb | 6 ++++ 16 files changed, 149 insertions(+), 18 deletions(-) create mode 100644 db/migrate/20150930110012_add_group_share_lock.rb diff --git a/CHANGELOG-EE b/CHANGELOG-EE index 601e045883..7fa25fcc17 100644 --- a/CHANGELOG-EE +++ b/CHANGELOG-EE @@ -1,5 +1,6 @@ v 8.1.0 - Add documentation for "Share project with group" API call + - Abiliy to disable 'Share with Group' feature (via UI and API) v 8.0.1 - Correct gem dependency versions diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 9eb160f619..d0b0b7cf01 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -135,7 +135,7 @@ class GroupsController < Groups::ApplicationController end def group_params - params.require(:group).permit(:name, :description, :path, :avatar, :membership_lock) + params.require(:group).permit(:name, :description, :path, :avatar, :membership_lock, :share_with_group_lock) end def load_events diff --git a/app/models/project.rb b/app/models/project.rb index 88938f0bf6..d73454bdb3 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -777,4 +777,8 @@ class Project < ActiveRecord::Base approvers.find_or_create_by(user_id: user_id, target_id: id) end end + + def allowed_to_share_with_group? + !namespace.share_with_group_lock + end end diff --git a/app/models/project_team.rb b/app/models/project_team.rb index 761b240109..829edf2827 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -145,7 +145,7 @@ class ProjectTeam access << group.group_members.find_by(user_id: user_id).try(:access_field) end - if project.invited_groups.any? + if project.invited_groups.any? && project.allowed_to_share_with_group? access << max_invited_level(user_id) end @@ -175,7 +175,7 @@ class ProjectTeam group_members = group ? group.group_members : [] invited_members = [] - if project.invited_groups.any? + if project.invited_groups.any? && project.allowed_to_share_with_group? project.project_group_links.each do |group_link| invited_group = group_link.group im = invited_group.group_members diff --git a/app/views/groups/edit.html.haml b/app/views/groups/edit.html.haml index 5fdce36bd1..9c95515a31 100644 --- a/app/views/groups/edit.html.haml +++ b/app/views/groups/edit.html.haml @@ -27,13 +27,22 @@ .form-group %hr - = f.label :name, class: 'control-label' do + = f.label :membership_lock, class: 'control-label' do Member lock .col-sm-10 .checkbox = f.check_box :membership_lock %span.descr Prevent adding new members to project membership within this group + .form-group + %hr + = f.label :share_with_group_lock, class: 'control-label' do + Share with group lock + .col-sm-10 + .checkbox + = f.check_box :share_with_group_lock + %span.descr Prevent sharing a project with another group within this group + .form-actions = f.submit 'Save group', class: "btn btn-save" diff --git a/app/views/layouts/nav/_project_settings.html.haml b/app/views/layouts/nav/_project_settings.html.haml index de05c1515b..98cb0c7824 100644 --- a/app/views/layouts/nav/_project_settings.html.haml +++ b/app/views/layouts/nav/_project_settings.html.haml @@ -13,11 +13,12 @@ = icon('pencil-square-o fw') %span Project Settings - = nav_link(controller: :group_links) do - = link_to namespace_project_group_links_path(@project.namespace, @project) do - = icon('share-square-o fw') - %span - Groups + - if @project.allowed_to_share_with_group? + = nav_link(controller: :group_links) do + = link_to namespace_project_group_links_path(@project.namespace, @project) do + = icon('share-square-o fw') + %span + Groups = nav_link(controller: :deploy_keys) do = link_to namespace_project_deploy_keys_path(@project.namespace, @project), title: 'Deploy Keys', data: {placement: 'right'} do = icon('key fw') diff --git a/app/views/projects/project_members/index.html.haml b/app/views/projects/project_members/index.html.haml index d08cc2bdd2..41b2e17293 100644 --- a/app/views/projects/project_members/index.html.haml +++ b/app/views/projects/project_members/index.html.haml @@ -32,7 +32,7 @@ - if @group = render "group_members", members: @group_members -- if @project_group_links.any? +- if @project_group_links.any? && @project.allowed_to_share_with_group? = render "shared_group_members" :coffeescript diff --git a/db/migrate/20150930110012_add_group_share_lock.rb b/db/migrate/20150930110012_add_group_share_lock.rb new file mode 100644 index 0000000000..78d1a4538f --- /dev/null +++ b/db/migrate/20150930110012_add_group_share_lock.rb @@ -0,0 +1,5 @@ +class AddGroupShareLock < ActiveRecord::Migration + def change + add_column :namespaces, :share_with_group_lock, :boolean, default: false + end +end diff --git a/db/schema.rb b/db/schema.rb index e43ceca5ea..b6681da0ea 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150920161119) do +ActiveRecord::Schema.define(version: 20150930110012) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -549,15 +549,16 @@ ActiveRecord::Schema.define(version: 20150920161119) do add_index "milestones", ["project_id"], name: "index_milestones_on_project_id", using: :btree create_table "namespaces", force: true do |t| - t.string "name", null: false - t.string "path", null: false + t.string "name", null: false + t.string "path", null: false t.integer "owner_id" t.datetime "created_at" t.datetime "updated_at" t.string "type" - t.string "description", default: "", null: false + t.string "description", default: "", null: false t.string "avatar" - t.boolean "membership_lock", default: false + t.boolean "membership_lock", default: false + t.boolean "share_with_group_lock", default: 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 bd7e5813d0..984c46fce2 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -46,6 +46,24 @@ Parameters: - `name` (required) - The name of the group - `path` (required) - The path of the group - `description` (optional) - The group's description +- `membership_lock` (optional, boolean) - Prevent adding new members to project membership within this group +- `share_with_group_lock` (optional, boolean) - Prevent sharing a project with another group within this group + +## Update group + +Updates a project group. Available only for users who can manage this group. + +``` +PUT /groups/:id +``` + +Parameters: + +- `name` (required) - The name of the group +- `path` (required) - The path of the group +- `description` (optional) - The group's description +- `membership_lock` (optional, boolean) - Prevent adding new members to project membership within this group +- `share_with_group_lock` (optional, boolean) - Prevent sharing a project with another group within this group ## Transfer project to group diff --git a/lib/api/groups.rb b/lib/api/groups.rb index 236d5d3bb4..9465afd3ee 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -23,15 +23,18 @@ module API # Create group. Available only for users who can create groups. # # Parameters: - # name (required) - The name of the group - # path (required) - The path of the group + # name (required) - The name of the group + # path (required) - The path of the group + # description (optional) - The details of the group + # membership_lock (optional, boolean) - Prevent adding new members to project membership within this group + # share_with_group_lock (optional, boolean) - Prevent sharing a project with another group within this group # Example Request: # POST /groups post do authorize! :create_group, current_user required_attributes! [:name, :path] - attrs = attributes_for_keys [:name, :path, :description] + attrs = attributes_for_keys [:name, :path, :description, :membership_lock, :share_with_group_lock] @group = Group.new(attrs) if @group.save @@ -51,6 +54,29 @@ module API end end + # Update group. Available only for users who can manage this group. + # + # Parameters: + # id (required) - The ID of a group + # name (required) - The name of the group + # path (required) - The path of the group + # description (optional) - The details of the group + # membership_lock (optional, boolean) - Prevent adding new members to project membership within this group + # share_with_group_lock (optional, boolean) - Prevent sharing a project with another group within this group + # Example Request: + # PUT /groups/:id + put ":id" do + attrs = attributes_for_keys [:name, :path, :description, :membership_lock, :share_with_group_lock] + @group = find_group(params[:id]) + authorize! :admin_group, @group + + if @group.update_attributes(attrs) + present @group, with: Entities::Group + else + render_api_error!("Failed to update group #{@group.errors.messages}", 400) + end + end + # Get a single group, with containing projects # # Parameters: diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 56a5f3c94f..67999e407b 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -265,6 +265,10 @@ module API authorize! :admin_project, user_project required_attributes! [:group_id, :group_access] + unless user_project.allowed_to_share_with_group? + return render_api_error!("The project sharing with group is disabled", 400) + end + link = user_project.project_group_links.new link.group_id = params[:group_id] link.group_access = params[:group_access] diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index ca5cacabb4..e68cd93c0b 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -418,4 +418,17 @@ describe Project do it { should eq "http://localhost#{avatar_path}" } end end + + describe :allowed_to_share_with_group? do + let(:project) { create(:project) } + + it "returns true" do + expect(project.allowed_to_share_with_group?).to be_truthy + end + + it "returns false" do + project.namespace.update(share_with_group_lock: true) + expect(project.allowed_to_share_with_group?).to be_falsey + end + end end diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index db86f6e1f0..d9b472c21d 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -85,4 +85,29 @@ describe ProjectTeam do it { expect(project.team.max_invited_level(reporter.id)).to eq(Gitlab::Access::REPORTER) } it { expect(project.team.max_invited_level(nonmember.id)).to be_nil } end + + describe :max_member_access do + let(:group) { create(:group) } + let(:project) { create(:empty_project) } + + before do + project.project_group_links.create( + group: group, + group_access: Gitlab::Access::DEVELOPER + ) + + group.add_user(master, Gitlab::Access::MASTER) + group.add_user(reporter, Gitlab::Access::REPORTER) + end + + it { expect(project.team.max_member_access(master.id)).to eq(Gitlab::Access::DEVELOPER) } + it { expect(project.team.max_member_access(reporter.id)).to eq(Gitlab::Access::REPORTER) } + it { expect(project.team.max_member_access(nonmember.id)).to be_nil } + + it "does not have an access" do + project.namespace.update(share_with_group_lock: true) + expect(project.team.max_member_access(master.id)).to be_nil + expect(project.team.max_member_access(reporter.id)).to be_nil + end + end end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 34bffb5434..7f47077382 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -142,6 +142,24 @@ describe API::API, api: true do end end + describe "PUT /groups" do + context "when authenticated as user without group permissions" do + it "should not create group" do + put api("/groups/#{group2.id}", user1), attributes_for(:group) + expect(response.status).to eq(403) + end + end + + context "when authenticated as user with group permissions" do + it "should update group" do + group2.update(owner: user2) + put api("/groups/#{group2.id}", user2), { name: 'Renamed' } + expect(response.status).to eq(200) + expect(group2.reload.name).to eq('Renamed') + end + end + end + describe "DELETE /groups/:id" do context "when authenticated as user" do it "should remove group" do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 68e5592957..9915560d80 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -655,6 +655,12 @@ describe API::API, api: true do expect(response.status).to eq 400 end + it "should return a 400 error when sharing is disabled" do + project.namespace.update(share_with_group_lock: true) + post api("/projects/#{project.id}/share", user), group_id: group.id, group_access: Gitlab::Access::DEVELOPER + expect(response.status).to eq 400 + end + it "should return a 409 error when wrong params passed" do post api("/projects/#{project.id}/share", user), group_id: group.id, group_access: 1234 expect(response.status).to eq 409