Update API to allow for granular permissions

This adds a new `:group` key to the permissions. All permissions that
share a `:group` can be updated at once by passing it a permission to
the `update` put request. For example, if you wanted to update all the
permissions under the `manage_wiki` group, you could use:

```
permissions[manage_wiki][enabled]=1
permissions[manage_wiki][explicit]=1
```

You can also update the permissions individually, by using the rest
api PUT as normal, for example:

```
permissions[manage_wiki_create][enabled]=1
permissions[manage_wiki_create][explicit]=1
```

If you were previously using the (now) group permission, after we
turn on granular permission it will for all intensive purposes still
work the same way, and can continue to be used without need to change
the API usage at all.

The only change for granular permissions is that you can no longer
change the locked status on granular permissions, you can only lock
or unlock permissions that have a group at the group level.

There is one other API change worth mentioning. If you attempt to
update several permission in a single API call now, and one of them
fails, none of the previously processed permissions will be saved.
They now all succeed or all fail.

Test Plan:
  - Enable the :granular_permissions_wiki_pages feature flag. This
    is just so that we have some granular permissions you can use
    to test things with
  - Make some manual API calls to update the permissions. See
    https://canvas.instructure.com/doc/api/roles.html (Update a role)
    for reference. Specifically check that:

    * All the permissions in a group can be updated in a single
      API call
    * Granular permissions can be updated individually
    * Grouped permissions can only be locked/unlocked at the group
      level
    * Granular permission can not be locked/unlocked individually
    * Non granular singular permissions (ex: read_reports) can still
      be locked/unlocked individually

fixes USERS-164

flag = granular_permissions_wiki_pages

Change-Id: I9695fde27724f72f8c762f0be8cb989d935e6f47
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/223993
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
Reviewed-by: Charley Kline <ckline@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: August Thornton <august@instructure.com>

app/controllers/role_overrides_controller.rb

@@ -365,9 +365,10 @@ class RoleOverridesController < ApplicationController
     end
 
     # allow setting permissions immediately through API
-    result = set_permissions_for(role, @context, params[:permissions])
-    unless result
-      return render json: {message: t('Permission must be enabled for someone')}, status: :bad_request
+    begin
+      set_permissions_for(role, @context, params[:permissions])
+    rescue BadPermissionSettingError => e
+      return render json: {message: e}, status: :bad_request
     end
 
     # Add base_role_type_label for this role
@@ -489,12 +490,13 @@ class RoleOverridesController < ApplicationController
         end
       end
     end
-    result = set_permissions_for(@role, @context, params[:permissions])
 
-    unless result
-      return render json: {message: t('Permission must be enabled for someone')}, status: :bad_request
+    begin
+      set_permissions_for(@role, @context, params[:permissions])
+      render :json => role_json(@context, @role, @current_user, session)
+    rescue BadPermissionSettingError => e
+      return render json: {message: e}, status: :bad_request
     end
-    render :json => role_json(@context, @role, @current_user, session)
   end
 
   def create
@@ -570,42 +572,70 @@ class RoleOverridesController < ApplicationController
   end
   protected :require_role
 
+  class BadPermissionSettingError < StandardError; end
+
   # Internal: Loop through and set permission on role given in params.
   #
   # role - The role to set permissions for.
   # context - The current context.
   # permissions - The permissions from the request params.
-  #
-  # Returns nothing.
   def set_permissions_for(role, context, permissions)
-    return true unless permissions.present?
+    return true if permissions.blank?
 
-    if role.course_role?
-      manageable_permissions = RoleOverride.manageable_permissions(context, role.base_role_type)
-    else
-      manageable_permissions = RoleOverride.manageable_permissions(context)
+    manageable_permissions =
+      if role.course_role?
+        RoleOverride.manageable_permissions(context, role.base_role_type)
+      else
+        RoleOverride.manageable_permissions(context)
+      end
+
+    # Hash of permission names and groups for easier updating. Make everything
+    # an array (even though only grouped permissions will have more then one
+    # element) so we can use the same logic for updating grouped and ungrouped permissions
+    grouped_permissions = Hash.new { |h, k| h[k] = [] }.with_indifferent_access
+    manageable_permissions.each do |permission_name, permission|
+      grouped_permissions[permission_name] << {name: permission_name, disable_locking: permission.key?(:group)}
+      if permission.key?(:group)
+        grouped_permissions[permission[:group]] << {name: permission_name, disable_locking: false}
+      end
     end
 
