don't return scribd_doc for locked attachments with a root_attachment

fixes #9873

The previous method for making this happen was just setting scribd_doc
to nil on the attachment before serializing it (wut?), but
Attachment#scribd_doc is overridden to return the root_attachment's
scribd_doc if there isn't one on this attachment.

The new strategy is to just use our filter_attributes_for_user stuff to
remove the secret info if the user doesn't have permission.

test plan: Upload a pdf or something, set it as locked, link to it from
a wiki page. As a student in that course, you shouldn't be able to
preview the document in-line. Then copy the course. In the new course,
you also shouldn't be able to preview the document in-line.

Change-Id: I66dc3a55a4e0371337846eb82179e6638a7d3852
Reviewed-on: https://gerrit.instructure.com/12921
Reviewed-by: Brian Palmer <brianp@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
This commit is contained in:
Brian Palmer 2012-08-14 12:25:45 -06:00
parent 63a2e3a5bc
commit 4701c9d3ce
4 changed files with 52 additions and 3 deletions

View File

@ -308,8 +308,6 @@ class FilesController < ApplicationController
)
options[:methods] = :authenticated_s3_url if service_enabled?(:google_docs_previews) && attachment.authenticated_s3_url
log_asset_access(@attachment, "files", "files")
else
@attachment.scribd_doc = nil
end
end
format.json { render :json => @attachment.to_json(options) }

View File

@ -1426,7 +1426,10 @@ class Attachment < ActiveRecord::Base
self.scribd_doc = @scribd_doc_backup
self.scribd_doc.secret_password = @scribd_password if self.scribd_doc
end
def filter_attributes_for_user(hash, user, session)
hash.delete(:scribd_doc) unless grants_right?(user, session, :download)
end
def self.process_scribd_conversion_statuses
# Runs periodically
@attachments = Attachment.needing_scribd_conversion_status

View File

@ -296,6 +296,39 @@ describe FilesController do
assigns(:attachment).should == new_file
end
describe "scribd_doc" do
before do
course_with_student_logged_in(:active_all => true)
@file = attachment_model(:scribd_doc => Scribd::Document.new, :uploaded_data => stub_png_data)
end
it "should be included if :download is allowed" do
get 'show', :course_id => @course.id, :id => @file.id, :format => 'json'
json_parse['attachment']['scribd_doc'].should be_present
end
it "should not be included if locked" do
@file.lock_at = 1.month.ago
@file.save!
get 'show', :course_id => @course.id, :id => @file.id, :format => 'json'
json_parse['attachment']['scribd_doc'].should be_blank
end
it "should not be included for locked attachments with a root_attachment" do
@file.lock_at = 1.month.ago
@file.save!
course2 = course(:active_all => true)
course2.enroll_student(@student).accept!
file2 = @file.clone_for(course2)
file2.save!
file2.scribd_doc.should be_present
file2.locked_for?(@student).should be_true
get 'show', :course_id => course2.id, :id => file2.id, :format => 'json'
json_parse['attachment']['scribd_doc'].should be_blank
end
end
end
describe "GET 'show_relative'" do

View File

@ -270,6 +270,21 @@ describe ContentMigration do
new_topic.message.should match(Regexp.new("/courses/#{@copy_to.id}/files/#{new_att.id}/preview"))
end
it "should keep date-locked files locked" do
student = user
@copy_from.enroll_student(student)
att = Attachment.create!(:filename => 'test.txt', :display_name => "testing.txt", :uploaded_data => StringIO.new('file'), :folder => Folder.root_folders(@copy_from).first, :context => @copy_from, :lock_at => 1.month.ago, :unlock_at => 1.month.from_now)
att.grants_right?(student, :download).should be_false
run_course_copy
@copy_to.enroll_student(student)
new_att = @copy_to.attachments.find_by_migration_id(CC::CCHelper.create_key(att))
new_att.should be_present
new_att.grants_right?(student, :download).should be_false
end
it "should tranlsate links to module items in html content" do
mod1 = @copy_from.context_modules.create!(:name => "some module")
asmnt1 = @copy_from.assignments.create!(:title => "some assignment")