Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

wip #1

Merged
merged 10 commits into from
Mar 19, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
# pdfjs_viewer-rails
**NOTE:** This is DOBT's fork of https://github.com/senny/pdfjs_viewer-rails
We've made the following changes:
- Replaced mozilla's domains with our own in the `HOSTED_VIEWER_ORIGINS` whitelist.
- this whitelist allows us to make cross origin requests for files (i.e. to s3).
- https://github.com/mozilla/pdf.js/pull/6916 is a summary of why this whitelist exists
- Renames main engine controller from `PdfjsViewer::ApplicationController` to `PdfjsViewer::PdfApplicationController` to prevent namespace collisions with our main app's ApplicationController
- Allows the engine routes to be embedded as iframes only under our base url
- implemented with a combination of `X-Frame-Options` and `Content-Security-Policy` headers
- Restricts the domains that files will be loaded from to a whitelist
- Viewer can only be loaded in an iframe


[![Build Status](https://travis-ci.org/senny/pdfjs_viewer-rails.svg?branch=master)](https://travis-ci.org/senny/pdfjs_viewer-rails)
# pdfjs_viewer-rails

## Installation

Expand Down
29 changes: 25 additions & 4 deletions app/assets/javascripts/pdfjs_viewer/viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -6444,19 +6444,39 @@ var pdfjsWebLibs;
});
}
};

// For DOBT's usage we don't want anyone to be able to load the pdf
// viewer as a standalone page.
var ensureLoadedInIframe;
ensureLoadedInIframe = function ensureViewerFocused() {
if (window.self === window.top) {
throw new Error('Unauthorized usage.');
}
};
var validateFileURL;
var HOSTED_VIEWER_ORIGINS = [
'null',
'http://mozilla.github.io',
'https://mozilla.github.io'
'https://attachments.dobt.dev',
'https://attachments.staging.dobt.co',
'https://attachments.dobt.co',
];
var FILE_ORIGIN_WHITELIST = [
'https://screendoor.dobt.dev',
'https://dobt-screendoor-staging.s3.amazonaws.com',
'https://dobt-screendoor.s3.amazonaws.com'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be pretty cool to just include this in production and the previous line in dev/staging, but definitely not necessary. I'm not sure how we'd even do that well if this is sitting in a separate gem, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to avoid dealing with injecting configuration into this file. The less we change this file, the easier it will be to pull in upstream updates from the pdf.js repo.

];
validateFileURL = function validateFileURL(file) {
try {
var viewerOrigin = new URL(window.location.href).origin || 'null';
var fileOrigin = new URL(file, window.location.href).origin;
if (HOSTED_VIEWER_ORIGINS.indexOf(viewerOrigin) >= 0) {
return;
// For DOBT's usage we want to restrict the addresses that the viewer will load files from.
if (FILE_ORIGIN_WHITELIST.indexOf(fileOrigin) >= 0) {
return;
} else {
throw new Error('Unpermitted file.');
}
}
var fileOrigin = new URL(file, window.location.href).origin;
if (fileOrigin !== viewerOrigin) {
throw new Error('file origin does not match viewer\'s');
}
Expand Down Expand Up @@ -6489,6 +6509,7 @@ var pdfjsWebLibs;
var queryString = document.location.search.substring(1);
var params = parseQueryString(queryString);
file = 'file' in params ? params.file : DEFAULT_URL;
ensureLoadedInIframe();
validateFileURL(file);
var waitForBeforeOpening = [];
var appConfig = PDFViewerApplication.appConfig;
Expand Down
4 changes: 0 additions & 4 deletions app/controllers/pdfjs_viewer/application_controller.rb

This file was deleted.

4 changes: 4 additions & 0 deletions app/controllers/pdfjs_viewer/pdf_application_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
module PdfjsViewer
class PdfApplicationController < ActionController::Base
end
end
28 changes: 26 additions & 2 deletions app/controllers/pdfjs_viewer/viewer_controller.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,38 @@
module PdfjsViewer
class ViewerController < ApplicationController
class ViewerController < PdfApplicationController
layout false

after_action :allow_embedding_in_iframe

def full
end

def minimal
end

def reduced
end

private

def allow_embedding_in_iframe
# By default Rails sends the 'X-Frame-Options: SAMEORIGIN' header with responses.
# This means that the response can only be rendered on an iframe whose parent has the same
# origin as the response.

# We are mounting this engine under a different subdomain from the main app, so the default Rails
# XFO header breaks the feature. The purpose of this method is to allow pdfs to be embedded
# by a specific domain (the screendoor domain).

# There are two ways to control which domains are allowed to embed documents as iframes:
# X-Frame-Options and Content-Security-Policy. XFO is considered obsolete with CSP.
# XFO offers the ALLOW-FROM directive, which only allows you to specify one domain. Chrome
# actively ignores this directive! CSP offers frame-ancestors which lets us provide a list
# of domains. Only modern browsers support CSP (read: not IE/edge)
# So we will send both.

response.headers['X-Frame-Options'] = "ALLOW-FROM #{::Rails.configuration.x.host_with_protocol}"
response.headers['Content-Security-Policy'] = "frame-ancestors #{::Rails.configuration.x.host_with_protocol}"
end
end
end