clear file permissions on folder change
closes OUT-4045 flag=none Test plan: - in course files, create three folders, each containing an image - also add three images not in folders - unpublish one folder and one of the top level files - set access to one of the files and one of the folders to unlock in several minutes and lock again a few minutes later - create a quiz in the course - add a quiz question - add the six images to the text of the quiz question - save the quiz - take the quiz as a student (test student works) - verify that four of the images are not visible to the student - go to course files and publish the hidden folder and file - take the quiz again as a student - verify that the two unlocked images are now visible - wait until after the unlock time set above is reached - take the quiz again as student - verify that all the images are now visible - wait until after the lock time set above is reached - take the quiz again - verify that the lock_at folder and file images are now not accessible Change-Id: I1e7cb7c103158c0ef5e7704459970554c59cd7e9 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/257917 Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com> Reviewed-by: Augusto Callejas <acallejas@instructure.com> Reviewed-by: Pat Renner <prenner@instructure.com> QA-Review: Augusto Callejas <acallejas@instructure.com> Product-Review: Michael Brewer-Davis <mbd@instructure.com>
This commit is contained in:
parent
95fdc1558d
commit
150ecd7d63
|
@ -1344,8 +1344,8 @@ class Attachment < ActiveRecord::Base
|
|||
return {:asset_string => self.asset_string, :manually_locked => true} if self.locked || Folder.is_locked?(self.folder_id)
|
||||
RequestCache.cache(locked_request_cache_key(user)) do
|
||||
locked = false
|
||||
if (self.unlock_at && Time.now < self.unlock_at)
|
||||
touch_on_unlock if Time.now + 1.hour >= self.unlock_at
|
||||
if (self.unlock_at && Time.zone.now < self.unlock_at)
|
||||
touch_on_unlock if (Time.zone.now + AdheresToPolicy::Cache::CACHE_EXPIRES_IN) >= self.unlock_at
|
||||
locked = {:asset_string => self.asset_string, :unlock_at => self.unlock_at}
|
||||
elsif (self.lock_at && Time.now > self.lock_at)
|
||||
locked = {:asset_string => self.asset_string, :lock_at => self.lock_at}
|
||||
|
|
|
@ -56,6 +56,7 @@ class Folder < ActiveRecord::Base
|
|||
validate :protect_root_folder_name, :if => :name_changed?
|
||||
validate :reject_recursive_folder_structures, on: :update
|
||||
validate :restrict_submission_folder_context
|
||||
after_commit :clear_permissions_cache, if: ->{[:workflow_state, :parent_folder_id, :locked, :lock_at, :unlock_at].any? {|k| saved_changes.key?(k)}}
|
||||
|
||||
def file_attachments_visible_to(user)
|
||||
if context_root_account(user).feature_enabled?(:granular_permissions_course_files) &&
|
||||
|
@ -584,4 +585,23 @@ class Folder < ActiveRecord::Base
|
|||
nil
|
||||
end
|
||||
private_class_method :find_visible_folders
|
||||
|
||||
def clear_permissions_cache
|
||||
GuardRail.activate(:primary) do
|
||||
delay_if_production(singleton: "clear_downstream_permissions_#{global_id}").clear_downstream_permissions
|
||||
next_clear_cache = next_lock_change
|
||||
if next_clear_cache.present? && next_clear_cache < (Time.zone.now + AdheresToPolicy::Cache::CACHE_EXPIRES_IN)
|
||||
delay(run_at: next_clear_cache, singleton: "clear_permissions_cache_#{global_id}").clear_permissions_cache
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def clear_downstream_permissions
|
||||
active_file_attachments.touch_all
|
||||
active_sub_folders.each(&:clear_permissions_cache)
|
||||
end
|
||||
|
||||
def next_lock_change
|
||||
[lock_at, unlock_at].compact.select {|t| t > Time.zone.now}.min
|
||||
end
|
||||
end
|
||||
|
|
|
@ -339,7 +339,7 @@ describe ContentZipper do
|
|||
|
||||
@attachment = Attachment.new(display_name: 'my_download.zip')
|
||||
@attachment.workflow_state = 'to_be_zipped'
|
||||
@attachment.context = folder
|
||||
@attachment.context = folder.reload
|
||||
end
|
||||
|
||||
def zipped_files_for_user(user=nil, check_user=true)
|
||||
|
|
|
@ -19,6 +19,7 @@
|
|||
#
|
||||
|
||||
require File.expand_path(File.dirname(__FILE__) + '/../sharding_spec_helper.rb')
|
||||
require 'delayed/testing'
|
||||
|
||||
describe Folder do
|
||||
before(:once) do
|
||||
|
@ -26,7 +27,7 @@ describe Folder do
|
|||
end
|
||||
|
||||
it "should create a new instance given valid attributes" do
|
||||
folder_model
|
||||
expect(folder_model).to be_present
|
||||
end
|
||||
|
||||
it "should infer its full name if it has a parent folder" do
|
||||
|
@ -447,4 +448,87 @@ describe Folder do
|
|||
expect { dup.save! }.to raise_exception(ActiveRecord::RecordNotUnique)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'permissions' do
|
||||
before(:once) do
|
||||
@course.offer!
|
||||
student_in_course(active_all: true)
|
||||
end
|
||||
|
||||
let_once(:folder) { @course.folders.create!(name: 'f') }
|
||||
let_once(:file) { attachment_model context: @course, display_name: 'normal.txt', folder: folder }
|
||||
|
||||
context 'clears own permissions' do
|
||||
def student_can_read_contents?
|
||||
Folder.find(folder.id).grants_right? @student, :read_contents
|
||||
end
|
||||
|
||||
it 'when unlock_at set' do
|
||||
Timecop.freeze do
|
||||
folder.update! unlock_at: 5.minutes.from_now
|
||||
expect(student_can_read_contents?).to be false
|
||||
Timecop.travel(10.minutes)
|
||||
Delayed::Testing.drain
|
||||
expect(student_can_read_contents?).to be true
|
||||
end
|
||||
end
|
||||
|
||||
it 'when lock_at set' do
|
||||
Timecop.freeze do
|
||||
folder.update! lock_at: 5.minutes.from_now
|
||||
expect(student_can_read_contents?).to be true
|
||||
Timecop.travel(10.minutes)
|
||||
Delayed::Testing.drain
|
||||
expect(student_can_read_contents?).to be false
|
||||
end
|
||||
end
|
||||
|
||||
it 'when lock_at and unlock_at set' do
|
||||
Timecop.freeze do
|
||||
folder.update! unlock_at: 5.minutes.from_now, lock_at: 15.minutes.from_now
|
||||
expect(student_can_read_contents?).to be false
|
||||
Timecop.travel(10.minutes)
|
||||
Delayed::Testing.drain
|
||||
expect(student_can_read_contents?).to be true
|
||||
Timecop.travel(10.minutes)
|
||||
Delayed::Testing.drain
|
||||
expect(student_can_read_contents?).to be false
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'clears file permissions' do
|
||||
def student_can_download?
|
||||
file.reload.grants_right? @student, :download
|
||||
end
|
||||
|
||||
it 'when locked' do
|
||||
expect(student_can_download?).to be true
|
||||
folder.update! locked: true
|
||||
expect(student_can_download?).to be false
|
||||
end
|
||||
|
||||
it 'when unlocked' do
|
||||
folder.update! locked: true
|
||||
expect(student_can_download?).to be false
|
||||
folder.update! locked: false
|
||||
expect(student_can_download?).to be true
|
||||
end
|
||||
|
||||
it 'when moved' do
|
||||
expect(student_can_download?).to be true
|
||||
parent_folder = @course.folders.create!(name: 'parent', locked: true)
|
||||
folder.update! parent_folder: parent_folder
|
||||
expect(student_can_download?).to be false
|
||||
end
|
||||
|
||||
it 'in subfolders' do
|
||||
parent_folder = @course.folders.create!(name: 'parent')
|
||||
parent_folder.sub_folders << folder
|
||||
expect(student_can_download?).to be true
|
||||
parent_folder.reload.update! locked: true
|
||||
expect(student_can_download?).to be false
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue