prevent non-students from taking quizzes

admins shouldn't be able to take quizzes unless
they're students or the course is public.

fixes CNVS-28380

test plan:
- check that the api for retrieving quizzes
  returns locked_for_user = true and lock_info
  when fetching quizzes as a teacher admin, but
  returns locked_for_user = false when fetching
  quizzes as a student
- check that the api for creating a quiz
  submission doesn't allow you to as a teacher
  or admin, but does allow you to as a student
- make sure that students can still take quizzes
- make sure that teachers and admins can still
  preview quizzes
- make sure that teachers and admins cannot take
  quizzes unless they are also students
- make sure teachers and admins can take quizzes
  in public courses
- students should still be able to take a quiz in
  any cases where they could take a quiz in a
  concluded course or with a completed enrollment
  (that is one where their section has ended)

Change-Id: Ia52ef6d4efd192e8ebd6ae91acb7a6308c08bd9b
Reviewed-on: https://gerrit.instructure.com/79418
Reviewed-by: Davis McClellan <dmcclellan@instructure.com>
Tested-by: Jenkins
QA-Review: Indira Pai <ipai@instructure.com>
Product-Review: Jason Sparks <jsparks@instructure.com>
This commit is contained in:
Ben Rinaca 2016-05-12 09:48:21 -06:00
parent a532d3f159
commit 9d8df61450
6 changed files with 415 additions and 195 deletions

View File

@ -245,13 +245,13 @@ class Quizzes::QuizSubmissionsApiController < ApplicationController
# "quiz_submissions": [QuizSubmission]
# }
def create
if module_locked?
raise RequestError.new("you are not allowed to participate in this quiz", 400)
end
quiz_submission = if previewing?
@service.create_preview(@quiz, session)
else
if module_locked?
raise RequestError.new("you are not allowed to participate in this quiz", 400)
end
@service.create(@quiz)
end
@ -402,8 +402,7 @@ class Quizzes::QuizSubmissionsApiController < ApplicationController
private
def module_locked?
@locked_reason = @quiz.locked_for?(@current_user, :check_policies => true, :deep_check_if_needed => true)
@locked_reason && !@quiz.grants_right?(@current_user, session, :update)
@quiz.locked_for?(@current_user, :check_policies => true, :deep_check_if_needed => true)
end
def previewing?

View File

@ -721,36 +721,39 @@ class Quizzes::Quiz < ActiveRecord::Base
def locked_for?(user, opts={})
return false if opts[:check_policies] && self.grants_right?(user, :update)
::Rails.cache.fetch(locked_cache_key(user), :expires_in => 1.minute) do
locked = false
user_submission = user && quiz_submissions.where(user_id: user.id).first
return false if user_submission && user_submission.manually_unlocked
quiz_for_user = self.overridden_for(user)
if (quiz_for_user.unlock_at && quiz_for_user.unlock_at > Time.now)
sub = user && quiz_submissions.where(user_id: user).first
if !sub || !sub.manually_unlocked
locked = {:asset_string => self.asset_string, :unlock_at => quiz_for_user.unlock_at}
end
elsif (quiz_for_user.lock_at && quiz_for_user.lock_at <= Time.now)
sub = user && quiz_submissions.where(user_id: user).first
if !sub || !sub.manually_unlocked
locked = {:asset_string => self.asset_string, :lock_at => quiz_for_user.lock_at, :can_view => true}
end
elsif !opts[:skip_assignment] && (self.for_assignment? && l = self.assignment.locked_for?(user, opts))
sub = user && quiz_submissions.where(user_id: user).first
if !sub || !sub.manually_unlocked
locked = l
end
elsif item = locked_by_module_item?(user, opts[:deep_check_if_needed])
sub = user && quiz_submissions.where(user_id: user).first
if !sub || !sub.manually_unlocked
locked = {:asset_string => self.asset_string, :context_module => item.context_module.attributes}
end
unlock_time_not_yet_reached = quiz_for_user.unlock_at && quiz_for_user.unlock_at > Time.zone.now
lock_time_already_occurred = quiz_for_user.lock_at && quiz_for_user.lock_at <= Time.zone.now
locked = false
lock_info = { asset_string: asset_string }
if unlock_time_not_yet_reached
locked = lock_info.merge({ unlock_at: quiz_for_user.unlock_at })
elsif lock_time_already_occurred
locked = lock_info.merge({ lock_at: quiz_for_user.lock_at, can_view: true })
elsif !opts[:skip_assignment] && (assignment_lock = locked_by_assignment?(user, opts))
locked = assignment_lock
elsif (module_lock = locked_by_module_item?(user, opts[:deep_check_if_needed]))
locked = lock_info.merge({ context_module: module_lock.context_module.attributes })
elsif !context.try_rescue(:is_public) && !context.grants_right?(user, :participate_as_student)
locked = lock_info.merge({ missing_permission: :participate_as_student.to_s })
end
locked
end
end
def locked_by_assignment?(user, opts = {})
return false unless for_assignment?
assignment.locked_for?(user, opts)
end
def clear_locked_cache(user)
super
Rails.cache.delete(assignment.locked_cache_key(user)) if self.for_assignment?

