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

Improve admin workstation updater GUI error handling #5169

Merged
merged 1 commit into from
Mar 31, 2020

Conversation

rmol
Copy link
Contributor

@rmol rmol commented Mar 20, 2020

Status

Ready for review

Description of Changes

Increase the time it will wait for tailsconfig to run.

Don't say the admin password was wrong on tailsconfig timeout. Instead, look for sudo in the output to determine if an incorrect password was given.

Add a new message explaining that tailsconfig took too long, and don't say "Exiting upgrade" when an incorrect admin password was given, as the updater is not exiting, and in fact we're telling the admin to try again.

Make sure the failure reason was cleared on each attempt.

Fixes #3642.

Testing

For those about to curse, we salute you. Ah, it's not that bad, but a bit manual.

  • Fire up an admin workstation, making sure to enable the persistent volume and set an admin password.
  • Start a terminal.
  • cd Persistent/securedrop
  • git fetch --all
  • git diff origin/develop..origin/gui-updater-error-handling > ../updater.patch
  • git checkout 1.2.1
  • git apply ../updater.patch

Now bounce your network, most easily via the taskbar menu. After the network is reconnected and Tor restarts, the updater should pop up.

  • Click Update Now.
  • When prompted, enter a wrong password. You should get a dialog saying Administrator password incorrect.
  • Close the dialog, click Update Now again, and enter the correct password. It should work this time. Close the updater.

Now back in the terminal:

  • git checkout 1.2.1
  • Edit journalist_gui/journalist_gui/SecureDropUpdater.py to change the timeout on line 137 from 120 to 1.
  • Bounce the network.
  • When the updater appears, click Update Now.
  • Enter the correct password when the dialog appears.
  • You should see a dialog saying Tails workstation configuration took too long..
  • Close the updater.
  • If you like, clean up your repo with git reset --hard HEAD.

Deployment

This doesn't change anything on the servers, just prevents a misleading error shown to admins.

Checklist

If you made changes to the server application code:

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

If you made non-trivial code changes:

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

Increase the time it will wait for tailsconfig to run.

Don't say the admin password was wrong on tailsconfig
timeout. Instead, look for sudo in the output to determine if an
incorrect password was given.

Add a new message explaining that tailsconfig took too long, and don't
say "Exiting upgrade" when an incorrect admin password was given, as
the updater is not exiting, and in fact we're telling the admin to try
again.

Make sure the failure reason was cleared on each attempt.
@rmol rmol force-pushed the gui-updater-error-handling branch from 734c07b to 118db3c Compare March 20, 2020 20:48
@kushaldas kushaldas self-assigned this Mar 31, 2020
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.

I could not follow the steps for apply the patch, instead I tested by making the 120 seconds to 1 second in a local commit and followed along the steps (I did similar many time before for other PRs for the journalist UI).

This PR works as suggested. Approved. I would still love to have another pair of eyes on this.

@eloquence
Copy link
Member

(Taking this for a spin now.)

@eloquence
Copy link
Member

git diff origin/develop..origin/gui-updater-error-handling > ../updater.patch

This diff introduces some unrelated changes to the tagged version. I don't think they're likely to interfere with the Tails changes, but just in case, I'm using git cherry-pick -n 118db3c38ce736d0c257eb9a76d8e1a577749c5a on the 1.2.1 tag instead, let me know if you see any issues with that.

@eloquence
Copy link
Member

  • (With incorrect password.) When prompted, enter a wrong password. You should get a dialog saying Administrator password incorrect.
  • (With correct password.) Close the dialog, click Update Now again, and enter the correct password. It should work this time. Close the updater.
  • (With decreased timeout.) You should see a dialog saying Tails workstation configuration took too long..

Running with correct password a couple more times to be sure.

@eloquence
Copy link
Member

Ran it 4 more times successfully, LGTM. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SecureDrop Updater Tails Admin Password Prompt Appears to Fail When Clicking OK
4 participants