Skip to content

Commit

Permalink
CSP: Enable Content Security Policies (#5186)
Browse files Browse the repository at this point in the history
  • Loading branch information
mishaschwartz authored Apr 14, 2021
1 parent 07b1c73 commit 62b1399
Show file tree
Hide file tree
Showing 135 changed files with 760 additions and 400 deletions.
1 change: 1 addition & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
- Ensure that browsers cache the correct state of overall comments when marking (#5173)
- Ensure that graders are shown the correct annotation categories (#5181)
- Show informative error message if an uploaded criteria yaml file did not contain a "type" key (#5184)
- Enable content security policies (#5186)
- Allow for multiple custom validation messages (#5194)
- Add ability to hold shift to select a range of values in checkbox tables (#5182)
- Update ssh authorization to be more flexible, secure, and permit a single user to use the same public key for multiple instances (#5199)
Expand Down
2 changes: 1 addition & 1 deletion app/assets/javascripts/Components/autotest_manager.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class AutotestManager extends React.Component {
};

toggleFormChanged = (value) => {
this.setState({form_changed: value}, () => set_onbeforeunload(this.state.form_changed));
this.setState({form_changed: value});
};

handleCreateFiles = (files, unzip) => {
Expand Down
5 changes: 5 additions & 0 deletions app/assets/javascripts/Components/groups_manager.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ class GroupsManager extends React.Component {
} else {
modalCreate.open();
$('#new_group_name').val('');
$(function() {
$('#modal-create-close').click(function() {
modalCreate.close();
})
})
}
};

Expand Down
2 changes: 1 addition & 1 deletion app/assets/javascripts/Components/starter_file_manager.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class StarterFileManager extends React.Component {
}

toggleFormChanged = (value) => {
this.setState({form_changed: value}, () => set_onbeforeunload(this.state.form_changed));
this.setState({form_changed: value});
};

fetchData = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ SyntaxHighlighter1p5Adapter.prototype.applyMods = function() {
// Get rid of some commands and add font size commands
delete original_commands['CopyToClipboard'];
delete original_commands['PrintSource'];
delete original_commands['ExpandSource'];

original_commands["BoostCode"] = {
label: '+A',
Expand Down Expand Up @@ -93,6 +94,20 @@ SyntaxHighlighter1p5Adapter.prototype.applyMods = function() {

// Attempt to replace tools menu with these new commands
if (!!document.getElementsByClassName('tools')[0]) {
document.getElementsByClassName('tools')[0].innerHTML = dp.sh.Toolbar.Create('code').innerHTML;
let tools = document.getElementsByClassName('tools')[0];
tools.innerHTML = '';
Object.entries(original_commands).forEach(entry => {
let [name, {label}] = entry;
let tool = document.createElement('a');
let href = document.createAttribute('href');
href.value = '#';
tool.setAttributeNode(href);
tool.addEventListener('click', (e) => {
dp.sh.Toolbar.Command(name, e.target);
e.preventDefault();
});
tool.innerText = label;
tools.appendChild(tool);
});
}
}
12 changes: 8 additions & 4 deletions app/assets/javascripts/ajax_events.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,14 @@ export function renderFlash(event, request) {
const messages = flashMessage.split(';');
const contents = flashDiv.getElementsByClassName('flash-content')[0] || flashDiv;
contents.innerHTML = '';
messages.forEach(message => {
contents.insertAdjacentHTML('beforeend', message);
});
flashDiv.style.display = '';
if (messages.length) {
messages.forEach(message => {
contents.insertAdjacentHTML('beforeend', message);
});
flashDiv.style.display = 'block';
} else {
flashDiv.style.display = 'none';
}
}
}
});
Expand Down
3 changes: 3 additions & 0 deletions app/assets/javascripts/modals.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,8 @@ $(document).ready(() => {
$('.dialog').each((_, dialog) => {
let open_link = dialog.getAttribute('data-open-link') || undefined;
new ModalMarkus('#' + dialog.id, open_link);
$('#' + dialog.id + '-close').click(function() {
dialog.close();
});
})
});
6 changes: 5 additions & 1 deletion app/assets/javascripts/redirect.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
$(document).ready(() => {
$(document).ajaxError((event, xhr) => {
if (xhr.status === 403) {
session_expired_modal.open()
session_expired_modal.open();
$('#session-expired-modal-close').click(function() {
session_expired_modal.close();
window.location.reload();
});
}
});
});
3 changes: 3 additions & 0 deletions app/assets/stylesheets/common/_annotations_dialog.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.annotation_dialog {
width: 600px;
}
70 changes: 70 additions & 0 deletions app/assets/stylesheets/common/_markus.scss
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
@import 'mixins';
@import 'modals';
@import 'notes_dialog';
@import 'annotations_dialog';
@import 'icons';
@import 'react_json_schema_form';
@import 'react_tabs';
Expand Down Expand Up @@ -37,6 +38,8 @@ body {

.flex-row-expand {
flex-grow: 1;
height: 100%;
margin-left: 10px;
}


Expand Down Expand Up @@ -157,6 +160,14 @@ strong a {
width: 2em;
}

.no-display {
display: none;
}

.flex-display {
display: flex;
}

/** Text and number field inputs */

input,
Expand Down Expand Up @@ -346,6 +357,10 @@ fieldset {
grid-template-columns: max-content max-content;
row-gap: 10px;

&.required-file-wrapper {
padding-bottom: 0.75em;
}

label {
text-align: right;
}
Expand Down Expand Up @@ -522,6 +537,22 @@ kbd {
.pane-wrapper {
display: flex;

.annotation-pane-wrapper {
width: 25%;
}

.criteria-pane-wrapper {
width: 37%;
}

&.mid-height {
height: calc(70vh);
}

&.small-bottom-margin {
margin-bottom: 1em;
}

.pane {
border: 1px solid $primary-three;
border-radius: $radius;
Expand All @@ -537,6 +568,20 @@ kbd {
background-color: $pane-highlight;
}

&.assignment-list-wrapper {
flex: 0.5;
width: 400px;
}

&.scrollable {
overflow-y: scroll;
}

&.slim-fixed {
flex: 0 auto;
width: 29.5%;
}

// TODO: decide whether to adopt this style more broadly
// (currently only used in assignments/show and assignments/peer_review)
&.block {
Expand Down Expand Up @@ -1077,6 +1122,10 @@ nav {
position: relative; // For image annotations
}

#crop-target {
border: 1px solid black;
max-width: 400px;
}

// Text editing and previews
.preview {
Expand Down Expand Up @@ -1251,3 +1300,24 @@ nav {
}
}

// Styling for grade_entry_forms/_form.html

.grade-entry-items-wrapper {
margin-top: 1em;
}

.add-new-button-wrapper {
float: right;
}

// Styling for graphs

.ta-grade-distribution-graph-wrapper {
display: inline-block;
width: 400px;
}

#gef_data_chart {
padding-bottom: 0;
text-align: center;
}
15 changes: 15 additions & 0 deletions app/assets/stylesheets/common/core.scss
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,21 @@ label.required::after, th.required::after {
clear: both;
}

// indents

.indent {
margin-left: 15px;
}

.small-indent {
margin-left: 10px;
}

// text boxes

.word-break-all {
word-break: break-all;
}

// Customize react-table styling
.ReactTable {
Expand Down
6 changes: 6 additions & 0 deletions app/controllers/annotation_categories_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ class AnnotationCategoriesController < ApplicationController

responders :flash

content_security_policy only: :index do |p|
# required because MathJax dynamically changes
# style. # TODO: remove this when possible
p.style_src :self, "'unsafe-inline'"
end

def index
@assignment = Assignment.find(params[:assignment_id])
@annotation_categories = AnnotationCategory.visible_categories(@assignment, current_user)
Expand Down
6 changes: 6 additions & 0 deletions app/controllers/assignments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@ class AssignmentsController < ApplicationController
responders :flash
before_action { authorize! }

content_security_policy only: [:edit, :new] do |p|
# required because jquery-ui-timepicker-addon inserts style
# dynamically. TODO: remove this when possible
p.style_src :self, "'unsafe-inline'"
end

# Publicly accessible actions ---------------------------------------

def show
Expand Down
10 changes: 10 additions & 0 deletions app/controllers/automated_tests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,16 @@ class AutomatedTestsController < ApplicationController

before_action { authorize! }

content_security_policy only: :manage do |p|
# required because jquery-ui-timepicker-addon inserts style
# dynamically. TODO: remove this when possible
p.style_src :self, "'unsafe-inline'"
# required because react-jsonschema-form uses ajv which calls
# eval (javascript) and creates an image as a blob.
# TODO: remove this when possible
p.script_src :self, "'strict-dynamic'", "'unsafe-eval'"
end

def update
assignment = Assignment.find(params[:assignment_id])
test_specs = params[:schema_form_data]
Expand Down
4 changes: 4 additions & 0 deletions app/controllers/exam_templates_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ class ExamTemplatesController < ApplicationController

layout 'assignment_content'

content_security_policy only: [:assign_errors] do |p|
p.img_src :self, :blob
end

def index
@assignment = Assignment.find(params[:assignment_id])
@exam_templates = @assignment.exam_templates.includes(:template_divisions)
Expand Down
4 changes: 4 additions & 0 deletions app/controllers/groups_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ class GroupsController < ApplicationController
before_action { authorize! }
layout 'assignment_content'

content_security_policy only: [:assign_scans] do |p|
p.img_src :self, :blob
end

# Group administration functions -----------------------------------------
# Verify that all functions below are included in the authorize filter above

Expand Down
6 changes: 1 addition & 5 deletions app/controllers/main_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -280,11 +280,7 @@ def clear_role_switch_session
end

def check_timeout
if check_imminent_expiry
render js: 'timeout_imminent_modal.open()'
else
head :ok
end
head :ok unless check_imminent_expiry
end

def refresh_session
Expand Down
11 changes: 11 additions & 0 deletions app/controllers/results_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,17 @@ class ResultsController < ApplicationController
only: [:update_remark_request, :cancel_remark_request,
:set_released_to_students]

content_security_policy only: [:edit, :view_marks] do |p|
# required because heic2any uses libheif which calls
# eval (javascript) and creates an image as a blob.
# TODO: remove this when possible
p.script_src :self, "'strict-dynamic'", "'unsafe-eval'"
p.img_src :self, :blob
# required because MathJax dynamically changes
# style. # TODO: remove this when possible
p.style_src :self, "'unsafe-inline'"
end

def show
respond_to do |format|
format.json do
Expand Down
8 changes: 8 additions & 0 deletions app/controllers/submissions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@ class SubmissionsController < ApplicationController
include RepositoryHelper
before_action { authorize! }

content_security_policy only: [:repo_browser, :file_manager] do |p|
# required because heic2any uses libheif which calls
# eval (javascript) and creates an image as a blob.
# TODO: remove this when possible
p.script_src :self, "'strict-dynamic'", "'unsafe-eval'"
p.img_src :self, :blob
end

def index
respond_to do |format|
format.json do
Expand Down
4 changes: 2 additions & 2 deletions app/views/admins/index.html.erb
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
<% content_for :head do %>
<script>
<%= javascript_tag nonce: true do %>
document.addEventListener('DOMContentLoaded', () => {
makeAdminTable(document.getElementById('admins_table'));
})
</script>
<% end %>
<% end %>

<% content_for :title do %>
Expand Down
Loading

0 comments on commit 62b1399

Please sign in to comment.