Portus admins are always team owners

This commit forces that whenever a Portus admin gets added into a team, it will
have owner privileges. Moreover, this commit also forbids demoting a Portus
admin from being an owner.

Fixes #977

Signed-off-by: Miquel Sabaté Solà <msabate@suse.com>

app/controllers/team_users_controller.rb

@@ -1,13 +1,17 @@
 # TeamUsersController manages the creation/removal/update of members of a team.
 class TeamUsersController < ApplicationController
   before_action :set_team_user
+  before_action :promoted_owner, only: [:create, :update]
   before_action :only_owner, only: [:update, :destroy]
   after_action :verify_authorized
 
   respond_to :js
 
   # POST /team_users
   def create
+    # Promote this user if it is a Portus admin.
+    @team_user.role = TeamUser.roles[:owner] if @promoted_role
+
     if @team_user.errors.empty? && @team_user.save
       @team_user.create_activity!(:add_member, current_user)
       respond_with @team_user
@@ -18,6 +22,13 @@ def create
 
   # PATCH/PUT /team_users/1
   def update
+    # Send an error if an admin was about to get demoted.
+    if @promoted_role
+      @team_user.errors.add(:user, "cannot be demoted because it's a Portus admin")
+      respond_with @team_user.errors, status: :unprocessable_entity
+      return
+    end
+
     team_user_params = params.require(:team_user).permit(:role)
 
     old_role = @team_user.role
@@ -68,6 +79,18 @@ def set_team_user
     authorize @team_user
   end
 
+  # Sets the @promoted_role instance variable if a Portus admin is going to be
+  # set a role other than owner.
+  def promoted_owner
+    return if @team_user.user.nil?
+
+    tu   = params.require(:team_user).permit(:role)
+    role = TeamUser.roles[tu["role"]]
+
+    return if role == TeamUser.roles[:owner] || !@team_user.user.admin?
+    @promoted_role = true
+  end
+
   # Responds with an error if the client is trying to remove the only owner of
   # the team through either the update or the destroy methods.
   def only_owner

app/views/team_users/create.js.erb

@@ -2,7 +2,11 @@
   $('#float-alert p').html("<%= escape_javascript(@team_user.errors.full_messages.join('<br/>')) %>");
 <% else %>
   $("<%= escape_javascript(render @team_user) %>").appendTo("#team_users");
-  $('#float-alert p').html("New user added to the team");
+  <% if @promoted_role %>
+    $('#float-alert p').html("New user added to the team (promoted to owner because it is a Portus admin).");
+  <% else %>
+    $('#float-alert p').html("New user added to the team");
+  <% end %>
   $('#add_team_user_form').fadeOut();
   $('#add_team_user_btn i').addClass("fa-chevron-down")
   $('#add_team_user_btn i').removeClass("fa-chevron-up")

spec/controllers/team_users_controller_spec.rb

@@ -55,6 +55,16 @@
         expect(assigns(:team_user).errors).to be_empty
       end
 
+      it "does not allow an admin to be demoted" do
+        user = create(:admin)
+        team.owners << user
+
+        put :update, id: team.team_users.find_by(user: user).id,
+                     team_user: { role: "contributor" }, format: "js"
+        expect(response.status).to eq 422
+        expect(team.owners.exists?(user.id)).to be true
+      end
+
       it "forces a page reload when the current user changes his role" do
         user = create(:user)
         team.owners << user
@@ -76,6 +86,18 @@
         expect(team.owners.exists?(new_user.id)).to be true
       end
 
+      it "sets admins as owners always" do
+        new_user = create(:admin)
+        post :create,
+             team_user: {
+               team: team.name,
+               user: new_user.username,
+               role: TeamUser.roles["viewer"]
+             },
+             format:    "js"
+        expect(team.owners.exists?(new_user.id)).to be true
+      end
+
       it "returns an error if the user is not found" do
         post :create,
              team_user: { team: team.name, user: "ghost", role: TeamUser.roles["owner"] },

spec/features/teams_spec.rb

@@ -104,6 +104,7 @@
 
   describe "teams#show" do
     let!(:another) { create(:user) }
+    let!(:another_admin) { create(:admin) }
 
     before :each do
       visit team_path(team)
@@ -143,6 +144,20 @@
       expect(page).to have_content("Contributor")
     end
 
+    scenario "An admin can only be added as a team owner", js: true do
+      find("#add_team_user_btn").click
+      wait_for_effect_on("#add_team_user_form")
+      find("#team_user_role").select "Contributor"
+      find("#team_user_user").set another_admin.username
+      find("#add_team_user_form .btn").click
+
+      wait_for_ajax
+      wait_for_effect_on("#float-alert")
+
+      expect(page).to have_content("New user added to the team (promoted to")
+      expect(page).to have_content("Owner")
+    end
+
     scenario "New team members have to exist on the system", js: true do
       find("#add_team_user_btn").click
       wait_for_effect_on("#add_team_user_form")