Provide end-of life messaging and disable on source interface after Xenial EOL #5789

sssoleileraaa · 2021-02-13T01:07:08Z

Status

Ready for review

Description of Changes

Resolves #5688

Needs review of messaging to admins/source/journalists.

Changes proposed in this pull request:

Testing

build both xenial and focal securedrop-app-code debs off of this pr branch with the edit from step 1
install one on your home server to make sure app armor changes are working and install the other on a staging vm
log into the journalist interface
- for xenial, confirm you see the critical eol warning + mention of April 30, 2021 date + learn more link that takes you to: https://securedrop.org/news/securedrop-advisory-end-life-v2-onion-services-and-ubuntu-1604/
- for focal, confirm you do not see the critical eol warning
- for both, confirm you do not see any v2 warning
visit the source interface
- for xenial, confirm you see that the interface is disabled
- for focal, confirm you see that the interface is enabled
change the dates in source_app/disable.py and jouranlist_app/os_eol.py to year 2020 instead of 2021
build the debs and install again
log into the journalist interface
- for xenial, confirm you see the critical eol warning + mention of Source Interface being disabled + learn more link that takes you to: https://securedrop.org/news/securedrop-advisory-end-life-v2-onion-services-and-ubuntu-1604/
- for focal, confirm you do not see the critical eol warning
- for both, confirm you do not see any v2 warning
visit the source interface
- for xenial, confirm you see that the interface is disabled
- for focal, confirm you see that the interface is enabled

New test

log into the source interface on a dev server
change EOL date so that the source interface is disabled
try hitting endpoints while source interface is disabled
- you see a message about securedrop being unavailable
- no logout button shows up
change EOL date so that the source interface is not longer disabled
try hitting endpoints
- verify that you have to log in again

Checklist

Linting (make lint) and tests (make test) pass in the development container
I have updated AppArmor rules to include the change

lgtm-com · 2021-02-13T01:15:10Z

This pull request introduces 6 alerts when merging 6973797 into f4b7805 - view on LGTM.com

new alerts:

5 for Unused import
1 for Unused local variable

lgtm-com · 2021-02-13T02:12:25Z

This pull request introduces 1 alert when merging 6e382b5 into f4b7805 - view on LGTM.com

new alerts:

1 for Unused local variable

lgtm-com · 2021-02-13T02:37:17Z

This pull request introduces 1 alert when merging 2f99b13 into f4b7805 - view on LGTM.com

new alerts:

1 for Unused local variable

emkll

Thanks @creviera , I've gone through the test plan on a staging instance and functionally everything is working with the exception of the instance's logo (see below)

This is because the decorators are applied for the org-logo route serving the icon, and not behind /static, so we may need to add the following changes to the decorators:

diff --git a/securedrop/source_app/decorators.py b/securedrop/source_app/decorators.py
index 2fae92b3d..786a80a3d 100644
--- a/securedrop/source_app/decorators.py
+++ b/securedrop/source_app/decorators.py
@@ -22,7 +22,7 @@ def ignore_static(f: Callable) -> Callable:
     a static resource."""
     @wraps(f)
     def decorated_function(*args: Any, **kwargs: Any) -> Any:
-        if request.path.startswith('/static'):
+        if request.path.startswith('/static')  or request.path == '/org-logo':
             return  # don't execute the decorated function
         return f(*args, **kwargs)
     return decorated_function

Good idea updating the language once the source interface is disabled:

securedrop/journalist_app/os_eol.py

securedrop/source_app/disable.py

securedrop/journalist_templates/base.html

securedrop/journalist_app/__init__.py

sssoleileraaa · 2021-02-16T23:53:12Z

This is because the decorators are applied for the org-logo route serving the icon, and not behind /static, so we may need to add the following changes to the decorators:

diff --git a/securedrop/source_app/decorators.py b/securedrop/source_app/decorators.py
index 2fae92b3d..786a80a3d 100644
--- a/securedrop/source_app/decorators.py
+++ b/securedrop/source_app/decorators.py
@@ -22,7 +22,7 @@ def ignore_static(f: Callable) -> Callable:
     a static resource."""
     @wraps(f)
     def decorated_function(*args: Any, **kwargs: Any) -> Any:
