#1648 support multiple realms by kerberos auth
Merged 2 years ago by tkopecek. Opened 2 years ago by julian8628.
julian8628/koji issue/1629  into  master

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

                  'krbservice': 'host',

                  'krb_rdns': True,

                  'krb_canon_host': False,

+                 'krb_server_realm': None,

                  'server': None,

                  'user': None,

                  'password': None,

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

  ;enable to lookup dns canonical hostname for krb auth

  ;krb_canon_host = no

  

+ ;The realm of server principal. Using client's realm if not set

+ ;krb_server_realm = EXAMPLE.COM

+ 

  

  ;configuration for SSL authentication

  

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

      elif authtype == koji.AUTHTYPE_GSSAPI:

          print("Authenticated via GSSAPI")

      elif authtype == koji.AUTHTYPE_KERB:

-         print("Authenticated via Kerberos principal %s" % u["krb_principal"])

+         print("Authenticated via Kerberos principal %s" % session.krb_principal)

      elif authtype == koji.AUTHTYPE_SSL:

          print("Authenticated via client certificate %s" % options.cert)

  

@@ -65,4 +65,16 @@ 

  -- add kernel-image and imitramfs

  insert into archivetypes (name, description, extensions) values ('kernel-image', 'Kernel BZ2 Image', 'vmlinuz vmlinuz.gz vmlinuz.xz');

  insert into archivetypes (name, description, extensions) values ('initramfs', 'Compressed Initramfs Image', 'img');

+ 

+ -- schema update for https://pagure.io/koji/issue/1629

+ CREATE TABLE user_krb_principals (

+     user_id INTEGER NOT NULL REFERENCES users(id),

+     krb_principal VARCHAR(255) NOT NULL UNIQUE,

+     PRIMARY KEY (user_id, krb_principal)

+ ) WITHOUT OIDS;

+ 

+ INSERT INTO user_krb_principals ( SELECT id, krb_principal FROM users WHERE users.krb_principal IS NOT NULL);

+ 

+ ALTER TABLE users DROP COLUMN krb_principal;

+ 

  COMMIT;

file modified
+7 -2
@@ -38,8 +38,13 @@ 

  	name VARCHAR(255) UNIQUE NOT NULL,

  	password VARCHAR(255),

  	status INTEGER NOT NULL,

- 	usertype INTEGER NOT NULL,

- 	krb_principal VARCHAR(255) UNIQUE

+ 	usertype INTEGER NOT NULL

+ ) WITHOUT OIDS;

+ 

