ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fixup docviewer audit event params 

This patchset will reconfigure the /docviewer_audit_events endpoint
to expect params for the created resource to be keyed by
'docviewer_audit_event'. The rational for this change is that we
shouldn't use strong params on params that aren't part of the created
resource. Their presence is already verified by `find()` type errors
like ActiveRecord::RecordNotFound. Also, having all the params flattened
confuses which params are for resource creation and which are not.

closes: GRADE-1595

test plan:
 - POST /submissions/:submission_id/docviewer_audit_events now looks
 body now looks like:

    {
      docviewer_audit_event: {
        annotation_body: {
          color: '',
          content: '',
          created_at: '',
          modified_at: '',
          page: '',
          type: ''
        },
        event_type: 'highlight_created',
        related_annotation_id: '123'
      },
      token: 'sekret',
      canvas_user_id: '456',
      document_id: '789'
    }

 - referncing an invalid submission_id raises a record not found response with
   the message "The specified resource does not exist.'
 - omitting the document_id return a a record not found response with
   the message "The specified resource does not exist.'
 - omitting the canvas_user_id returns a record not found response with
   the message "The specified resource does not exist.'
 - omitting the token returns an unauthorized response with the
   message "JWT signature invalid"

Change-Id: I9ad086c82218f46cd623fde20d1a62efa3110b1a
Reviewed-on: https://gerrit.instructure.com/163776
Reviewed-by: Gary Mei <gmei@instructure.com>
QA-Review: Gary Mei <gmei@instructure.com>
Reviewed-by: Keith T. Garner <kgarner@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
Tested-by: Jenkins
This commit is contained in:
Derek Bender 2018-09-10 11:55:37 -05:00
parent a7f8b6143a
commit 01566c20f8
2 changed files with 72 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
app/controllers/docviewer_audit_events_controller.rb
View File

@ -18,7 +18,6 @@
class DocviewerAuditEventsController < ApplicationController
skip_before_action :verify_authenticity_token
before_action :check_params
before_action :check_jwt_token
def create
@ -42,22 +41,20 @@ class DocviewerAuditEventsController < ApplicationController
event = AnonymousOrModerationEvent.new(
assignment: assignment,
canvadoc: canvadoc,
event_type: 'docviewer_' + params[:event_type],
payload: {
annotation_body: params[:annotation_body].permit(:color, :content, :created_at, :modified_at, :page, :type),
related_annotation_id: params[:related_annotation_id]
},
event_type: "docviewer_#{docviewer_audit_event_params[:event_type]}",
submission: submission,
user: user
user: user,
payload: {
annotation_body: docviewer_audit_event_params[:annotation_body],
related_annotation_id: docviewer_audit_event_params[:related_annotation_id]
},
)
respond_to do |format|
if event.save
return render json: event.as_json, status: :ok
format.json { render json: event.as_json, status: :ok }
else
format.json do
render json: event.errors.as_json.merge(message: 'Invalid param values'), status: :unprocessable_entity
end
format.json { render json: event.errors, status: :unprocessable_entity }
end
end
end
@ -71,19 +68,17 @@ class DocviewerAuditEventsController < ApplicationController
end
def check_jwt_token
Canvas::Security.decode_jwt(params[:token], [Canvadoc.jwt_secret])
Canvas::Security.decode_jwt(params[:token], [Canvadoc.jwt_secret])
rescue
return render json: {message: 'JWT signature invalid'}, status: :unauthorized
return render json: {message: 'JWT signature invalid'}, status: :unauthorized
end
def check_params
required_params = %i[annotation_body token canvas_user_id document_id event_type submission_id]
begin
params.require(required_params)
rescue ActionController::ParameterMissing => error
return render json: {message: error.to_s}, status: :bad_request
end
def docviewer_audit_event_params
params.require(:docviewer_audit_event).permit(
:event_type,
:related_annotation_id,
annotation_body: %i[color content created_at modified_at page type]
)
end
def canvadoc_from_submission(submission, document_id)
@ -96,6 +91,6 @@ class DocviewerAuditEventsController < ApplicationController
end
end
raise ActiveRecord::RecordNotFound.new('No canvadoc with given document id was found for this submission')
raise ActiveRecord::RecordNotFound, 'No canvadoc with given document id was found for this submission'
end
end

105
spec/controllers/docviewer_audit_events_controller_spec.rb
View File

