Add instructions for working around NoScript XSS upload problem #4153
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #4153 +/- ##
===========================================
+ Coverage 84.78% 84.79% +0.01%
===========================================
Files 43 43
Lines 2780 2782 +2
Branches 303 303
===========================================
+ Hits 2357 2359 +2
Misses 355 355
Partials 68 68
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
(I'm commenting and not "requesting changes" in a blocking fashion such that if my comments are addressed and this is ready to merge tomorrow before I am not online, another maintainer should feel free to merge)
UI changes and tests look great! Just two things to resolve:
- add
gettextaround the strings on the new page (disable-noscript-xss.html) - update the Apache AppArmor profile, see this diff:
diff --git a/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/usr.sbin.apache2 b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/usr.sbin.apache2
index cc60713dc..5fe5bb89e 100644
--- a/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/usr.sbin.apache2
+++ b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/usr.sbin.apache2
@@ -211,6 +211,7 @@
/var/www/securedrop/source_templates/banner_warning_flashed.html r,
/var/www/securedrop/source_templates/base.html r,
/var/www/securedrop/source_templates/error.html r,
+ /var/www/securedrop/source_templates/disable-noscript-xss.html r,
/var/www/securedrop/source_templates/first_submission_flashed_message.html r,
/var/www/securedrop/source_templates/flashed.html r,
/var/www/securedrop/source_templates/generate.html r,
@@ -266,6 +267,12 @@
/var/www/securedrop/static/i/font-awesome/trash-black.png r,
/var/www/securedrop/static/i/toronion.png r,
/var/www/securedrop/static/i/hand_with_fingerprint.png r,
+ /var/www/securedrop/static/i/noscript-xss/advanced-settings.png r,
+ /var/www/securedrop/static/i/noscript-xss/noscript-icon-plain.png r,
+ /var/www/securedrop/static/i/noscript-xss/noscript-icon.png r,
+ /var/www/securedrop/static/i/noscript-xss/noscript-pane.png r,
+ /var/www/securedrop/static/i/noscript-xss/noscript-settings-icon.png r,
+ /var/www/securedrop/static/i/noscript-xss/settings-pane.png r,
/var/www/securedrop/static/i/success_checkmark.png r,
/var/www/securedrop/static/i/relieved_face.png r,
/var/www/securedrop/static/i/server_upload.png r,|
|
||
| <hr class="dotted"> | ||
|
|
||
| <h2>2. In the NoScript pane, click on the Settings icon: <img src="{{ url_for('static', filename='i/noscript-xss/noscript-settings-icon.png') }}"></h2> |
There was a problem hiding this comment.
Let's wrap these strings in gettext() such that they will be extracted for localization
There was a problem hiding this comment.
Updated the AppArmor profile and added gettext. Switched locale and tested local translations, and I believe all the new text is now marked for translation.
securedrop/tests/test_source.py
Outdated
| resp = app.get(url_for('info.disable_noscript_xss')) | ||
| assert resp.status_code == 200 | ||
| text = resp.data.decode('utf-8') | ||
| print(text) |
There was a problem hiding this comment.
print statement left in from debugging i think
There was a problem hiding this comment.
looks good now, thanks @rmol! (note: I tested AppArmor profile in staging, no problems detected)
|
|
||
| <p class="explanation">{{ gettext('You can submit any kind of file, a message, or both.') }}</p> | ||
| <p class="explanation"><b>1.</b> {{ gettext('Before you begin, <a href="{url}" class="text-link">turn off cross-site request sanitization in NoScript</a>. This is <strong>required</strong> for uploads to work.').format(url=url_for('info.disable_noscript_xss'))}}</p> | ||
|
|
There was a problem hiding this comment.
Per the final messaging, instead of linking the "turn off" text, there should be a "Show me how" link at the end. (The UX reasoning is that a "turn off" link could cause inexperienced users to think that clicking the link will immediately change their settings.)
|
hey @rmol can you modify this PR to no longer be in draft mode? |
rmol
requested review from
conorsch,
emkll,
heartsucker,
kushaldas and
msheiny
as code owners
There is currently an unpredictable problem with uploads, due to a Firefox bug. For now, the only workarounds are implementing AJAX uploads or disabling NoScript's cross-site request sanitization. We're recommending the latter, since it should be safe with our recommended Tor Browser settings, and doesn't require enabling JavaScript to use SecureDrop. This change adds: - instructions in the source UI for disabling NoScript cross-site request sanitization before uploading - a note about the bug in the source guide - updated screenshots of the source submission page Fixes: freedomofpress#4078
rmol
force-pushed
the
noscript-workaround
branch
from
cdaa565 to
4d7851a
Compare
There is currently an unpredictable problem with uploads, due to a
Firefox bug. For now, the only workarounds are implementing AJAX
uploads or disabling NoScript's cross-site request sanitization. We're
recommending the latter, since it should be safe with our recommended
Tor Browser settings, and doesn't require enabling JavaScript to use
SecureDrop.
Status
Ready for review / Work in progress
Description of Changes
Fixes #4078.
This change adds:
request sanitization before uploading
Testing
Please proofread the changes to the instructions both on the source submission page and in the source guide.
Deployment
There are no special considerations for deployment.
Checklist
If you made changes to the server application code:
make ci-lint) and tests (make -C securedrop test) pass in the development containerIf you made changes to documentation:
make docs-lint) passed locally