Allow configuring a command to be automatically run when disconnecting from VPN (take 2) #33
Conversation
This allows the config file to be managed by home-manager.
|
I'll check it out tomorrow. |
openconnect_sso/app.py
Outdated
| argv = [str(Path(command).expanduser())] | ||
| return subprocess.run(argv, timeout=5).returncode |
There was a problem hiding this comment.
IMHO this command does not support arguments right now, is that right? If you set shell=True in subprocess.run then you could use a command line with arguments. (or you can split the command line with shlex.quote instead)
There was a problem hiding this comment.
Correct. Didn't need arguments in my specific use case, but it's certainly needed in a more general version. Since we're going via a string in the config file, I'll use shell=True here. Although I'm usually not a fan of shell=True, we should be fine as all the input ultimately comes from the (trusted) user. Alternatively, we could use a list of string args in config.toml, but that doesn't feel very natural.
openconnect_sso/app.py
Outdated
| except KeyboardInterrupt: | ||
| logger.warn("CTRL-C pressed, exiting") | ||
| return 1 |
There was a problem hiding this comment.
Previously CTRL-C was not regarded as an error, it exited gracefully. I can see the value of returning an error code for it. AFAIK it should be 130 (-2 in twos complement, or 256+signal.SIGINT casted to a byte) for CTRL-C
There was a problem hiding this comment.
My thinking here was that if you abort with CTRL-C during the initial part of the script (before we launch openconnect), then a non-zero exit code is warranted, because one cannot argue that the program has completed successfully in any way, when the user aborted authentication and we never got around to even starting openconnect.
Conversely, once openconnect is running, pressing CTRL-C is a natural way to disconnect the VPN session (is there an easier way for the user to signal the intent to disconnect?), so CTRL-C then should not by regarded as an error. You'll see this towards the end of run() where I handle KeyboardInterrupt with return 0.
That said, I will change the exit code in the first case from 1 to 130.
By the time we run openconnect there's no need to use asyncio any more: The async GUI interactions are complete, and we simply want to run openconnect as a subprocess and wait for its result. Move this part of the program (as well as some other "sync" stuff) out of the async `_run()` and into the sync `run()` function. This fixes/simplifies the control flow when the users aborts with Ctrl+C while openconnect is running: Without it, asyncio's run_until_complete() will abort the asyncio event loop on SIGINT without running any exception handlers or finally clasues. This is in preparation for adding code to be run when openconnect is being aborted. Suggested-by: László Vaskó <laszlo.vasko@outlook.com>
jherland
force-pushed
the
run-command-on-disconnect
branch
from
8009843
to
6a43e67
Compare
When using VPN, I typically run a handful of SSH sessions to different servers. For convenience/stability/performance, I run those sessions over SSH ControlMaster connections. When I jump off the VPN, those master connections are left hanging and so are any SSH sessions using them. Even if I reconnect VPN, the master connection and its sessions are still non-responsive. And if I try to start a new SSH session, that will simply reuse the stale connection and will also hang with no progress or indication of what's wrong. My solution is to kill the stale master connections (also kills any lingering SSH sessions, and allows new sessions to establish a new, working, ControlMaster connection). I want this to happen automatically when I disconnect from VPN. This patch allows me to configure a command to be run automatically when disconnecting from VPN.
jherland
force-pushed
the
run-command-on-disconnect
branch
from
6a43e67
to
718d081
Compare
|
Pushed updated version with requested changes. |
|
Thank you for your contribution. Will try to publish a new release this weekend. |
I configure my SSH with ControlMaster connections that must be closed when I disconnect from VPN, otherwise they are left stale and existing and future SSH session are left hanging/wedged.
To accomodate this, the last patch in this series teaches openconnect-sso automatically run a configured command on VPN disconnection. In my case, I run a shell script that does
ssh -O exit ...on my connections, although this doesn't matter from openconnect-sso's POV.Otherwise the other patches are only tangentially related:
developbranch._run()into the syncrun(). Specifically it moves the running of theopenconnectsubprocess which does not benefit from being async. This also fixes the control flow when abortingopenconnectwithCtrl+C.This supersedes #32, but is based on
developinstead ofmaster, as I found it easier to get this done on top of those changes.