+ CREATE TABLE user_krb_principals (

+ 	user_id INTEGER NOT NULL REFERENCES users(id),

+ 	krb_principal VARCHAR(255) NOT NULL UNIQUE,

+ 	PRIMARY KEY (user_id, krb_principal)

  ) WITHOUT OIDS;

  

  CREATE TABLE permissions (

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

  # ProxyPrincipals = koji/kojiweb@EXAMPLE.COM

  ## format string for host principals (%s = hostname)

  # HostPrincipalFormat = compile/%s@EXAMPLE.COM

+ ## Allowed Kerberos Realms separated by ','.

+ ## Default value "*" indicates any Realm is allowed

+ # AllowedKrbRealms = *

  

  ## end Kerberos auth configuration

  

file modified
+156 -26
@@ -3663,40 +3663,104 @@ 

      return repos

  

  

- def get_user(userInfo=None, strict=False):

+ def get_user(userInfo=None, strict=False, krb_princs=False):

      """Return information about a user.

  

-     userInfo may be either a str (Kerberos principal or name) or an int (user id).

- 

-     A map will be returned with the following keys:

-       id: user id

-       name: user name

-       status: user status (int), may be null

-       usertype: user type (int), 0 person, 1 for host, may be null

-       krb_principal: the user's Kerberos principal

+     :param userInfo: either a str (Kerberos principal or name) or an int (user id)

+     :param strict: whether raising Error when no user found

+     :param krb_princs: whether show krb_principals in result

+     :return: a dict as user's information:

+         id: user id

+         name: user name

+         status: user status (int), may be null

+         usertype: user type (int), 0 person, 1 for host, may be null

+         krb_principals: the user's Kerberos principals (list)

      """

      if userInfo is None:

          userInfo = context.session.user_id

          if userInfo is None:

              # not logged in

              raise koji.GenericError("No user provided")

-     fields = ['id', 'name', 'status', 'usertype', 'krb_principal']

-     #fields, aliases = zip(*fields.items())

-     data = {'info' : userInfo}

+     fields = ['id', 'name', 'status', 'usertype']

+     data = {'info': userInfo}

      if isinstance(userInfo, six.integer_types):

          clauses = ['id = %(info)i']

      elif isinstance(userInfo, str):

          clauses = ['krb_principal = %(info)s OR name = %(info)s']

      else:

-         raise koji.GenericError('invalid type for userInfo: %s' % type(userInfo))

+         raise koji.GenericError('invalid type for userInfo: %s'

+                                 % type(userInfo))

      query = QueryProcessor(tables=['users'], columns=fields,

+                            joins=['LEFT JOIN user_krb_principals'

+                                   ' ON users.id = user_krb_principals.user_id'],

                             clauses=clauses, values=data)

      user = query.executeOne()

      if not user and strict:

          raise koji.GenericError("No such user: %r" % userInfo)

+     if user and krb_princs:

+         user['krb_principals'] = list_user_krb_principals(user['id'])

      return user

  

  

+ def list_user_krb_principals(user_info=None):

+     """Return kerberos principal list of a user.

+ 

+     :param user_info: either a str (username) or an int (user id)

+     :return: user's kerberos principals (list)

+     """

+     if user_info is None:

+         user_info = context.session.user_id

+         if user_info is None:

+             # not logged in

+             raise koji.GenericError("No user provided")

+     fields = ['krb_principal']

+     data = {'info': user_info}

+     if isinstance(user_info, six.integer_types):

+         joins = []

+         clauses = ['user_id = %(info)i']

+     elif isinstance(user_info, str):

+         joins = ['users ON users.id = user_krb_principals.user_id']

+         clauses = ['name = %(info)s']

+     else:

+         raise koji.GenericError('invalid type for user_info: %s'

+                                 % type(user_info))

+     query = QueryProcessor(tables=['user_krb_principals'],

+                            columns=fields, joins=joins,

+                            clauses=clauses, values=data,

+                            transform=lambda row: row['krb_principal'])

+     return query.execute() or []

+ 

+ 

+ def get_user_by_krb_principal(krb_principal, strict=False, krb_princs=False):

+     """get information about a user by kerberos principal.

+ 

+     :param krb_principal: full user kerberos principals

+     :param strict: whether raising Error when no user found

+     :param krb_princs: whether show krb_principals in result

+     :return: a dict as user's information:

+         id: user id

+         name: user name

+         status: user status (int), may be null

+         usertype: user type (int), 0 person, 1 for host, may be null

+         krb_principals: the user's Kerberos principals (list)

+     """

+     if krb_principal is None:

+         raise koji.GenericError("No kerberos principal provided")

+     if not isinstance(krb_principal, str):

+         raise koji.GenericError("invalid type for krb_principal: %s"

+                                 % type(krb_principal))

+     fields = ['user_id']

+     data = {'krb_principal': krb_principal}

+     clauses = ['krb_principal = %(krb_principal)s']

+     query = QueryProcessor(tables=['users_krb_principals'], columns=fields,

+                            clauses=clauses, values=data)

+     princ_item = query.executeOne()

+     if not princ_item and strict:

+         raise koji.GenericError("Cannot find user with kerberos principal: %s"

+                                 % krb_principal)

+     return get_user(data, strict=strict, krb_princ=krb_princs)

+ 

+ 

  def find_build_id(X, strict=False):

      if isinstance(X, six.integer_types):

          return X
@@ -7936,10 +8000,13 @@ 

      if not ginfo or ginfo['usertype'] != koji.USERTYPES['GROUP']:

          raise koji.GenericError("Not a group: %s" % group)

      group_id = ginfo['id']

-     fields = ('id', 'name', 'usertype', 'krb_principal')

+     fields = ('id', 'name', 'usertype', 'array_remove(array_agg(krb_principal)'

+                                         ', NULL) AS krb_principals')

      q = """SELECT %s FROM user_groups

-     JOIN users ON user_id = users.id

-     WHERE active = TRUE AND group_id = %%(group_id)i""" % ','.join(fields)

+     JOIN users ON user_groups.user_id = users.id

+     LEFT JOIN user_krb_principals ON users.id = user_krb_principals.user_id

+     WHERE active = TRUE AND group_id = %%(group_id)i

+     GROUP BY users.id""" % ','.join(fields)

      return _multiRow(q, locals(), fields)

  

  def set_user_status(user, status):
@@ -8243,13 +8310,16 @@ 

          asList: if True, return results as a list of lists, where each list contains the

                  column values in query order, rather than the usual list of maps

          rowlock: if True, use "FOR UPDATE" to lock the queried rows

+         group: a column or alias name to use in the 'GROUP BY' clause

+                (controlled by enable_group)

+     - enable_group: if True, opts.group will be enabled

      """

  

      iterchunksize = 1000

  

      def __init__(self, columns=None, aliases=None, tables=None,

                   joins=None, clauses=None, values=None, transform=None,

-                  opts=None):

+                  opts=None, enable_group=False):

          self.columns = columns

          self.aliases = aliases

          if columns and aliases:
