Add Windows compatibility #16
Conversation
Reading from STDIN worked well on Linux, but on Windows communicating via STDIN is not available for evelated processes.
As its name says, command_line contains the command line. During debug it is not helpful having only one parameter set by a different way.
Unfortunately the embedded browser did not work with elevated mode, so it was required to keep the "sudo" part. In Windows there is no easy way to do it, but there are many "sudo" implementations, so this way the source code can be unchanged and the user can select for themselves any sudo implementation.
|
I am wondering if bringing in elevate would make privilege elevation simpler as it is a cross platform library.
|
Also clarify why this magic is needed Cherry-picked from commit f3fd7e3
|
I have tried elevate as well, but it simply calls the script itself with elevated privileges, and has no magic at all to jump to the execution point. On Windows it is done within a new terminal (which can be hidden or shown). It would be good to do it as the first step (before running the async loop), but unfortunately the built-in browser did not work (at least for me) with elevated privileges, and I did not want to start debugging it. The other way can be to create a simple wrapper script to run openconnect with elevated privileges, which means to create another sudo implementation... It can be done, but also it would be better to run openconnect directly with elevated privileges on Windows, but I do not know how can we an async way to run windll.shell32.ShellExecuteEx, and at this point I did not want to increase the complexity. (Moreover I have found a sudo implementation for Windows which does not start a new console, but shows the output of the elevated command within the same console, and I was not able to find a solution to do it with Python, so I am happy having only one terminal.) We can add experimental flag to the Windows section of README if you think. |
|
I was thinking about moving privilege elevation to an additional script or call it via multiprocessing in the same way browser is handled. I do not like the idea of elavating the whole process. On the other hand openconnect binary itself can be marked to require elevation. Won't it work then without I am happy with the changes but am curious how could we remove the need of an explicit |
|
It is merged into |
|
If openconnect binary is marked to run in elevated mode, then Python cannot start it, the error thrown on https://github.com/python/cpython/blob/v3.7.7/Lib/subprocess.py#L1200 is that The requested operation requires elevation. (I may create an additional script in a separated pull request later to do the elevation and call openconnect.) |
|
Here is a working-for-me-on-Windows PoC of a wrapper script: presidento@3c1d3f2. You can copy/implement it if you want. |
|
This feature has been merged to master and is part of v0.6.0 |
|
I created this wrapper over the openconnect-sso.exe: https://gist.github.com/anishsane/43b72f4fa117dea389b4e971bf6cbc1c |
Implements #3. Unfortunately privilege escalation is still needed, because in escalated mode the builtin browser did not work for me. But there are quite working sudo implementations in the field, so I kept it this way, because I did not want to increase the complexity.