View File

@ -320,16 +320,16 @@
<div class="lock_explanation">
<%= t(:only_registered_users, "Only registered, enrolled users can take graded quizzes") %>
</div>
<% elsif @locked && !can_do(@quiz, @current_user, :update) %>
<div class="lock_explanation">
<%= lock_explanation(@locked_reason, 'quiz', @context) %>
</div>
<% elsif @quiz.context.soft_concluded? %>
<div class="alert alert-danger lock_explanation" role="alert">
<%= t(:course_concluded_notice,
"This quiz is no longer available as the course has been concluded.")
%>
</div>
<% elsif @locked && !can_do(@quiz, @current_user, :update) %>
<div class="lock_explanation">
<%= lock_explanation(@locked_reason, 'quiz', @context) %>
</div>
<% end %>
<% end %>
<% elsif !can_do(@quiz, @current_user, :update) %>

View File

@ -373,100 +373,132 @@ describe Quizzes::QuizSubmissionsApiController, type: :request do
end
describe 'POST /courses/:course_id/quizzes/:quiz_id/submissions [create]' do
before :once do
enroll_student
context 'as a teacher' do
it 'should create a preview quiz submission' do
json = qs_api_create false, { preview: true }
expect(Quizzes::QuizSubmission.find(json['quiz_submissions'][0]['id']).preview?).to be_truthy
end
end
it 'should create a quiz submission' do
json = qs_api_create
expect(json.key?('quiz_submissions')).to be_truthy
expect(json['quiz_submissions'].length).to eq 1
expect(json['quiz_submissions'][0]['workflow_state']).to eq 'untaken'
end
it 'should create a preview quiz submission' do
json = qs_api_create false, { preview: true }
expect(Quizzes::QuizSubmission.find(json['quiz_submissions'][0]['id']).preview?).to be_truthy
end
it 'should allow the creation of multiple, subsequent QSes' do
@quiz.allowed_attempts = -1
@quiz.save
json = qs_api_create
qs = Quizzes::QuizSubmission.find(json['quiz_submissions'][0]['id'])
qs.mark_completed
qs.save
qs_api_create
end
context 'access validations' do
include_examples 'Quiz Submissions API Restricted Endpoints'
before :each do
@request_proxy = method(:qs_api_create)
context 'as a student' do
before :once do
enroll_student
@user = @student
end
it 'should reject creating a QS when one already exists' do
qs_api_create
qs_api_create(true)
expect(response.status.to_i).to eq 409
it 'should create a quiz submission' do
json = qs_api_create
expect(json.key?('quiz_submissions')).to be_truthy
expect(json['quiz_submissions'].length).to eq 1
expect(json['quiz_submissions'][0]['workflow_state']).to eq 'untaken'
end
it 'should respect the number of allowed attempts' do
it 'should allow the creation of multiple, subsequent QSes' do
@quiz.allowed_attempts = -1
@quiz.save
json = qs_api_create
qs = Quizzes::QuizSubmission.find(json['quiz_submissions'][0]['id'])
qs.mark_completed
qs.save!
qs.save
qs_api_create(true)
expect(response.status.to_i).to eq 409
end
end
context "unpublished module quiz" do
before do
student_in_course(active_all: true)
@quiz = @course.quizzes.create! title: "Test Quiz w/ Module"
@quiz.quiz_questions.create!(
{question_data:
{name: 'question', points_possible: 1, question_type: 'multiple_choice_question',
'answers' => [{'answer_text' => '1', 'weight' => '100'}, {'answer_text' => '2'}, {'answer_text' => '3'}, {'answer_text' => '4'}]
}
})
@quiz.published_at = Time.now
@quiz.workflow_state = 'available'
@quiz.save!
@pre_module = @course.context_modules.create!(:name => 'pre_module')
# No meaning in this URL
@tag = @pre_module.add_item(:type => 'external_url', :url => 'http://example.com', :title => 'example')
@tag.publish! if @tag.unpublished?
@pre_module.completion_requirements = { @tag.id => { :type => 'must_view' } }
@pre_module.save!
locked_module = @course.context_modules.create!(:name => 'locked_module', :require_sequential_progress => true)
item_tag = locked_module.add_item(:id => @quiz.id, :type => 'quiz')
locked_module.prerequisites = "module_#{@pre_module.id}"
locked_module.save!
qs_api_create
end
it "shouldn't allow access to quiz until module is completed" do
expect(@quiz.grants_right?(@student, :submit)).to be_truthy # precondition
json = api_call(:post, "/api/v1/courses/#{@course.id}/quizzes/#{@quiz.id}/submissions",
{controller: "quizzes/quiz_submissions_api", action: "create", format: "json", course_id: "#{@course.id}", quiz_id: "#{@quiz.id}"}, {},
{'Accept' => 'application/vnd.api+json'}, {expected_status: 400})
expect(json['status']).to eq "bad_request"
context 'access validations' do
include_examples 'Quiz Submissions API Restricted Endpoints'
before :each do
@request_proxy = method(:qs_api_create)
end
it 'should reject creating a QS when one already exists' do
qs_api_create
qs_api_create(true)
expect(response.status.to_i).to eq 409
end
it 'should respect the number of allowed attempts' do
json = qs_api_create
qs = Quizzes::QuizSubmission.find(json['quiz_submissions'][0]['id'])
qs.mark_completed
qs.save!
qs_api_create(true)
expect(response.status.to_i).to eq 409
end
end
it "should allow access to quiz once module is completed" do
@course.context_modules.first.update_for(@student, :read, @tag)
@course.context_modules.first.update_downstreams
expect(@quiz.grants_right?(@student, :submit)).to be_truthy # precondition
json = api_call(:post, "/api/v1/courses/#{@course.id}/quizzes/#{@quiz.id}/submissions",
{controller: "quizzes/quiz_submissions_api", action: "create", format: "json", course_id: "#{@course.id}", quiz_id: "#{@quiz.id}"}, {},
{'Accept' => 'application/vnd.api+json'})
expect(json['quiz_submissions'][0]['user_id']).to eq @student.id
context "unpublished module quiz" do
before do
student_in_course(active_all: true)
@quiz = @course.quizzes.create! title: "Test Quiz w/ Module"
@quiz.quiz_questions.create!(
{
question_data:
{
name: 'question',
points_possible: 1,
question_type: 'multiple_choice_question',
'answers' =>
[
{'answer_text' => '1', 'weight' => '100'},
{'answer_text' => '2'},
{'answer_text' => '3'},
{'answer_text' => '4'}
]
}
})
@quiz.published_at = Time.zone.now
@quiz.workflow_state = 'available'
@quiz.save!
@pre_module = @course.context_modules.create!(:name => 'pre_module')
# No meaning in this URL
@tag = @pre_module.add_item(:type => 'external_url', :url => 'http://example.com', :title => 'example')
@tag.publish! if @tag.unpublished?
@pre_module.completion_requirements = { @tag.id => { :type => 'must_view' } }
@pre_module.save!
locked_module = @course.context_modules.create!(name: 'locked_module', require_sequential_progress: true)
locked_module.add_item(:id => @quiz.id, :type => 'quiz')
locked_module.prerequisites = "module_#{@pre_module.id}"
locked_module.save!
end
it "shouldn't allow access to quiz until module is completed" do
expect(@quiz.grants_right?(@student, :submit)).to be_truthy # precondition
json = api_call(:post,
"/api/v1/courses/#{@course.id}/quizzes/#{@quiz.id}/submissions",
{
controller: "quizzes/quiz_submissions_api",
action: "create",
format: "json",
course_id: "#{@course.id}",
quiz_id: "#{@quiz.id}"
},
{},
{'Accept' => 'application/vnd.api+json'},
{expected_status: 400})
expect(json['status']).to eq "bad_request"
end
it "should allow access to quiz once module is completed" do
@course.context_modules.first.update_for(@student, :read, @tag)
@course.context_modules.first.update_downstreams
expect(@quiz.grants_right?(@student, :submit)).to be_truthy # precondition
json = api_call(:post,
"/api/v1/courses/#{@course.id}/quizzes/#{@quiz.id}/submissions",
{
controller: "quizzes/quiz_submissions_api",
action: "create",
format: "json",
course_id: "#{@course.id}",
quiz_id: "#{@quiz.id}"
},
{},
{'Accept' => 'application/vnd.api+json'})
expect(json['quiz_submissions'][0]['user_id']).to eq @student.id
end
end
end
end

