#493 modify activate_session to be easily used without CLI
Merged 2 years ago by mikem. Opened 3 years ago by tkopecek.
tkopecek/koji issue436  into  master

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:

So currently this is does two things:

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

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....

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

Commit 9a1562f fixes this pull-request

Pull-Request has been merged by mikem@redhat.com

2 years ago