Fix #3354: Tor over SSH prompt should not be populated with invalid response #3357
Conversation
There was a problem hiding this comment.
Perhaps being explicit in the wording that SSH is exposed over Tor xor Localnet by explicitly enumerating what is enabled/disabled might make the prompt clearer (see inline comment).
I don't have any strong objections to the wording as-is, and otherwise good to merge from my perspective, so I'll let @redshiftzero either merge as-is or rephrase the prompt as they see fit.
admin/securedrop_admin/__init__.py
Outdated
| SiteConfig.ValidateSSH(), | ||
| self.sanitize_ssh_over_tor_or_lan], | ||
| u'Enable SSH over Tor instead of LAN (recommended). ' | ||
| u'If you respond no, SSH will be available over LAN only', |
There was a problem hiding this comment.
Enable SSH over Tor (recommended, disables SSH over LAN). If you respond no, SSH will be available over LAN (disables SSH over Tor)
There was a problem hiding this comment.
OK adopted a hybrid, I think this should be good 😇:
Enable SSH over Tor (recommended, disables SSH over LAN). If you respond no, SSH will be available over LAN only:
Codecov Report
@@ Coverage Diff @@
## develop #3357 +/- ##
========================================
Coverage 85.81% 85.81%
========================================
Files 34 34
Lines 2157 2157
Branches 238 238
========================================
Hits 1851 1851
Misses 250 250
Partials 56 56Continue to review full report at Codecov.
|
This is a minimal change to make clear: 1. Tor is the recommended option. 2. Disabling Tor enables LAN and vice versa.
redshiftzero
force-pushed
the
no-fancy-tor-ssh-sdconfig
branch
from
cad8b4e to
61e7778
Compare
Status
Ready for review
Description of Changes
Fixes #3354. Basically just changes the prompt in order to make minimal changes but still make clear to admins that:
Testing
This is reverting to the existing behavior, but consider if this prompt could be clearer in any way (it's easy to make a string change now).
Deployment
No special issues
Checklist
If you made changes to
securedrop-admin:make -C admin test) pass in the admin development container