PR#493: modify activate_session to be easily used without CLI - koji

koji

#493 modify activate_session to be easily used without CLI

Merged 6 years ago by mikem. Opened 6 years ago by tkopecek.

tkopecek/koji issue436 into master

modify activate_session to be easily used without CLI

Tomas Kopecek • 6 years ago

5c76bd1

cli/koji_cli/lib.py

file modified

+9 -5

		`@@ -496,21 +496,25 @@`

		`def activate_session(session, options):`
		`"""Test and login the session is applicable"""`
		`- if options.authtype == "noauth" or options.noauth:`
		`+ if isinstance(options, dict):`
		`+ options = optparse.Values(options)`
		`+ noauth = options.authtype == "noauth" or getattr(options, 'noauth', False)`
		`+ if noauth:`
		`#skip authentication`
		`pass`
		`elif options.authtype == "ssl" or os.path.isfile(options.cert) and options.authtype is None:`
		`# authenticate using SSL client cert`
		`session.ssl_login(options.cert, None, options.serverca, proxyuser=options.runas)`
		`- elif options.authtype == "password" or options.user and options.authtype is None:`
		`+ elif options.authtype == "password" or getattr(options, 'user', None) and options.authtype is None:`
		`# authenticate using user/password`
		`session.login()`
		`elif options.authtype == "kerberos" or has_krb_creds() and options.authtype is None:`
		`try:`
		`+ runas = getattr(options, 'runas', None)`
		`if options.keytab and options.principal:`
		`- session.krb_login(principal=options.principal, keytab=options.keytab, proxyuser=options.runas)`
		`+ session.krb_login(principal=options.principal, keytab=options.keytab, proxyuser=runas)`
		`else:`
		`- session.krb_login(proxyuser=options.runas)`
		`+ session.krb_login(proxyuser=runas)`
		`except socket.error as e:`
		`warn(_("Could not connect to Kerberos authentication service: %s") % e.args[1])`
		`except Exception as e:`
		`@@ -518,7 +522,7 @@`
		`error(_("Kerberos authentication failed: %s (%s)") % (e.args[1], e.args[0]))`
		`else:`
		`raise`
		`- if not options.noauth and options.authtype != "noauth" and not session.logged_in:`
		`+ if not noauth and not session.logged_in:`
		`error(_("Unable to log in, no authentication methods available"))`
		`ensure_connection(session)`
		`if options.debug:`

tkopecek commented 6 years ago

mikem commented 6 years ago

So currently this is does two things:

allows passing options as a dict rather than a record
provides some default values for option lookups in the function

Edited 6 years ago by mikem

mikem commented 6 years ago

I get the convenience of (1), but otoh, it's not hard to wrap a dict in optparse.Values.
I'm worried somewhat worried about the inconsistencies of (2). Only some of the options lookups are given defaults, potentially making for ugly errors if the options passed in is missing something.

I'm also wondering it wouldn't be better to solve this another way.

otoh, maybe this is good as a stopgap measure....

mikem commented 6 years ago

Merging this with a small fix (don't assume runas in the ssl check either), and adding a unit test for activate_session.

mikem commented 6 years ago

Commit 9a1562f fixes this pull-request