-    manageable_permissions.keys.each do |permission|
-      if settings = permissions[permission]
-        if !value_to_boolean(settings[:readonly])
-          if settings.has_key?(:enabled) && value_to_boolean(settings[:explicit])
-            override = value_to_boolean(settings[:enabled])
-          end
-          locked = value_to_boolean(settings[:locked]) if settings.key?(:locked)
-          applies_to_self = value_to_boolean(settings[:applies_to_self]) if settings.key? :applies_to_self
-          if settings.key? :applies_to_descendants
-            applies_to_descendants = value_to_boolean(settings[:applies_to_descendants])
-          end
+    RoleOverride.transaction do
+      permissions.each do |permission_or_group_name, permission_updates|
+        next if value_to_boolean(permission_updates[:readonly])
 
-          if applies_to_descendants == false && applies_to_self == false
-            return false
-          end
+        target_permissions = grouped_permissions[permission_or_group_name]
+        next if target_permissions.empty?
 
-          RoleOverride.manage_role_override(context, role, permission.to_s,
-                                            override: override, locked: locked,
-                                            applies_to_self: applies_to_self,
-                                            applies_to_descendants: applies_to_descendants)
+        if permission_updates.key?(:locked)
+          if target_permissions.any? { |permission| permission[:disable_locking] }
+            raise BadPermissionSettingError, t('Cannot change locked status on granular permission')
+          else
+            locked = value_to_boolean(permission_updates[:locked])
+          end
+        end
+
+        if permission_updates.key?(:enabled) && value_to_boolean(permission_updates[:explicit])
+          override = value_to_boolean(permission_updates[:enabled])
+        end
+
+        if permission_updates.key? :applies_to_self
+          applies_to_self = value_to_boolean(permission_updates[:applies_to_self])
+        end
+
+        if permission_updates.key? :applies_to_descendants
+          applies_to_descendants = value_to_boolean(permission_updates[:applies_to_descendants])
+        end
+
+        if applies_to_descendants == false && applies_to_self == false
+          raise BadPermissionSettingError, t('Permission must be enabled for someone')
+        end
+
+        target_permissions.each do |permission|
+          RoleOverride.manage_role_override(
+            context, role, permission[:name].to_s, override: override, locked: locked,
+            applies_to_self: applies_to_self, applies_to_descendants: applies_to_descendants
+          )
         end
       end
     end

app/models/role_override.rb

@@ -707,6 +707,7 @@ class RoleOverride < ActiveRecord::Base
         'AccountAdmin'
       ],
       :account_allows => lambda {|a| a.root_account.feature_enabled?(:granular_permissions_wiki_pages)},
+      :group => 'manage_wiki'
     },
     :manage_wiki_delete => {
       :label => lambda { t("Delete pages") },
@@ -727,6 +728,7 @@ class RoleOverride < ActiveRecord::Base
         'AccountAdmin'
       ],
       :account_allows => lambda {|a| a.root_account.feature_enabled?(:granular_permissions_wiki_pages)},
+      :group => 'manage_wiki'
     },
     :manage_wiki_update => {
       :label => lambda { t("Update pages") },
@@ -747,6 +749,7 @@ class RoleOverride < ActiveRecord::Base
         'AccountAdmin'
       ],
       :account_allows => lambda {|a| a.root_account.feature_enabled?(:granular_permissions_wiki_pages)},
+      :group => 'manage_wiki'
     },
     :moderate_forum => {
       :label => lambda { t('permissions.moderate_form', "Moderate discussions ( delete / edit other's posts, lock topics)") },
@@ -1153,8 +1156,9 @@ class RoleOverride < ActiveRecord::Base
       :explicit   => false,
       :base_role_type => base_role,
       :enrollment_type => role.name,
-      :role_id => role.id
+      :role_id => role.id,
     }
+    generated_permission[:group] = default_data[:group] if default_data[:group].present?
 
     if default_data[:account_only]
       # note: built-in roles don't have an account so we need to remember to send it in explicitly

lib/api/v1/role.rb

@@ -54,6 +54,6 @@ module Api::V1::Role
     permission[:prior_default] = !!permission[:prior_default]
     permission.delete(:prior_default) unless permission[:explicit]
     permission.slice(:enabled, :locked, :readonly, :explicit, :prior_default,
-                     :applies_to_descendants, :applies_to_self)
+                     :applies_to_descendants, :applies_to_self, :group)
   end
 end

