Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Enable Content Security Policies #5186

Merged
merged 23 commits into from
Apr 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
36d6fe2
CSP: enable csp policies... initial attempt
mishaschwartz Mar 19, 2021
4ea238a
style: style fixes [ci skip]
mishaschwartz Mar 22, 2021
aac3802
syntax: fix syntax error in view
mishaschwartz Mar 22, 2021
4a61e83
CSP: more updates
mishaschwartz Mar 30, 2021
048da49
CSP: even more updates
mishaschwartz Mar 31, 2021
d2bdf5c
CSP: updates III
mishaschwartz Apr 1, 2021
0fee0ad
CSP: updates, mostly to inline styles this time
mishaschwartz Apr 1, 2021
e1be807
CSP: updates, enable minimal unsafe configurations for full functiona…
mishaschwartz Apr 5, 2021
e2de7ec
style: update style
mishaschwartz Apr 5, 2021
7baf825
style: update more style
mishaschwartz Apr 5, 2021
5609e70
style: update more style
mishaschwartz Apr 5, 2021
2972de3
changelog: update changelog [ci skip]
mishaschwartz Apr 6, 2021
f27d8c1
CSP: simplify defaults and add specific policies for routes (MathJax)
mishaschwartz Apr 6, 2021
6b9b70e
csp: clean up additional js bugs, add hole in security policy for rea…
mishaschwartz Apr 12, 2021
4cf80fb
style: style fixes [ci skip]
mishaschwartz Apr 12, 2021
65a49b0
js: add callback after session expired modal is rendered
mishaschwartz Apr 12, 2021
8fd8db6
flash-messages: hide flash message button needs to work even if jquer…
mishaschwartz Apr 12, 2021
370c352
review: update for review suggestions
mishaschwartz Apr 13, 2021
4d68a7c
review: show pdfs in exam templates and enable source code glower fun…
mishaschwartz Apr 13, 2021
a25ed20
style: style fixes
mishaschwartz Apr 13, 2021
62faf43
style: style fixes
mishaschwartz Apr 13, 2021
48eb887
csp: add blob permissions for assign_scans route
mishaschwartz Apr 14, 2021
06b127c
Merge branch 'master' into enable-basic-csp
david-yz-liu Apr 14, 2021
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
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 {
mishaschwartz marked this conversation as resolved.
Show resolved Hide resolved
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 {
mishaschwartz marked this conversation as resolved.
Show resolved Hide resolved
border: 1px solid black;
mishaschwartz marked this conversation as resolved.
Show resolved Hide resolved
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 {
mishaschwartz marked this conversation as resolved.
Show resolved Hide resolved
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