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

Fixes #4677 warns admin if v3 and https both are enabled #4720

Merged
merged 2 commits into from Aug 30, 2019

Conversation

kushaldas
Copy link
Contributor

Status

Ready for review

Description of Changes

Fixes #4677 This should be pulled in after #4710 is merged

We should decide the actual warning text and update in this PR. @redshiftzero

Testing

./securedrop-admin sdconfig

  • If you enable v3 and https both, this should print an warning message
  • If either of them disabled, the user should not see any warning.

Deployment

Any special considerations for deployment? Consider both:

  1. Upgrading existing production instances.
  2. New installs.

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

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #4720   +/-   ##
========================================
  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 549a056...33298e9. Read the comment docs.

@eloquence eloquence added this to Ready for review in SecureDrop Team Board Aug 30, 2019
@conorsch
Copy link
Contributor

#4710 is merged. I see conflicts, @kushaldas—can you rebase and mark ready for review when you're satisfied?

@kushaldas kushaldas force-pushed the v3_and_https_may_not_be_friends branch from 54be689 to 33298e9 Compare August 30, 2019 09:30
@kushaldas kushaldas removed the blocked label Aug 30, 2019
@conorsch conorsch moved this from Ready for review to Under Review in SecureDrop Team Board Aug 30, 2019
Copy link
Contributor

@redshiftzero redshiftzero left a comment

Choose a reason for hiding this comment

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

tested in Tails, looks good!

@redshiftzero redshiftzero merged commit 3b254a9 into develop Aug 30, 2019
SecureDrop Team Board automation moved this from Under Review to Done Aug 30, 2019
@redshiftzero redshiftzero deleted the v3_and_https_may_not_be_friends branch August 30, 2019 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

[v3 onion migration] warn user if they enable v3 and have https enabled
5 participants