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

Eliminate flag-for-reply process #5954

Merged
merged 3 commits into from Jun 7, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 0 additions & 7 deletions install_files/ansible-base/roles/app/handlers/main.yml
Expand Up @@ -11,14 +11,7 @@
name: apache2
state: restarted

## Here, we list apparmor before haveged to ensure the correct AppArmor
## profile is loaded prior to restarting haveged
- name: restart apparmor
service:
name: apparmor
state: restarted

- name: restart haveged
service:
name: haveged
state: restarted

This file was deleted.

2 changes: 0 additions & 2 deletions install_files/ansible-base/roles/app/tasks/main.yml
Expand Up @@ -17,5 +17,3 @@
- include: install_and_harden_apache.yml

- include: setup_cron.yml

- include: configure_haveged.yml
Expand Up @@ -169,6 +169,7 @@
/var/www/securedrop/db.py r,
/var/www/securedrop/dictionaries/adjectives.txt r,
/var/www/securedrop/dictionaries/nouns.txt r,
/var/www/securedrop/execution.py r,
/var/www/securedrop/i18n.py r,
/var/www/securedrop/journalist.py r,
/var/www/securedrop/journalist_app/ r,
Expand Down Expand Up @@ -197,7 +198,6 @@
/var/www/securedrop/journalist_templates/delete.html r,
/var/www/securedrop/journalist_templates/edit_account.html r,
/var/www/securedrop/journalist_templates/error.html r,
/var/www/securedrop/journalist_templates/flag.html r,
/var/www/securedrop/journalist_templates/flashed.html r,
/var/www/securedrop/journalist_templates/index.html r,
/var/www/securedrop/journalist_templates/js-strings.html r,
Expand Down
Expand Up @@ -8,11 +8,11 @@ Standards-Version: 3.9.8

Package: securedrop-app-code
Architecture: amd64
Conflicts: libapache2-mod-wsgi,supervisor
Replaces: libapache2-mod-wsgi,supervisor
Conflicts: haveged, libapache2-mod-wsgi, supervisor
Replaces: haveged, libapache2-mod-wsgi, supervisor
{% if securedrop_target_distribution == "focal" %}
Depends: ${dist:Depends}, ${misc:Depends}, ${python3:Depends}, apache2, apparmor-utils, coreutils, gnupg2, haveged, libapache2-mod-xsendfile, libpython3.8, paxctld, python3, python3-distutils, redis-server, securedrop-config, securedrop-keyring, sqlite3
Depends: ${dist:Depends}, ${misc:Depends}, ${python3:Depends}, apache2, apparmor-utils, coreutils, gnupg2, libapache2-mod-xsendfile, libpython3.8, paxctld, python3, python3-distutils, redis-server, securedrop-config, securedrop-keyring, sqlite3
{% else %}
Depends: ${dist:Depends}, ${misc:Depends}, ${python3:Depends}, apache2, apparmor-utils, coreutils, gnupg2, haveged, libapache2-mod-xsendfile, libpython3.5, paxctld, python3 (>= 3.5), python3 (<< 3.6), redis-server, securedrop-config, securedrop-keyring, sqlite3
Depends: ${dist:Depends}, ${misc:Depends}, ${python3:Depends}, apache2, apparmor-utils, coreutils, gnupg2, libapache2-mod-xsendfile, libpython3.5, paxctld, python3 (>= 3.5), python3 (<< 3.6), redis-server, securedrop-config, securedrop-keyring, sqlite3
{% endif %}
Description: SecureDrop application code, dependencies, Apache configuration, systemd services, and AppArmor profiles. This package will put the AppArmor profiles in enforce mode.
1 change: 0 additions & 1 deletion molecule/builder-focal/Dockerfile
Expand Up @@ -16,7 +16,6 @@ RUN apt-get -y update && apt-get upgrade -y && apt-get install -y \
gdb \
git \
gnupg2 \
haveged \
inotify-tools \
libffi-dev \
libssl-dev \
Expand Down
40 changes: 0 additions & 40 deletions molecule/testinfra/app-code/test_haveged.py

This file was deleted.

