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.

Sign up for GitHub

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

Jump to bottom

Expose v2/v3 Onion URLs to webapp code #4718

Merged
merged 2 commits into from
Aug 30, 2019
Merged

Expose v2/v3 Onion URLs to webapp code #4718

merged 2 commits into from
Aug 30, 2019

Conversation

conorsch
Copy link
Contributor

@conorsch conorsch commented Aug 29, 2019 •
edited
Loading

Status

Ready for review.

Description of Changes

Fixes #4631. Fixes #4674.

Changes proposed in this pull request:

  • Writes plaintext files to disk, readable by the www-data user, containing Onion URL info. Separate files are used for each URL.
  • Updates documentation with a quick tip about running a subset of testinfra tests for developer velocity.

Files will exist only if the relevant generation of Onion URLs are configured in site-specific vars. That means that future implementations consuming these files must not assume file existence, and shold also validate the strings inside the files.

Testing

CI will validate that the staging setup works well. Bust out your Tails/prod VM setup.

  • Configure site-specific vars so that only v2 URLs are enabled, install
  • Log into app server (ssh app), confirm that /var/lib/securedrop/source_v2_url exists and matches the contents of /var/lib/tor/services/source/hostname.
  • Confirm /var/lib/securedrop/source_v3_url does not exist
  • Re-configure site-specific vars to enable v3 Onions, re-install
  • Log into app server (ssh app), confirm that /var/lib/securedrop/source_v2_url exists and matches the contents of /var/lib/tor/services/source/hostname.
  • Log into app server (ssh app), confirm that /var/lib/securedrop/source_v3_url exists and matches the contents of /var/lib/tor/services/sourcev3/hostname.

Deployment

These changes are intended to enable future user-facing messaging regarding URL transitions. We aim to ship them in 1.0 to take advantage of the Admin process of enabling v3 URLs, which requires running the playbooks, to create these files.

Checklist

If you made changes to the server application code:

  • Linting (make lint) and tests (make -C securedrop test) pass in the development container

If you made changes to securedrop-admin:

  • Linting and tests (make -C admin test) pass in the admin development container

If you made changes to the system configuration:

If you made non-trivial code changes:

  • I have written a test plan and validated it for this PR

If you made changes to documentation:

  • Doc linting (make docs-lint) passed locally

@codecov-io
Copy link

codecov-io commented Aug 29, 2019 •
edited
Loading

Codecov Report

Merging #4718 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #4718   +/-   ##
========================================
  Coverage    81.61%   81.61%           
========================================
  Files           49       49           
  Lines         3416     3416           
  Branches       391      391           
========================================
  Hits          2788     2788           
  Misses         535      535           
  Partials        93       93

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d3d3ab7...f6b0a69. Read the comment docs.

redshiftzero
redshiftzero reviewed Aug 29, 2019
View reviewed changes
install_files/ansible-base/roles/app/Session.vim Outdated
@@ -0,0 +1,121 @@
let SessionLoad = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

accidentally committed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, addressed!

install_files/ansible-base/roles/app/tasks/install_and_harden_apache.yml Outdated Show resolved Hide resolved
@conorsch conorsch force-pushed the 4631-expose-v3-onion-urls-to-app branch from d13b873 to 4be11ff Compare August 29, 2019 18:21
@conorsch conorsch removed the blocked label Aug 29, 2019
@conorsch
Copy link
Contributor Author

Rebased to remove the dependency on #4715. Ready for review.

@redshiftzero
Copy link
Contributor

can you elaborate why you think prod VMs/Tails testing should be done here? (slower to test). it seems like the main thing we might to verify would be that for v2 only things work as expected (since the logic is straightforward I'm inclined to just rely on the testinfra tests passing for v2/v3)

@conorsch
Copy link
Contributor Author

can you elaborate why you think prod VMs/Tails testing should be done here?

Good point, I'm fine with staging VMs being used here. Bear in mind that this step:

Configure site-specific vars so that only v2 URLs are enabled, install

Requires editing (and generating a diff that should not be committed) install_files/ansible-base/group_vars/staging.yml for the various setups.

@conorsch
Copy link
Contributor Author

python3-app-tests failing in CI with timeouts: https://circleci.com/gh/freedomofpress/securedrop/32940 Appears unrelated to changes in this PR, let's bounce a few times, then raise timeout if still unresolved.

@redshiftzero
Copy link
Contributor

ah that is #4691

Conor Schaefer added 2 commits August 29, 2019 15:38
Exposes Onion URLs (v2 & v3) to webapp
ccb2b27 
We write plaintext files to /var/lib/securedrop so that the application
can provide user-facing messaging about upcoming v2 -> v3 Onion URL
transitions. For example, if a v3 Source Onion URL exists, but a source
is browsing via the v2 URL, we can recommend the v3 URL instead.
Documents running subset of testinfra tests
f6b0a69 
Since testinfra is based on pytest, the pytest env var for passing
additional options works just fine. We can use the `-k` option to
perform a substring search on test names, and run only those tests
matching the selector.
@conorsch conorsch force-pushed the 4631-expose-v3-onion-urls-to-app branch from 4be11ff to f6b0a69 Compare August 29, 2019 22:39
@conorsch
Copy link
Contributor Author

Rebased on top of latest develop (d3d3ab7) in an attempt to satisfy CI; as @redshiftzero points out, we've seen this before, so we may need to raise the timeouts here.

kushaldas
kushaldas approved these changes Aug 30, 2019
View reviewed changes
Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

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

  • Configure site-specific vars so that only v2 URLs are enabled, install
  • Log into app server (ssh app), confirm that /var/lib/securedrop/source_v2_url exists and matches the contents of /var/lib/tor/services/source/hostname.
  • Confirm /var/lib/securedrop/source_v3_url does not exist
  • Re-configure site-specific vars to enable v3 Onions, re-install
  • Log into app server (ssh app), confirm that /var/lib/securedrop/source_v2_url exists and matches the contents of /var/lib/tor/services/source/hostname.
  • Log into app server (ssh app), confirm that /var/lib/securedrop/source_v3_url exists and matches the contents of /var/lib/tor/services/sourcev3/hostname.

@kushaldas kushaldas merged commit ef8d961 into develop Aug 30, 2019
@conorsch conorsch deleted the 4631-expose-v3-onion-urls-to-app branch June 18, 2020 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@redshiftzero redshiftzero redshiftzero left review comments

@kushaldas kushaldas kushaldas approved these changes

@emkll emkll Awaiting requested review from emkll

@msheiny msheiny Awaiting requested review from msheiny

@zenmonkeykstop zenmonkeykstop Awaiting requested review from zenmonkeykstop

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@conorsch @codecov-io @redshiftzero @kushaldas