phase2 / rig

Outrigger command line tool
MIT License
11 stars 8 forks source link

Add explicit privilege prompt to improve sudo UX #138

Closed grayside closed 6 years ago

grayside commented 6 years ago

Fixes #134

This change identifies that we need to sudo and adjusts logging/spinning behaviors accordingly. Needs some more testing (e.g., with verbose) and a bit more smarts in sudo detection, hence the WIP flag. Also some UI issues around the asynchronous nature of the spinner.

I am pausing here for approach feedback.

Here's what it looks like when it operates as intended:

Paused for Input

screen shot 2018-01-29 at 1 12 27 pm

Success!

screen shot 2018-01-29 at 1 11 34 pm

febbraro commented 6 years ago

So I would simply say Administrative privileges needed and nothing about a password, since the Password prompt does that already. Look at this example, I ran rig dns twice. Once to get the initial prompt because I was unprivileged...

https://www.dropbox.com/s/m8twc771il6llhq/Screenshot%202018-01-29%2013.25.10.png?dl=0

And then once again, immediately, while I still had privs....

https://www.dropbox.com/s/vtx768cuhep9xk2/Screenshot%202018-01-29%2013.25.52.png?dl=0

It gave me "please enter your password" even though it just blasted right past it. Claiming privs needed is a good plan and it gives us the newline needed for an unobscured prompt. Claiming they are needed and blowing past it is really just a heads up and it is not confusing.

grayside commented 6 years ago

Unfortunately here are some of the other results I'm getting, with a password needed:

screen shot 2018-01-29 at 1 42 48 pm screen shot 2018-01-29 at 1 42 59 pm screen shot 2018-01-29 at 1 43 25 pm

grayside commented 6 years ago

Simplification helped tremendously. This is ready for final review.

I would appreciate if someone took it for a spin to confirm the timing issues are not still lurking.

My test has been:

./build/darwin/rig dns
tekante commented 6 years ago

Looks like it's working as intended. First run through didn't need password as I had recently used sudo, second time includes the password prompt.

privilege-prompt-test-results

grayside commented 6 years ago

In addition to addressing CJ's feedback, I took the following steps:

febbraro commented 6 years ago

I'm getting prompt overlap on rig stop

Using sudo -k first to remove privs

febbraro commented 6 years ago

Prompt overlap is only when stopping a running machine. If I stop an already stopped machine, the prompt works fine.

grayside commented 6 years ago

I haven't found a way to crack the rig stop spinner yet. It appears that the route command does something unusual in how it interacts with newlines or the password prompt, and may require much deeper research. Is this change a sufficient improvement to move forward anyway?

Should I output an extra message on rig stop to provide instruction on dealing with the confusion?

tekante commented 6 years ago

I could go with proceeding if it is just the stop portion that overlap is wonky on. If it's something with route would an initial sudo cat /dev/null just so that the password gets prompted for alleviate that without causing ill effects?

grayside commented 6 years ago

cat /dev/null seems to have helped. Will need to research the implications of that trick.