17 changes: 16 additions & 1 deletion molecule/testinfra/app-code/test_securedrop_app_code.py
Expand Up @@ -22,7 +22,6 @@ def test_apache_default_docroot_is_absent(host):
'apparmor-utils',
'coreutils',
'gnupg2',
'haveged',
'libapache2-mod-xsendfile',
'libpython{}'.format(python_version),
'paxctld',
Expand All @@ -41,6 +40,22 @@ def test_securedrop_application_apt_dependencies(host, package):
assert host.package(package).is_installed


@pytest.mark.parametrize('package', [
'cron-apt',
'haveged',
'libapache2-mod-wsgi',
'ntp',
'ntpdate',
'supervisor'
])
def test_unwanted_packages_absent(host, package):
"""
Ensure packages that conflict with `securedrop-app-code`
or are otherwise unwanted are not present.
"""
assert not host.package(package).is_installed


@pytest.mark.skip_in_prod
def test_securedrop_application_test_locale(host):
"""
Expand Down
70 changes: 70 additions & 0 deletions securedrop/alembic/versions/b060f38c0c31_drop_source_flagged.py
@@ -0,0 +1,70 @@
"""drop Source.flagged

Revision ID: b060f38c0c31
Revises: 92fba0be98e9
Create Date: 2021-05-10 18:15:56.071880