@@ -8282,6 +8352,7 @@ 

              self.opts = opts

          else:

              self.opts = {}

+         self.enable_group = enable_group

  

      def countOnly(self, count):

          self.opts['countOnly'] = count
@@ -8293,6 +8364,7 @@ 

    FROM %(table_str)s

  %(join_str)s

  %(clause_str)s

+  %(group_str)s

   %(order_str)s

  %(offset_str)s

   %(limit_str)s
@@ -8314,6 +8386,10 @@ 

          clause_str = self._seqtostr(self.clauses, sep=')\n   AND (')

          if clause_str:

              clause_str = ' WHERE (' + clause_str + ')'

+         if self.enable_group:

+             group_str = self._group()

+         else:

+             group_str = ''

          order_str = self._order()

          offset_str = self._optstr('offset')

          limit_str = self._optstr('limit')
@@ -8379,6 +8455,17 @@ 

          else:

              return ''

  

+     def _group(self):

+         group_opt = self.opts.get('group')

+         if group_opt:

+             group_exprs = []

+             for group in group_opt.split(','):

+                 if group:

+                     group_exprs.append(group)

+             return 'GROUP BY ' + ', '.join(group_exprs)

+         else:

+             return ''

+ 

      def _optstr(self, optname):

          optval = self.opts.get(optname)

          if optval:
@@ -8480,9 +8567,11 @@ 

        offset

        limit

  

-     Note: asList is supported by QueryProcessor but not by this method.

+     Note:

+     - asList is supported by QueryProcessor but not by this method.

      We don't know the original query order, and so don't have a way to

      return a useful list.  asList should be handled by the caller.

+     - group is supported by QueryProcessor but not by this method as well.

      """

      if queryOpts is None:

          queryOpts = {}
@@ -10885,10 +10974,34 @@ 

          context.session.assertPerm('admin')

          if get_user(username):

              raise koji.GenericError('user already exists: %s' % username)

-         if krb_principal and get_user(krb_principal):

-             raise koji.GenericError('user with this Kerberos principal already exists: %s' % krb_principal)

+         if krb_principal and get_user_by_krb_principal(krb_principal):

+             raise koji.GenericError(

+                 'user with this Kerberos principal already exists: %s'

+                 % krb_principal)

+ 

+         return context.session.createUser(username, status=status,

+                                           krb_principal=krb_principal)

  

-         return context.session.createUser(username, status=status, krb_principal=krb_principal)

+     def addUserKrbPrincipal(self, user, krb_principal):

+         """Add a Kerberos principal for user"""

+         context.session.assertPerm('admin')

+         userinfo = get_user(user, strict=True)

+         if not krb_principal:

+             raise koji.GenericError('krb_principal must be specified')

+         if get_user_by_krb_principal(krb_principal):

+             raise koji.GenericError(

+                 'user with this Kerberos principal already exists: %s'

+                 % krb_principal)

+         return context.session.setKrbPrincipal(userinfo['name'], krb_principal)

+ 

+     def removeUserKrbPrincipal(self, user, krb_principal):

+         """remove a Kerberos principal for user"""

+         context.session.assertPerm('admin')

+         userinfo = get_user(user, strict=True)

+         if not krb_principal:

+             raise koji.GenericError('krb_principal must be specified')

+         return context.session.removeKrbPrincipal(userinfo['name'],

+                                                   krb_principal)

  

      def enableUser(self, username):

          """Enable logins by the specified user"""
@@ -10922,16 +11035,28 @@ 

          - name

          - status

          - usertype

-         - krb_principal

+         - krb_principals

  

          If no users of the specified

          type exist, return an empty list."""

-         fields = ('id', 'name', 'status', 'usertype', 'krb_principal')

+         fields = ('id', 'name', 'status', 'usertype',

+                   'array_remove(array_agg(krb_principal), NULL)')

+         aliases = ('id', 'name', 'status', 'usertype', 'krb_principals')

+         joins = ('LEFT JOIN user_krb_principals'

+                  ' ON users.id = user_krb_principals.user_id',)

          clauses = ['usertype = %(userType)i']

          if prefix:

              clauses.append("name ilike %(prefix)s || '%%'")