-        if request.path.startswith('/static'):
+        if request.path.startswith('/static')  or request.path == '/org-logo':
             return  # don't execute the decorated function
         return f(*args, **kwargs)
     return decorated_function

Trying to figure out why this doesn't work for me. The logo still doesn't show up.

sssoleileraaa · 2021-02-17T00:12:47Z

Okay, the logo started showing up after I add that change ^ and uploaded a custom logo.

sssoleileraaa · 2021-02-17T00:47:04Z

In the case where someone is logged in during which the eol of the os is reached, we should log that user out before showing the "We're sorry, our SecureDrop is currently offline." Right now we show the logout button with the error message but the user actually won't be able to log out since everything is disabled.

Update

See new test (#5789 (comment)) to verify ^

sssoleileraaa · 2021-02-17T22:05:31Z

"ERROR: Unable to contact the Docker daemon. Please refer to https://docs.docker.com/config/daemon/ for managing the daemon" is the same error seen in #5798. Perhaps we are not configuring the daemon properly or something wrong with the image. This is from our config file:

  deb-tests-focal:
    docker:
      - image: cimg/python:3.7
    environment:
      LC_ALL: C.UTF-8
      LANG: C.UTF-8
    steps:
      - run: sudo apt-get update && sudo apt-get install -y make virtualenv enchant jq python3-dev build-essential rsync
      - checkout
      - setup_remote_docker
      - run:
          name: Test Debian package build on Focal
          command: |
            make ci-deb-tests-focal

versus xenial:

  deb-tests:
    docker:
      - image: cimg/python:3.7
    environment:
      LC_ALL: C.UTF-8
      LANG: C.UTF-8
    steps:
      - run: sudo apt-get update && sudo apt-get install -y make virtualenv enchant jq python3-dev build-essential rsync
      - checkout
      - setup_remote_docker
      - run:
          name: Test Debian package build
          command: |
            BRANCH_MATCH=$(devops/scripts/match-ci-branch.sh "^update-builder")
            if ! [[ $BRANCH_MATCH =~ ^found ]]; then echo "Skipping: ${BRANCH_MATCH}"; exit 0; fi
            make ci-deb-tests

🤔

emkll · 2021-02-18T15:50:32Z

rebased on latest develop after merge of #5801

eloquence · 2021-02-18T18:33:45Z

I've committed to helping with UI/string review before merge.

emkll

Thanks @creviera for the changes. I went through the test plan and confirmed the functionality is still working as expected

I've added a minor suggestion inline to reduce the diff, since we will likely revert these changes once SecureDrop 2.0.0 is released and Xenial support is eliminated.

Once @eloquence's reviews the strings, I would ask that you please rebase/squash in a single commit.

I will be happy to provide final review once the steps above have been completed, otherrwise we should consider these changes approved.

emkll · 2021-02-18T20:38:49Z

securedrop/source_templates/base.html

          {% if 'logged_in' in session %}
-            <a href="{{ url_for('main.logout') }}" class="btn pull-right" id="logout">{{ gettext('LOG OUT') }}</a>
+          <a href="{{ url_for('main.logout') }}" class="btn pull-right" id="logout">{{ gettext('LOG OUT') }}</a>


looks like the changes on lines 37, 41, 44 and 46 are not strictly required here and make the diff more complicated to read

ah, i see. i was trying to mimic the code style i saw in other areas of the code. it looked like other areas of the code try to keep the indents of the html correct rather than the template code, but if you want i can revert to make it easier for you to review.

easier to review but also preserves a cleaner history, making it easier to revert changes in the future if/when this code is no longer be necessary (i.e. 2.0)

eloquence · 2021-02-18T21:07:01Z

(Taking a look now.)

eloquence · 2021-02-18T21:14:52Z

securedrop/journalist_templates/base.html

+    </div>
+    {% elif g.show_os_near_eol_warning %}
+    <div id="os-near-eol" class="alert-banner">
+      <img src="{{ url_for('static', filename='i/bang-circle.png') }}" width="20" height="20"> {{ gettext ('<strong>Critical Security:</strong>&nbsp;&nbsp;The operating system used by your SecureDrop servers is approaching its end-of-life on April 30, 2021. A manual update is urgently required to remain safe. Please contact your adminstrator. <a href="//securedrop.org/xenial-eol" rel="noreferrer">Learn More</a>') }}


Nit: I would say will reach its end-of-life on April 30 and not is approaching its end-of-life on April 30; the latter could be interpreted (and translated) to suggest that end-of-life will start approaching on April 30.

eloquence · 2021-02-18T21:34:31Z

install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/usr.sbin.apache2

@@ -1,4 +1,4 @@
-# Last Modified: Wed Oct 29 08:16:32 2014
+# Last Modified: Wed Feb 10 16:27:32 2021


Clearly not the most well-maintained timestamp. ;-) Maybe we should just ditch it?

