Fix 401 errors on adding rubrics to an assignment

closes OUT-1482

test plan:
  - start up canvas
  - on the account permission page
    (http://canvas.docker/accounts/1/permissions),
    disable "Create and edit assessing rubrics" permission
    and enable "Manage (add / edit / delete) assignments and quizzes"
    for the "Teacher" role
  - create a teacher account in a course
  - as a teacher, confirm with two different assignments that:
    - after creating an assignment, you can create a rubric
      on that assignment
    - after creating an assignment, you can add a rubric
      to an assignment by using the "Find Rubric" button

Change-Id: I99fa38bc41755f307955218004eb6b4457c0463e
Reviewed-on: https://gerrit.instructure.com/126624
Tested-by: Jenkins
Reviewed-by: Michael Brewer-Davis <mbd@instructure.com>
QA-Review: Andrew Porter <hporter-c@instructure.com>
Product-Review: Pert Eilers <peilers@instructure.com>

app/controllers/rubric_associations_controller.rb

@@ -32,22 +32,23 @@ class RubricAssociationsController < ApplicationController
     rubric_id = association_params.delete(:rubric_id)
     @rubric = @association ? @association.rubric : Rubric.find(rubric_id)
     # raise "User doesn't have access to this rubric" unless @rubric.grants_right?(@current_user, session, :read)
-    if !@association && !authorized_action(@context, @current_user, :manage_rubrics)
-      return
-    elsif !@association || authorized_action(@association, @current_user, :update)
-      if params[:rubric] && @rubric.grants_right?(@current_user, session, :update)
-        @rubric.update_criteria(params[:rubric])
-      end
-      association_params[:association_object] = @association.association_object if @association
-      association_params[:association_object] ||= @association_object
-      association_params[:id] = @association.id if @association
-      @association = RubricAssociation.generate(@current_user, @rubric, @context, association_params)
-      json_res = {
-        :rubric => @rubric.as_json(:methods => :criteria, :include_root => false, :permissions => {:user => @current_user, :session => session}),
-        :rubric_association => @association.as_json(:include_root => false, :include => [:rubric_assessments, :assessment_requests], :permissions => {:user => @current_user, :session => session})
-      }
-      render :json => json_res
+    return unless can_manage_rubrics_or_association_object?(@assocation, @association_object)
+    return unless can_update_association?(@association)
+    if params[:rubric] && @rubric.grants_right?(@current_user, session, :update)
+      @rubric.update_criteria(params[:rubric])
     end
+    association_params[:association_object] = @association.association_object if @association
+    association_params[:association_object] ||= @association_object
+    association_params[:id] = @association.id if @association
+    @association = RubricAssociation.generate(@current_user, @rubric, @context, association_params)
+    json_res = {
+      :rubric => @rubric.as_json(:methods => :criteria, :include_root => false, :permissions => {:user => @current_user,
+                                                                                                 :session => session}),
+      :rubric_association => @association.as_json(:include_root => false,
+                                                  :include => %i{rubric_assessments assessment_requests},
+                                                  :permissions => {:user => @current_user, :session => session})
+    }
+    render :json => json_res
   end
 
   def destroy
@@ -65,4 +66,19 @@ class RubricAssociationsController < ApplicationController
       render :json => @association
     end
   end
+
+  private
+
+  def can_manage_rubrics_or_association_object?(association, association_object)
+    return true if association ||
+                   @context.grants_right?(@current_user, session, :manage_rubrics) ||
+                   association_object && association_object.grants_right?(@current_user, session, :update)
+    render_unauthorized_action
+    false
+  end
+
+  def can_update_association?(association)
+    !association || authorized_action(association, @current_user, :update)
+  end
+
 end

app/controllers/rubrics_controller.rb

@@ -103,7 +103,7 @@ class RubricsController < ApplicationController
 
     @association_object = RubricAssociation.get_association_object(params[:rubric_association])
     params[:rubric][:user] = @current_user if params[:rubric]
-    if (!@association_object || authorized_action(@association_object, @current_user, :read)) && authorized_action(@context, @current_user, :manage_rubrics)
+    if can_manage_rubrics_or_association_object?(@association_object)
       @association = @context.rubric_associations.where(id: params[:rubric_association_id]).first if params[:rubric_association_id].present?
       @association_object ||= @association.association_object if @association
       association_params[:association_object] = @association_object
@@ -159,4 +159,25 @@ class RubricsController < ApplicationController
     outcome_group_json(root_outcome, @current_user, session)
   end
   protected :get_root_outcome
+
+  private
+
+  def can_manage_rubrics_or_association_object?(object)
+    return true if object && (can_update?(object) || can_read?(object) && can_manage_rubrics_context?) ||
+                   !object && can_manage_rubrics_context?
+    render_unauthorized_action
+    false
+  end
+
+  def can_update?(object)
+    object.grants_right?(@current_user, session, :update)
+  end
+
+  def can_read?(object)
+    object.grants_right?(@current_user, session, :read)
+  end
+
+  def can_manage_rubrics_context?
+    @context.grants_right?(@current_user, session, :manage_rubrics)
+  end
 end

spec/controllers/rubric_associations_controller_spec.rb

@@ -29,11 +29,28 @@ describe RubricAssociationsController do
     it "should assign variables" do
       course_with_teacher_logged_in(:active_all => true)
       rubric_association_model(:user => @user, :context => @course)
-      post 'create', params: {:course_id => @course.id, :rubric_association => {:rubric_id => @rubric.id, :title => "some association", :association_type => @rubric_association.association_object.class.name, :association_id => @rubric_association.association_object.id}}
+      post 'create', params: {:course_id => @course.id,
+                              :rubric_association => {:rubric_id => @rubric.id,
+                                                      :title => "some association",
+                                                      :association_type =>
+                                                        @rubric_association.association_object.class.name,
+                                                      :association_id => @rubric_association.association_object.id}}
       expect(assigns[:association]).not_to be_nil
       expect(assigns[:association].title).to eql("some association")
       expect(response).to be_success
     end
+    it "should create without manager_rubrics permission" do
+      course_with_teacher_logged_in(:active_all => true)
+      @course.account.role_overrides.create! :role => teacher_role, :permission => 'manage_rubrics', :enabled => false
+      rubric_association_model(:user => @user, :context => @course)
+      post 'create', params: {:course_id => @course.id,
+                              :rubric_association => {:rubric_id => @rubric.id,
+                                                      :title => "some association",
+                                                      :association_type =>
+                                                        @rubric_association.association_object.class.name,
+                                                      :association_id => @rubric_association.association_object.id}}
+      expect(response).to be_success
+    end
   end
 
   describe "PUT 'update'" do

spec/controllers/rubrics_controller_spec.rb

@@ -62,7 +62,25 @@ describe RubricsController do
       course_with_teacher_logged_in(:active_all => true)
       association = @course.assignments.create!(assignment_valid_attributes)
       request.content_type = 'application/json'
-      post 'create', params: {:course_id => @course.id, :rubric => {}, :rubric_association => {:association_type => association.class.to_s, :association_id => association.id}}
+      post 'create', params: {:course_id => @course.id,
+                              :rubric => {},
+                              :rubric_association => {:association_type => association.class.to_s,
+                                                      :association_id => association.id}}
+      expect(assigns[:rubric]).not_to be_nil
+      expect(assigns[:rubric]).not_to be_new_record
+      expect(assigns[:rubric].rubric_associations.length).to eql(1)
+      expect(response).to be_success
+    end
+
+    it "should create an association if specified without manage_rubrics permission " do
+      course_with_teacher_logged_in(:active_all => true)
+      allow(@course).to receive(:grants_any_rights?).and_return(false)
+      association = @course.assignments.create!(assignment_valid_attributes)
+      request.content_type = 'application/json'
+      post 'create', params: {:course_id => @course.id,
+                              :rubric => {},
+                              :rubric_association => {:association_type => association.class.to_s,
+                                                      :association_id => association.id}}
       expect(assigns[:rubric]).not_to be_nil
       expect(assigns[:rubric]).not_to be_new_record
       expect(assigns[:rubric].rubric_associations.length).to eql(1)