-         query = QueryProcessor(columns=fields, tables=('users',), clauses=clauses,

-                                values=locals(), opts=queryOpts)

+         if queryOpts is None:

+             queryOpts = {}

+         if not queryOpts.get('group'):

+             queryOpts['group'] = 'users.id'

+         else:

+             raise koji.GenericError('queryOpts.group is not available for this API')

+         query = QueryProcessor(columns=fields, aliases=aliases,

+                                tables=('users',), joins=joins, clauses=clauses,

+                                values=locals(), opts=queryOpts,

+                                enable_group=True)

          return query.execute()

  

      def getBuildConfig(self, tag, event=None):
@@ -11476,8 +11601,13 @@ 

          in the same format as getUser(), plus the authtype.  If there is no

          currently logged-in user, return None."""

          if context.session.logged_in:

-             me = self.getUser(context.session.user_id)

+             me = self.getUser(context.session.user_id, krb_princs=True)

              me['authtype'] = context.session.authtype

+             # backward compatible for cli moshimoshi, but it's not real

+             if me.get('krb_principals'):

+                 me['krb_principal'] = me['krb_principals'][0]

+             else:

+                 me['krb_principal'] = None

              return me

          else:

              return None

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

          ['AuthKeytab', 'string', None],

          ['ProxyPrincipals', 'string', ''],

          ['HostPrincipalFormat', 'string', None],

+         ['AllowedKrbRealms', 'string', '*'],

  

          ['DNUsernameComponent', 'string', 'CN'],

          ['ProxyDNs', 'string', ''],

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

          'krbservice': 'host',

          'krb_rdns': True,

          'krb_canon_host': False,

+         'krb_server_realm': None,

          'principal': None,

          'keytab': None,

          'cert': None,
@@ -2124,6 +2125,7 @@ 

          'upload_blocksize',

          'krb_rdns',

          'krb_canon_host',

+         'krb_server_realm',

          'no_ssl_verify',

          'serverca',

      )
@@ -2293,6 +2295,7 @@ 

              return False

          self.setSession(sinfo)

  

+         self.krb_principal = cprinc.name

          self.authtype = AUTHTYPE_KERB

          return True

  
@@ -2302,7 +2305,9 @@ 

  

          host = six.moves.urllib.parse.urlparse(self.baseurl).hostname

          servername = self._fix_krb_host(host)

-         realm = cprinc.realm

+         realm = self.opts.get('krb_server_realm')

+         if not realm:

+             realm = cprinc.realm

          service = self.opts.get('krbservice', 'host')

  

          ret = '%s/%s@%s' % (service, servername, realm)

file modified
+103 -33
@@ -23,6 +23,7 @@ 

  import socket

  import string

  import random

+ import re

  import base64

  try:

      import krbV
@@ -339,16 +340,22 @@ 

                        'Kerberos principal %s is not authorized to log in other users' % cprinc.name)

          else:

              login_principal = cprinc.name

-         user_id = self.getUserIdFromKerberos(login_principal)

-         if not user_id:

+ 

+         if '@' in login_principal:

+             user_id = self.getUserIdFromKerberos(login_principal)

+         else:

+             # backward compatible

+             # it's only possible when proxyuser is username, but we shouldn't

+             # allow this.

              user_id = self.getUserId(login_principal)

-             if not user_id:

-                 # Only do autocreate if we also couldn't find by username AND the proxyuser

-                 # looks like a krb5 principal

-                 if context.opts.get('LoginCreatesUser') and '@' in login_principal:

-                     user_id = self.createUserFromKerberos(login_principal)

-                 else:

-                     raise koji.AuthError('Unknown Kerberos principal: %s' % login_principal)

+ 

+         if not user_id:

+             # Only do autocreate if we also couldn't find by username AND the proxyuser

+             # looks like a krb5 principal

+             if context.opts.get('LoginCreatesUser') and '@' in login_principal:

+                 user_id = self.createUserFromKerberos(login_principal)

+             else:

+                 raise koji.AuthError('Unknown Kerberos principal: %s' % login_principal)

  

          self.checkLoginAllowed(user_id)

  
@@ -514,6 +521,7 @@ 

          c.execute(q, {})

          (session_id,) = c.fetchone()

  

+ 

          #add session id to database

          q = """

          INSERT INTO sessions (id, user_id, key, hostip, authtype, master)
@@ -602,8 +610,12 @@ 

      def getUserIdFromKerberos(self, krb_principal):

          """Return the user ID associated with a particular Kerberos principal.

          If no user with the given princpal if found, return None."""

