vlaci / openconnect-sso

Wrapper script for OpenConnect supporting Azure AD (SAMLv2) authentication to Cisco SSL-VPNs
GNU General Public License v3.0
278 stars 117 forks source link

Extra arguments are not parsed correctly #135

Open waldner opened 11 months ago

waldner commented 11 months ago

When using extra arguments to openconnect (ie those after --), depending on the syntax used an error might be produced, or the results might be unexpected.

❯ openconnect-sso --version
openconnect-sso 0.8.1

For example:

openconnect-sso --log-level=DEBUG --server https://example.com --authgroup foo --user joe@example.net -- --script /path/to/my-connection-script

(note the missing = in the --script argument, which is legal syntax) the following openconnect invocation is produced:

[debug    ] Starting OpenConnect           [openconnect_sso.app] command_line=['sudo', 'openconnect', '--useragent', 'AnyConnect Linux_64 4.7.00136', '--version-string', '4.7.00136', '--cookie-on-stdin', '--servercert', 'XXXXXXXXXXXXXXXXXXXXXXXXXXX', '/path/to/my-connection-script', 'https://example.com/']

which openconnect rejects with Too many arguments on command line.

On the other hand, if using the following syntax (also legal):

openconnect-sso --log-level=DEBUG --server https://example.com --authgroup foo --user joe@example.net -- --script=/path/to/my-connection-script

(ie with --script=...)

The resulting openconnect invocation is

[debug    ] Starting OpenConnect           [openconnect_sso.app] command_line=['sudo', 'openconnect', '--useragent', 'AnyConnect Linux_64 4.7.00136', '--version-string', '4.7.00136', '--cookie-on-stdin', '--servercert', 'XXXXXXXXXXXXXXXXXXXXXXXXXXX', 'https://example.com/']

which is valid openconnect syntax, but now the extra option is not passed at all, and indeed the custom script is not executed.

waldner commented 11 months ago

The problem seems to be in the way that argparse.REMAINDER is handled in python 3.11, I'm looking for more information.

waldner commented 11 months ago

No, in fact the problem is that the fix (https://github.com/vlaci/openconnect-sso/blame/master/openconnect_sso/cli.py#L125) is not in Pypi yet. So, released version 0.8.1 is broken because of this line:

setattr(namespace, self.dest, values[1:])
vlaci commented 11 months ago

If you could check if the latest git master is working, I'll release it

waldner commented 11 months ago

I'll check it out as soon as I have a moment, thanks.

waldner commented 11 months ago

Yes master is working (note I only tested this specific thing).

TechupBusiness commented 9 months ago

@vlaci released?

zabealbe commented 7 months ago

No, in fact the problem is that the fix (https://github.com/vlaci/openconnect-sso/blame/master/openconnect_sso/cli.py#L125) is not in Pypi yet. So, released version 0.8.1 is broken because of this line:

setattr(namespace, self.dest, values[1:])

I confirm that by manually chaning setattr(namespace, self.dest, values[1:]) to setattr(namespace, self.dest, values) fixes the problem. A temporary solution is to insert a dummy argument such as -V:


openconnect-sso --user joe@example.io --server example.io -- -V --script='vpn-slice example.io'