koji

Clone

#92 Added PAM authentication support
Closed 6 years ago by tkopecek. Opened 9 years ago by ctria.
ctria/koji pam_authentication  into  master

file modified
+3 
@@ -37,6 +37,9 @@ 
 

  ## end SSL client certificate auth configuration
 

  

  

+ ##  PAM auth configuration ##
 

+ # PAMService = koji
 

+ ## end PAM auth configuration ##
 

  

  ##  Other options  ##
 

  LoginCreatesUser = On

file modified
+2 
@@ -432,6 +432,8 @@ 
 

  

          ['CheckClientIP', 'boolean', True],
 

  

+         ['PAMService', 'string', None],
 

+ 

          ['LoginCreatesUser', 'boolean', True],
 

          ['KojiWebURL', 'string', 'http://localhost.localdomain/koji'],
 

          ['EmailDomain', 'string', None],

file modified
+7 -1 
@@ -78,6 +78,7 @@ 
 

  import xml.sax
 

  import xml.sax.handler
 

  from xmlrpclib import loads, dumps, Fault
 

+ from getpass import getpass
 

  

  PROFILE_MODULES = {}  # {module_name: module_instance}
 

  
@@ -1590,7 +1591,8 @@ 
 

          'ca': '',  # FIXME: remove in next major release
 

          'serverca': None,
 

          'no_ssl_verify': False,
 

-         'authtype': None
 

+         'authtype': None,
 

+         'user': None
 

      }
 

  

      result = config_defaults.copy()
 
@@ -2023,6 +2025,10 @@ 
 

          self.sinfo = sinfo
 

  

      def login(self, opts=None):
 

+         if not 'user' in self.opts:
 

+             self.opts['user'] = raw_input('Username: ')
 

+         if not 'password' in self.opts:
 

+             self.opts['password'] = getpass('Password: ')
 

          sinfo = self.callMethod('login', self.opts['user'], self.opts['password'], opts)
 

          if not sinfo:
 

              return False

file modified
+26 -7 
@@ -27,6 +27,10 @@ 
 

  import koji
 

  import cgi      #for parse_qs
 

  from context import context
 

+ try:
 

+     import pam
 

+ except ImportError:  # pragma: no cover
 

+     pam = None
 

  

  # 1 - load session if provided
 

  #       - check uri for session id
 
@@ -272,13 +276,28 @@ 
 

  

          # check passwd
 

          c = context.cnx.cursor()
 

-         q = """SELECT id FROM users
 

-         WHERE name = %(user)s AND password = %(password)s"""
 

-         c.execute(q, locals())
 

-         r = c.fetchone()
 

-         if not r:
 

-             raise koji.AuthError, 'invalid username or password'
 

-         user_id = r[0]
 

+         if context.opts.get('PAMService') and pam is not None:
 

+             if not pam.authenticate(user, password, context.opts.get('PAMService')):
 

+                 raise koji.AuthError, 'invalid username or password'
 

+             q = """SELECT id FROM users
 

+             WHERE name = %(user)s"""
 

+             c.execute(q, locals())
 

+             r = c.fetchone()
 

+             if r:
 

+                 user_id = r[0]
 

+             else:
 

+                 if context.opts.get('LoginCreatesUser'):
 

+                     user_id = self.createUser(user)
 

+                 else:
 

+                     raise koji.AuthError, 'Unknown user: %s' % user
 

+         else:
 

+             q = """SELECT id FROM users
 

+             WHERE name = %(user)s AND password = %(password)s"""
 

+             c.execute(q, locals())
 

+             r = c.fetchone()
 

+             if not r:
 

+                 raise koji.AuthError, 'invalid username or password'
 

+             user_id = r[0]
 

  

          self.checkLoginAllowed(user_id)

file modified
+2 
@@ -35,6 +35,8 @@ 
 

  class ServerRedirect(ServerError):
 

      """Used to handle redirects"""
 

  

+ class NotAuthorized(ServerError):
 

