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

Fix file preview for files-domain with inst-fs sw 

CORS limitations with service workers was breaking file previews for
Safari browsers where we had enabled the service worker based auth. We
add an "instfs" query param to preview to cue the service worker to not
attempt to do auth injection for previews of files not stored in instfs.

Test Plan
========
This functionality is hidden behind a feature flag, and local
development setup is extremely tedious (MRA, files domain, SSL),
so warmfixing and testing in beta is the right path. Once in beta, test
plan:

* Login to beta sandbox
* Create a file served by both inst-fs and files domain
  * Navigate to `/plugins/inst_fs`
  * Disable instfs plugin for your sandbox only
  * Upload a new file
  * Confirm that requests to file ultimately resolve to files domain,
    especially when doing a "file preview"
  * Re-enable instfs plugin, and upload another file
  * Confirm that requests to that file ultimately resolve to inst-fs
* Open same sandbox account in safari 13+ browser with ITP enabled
* Confirm that file preview and file download work for both file served
  from instfs and files domain. Note: running same test on prod system
  should result in files domain file from being previewed properly

closes SAS-1543

Change-Id: I587e4c2eed3eefbfe00f913f4308f57ac9525b4d
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/246406
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Nathan Mills <nathanm@instructure.com>
QA-Review: Jonathan Featherstone <jfeatherstone@instructure.com>
Product-Review: Jonathan Featherstone <jfeatherstone@instructure.com>
This commit is contained in:
Jonathan Featherstone 2020-08-28 15:38:45 -06:00
parent 030307663d
commit eaf502ed22
3 changed files with 15 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
app/jsx/bundles/inst_fs_service_worker.js
View File

@ -23,13 +23,7 @@ ready(() => {
// See service worker definition for comments on purpose and why we only
// install it for recent (13+) Safari.
const {name, version} = getBrowser()
if (name !== 'Safari' || parseFloat(version) < 13) {
console.log('Inst-FS ServiceWorker not necessary for this browser.')
} else if (!('serviceWorker' in navigator)) {
console.log('ServiceWorkers not available. :( Inst-FS files may fail to inline.')
} else if (navigator.serviceWorker.controller) {
console.log('Inst-FS ServiceWorker already registered.')
} else {
if (name === 'Safari' && parseFloat(version) >= 13 && 'serviceWorker' in navigator) {
navigator.serviceWorker
.register('/inst-fs-sw.js')
.then(() => {

2
app/views/file_previews/img_preview.html.erb
View File

@ -17,5 +17,5 @@
%>
<body style="height: 100vh; margin:0; padding: 0;">
<img src="<%= context_url(@context, :context_file_preview_url, @file.id) %>" alt="<%= @file.display_name %>" style="margin: 0; padding:0; max-width: 100%; max-height: 100%; position: absolute; top: 50%; left: 50%; -webkit-transform: translate(-50%, -50%); transform: translate(-50%, -50%);">
<img src="<%= context_url(@context, :context_file_preview_url, @file.id, :instfs => @file.instfs_hosted?) %>" alt="<%= @file.display_name %>" style="margin: 0; padding:0; max-width: 100%; max-height: 100%; position: absolute; top: 50%; left: 50%; -webkit-transform: translate(-50%, -50%); transform: translate(-50%, -50%);">
</body>

14
public/inst-fs-sw.js
View File

@ -43,7 +43,10 @@ self.addEventListener('fetch', function(event) {
function eligibleRequest(request) {
const url = new URL(request.url)
return request.method === 'GET' && request.mode !== 'navigate' && eligiblePath(url.pathname)
return request.method === 'GET'
&& request.mode !== 'navigate'
&& !imagePreviewWithoutInstfs(url)
&& eligiblePath(url.pathname)
}
// most files links we care about look like this regex fragment with a context
@ -54,6 +57,15 @@ function contextFilePath(context) {
return new RegExp(`^/${context}/[^/]+${commonFilePath}$`)
}
function imagePreviewWithoutInstfs(url) {
// Image preview without inst-fs (files domain)
// causes some CORS issues. We ignore that path here.
const previewPath = new RegExp(`/files/[^/]+/preview(.\\w+)?`)
return previewPath.test(url.pathname)
&& url.searchParams.has('instfs')
&& url.searchParams.get('instfs') == 'false'
}
const eligiblePaths = [
contextFilePath(`accounts`),
contextFilePath(`assessment_questions`),