+         self.checkKrbPrincipal(krb_principal)

          c = context.cnx.cursor()

-         q = """SELECT id FROM users WHERE krb_principal = %(krb_principal)s"""

+         q = """SELECT id FROM users

+                JOIN user_krb_principals

+                ON users.id = user_krb_principals.user_id

+                WHERE krb_principal = %(krb_principal)s"""

          c.execute(q, locals())

          r = c.fetchone()

          c.close()
@@ -612,7 +624,8 @@ 

          else:

              return None

  

-     def createUser(self, name, usertype=None, status=None, krb_principal=None):

+     def createUser(self, name, usertype=None, status=None, krb_principal=None,

+                    krb_princ_check=True):

          """

          Create a new user, using the provided values.

          Return the user_id of the newly-created user.
@@ -620,41 +633,75 @@ 

          if not name:

              raise koji.GenericError('a user must have a non-empty name')

  

-         if usertype == None:

+         if usertype is None:

              usertype = koji.USERTYPES['NORMAL']

          elif not koji.USERTYPES.get(usertype):

              raise koji.GenericError('invalid user type: %s' % usertype)

  

-         if status == None:

+         if status is None:

              status = koji.USER_STATUS['NORMAL']

          elif not koji.USER_STATUS.get(status):

              raise koji.GenericError('invalid status: %s' % status)

  

+         # check if krb_principal is allowed

+         if krb_princ_check:

+             self.checkKrbPrincipal(krb_principal)

+ 

          cursor = context.cnx.cursor()

          select = """SELECT nextval('users_id_seq')"""

          cursor.execute(select, locals())

          user_id = cursor.fetchone()[0]

  

-         insert = """INSERT INTO users (id, name, usertype, status, krb_principal)

-         VALUES (%(user_id)i, %(name)s, %(usertype)i, %(status)i, %(krb_principal)s)"""

+         insert = """INSERT INTO users (id, name, usertype, status)