@ -43,19 +43,21 @@ describe DocviewerAuditEventsController do
let(:default_params) do
{
annotation_body: {
color: '',
content: '',
created_at: '',
modified_at: '',
page: '',
type: '',
docviewer_audit_event: {
annotation_body: {
color: '',
content: '',
created_at: '',
modified_at: '',
page: '',
type: ''
},
event_type: 'highlight_created',
related_annotation_id: 1
},
token: Canvas::Security.create_jwt({}, nil, @secret, :HS512),
canvas_user_id: @teacher.id,
document_id: @attachment.canvadoc.document_id,
event_type: 'highlight_created',
related_annotation_id: 1,
submission_id: @submission.id
}
end
@ -64,31 +66,24 @@ describe DocviewerAuditEventsController do
it 'renders status unauthorized if not passed a correct jwt auth token' do
assignment = Assignment.create!(course: @course, name: 'anonymous', anonymous_grading: true)
@submission = assignment.submit_homework(@student, submission_type: 'online_upload', attachments: [@attachment])
post :create, format: :json, params: default_params.merge({token: 'wrong token'})
post :create, format: :json, params: default_params.merge(token: 'wrong token')
expect(response).to have_http_status(:unauthorized)
end
it 'explains if not passed a correct jwt auth token' do
assignment = Assignment.create!(course: @course, name: 'anonymous', anonymous_grading: true)
@submission = assignment.submit_homework(@student, submission_type: 'online_upload', attachments: [@attachment])
post :create, format: :json, params: default_params.merge({token: 'wrong token'})
expect(JSON.parse(response.body)['message']).to eq 'JWT signature invalid'
post :create, format: :json, params: default_params.merge(token: 'wrong token')
expect(JSON.parse(response.body).fetch('message')).to eq 'JWT signature invalid'
end
it 'renders status bad_request if param values are missing' do
assignment = Assignment.create!(course: @course, name: 'anonymous', anonymous_grading: true)
@submission = assignment.submit_homework(@student, submission_type: 'online_upload', attachments: [@attachment])
post :create, format: :json, params: default_params.except(:event_type)
post :create, format: :json, params: default_params.except(:docviewer_audit_event)
expect(response).to have_http_status(:bad_request)
end
it 'explains that the param values are missing, when rendering status bad_request' do
assignment = Assignment.create!(course: @course, name: 'anonymous', anonymous_grading: true)
@submission = assignment.submit_homework(@student, submission_type: 'online_upload', attachments: [@attachment])
post :create, format: :json, params: default_params.except(:event_type)
expect(JSON.parse(response.body)['message']).to eq 'param is missing or the value is empty: event_type'
end
it 'renders status not_acceptable for a non-moderated, non-anonymous assignment' do
assignment = Assignment.create!(course: @course, name: 'non-moderated and non-anonymous')
@submission = assignment.submit_homework(@student, submission_type: 'online_upload', attachments: [@attachment])
@ -100,7 +95,7 @@ describe DocviewerAuditEventsController do
assignment = Assignment.create!(course: @course, name: 'non-moderated and non-anonymous')
@submission = assignment.submit_homework(@student, submission_type: 'online_upload', attachments: [@attachment])
post :create, format: :json, params: default_params
expect(JSON.parse(response.body)['message']).to eq 'Assignment is neither anonymous nor moderated'
expect(JSON.parse(response.body).fetch('message')).to eq 'Assignment is neither anonymous nor moderated'
end
context 'for a moderated assignment' do
@ -113,7 +108,7 @@ describe DocviewerAuditEventsController do
final_grader: @teacher
)
@submission = assignment.submit_homework(@student, submission_type: 'online_upload', attachments: [@attachment])
post :create, format: :json, params: default_params.merge({canvas_user_id: @first_ta.id})
post :create, format: :json, params: default_params.merge(canvas_user_id: @first_ta.id)
expect(response).to have_http_status(:ok)
end
@ -141,7 +136,7 @@ describe DocviewerAuditEventsController do
)
@submission = assignment.submit_homework(@student, submission_type: 'online_upload', attachments: [@attachment])
assignment.grade_student(@student, grade: 10, grader: @first_ta, provisional: true)
post :create, format: :json, params: default_params.merge({canvas_user_id: @second_ta.id})
post :create, format: :json, params: default_params.merge(canvas_user_id: @second_ta.id)
expect(response).to have_http_status(:forbidden)
end
@ -155,8 +150,8 @@ describe DocviewerAuditEventsController do
)
@submission = assignment.submit_homework(@student, submission_type: 'online_upload', attachments: [@attachment])
assignment.grade_student(@student, grade: 10, grader: @first_ta, provisional: true)
post :create, format: :json, params: default_params.merge({canvas_user_id: @second_ta.id})
expect(JSON.parse(response.body)['message']).to eq 'Reached maximum number of graders for assignment'
post :create, format: :json, params: default_params.merge(canvas_user_id: @second_ta.id)
expect(JSON.parse(response.body).fetch('message')).to eq 'Reached maximum number of graders for assignment'
end
end
@ -173,9 +168,9 @@ describe DocviewerAuditEventsController do
it 'allows students to annotate, if assignment is anonymous or moderated' do
assignment = Assignment.create!(course: @course, name: 'anonymous', anonymous_grading: true)
@submission = assignment.submit_homework(@student, submission_type: 'online_upload', attachments: [@attachment])
post :create, format: :json, params: default_params.merge({canvas_user_id: @student.id})
events = AnonymousOrModerationEvent.where(assignment: assignment, submission: @submission)
expect(events.count).to be 1
expect {
post :create, format: :json, params: default_params.merge(canvas_user_id: @student.id)
}.to change { AnonymousOrModerationEvent.where(assignment: assignment, submission: @submission).count }.by(1)
end
it 'allows fake students to annotate, if assignment is anonymous or moderated' do
@ -184,9 +179,12 @@ describe DocviewerAuditEventsController do
doc = Canvadoc.create!(document_id: "abc123#{attachment.id}", attachment_id: attachment.id)
assignment = Assignment.create!(course: @course, name: 'anonymous', anonymous_grading: true)
@submission = assignment.submit_homework(fake_student, submission_type: 'online_upload', attachments: [attachment])
post :create, format: :json, params: default_params.merge({canvas_user_id: fake_student.id, document_id: doc.document_id})
events = AnonymousOrModerationEvent.where(assignment: assignment, submission: @submission)
expect(events.count).to be 1
params = default_params.merge(canvas_user_id: fake_student.id, document_id: doc.document_id)
expect {
post :create, format: :json, params: params
}.to change {
AnonymousOrModerationEvent.where(assignment: assignment, submission: @submission).count
}.by(1)
end
context "as an admin" do
@ -202,19 +200,19 @@ describe DocviewerAuditEventsController do
@assignment.grade_student(@student, grade: 10, grader: @first_ta, provisional: true)
end
def annotate_as_admin
post :create, format: :json, params: default_params.merge({canvas_user_id: account_admin_user.id})
subject(:annotate_as_admin) do
-> { post :create, format: :json, params: default_params.merge(canvas_user_id: account_admin_user.id) }
end
it "can annotate even if there are no slots available" do
@assignment.update!(grader_count: 1)
expect{ annotate_as_admin }.to change {
is_expected.to change {
AnonymousOrModerationEvent.where(assignment: @assignment, submission: @submission).count
}.by(1)
end
it "does not occupy a slot when annotating" do
expect{ annotate_as_admin }.not_to change { @assignment.provisional_moderation_graders.count }
is_expected.not_to change { @assignment.provisional_moderation_graders.count }
end
end
@ -228,7 +226,7 @@ describe DocviewerAuditEventsController do
)
existing_grader = assignment.moderation_graders.create!(user: @first_ta, anonymous_id: '12345', slot_taken: false)
@submission = assignment.submit_homework(@student, submission_type: 'online_upload', attachments: [@attachment])
post :create, format: :json, params: default_params.merge({canvas_user_id: @first_ta.id})
post :create, format: :json, params: default_params.merge(canvas_user_id: @first_ta.id)
expect(existing_grader.reload.slot_taken).to be true
end
@ -239,9 +237,11 @@ describe DocviewerAuditEventsController do
@submission = assignment.submit_homework(@student, submission_type: 'online_upload', attachments: [@attachment])
@submission.update!(submitted_at: 1.hour.ago)
@submission = assignment.submit_homework(@student, submission_type: 'online_upload', attachments: [second_attachment])
post :create, format: :json, params: default_params.merge({canvas_user_id: @teacher.id})
events = AnonymousOrModerationEvent.where(assignment: assignment, submission: @submission)
expect(events.count).to be 1
expect {
post :create, format: :json, params: default_params.merge(canvas_user_id: @teacher.id)
}.to change {
AnonymousOrModerationEvent.where(assignment: assignment, submission: @submission).count
}.by(1)
end
it 'creates a moderation grader' do
@ -253,7 +253,7 @@ describe DocviewerAuditEventsController do
final_grader: @teacher
)
@submission = assignment.submit_homework(@student, submission_type: 'online_upload', attachments: [@attachment])
post :create, format: :json, params: default_params.merge({canvas_user_id: @first_ta.id})
post :create, format: :json, params: default_params.merge(canvas_user_id: @first_ta.id)
expect(assignment.moderation_graders.pluck(:user_id)).to include @first_ta.id
end
@ -282,31 +282,36 @@ describe DocviewerAuditEventsController do
@submission = assignment.submit_homework(@student, submission_type: 'online_upload', attachments: [@attachment])
assignment.grade_student(@student, grade: 10, grader: @teacher, provisional: true)
assignment.update!(grades_published_at: Time.zone.now)
post :create, format: :json, params: default_params.merge({canvas_user_id: @first_ta.id})
events = AnonymousOrModerationEvent.where(assignment: assignment, canvadoc: @attachment.canvadoc, submission: @submission)
expect(events.count).to be 1
expect {
post :create, format: :json, params: default_params.merge(canvas_user_id: @first_ta.id)
}.to change {
AnonymousOrModerationEvent.where(assignment: assignment, canvadoc: @attachment.canvadoc, submission: @submission).count
}.by(1)
end
it 'creates an AnonymousOrModerationEvent' do
assignment = Assignment.create!(course: @course, anonymous_grading: true, name: 'anonymous')
@submission = assignment.submit_homework(@student, submission_type: 'online_upload', attachments: [@attachment])
post :create, format: :json, params: default_params
events = AnonymousOrModerationEvent.where(assignment: assignment, canvadoc: @attachment.canvadoc, submission: @submission)
expect(events.count).to be 1
expect {
post :create, format: :json, params: default_params
}.to change {
AnonymousOrModerationEvent.where(assignment: assignment, canvadoc: @attachment.canvadoc, submission: @submission).count
}.by(1)
end
it 'saves a copy of the annotation_body in the payload' do
assignment = Assignment.create!(course: @course, anonymous_grading: true, name: 'anonymous')
@submission = assignment.submit_homework(@student, submission_type: 'online_upload', attachments: [@attachment])
post :create, format: :json, params: default_params.merge({annotation_body: {type: 'a type'}})
post :create, format: :json, params: default_params.deep_merge(docviewer_audit_event: { annotation_body: { type: 'a type' } })
event = AnonymousOrModerationEvent.find_by!(assignment: assignment, canvadoc: @attachment.canvadoc, submission: @submission)
expect(event.payload['annotation_body']['type']).to eq 'a type'
type = event.payload.fetch('annotation_body').fetch('type')
expect(type).to eq 'a type'
end
it 'saves the related_annotation_id in the payload' do
assignment = Assignment.create!(course: @course, anonymous_grading: true, name: 'anonymous')
@submission = assignment.submit_homework(@student, submission_type: 'online_upload', attachments: [@attachment])
post :create, format: :json, params: default_params.merge({related_annotation_id: 23})
post :create, format: :json, params: default_params.deep_merge(docviewer_audit_event: { related_annotation_id: 23 })
event = AnonymousOrModerationEvent.find_by!(assignment: assignment, canvadoc: @attachment.canvadoc, submission: @submission)
expect(event.payload['related_annotation_id']).to eq '23'
end
@ -316,7 +321,7 @@ describe DocviewerAuditEventsController do
@submission = assignment.submit_homework(@student, submission_type: 'online_upload', attachments: [@attachment])
post :create, format: :json, params: default_params
event = AnonymousOrModerationEvent.find_by!(assignment: assignment, canvadoc: @attachment.canvadoc, submission: @submission)
expect(JSON.parse(response.body)['anonymous_or_moderation_event']['id'].to_i).to be event.id
expect(JSON.parse(response.body).fetch('anonymous_or_moderation_event').fetch('id')).to eq event.id
end
it "is okay if related_annotation_id is not passed" do