[35508] Add global permission to manage placeholder users (#9000)

* Add global permission to manage (but not delete) placeholders

https://community.openproject.com/work_packages/35508

* Restore breadcrumbs for non-show routes

* Remove permissions to memberships for add_placeholder_user permission

* Allow non-admins with global permission to access membership

* Remove permissions to memberships for add_placeholder_user permission

* Rename shared_examples for admin contract validations

Co-authored-by: ulferts <jens.ulferts@googlemail.com>

app/contracts/placeholder_users/base_contract.rb

@@ -47,9 +47,13 @@ module PlaceholderUsers
     ##
     # Placeholder users can only be updated by Admins
     def user_allowed_to_modify
-      unless user.admin? && user.active?
+      unless user_allowed_to_add?
         errors.add :base, :error_unauthorized
       end
     end
+
+    def user_allowed_to_add?
+      user.allowed_to_globally?(:add_placeholder_user)
+    end
   end
 end

app/controllers/placeholder_users/memberships_controller.rb

@@ -32,7 +32,7 @@ class PlaceholderUsers::MembershipsController < ApplicationController
   include IndividualPrincipals::MembershipControllerMethods
   layout 'admin'
 
-  before_action :require_admin
+  before_action :authorize_global
   before_action :find_individual_principal
 
   def find_individual_principal

app/controllers/placeholder_users_controller.rb

@@ -33,7 +33,8 @@ class PlaceholderUsersController < ApplicationController
 
   helper_method :gon
 
-  before_action :require_admin, except: [:show]
+  before_action :authorize_global, except: %i[show]
+
   before_action :find_placeholder_user, only: %i[show
                                                  edit
                                                  update
@@ -53,7 +54,7 @@ class PlaceholderUsersController < ApplicationController
   def show
     # show projects based on current user visibility
     @memberships = @placeholder_user.memberships
-                        .visible(current_user)
+      .visible(current_user)
 
     respond_to do |format|
       format.html { render layout: 'no_menu' }
@@ -62,11 +63,11 @@ class PlaceholderUsersController < ApplicationController
 
   def new
     @placeholder_user = PlaceholderUsers::SetAttributesService
-                          .new(user: User.current,
-                               model: PlaceholderUser.new,
-                               contract_class: EmptyContract)
-                          .call({})
-                          .result
+      .new(user: User.current,
+           model: PlaceholderUser.new,
+           contract_class: EmptyContract)
+      .call({})
+      .result
   end
 
   def create
@@ -82,6 +83,7 @@ class PlaceholderUsersController < ApplicationController
         end
       end
     else
+      @errors = service_result.errors
       respond_to do |format|
         format.html do
           render action: :new
@@ -97,9 +99,9 @@ class PlaceholderUsersController < ApplicationController
 
   def update
     service_result = PlaceholderUsers::UpdateService
-                       .new(user: User.current,
-                            model: @placeholder_user)
-                       .call(permitted_params.placeholder_user)
+      .new(user: User.current,
+           model: @placeholder_user)
+      .call(permitted_params.placeholder_user)
 
     if service_result.success?
       respond_to do |format|
@@ -157,6 +159,6 @@ class PlaceholderUsersController < ApplicationController
   end
 
   def show_local_breadcrumb
-    current_user.admin?
+    action_name != 'show'
   end
 end

app/controllers/users/memberships_controller.rb

@@ -32,7 +32,7 @@ class Users::MembershipsController < ApplicationController
   include IndividualPrincipals::MembershipControllerMethods
   layout 'admin'
 
-  before_action :require_admin
+  before_action :authorize_global
   before_action :find_individual_principal
 
   def find_individual_principal

app/controllers/users_controller.rb

@@ -322,7 +322,7 @@ class UsersController < ApplicationController
   end
 
   def show_local_breadcrumb
-    current_user.admin?
+    action_name != 'show'
   end
 
   def build_user_update_params

app/views/individual_principals/_memberships.html.erb

@@ -27,11 +27,12 @@ See docs/COPYRIGHT.rdoc for more details.
 
 ++#%>
 <% roles = Role.givable %>
-<% projects = Project.active.order(Arel.sql('lft')) %>
+<% projects = Project.visible.active.order(Arel.sql('lft')) %>
+<% memberships = @individual_principal.memberships.visible(current_user) %>
 
 <div class="grid-block">
   <div class="grid-content">
-  <% if @individual_principal.memberships.any? %>
+  <% if memberships.any? %>
     <div class="generic-table--container">
       <div class="generic-table--results-container">
         <table class="generic-table memberships">
@@ -66,7 +67,7 @@ See docs/COPYRIGHT.rdoc for more details.
             </tr>
           </thead>
           <tbody>
-            <% @individual_principal.memberships.where.not(project: nil).each do |membership| %>
+            <% memberships.where.not(project: nil).each do |membership| %>
               <% next if membership.new_record? %>
                 <tr id="member-<%= membership.id %>" class="member">
                   <td class="project">

app/views/placeholder_users/_form.html.erb

@@ -27,7 +27,7 @@ See docs/COPYRIGHT.rdoc for more details.
 
 ++#%>
 
-<%= error_messages_for 'placeholder_user' %>
+<%= error_messages_for_contract @placeholder_user, @errors %>
 
 <!--[form:placeholder_user]-->
 <section class="form--section">

config/initializers/menus.rb

@@ -133,10 +133,16 @@ Redmine::MenuManager.map :admin_menu do |menu|
 
   menu.push :users,
             { controller: '/users' },
-            if: Proc.new { !User.current.admin? && User.current.allowed_to?(:add_user, nil, global: true) },
+            if: Proc.new { !User.current.admin? && User.current.allowed_to_globally?(:add_user) },
             caption: :label_user_plural,
             icon: 'icon2 icon-group'
 
+  menu.push :placeholder_users,
+            { controller: '/placeholder_users' },
+            if: Proc.new { !User.current.admin? && User.current.allowed_to_globally?(:add_placeholder_user) },
+            caption: :label_placeholder_user_plural,
+            icon: 'icon2 icon-group'
+
   menu.push :users_and_permissions,
             { controller: '/users' },
             if: Proc.new { User.current.admin? },
@@ -157,6 +163,7 @@ Redmine::MenuManager.map :admin_menu do |menu|
 
   menu.push :placeholder_users,
             { controller: '/placeholder_users' },
+            if: Proc.new { User.current.admin? },
             caption: :label_placeholder_user_plural,
             parent: :users_and_permissions
 

config/initializers/permissions.rb

@@ -52,6 +52,16 @@ OpenProject::AccessControl.map do |map|
     map.permission :add_user,
                    {
                      users: %i[index show new create edit update resend_invitation],
+                     "users/memberships": %i[create update destroy],
+                     admin: %i[index]
+                   },
+                   require: :loggedin,
+                   global: true
+
+    map.permission :add_placeholder_user,
+                   {
+                     placeholder_users: %i[index show new create edit update],
+                     "placeholder_users/memberships": %i[create update destroy],
                      admin: %i[index]
                    },
                    require: :loggedin,

config/locales/en.yml

@@ -1975,6 +1975,8 @@ en:
   permission_add_work_packages: "Add work packages"
   permission_add_messages: "Post messages"
   permission_add_project: "Create project"
+  permission_add_user: "Create and edit users"
+  permission_add_placeholder_user: "Create and edit placeholder users"
   permission_add_subprojects: "Create subprojects"
   permission_add_work_package_watchers: "Add watchers"
   permission_assign_versions: "Assign versions"

lib/open_project/ui/extensible_tabs.rb

@@ -68,8 +68,7 @@ module OpenProject
               name: 'memberships',
               partial: 'individual_principals/memberships',
               path: ->(params) { edit_user_path(params[:user], tab: :memberships) },
-              label: :label_project_plural,
-              only_if: ->(*) { User.current.admin? }
+              label: :label_project_plural
             },
             {
               name: 'groups',

spec/contracts/attribute_help_texts/base_contract_spec.rb

@@ -35,5 +35,5 @@ describe AttributeHelpTexts::BaseContract do
   let(:model) { FactoryBot.build_stubbed :work_package_help_text }
   let(:contract) { described_class.new(model, current_user) }
 
-  it_behaves_like 'contract is valid for active admin users only'
+  it_behaves_like 'contract is valid for active admins and invalid for regular users'
 end

spec/contracts/custom_fields/create_contract_spec.rb

@@ -39,5 +39,5 @@ describe CustomFields::CreateContract do
     described_class.new(cf, current_user, options: { changed_by_system: [] })
   end
 
-  it_behaves_like 'contract is valid for active admin users only'
+  it_behaves_like 'contract is valid for active admins and invalid for regular users'
 end

spec/contracts/custom_fields/update_contract_spec.rb

@@ -39,5 +39,5 @@ describe CustomFields::UpdateContract do
     described_class.new(cf, current_user, options: { changed_by_system: [] })
   end
 
-  it_behaves_like 'contract is valid for active admin users only'
+  it_behaves_like 'contract is valid for active admins and invalid for regular users'
 end

spec/contracts/placeholder_users/create_contract_spec.rb

@@ -37,6 +37,5 @@ describe PlaceholderUsers::CreateContract do
   it_behaves_like 'placeholder user contract' do
     let(:placeholder_user) { PlaceholderUser.new(name: placeholder_user_name) }
     let(:contract) { described_class.new(placeholder_user, current_user) }
-    let(:current_user) { FactoryBot.build_stubbed(:admin) }
   end
 end

spec/contracts/placeholder_users/delete_contract_spec.rb

@@ -37,5 +37,5 @@ describe PlaceholderUsers::DeleteContract do
   let(:placeholder_user) { FactoryBot.build_stubbed(:placeholder_user) }
   let(:contract) { described_class.new(placeholder_user, current_user) }
 
-  it_behaves_like 'contract is valid for active admin users only'
+  it_behaves_like 'contract is valid for active admins and invalid for regular users'
 end

spec/contracts/placeholder_users/shared_contract_examples.rb

@@ -33,41 +33,50 @@ require 'spec_helper'
 shared_examples_for 'placeholder user contract' do
   let(:placeholder_user_name) { 'UX Designer' }
 
-  it_behaves_like 'contract is valid for active admin users only'
+  context 'when user with global permission' do
+    let(:current_user) { FactoryBot.create(:user, global_permission: %i[add_placeholder_user]) }
 
-  context 'name' do
-    context 'is valid' do
-      it_behaves_like 'contract is valid'
-    end
+    it_behaves_like 'contract is valid'
+  end
 
-    context 'is not too long' do
-      let(:placeholder_user) { PlaceholderUser.new(name: 'X' * 257) }
+  it_behaves_like 'contract is valid for active admins and invalid for regular users'
 
-      it_behaves_like 'contract is invalid'
-    end
+  describe 'validations' do
+    let(:current_user) { FactoryBot.build_stubbed :admin }
+    context 'name' do
+      context 'is valid' do
+        it_behaves_like 'contract is valid'
+      end
 
-    context 'is not empty' do
-      let(:placeholder_user) { PlaceholderUser.new(name: '') }
+      context 'is not too long' do
+        let(:placeholder_user) { PlaceholderUser.new(name: 'X' * 257) }
 
-      it_behaves_like 'contract is invalid'
-    end
+        it_behaves_like 'contract is invalid'
+      end
 
-    context 'is unique' do
-      before do
-        PlaceholderUser.create(name: placeholder_user_name)
+      context 'is not empty' do
+        let(:placeholder_user) { PlaceholderUser.new(name: '') }
+
+        it_behaves_like 'contract is invalid'
       end
 
-      it_behaves_like 'contract is invalid'
-    end
-  end
+      context 'is unique' do
+        before do
+          PlaceholderUser.create(name: placeholder_user_name)
+        end
 
-  describe 'type' do
-    context 'type and class mismatch' do
-      before do
-        placeholder_user.type = User.name
+        it_behaves_like 'contract is invalid'
       end
+    end
+
+    describe 'type' do
+      context 'type and class mismatch' do
+        before do
+          placeholder_user.type = User.name
+        end
 
-      it_behaves_like 'contract is invalid'
+        it_behaves_like 'contract is invalid'
+      end
     end
   end
 end

spec/contracts/placeholder_users/update_contract_spec.rb

@@ -37,6 +37,5 @@ describe PlaceholderUsers::UpdateContract do
   it_behaves_like 'placeholder user contract' do
     let(:placeholder_user) { FactoryBot.build_stubbed(:placeholder_user, name: placeholder_user_name) }
     let(:contract) { described_class.new(placeholder_user, current_user) }
-    let(:current_user) { FactoryBot.build_stubbed(:admin) }
   end
 end

spec/contracts/projects/archive_contract_spec.rb

@@ -37,5 +37,5 @@ describe Projects::ArchiveContract do
   let(:project) { FactoryBot.build_stubbed(:project) }
   let(:contract) { described_class.new(project, current_user) }
 
-  it_behaves_like 'contract is valid for active admin users only'
+  it_behaves_like 'contract is valid for active admins and invalid for regular users'
 end

spec/contracts/projects/delete_contract_spec.rb

@@ -37,5 +37,5 @@ describe Projects::DeleteContract do
   let(:project) { FactoryBot.build_stubbed(:project) }
   let(:contract) { described_class.new(project, current_user) }
 
-  it_behaves_like 'contract is valid for active admin users only'
+  it_behaves_like 'contract is valid for active admins and invalid for regular users'
 end

spec/contracts/projects/unarchive_contract_spec.rb

@@ -37,5 +37,5 @@ describe Projects::UnarchiveContract do
   let(:project) { FactoryBot.build_stubbed(:project) }
   let(:contract) { described_class.new(project, current_user) }
 
-  it_behaves_like 'contract is valid for active admin users only'
+  it_behaves_like 'contract is valid for active admins and invalid for regular users'
 end

spec/contracts/shared/model_contract_shared_context.rb

@@ -29,7 +29,7 @@ shared_context 'ModelContract shared context' do
     end
   end
 
-  shared_examples 'contract is valid for active admin users only' do
+  shared_examples 'contract is valid for active admins and invalid for regular users' do
     context 'when admin' do
       let(:current_user) { FactoryBot.build_stubbed(:admin) }
 

spec/controllers/placeholder_users/memberships_controller_spec.rb

@@ -30,27 +30,22 @@ require 'spec_helper'
 require 'work_package'
 
 describe PlaceholderUsers::MembershipsController, type: :controller do
-  shared_let(:admin) { FactoryBot.create :admin }
-
-  let(:placeholder_user) { FactoryBot.create(:placeholder_user) }
-  let(:anonymous) { FactoryBot.create(:anonymous) }
-
-  describe 'update memberships' do
-    let(:project) { FactoryBot.create(:project) }
-    let(:role) { FactoryBot.create(:role) }
+  shared_let(:placeholder_user) { FactoryBot.create(:placeholder_user) }
+  shared_let(:anonymous) { FactoryBot.create(:anonymous) }
+  shared_let(:project) { FactoryBot.create(:project) }
+  shared_let(:role) { FactoryBot.create(:role) }
 
+  shared_examples 'update memberships flow' do
     it 'works' do
       # i.e. it should successfully add a placeholder user to a project's members
-      as_logged_in_user admin do
-        post :create,
-             params: {
-               placeholder_user_id: placeholder_user.id,
-               membership: {
-                 project_id: project.id,
-                 role_ids: [role.id]
-               }
+      post :create,
+           params: {
+             placeholder_user_id: placeholder_user.id,
+             membership: {
+               project_id: project.id,
+               role_ids: [role.id]
              }
-      end
+           }
 
       expect(response).to redirect_to(controller: '/placeholder_users',
                                       action: 'edit',
@@ -63,4 +58,60 @@ describe PlaceholderUsers::MembershipsController, type: :controller do
       expect(is_member).to eql(true)
     end
   end
+
+  shared_examples 'update memberships forbidden flow' do
+    describe 'POST create' do
+      it 'returns an error' do
+        post :create, params: {
+          placeholder_user_id: placeholder_user.id,
+          membership: {
+            project_id: project.id,
+            role_ids: [role.id]
+          }
+        }
+
+        expect(response.status).to eq 403
+      end
+    end
+
+    describe 'PUT update' do
+      it 'returns an error' do
+        put :update, params: {
+          placeholder_user_id: placeholder_user.id,
+          id: 1234
+        }
+
+        expect(response.status).to eq 403
+      end
+    end
+
+    describe 'DELETE destroy' do
+      it 'returns an error' do
+        delete :destroy, params: {
+          placeholder_user_id: placeholder_user.id,
+          id: 1234
+        }
+
+        expect(response.status).to eq 403
+      end
+    end
+  end
+
+  context 'as admin' do
+    current_user { FactoryBot.create :admin }
+
+    it_behaves_like 'update memberships flow'
+  end
+
+  context 'as user with global permission' do
+    current_user { FactoryBot.create :user, global_permission: %i[add_placeholder_user] }
+
+    it_behaves_like 'update memberships flow'
+  end
+
+  context 'as user without global permission' do
+    current_user { FactoryBot.create :user }
+
+    it_behaves_like 'update memberships forbidden flow'
+  end
 end

spec/controllers/placeholder_users_controller_spec.rb

@@ -30,97 +30,55 @@ require 'spec_helper'
 require 'work_package'
 
 describe PlaceholderUsersController, type: :controller do
-  let(:current_user) { FactoryBot.build(:admin) }
-  let(:placeholder_user) { FactoryBot.create(:placeholder_user) }
+  shared_let(:placeholder_user) { FactoryBot.create(:placeholder_user) }
 
   shared_examples 'do not allow non-admins' do
-    let(:current_user) { FactoryBot.build(:user) }
-
     it 'responds with unauthorized status' do
       expect(response).to_not be_successful
       expect(response.status).to eq 403
     end
   end
 
-  describe 'GET new' do
-    before do
-      as_logged_in_user(current_user) do
-        get :new
-      end
+  shared_examples 'renders the show template' do
+    it 'renders the show template' do
+      get :show, params: { id: placeholder_user.id }
+
+      expect(response).to be_successful
+      expect(response).to render_template 'placeholder_users/show'
+      expect(assigns(:placeholder_user)).to be_present
+      expect(assigns(:memberships)).to be_empty
     end
+  end
 
-    context 'as admin' do
+  shared_examples 'authorized flows' do
+    describe 'GET new' do
       it 'renders the new template' do
+        get :new
+
         expect(response).to be_successful
         expect(response).to render_template 'placeholder_users/new'
         expect(assigns(:placeholder_user)).to be_present
       end
     end
 
-    context 'not as admin' do
-      let(:current_user) { FactoryBot.build(:user) }
-
-      it 'responds with unauthorized status' do
-        expect(response).to_not be_successful
-        expect(response.status).to eq 403
-      end
-    end
-  end
-
-  describe 'GET index' do
-    before do
-      as_logged_in_user(current_user) do
+    describe 'GET index' do
+      it 'renders the index template' do
         get :index
-      end
-    end
 
-    context 'as admin' do
-      it 'renders the index template' do
         expect(response).to be_successful
         expect(response).to render_template 'placeholder_users/index'
-        expect(assigns(:placeholder_users)).to be_empty
+        expect(assigns(:placeholder_users)).to be_present
         expect(assigns(:groups)).not_to be_present
       end
     end
 
-    context 'not as admin' do
-      let(:current_user) { FactoryBot.build(:user) }
-
-      it_behaves_like 'do not allow non-admins'
-    end
-  end
-
-  describe 'GET show' do
-    shared_examples 'renders the show template' do
-      it 'renders the show template' do
-        expect(response).to be_successful
-        expect(response).to render_template 'placeholder_users/show'
-        expect(assigns(:placeholder_user)).to be_present
-        expect(assigns(:memberships)).to be_empty
-      end
-    end
-
-    before do
-      as_logged_in_user(current_user) do
-        get :show, params: { id: placeholder_user.id }
-      end
-    end
-
-    context 'as admin' do
-      it_behaves_like 'renders the show template'
-    end
-
-    context 'not as admin' do
-      let(:current_user) { FactoryBot.build(:user) }
-
-      # normal users can also checkout the profile page of placeholder user.
+    describe 'GET show' do
       it_behaves_like 'renders the show template'
     end
-  end
 
-  describe 'GET edit' do
-    shared_examples 'renders the edit template' do
+    describe 'GET edit' do
       it 'renders the show template' do
+        get :edit, params: { id: placeholder_user.id }
         expect(response).to be_successful
         expect(response).to render_template "placeholder_users/edit"
         expect(assigns(:placeholder_user)).to eql(placeholder_user)
@@ -129,40 +87,19 @@ describe PlaceholderUsersController, type: :controller do
       end
     end
 
-    before do
-      as_logged_in_user(current_user) do
-        get :edit, params: { id: placeholder_user.id }
-      end
-    end
-
-    context 'as admin' do
-      it_behaves_like 'renders the edit template'
-    end
-
-    context 'not as admin' do
-      let(:current_user) { FactoryBot.build(:user) }
-
-      # normal users can also checkout the profile page of placeholder user.
-      it_behaves_like 'do not allow non-admins'
-    end
-  end
-
-  describe 'POST create' do
-    let(:params) do
-      {
-        placeholder_user: {
-          name: 'UX Developer'
+    describe 'POST create' do
+      let(:params) do
+        {
+          placeholder_user: {
+            name: 'UX Developer'
+          }
         }
-      }
-    end
+      end
 
-    before do
-      as_logged_in_user(current_user) do
+      before do
         post :create, params: params
       end
-    end
 
-    context 'as admin' do
       it 'should be assigned their new values' do
         user_from_db = PlaceholderUser.last
         expect(user_from_db.name).to eq('UX Developer')
@@ -180,7 +117,7 @@ describe PlaceholderUsersController, type: :controller do
         let(:params) do
           {
             placeholder_user: {
-                name: 'UX Developer'
+              name: 'UX Developer'
             },
             continue: true
           }
@@ -205,43 +142,37 @@ describe PlaceholderUsersController, type: :controller do
           expect(response).to redirect_to(edit_placeholder_user_url(user_from_db))
         end
       end
-    end
 
-    it_behaves_like 'do not allow non-admins'
+      context 'invalid params' do
+        let(:params) do
+          {
+            placeholder_user: {
+              name: 'x' * 300 # Name is too long
+            }
+          }
+        end
+
+        it 'should render the edit form with a validation error message' do
+          expect(assigns(:'placeholder_user').errors.messages[:name].first).to include('is too long')
+          expect(response).to render_template 'placeholder_users/new'
+        end
+      end
+    end
 
-    context 'invalid params' do
+    describe 'PUT update' do
       let(:params) do
         {
+          id: placeholder_user.id,
           placeholder_user: {
-            name: 'x' * 300 # Name is too long
+            name: 'UX Guru'
           }
         }
       end
 
-      it 'should render the edit form with a validation error message' do
-        expect(assigns(:'placeholder_user').errors.messages[:name].first).to include('is too long')
-        expect(response).to render_template 'placeholder_users/new'
-      end
-    end
-  end
-
-  describe 'PUT update' do
-    let(:params) do
-      {
-        id: placeholder_user.id,
-        placeholder_user: {
-          name: 'UX Guru'
-        }
-      }
-    end
-
-    before do
-      as_logged_in_user(current_user) do
+      before do
         put :update, params: params
       end
-    end
 
-    context 'as admin' do
       it 'should redirect to the edit page' do
         expect(response).to redirect_to(edit_placeholder_user_url(placeholder_user))
       end
@@ -254,30 +185,114 @@ describe PlaceholderUsersController, type: :controller do
       it 'should not send an email' do
         expect(ActionMailer::Base.deliveries.empty?).to be_truthy
       end
+
+      context 'invalid params' do
+        let(:params) do
+          {
+            id: placeholder_user.id,
+            placeholder_user: {
+              name: 'x' * 300 # Name is too long
+            }
+          }
+        end
+
+        it 'should render the edit form with a validation error message' do
+          expect(assigns(:'placeholder_user').errors.messages[:name].first).to include('is too long')
+          expect(response).to render_template 'placeholder_users/edit'
+        end
+      end
+    end
+
+    describe 'POST destroy' do
+      before do
+        delete :destroy, params: { id: placeholder_user.id }
+      end
+
+      pending 'not yet implemented'
+    end
+  end
+
+  context 'as an admin' do
+    current_user { FactoryBot.create :admin }
+    it_behaves_like 'authorized flows'
+  end
+
+  context 'as a user with global permission' do
+    current_user { FactoryBot.create :user, global_permission: %i[add_placeholder_user] }
+    it_behaves_like 'authorized flows'
+  end
+
+  context 'as an unauthorized user' do
+    current_user { FactoryBot.create :user }
+
+    describe 'GET new' do
+      before do
+        get :new
+      end
+
+      it_behaves_like 'do not allow non-admins'
+    end
+
+    describe 'GET index' do
+      before do
+        get :index
+      end
+
+      it_behaves_like 'do not allow non-admins'
     end
 
-    it_behaves_like 'do not allow non-admins'
+    describe 'GET show' do
+      it_behaves_like 'renders the show template'
+    end
+
+    describe 'GET edit' do
+      before do
+        get :edit, params: { id: placeholder_user.id }
+      end
+
+      it_behaves_like 'do not allow non-admins'
+    end
+
+    describe 'POST create' do
+      let(:params) do
+        {
+          placeholder_user: {
+            name: 'UX Developer'
+          }
+        }
+      end
+
+      before do
+        post :create, params: params
+      end
+
+      it_behaves_like 'do not allow non-admins'
+    end
 
-    context 'invalid params' do
+    describe 'PUT update' do
       let(:params) do
         {
           id: placeholder_user.id,
           placeholder_user: {
-            name: 'x' * 300 # Name is too long
+            name: 'UX Guru'
           }
         }
       end
 
-      it 'should render the edit form with a validation error message' do
-        expect(assigns(:'placeholder_user').errors.messages[:name].first).to include('is too long')
-        expect(response).to render_template 'placeholder_users/edit'
+      before do
+        put :update, params: params
       end
+
+      it_behaves_like 'do not allow non-admins'
     end
-  end
 
-  describe 'POST destroy' do
-    pending 'Admins can destroy placeholder users'
-    pending 'Non admins cannot destroy placeholder users'
+    describe 'POST destroy' do
+      before do
+        delete :destroy, params: { id: placeholder_user.id }
+      end
+
+      it_behaves_like 'do not allow non-admins'
+    end
   end
 end
 

spec/features/placeholder_users/create_spec.rb

@@ -29,15 +29,15 @@
 require 'spec_helper'
 
 describe 'create placeholder users', type: :feature, selenium: true do
-  let(:current_user) { FactoryBot.create :admin }
   let(:new_placeholder_user_page) { Pages::NewPlaceholderUser.new }
 
-  before do
-    allow(User).to receive(:current).and_return current_user
-  end
-
-  shared_examples_for 'successful placeholder user creation' do
+  shared_examples_for 'placeholders creation flow' do
     it 'creates the placeholder user' do
+      visit new_placeholder_user_path
+
+      new_placeholder_user_page.fill_in! name: 'UX Designer'
+      new_placeholder_user_page.submit!
+
       expect(page).to have_selector('.flash', text: 'Successful creation.')
 
       new_placeholder_user = PlaceholderUser.order(Arel.sql('id DESC')).first
@@ -47,16 +47,23 @@ describe 'create placeholder users', type: :feature, selenium: true do
   end
 
   context 'as admin' do
-    before do
-      visit new_placeholder_user_path
+    current_user { FactoryBot.create :admin }
 
-      new_placeholder_user_page.fill_in! name: 'UX Designer'
+    it_behaves_like 'placeholders creation flow'
+  end
 
-      perform_enqueued_jobs do
-        new_placeholder_user_page.submit!
-      end
-    end
+  context 'as user with global permission' do
+    current_user { FactoryBot.create :user, global_permission: %i[add_placeholder_user] }
+
+    it_behaves_like 'placeholders creation flow'
+  end
 
-    it_behaves_like 'successful placeholder user creation'
+  context 'as user without global permission' do
+    current_user { FactoryBot.create :user }
+
+    it 'returns an error' do
+      visit new_placeholder_user_path
+      expect(page).to have_text 'You are not authorized to access this page.'
+    end
   end
 end

spec/features/placeholder_users/edit_placeholder_users_spec.rb

@@ -29,19 +29,12 @@
 require 'spec_helper'
 
 describe 'edit placeholder users', type: :feature, js: true do
-  let(:current_user) { FactoryBot.create :admin }
-  let(:placeholder_user) { FactoryBot.create :placeholder_user, name: 'UX Developer' }
+  shared_let(:placeholder_user) { FactoryBot.create :placeholder_user, name: 'UX Developer' }
 
-  before do
-    login_as current_user
-  end
-
-  context 'as admin' do
-    before do
+  shared_examples 'placeholders edit flow' do
+    it 'can edit name' do
       visit edit_placeholder_user_path(placeholder_user)
-    end
 
-    it 'can edit name' do
       expect(page).to have_selector '#placeholder_user_name'
 
       fill_in 'placeholder_user[name]', with: 'NewName', fill_options: { clear: :backspace }
@@ -55,4 +48,25 @@ describe 'edit placeholder users', type: :feature, js: true do
       expect(placeholder_user.name).to eq 'NewName'
     end
   end
+
+  context 'as admin' do
+    current_user { FactoryBot.create :admin }
+
+    it_behaves_like 'placeholders edit flow'
+  end
+
+  context 'as user with global permission' do
+    current_user { FactoryBot.create :user, global_permission: %i[add_placeholder_user] }
+
+    it_behaves_like 'placeholders edit flow'
+  end
+
+  context 'as user without global permission' do
+    current_user { FactoryBot.create :user }
+
+    it 'returns an error' do
+      visit edit_placeholder_user_path(placeholder_user)
+      expect(page).to have_text 'You are not authorized to access this page.'
+    end
+  end
 end

spec/features/placeholder_users/index_spec.rb

@@ -48,27 +48,46 @@ describe 'index placeholder users', type: :feature do
   end
   let(:index_page) { Pages::Admin::PlaceholderUsers::Index.new }
 
-  before do
-    login_as(current_user)
+  shared_examples 'placeholders index flow' do
+    it 'shows the placeholder users and allows filtering and ordering' do
+      index_page.visit!
+
+      index_page.expect_not_listed(anonymous, current_user)
+
+      # Order is by id, asc
+      # so first ones created are on top.
+      index_page.expect_listed(placeholder_user_1, placeholder_user_2, placeholder_user_3)
+
+      index_page.order_by('Created on')
+      index_page.expect_listed(placeholder_user_3, placeholder_user_2, placeholder_user_1)
+
+      index_page.order_by('Created on')
+      index_page.expect_listed(placeholder_user_1, placeholder_user_2, placeholder_user_3)
+
+      index_page.filter_by_name(placeholder_user_3.name)
+      index_page.expect_listed(placeholder_user_3)
+      index_page.expect_not_listed(placeholder_user_1, placeholder_user_2)
+    end
   end
 
-  it 'shows the placeholder users and allows filtering and ordering' do
-    index_page.visit!
+  context 'as admin' do
+    current_user { FactoryBot.create :admin }
 
-    index_page.expect_not_listed(anonymous, current_user)
+    it_behaves_like 'placeholders index flow'
+  end
 
-    # Order is by id, asc
-    # so first ones created are on top.
-    index_page.expect_listed(placeholder_user_1, placeholder_user_2, placeholder_user_3)
+  context 'as user with global permission' do
+    current_user { FactoryBot.create :user, global_permission: %i[add_placeholder_user] }
 
-    index_page.order_by('Created on')
-    index_page.expect_listed(placeholder_user_3, placeholder_user_2, placeholder_user_1)
+    it_behaves_like 'placeholders index flow'
+  end
 
-    index_page.order_by('Created on')
-    index_page.expect_listed(placeholder_user_1, placeholder_user_2, placeholder_user_3)
+  context 'as user without global permission' do
+    current_user { FactoryBot.create :user }
 
-    index_page.filter_by_name(placeholder_user_3.name)
-    index_page.expect_listed(placeholder_user_3)
-    index_page.expect_not_listed(placeholder_user_1, placeholder_user_2)
+    it 'returns an error' do
+      index_page.visit!
+      expect(page).to have_text 'You are not authorized to access this page.'
+    end
   end
 end

spec/features/placeholder_users/placeholder_user_memberships_spec.rb

@@ -27,51 +27,19 @@
 #++
 
 require 'spec_helper'
+require_relative '../principals/shared_memberships_examples'
 
 feature 'placeholder user memberships through placeholder user page', type: :feature, js: true do
-  let!(:project) { FactoryBot.create :project, name: 'Project 1', identifier: 'project1' }
-  let!(:project2) { FactoryBot.create :project, name: 'Project 2', identifier: 'project2' }
-  let(:admin) { FactoryBot.create :admin }
-  let(:placeholder_user) { FactoryBot.create :placeholder_user, name: 'UX Designer' }
+  shared_let(:principal) { FactoryBot.create :placeholder_user, name: 'UX Designer' }
+  shared_let(:principal_page) { Pages::Admin::IndividualPrincipals::Edit.new(principal) }
 
-  let!(:manager)   { FactoryBot.create :role, name: 'Manager' }
-  let!(:developer) { FactoryBot.create :role, name: 'Developer' }
+  include_context 'principal membership management context'
 
-  let(:placeholder_user_page) { Pages::Admin::IndividualPrincipals::Edit.new(placeholder_user) }
+  context 'as admin' do
+    current_user { FactoryBot.create :admin }
 
-  before do
-    login_as(admin)
-
-    placeholder_user_page.visit!
-    placeholder_user_page.open_projects_tab!
+    it_behaves_like 'principal membership management flows'
   end
 
-  scenario 'handles role modification flow' do
-    placeholder_user_page.add_to_project! project.name, as: 'Manager'
-
-    member = placeholder_user.memberships.where(project_id: project.id).first
-    placeholder_user_page.edit_roles!(member, %w(Manager Developer))
-
-    # Modify roles
-    placeholder_user_page.expect_project(project.name)
-    placeholder_user_page.expect_roles(project.name, %w(Manager Developer))
-
-    placeholder_user_page.expect_no_membership(project2.name)
-
-    # Remove all roles
-    placeholder_user_page.expect_project(project.name)
-    placeholder_user_page.edit_roles!(member, %w())
-
-    expect(page).to have_selector('.flash.error', text: 'Roles need to be assigned.')
-
-    # Remove the user from the project
-    placeholder_user_page.remove_from_project!(project.name)
-    placeholder_user_page.expect_no_membership(project.name)
-
-    # Re-add the user
-    placeholder_user_page.add_to_project! project.name, as: %w(Manager Developer)
-
-    placeholder_user_page.expect_project(project.name)
-    placeholder_user_page.expect_roles(project.name, %w(Manager Developer))
-  end
+  it_behaves_like 'global user principal membership management flows', :add_placeholder_user
 end

spec/features/principals/shared_memberships_examples.rb

@@ -0,0 +1,93 @@
+shared_context 'principal membership management context' do
+  shared_let(:project) { FactoryBot.create :project, name: 'Project 1', identifier: 'project1' }
+  shared_let(:project2) { FactoryBot.create :project, name: 'Project 2', identifier: 'project2' }
+
+  shared_let(:manager)   { FactoryBot.create :role, name: 'Manager', permissions: %i[view_members manage_members] }
+  shared_let(:developer) { FactoryBot.create :role, name: 'Developer' }
+end
+
+shared_examples 'principal membership management flows' do
+  scenario 'handles role modification flow' do
+    principal_page.visit!
+    principal_page.open_projects_tab!
+
+    principal_page.add_to_project! project.name, as: 'Manager'
+
+    member = principal.memberships.where(project_id: project.id).first
+    principal_page.edit_roles!(member, %w(Manager Developer))
+
+    # Modify roles
+    principal_page.expect_project(project.name)
+    principal_page.expect_roles(project.name, %w(Manager Developer))
+
+    principal_page.expect_no_membership(project2.name)
+
+    # Remove all roles
+    principal_page.expect_project(project.name)
+    principal_page.edit_roles!(member, %w())
+
+    expect(page).to have_selector('.flash.error', text: 'Roles need to be assigned.')
+
+    # Remove the user from the project
+    principal_page.remove_from_project!(project.name)
+    principal_page.expect_no_membership(project.name)
+
+    # Re-add the user
+    principal_page.add_to_project! project.name, as: %w(Manager Developer)
+
+    principal_page.expect_project(project.name)
+    principal_page.expect_roles(project.name, %w(Manager Developer))
+  end
+end
+
+shared_examples 'global user principal membership management flows' do |permission|
+  context 'as global user' do
+    shared_let(:global_user) { FactoryBot.create :user, global_permission: permission }
+    current_user { global_user }
+
+    context 'when the user is member in the projects' do
+      it_behaves_like 'principal membership management flows' do
+      before do
+          project.add_member global_user, [manager]
+          project.save!
+
+          project2.add_member global_user, [manager]
+          project2.save!
+        end
+      end
+    end
+
+    context 'when the user cannot see the two projects' do
+      it 'does not show them' do
+        principal_page.visit!
+        principal_page.open_projects_tab!
+
+        expect(page).to have_no_selector('#membership_project_id option', text: project.name, visible: :all)
+        expect(page).to have_no_selector('#membership_project_id option', text: project2.name, visible: :all)
+      end
+
+      it 'does not show the membership' do
+        project.add_member principal, [developer]
+        project.save!
+
+        principal_page.visit!
+        principal_page.open_projects_tab!
+
+        expect(page).to have_no_selector('tr.member')
+        expect(page).to have_text 'There is currently nothing to display.'
+        expect(page).to have_no_text project2.name
+        expect(page).to have_no_text project2.name
+      end
+    end
+  end
+
+  context 'as user without global permission' do
+    current_user { FactoryBot.create :user }
+
+    it 'returns an error' do
+      principal_page.visit!
+      expect(page).to have_text 'You are not authorized to access this page.'
+      expect(page).to have_no_text principal.name
+    end
+  end
+end

spec/features/users/user_memberships_spec.rb

@@ -27,94 +27,19 @@
 #++
 
 require 'spec_helper'
+require_relative '../principals/shared_memberships_examples'
 
 feature 'user memberships through user page', type: :feature, js: true do
-  let!(:project) { FactoryBot.create :project, name: 'Project 1', identifier: 'project1' }
-  let!(:project2) { FactoryBot.create :project, name: 'Project 2', identifier: 'project2' }
-  let(:admin) { FactoryBot.create :admin, firstname: 'Foobar', lastname: 'Blabla' }
+  include_context 'principal membership management context'
 
-  let!(:manager)   { FactoryBot.create :role, name: 'Manager' }
-  let!(:developer) { FactoryBot.create :role, name: 'Developer' }
+  shared_let(:principal) { FactoryBot.create :user, firstname: 'Foobar', lastname: 'Blabla' }
+  shared_let(:principal_page) { Pages::Admin::IndividualPrincipals::Edit.new(principal) }
 
-  let(:user_page) { Pages::Admin::IndividualPrincipals::Edit.new(admin) }
+  context 'as admin' do
+    current_user { FactoryBot.create :admin }
 
-  before do
-    login_as(admin)
-
-    user_page.visit!
-    user_page.open_projects_tab!
-  end
-
-  scenario 'handles role modification flow' do
-    SeleniumHubWaiter.wait
-    user_page.add_to_project! project.name, as: 'Manager'
-
-    member = admin.memberships.where(project_id: project.id).first
-    SeleniumHubWaiter.wait
-    user_page.edit_roles!(member, %w(Manager Developer))
-
-    # Modify roles
-    user_page.expect_project(project.name)
-    user_page.expect_roles(project.name, %w(Manager Developer))
-
-    user_page.expect_no_membership(project2.name)
-
-    # Remove all roles
-    user_page.expect_project(project.name)
-    SeleniumHubWaiter.wait
-    user_page.edit_roles!(member, %w())
-
-    expect(page).to have_selector('.flash.error', text: 'Roles need to be assigned.')
-
-    # Remove the user from the project
-    user_page.remove_from_project!(project.name)
-    user_page.expect_no_membership(project.name)
-
-    # Re-add the user
-    SeleniumHubWaiter.wait
-    user_page.add_to_project! project.name, as: %w(Manager Developer)
-
-    user_page.expect_project(project.name)
-    user_page.expect_roles(project.name, %w(Manager Developer))
+    it_behaves_like 'principal membership management flows'
   end
 
-  context 'when user has an inherited role' do
-    let(:group) { FactoryBot.create :group, lastname: 'A-Team' }
-    let(:group_page) { Pages::Groups.new.group(group.id) }
-
-    before do
-      group.add_members! admin
-    end
-
-    scenario 'it can remove all other roles' do
-      user_page.expect_no_membership(project.name)
-
-      group_page.visit!
-      SeleniumHubWaiter.wait
-      group_page.add_to_project! project.name, as: 'Manager'
-      expect(page).to have_text 'Successful update'
-
-      user_page.visit!
-      user_page.open_projects_tab!
-
-      # Expect inherited membership
-      user_page.expect_project(project.name)
-      user_page.expect_roles(project.name, %w(Manager))
-
-      # Remove all roles
-      member = admin.memberships.where(project_id: project.id).first
-      SeleniumHubWaiter.wait
-      user_page.edit_roles!(member, %w())
-
-      # Keeps inherited role
-      user_page.expect_project(project.name)
-      user_page.expect_roles(project.name, %w(Manager))
-
-      # Extend roles
-      SeleniumHubWaiter.wait
-      user_page.edit_roles!(member, %w(Developer))
-      user_page.expect_project(project.name)
-      user_page.expect_roles(project.name, %w(Manager Developer))
-    end
-  end
-end
+  it_behaves_like 'global user principal membership management flows', :add_user
+end

spec/services/placeholder_users/create_service_spec.rb

@@ -0,0 +1,36 @@
+#-- encoding: UTF-8
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2021 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+require 'spec_helper'
+require 'services/base_services/behaves_like_create_service'
+
+describe PlaceholderUsers::CreateService, type: :model do
+  it_behaves_like 'BaseServices create service'
+end

spec/services/placeholder_users/delete_service_spec.rb

@@ -33,7 +33,7 @@ describe ::PlaceholderUsers::DeleteService, type: :model do
     let(:input_user) { FactoryBot.build_stubbed(:user) }
     let(:project) { FactoryBot.build_stubbed(:project) }
 
-    let(:instance) { described_class.new(input_user, actor) }
+    let(:instance) { described_class.new(model: input_user, user: actor) }
 
     subject { instance.call }
 

spec/services/placeholder_users/set_attributes_service_spec.rb

@@ -0,0 +1,116 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2021 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+require 'spec_helper'
+
+describe PlaceholderUsers::SetAttributesService, type: :model do
+  let(:current_user) { FactoryBot.build_stubbed(:user) }
+
+  let(:contract_instance) do
+    contract = double('contract_instance')
+    allow(contract)
+      .to receive(:validate)
+      .and_return(contract_valid)
+    allow(contract)
+      .to receive(:errors)
+      .and_return(contract_errors)
+    contract
+  end
+
+  let(:contract_errors) { double('contract_errors') }
+  let(:contract_valid) { true }
+  let(:model_valid) { true }
+
+  let(:instance) do
+    described_class.new(user: current_user,
+                        model: model_instance,
+                        contract_class: contract_class,
+                        contract_options: {})
+  end
+  let(:model_instance) { PlaceholderUser.new }
+  let(:contract_class) do
+    allow(PlaceholderUsers::CreateContract)
+      .to receive(:new)
+      .and_return(contract_instance)
+
+    PlaceholderUsers::CreateContract
+  end
+
+  let(:params) { {} }
+
+  before do
+    allow(model_instance)
+      .to receive(:valid?)
+      .and_return(model_valid)
+  end
+
+  subject { instance.call(params) }
+
+  it 'returns the instance as the result' do
+    expect(subject.result)
+      .to eql model_instance
+  end
+
+  it 'is a success' do
+    is_expected
+      .to be_success
+  end
+
+  context 'with params' do
+    let(:params) do
+      {
+        name: 'Foobar'
+      }
+    end
+
+    it 'assigns the params' do
+      subject
+
+      expect(model_instance.name).to eq 'Foobar'
+      expect(model_instance.lastname).to eq 'Foobar'
+    end
+  end
+
+  context 'with an invalid contract' do
+    let(:contract_valid) { false }
+    let(:expect_time_instance_save) do
+      expect(model_instance)
+        .not_to receive(:save)
+    end
+
+    it 'returns failure' do
+      is_expected
+        .not_to be_success
+    end
+
+    it "returns the contract's errors" do
+      expect(subject.errors)
+        .to eql(contract_errors)
+    end
+  end
+end

spec/services/placeholder_users/update_service_spec.rb

@@ -0,0 +1,36 @@
+#-- encoding: UTF-8
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2021 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+require 'spec_helper'
+require 'services/base_services/behaves_like_update_service'
+
+describe PlaceholderUsers::UpdateService, type: :model do
+  it_behaves_like 'BaseServices update service'
+end