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

Better file URLs for viewing, not downloading 

closes: LS-1265
flag=new_file_url_rewriting

- gets the verifier, if present, forwarded to the view ping-back url so
  it doesn't fail for one student viewing another student's file.
  Before, when clicking on a pdf and viewing it in canvadoc, the viewing
  of the file succeeds, but the ping-back to log access failed with a
  401. The pin-back now succeeds.
- gets the verifier added to the iframe src when the file is being
  viewed locally. You'll see this when stud1 views a .txt file from
  stud2. plain text files aren't canvadoc viewable, so they're viewed
  in a vanilla iframe.
- stops rewriting non /preview file URLs as download URLs.
  ?download_frd=1 is reserved for that. Before, when users deleted
  "/download" from a link using the html editor in the RCE, canvas put
  it back. Not any more. They shouldn't need to any more either,
  since...
- updates the RCE so links to files do not include /download in the
  URL.  When clicking on a file in the tray, the generated link no
  longer incfludes /download (and canvas won't put it back)
  Embedded images use /preview. Using Image Options to convert the
  image to a link removes /preview, and no longer replaces it with
  /download.

There's some weird file URL handling on in canvas. If the URL is
/preview, it is not logged as a page view. There are comments
indicating that /download will log access, though not always actually
download the file, and that download_frd is used for that. I found
this to be hard to confirm, /download seemed to download for me a lot.
It might be in conjunction with wrap=1, or the inline class name?

This change sets the "new_file_url_rewriting" flag, which enables
these changes, to on in ci and dev.
This change scares me a little and I really want to know that it's
OK.

test plan:
  - ensure new_file_url_rewriting is enabled (which it should be
    unless you're in test or prod environments)
  - it's nice to have canvadocs enabled too
  - do everything you can think of that revolves around files/attachments
    and make sure it still works. No, I/m not kidding

  - link files in the RCE
    - try file types that will be viewed in canvadocs (.pdf),
      in google docs (.rtf), and in a vanilla iffame (.txt)
  > expect clicking on the link to open the file in another tab
  - embed images in the RCE
  > expect the image to be shown, and the <img src> to be /preview
    (when loading a page with an image, the image will not show up in
     recent history, if that's on)
  - using Image Options, convert an image to a link
  > expect the image to be displayed in a new tab when the link is
    clicked
  - link a file, then add "download_frd=1" to the href's query_string
  > click on the link and expect the file to be downloaded, not viewed

  - I think there's some special handling WRT student submissions, so
    try all ^that in a submission.
  - All ^that should work if student1 links/embeds user files and
    images, then
  > expect student2 to be able to view them all
    (discussions are good for this)

  > expect existing content behavior to be the same as it ever was.

Change-Id: Ieae7e4daf549ececb982007b6ce97c8c091c099c
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/249094
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jackson Howe <jackson.howe@instructure.com>
QA-Review: Robin Kuss <rkuss@instructure.com>
Product-Review: Ed Schiebel <eschiebel@instructure.com>
This commit is contained in:
Ed Schiebel 2020-10-01 14:53:15 -04:00
parent 517813d1bc
commit 89c51fc89b
14 changed files with 343 additions and 124 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
app/controllers/files_controller.rb
View File

@ -440,13 +440,16 @@ class FilesController < ApplicationController
#
def public_url
@attachment = Attachment.find(params[:id])
verifier_checker = Attachments::Verification.new(@attachment)
# if the attachment is part of a submisison, its 'context' will be the student that submmited the assignment. so if @current_user is a
# teacher authorized_action(@attachment, @current_user, :download) will be false, we need to actually check if they have perms to see the
# submission.
@submission = Submission.active.find(params[:submission_id]) if params[:submission_id]
# verify that the requested attachment belongs to the submission
return render_unauthorized_action if @submission && !@submission.includes_attachment?(@attachment)
if @submission ? authorized_action(@submission, @current_user, :read) : authorized_action(@attachment, @current_user, :download)
if ((@submission && authorized_action(@submission, @current_user, :read)) ||
((params[:verifier] && verifier_checker.valid_verifier_for_permission?(params[:verifier], :download, session)) || authorized_action(@attachment, @current_user, :download)))
render :json => { :public_url => @attachment.public_url(:secure => request.ssl?) }
end
end

2
app/helpers/attachment_helper.rb
View File

@ -41,7 +41,7 @@ module AttachmentHelper
context_name = url_helper_context_from_object(attachment.context)
url_helper = "#{context_name}_file_inline_view_url"
if self.respond_to?(url_helper)
attrs[:attachment_view_inline_ping_url] = self.send(url_helper, attachment.context, attachment.id)
attrs[:attachment_view_inline_ping_url] = self.send(url_helper, attachment.context, attachment.id, {:verifier => params[:verifier]})
end
if attachment.pending_upload? || attachment.processing?
attrs[:attachment_preview_processing] = true

8
app/views/files/show.html.erb
View File

@ -20,7 +20,7 @@
<%= join_title @attachment.display_name, @context&.name %>
<% end %>
<% add_crumb @attachment.display_name, context_url(@context, :context_file_url, @attachment) %>
<% download_url = context_url(@context, :context_file_download_url, @attachment.id, download_frd: 1) %>
<% download_url = context_url(@context, :context_file_download_url, @attachment.id, download_frd: 1, verifier: params[:verifier]) %>
<% js_bundle 'module_sequence_footer' %>
<% if (locked = @attachment.locked_for?(@current_user, :check_policies => true)) %>
<div style="margin: 10px 50px;">
@ -39,12 +39,12 @@
</div>
<% if @attachment.content_type == "application/x-shockwave-flash" %>
<object classid="clsid:D27CDB6E-AE6D-11cf-96B8-444553540000" class="embedded_swf">
<param name="movie" value="<%= safe_domain_file_url(@attachment, fallback_url: download_url) %>" />
<param name="movie" value="<%= safe_domain_file_url(@attachment, fallback_url: download_url, verifier: params[:verifier]) %>" />
<param name="wmode" value="transparent" />
<param name="salign" value="t" />
<param name="allowscriptaccess" value="never" />
<!--[if !IE]>-->
<object type="application/x-shockwave-flash" data="<%= safe_domain_file_url(@attachment, fallback_url: download_url) %>" class="embedded_swf">
<object type="application/x-shockwave-flash" data="<%= safe_domain_file_url(@attachment, fallback_url: download_url, verifier: params[:verifier]) %>" class="embedded_swf">
<param name="wmode" value="transparent" />
<param name="salign" value="t" />
<param name="allowscriptaccess" value="never" />
@ -55,7 +55,7 @@
</object>
<% elsif @attachment.inline_content? && !@attachment.canvadocable? %>
<% js_bundle :file_inline %>
<iframe id="file_content" src="<%= safe_domain_file_url(@attachment, fallback_url: download_url) %>" style="width: 100%; height: 400px;" title="<%= t('File Content') %>"></iframe>
<iframe id="file_content" src="<%= safe_domain_file_url(@attachment, fallback_url: download_url, verifier: params[:verifier]) %>" style="width: 100%; height: 400px;" title="<%= t('File Content') %>"></iframe>
<% elsif @attachment.content_type && @attachment.content_type.match(/\Aimage\//) %>
<%= link_to(image_tag(download_url, :alt => @attachment.display_name), download_url) %>
<% elsif @attachment.content_type && @attachment.content_type.match(/\Avideo\/|audio\//) %>

15
config/feature_flags/learning_activities_release_flags.yml
View File

@ -75,4 +75,17 @@ enhanced_grade_change_query:
display_name: Enhanced grade change auditing
description: Allow searching grade change audit logs by multiple criteria
applies_to: SiteAdmin
new_file_url_rewriting:
state: hidden
display_name: Update file links to prefer preview, not download
description: |-
Canvas rewrites file URLs in user generated content. In the process, it
often added "/download" where it was not wanted and inappropriate. This
setting updates how URL rewriting takes place to facilitate previewing
files within Canvas and not downloading.
applies_to: SiteAdmin
environments:
development:
state: on
ci:
state: on

39
lib/user_content/files_handler.rb
View File

@ -22,6 +22,14 @@ module UserContent
def preview?
rest.start_with?('/preview')
end
def download?
rest.start_with?('/download')
end
def download_frd?
rest.include?('download_frd=1')
end
end
class ProcessedUrl
@ -32,6 +40,7 @@ module UserContent
@attachment = attachment
@is_public = is_public
@in_app = in_app
@new_file_url_rewriting = Account.site_admin.feature_enabled?(:new_file_url_rewriting)
end
def url
@ -58,7 +67,11 @@ module UserContent
def options
{ only_path: true }.tap do |h|
h[:download] = 1 unless match.preview?
if @new_file_url_rewriting
h[:download] = 1 if match.download_frd?
else
h[:download] = 1 unless match.preview?
end
h[:verifier] = attachment.uuid unless in_app && !is_public
if !match.preview? && match.rest.include?('wrap=1')
h[:wrap] = 1
@ -67,14 +80,28 @@ module UserContent
end
def path
if Attachment.relative_context?(attachment.context_type)
if match.preview?
"#{attachment.context_type.downcase}_file_preview_url"
if @new_file_url_rewriting
if Attachment.relative_context?(attachment.context_type)
if match.preview?
"#{attachment.context_type.downcase}_file_preview_url"
elsif match.download? || match.download_frd?
"#{attachment.context_type.downcase}_file_download_url"
else
"#{attachment.context_type.downcase}_file_url"
end
else
"#{attachment.context_type.downcase}_file_download_url"
"file_download_url"
end
else
"file_download_url"
if Attachment.relative_context?(attachment.context_type)
if match.preview?
"#{attachment.context_type.downcase}_file_preview_url"
else
"#{attachment.context_type.downcase}_file_download_url"
end
else
"file_download_url"
end
end
end
end

7
packages/canvas-rce/src/common/fileUrl.js
View File

@ -35,6 +35,8 @@ export function downloadToWrap(url) {
delete parsed.search
delete parsed.query.download_frd
parsed.query.wrap = '1'
parsed.pathname = parsed.pathname.replace(/\/download\/?$/, '')
return format(parsed)
}
@ -56,6 +58,7 @@ export function fixupFileUrl(contextType, contextId, fileInfo) {
delete parsed.search
delete parsed.query.download_frd
parsed.query.wrap = '1'
parsed.pathname = parsed.pathname.replace(/\/download\/?$/, '')
// if this is a http://canvas/files... url. change it to be contextual
if (/^\/files/.test(parsed.pathname)) {
@ -87,7 +90,7 @@ export function prepEmbedSrc(url) {
if (parsed.host && window.location.host !== parsed.host) {
return url
}
parsed.pathname = parsed.pathname.replace(/\/download(\?|$)/, '/preview$1')
parsed.pathname = parsed.pathname.replace(/(?:\/download)?\/?(\?|$)/, '/preview$1')
delete parsed.search
delete parsed.query.wrap
return format(parsed)
@ -106,7 +109,7 @@ export function prepLinkedSrc(url) {
return url
}
delete parsed.search
parsed.pathname = parsed.pathname.replace(/\/preview(\?|$)/, '/download$1')
parsed.pathname = parsed.pathname.replace(/\/preview(\?|$)/, '$1')
parsed.query.wrap = '1'
return format(parsed)
}

4
packages/canvas-rce/src/rce/__tests__/contentInsertion.test.js
View File

@ -260,7 +260,7 @@ describe('contentInsertion', () => {
it('builds image html from image data', () => {
contentInsertion.insertImage(editor, image)
expect(editor.content).toEqual('<img alt="Here Be Images" src="/some/path"/>')
expect(editor.content).toEqual('<img alt="Here Be Images" src="/some/path/preview"/>')
})
it('uses url if no href', () => {
@ -290,7 +290,7 @@ describe('contentInsertion', () => {
contentInsertion.insertImage(ed, image)
expect(ed.content).toEqual(
'<a href="http://bogus.edu" data-mce-href="http://bogus.edu"><img alt="Here Be Images" src="/some/path"/></a>'
'<a href="http://bogus.edu" data-mce-href="http://bogus.edu"><img alt="Here Be Images" src="/some/path/preview"/></a>'
)
})
})

4
packages/canvas-rce/src/rce/__tests__/contentRendering.test.js
View File

@ -96,11 +96,11 @@ describe('contentRendering', () => {
)
})
it('replaces a preview link with download', () => {
it('removes /preview when rendering a link', () => {
link.href = '/users/2/files/17/preview?verifier=xyzzy'
const rendered = contentRendering.renderLink(link)
expect(rendered).toEqual(
'<a href="/users/2/files/17/download?verifier=xyzzy&amp;wrap=1" title="Here Be Links">Click On Me</a>'
'<a href="/users/2/files/17?verifier=xyzzy&amp;wrap=1" title="Here Be Links">Click On Me</a>'
)
})
})

2
packages/canvas-rce/src/rce/plugins/shared/__tests__/FileBrowser.test.js
View File

@ -41,7 +41,7 @@ describe('RceFileBrowser', () => {
expect(onFileSelect).toHaveBeenCalledWith({
name: 'a file',
title: 'a file',
href: '/file/download?wrap=1',
href: '/file?wrap=1',
embedded_iframe_url: undefined,
content_type: 'application/pdf',
target: '_blank',

19
packages/canvas-rce/test/common/fileUrl.test.js
View File

@ -75,13 +75,13 @@ describe('Common file url utils', () => {
it('transforms course file urls', () => {
// removes download_frd and adds wrap
const result = fixupFileUrl('course', 2, fileInfo)
strictEqual(result.href, '/courses/2/files/17/download?wrap=1')
strictEqual(result.href, '/courses/2/files/17?wrap=1')
})
it('adds the verifier to user files', () => {
// while removing download_frd
const result = fixupFileUrl('user', 2, fileInfo)
strictEqual(result.href, '/users/2/files/17/download?wrap=1&verifier=xyzzy')
strictEqual(result.href, '/users/2/files/17?wrap=1&verifier=xyzzy')
})
})
@ -102,13 +102,13 @@ describe('Common file url utils', () => {
it('transforms course file urls', () => {
// removes download_frd and adds wrap
const result = fixupFileUrl('course', 2, fileInfo)
strictEqual(result.url, '/courses/2/files/17/download?wrap=1')
strictEqual(result.url, '/courses/2/files/17?wrap=1')
})
it('adds the verifier to user files', () => {
// while removing download_frd and does not add wrap
const result = fixupFileUrl('user', 2, fileInfo)
strictEqual(result.url, '/users/2/files/17/download?wrap=1&verifier=xyzzy')
strictEqual(result.url, '/users/2/files/17?wrap=1&verifier=xyzzy')
})
})
})
@ -132,7 +132,7 @@ describe('Common file url utils', () => {
it('does not indescriminetly replace /preview in a url', () => {
const url = '/please/preview/me'
strictEqual(prepEmbedSrc(url), url)
strictEqual(prepEmbedSrc(url), '/please/preview/me/preview')
})
})
@ -143,14 +143,9 @@ describe('Common file url utils', () => {
strictEqual(url, result)
})
it('replaces /preview?some_params with /download?some_params', () => {
it('removes /preview', () => {
const url = '/users/2/files/17/preview?verifier=xyzzy'
strictEqual(prepLinkedSrc(url), '/users/2/files/17/download?verifier=xyzzy&wrap=1')
})
it('replaces /preview and no params with /download', () => {
const url = '/users/2/files/17/preview'
strictEqual(prepLinkedSrc(url), '/users/2/files/17/download?wrap=1')
strictEqual(prepLinkedSrc(url), '/users/2/files/17?verifier=xyzzy&wrap=1')
})
it('does not indescriminetly replace /download in a url', () => {

8
public/javascripts/jquery.doc_previews.js
View File

@ -177,12 +177,18 @@ $.fn.loadDocPreview = function(options) {
if (opts.public_url) {
loadGooglePreview()
} else if (opts.attachment_id) {
let url = '/api/v1/files/' + opts.attachment_id + '/public_url.json'
let url = `/api/v1/files/${opts.attachment_id}/public_url.json`
if (opts.submission_id) {
url += '?' + $.param({submission_id: opts.submission_id})
}
if (opts.verifier) {
url += `${opts.submission_id ? '&' : '?'}verifier=${opts.verifier}`
} else {
const match = window.location.search.match(/verifier=([^&]+)(?:&|$)/)
const ver = match && match[1]
if (ver) {
url += `${opts.submission_id ? '&' : '?'}verifier=${ver}`
}
}
$this.loadingImage()
$.ajaxJSON(url, 'GET', {}, data => {

8
spec/apis/api_spec_helper.rb
View File

@ -148,6 +148,13 @@ def check_document(html, course, attachment, include_verifiers)
img2 = doc.at_css('img#2')
expect(img2).to be_present
expect(img2['src']).to eq "http://www.example.com/courses/#{course.id}/files/#{attachment.id}/download#{params}"
img3 = doc.at_css('img#3')
expect(img3).to be_present
if Account.site_admin.feature_enabled?(:new_file_url_rewriting)
expect(img3['src']).to eq "http://www.example.com/courses/#{course.id}/files/#{attachment.id}#{params}"
else
expect(img3['src']).to eq "http://www.example.com/courses/#{course.id}/files/#{attachment.id}/download#{params}"
end
video = doc.at_css('video')
expect(video).to be_present
expect(video['poster']).to match(%r{http://www.example.com/media_objects/qwerty/thumbnail})
@ -166,6 +173,7 @@ def should_translate_user_content(course, include_verifiers=true)
Hello, students.<br>
This will explain everything: <img id="1" src="/courses/#{course.id}/files/#{attachment.id}/preview" alt="important">
This won't explain anything: <img id="2" src="/courses/#{course.id}/files/#{attachment.id}/download" alt="important">
This might explain something: <img id="3" src="/courses/#{course.id}/files/#{attachment.id}" alt="important">
Also, watch this awesome video: <a href="/media_objects/qwerty" class="instructure_inline_media_comment video_comment" id="media_comment_qwerty"><img></a>
And refer to this <a href="/courses/#{course.id}/pages/awesome-page">awesome wiki page</a>.
</p>

181
spec/lib/user_content/files_handler_spec.rb
View File

@ -21,122 +21,123 @@ require 'spec_helper'
describe UserContent::FilesHandler do
let(:is_public) { false }
let(:in_app) { false }
let(:course) { course_factory(active_all: true) }
let(:attachment) do
attachment_with_context(course_factory(active_all:true), {
filename: 'test.mp4',
content_type: 'video'
})
attachment_with_context(course, { filename: 'test.mp4', content_type: 'video' })
end
let(:match_url) do
[
attachment.context_type.tableize,
attachment.context_id,
'files',
match_part
].join('/')
[attachment.context_type.tableize, attachment.context_id, 'files', match_part].join('/')
end
let(:match_part) { 'download?wrap=1' }
let(:uri_match) do
UserContent::FilesHandler::UriMatch.new(OpenStruct.new({
url: match_url,
type: 'files',
obj_class: Attachment,
obj_id: attachment.id,
rest: "/#{match_part}"
}))
UserContent::FilesHandler::UriMatch.new(
OpenStruct.new(
{
url: match_url,
type: 'files',
obj_class: Attachment,
obj_id: attachment.id,
rest: "/#{match_part}"
}
)
)
end
let(:old_file_url_rewriting) { Account.site_admin.disable_feature!(:new_file_url_rewriting) }
describe UserContent::FilesHandler::ProcessedUrl do
subject(:processed_url) do
UserContent::FilesHandler::ProcessedUrl.new(
match: uri_match,
attachment: attachment,
is_public: is_public,
in_app: in_app
).url
end
context 'with new_file_url_rewriting off' do
before(:each) { old_file_url_rewriting }
describe '#url' do
it 'includes context class' do
expect(processed_url).to match(/#{attachment.context_type.tableize}/)
describe UserContent::FilesHandler::ProcessedUrl do
subject(:processed_url) do
UserContent::FilesHandler::ProcessedUrl.new(
match: uri_match, attachment: attachment, is_public: is_public, in_app: in_app
)
.url
end
it 'includes wrap=1' do
query_string = processed_url.split('?')[1]
expect(Rack::Utils.parse_nested_query(query_string)['wrap']).to eq '1'
end
describe '#url' do
it 'includes context class' do
expect(processed_url).to match(/#{attachment.context_type.tableize}/)
end
it 'includes verifier query param' do
query_string = processed_url.split('?')[1]
expect(Rack::Utils.parse_nested_query(query_string)).to have_key('verifier')
end
context 'is in_app' do
let(:in_app) { true }
it 'does not include verifier' do
it 'includes wrap=1' do
query_string = processed_url.split('?')[1]
expect(Rack::Utils.parse_nested_query(query_string)).not_to have_key('verifier')
end
end
context 'and match is a preview' do
let(:match_part) { 'preview' }
it 'is a preview url' do
expect(processed_url).to match(/files\/(\d)+\/preview/)
expect(Rack::Utils.parse_nested_query(query_string)['wrap']).to eq '1'
end
it 'does not include wrap param' do
it 'includes verifier query param' do
query_string = processed_url.split('?')[1]
expect(Rack::Utils.parse_nested_query(query_string)).not_to have_key('wrap')
expect(Rack::Utils.parse_nested_query(query_string)).to have_key('verifier')
end
end
context 'when attachment does not support relative paths' do
let(:attachment) { attachment_with_context(submission_model) }
context 'is in_app' do
let(:in_app) { true }
it 'should not include context name' do
expect(processed_url).not_to match(/#{attachment.context_type.tableize}/)
it 'does not include verifier' do
query_string = processed_url.split('?')[1]
expect(Rack::Utils.parse_nested_query(query_string)).not_to have_key('verifier')
end
end
context 'and match is a preview' do
let(:match_part) { 'preview' }
it 'is a preview url' do
expect(processed_url).to match(%r{files\/(\d)+\/preview})
end
it 'does not include wrap param' do
query_string = processed_url.split('?')[1]
expect(Rack::Utils.parse_nested_query(query_string)).not_to have_key('wrap')
end
end
context 'when attachment does not support relative paths' do
let(:attachment) { attachment_with_context(submission_model) }
it 'should not include context name' do
expect(processed_url).not_to match(/#{attachment.context_type.tableize}/)
end
end
end
end
end
describe UserContent::FilesHandler do
subject(:processed_url) do
UserContent::FilesHandler.new(
match: uri_match,
context: attachment.context,
user: current_user,
preloaded_attachments: preloaded_attachments,
is_public: is_public,
in_app: in_app
).processed_url
end
let(:current_user) do
student_in_course(active_all: true, course: attachment.context)
@student
end
let(:preloaded_attachments) { {} }
describe '#processed_url' do
it 'delegates to ProcessedUrl' do
expect(processed_url).to match(/#{attachment.context_type.tableize}/)
describe UserContent::FilesHandler do
subject(:processed_url) do
UserContent::FilesHandler.new(
match: uri_match,
context: attachment.context,
user: current_user,
preloaded_attachments: preloaded_attachments,
is_public: is_public,
in_app: in_app
)
.processed_url
end
let(:current_user) do
student_in_course(active_all: true, course: attachment.context)
@student
end
let(:preloaded_attachments) { {} }
context 'user does not have download rights' do
let(:current_user) { user_factory }
it 'returns match_url with preview=1' do
expect(processed_url).to eq "#{match_url}&no_preview=1"
describe '#processed_url' do
it 'delegates to ProcessedUrl' do
expect(processed_url).to match(/#{attachment.context_type.tableize}/)
end
context 'but attachment is public' do
let(:is_public) { true }
context 'user does not have download rights' do
let(:current_user) { user_factory }
it 'delegates to ProcessedUrl' do
expect(processed_url).to match(/#{attachment.context_type.tableize}/)
it 'returns match_url with preview=1' do
expect(processed_url).to eq "#{match_url}&no_preview=1"
end
context 'but attachment is public' do
let(:is_public) { true }
it 'delegates to ProcessedUrl' do
expect(processed_url).to match(/#{attachment.context_type.tableize}/)
end
end
end
end
@ -153,9 +154,7 @@ describe UserContent::FilesHandler do
)
end
before(:each) do
allow(subject).to receive(:user_can_access_attachment?).and_return false
end
before(:each) { allow(subject).to receive(:user_can_access_attachment?).and_return false }
context 'url contains invalid uri' do
# single quotes will make it valid uri, so keep this in double quotes

165
spec/lib/user_content/files_handler_spec_new.rb Normal file
View File

@ -0,0 +1,165 @@
#
# Copyright (C) 2020 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
# This file is a copy of ./files_handler_spec.rb, but runs with the
# new_file_url_rewriting flag on, while the other files has it off
require 'spec_helper'
describe UserContent::FilesHandler do
let(:is_public) { false }
let(:in_app) { false }
let(:course) { course_factory(active_all: true) }
let(:attachment) do
attachment_with_context(course, { filename: 'test.mp4', content_type: 'video' })
end
let(:match_url) do
[attachment.context_type.tableize, attachment.context_id, 'files', match_part].join('/')
end
let(:match_part) { 'download?wrap=1' }
let(:uri_match) do
UserContent::FilesHandler::UriMatch.new(
OpenStruct.new(
{
url: match_url,
type: 'files',
obj_class: Attachment,
obj_id: attachment.id,
rest: "/#{match_part}"
}
)
)
end
let(:new_file_url_rewriting) { Account.site_admin.enable_feature!(:new_file_url_rewriting) }
context 'with new_file_url_rewriting on' do
before(:each) { new_file_url_rewriting }
describe UserContent::FilesHandler::ProcessedUrl do
subject(:processed_url) do
UserContent::FilesHandler::ProcessedUrl.new(
match: uri_match, attachment: attachment, is_public: is_public, in_app: in_app
)
.url
end
describe '#url' do
it 'includes context class' do
expect(processed_url).to match(/#{attachment.context_type.tableize}/)
end
it 'includes wrap=1' do
query_string = processed_url.split('?')[1]
expect(Rack::Utils.parse_nested_query(query_string)['wrap']).to eq '1'
end
it 'includes verifier query param' do
query_string = processed_url.split('?')[1]
expect(Rack::Utils.parse_nested_query(query_string)).to have_key('verifier')
end
context 'is in_app' do
let(:in_app) { true }
it 'does not include verifier' do
query_string = processed_url.split('?')[1]
expect(Rack::Utils.parse_nested_query(query_string)).not_to have_key('verifier')
end
end
context 'and match is a preview' do
let(:match_part) { 'preview' }
it 'is a preview url' do
expect(processed_url).to match(%r{files\/(\d)+\/preview})
end
it 'does not include wrap param' do
query_string = processed_url.split('?')[1]
expect(Rack::Utils.parse_nested_query(query_string)).not_to have_key('wrap')
end
end
context 'when download_frd=1' do
let(:match_part) { '?download_frd=1' }
it 'includes /download in the url' do
expect(processed_url).to match(%r{files\/(\d)+\/download})
end
end
context 'when no download_frd' do
let(:match_part) { '?wrap=1' }
it 'omits /download in the url' do
expect(processed_url).to match(%r{files\/(\d)+(\?|$)})
end
end
context 'when attachment does not support relative paths' do
let(:attachment) { attachment_with_context(submission_model) }
it 'should not include context name' do
expect(processed_url).not_to match(/#{attachment.context_type.tableize}/)
end
end
end
end
describe UserContent::FilesHandler do
subject(:processed_url) do
UserContent::FilesHandler.new(
match: uri_match,
context: attachment.context,
user: current_user,
preloaded_attachments: preloaded_attachments,
is_public: is_public,
in_app: in_app
)
.processed_url
end
let(:current_user) do
student_in_course(active_all: true, course: attachment.context)
@student
end
let(:preloaded_attachments) { {} }
describe '#processed_url' do
it 'delegates to ProcessedUrl' do
expect(processed_url).to match(/#{attachment.context_type.tableize}/)
end
context 'user does not have download rights' do
let(:current_user) { user_factory }
it 'returns match_url with preview=1' do
expect(processed_url).to eq "#{match_url}&no_preview=1"
end
context 'but attachment is public' do
let(:is_public) { true }
it 'delegates to ProcessedUrl' do
expect(processed_url).to match(/#{attachment.context_type.tableize}/)
end
end
end
end
end
end
end