View File

@ -42,106 +42,167 @@ describe Quizzes::QuizzesApiController, type: :request do
end
describe "GET /courses/:course_id/quizzes (index)" do
let(:quizzes) { (0..3).map { |i| @course.quizzes.create! :title => "quiz_#{i}"} }
before(:once) { teacher_in_course(:active_all => true) }
it "should return list of quizzes" do
quizzes = (0..3).map{ |i| @course.quizzes.create! :title => "quiz_#{i}" }
json = api_call(:get, "/api/v1/courses/#{@course.id}/quizzes",
:controller=>"quizzes/quizzes_api", :action=>"index", :format=>"json", :course_id=>"#{@course.id}")
quiz_ids = json.collect { |quiz| quiz['id'] }
expect(quiz_ids).to eq quizzes.map(&:id)
before do
quizzes
end
it "should search for quizzes by title" do
2.times{ |i| @course.quizzes.create! :title => "first_#{i}" }
ids = @course.quizzes.map(&:id)
2.times{ |i| @course.quizzes.create! :title => "second_#{i}" }
context 'as a teacher' do
subject do
api_call(:get,
"/api/v1/courses/#{@course.id}/quizzes",
controller: "quizzes/quizzes_api",
action: "index",
format: "json",
course_id: "#{@course.id}")
end
json = api_call(:get, "/api/v1/courses/#{@course.id}/quizzes?search_term=fir",
:controller=>"quizzes/quizzes_api", :action=>"index", :format=>"json", :course_id=>"#{@course.id}",
:search_term => 'fir')
it "should return list of quizzes" do
quiz_ids = subject.collect { |quiz| quiz['id'] }
expect(quiz_ids).to eq quizzes.map(&:id)
end
expect(json.map{|h| h['id'] }.sort).to eq ids.sort
end
it 'should not be able to take quiz' do
actual_values = subject.map { |quiz| quiz['locked_for_user'] }
expected_values = Array.new(actual_values.count, true)
expect(actual_values).to eq(expected_values)
end
it "should deterministically order quizzes for pagination" do
quiz_number = 10
quiz_number.times{ @course.quizzes.create! :title => "the same title" }
found_quiz_ids = []
describe 'search_term query param' do
let(:search_term) { 'waldo' }
let(:quizzes_with_search_term) { (0..2).map { |i| @course.quizzes.create! :title => "#{search_term}_#{i}" } }
let(:quizzes_without_search_term) { (0..2).map { |i| @course.quizzes.create! :title => "quiz_#{i}" } }
let(:quizzes) { quizzes_with_search_term + quizzes_without_search_term }
quiz_number.times do |i|
page_num = i + 1
json = api_call(:get, "/api/v1/courses/#{@course.id}/quizzes?page=#{page_num}&per_page=1",
:controller=>"quizzes/quizzes_api", :action=>"index", :format=>"json", :course_id=>"#{@course.id}",
:per_page => 1, :page => page_num)
it "should search for quizzes by title" do
response = api_call(:get,
"/api/v1/courses/#{@course.id}/quizzes?search_term=#{search_term}",
controller: "quizzes/quizzes_api",
action: "index",
format: "json",
course_id: "#{@course.id}",
search_term: search_term)
id = json[0]["id"]
id_already_found = found_quiz_ids.include?(id)
expect(id_already_found).to be_falsey
found_quiz_ids << id
response_quiz_ids = response.map { |quiz| quiz['id'] }
expect(response_quiz_ids.sort).to eq(quizzes_with_search_term.map(&:id).sort)
end
end
context 'quizzes with the same title' do
let(:quiz_count) { 10 }
let(:quizzes) { (0..quiz_count).map { @course.quizzes.create! :title => "the same title" } }
it "should deterministically order quizzes for pagination" do
found_quiz_ids = []
quiz_count.times do |i|
page_num = i + 1
response = api_call(:get,
"/api/v1/courses/#{@course.id}/quizzes?page=#{page_num}&per_page=1",
controller: "quizzes/quizzes_api",
action: "index",
format: "json",
course_id: "#{@course.id}",
per_page: 1,
page: page_num)
id = response.first["id"]
id_already_found = found_quiz_ids.include?(id)
expect(id_already_found).to be_falsey
found_quiz_ids << id
end
end
end
context "jsonapi style" do
it "renders a jsonapi style response" do
response = api_call(:get,
"/api/v1/courses/#{@course.id}/quizzes",
{
controller: "quizzes/quizzes_api",
action: "index",
format: "json",
course_id: "#{@course.id}"
},
{},
'Accept' => 'application/vnd.api+json')
meta = response['meta']
expect(meta['permissions']['quizzes']['create']).to eq(true)
quizzes_json = response['quizzes']
quiz_ids = quizzes_json.collect { |quiz| quiz['id'] }
expect(quiz_ids).to eq quizzes.map(&:id).map(&:to_s)
end
end
end
it "should return unauthorized if the quiz tab is disabled" do
@course.tab_configuration = [ { :id => Course::TAB_QUIZZES, :hidden => true } ]
@course.save!
student_in_course(:active_all => true, :course => @course)
raw_api_call(:get, "/api/v1/courses/#{@course.id}/quizzes",
:controller => "quizzes/quizzes_api",
:action => "index",
:format => "json",
:course_id => "#{@course.id}")
assert_status(404)
end
it "limits student requests to published quizzes" do
student_in_course(:active_all => true)
quizzes = (0..1).map { |i| @course.quizzes.create! :title => "quiz_#{i}"}
published_quiz = quizzes.first
published_quiz.publish!
json = api_call(:get, "/api/v1/courses/#{@course.id}/quizzes",
:controller => 'quizzes/quizzes_api',
:action => 'index',
:format => 'json',
:course_id => "#{@course.id}")
quiz_ids = json.collect { |quiz| quiz['id'] }
expect(quiz_ids).to eq [ published_quiz.id]
end
context 'as a student' do
before(:once) { student_in_course(:active_all => true) }
context "jsonapi style" do
it "renders a jsonapi style response" do
quizzes = (0..3).map{ |i| @course.quizzes.create! :title => "quiz_#{i}" }
json = api_call(:get, "/api/v1/courses/#{@course.id}/quizzes",
{:controller=>"quizzes/quizzes_api", :action=>"index", :format=>"json", :course_id=>"#{@course.id}"},
{},
'Accept' => 'application/vnd.api+json')
meta = json['meta']
meta = json['meta']
expect(meta['permissions']['quizzes']['create']).to eq true
json = json['quizzes']
quiz_ids = json.collect { |quiz| quiz['id'] }
expect(quiz_ids).to eq quizzes.map(&:id).map(&:to_s)
context 'quiz tab is disabled' do
before do
@course.tab_configuration = [{ :id => Course::TAB_QUIZZES, :hidden => true }]
@course.save!
end
it "should return unauthorized" do
raw_api_call(:get,
"/api/v1/courses/#{@course.id}/quizzes",
controller: "quizzes/quizzes_api",
action: "index",
format: "json",
course_id: "#{@course.id}")
assert_status(404)
end
end
it "limits student requests to available quizzes" do
student_in_course(:active_all => true)
quizzes = (0..3).map{ |i| @course.quizzes.create! :title => "quiz_#{i}" }
available_quiz = quizzes.first
available_quiz.workflow_state = 'available'
available_quiz.save!
context 'a published quiz' do
let(:published_quiz) { quizzes.first }
json = api_call(:get, "/api/v1/courses/#{@course.id}/quizzes",
{:controller=>"quizzes/quizzes_api", :action=>"index", :format=>"json", :course_id=>"#{@course.id}"},
{},
'Accept' => 'application/vnd.api+json')
json = json['quizzes']
quiz_ids = json.collect { |quiz| quiz['id'] }
expect(quiz_ids).to eq [ available_quiz.id.to_s ]
before do
published_quiz.publish!
end
subject do
api_call(:get,
"/api/v1/courses/#{@course.id}/quizzes",
controller: 'quizzes/quizzes_api',
action: 'index',
format: 'json',
course_id: "#{@course.id}")
end
it "only returns published quizzes" do
quiz_ids = subject.map { |quiz| quiz['id'] }
expect(quiz_ids).to eq([published_quiz.id])
end
it 'should be able to take quiz' do
actual_values = subject.map { |quiz| quiz['locked_for_user'] }
expected_values = [false]
expect(actual_values).to eq(expected_values)
end
context "jsonapi style" do
it "only returns published quizzes" do
response = api_call(:get,
"/api/v1/courses/#{@course.id}/quizzes",
{
controller: "quizzes/quizzes_api",
action: "index",
format: "json",
course_id: "#{@course.id}"
},
{},
'Accept' => 'application/vnd.api+json')
quizzes_json = response['quizzes']
quiz_ids = quizzes_json.collect { |quiz| quiz['id'] }
expect(quiz_ids).to eq([published_quiz.id.to_s])
end
end
end
end
end