eloquence · 2021-02-18T21:51:24Z

I verified

pre-EOL state
post-EOL state
Source Interface post-EOL state

in the Docker-based dev env. UI and strings look great to me; just one small recommended tweak.

sssoleileraaa · 2021-02-18T23:20:05Z

the only reason id like to push back a little on the request to rebase into a single commit is because each unit is in a clear commit and we'll probably revisit these commits once focal approaches its EOL. the three commits are:

remove v2 and v2+v3 warning
disable source interface for xenial after april 30th
add server os eol warning

actually, it might make sense to combine the bottom two.

emkll · 2021-02-19T17:57:40Z

Good point, I agree: a separate commit to remove the v2/v2+v3 warnings makes sense here. I would still suggest squashing the two others, at your option.

Signed-off-by: Allie Crevier <allie@freedom.press>

emkll

lgtm!

sssoleileraaa requested review from conorsch, emkll, kushaldas and redshiftzero as code owners February 13, 2021 01:07

sssoleileraaa force-pushed the v2-and-xenial-eol branch from 6973797 to 6e382b5 Compare February 13, 2021 02:04

sssoleileraaa force-pushed the v2-and-xenial-eol branch from 6e382b5 to 2f99b13 Compare February 13, 2021 02:28

emkll changed the title ~~V2 and xenial eol~~ Provide end-of life messaging and disable on source interface after Xenial EOL Feb 15, 2021

emkll suggested changes Feb 16, 2021

View reviewed changes

securedrop/journalist_app/os_eol.py Outdated Show resolved Hide resolved

securedrop/source_app/disable.py Outdated Show resolved Hide resolved

securedrop/journalist_templates/base.html Outdated Show resolved Hide resolved

emkll reviewed Feb 16, 2021

View reviewed changes

securedrop/journalist_app/__init__.py Outdated Show resolved Hide resolved

eloquence added this to the 1.8.0 milestone Feb 17, 2021

sssoleileraaa force-pushed the v2-and-xenial-eol branch 3 times, most recently from 4c56f48 to 68e332b Compare February 17, 2021 18:39

sssoleileraaa mentioned this pull request Feb 17, 2021

deb-tests-focal in CI is failing with: ERROR: Unable to contact the Docker daemon #5799

Closed

emkll force-pushed the v2-and-xenial-eol branch from 68e332b to a80255a Compare February 18, 2021 15:50

emkll reviewed Feb 18, 2021

View reviewed changes

eloquence reviewed Feb 18, 2021

View reviewed changes

