Merge pull request #5335 from freedomofpress/fix-5334

Fix onion service version validation in securedrop-admin install

admin/Dockerfile

@@ -12,13 +12,12 @@ RUN apt-get update && \
     apt-get install -y python3 sudo lsb-release gnupg2 git
 RUN if test $USER_NAME != root ; then useradd --no-create-home --home-dir /tmp --uid $USER_ID $USER_NAME && echo "$USER_NAME ALL=(ALL) NOPASSWD:ALL" >> /etc/sudoers ; fi
 
-WORKDIR /opt
-COPY bootstrap.py .
-COPY requirements.txt .
-RUN python3 bootstrap.py -v
-ENV VIRTUAL_ENV /opt/.venv
+WORKDIR /opt/admin
+COPY . /opt
+RUN rm -rf /opt/admin/.venv3
+RUN cd /opt/admin && python3 bootstrap.py -v
+ENV VIRTUAL_ENV /opt/admin/.venv3
 ENV PATH="$VIRTUAL_ENV/bin:$PATH"
-COPY requirements-dev.txt .
-RUN pip3 install --no-deps --require-hashes -r requirements-dev.txt
+RUN pip3 install --no-deps --require-hashes -r /opt/admin/requirements-dev.txt
 
 RUN chown -R $USER_NAME /opt

admin/bin/dev-shell

@@ -9,45 +9,22 @@ set -euo pipefail
 TOPLEVEL=$(git rev-parse --show-toplevel)
 source "${BASH_SOURCE%/*}/../../devops/scripts/ticker"
 
-function docker_image_circle() {
+function docker_image() {
     local out
     out="$(mktemp)"
-    ( cat Dockerfile ; echo COPY . . ) > circle.docker
-    if ! docker build ${DOCKER_BUILD_ARGUMENTS:-} -t securedrop-admin -f circle.docker . >& "$out" ; then
+    cd "${TOPLEVEL}"
+    if ! docker build ${DOCKER_BUILD_ARGUMENTS:-} -t securedrop-admin -f admin/Dockerfile . >& "$out" ; then
         cat "$out"
         status=1
     else
         status=0
     fi
-    rm circle.docker
     return $status
 }
 
-function docker_run_circle() {
-    docker run --rm -ti ${DOCKER_RUN_ARGUMENTS:-} securedrop-admin "$@"
-}
-
-function docker_image() {
-    docker build \
-           ${DOCKER_BUILD_ARGUMENTS:-} \
-	   --build-arg=USER_ID="$(id -u)" \
-	   --build-arg=USER_NAME="${USER:-root}" \
-           -t securedrop-admin "${TOPLEVEL}/admin"
-}
-
 function docker_run() {
-    docker run \
-	   --rm \
-	   --user "${USER:-root}" \
-	   --volume "${TOPLEVEL}:${TOPLEVEL}" \
-	   --workdir "${TOPLEVEL}/admin" \
-	   -ti ${DOCKER_RUN_ARGUMENTS:-} securedrop-admin "$@"
+    docker run --rm -ti ${DOCKER_RUN_ARGUMENTS:-} securedrop-admin "$@"
 }
 
-if test -n "${CIRCLE_SHA1:-}" ; then
-   docker_image_circle
-   docker_run_circle "$@"
-else
-   ticker docker_image
-   docker_run "$@"
-fi
+docker_image
+docker_run "$@"

admin/securedrop_admin/__init__.py

@@ -424,6 +424,9 @@ def __init__(self, args):
     def load_and_update_config(self, validate: bool = True, prompt: bool = True):
         if self.exists():
             self.config = self.load(validate)
+        elif not prompt:
+            sdlog.error('Please run "securedrop-admin sdconfig" first.')
+            sys.exit(1)
 
         return self.update_config(prompt)
 
@@ -624,6 +627,8 @@ def clean_config(self, config: dict) -> dict:
                         )
                         raise
                 clean_config[var] = transform(text) if transform else text
+                if var not in self._config_in_progress:
+                    self._config_in_progress[var] = clean_config[var]
         return clean_config
 
     def load(self, validate=True):

admin/tests/test_integration.py