+     """Used to handle unauthorized"""
 

  

  class WSGIWrapper(object):
 

      """A very thin wsgi compat layer for mod_python

file modified
+5 
@@ -54,6 +54,11 @@ 
 

  #     SSLOptions +StdEnvVars
 

  # </Location>
 

  

+ # uncomment this to enable authentication via BasicAuth
 

+ # <Location /koji/login>
 

+ #     WSGIPassAuthorization On
 

+ # </Location>
 

+ 

  Alias /koji-static/ "/usr/share/koji-web/static/"
 

  

  <Directory "/usr/share/koji-web/static/">

file modified
+3 
@@ -17,6 +17,9 @@ 
 

  # WebCert = /etc/kojiweb/kojiweb.crt
 

  # KojiHubCA = /etc/kojiweb/kojihubca.crt
 

  

+ # BasicAuth authentication options
 

+ # BasicAuthRealm = Koji
 

+ 

  LoginTimeout = 72
 

  

  # This must be changed and uncommented before deployment

file modified
+17 -1 
@@ -31,7 +31,7 @@ 
 

  import time
 

  import koji
 

  import kojiweb.util
 

- from koji.server import ServerRedirect
 

+ from koji.server import ServerRedirect, NotAuthorized
 

  from kojiweb.util import _initValues
 

  from kojiweb.util import _genHTML
 

  from kojiweb.util import _getValidTokens
 
@@ -251,6 +251,22 @@ 
 

  

          username = principal
 

          authlogger.info('Successful Kerberos authentication by %s', username)
 

+     elif options['BasicAuthRealm']:
 

+         if environ['wsgi.url_scheme'] != 'https':
 

+             dest = 'login'
 

+             if page:
 

+                 dest = dest + '?page=' + page
 

+             _redirectBack(environ, dest, forceSSL=True)
 

+             return
 

+ 

+         http_authorization = environ.get('HTTP_AUTHORIZATION')
 

+         if not http_authorization:
 

+             raise NotAuthorized
 

+         session.opts['user'], session.opts['password'] = http_authorization.split(' ')[1].decode('base64').split(':')
 

+         if not session.login():
 

+             raise koji.AuthError, 'could not login %s using those credentials' % http_username
 

+         username = session.opts['user']
 

+         authlogger.info('Successful BasicAuth authentication by %s', username)
 

      else:
 

          raise koji.AuthError, 'KojiWeb is incorrectly configured for authentication, contact the system administrator'

file modified
+7 -1 
@@ -30,7 +30,7 @@ 
 

  import traceback
 

  

  from ConfigParser import RawConfigParser
 

- from koji.server import WSGIWrapper, ServerError, ServerRedirect
 

+ from koji.server import WSGIWrapper, ServerError, ServerRedirect, NotAuthorized
 

  from koji.util import dslice
 

  

  
@@ -80,6 +80,8 @@ 
 

          ['WebCert', 'string', None],
 

          ['KojiHubCA', 'string', '/etc/kojiweb/kojihubca.crt'],
 

  

+         ['BasicAuthRealm', 'string', None],
 

+ 

          ['PythonDebug', 'boolean', False],
 

  

          ['LoginTimeout', 'integer', 72],
 
@@ -403,6 +405,10 @@ 
 

              result, headers = self.error_page(environ, message=msg, err=False)
 

              start_response(status, headers)
 

              return result
 

+         except NotAuthorized:
 

+             status = "401 Not Authorized"
 

+             start_response(status, [('WWW-Authenticate', 'Basic realm="%s"' % self.options['BasicAuthRealm'])])
 

+             return '401 Not Authorized'
 

          except Exception:
 

              tb_str = ''.join(traceback.format_exception(*sys.exc_info()))
 

              self.logger.error(tb_str)

Adds support for PAM authentication for the koji-hub and BasicAuth for the koji-web.

This is useful for our internal use case as it allows us to login without the overhead of setting up either a CA or a kerberos realm for our users.

The configuration is backwards compatible and hopefully similar to the other authentication methods.

To active PAM support on hub you define the option:
PAMService = koji
in hub.conf. The value will be the name of the PAM service. Note the call to the PAM module is done via unprivileged call thus the use of pam_unix
won't be possible.

Note that activating this option will have as result that username/password combinations from the DB will no longer be checked (similarly to when activating kerberos or SSL client auth).

The BasicAuth for koji-web requires 2 changes:
a) To enable WSGIPassAuthorization for /koji/login in httpd configuration. That passes the authorization variable from the apache to the application.
b) Set the "BasicAuthRealm" option to the Basic Authentication Realm that will be presented to the user to login.

Finally python-pam package has been added to the hub's dependencies.

Thanks for the patch. At a glance the code looks fine, though I have a few concerns.

I'll start with the easy one. I'd rather not add a new dependency to cover this case. I think most people will continue to use other forms of auth. We could make this a soft dep, wrap the import in a try, and only error out if the option is actually enabled.

Now on to the harder one. The current password auth in koji is terrible and I recommend folks not use it (except perhaps for testing/debugging). I'm hesitant to add a feature like this without making regular password auth usable, particularly in the cli

Thank you for the review!

For the first one, agreed. I'll change that.

For the second I'm not sure I understand what you mean, would a password prompt instead of expecting --password do the trick?

2 new commits added

  • Removed python-pam requirement, dynamically enable PAM support instead
  • Prompt for username/password if they are not provided
9 years ago

@mikem does updated commit cover your concerns? Let me know if there is anything else that will be needed.

@mikem @ctria Any chance this would get merged in any time soon? The changes look fine to me, just from taking a look at the change diff.

I'm waiting on a new review from @mikem. Happy to rebase it to current master if that is the issue.

@mikem do you have any news on that? How should we proceed? Happy to rebase it if that is the only blocking issue.

rebased
8 years ago

rebased
8 years ago

@ctria I'm going through the Koji backlog... is this issue still something that you are looking for?

@tkopecek let's drop this pr as it's quite old and unclear whether it's still needed.

Pull-Request has been closed by tkopecek
6 years ago
Metadata
None
No Tags
Changes Summary 9
+3 -0
file changed
hub/hub.conf
+2 -0
file changed
hub/kojixmlrpc.py
+7 -1
file changed
koji/__init__.py
+26 -7
file changed
koji/auth.py
+2 -0
file changed
koji/server.py
+5 -0
file changed
www/conf/kojiweb.conf
+3 -0
file changed
www/conf/web.conf
+17 -1
file changed
www/kojiweb/index.py
+7 -1
file changed
www/kojiweb/wsgi_publisher.py
Powered by Pagure 5.14.1
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.