+                     VALUES (%(user_id)i, %(name)s, %(usertype)i, %(status)i"""

          cursor.execute(insert, locals())

+         if krb_principal:

+             insert = """INSERT INTO user_krb_principals (user_id, krb_principal)

+                         VALUES (%(user_id)i, %(krb_principal)s)"""

+             cursor.execute(insert, locals())

          context.cnx.commit()

  

          return user_id

  

-     def setKrbPrincipal(self, name, krb_principal):

-         usertype = koji.USERTYPES['NORMAL']

-         status = koji.USER_STATUS['NORMAL']

-         update = """UPDATE users SET krb_principal = %(krb_principal)s WHERE name = %(name)s AND usertype = %(usertype)i AND status = %(status)i RETURNING users.id"""

+     def setKrbPrincipal(self, name, krb_principal, krb_princ_check=True):

+         if krb_princ_check:

+             self.checkKrbPrincipal(krb_principal)

+         select = """SELECT id FROM users WHERE name = %(name)s"""

+         cursor = context.cnx.cursor()

+         cursor.execute(select, locals())

+         r = cursor.fetchone()

+         if not r:

+             context.cnx.rollback()

+             raise koji.AuthError('No such user: %s' % name)

+         else:

+             user_id = r[0]

+         insert = """INSERT INTO user_krb_principals (user_id, krb_principal)

+                     VALUES (%(user_id)i, %(krb_principal)s)"""

+         cursor.execute(insert, locals())

+         context.cnx.commit()

+         return user_id

+ 

+     def removeKrbPrincipal(self, name, krb_principal):

+         select = """SELECT id FROM users

+                     JOIN user_krb_principals

+                     WHERE name = %(name)s

+                     AND krb_principal = %(krb_principal)s"""

          cursor = context.cnx.cursor()

-         cursor.execute(update, locals())

-         r = cursor.fetchall()

-         if len(r) != 1:

+         cursor.execute(select, locals())

+         r = cursor.fetchone()

+         if not r:

              context.cnx.rollback()

-             raise koji.AuthError('could not automatically associate Kerberos Principal with existing user %s' % name)

+             raise koji.AuthError(

+                 'could not automatically remove Kerberos Principal:'

+                 ' %(krb_principal)s with user %(name)s' % locals())

          else:

-             context.cnx.commit()

-             return r[0][0]

+             user_id = r[0]

+         delete = """DELETE FROM user_krb_principals

+                     WHERE user_id = (%(user_id)i

+                     AND krb_principal = %(krb_principal)s"""

+         cursor.execute(delete, locals())

+         context.cnx.commit()

+         return user_id

  

      def createUserFromKerberos(self, krb_principal):

          """Create a new user, based on the Kerberos principal.  Their
@@ -667,17 +714,40 @@ 

  

          # check if user already exists

          c = context.cnx.cursor()

-         q = """SELECT krb_principal FROM users

-         WHERE name = %(user_name)s"""

+         q = """SELECT id, krb_principal FROM users

+                LEFT JOIN user_krb_principals

+                ON users.id = user_krb_principals.user_id

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

          c.execute(q, locals())

-         r = c.fetchone()

+         r = c.fetchall()

          if not r:

-             return self.createUser(user_name, krb_principal=krb_principal)

+             return self.createUser(user_name, krb_principal=krb_principal,

+                                    krb_princ_check=False)

          else:

-             existing_user_krb = r[0]

-             if existing_user_krb is not None:

-                 raise koji.AuthError('user %s already associated with other Kerberos principal: %s' % (user_name, existing_user_krb))

-             return self.setKrbPrincipal(user_name, krb_principal)

+             existing_user_krb_princs = [row[1] for row in r]

+             if krb_principal in existing_user_krb_princs:

+                 # do not set Kerberos principal if it already exists

+                 return r[0][0]

+             return self.setKrbPrincipal(user_name, krb_principal,

+                                         krb_princ_check=False)

+ 

+     def checkKrbPrincipal(self, krb_principal):

+         """Check if the Kerberos principal is allowed"""

+         if krb_principal is None:

+             return

+         allowed_realms = context.opts.get('AllowedKrbRealms', '*')

+         if allowed_realms == '*':

+             return

+         allowed_realms = re.split(r'\s*,\s*', allowed_realms)

+         atidx = krb_principal.find('@')

+         if atidx == -1 or atidx == len(krb_principal) - 1:

+             raise koji.AuthError(

+                 'invalid Kerberos principal: %s' % krb_principal)

+         realm = krb_principal[atidx + 1:]

+         if realm not in allowed_realms:

+             raise koji.AuthError(

+                 "Kerberos principal's realm: %s is not allowed" % realm)

+ 

  

  def get_user_groups(user_id):

      """Get user groups

file modified
+1 -1
@@ -52,10 +52,10 @@ 

                  'krb_principal': '%s@localhost' % self.progname}

          cert = '/etc/pki/user.cert'

          options = mock.MagicMock()

- 

          # Mock out the xmlrpc server

          session = mock.MagicMock(baseurl=self.huburl, authtype=None)

          session.getLoggedInUser.return_value = None

+         session.krb_principal = user['krb_principal']

          print_unicode_mock.return_value = "Hello"

  

          expect = """Usage: %s moshimoshi [options]

@@ -26,8 +26,10 @@ 

                  'order': 'other',

                  'offset': 10,

                  'limit': 3,

+                 'group': 'awesome.aha'

                  #'rowlock': True,

              },

+             enable_group=True

          )

          self.original_chunksize = kojihub.QueryProcessor.iterchunksize

          kojihub.QueryProcessor.iterchunksize = 2
@@ -62,7 +64,15 @@ 

      def test_complex_as_string(self):

          proc = kojihub.QueryProcessor(**self.complex_arguments)

          actual = " ".join([token for token in str(proc).split() if token])

-         expected = "SELECT something FROM awesome JOIN morestuff ORDER BY something OFFSET 10 LIMIT 3"

+         expected = "SELECT something FROM awesome JOIN morestuff" \

+                    " GROUP BY awesome.aha ORDER BY something OFFSET 10 LIMIT 3"

+         self.assertEqual(actual, expected)

+         args2 = self.complex_arguments.copy()

+         args2['enable_group'] = False

+         proc = kojihub.QueryProcessor(**args2)

+         actual = " ".join([token for token in str(proc).split() if token])

+         expected = "SELECT something FROM awesome JOIN morestuff" \

+                    " ORDER BY something OFFSET 10 LIMIT 3"

          self.assertEqual(actual, expected)

  

      @mock.patch('kojihub.context')
@@ -71,7 +81,7 @@ 

          context.cnx.cursor.return_value = cursor

          proc = kojihub.QueryProcessor(**self.simple_arguments)

          proc.execute()

-         cursor.execute.assert_called_once_with('\nSELECT something\n  FROM awesome\n\n\n \n\n \n', {})

+         cursor.execute.assert_called_once_with('\nSELECT something\n  FROM awesome\n\n\n \n \n\n \n', {})

  

      @mock.patch('kojihub.context')

      def test_simple_count_with_execution(self, context):
@@ -82,7 +92,7 @@ 

          args['opts'] = {'countOnly': True}

          proc = kojihub.QueryProcessor(**args)

          results = proc.execute()

-         cursor.execute.assert_called_once_with('\nSELECT count(*)\n  FROM awesome\n\n\n \n\n \n', {})

+         cursor.execute.assert_called_once_with('\nSELECT count(*)\n  FROM awesome\n\n\n \n \n\n \n', {})

          self.assertEqual(results, 'some count')

  

      @mock.patch('kojihub.context')

file modified
+50 -2
@@ -209,16 +209,64 @@ 

                  self.assertEqual(cm.exception.args[0],

                                   'Unknown Kerberos principal:'

                                   ' proxyuser@realm.com')

+                 # case: create user by kerberos

                  context.opts['LoginCreatesUser'] = True

                  context.cnx.cursor.return_value. \

                      fetchone.side_effect = [None,

-                                             None,

-                                             None,

                                              (1,),

                                              ('name', 'type',

                                               koji.USER_STATUS['NORMAL']),

                                              ('session-id',)]

+                 context.cnx.cursor.return_value.fetchall.return_value = None

                  s.krbLogin('krb_req', 'proxyuser@realm.com')

+                 self.assertEqual(context.cnx.cursor.return_value.execute.

+                                  call_count, 14)

+                 # case: create user by username, proxyuser is username

+                 context.cnx.cursor.return_value. \

+                     fetchone.side_effect = [None]

+                 context.cnx.cursor.return_value.fetchall.return_value = None

+                 with self.assertRaises(koji.AuthError) as cm:

+                     s.krbLogin('krb_req', 'proxyuser')

+                 self.assertEqual(cm.exception.args[0],

+                                  'Unknown Kerberos principal: proxyuser')

+                 # case: create user by kerberos - set krb princ

+                 context.opts['LoginCreatesUser'] = True

+                 context.cnx.cursor.return_value. \

+                     fetchone.side_effect = [None,

+                                             (1,),

+                                             ('name', 'type',

+                                              koji.USER_STATUS['NORMAL']),

+                                             ('session-id',)]

+                 context.cnx.cursor.return_value.fetchall. \

+                     return_value = [('proxyuser', 'proxyuser@otherrealm.com')]

+                 s.krbLogin('krb_req', 'proxyuser@realm.com')

+                 self.assertEqual(context.cnx.cursor.return_value.execute.

+                                  call_count, 22)

+ 

+     @mock.patch('koji.auth.context')

+     def test_checkKrbPrincipal(self, context):

+         s, cntext, cursor = self.get_session()

+         self.assertIsNone(s.checkKrbPrincipal(None))

+         context.opts = {'AllowedKrbRealms': '*'}

+         self.assertIsNone(s.checkKrbPrincipal('any'))

+         context.opts = {'AllowedKrbRealms': 'example.com'}

+         with self.assertRaises(koji.AuthError) as cm:

+             s.checkKrbPrincipal('any')

+         self.assertEqual(cm.exception.args[0],

+                          'invalid Kerberos principal: any')

+         with self.assertRaises(koji.AuthError) as cm:

+             s.checkKrbPrincipal('any@')

+         self.assertEqual(cm.exception.args[0],

+                          'invalid Kerberos principal: any@')

+         with self.assertRaises(koji.AuthError) as cm:

+             s.checkKrbPrincipal('any@bannedrealm')

+         self.assertEqual(cm.exception.args[0],

+                          "Kerberos principal's realm:"

+                          " bannedrealm is not allowed")

+         self.assertIsNone(s.checkKrbPrincipal('user@example.com'))

+         context.opts = {'AllowedKrbRealms': 'example.com,example.net'

+                                             ' , example.org'}

+         self.assertIsNone(s.checkKrbPrincipal('user@example.net'))

  

      # functions outside Session object

  

file modified
+3
@@ -48,6 +48,8 @@ 

                        help=_("get reverse dns FQDN for krb target"))

      parser.add_option("--krb-canon-host", action="store_true", default=False,

                        help=_("get canonical hostname for krb target"))

+     parser.add_option("--krb-server-realm",

+                       help=_("the realm of server Kerberos principal"))

      parser.add_option("--runas", metavar="USER",

                        help=_("run as the specified user (requires special privileges)"))

      parser.add_option("--user", help=_("specify user"))
@@ -131,6 +133,7 @@ 

              ['krbservice', None, 'string'],

              ['krb_rdns', None, 'boolean'],

              ['krb_canon_host', None, 'boolean'],

+             ['krb_server_realm', None, 'string'],

              ['runas', None, 'string'],

              ['user', None, 'string'],

              ['password', None, 'string'],

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

  # The service name of the principal being used by the hub

  #krbservice = host

  

+ ## The realm of server principal. Using client's realm if not set

+ # krb_server_realm = EXAMPLE.COM

+ 

  # The domain name that will be appended to Koji usernames

  # when creating email notifications

  #email_domain = fedoraproject.org

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

                        help=_("get reverse dns FQDN for krb target"))

      parser.add_option("--krb-canon-host", action="store_true", default=False,

                        help=_("get canonical hostname for krb target"))

+     parser.add_option("--krb-server-realm",

+                       help=_("the realm of server Kerberos principal"))

      parser.add_option("--noauth", action="store_true", default=False,

                        help=_("do not authenticate"))

      parser.add_option("-n", "--test", action="store_true", default=False,

file modified
+3
@@ -5,3 +5,6 @@ 

  server=http://localhost/kojihub/

  krbservice=host

  remote=https://koji.fedoraproject.org/kojihub

+ 

+ ## The realm of server principal. Using client's realm if not set

+ # krb_server_realm = EXAMPLE.COM

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

                  'krbservice': 'host',

                  'krb_rdns': True,

                  'krb_canon_host': False,

+                 'krb_server_realm': None,

                  'retry_interval': 60,

                  'max_retries': 120,

                  'offline_retry': True,

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

  ;the service name of the principal being used by the hub

  ;krbservice = host

  

+ ;The realm of server principal. Using client's realm if not set

+ ;krb_server_realm = EXAMPLE.COM

+ 

  ;configuration for SSL authentication

  

  ;client certificate

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

                  'krbservice': 'host',

                  'krb_rdns': True,

                  'krb_canon_host': False,

+                 'krb_server_realm': None,

                  'server': None,

                  'user': None,

                  'password': None,

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

  ;the service name of the principal being used by the hub

  ;krbservice = host

  

+ ;The realm of server principal. Using client's realm if not set

+ ;krb_server_realm = EXAMPLE.COM

+ 

  ;configuration for SSL authentication

  

  ;client certificate

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

  # WebCCache = /var/tmp/kojiweb.ccache

  # The service name of the principal being used by the hub

  # KrbService = host

+ ## The realm of server principal. Using client's realm if not set

+ # KrbServerRealm = EXAMPLE.COM

  

  # SSL authentication options

  # WebCert = /etc/kojiweb/kojiweb.crt

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

      s_opts = {'krbservice': opts['KrbService'],

                'krb_rdns': opts['KrbRDNS'],

                'krb_canon_host': opts['KrbCanonHost'],

+               'krb_server_realm': opts['KrbServerRealm']

                }

      session = koji.ClientSession(opts['KojiHubURL'], opts=s_opts)

  

@@ -79,6 +79,7 @@ 

          ['KrbService', 'string', 'host'],

          ['KrbRDNS', 'boolean', True],

          ['KrbCanonHost', 'boolean', False],

+         ['KrbServerRealm', 'string', None],

  

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

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

  • a new table user_krb_principals instead of one-on-one mapping users.krb_principal
  • all APIs related to user or krb principals are changed
  • now, userinfo of getUser won't contain krb_principal. Instead, it will contain a new list krb_principals having all available princs if krb_princs=True
  • a new hub option: AllowedKrbRealms to indicate which realms are allowed
  • a new client option krb_server_realm to allow krbV login to set server realm, which was the same as client princ realm before. This support all clients - cli, builder, web(KrbServerRealm), vmbuilder, gc, kojira, shadow
  • QueryProcessor has a new queryOpt group, which is used to generate GROUP BY section. By default, this feature is disabled by arg: enable_group=False
  • with PR #1419, GSSAPI will natively support multiple realms as well, auth_to_local mapping in /etc/krb5.conf is not necessary anymore, we could just set GssapiLocalName Off in httpd.conf

related: #1419
fixes: #1629

Would it make sense to be able to set the server realm in hub.conf or something similar?

Would it make sense to be able to set the server realm in hub.conf or something similar?

Do you mean let hub decide which server realm should be used in krbV way?
I think that, client doesn't know the server realm, unless having a new API to return it before aunthenticating. It looks a little complex and maybe introduce some extra time consuming. (client has to call this API, no matter if server realm is the same as the client one)

Makes sense, LGTM as is.

@julian8628 Also here we need simple rebase.

rebased onto 8b43f03f437031c6a0785ec9754e0b778ebc1c7e

2 years ago

rebased onto bfdcb1b

2 years ago

Commit 0fa9eb0 fixes this pull-request

Pull-Request has been merged by tkopecek

2 years ago