@@ -5,6 +5,7 @@
 import pytest
 import re
 import requests
+import shutil
 import subprocess
 import tempfile
 
@@ -191,6 +192,19 @@ def setup_function(function):
     global SD_DIR
     SD_DIR = tempfile.mkdtemp()
     ANSIBLE_BASE = '{0}/install_files/ansible-base'.format(SD_DIR)
+
+    for name in ["roles", "tasks"]:
+        shutil.copytree(
+            os.path.join(CURRENT_DIR, "../../install_files/ansible-base", name),
+            os.path.join(ANSIBLE_BASE, name)
+        )
+
+    for name in ["ansible.cfg", "securedrop-prod.yml"]:
+        shutil.copy(
+            os.path.join(CURRENT_DIR, '../../install_files/ansible-base', name),
+            ANSIBLE_BASE
+        )
+
     cmd = 'mkdir -p {0}/group_vars/all'.format(ANSIBLE_BASE).split()
     subprocess.check_call(cmd)
     for name in ['sd_admin_test.pub', 'ca.crt', 'sd.crt', 'key.asc']:
@@ -340,6 +354,29 @@ def verify_v3_onion_when_v2_is_enabled(child):
     assert ANSI_ESCAPE.sub('', child.buffer.decode("utf-8")).strip() == 'yes'  # noqa: E501
 
 
+def verify_install_has_valid_config():
+    """
+    Checks that securedrop-admin install validates the configuration.
+    """
+    cmd = os.path.join(os.path.dirname(CURRENT_DIR), 'securedrop_admin/__init__.py')
+    child = pexpect.spawn('python {0} --root {1} install'.format(cmd, SD_DIR))
+    child.expect(b"SUDO password:", timeout=5)
+    child.close()
+
+
+def test_install_with_no_config():
+    """
+    Checks that securedrop-admin install complains about a missing config file.
+    """
+    cmd = os.path.join(os.path.dirname(CURRENT_DIR), 'securedrop_admin/__init__.py')
+    child = pexpect.spawn('python {0} --root {1} install'.format(cmd, SD_DIR))
+    child.expect(b'ERROR: Please run "securedrop-admin sdconfig" first.', timeout=5)
+    child.expect(pexpect.EOF, timeout=5)
+    child.close()
+    assert child.exitstatus == 1
+    assert child.signalstatus is None
+
+
 def test_sdconfig_on_first_run():
     cmd = os.path.join(os.path.dirname(CURRENT_DIR),
                        'securedrop_admin/__init__.py')
@@ -401,6 +438,8 @@ def test_sdconfig_on_first_run():
         data = fobj.read()
     assert data == OUTPUT1
 
+    verify_install_has_valid_config()
+
 
 def test_sdconfig_both_v2_v3_true():
     cmd = os.path.join(os.path.dirname(CURRENT_DIR),
@@ -463,6 +502,8 @@ def test_sdconfig_both_v2_v3_true():
         data = fobj.read()
     assert data == WHEN_BOTH_TRUE
 
+    verify_install_has_valid_config()
+
 
 def test_sdconfig_only_v2_true():
     cmd = os.path.join(os.path.dirname(CURRENT_DIR),
@@ -525,6 +566,8 @@ def test_sdconfig_only_v2_true():
         data = fobj.read()
     assert data == WHEN_ONLY_V2
 
+    verify_install_has_valid_config()
+
 
 def test_sdconfig_enable_journalist_alerts():
     cmd = os.path.join(os.path.dirname(CURRENT_DIR),
@@ -592,6 +635,8 @@ def test_sdconfig_enable_journalist_alerts():
         data = fobj.read()
     assert JOURNALIST_ALERT_OUTPUT == data
 
+    verify_install_has_valid_config()
+
 
 def test_sdconfig_enable_https_on_source_interface():
     cmd = os.path.join(os.path.dirname(CURRENT_DIR),
@@ -666,6 +711,8 @@ def test_sdconfig_enable_https_on_source_interface():
         data = fobj.read()
     assert HTTPS_OUTPUT == data
 
+    verify_install_has_valid_config()
+
 
 # The following is the minimal git configuration which can be used to fetch
 # from the SecureDrop Github repository. We want to use this because the