spec/controllers/role_overrides_controller_spec.rb

@@ -58,6 +58,98 @@ describe RoleOverridesController do
     expect(@account.roles.where(name: 'NewRole').first).to be_inactive
   end
 
+  describe 'update' do
+    before :each do
+      @role_name = 'NewRole'
+      @permission = 'read_reports'
+      @role = @account.roles.build(:name => @role_name)
+      @role.base_role_type = Role::DEFAULT_ACCOUNT_TYPE
+      @role.workflow_state = 'active'
+      @role.save!
+    end
+
+    def update_permissions(permissions)
+      put('update', params: {account_id: @account.id, id: @role.id, permissions: permissions})
+    end
+
+    it 'should let you update a permission' do
+      update_permissions({@permission => { enabled: true, explicit: true }})
+      override = RoleOverride.last
+      expect(override.permission).to eq @permission
+      expect(override.role_id).to eq @role.id
+      expect(override.enabled).to eq true
+    end
+
+    it 'should return an error if the updated permission is invalid' do
+      resp = update_permissions({@permission => { applies_to_descendants: false, applies_to_self: false }})
+      expect(resp.status).to eq 400
+      expect(resp.body).to include("Permission must be enabled for someone")
+      expect(RoleOverride.count).to eq 0
+    end
+
+    it 'should not commit any changes if any permission update fails' do
+      updates = {
+        manage_students: { enabled: true, explicit: true },
+        read_reports: { applies_to_descendants: false, applies_to_self: false },
+      }
+      resp = update_permissions(updates)
+      expect(resp.status).to eq 400
+      expect(resp.body).to include("Permission must be enabled for someone")
+      expect(RoleOverride.count).to eq 0
+    end
+
+    describe 'grouped permissions' do
+      before :each do
+        @account.root_account.enable_feature!(:granular_permissions_wiki_pages)
+        @grouped_permission = 'manage_wiki'
+        @granular_permissions = ['manage_wiki_create', 'manage_wiki_delete', 'manage_wiki_update']
+      end
+
+      it 'should update all permissions in a group' do
+        update_permissions({@grouped_permission => { enabled: true, explicit: true }})
+        expect(RoleOverride.count).to eq 3
+        expect(RoleOverride.pluck(:permission)).to match_array @granular_permissions
+        expect(RoleOverride.pluck(:role_id)).to match_array Array.new(3, @role.id)
+        expect(RoleOverride.pluck(:enabled)).to match_array Array.new(3, true)
+      end
+
+      it 'should allow locking all permissions in a group' do
+        update_permissions({@grouped_permission => { locked: true }})
+        expect(RoleOverride.count).to eq 3
+        expect(RoleOverride.pluck(:locked)).to match_array Array.new(3, true)
+      end
+
+      it 'should allow updating an individual permissions that belongs to a group' do
+        update_permissions({@granular_permissions[0] => { enabled: true, explicit: true }})
+        expect(RoleOverride.count).to eq 1
+
+        override = RoleOverride.last
+        expect(override.permission).to eq @granular_permissions[0]
+        expect(override.role_id).to eq @role.id
+        expect(override.enabled).to eq true
+      end
+
+      it 'should not allow locking an individual permissions that belongs to a group' do
+        resp = update_permissions({@granular_permissions[0] => { locked: true }})
+        expect(resp.status).to eq 400
+        expect(resp.body).to include("Cannot change locked status on granular permission")
+        expect(RoleOverride.count).to eq 0
+      end
+
+      it 'should handle updating a group and individual permission in the same group in one request' do
+        updates = {
+          @grouped_permission => { enabled: true, explicit: true },
+          @granular_permissions[0] => { enabled: false, explicit: true },
+        }
+        update_permissions(updates)
+        expect(RoleOverride.count).to eq 3
+        expect(RoleOverride.pluck(:permission)).to match_array @granular_permissions
+        expect(RoleOverride.pluck(:role_id)).to match_array Array.new(3, @role.id)
+        expect(RoleOverride.pluck(:enabled)).to match_array [true, true, false]
+      end
+    end
+  end
+
   describe "create" do
     before :each do
       @role_name = 'NewRole'