View File

@ -1751,6 +1751,131 @@ describe Quizzes::Quiz do
end
end
describe '#locked_for?' do
let(:quiz) { @course.quizzes.create! }
let(:student) { student_in_course(course: @course, active_enrollment: true).user }
let(:subject_user) { student }
let(:opts) { {} }
subject { quiz.locked_for?(subject_user, opts) }
before do
@course.offer!
end
shared_examples 'overrides' do
context 'when a user has a manually unlocked submission' do
before do
quiz_submission = quiz.generate_submission(subject_user)
quiz_submission.manually_unlocked = true
quiz_submission.save
end
it { is_expected.to be false }
end
end
context '#unlock_at time has not yet occurred' do
before do
quiz.unlock_at = 1.hour.from_now
end
include_examples 'overrides'
it { is_expected.to be_truthy }
it { is_expected.to have_key :unlock_at }
end
context '#unlock_at time has passed' do
before do
quiz.unlock_at = 1.hour.ago
end
it { is_expected.to be false }
end
context '#lock_at time as passed' do
before do
quiz.lock_at = 1.hour.ago
end
include_examples 'overrides'
it { is_expected.to be_truthy }
it { is_expected.to have_key :lock_at }
end
context '#lock_at time has not yet occurred' do
before do
quiz.lock_at = 1.hour.from_now
end
it { is_expected.to be false }
end
context 'quiz is for an assignment' do
let(:assignment_lock_info) { quiz.assignment.locked_for?(subject_user, opts) }
before do
# need the after_save hooks to run for fully-prepare the quiz
quiz.save!
end
context 'assignment is not locked' do
before do
expect(assignment_lock_info).not_to be_present
end
it { is_expected.to be false }
end
context 'assignment is locked' do
before do
quiz.assignment.unlock_at = 1.day.from_now
quiz.assignment.save
end
include_examples 'overrides'
it { is_expected.to be_truthy }
it 'returns the lock info for the assignment' do
expect(subject).to eq assignment_lock_info
end
context 'skipping assignments' do
let(:opts) { { skip_assignment: true } }
it { is_expected.to be false }
end
end
end
context 'modules are locked' do
before do
locked_module = @course.context_modules.create!(name: 'locked module', unlock_at: 1.day.from_now)
locked_module.add_item(id: quiz.id, type: 'quiz')
end
include_examples 'overrides'
it { is_expected.to be_truthy }
it { is_expected.to have_key :context_module }
end
context 'user is not a student' do
let(:admin) { account_admin_user(account: @course.account) }
let(:subject_user) { admin }
it { is_expected.to be_truthy }
it { is_expected.to have_key :missing_permission }
end
end
describe '.class_names' do
it 'returns an array of all acceptable class names' do
expect(Quizzes::Quiz.class_names).to eq ['Quiz', 'Quizzes::Quiz']