sssoleileraaa force-pushed the v2-and-xenial-eol branch from a80255a to 9b56878 Compare February 19, 2021 18:25

Allie Crevier added 2 commits February 19, 2021 10:43

remove v2 and v2+v3 warning

e1e9aa4

Signed-off-by: Allie Crevier <allie@freedom.press>

inform user about xenial EOL and disable SI if reached

ecfecea

sssoleileraaa force-pushed the v2-and-xenial-eol branch from 9b56878 to ecfecea Compare February 19, 2021 18:43

emkll approved these changes Feb 19, 2021

View reviewed changes

emkll merged commit 33f94c9 into develop Feb 19, 2021

emkll deleted the v2-and-xenial-eol branch February 19, 2021 22:00

kushaldas mentioned this pull request Feb 26, 2021

Release SecureDrop 1.8.0 #5794

Closed

27 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Provide end-of life messaging and disable on source interface after Xenial EOL #5789

Provide end-of life messaging and disable on source interface after Xenial EOL #5789

sssoleileraaa commented Feb 13, 2021 •

edited by emkll

Loading

lgtm-com bot commented Feb 13, 2021

lgtm-com bot commented Feb 13, 2021

lgtm-com bot commented Feb 13, 2021

emkll left a comment

sssoleileraaa commented Feb 16, 2021

sssoleileraaa commented Feb 17, 2021

sssoleileraaa commented Feb 17, 2021 •

edited

Loading

sssoleileraaa commented Feb 17, 2021

emkll commented Feb 18, 2021

eloquence commented Feb 18, 2021

emkll left a comment

emkll Feb 18, 2021

sssoleileraaa Feb 18, 2021

emkll Feb 19, 2021

eloquence commented Feb 18, 2021

eloquence Feb 18, 2021

eloquence Feb 18, 2021

eloquence commented Feb 18, 2021 •

edited

Loading

sssoleileraaa commented Feb 18, 2021 •

edited

Loading

emkll commented Feb 19, 2021

emkll left a comment

		@@ -1,4 +1,4 @@
		# Last Modified: Wed Oct 29 08:16:32 2014
		# Last Modified: Wed Feb 10 16:27:32 2021

Provide end-of life messaging and disable on source interface after Xenial EOL #5789

Provide end-of life messaging and disable on source interface after Xenial EOL #5789

Conversation

sssoleileraaa commented Feb 13, 2021 • edited by emkll Loading

Status

Description of Changes

Testing

New test

Checklist

lgtm-com bot commented Feb 13, 2021

lgtm-com bot commented Feb 13, 2021

lgtm-com bot commented Feb 13, 2021

emkll left a comment

Choose a reason for hiding this comment

sssoleileraaa commented Feb 16, 2021

sssoleileraaa commented Feb 17, 2021

sssoleileraaa commented Feb 17, 2021 • edited Loading

Update

sssoleileraaa commented Feb 17, 2021

emkll commented Feb 18, 2021

eloquence commented Feb 18, 2021

emkll left a comment

Choose a reason for hiding this comment

emkll Feb 18, 2021

Choose a reason for hiding this comment

sssoleileraaa Feb 18, 2021

Choose a reason for hiding this comment

emkll Feb 19, 2021

Choose a reason for hiding this comment

eloquence commented Feb 18, 2021

eloquence Feb 18, 2021

Choose a reason for hiding this comment

eloquence Feb 18, 2021

Choose a reason for hiding this comment

eloquence commented Feb 18, 2021 • edited Loading

sssoleileraaa commented Feb 18, 2021 • edited Loading

emkll commented Feb 19, 2021

emkll left a comment

Choose a reason for hiding this comment

sssoleileraaa commented Feb 13, 2021 •

edited by emkll

Loading

sssoleileraaa commented Feb 17, 2021 •

edited

Loading

eloquence commented Feb 18, 2021 •

edited

Loading

sssoleileraaa commented Feb 18, 2021 •

edited

Loading