"""
from alembic import op
import sqlalchemy as sa


# revision identifiers, used by Alembic.
revision = "b060f38c0c31"
down_revision = "92fba0be98e9"
branch_labels = None
depends_on = None


def upgrade():
with op.batch_alter_table("sources", schema=None) as batch_op:
batch_op.drop_column("flagged")


def downgrade():
# You might be tempted to try Alembic's batch_ops for the
# downgrade too. Don't. SQLite's unnamed check constraints require
# kludges.

conn = op.get_bind()
conn.execute("PRAGMA legacy_alter_table=ON")

op.rename_table("sources", "sources_tmp")

conn.execute(
sa.text(
"""
CREATE TABLE "sources" (
id INTEGER NOT NULL,
uuid VARCHAR(36) NOT NULL,
filesystem_id VARCHAR(96),
journalist_designation VARCHAR(255) NOT NULL,
last_updated DATETIME,
pending BOOLEAN,
interaction_count INTEGER NOT NULL,
deleted_at DATETIME,
flagged BOOLEAN,
PRIMARY KEY (id),
CHECK (pending IN (0, 1)),
CHECK (flagged IN (0, 1)),
UNIQUE (filesystem_id),
UNIQUE (uuid)
)
"""
)
)

conn.execute(
"""
INSERT INTO sources (
id, uuid, filesystem_id, journalist_designation,
last_updated, pending, interaction_count, deleted_at
) SELECT
id, uuid, filesystem_id, journalist_designation,
last_updated, pending, interaction_count, deleted_at
FROM sources_tmp;
"""
)

# Now delete the old table.
op.drop_table("sources_tmp")
2 changes: 0 additions & 2 deletions securedrop/crypto_util.py
Expand Up @@ -219,8 +219,6 @@ def find_source_key(self, fingerprint: str) -> Optional[Dict]:
def delete_reply_keypair(self, source_filesystem_id: str) -> None:
fingerprint = self.get_fingerprint(source_filesystem_id)

# If this source was never flagged for review, they won't have a reply
# keypair
if not fingerprint:
return

Expand Down
2 changes: 1 addition & 1 deletion securedrop/dockerfiles/focal/python3/Dockerfile
Expand Up @@ -11,7 +11,7 @@ RUN apt-get update && apt-get install -y paxctl && \
paxctl -cm /usr/bin/mono-sgen && dpkg-reconfigure mono-runtime-sgen && \
apt-get install -y apache2-dev coreutils devscripts vim \
python3-pip python3-all python3-venv virtualenv libpython3.8-dev libssl-dev \
gnupg2 ruby redis-server git xvfb haveged curl wget \
gnupg2 ruby redis-server git xvfb curl wget \
gettext paxctl x11vnc enchant libffi-dev sqlite3 gettext sudo \
libasound2 libdbus-glib-1-2 libgtk2.0-0 libfontconfig1 libxrender1 \
libcairo-gobject2 libgtk-3-0 libstartup-notification0 tor
Expand Down
2 changes: 1 addition & 1 deletion securedrop/dockerfiles/xenial/python3/Dockerfile
Expand Up @@ -11,7 +11,7 @@ RUN apt-get update && apt-get install -y paxctl && \
paxctl -cm /usr/bin/mono-sgen && dpkg-reconfigure mono-runtime-sgen && \
apt-get install -y apache2-dev coreutils devscripts dh-virtualenv vim \
python3-pip python3-all python3-venv virtualenv libpython3.5-dev libssl-dev \
gnupg2 ruby redis-server git xvfb haveged curl wget \
gnupg2 ruby redis-server git xvfb curl wget \
gettext paxctl x11vnc enchant libffi-dev sqlite3 gettext sudo \
libasound2 libdbus-glib-1-2 libgtk2.0-0 libfontconfig1 libxrender1 \
libcairo-gobject2 libgtk-3-0 libstartup-notification0 tor
Expand Down
11 changes: 11 additions & 0 deletions securedrop/execution.py
@@ -0,0 +1,11 @@
from threading import Thread


def asynchronous(f): # type: ignore
"""
Wraps a
"""
def wrapper(*args, **kwargs): # type: ignore
thread = Thread(target=f, args=args, kwargs=kwargs)
thread.start()
return wrapper
2 changes: 1 addition & 1 deletion securedrop/journalist.py
Expand Up @@ -4,7 +4,7 @@

from journalist_app import create_app
from models import Source
from source_app.utils import asynchronous
from execution import asynchronous

app = create_app(config)

Expand Down
6 changes: 1 addition & 5 deletions securedrop/journalist_app/api.py
Expand Up @@ -175,11 +175,7 @@ def remove_star(source_uuid: str) -> Tuple[flask.Response, int]:
@api.route('/sources/<source_uuid>/flag', methods=['POST'])
@token_required
def flag(source_uuid: str) -> Tuple[flask.Response, int]:
source = get_or_404(Source, source_uuid,
column=Source.uuid)
source.flagged = True
db.session.commit()
return jsonify({'message': 'Source flagged for reply'}), 200
return jsonify({'message': 'Sources no longer need to be flagged for reply'}), 200
Copy link
Member

@eloquence eloquence May 20, 2021

Choose a reason for hiding this comment

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

It may be worth marking the endpoint explicitly as deprecated, to be removed in a future release.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we can mark it as deprecated in the documentation, then we don't have to update this PR.


@api.route('/sources/<source_uuid>/submissions', methods=['GET'])
@token_required
Expand Down
7 changes: 0 additions & 7 deletions securedrop/journalist_app/main.py
Expand Up @@ -169,13 +169,6 @@ def reply() -> werkzeug.Response:
finally:
return redirect(url_for('col.col', filesystem_id=g.filesystem_id))

@view.route('/flag', methods=('POST',))
def flag() -> str:
g.source.flagged = True
db.session.commit()
return render_template('flag.html', filesystem_id=g.filesystem_id,
codename=g.source.journalist_designation)

@view.route('/bulk', methods=('POST',))
def bulk() -> Union[str, werkzeug.Response]:
action = request.form['action']
Expand Down
11 changes: 2 additions & 9 deletions securedrop/journalist_templates/col.html
Expand Up @@ -106,16 +106,9 @@ <h3>{{ gettext('Reply') }}</h3>
<hr class="no-line">
<button id="reply-button" class="button pull-right" type="submit">{{ gettext('SUBMIT') }}</button>
</form>
{% elif source.flagged %}
<p class="notification">{{ gettext("You've flagged this source for reply.") }}</p>
<p>{{ gettext("An encryption key will be generated for the source the next time they log in, after which you will be able to reply to the source here.") }}</p>
{% else %}
<p>{{ gettext("Click below if you would like to write a reply to this source.") }}</p>
<form action="{{ url_for('main.flag') }}" method="post">
<input type="hidden" name="filesystem_id" value="{{ filesystem_id }}">
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
<button id="flag-button" type="submit">{{ gettext('FLAG THIS SOURCE FOR REPLY') }}</button>
</form>
<p class="notification">{{ gettext("This source has no encryption key.") }}</p>
<p>{{ gettext("An encryption key will be generated for the source the next time they log in, after which you will be able to reply to the source here.") }}</p>
{% endif %}
<hr class="no-line">
<form id="delete-collection" action="{{ url_for('col.delete_single', filesystem_id=filesystem_id) }}" method="post">
Expand Down
11 changes: 0 additions & 11 deletions securedrop/journalist_templates/flag.html

This file was deleted.