Designers cannot set others to admins, fixes #197

app/controllers/admin/base_controller.rb

@@ -15,6 +15,8 @@ module Admin
 
     before_filter :set_locale
 
+    before_filter :set_current_thread_variables
+
     helper_method :sections, :current_site_url, :site_url, :page_url, :current_ability
 
     # https://rails.lighthouseapp.com/projects/8994/tickets/1905-apphelpers-within-plugin-not-being-mixed-in
@@ -42,6 +44,11 @@ module Admin
 
     protected
 
+    def set_current_thread_variables
+      Thread.current[:admin] = current_admin
+      Thread.current[:site]  = current_site
+    end
+
     def current_ability
       @current_ability ||= Ability.new(current_admin, current_site)
     end

app/models/ability.rb

@@ -65,7 +65,9 @@ class Ability
 
     can :manage, Membership
 
-    cannot :change_role, Membership do |membership|
+    cannot :grant_admin, Membership
+
+    cannot [:update, :destroy], Membership do |membership|
       @membership.account_id == membership.account_id || # can not edit myself
       membership.admin? # can not modify an administrator
     end
@@ -74,7 +76,7 @@ class Ability
   def setup_admin_permissions!
     can :manage, :all
 
-    cannot :change_role, Membership do |membership|
+    cannot [:update, :destroy], Membership do |membership|
       @membership.account_id == membership.account_id # can not edit myself
     end
   end

app/models/membership.rb

@@ -11,6 +11,7 @@ class Membership
 
   ## validations ##
   validates_presence_of :account
+  validate :can_change_role, :if => :role_changed?
 
   ## callbacks ##
   before_save :define_role
@@ -55,4 +56,18 @@ class Membership
     self.role = Ability::ROLES.include?(role.downcase) ? role.downcase : Ability::ROLES.first
   end
 
+  # Users should not be able to set the role of another user to be higher than
+  # their own. A designer for example should not be able to set another user to
+  # be an administrator
+  def can_change_role
+    current_site       = Thread.current[:site]
+    current_membership = current_site.memberships.where(:account_id => Thread.current[:admin].id).first if current_site.present?
+
+    if current_membership.present?
+      # The role cannot be set higher than the current one (we use the index in
+      # the roles array to check role presidence)
+      errors.add(:role, :invalid) if Ability::ROLES.index(role) < Ability::ROLES.index(current_membership.role)
+    end
+  end
+
 end

app/views/admin/current_site/_form.html.haml

@@ -54,11 +54,14 @@
 
         %em.email= account.email
 
-        - if can?(:change_role, membership)
+        - if can?(:update, membership)
           .role
             %em.editable= t("admin.memberships.roles.#{membership.role}")
 
-            = fm.select :role, Ability::ROLES.collect { |r| [t("admin.memberships.roles.#{r}"), r] }, :include_blank => false
+            - if can?(:grant_admin, membership)
+              = fm.select :role, Ability::ROLES.map { |r| [t("admin.memberships.roles.#{r}"), r] }, :include_blank => false
+            - else
+              = fm.select :role, (Ability::ROLES - ['admin']).map { |r| [t("admin.memberships.roles.#{r}"), r] }, :include_blank => false
 
           %span.actions
             = link_to image_tag('admin/form/icons/trash.png'), admin_membership_url(membership), :class => 'remove first', :confirm =>t('admin.messages.confirm'), :method => :delete
@@ -73,4 +76,4 @@
     = f.custom_input :robots_txt, :css => 'code full', :with_label => false do
       = f.label :robots_txt
       %code{ :class => 'html' }
-        = f.text_area :robots_txt, :class => 'small'
+        = f.text_area :robots_txt, :class => 'small'

features/admin/authorization/current_site.feature

@@ -36,6 +36,7 @@ Background:
     And I should not see the role dropdown on myself
     And I should not see the role dropdown on the "admin"
     And I should see the role dropdown on the "author"
+    And I should not the role dropdown on the "author" without the "Administrator" option
     And I should not see delete on the "admin"
     And I should not see delete on myself
     And I should see delete on the "author"

features/step_definitions/current_site_steps.rb

@@ -1,5 +1,9 @@
 Then /^I should see the role dropdown on the "([^"]*)"$/ do |user|
-  find(:css, "li.membership[data-role=#{user}] select").text.should == 'AdministratorDesignerAuthor'
+  find(:css, "li.membership[data-role=#{user}] select").should be_present
+end
+
+Then /^I should not the role dropdown on the "([^"]*)" without the "([^"]*)" option$/ do |user, option|
+  find(:css, "li.membership[data-role=#{user}] select").text.should_not include option
 end
 
 Then /^I should see the role dropdown on myself$/ do
@@ -33,3 +37,7 @@ end
 Then /^I should not see any delete buttons$/ do
  page.has_css?('li.membership .actions a.remove').should be_false
 end
+
+When /^I select the "([^"]*)" role for the "author" user/ do |role|
+  Given %{I select "#{role}" from "site[memberships_attributes][2][role]"}
+end

features/step_definitions/site_steps.rb

@@ -5,6 +5,7 @@
 # - I have the site: "some site" set up with name: "Something", domain: "test2"
 #
 Given /^I have the site: "([^"]*)" set up(?: with #{capture_fields})?$/ do |site_factory, fields|
+  Thread.current[:site] = nil
   @site = FactoryGirl.create(site_factory, parse_fields(fields))
   @site.should_not be_nil
 

features/support/selectors.rb

@@ -22,6 +22,9 @@ module HtmlSelectorsHelpers
       
     when "the has many selector"
       ".has-many-selector ul li.template"
+
+    when 'the role'
+      '.role'
     
     # Add more mappings here.
     # Here is an example that pulls values out of the Regexp:

spec/models/ability_spec.rb

@@ -125,6 +125,15 @@ describe Ability do
       end
     end
 
+    context 'granting admin' do
+      it 'should allow only admins to grant admin role' do
+        should     allow_permission_from :grant_admin, @admin
+        should_not allow_permission_from :grant_admin, @designer
+        should_not allow_permission_from :grant_admin, @author
+      end
+
+    end
+
   end
 
 end