#313 Make the test suite pass on Python3
Merged 4 years ago by simo. Opened 5 years ago by puiterwijk.
puiterwijk/ipsilon py3_everything  into  master

file modified
+11 -12
@@ -8,7 +8,6 @@ 

  testdeps:

  	# Determine if test deps are installed

  	# First, some binaries

- 	which pylint-2

  	which pep8

  	which httpd

  	which postgres
@@ -27,7 +26,6 @@ 

  	python -c 'import ldap'

  	python -c 'import pam'

  	python -c 'import fedora'

- 	python -c 'import ipapython'

  	python -c 'import jinja2'

  	python -c 'import psycopg2'

  	# And now everything else
@@ -42,7 +40,7 @@ 

          # W0613 - unused argument

  	# Ignore cherrypy class members as they are dynamically added

  	# Ignore IPA API class members as they are dynamically added

- 	pylint-2 -d c,r,i,W0613 -r n -f colorized \

+ 	pylint-3 -d c,r,i,W0613 -r n -f colorized \

  		   --notes= \

  		   --ignored-classes=cherrypy,API \

  		   --disable=star-args \
@@ -50,7 +48,7 @@ 

  

  pep8:

  	# Check style consistency

- 	pep8 ipsilon

+ 	pep8 ipsilon || python3-pep8 ipsilon

pycodestyle-3 (from python3-pycodestyle) is the actively maintained fork of pep8.

  

  security:

  	# Run a static analyzer aimed at security (OpenStack Bandit)
@@ -78,7 +76,7 @@ 

  	git ls-files | xargs pycscope

  

  lp-test:

- 	pylint-2 -d c,r,i,W0613 -r n -f colorized \

+ 	pylint-3 -d c,r,i,W0613 -r n -f colorized \

  		   --notes= \

  		   --ignored-classes=cherrypy \

  		   --disable=star-args \
@@ -91,11 +89,11 @@ 

  	echo "Testdir: $(TESTDIR)"

  	./runtests --path=$(TESTDIR)

  

- test: lp-test unittests tests

+ test: unittests tests

  

  unittests:

- 	PYTHONPATH=./ ./ipsilon/tools/saml2metadata.py

- 	PYTHONPATH=./ python ./ipsilon/util/policy.py

+ 	which python && PYTHONPATH=./ python ./ipsilon/tools/saml2metadata.py || PYTHONPATH=./ python3 ./ipsilon/tools/saml2metadata.py

+ 	which python && PYTHONPATH=./ python ./ipsilon/util/policy.py || PYTHONPATH=./ python3 ./ipsilon/util/policy.py

  

  sdist:

  	python setup.py sdist
@@ -139,21 +137,22 @@ 

  # Testing within containers

  container-centos6:

  	@echo "Building CentOS 6 container ..."

- 	@(cat tests/containers/Dockerfile-base tests/containers/Dockerfile-centos tests/containers/Dockerfile-rpm; echo "USER testuser") | sed -e 's/BASE/centos:6/' | docker build -f - -q -t ipsilon-centos6 - && echo "CentOS 6 container built" || echo "CentOS 6 container build failed (optional)"

+ 	@(cat tests/containers/Dockerfile-base tests/containers/Dockerfile-centos tests/containers/Dockerfile-rpm tests/containers/Dockerfile-rpm-py2; echo "USER testuser") | sed -e 's/BASE/centos:6/' | docker build -f - -q -t ipsilon-centos6 - && echo "CentOS 6 container built" || echo "CentOS 6 container build failed (optional)"

  

  container-centos7:

  	@echo "Building CentOS 7 container ..."

- 	@(cat tests/containers/Dockerfile-base tests/containers/Dockerfile-centos tests/containers/Dockerfile-rpm; echo "USER testuser") | sed -e 's/BASE/centos:7/' | docker build -f - -q -t ipsilon-centos7 -

+ 	@(cat tests/containers/Dockerfile-base tests/containers/Dockerfile-centos tests/containers/Dockerfile-rpm tests/containers/Dockerfile-rpm-py2; echo "USER testuser") | sed -e 's/BASE/centos:7/' | docker build -f - -q -t ipsilon-centos7 -

  	@echo "CentOS 7 container built"

  

  container-fedora28:

  	@echo "Building Fedora 28 container ..."

- 	@(cat tests/containers/Dockerfile-base tests/containers/Dockerfile-fedora tests/containers/Dockerfile-rpm; echo "USER testuser") | sed -e 's/BASE/fedora:28/' | docker build -f - -q -t ipsilon-fedora28 -

+ 	# Fedora 28 is missing python3-lasso. When this gets bumped, use py3

+ 	@(cat tests/containers/Dockerfile-base tests/containers/Dockerfile-fedora tests/containers/Dockerfile-rpm tests/containers/Dockerfile-rpm-py2; echo "USER testuser") | sed -e 's/BASE/fedora:28/' | docker build -f - -q -t ipsilon-fedora28 -

  	@echo "Fedora 28 container built"

  

  container-fedora29:

  	@echo "Building Fedora 29 container ..."

- 	@(cat tests/containers/Dockerfile-base tests/containers/Dockerfile-fedora tests/containers/Dockerfile-rpm; echo "USER testuser") | sed -e 's/BASE/fedora:29/' | docker build -f - -q -t ipsilon-fedora29 -

+ 	@(cat tests/containers/Dockerfile-base tests/containers/Dockerfile-fedora tests/containers/Dockerfile-rpm tests/containers/Dockerfile-rpm-py3; echo "USER testuser") | sed -e 's/BASE/fedora:29/' | docker build -f - -q -t ipsilon-fedora29 -

  	@echo "Fedora 29 container built"

  

  containers: container-centos6 container-centos7 container-fedora28 container-fedora29

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

  

  import cherrypy

  import logging

+ import six

  from ipsilon.util.page import Page

  from ipsilon.util.page import admin_protect

  from ipsilon.util.endpoint import allow_iframe
@@ -66,7 +67,7 @@ 

  

          conf = self._po.get_config_obj()

  

-         for name, option in conf.iteritems():

+         for name, option in six.iteritems(conf):

              if name in kwargs:

                  value = kwargs[name]

                  if isinstance(option, pconfig.List):
@@ -338,7 +339,7 @@ 

  def get_complex_list_value(name, old_value, **kwargs):

      delete = list()

      change = dict()

-     for key, val in kwargs.iteritems():

+     for key, val in six.iteritems(kwargs):

          if not key.startswith(name):

              continue

          n = key[len(name):]
@@ -364,7 +365,7 @@ 

              del change[i]

  

      # perform requested changes

-     for index, val in change.iteritems():

+     for index, val in six.iteritems(change):

          if val.startswith(('http://', 'https://')):

              val_list = [val]

          else:
@@ -384,7 +385,7 @@ 

  

      # the previous loop may add 'None' entries

      # if any still exists mark them to be deleted

-     for i in xrange(0, len(value)):

+     for i in six.moves.range(0, len(value)):

          if value[i] is None:

              delete.append(i)

  
@@ -404,7 +405,7 @@ 

  def get_mapping_list_value(name, old_value, **kwargs):

      delete = list()

      change = dict()

-     for key, val in kwargs.iteritems():

+     for key, val in six.iteritems(kwargs):

          if not key.startswith(name):

              continue

          n = key[len(name):]
@@ -433,7 +434,7 @@ 

              del change[i]

  

      # perform requested changes

-     for index, fields in change.iteritems():

+     for index, fields in six.iteritems(change):

          for k in 'from', 'to':

              if k in fields:

                  val = fields[k]
@@ -468,7 +469,7 @@ 

  

      # the previous loop may add 'None' entries

      # if any still exists mark them to be deleted

-     for i in xrange(0, len(value)):

+     for i in six.moves.range(0, len(value)):

          if value[i] is None:

              delete.append(i)

  

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

  from ipsilon.util.policy import Policy

  from ipsilon.util import config as pconfig

  import ldap

+ import six

  import subprocess

  

  
@@ -120,7 +121,7 @@ 

          elif len(result) > 1:

              raise Exception('No unique user object could be found!')

          data = dict()

-         for name, value in result[0][1].iteritems():

+         for name, value in six.iteritems(result[0][1]):

              if isinstance(value, list) and len(value) == 1:

                  value = value[0]

              data[name] = value

file modified
+2 -3
@@ -32,11 +32,10 @@ 

  

      def _get_posix_groups(self, user, group):

          groups = set()

-         getgrouplist = getattr(os, 'getgrouplist', None)

-         if getgrouplist:

+         if hasattr(os, 'getgrouplist'):

              # On python2, None is not callable. On python3, this is a function

              # pylint: disable=not-callable

-             ids = getgrouplist(user, group)

+             ids = os.getgrouplist(user, group)

              for i in ids:

                  try:

                      g = grp.getgrgid(i)

@@ -7,9 +7,7 @@ 

  from ipsilon.tools.saml2metadata import SAML2_SERVICE_MAP

  from ipsilon.tools.certs import Certificate

  from ipsilon.tools import files

- from urllib import urlencode

  import argparse

- import ConfigParser

  import getpass

  import json

  import logging
@@ -17,6 +15,8 @@ 

  import pwd

  import requests

  import shutil

+ from six.moves import configparser

+ from six.moves.urllib.parse import urlencode

  import socket

  import sys

  import base64
@@ -77,7 +77,7 @@ 

          path = os.path.join(HTTPDIR % 'saml2', args['hostname'])

          if os.path.exists(path):

              raise Exception('Service Provider is already configured')

-         os.makedirs(path, 0750)

+         os.makedirs(path, 0o750)

      else:

          path = os.getcwd()

  
@@ -169,7 +169,9 @@ 

  

      if not args['saml_no_httpd']:

          idp_metafile = os.path.join(path, 'idp-metadata.xml')

-         with open(idp_metafile, 'w+') as f:

+         with open(idp_metafile, 'w+b') as f:

+             if not isinstance(idpmeta, bytes):

+                 idpmeta = idpmeta.encode('utf-8')

              f.write(idpmeta)

  

          saml_protect = 'auth'
@@ -327,7 +329,7 @@ 

  

  def saml2_verify_arguments(args):

      if args['saml_auth']:

-         logger.warn('--saml-auth is deprecated. Please use --auth-location')

+         logger.warning('--saml-auth is deprecated. Please use --auth-location')

          args['auth_location'] = args['saml_auth']

  

      # Validate that all path options begin with '/'
@@ -471,7 +473,7 @@ 

  

  

  def parse_config_profile(args):

-     config = ConfigParser.ConfigParser()

+     config = configparser.RawConfigParser()

      files = config.read(args['config_profile'])

      if len(files) == 0:

          raise ConfigurationError('Config Profile file %s not found!' %
@@ -590,6 +592,7 @@ 

              elif service_type == 'openidc':

                  openidc()

      except Exception as e:  # pylint: disable=broad-except

+         raise
simo commented 4 years ago

what's the point of this raise ?

          log_exception(e)

          if 'uninstall' in args and args['uninstall'] is True:

              logging.info('Uninstallation aborted.')

@@ -7,9 +7,9 @@ 

  

  import argparse

  import cherrypy

- from ConfigParser import RawConfigParser

  import logging

  import os

+ from six.moves.configparser import RawConfigParser

  import sys

  import unicodedata

  

@@ -11,7 +11,6 @@ 

  from ipsilon.authz.common import AuthzProviderInstall

  from ipsilon.util.data import UserStore

  from ipsilon.tools import files, dbupgrade

- import ConfigParser

  import argparse

  import cherrypy

  import json
@@ -20,6 +19,8 @@ 

  import pwd

  import shutil

  import socket

+ import six

+ from six.moves import configparser

  import subprocess

  import sys

  import time
@@ -35,10 +36,10 @@ 

  WSGI_SOCKET_PREFIX = None

  

  

- class ConfigurationError(StandardError):

+ class ConfigurationError(Exception):

  

      def __init__(self, message):

-         StandardError.__init__(self, message)

+         super(ConfigurationError, self).__init__(message)

  

  

  #Silence cherrypy logging to screen
@@ -94,7 +95,7 @@ 

      if os.path.exists(idp_conf):

          shutil.move(idp_conf, '%s.backup.%s' % (idp_conf, now))

      if not os.path.exists(instance_conf):

-         os.makedirs(instance_conf, 0700)

+         os.makedirs(instance_conf, 0o700)

      confopts = {'instance': args['instance'],

                  'instanceurl': args['instanceurl'],

                  'needs_mount': args.get('needs_mount'),
@@ -150,12 +151,12 @@ 

      if not os.path.exists(args['httpd_conf']):

          os.symlink(idp_conf, args['httpd_conf'])

      if not os.path.exists(args['public_data_dir']):

-         os.makedirs(args['public_data_dir'], 0755)

+         os.makedirs(args['public_data_dir'], 0o755)

      if not os.path.exists(args['wellknown_dir']):

-         os.makedirs(args['wellknown_dir'], 0755)

+         os.makedirs(args['wellknown_dir'], 0o755)

      sessdir = os.path.join(args['data_dir'], 'sessions')

      if not os.path.exists(sessdir):

-         os.makedirs(sessdir, 0700)

+         os.makedirs(sessdir, 0o700)

      data_conf = os.path.join(args['data_dir'], 'ipsilon.conf')

      if not os.path.exists(data_conf):

          os.symlink(ipsilon_conf, data_conf)
@@ -265,10 +266,10 @@ 

      data_dir = os.path.join(DATADIR, args['instance'])

  

      try:

-         tconf = ConfigParser.SafeConfigParser()

+         tconf = configparser.SafeConfigParser()

          tconf.read(os.path.join(instance_conf, 'ipsilon.conf'))

          cache_dir = tconf.get('global', 'cache_dir')

-     except (ConfigParser.NoOptionError, ConfigParser.NoSectionError):

+     except (configparser.NoOptionError, configparser.NoSectionError):

          cache_dir = None

      else:

          cache_dir = cache_dir.replace('"', '')
@@ -351,7 +352,7 @@ 

  

  

  def parse_config_profile(args):

-     config = ConfigParser.RawConfigParser()

+     config = configparser.RawConfigParser()

      files = config.read(args['config_profile'])

      if len(files) == 0:

          raise ConfigurationError('Config Profile file %s not found!' %
@@ -501,7 +502,7 @@ 

          opts = parse_args(fplugins)

  

          logger.debug('Installation arguments:')

-         for k in sorted(opts.iterkeys()):

+         for k in sorted(six.iterkeys(opts)):

Sorted returns a list, so the iterkeys here sounds redundant, plain sorted(opts) would work the same (faster maybe).

              logger.debug('%s: %s', k, opts[k])

  

          if not opts['root_instance'] and opts['instance'] == 'root':

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

  import atexit

  import string

  import cherrypy

+ import six

  from ipsilon import find_config

  from ipsilon.util.data import AdminStore

  from ipsilon.util import page
@@ -70,7 +71,11 @@ 

                             extensions=['jinja2.ext.autoescape'])

  

  transchars = string.punctuation.replace('-', '').replace('.', '')

- trans = string.maketrans(transchars, '_' * len(transchars))

+ if six.PY2:

+     trans = string.maketrans(transchars, '_' * len(transchars))

+ elif six.PY3:

This will probably break in Python 4. Why not just else?

+     trans = str.maketrans(transchars, '_' * len(transchars))

+ 

  if __name__ == "__main__":

      conf = {'global': {'server.socket_host': '0.0.0.0'},

              '/': {'tools.staticdir.root': os.getcwd(),

file modified
+3 -3
@@ -261,11 +261,11 @@ 

              lh.search_s(test_dn, ldap.SCOPE_BASE,

                          attrlist=['objectclasses'])

          except ldap.INSUFFICIENT_ACCESS:

-             logging.warn('Anonymous access not allowed, continuing')

+             logging.warning('Anonymous access not allowed, continuing')

          except ldap.UNWILLING_TO_PERFORM:  # probably minSSF issue

-             logging.warn('LDAP server unwilling to perform, expect issues')

+             logging.warning('LDAP server unwilling to perform, expect issues')

          except ldap.SERVER_DOWN:

-             logging.warn('LDAP server is down')

+             logging.warning('LDAP server is down')

          except ldap.NO_SUCH_OBJECT:

              logging.error('Base DN not found')

              return False

@@ -160,7 +160,11 @@ 

  

          # We base64 encode the trust_root when looking up consent data to

          # ensure the client ID is safe for the cherrypy url routing

-         consentdata = user.get_consent('openid', b64encode(request.trust_root))

+         trust_root = request.trust_root

+         if isinstance(trust_root, str):

+             trust_root = trust_root.encode('utf-8')

+         trust_root_b64 = b64encode(trust_root)

+         consentdata = user.get_consent('openid', trust_root_b64)

          if consentdata is not None:

              # Consent has already been granted

              self.debug('Consent already granted')
@@ -183,10 +187,9 @@ 

  

              # Store new consent

              consentdata = {

-                 'attributes': ad.keys()

+                 'attributes': list(ad.keys())

              }

-             user.grant_consent('openid', b64encode(request.trust_root),

-                                consentdata)

+             user.grant_consent('openid', trust_root_b64, consentdata)

  

              # all done we consent!

              return self._respond(self._response(request, us))

@@ -14,6 +14,7 @@ 

  from copy import deepcopy

  import logging

  import re

+ import six

  

  

  INVALID_IN_CLIENT_ID = r'[^a-zA-Z0-9\-\.]'
@@ -59,7 +60,7 @@ 

  

          conf = self.client.get_config_obj()

  

-         for name, option in conf.iteritems():

+         for name, option in six.iteritems(conf):

              if name in kwargs:

                  value = kwargs[name]

                  if isinstance(option, pconfig.List):
@@ -202,6 +203,8 @@ 

      unknown_client.exposed = True

  

      def __getattr__(self, attr):

+         if attr == '_cp_config':

+             raise AttributeError('No attribute %s' % attr)

          client = self.main.cfg.datastore.getClient(attr)

          if client is None:

              return self.unknown_client()

@@ -20,7 +20,7 @@ 

          response = {'error': error}

          if description:

              response['error_description'] = description

-         self._error_response = json.dumps(response)

+         self._error_response = json.dumps(response).encode('utf-8')

          cherrypy.response.headers.update({

              'Content-Type': 'application/json'

          })
@@ -137,6 +137,8 @@ 

                             'client authentication error')

  

          self._set_apistore_key('api_client_authenticated', True)

+         if isinstance(client_id, bytes):

+             client_id = client_id.decode('utf-8')

          self._set_apistore_key('api_client_id', client_id)

          self._set_apistore_key('api_client', client)

  
@@ -151,11 +153,11 @@ 

                  hdr = hdr[len('Basic '):]

                  try:

                      client_id, client_secret = \

-                         base64.b64decode(hdr).split(':', 1)

+                         base64.b64decode(hdr).split(b':', 1)

                  except Exception as e:  # pylint: disable=broad-except

                      self.error('Invalid request received: %s' % repr(e))

-                     self._respond_error('invalid_request',

-                                         'invalid auth header')

+                     return self._respond_error('invalid_request',

+                                                'invalid auth header')

                  self.debug('Client ID: %s' % client_id)

                  self._handle_client_authentication('client_secret_basic',

                                                     client_id,

@@ -21,7 +21,7 @@ 

  import requests

  import time

  import json

- import urllib

+ from six.moves.urllib.parse import urlencode

  

  URLROOT = 'openidc'

  
@@ -101,7 +101,7 @@ 

              else:

                  url += '&'

  

-             url += urllib.urlencode(contents)

+             url += urlencode(contents)

  

          if response_mode in ['query', 'fragment', 'none']:

              raise cherrypy.HTTPRedirect(url)
@@ -446,9 +446,9 @@ 

                  # We are guaranteed that we either have a sector_identifier_uri

                  # or that the hostpart of all redirect_uris are equal

                  domain = get_url_hostpart(client['redirect_uris'][0])

-             h.update(domain)

-             h.update(user.name)

-             h.update(self.cfg.idp_subject_salt)

+             h.update(domain.encode('utf-8'))

+             h.update(user.name.encode('utf-8'))

+             h.update(self.cfg.idp_subject_salt.encode('utf-8'))

              userinfo['sub'] = h.hexdigest()

  

          claims_userinfo = {}

@@ -9,7 +9,8 @@ 

  import json

  import time

  import requests

- from urlparse import urlparse

+ import six

+ from six.moves.urllib.parse import urlparse

  

  

  def get_url_hostpart(url):
@@ -45,10 +46,10 @@ 

              clt.generate_secret()

          except InvalidMetadata as ex:

              raise APIError(400, 'invalid_client_metadata',

-                            ex.message)

+                            str(ex))

          except InvalidRedirectURI as ex:

              raise APIError(400, 'invalid_redirect_uri',

-                            ex.message)

+                            str(ex))

          except pconfig.FieldValueError as ex:

              raise APIError(400, 'invalid_request',

                             'invalid field value for %s' % ex.field)
@@ -117,7 +118,7 @@ 

  

      def generate_public(self):

          metadata = {}

-         for option, value in self.get_config_obj().iteritems():

+         for option, value in six.iteritems(self.get_config_obj()):

              name = option.replace(' ', '_').lower()

              metadata[name] = value.get_value()

          return metadata

@@ -35,6 +35,8 @@ 

          data = {}

  

          for key in client:

+             if isinstance(client[key], bytes):

+                 client[key] = client[key].decode('utf-8')

              data[key] = json.dumps(client[key])

  

          client_id = self.new_unique_data('client', data)
@@ -99,6 +101,8 @@ 

          return clients

  

      def getClient(self, client_id):

+         if isinstance(client_id, bytes):

+             client_id = client_id.decode('utf-8')

          if client_id.startswith('D-'):

              # This is a dynamically registered client

              ctype = 'dynamic'
@@ -161,7 +165,7 @@ 

              return None

  

          if not return_expired and \

-                 datum['expires_at'] <= int(time.time()):

+                 int(datum['expires_at']) <= int(time.time()):

              return None

  

          if expected_type and expected_type != 'Refresh' and \

@@ -17,7 +17,8 @@ 

  import requests

  import logging

  import base64

- from urlparse import urlparse

+ import six

+ from six.moves.urllib.parse import urlparse

  

  

  class NewSPAdminPage(AdminPage):
@@ -60,7 +61,7 @@ 

              if ctype != 'multipart/form-data':

                  self.debug("Invalid form type (%s), trying to cope" % (

                             cherrypy.request.content_type,))

-             for key, value in kwargs.iteritems():

+             for key, value in six.iteritems(kwargs):

                  if key == 'name':

                      name = value

                  elif key == 'description':
@@ -170,7 +171,7 @@ 

  

          conf = self.sp.get_config_obj()

  

-         for name, option in conf.iteritems():

+         for name, option in six.iteritems(conf):

              if name in kwargs:

                  value = kwargs[name]

                  if isinstance(option, pconfig.List):
@@ -220,7 +221,7 @@ 

          if len(new_db_values) != 0:

              try:

                  # Validate user can make these changes

-                 for (key, value) in new_db_values.iteritems():

+                 for (key, value) in six.iteritems(new_db_values):

                      if key == 'Name':

                          if (not self.user.is_admin and

                                  self.user.name != self.sp.owner):
@@ -237,7 +238,7 @@ 

                              )

  

                  # Make changes in current config

-                 for name, option in conf.iteritems():

+                 for name, option in six.iteritems(conf):

                      if name not in new_db_values:

                          continue

                      value = new_db_values.get(name, False)

@@ -290,9 +290,9 @@ 

                      "idp nameid salt is not set in configuration"

                  )

              value = hashlib.sha512()

-             value.update(idpsalt)

-             value.update(login.remoteProviderId)

-             value.update(mappedattrs.get('_username'))

+             value.update(idpsalt.encode('utf-8'))

+             value.update(login.remoteProviderId.encode('utf-8'))

+             value.update(mappedattrs.get('_username').encode('utf-8'))

              nameid = '_' + value.hexdigest()

          elif nameidfmt == lasso.SAML2_NAME_IDENTIFIER_FORMAT_TRANSIENT:

              nameid = '_' + uuid.uuid4().hex
@@ -345,6 +345,8 @@ 

                      self.log('Ignoring None value for attribute %s' % key)

                      continue

                  self.debug('value %s' % value)

+                 if isinstance(value, bytes):

+                     value = value.decode('utf-8')

                  node = lasso.MiscTextNode.newWithString(value)

                  node.textChild = True

                  attrvalue = lasso.Saml2AttributeValue()

@@ -62,7 +62,7 @@ 

      if len(newsps) != 1:

          raise InvalidProviderMetadata("Metadata must contain one Provider")

  

-     spid = newsps.keys()[0]

+     spid = list(newsps.keys())[0]

      return spid

  

  
@@ -78,13 +78,13 @@ 

          data = self.cfg.get_data(name='id', value=provider_id)

          if len(data) != 1:

              raise InvalidProviderId('multiple matches')

-         idval = data.keys()[0]

+         idval = list(data.keys())[0]

          data = self.cfg.get_data(idval=idval)

          self._properties = data[idval]

          self._staging = dict()

          self.load_config()

          self.logout_mechs = []

-         xmldoc = etree.XML(str(data[idval]['metadata']))

+         xmldoc = etree.XML(data[idval]['metadata'].encode('utf-8'))

          logout = xmldoc.xpath('//md:EntityDescriptor'

                                '/md:SPSSODescriptor'

                                '/md:SingleLogoutService',
@@ -291,7 +291,7 @@ 

          data = self.cfg.get_data(name='id', value=self.provider_id)

          if len(data) != 1:

              raise InvalidProviderId('Could not find SP data')

-         idval = data.keys()[0]

+         idval = list(data.keys())[0]

          data = dict()

          data[idval] = self._staging

          self.cfg.save_data(data)
@@ -343,7 +343,7 @@ 

          data = self.cfg.get_data(name='id', value=self.provider_id)

          if len(data) != 1:

              raise InvalidProviderId('Could not find SP data')

-         idval = data.keys()[0]

+         idval = list(data.keys())[0]

          self.cfg.del_datum(idval)

  

      def normalize_username(self, username):
@@ -397,7 +397,7 @@ 

          data = self.cfg.get_data(name='id', value=spid)

          if len(data) != 1:

              raise InvalidProviderId("Internal Error")

-         idval = data.keys()[0]

+         idval = list(data.keys())[0]

          data = self.cfg.get_data(idval=idval)

          sp = data[idval]

          self.cfg.idp.add_provider(sp)

@@ -65,7 +65,7 @@ 

              data = idp.get_data(name='name', value=instance)

              if len(data) == 0:

                  return rest_error(404, 'Provider %s not found' % instance)

-             idval = data.keys()[0]

+             idval = list(data.keys())[0]

              data = idp.get_data(idval=idval)

          else:

              data = idp.get_data()

@@ -170,10 +170,15 @@ 

  

          session_ids = []

          for c in candidates:

-             key = c.keys()[0]

+             key = list(c.keys())[0]

              if c[key].get('provider_id') == provider_id:

                  samlsession = self._data_to_samlsession(key, c[key])

-                 session_ids.append(samlsession.session_id.encode('utf-8'))

+                 sesid = samlsession.session_id

+                 if not isinstance(sesid, str):

+                     # In py3, we already has an str instance, in py2 we need to

+                     # encode it

+                     sesid = sesid.encode('utf-8')

+                 session_ids.append(sesid)

  

          return tuple(session_ids)

  
@@ -239,7 +244,7 @@ 

  

          for mech in logout_mechs:

              for c in candidates:

-                 key = c.keys()[0]

+                 key = list(c.keys())[0]

                  if ((int(c[key].get('logoutstate', 0)) == LOGGED_IN) and

                          (mech in c[key].get('supported_logout_mechs'))):

                      samlsession = self._data_to_samlsession(key, c[key])
@@ -260,7 +265,7 @@ 

          #        it's the "right" one if multiple logouts are started

          #        at the same time from different SPs?

          for c in candidates:

-             key = c.keys()[0]

+             key = list(c.keys())[0]

              if int(c[key].get('logoutstate', 0)) == INIT_LOGOUT:

                  samlsession = self._data_to_samlsession(key, c[key])

                  return samlsession
@@ -277,7 +282,7 @@ 

  

          count = 0

          for c in candidates:

-             key = c.keys()[0]

+             key = list(c.keys())[0]

              samlsession = self._data_to_samlsession(key, c[key])

              self.debug('session %d: %s' % (count, samlsession.convert()))

              count += 1

@@ -12,7 +12,7 @@ 

  from ipsilon.tools.certs import Certificate

  from ipsilon.tools import saml2metadata as metadata

  from ipsilon.tools import files

- from ipsilon.util.http import require_content_type

+ from ipsilon.util.httputils import require_content_type

  from ipsilon.util.constants import SOAP_MEDIA_TYPE, XML_MEDIA_TYPE

  from ipsilon.util.user import UserSession

  from ipsilon.util.plugin import PluginObject

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

  

  from subprocess import Popen

  import os

- import string

  

  

  class Certificate(object):
@@ -49,4 +48,4 @@ 

          if cert[-1] == '-----END CERTIFICATE-----\n':

              cert = cert[:-1]

  

-         return string.join(cert)

+         return " ".join(cert)

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

  from ipsilon.tools.certs import Certificate

  from lxml import etree

  import lasso

+ import six

  

  

  SAML2_NAMEID_MAP = {
@@ -125,7 +126,7 @@ 

          svc = mdElement(self.role, service[0])

          svc.set('Binding', service[1])

          svc.set('Location', location)

-         for key, value in kwargs.iteritems():

+         for key, value in six.iteritems(kwargs):

              svc.set(key, value)

  

      def add_allowed_name_format(self, name_format):
@@ -138,7 +139,7 @@ 

          if path is None:

              return data

          else:

-             with open(path, 'w') as f:

+             with open(path, 'wb') as f:

                  f.write(data)

  

  

file modified
+3 -2
@@ -7,6 +7,7 @@ 

  import imghdr

  import hashlib

  import cherrypy

+ import six

  

  

  def name_from_image(image):
@@ -263,7 +264,7 @@ 

              return None

  

          if not os.path.exists(self.__filename()):

-             with open(self.__filename(), 'w') as imagefile:

+             with open(self.__filename(), 'wb') as imagefile:

                  imagefile.write(base64.b64decode(self._image))

  

  
@@ -511,7 +512,7 @@ 

          if not self._config:

              raise AttributeError('Config not initialized, cannot import')

  

-         for key, value in config.iteritems():

+         for key, value in six.iteritems(config):

              if key in self._config:

                  self._config[key].import_value(str(value))

  

file modified
+10 -8
@@ -9,7 +9,6 @@ 

  from sqlalchemy.schema import (PrimaryKeyConstraint, Index, AddConstraint,

                                 CreateIndex)

  from sqlalchemy.sql import select, and_

- import ConfigParser

  try:

      import etcd

  except ImportError:
@@ -17,7 +16,9 @@ 

  import os

  import json

  import uuid

- from urlparse import urlparse

+ from six import text_type, binary_type

+ from six.moves import configparser

+ from six.moves.urllib.parse import urlparse

  import logging

  import time

  
@@ -293,9 +294,10 @@ 

              raise

          timestamp = stat.st_mtime

          if self._config is None or timestamp > self._timestamp:

-             self._config = ConfigParser.RawConfigParser()

+             self._config = configparser.RawConfigParser()

              self._config.optionxform = str

              self._config.read(self._filename)

+             self._timestamp = timestamp

          return self._config

  

      def add_constraint(self, constraint):
@@ -1302,7 +1304,7 @@ 

              return None

          elif count != 1:

              raise ValueError("Multiple entries returned")

-         return data.keys()[0]

+         return list(data.keys())[0]

  

      def get_data(self, idval=None, name=None, value=None):

          return self.get_unique_data(self.table, idval, name, value)
@@ -1313,8 +1315,8 @@ 

                  datum['supported_logout_mechs']

              )

          for attr in datum:

-             if isinstance(datum[attr], str):

-                 datum[attr] = unicode(datum[attr], 'utf-8')

+             if isinstance(datum[attr], binary_type):

+                 datum[attr] = text_type(datum[attr], 'utf-8')

          return self.new_unique_data(self.table, datum, ttl)

  

      def get_session(self, session_id=None, request_id=None):
@@ -1347,8 +1349,8 @@ 

  

      def update_session(self, datum):

          for attr in datum:

-             if isinstance(datum[attr], str):

-                 datum[attr] = unicode(datum[attr], 'utf-8')

+             if isinstance(datum[attr],  binary_type):

+                 datum[attr] = text_type(datum[attr], 'utf-8')

          self.save_unique_data(self.table, datum)

  

      def remove_session(self, uuidval):

file modified
+4 -9
@@ -3,13 +3,8 @@ 

  import cherrypy

  from ipsilon.util.log import Log

  from ipsilon.util.user import UserSession

- from urllib import unquote

+ from six.moves.urllib.parse import unquote, urlparse

  from functools import wraps

- try:

-     from urlparse import urlparse

- except ImportError:

-     # pylint: disable=no-name-in-module, import-error

-     from urllib.parse import urlparse

  

  

  def allow_iframe(func):
@@ -84,13 +79,13 @@ 

                      self.debug("Wrong referer %s in request to %s"

                                 % (referer, url))

                      raise cherrypy.HTTPError(403)

-             return op(*args, **kwargs)

+             return op(*args, **kwargs).encode('utf-8')

          else:

              op = getattr(self, 'root', None)

              if callable(op):

-                 return op(*args, **kwargs)

+                 return op(*args, **kwargs).encode('utf-8')

  

-         return self.default(*args, **kwargs)

+         return self.default(*args, **kwargs).encode('utf-8')

  

      def default(self, *args, **kwargs):

          raise cherrypy.NotFound()

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

          output_page = self._template(*args, **kwargs)

          # for some reason cherrypy will choke if the output

          # is a unicode object, so use str() here to please it

-         return str(output_page)

+         return output_page.encode('utf-8')

  

      def handler(self, status, message, traceback, version):

          self.debug(repr([status, message, traceback, version]))

ipsilon/util/httputils.py ipsilon/util/http.py
file renamed
+2 -1
@@ -2,6 +2,7 @@ 

  

  import cherrypy

  import fnmatch

+ from six import string_types

  

  

  def require_content_type(required=None, absent_ok=True, debug=False):
@@ -36,7 +37,7 @@ 

      if required is None:

          return

  

-     if isinstance(required, basestring):

+     if isinstance(required, string_types):

          required = [required]

  

      content_type = cherrypy.request.body.content_type.value

file modified
+10 -7
@@ -1,11 +1,11 @@ 

  # Copyright (C) 2014 Ipsilon project Contributors, for license see COPYING

  

  import cherrypy

- import cStringIO

  import inspect

  import os

  import traceback

  import logging

+ from six import StringIO

  

  

  def log_request_response():
@@ -95,7 +95,7 @@ 

          original lines of text are indented.

          '''

  

-         f = cStringIO.StringIO()

+         f = StringIO()

  

          lines = text.split('\n')

  
@@ -120,7 +120,7 @@ 

          each part of the multipart into a Part object containing

          information about the part and it's content.

          '''

-         f = cStringIO.StringIO()

+         f = StringIO()

  

          f.write(indent_text('Name = %s\n' % part.name))

          if part.headers:
@@ -136,7 +136,7 @@ 

          return string

  

      def print_param(name, value):

-         f = cStringIO.StringIO()

+         f = StringIO()

  

          # Might be a multipart Part object, if so format it

          if isinstance(value, cherrypy._cpreqbody.Part):  # pylint:disable=W0212
@@ -176,7 +176,7 @@ 

          as the new body contents for remainder of the processing

          pipeline to act upon (i.e. sent to the client)

          '''

-         f = cStringIO.StringIO()

+         f = StringIO()

  

          for chunk in body:

              f.write(chunk)
@@ -187,7 +187,7 @@ 

  

      # --- End local functions ---

  

-     f = cStringIO.StringIO()

+     f = StringIO()

      request = cherrypy.serving.request

      remote = request.remote

  
@@ -261,7 +261,7 @@ 

  

      @staticmethod

      def stacktrace():

-         buf = cStringIO.StringIO()

+         buf = StringIO()

  

          stack = traceback.extract_stack()

          traceback.print_list(stack[:-2], file=buf)
@@ -285,6 +285,9 @@ 

          the instance parameter of a method self, then you could do this.

          '''

  

+         # Per the python3 docs on getargvalues():

+         # "This function was inadvertently marked as deprecated in Python 3.5."

+         # pylint: disable=deprecated-method

          args, _, _, value_dict = inspect.getargvalues(frame_obj)

          # Is the functions first parameter named 'self'?

          if len(args) and args[0] == 'self':

file modified
+1 -8
@@ -4,14 +4,7 @@ 

  from ipsilon.util.endpoint import Endpoint

  from ipsilon.util.user import UserSession

  from ipsilon.util.trans import Transaction

- from urllib import unquote

- try:

-     from urlparse import urlparse

-     from urlparse import parse_qs

- except ImportError:

-     # pylint: disable=no-name-in-module, import-error

-     from urllib.parse import urlparse

-     from urllib.parse import parse_qs

+ from six.moves.urllib.parse import unquote, urlparse, parse_qs

  

  

  def admin_protect(fn):

file modified
+7 -2
@@ -3,11 +3,16 @@ 

  import base64

  from cryptography.hazmat.primitives.constant_time import bytes_eq

  import os

+ from six import binary_type

  

  

  def generate_random_secure_string(size=32):

-     return base64.urlsafe_b64encode(os.urandom(size))[:size]

+     return base64.urlsafe_b64encode(os.urandom(size))[:size].decode('utf-8')

  

  

  def constant_time_string_comparison(stra, strb):

-     return bytes_eq(str(stra), str(strb))

+     if not isinstance(stra, binary_type):

+         stra = stra.encode('utf-8')

+     if not isinstance(strb, binary_type):

+         strb = strb.encode('utf-8')

+     return bytes_eq(stra, strb)

file modified
+5 -3
@@ -80,7 +80,7 @@ 

              result = q.select({'id': self.id})

              r = result.fetchone()

              if r:

-                 data = str(base64.b64decode(r[1]))

+                 data = base64.b64decode(r[1]).decode('utf-8')

                  if not data.startswith('['):

                      # This is a pre-upgrade pickle'd session. Just invalidate.

                      self._delete()
@@ -94,8 +94,10 @@ 

          q = SqlQuery(self._db, 'sessions', SESSION_TABLE, trans=True)

          with q:

              q.delete({'id': self.id})

-             data = json.dumps((self._data, expiration_time))

-             q.insert((self.id, base64.b64encode(data), expiration_time))

+             data = json.dumps((self._data, expiration_time)).encode('utf-8')

+             q.insert((self.id,

+                       base64.b64encode(data).decode('utf-8'),

+                       expiration_time))

  

      def _delete(self):

          q = SqlQuery(self._db, 'sessions', SESSION_TABLE)

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

  from ipsilon.util.endpoint import allow_iframe

  

  import json

+ from six import string_types

  

  

  class WebFinger(Page, Log):
@@ -34,7 +35,7 @@ 

  

          if 'rel' in kwargs:

              rels = kwargs['rel']

-             if isinstance(rels, basestring):

+             if isinstance(rels, string_types):

                  rels = [rels]

          else:

              rels = self.supported_rels.keys()

file modified
+29 -1
@@ -1,3 +1,31 @@ 

  #!/bin/sh

+ pyver=""

+ if [ "$1" == "python2" -o "$1" == "python3" ];

+ then

+ 	pyver="$1"

+ 	shift

+ fi

+ 

+ if [ "$pyver" == "" ];

+ then

+ 	echo "Determining Python version"

+ 	if which python >/dev/null 2>&1;

+ 	then

+ 		echo "Using Python"

+ 		pyver="python"

+ 	elif which python2 >/dev/null 2>&1;

+ 	then

+ 		echo "Using Python2"

+ 		pyver="python2"

+ 	elif which python3 >/dev/null 2>&1;

+ 	then

+ 		echo "Using Python3"

+ 		pyver="python3"

+ 	else

+ 		echo "Unable to find a usable python interpreter"

+ 		exit 1

+ 	fi

+ fi

+ 

  export PYTHONPATH=.

- exec python ./tests/tests.py "$@"

+ exec $pyver ./tests/tests.py "$@"

@@ -3,4 +3,4 @@ 

  RUN useradd testuser \

      && chown testuser:testuser /code

  WORKDIR /code

- ENTRYPOINT ["/usr/bin/make", "testdeps", "test"]

+ ENTRYPOINT ["/usr/bin/make", "test"]

@@ -1,1 +1,1 @@ 

- RUN yum install -y etcd python2-python-etcd dbus-python python2-ipalib bandit

+ RUN yum install -y etcd python3-python-etcd dbus-python python3-ipalib bandit

@@ -1,13 +1,8 @@ 

  RUN yum update -y \

      && yum install -y which

  # This should be kept in sync with the develop page on the website.

- # Distro-specific packages should go in the distro sub-dockerfiles.

- RUN yum install -y make python2-pylint python-pep8 python-openid python-openid-teams \

-         python-openid-cla python-cherrypy m2crypto lasso-python \

-         python-sqlalchemy python-ldap python-pam python-fedora \

-         freeipa-python httpd mod_auth_mellon postgresql-server \

-         openssl mod_wsgi python-jinja2 python-psycopg2 sssd \

-         libsss_simpleifp openldap-servers mod_auth_gssapi krb5-server \

-         socket_wrapper nss_wrapper python-requests-kerberos python-lesscpy\

-         nodejs-less krb5-workstation python-sssdconfig sqlite python-jwcrypto \

-         mod_ssl mod_auth_openidc python-jwcrypto

+ # Distro-specific and python packages should go in the distro sub-dockerfiles.

+ RUN yum install -y make httpd mod_auth_mellon postgresql-server \

+         openssl sssd libsss_simpleifp openldap-servers mod_auth_gssapi \

+ 	krb5-server socket_wrapper nss_wrapper nodejs-less krb5-workstation \

+ 	sqlite mod_ssl mod_auth_openidc

@@ -0,0 +1,6 @@ 

+ RUN yum install -y python2-pylint python-pep8 python-openid python-openid-teams \

+         python-openid-cla python-cherrypy m2crypto lasso-python \

+         python-sqlalchemy python-ldap python-pam python-fedora \

+         mod_wsgi python-jinja2 python-psycopg2 \

+         python-requests-kerberos python-lesscpy python-sssdconfig \

+ 	python-jwcrypto python-jwcrypto python-six

@@ -0,0 +1,10 @@ 

+ RUN yum install -y python3-pylint python3-pep8 python3-openid python3-openid-teams \

+         python3-openid-cla python3-cherrypy python3-m2crypto python3-lasso \

+         python3-sqlalchemy python3-ldap python3-pam python3-fedora \

+         python3-ipalib python3-mod_wsgi python3-jinja2 python3-psycopg2 \

+         python3-requests-kerberos python3-lesscpy python3-sssdconfig \

+ 	python3-jwcrypto python3-jwcrypto python3-six

+ RUN yum erase -y python2

+ RUN yum install -y make

+ # There's a double-free in the f29 version

+ RUN yum downgrade -y https://dl.fedoraproject.org/pub/fedora/linux/releases/28/Everything/x86_64/os/Packages/m/mod_auth_openidc-1.8.10.1-7.fc28.x86_64.rpm

file modified
+7 -6
@@ -108,7 +108,8 @@ 

                                            stderr=self.stderr)

  

              # Upgrade that database

-             cmd = [os.path.join(self.rootdir,

+             cmd = [self.pycmd,

+                    os.path.join(self.rootdir,

                                  'ipsilon/install/ipsilon-upgrade-database'),

                     cfgfile]

              subprocess.check_call(cmd,
@@ -121,16 +122,16 @@ 

              # Check all features in a newly created database

              # Let's verify if at least one index was created

              output = self.dump_db(db_outdir, with_readonly)

-             if 'CREATE INDEX' not in output:

+             if b'CREATE INDEX' not in output:

                  raise Exception('Database upgrade did not introduce index')

-             if 'PRIMARY KEY' not in output:

+             if b'PRIMARY KEY' not in output:

                  raise Exception('Database upgrade did not introduce primary ' +

                                  'key')

          elif old_version == 1:

              # In 1 -> 2, we added indexes and primary keys

              # Let's verify if at least one index was created

              output = self.dump_db(db_outdir, with_readonly)

-             if 'CREATE INDEX' not in output:

+             if b'CREATE INDEX' not in output:

                  raise Exception('Database upgrade did not introduce index')

              # SQLite did not support creating primary keys, so we can't test

  
@@ -138,7 +139,7 @@ 

              # Version 3 added the authz_config table

              # Make sure it exists

              output = self.dump_db(db_outdir, with_readonly)

-             if 'TABLE authz_config' not in output:

+             if b'TABLE authz_config' not in output:

                  raise Exception('Database upgrade did not introduce ' +

                                  'authz_config table')

  
@@ -149,7 +150,7 @@ 

          exe = self.execname

          if exe.endswith('c'):

              exe = exe[:-1]

-         exe = [exe]

+         exe = [self.pycmd, exe]

          exe.append(str(old_version))

          if with_readonly:

              exe.append('readonly')

file modified
+4 -3
@@ -5,7 +5,7 @@ 

  from helpers.common import IpsilonTestBase  # pylint: disable=relative-import

  from helpers.control import TC  # pylint: disable=relative-import

  from helpers.http import HttpSessions  # pylint: disable=relative-import

- import ConfigParser

+ from six.moves import configparser

  import os

  import pwd

  from string import Template
@@ -122,7 +122,7 @@ 

          f.write(text)

  

      ipsilonconf = os.path.join(testdir, 'etc', idpname, 'ipsilon.conf')

-     newconf = ConfigParser.ConfigParser()

+     newconf = configparser.ConfigParser()

      with open(ipsilonconf, 'r') as f:

          newconf.readfp(f)

      with open(ipsilonconf, 'w+') as f:
@@ -152,7 +152,8 @@ 

  

          self.setup_step("Testing database upgrade")

          cfgfile = os.path.join(self.testdir, 'etc', idpname, 'ipsilon.conf')

-         cmd = [os.path.join(self.rootdir,

+         cmd = [self.pycmd,

+                os.path.join(self.rootdir,

                              'ipsilon/install/ipsilon-upgrade-database'),

                 cfgfile]

          subprocess.check_call(cmd,

file modified
+25 -10
@@ -2,17 +2,18 @@ 

  #

  # Copyright (C) 2014 Ipsilon project Contributors, for license see COPYING

  

- import ConfigParser

+ from six.moves import configparser

  import io

  import os

  import pwd

  import shutil

  import signal

+ import six

  import random

  from string import Template

  import subprocess

  

- from control import TC  # pylint: disable=relative-import

+ from .control import TC

  

  

  WRAP_HOSTNAME = 'idp.ipsilon.dev'
@@ -78,6 +79,7 @@ 

      def __init__(self, name, execname, allow_wrappers=True):

          self.name = name

          self.execname = execname

+         self.py3 = False

          self.rootdir = os.getcwd()

          self.testdir = None

          self.testuser = pwd.getpwuid(os.getuid())[0]
@@ -88,6 +90,13 @@ 

          self.stdout = None

          self.stderr = None

  

+     def set_py3(self, use_py3):

+         self.py3 = use_py3

+ 

+     @property

+     def pycmd(self):

+         return 'python3' if self.py3 else 'python'

+ 

      def platform_supported(self):

          """This return whether the current platform supports this test.

  
@@ -102,7 +111,7 @@ 

          return None

  

      def force_remove(self, op, name, info):

-         os.chmod(name, 0700)

+         os.chmod(name, 0o700)

          os.remove(name)

  

      def setup_base(self, path, test):
@@ -157,7 +166,7 @@ 

                           nameid='unspecified'):

          args_opts['port'] = port

  

-         newconf = ConfigParser.ConfigParser()

+         newconf = configparser.RawConfigParser()

          newconf.add_section('globals')

          for k in global_opts:

              newconf.set('globals', k, global_opts[k])
@@ -165,7 +174,10 @@ 

          for k in args_opts:

              newconf.set('arguments', k, args_opts[k])

  

-         profile = io.BytesIO()

+         if six.PY2:

+             profile = io.BytesIO()

+         elif six.PY3:

+             profile = io.StringIO()

          newconf.write(profile)

  

          t = Template(profile.getvalue())
@@ -177,7 +189,7 @@ 

                               'TEST_USER': self.testuser})

  

          filename = os.path.join(self.testdir, '%s_profile.cfg' % name)

-         with open(filename, 'wb') as f:

+         with open(filename, 'w') as f:

              f.write(text)

  

          return filename
@@ -197,7 +209,8 @@ 

                                   'HTTPPORT': port,

                                   'NAME': name,

                                   'CERTROOT': os.path.join(self.testdir,

-                                                           'certs')})

+                                                           'certs'),

+                                  'PYTHON3': '_python3' if self.py3 else ''})

          filename = os.path.join(httpdir, 'httpd.conf')

          with open(filename, 'w+') as f:

              f.write(text)
@@ -243,7 +256,8 @@ 

              env['LOGFILE'] = logfile

          else:

              env = {'LOGFILE': logfile}

-         cmd = [os.path.join(self.rootdir,

+         cmd = [self.pycmd,

+                os.path.join(self.rootdir,

                              'ipsilon/install/ipsilon-server-install'),

                 '--config-profile=%s' % profile]

          subprocess.check_call(cmd, env=env,
@@ -255,7 +269,8 @@ 

  

      def setup_sp_server(self, profile, name, addr, port, env):

          http_conf_file = self.setup_http(name, addr, port)

-         cmd = [os.path.join(self.rootdir,

+         cmd = [self.pycmd,

+                os.path.join(self.rootdir,

                              'ipsilon/install/ipsilon-client-install'),

                 '--config-profile=%s' % profile]

          subprocess.check_call(cmd, env=env,
@@ -462,7 +477,7 @@ 

          exe = self.execname

          if exe.endswith('c'):

              exe = exe[:-1]

-         return self.run_and_collect([exe], env)

+         return self.run_and_collect([self.pycmd, exe], env)

  

      def run_and_collect(self, cmd, env):

          p = subprocess.Popen(cmd, env=env,

file modified
+2 -2
@@ -80,8 +80,8 @@ 

          case done:  ('done',)

          case fail:  ('fail', 'some error')

          """

-         if line.startswith(TC.prefix):

-             return tuple(line[len(TC.prefix):].split(':'))

+         if line.startswith(TC.prefix.encode('utf-8')):

+             return tuple(line[len(TC.prefix):].split(b':'))

          else:

              return None

  

file modified
+3 -4
@@ -5,9 +5,8 @@ 

  from lxml import html

  import requests

  import string

- import urlparse

+ from six.moves.urllib.parse import urlparse, urlencode

  import json

- from urllib import urlencode

  from requests_kerberos import HTTPKerberosAuth, OPTIONAL

  

  
@@ -103,7 +102,7 @@ 

          return session.post(url, allow_redirects=False, **kwargs)

  

      def access(self, action, url, krb=False, **kwargs):

-         action = string.lower(action)

+         action = action.lower()

          if action == 'get':

              return self.get(url, krb, **kwargs)

          elif action == 'post':
@@ -113,7 +112,7 @@ 

  

      def new_url(self, referer, action):

          if action.startswith('/'):

-             u = urlparse.urlparse(referer)

+             u = urlparse(referer)

              return '%s://%s%s' % (u.scheme, u.netloc, action)

          return action

  

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

  LoadModule alias_module modules/mod_alias.so

  LoadModule rewrite_module modules/mod_rewrite.so

  LoadModule version_module modules/mod_version.so

- LoadModule wsgi_module modules/mod_wsgi.so

+ LoadModule wsgi_module modules/mod_wsgi${PYTHON3}.so

  <IfVersion >= 2.4>

      # openidc needs to be before mellon: https://bugzilla.redhat.com/show_bug.cgi?id=1332729

      LoadModule auth_openidc_module modules/mod_auth_openidc.so

file modified
+12 -12
@@ -252,9 +252,9 @@ 

          page = sess.fetch_page(idpname, 'https://127.0.0.11:45081/sp/',

                                 require_consent=True)

          h = hashlib.sha256()

-         h.update('127.0.0.11')

-         h.update(user)

-         h.update('testcase')

+         h.update(b'127.0.0.11')

+         h.update(user.encode('utf-8'))

+         h.update(b'testcase')

          expect = {

              'sub': h.hexdigest(),

              'iss': 'https://127.0.0.10:45080/idp1/openidc/',
@@ -278,9 +278,9 @@ 

                                 'out=https%3A%2F%2F127.0.0.11%3A45081%2Fsp%2F',

                                 require_consent=True)

          h = hashlib.sha256()

-         h.update('127.0.0.11')

-         h.update(user)

-         h.update('testcase')

+         h.update(b'127.0.0.11')

+         h.update(user.encode('utf-8'))

+         h.update(b'testcase')

          expect = {

              'sub': h.hexdigest(),

              'iss': 'https://127.0.0.10:45080/idp1/openidc/',
@@ -443,9 +443,9 @@ 

          if 'sub' not in info:

              raise Exception('No sub claim provided')

          h = hashlib.sha256()

-         h.update('127.0.0.11')

-         h.update(user)

-         h.update('testcase')

+         h.update(b'127.0.0.11')

+         h.update(user.encode('utf-8'))

+         h.update(b'testcase')

          if info['sub'] != h.hexdigest():

              raise Exception('Sub claim invalid')

  
@@ -462,9 +462,9 @@ 

      with TC.case('Access third SP Protected Area'):

          page = sess.fetch_page(idpname, 'https://127.0.0.13:45083/sp/')

          h = hashlib.sha256()

-         h.update('127.0.0.13')

-         h.update(user)

-         h.update('testcase')

+         h.update(b'127.0.0.13')

+         h.update(user.encode('utf-8'))

+         h.update(b'testcase')

          expect = {

              'sub': h.hexdigest(),

              'iss': 'https://127.0.0.10:45080/idp1/openidc/',

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

  from helpers.http import HttpSessions  # pylint: disable=relative-import

  import os

  import pwd

+ from six.moves import xrange

  from string import Template

  

  

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

          if not p.wait() == 0:

              return 'No etcd installed'

          # Example line: etcd Version: 3.0.13

-         if int(stdout.split('\n')[0].split(': ')[1][0]) < 3:

+         if int(stdout.split(b'\n')[0].split(b': ')[1][0]) < 3:

              return 'Etcd version < 3.0'

          try:

              import etcd  # pylint: disable=unused-variable,import-error

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

  import os

  import pwd

  import re

+ from six.moves import xrange

  from string import Template

  

  

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

  from ipsilon.util import plugin

  import os

  import sys

+ import six

  import subprocess

  from helpers.common import WRAP_HOSTNAME  # pylint: disable=relative-import

  from helpers.control import TC  # pylint: disable=relative-import
@@ -43,6 +44,8 @@ 

                          help='Test results header')

      parser.add_argument('--path', default='%s/testdir' % os.getcwd(),

                          help="Directory in which tests are run")

+     parser.add_argument('--python-3', '-3', action='store_true',

+                         help='Use python 3')

      parser.add_argument('--fail-on-first-error', '-x', action='store_true',

                          help='Abort test run on first test failure')

      parser.add_argument('--test', action='append', default=None,
@@ -51,7 +54,7 @@ 

                          help='List all available tests')

      parser.add_argument('--no-overview', '-q', action='store_true',

                          help='Suppress final summary')

-     parser.add_argument('--verbose', '-v', action='count',

+     parser.add_argument('--verbose', '-v', action='count', default=0,

                          help='Increase verbosity')

      parser.add_argument('--wrappers', default='auto',

                          choices=['yes', 'no', 'auto'],
@@ -98,6 +101,8 @@ 

  

  

  def run_test(testname, test, args):

+     if args['python_3']:

+         test.set_py3(True)

      supported = test.platform_supported()

      if supported is not None:

          return (TEST_RESULT_SKIP, supported)
@@ -126,6 +131,7 @@ 

          if code:

              return (TEST_RESULT_FAIL, code, results)

      except Exception as e:  # pylint: disable=broad-except

+         raise

          if post_setup:

              return (TEST_RESULT_EXCEPTION, e, results)

          else:
@@ -158,6 +164,10 @@ 

  def main():

      args = parse_args()

  

+     if six.PY3 and not args['python_3']:

+         print("Forcing Python3 tests")

+         args['python_3'] = True

+ 

      tests = get_tests()

      if args['list_tests']:

          for testname in tests.keys():

With this patch set, the code will all be python3 compatible, and the container tests for Fedora 29 will run on an a system with python2 removed entirely.
Fedora 28 and CentOS will still be using Python2.

Note that this is based on top of #312.

rebased onto 866f7b6

5 years ago

Can't we also revert a4abfb2 now, since it'll be able to use Python 3 where FreeIPA is only offers Python 3?

Technically, sure.
But if the CLI gives us what we need, I don't think it gains us anything useful.

pycodestyle-3 (from python3-pycodestyle) is the actively maintained fork of pep8.

Aside from the pep8/pycodestyle thing, this looks good to me, though I'm concerned about the lack of test results returned from Jenkins. Doesn't this have CI?

pretty please pagure-ci rebuild

5 years ago

pretty please pagure-ci rebuild

5 years ago

@abbra unfortunately, it looks like there's no ci.centos.org pipeline wired up anymore, so it needs to be reviewed and tested locally.

Sorted returns a list, so the iterkeys here sounds redundant, plain sorted(opts) would work the same (faster maybe).

This will probably break in Python 4. Why not just else?

I generally see a lot of six.iteritems() and friends for small dicts (such as kwargs). Maybe this is overkill? If you just use .items() you get the proper behavior on Python 3 and the slightly worse behavior on Python 2 without noticing any difference. But the code will be more readable.

The omnipresent conditional decoding/encoding also feels like the str/bytes distinction wasn't given much thoughts. I'd consider imagining the data flow and agree whether you want to work with sequence of bytes or text and only do encoding/decoding at boundary level. However if you are confident in your tests, trying to pretend they are interchangeable (sticking with the wrong Python 2 paradigm) will work as well. Sorry if this sounds pedantic, I was bitten by this before, I merely try to share the experience.

Excellent read: https://portingguide.readthedocs.io/en/latest/strings.html

How can we move this forward?

Hi Miro, I will try to take a look soon, the patchset is huge ...

@simo Have you had a chance to look over this? I'm starting to work on shipping Ipsilon with this patchset integrated for openSUSE because we're in Python 3 only land there...

Sorry, not yet, working vacation happened in the middle.

Except for the extraneous raise all look good to me.
@puiterwijk if you can fix it I think we can merge

@puiterwijk do you want me to take over and fixe the raise as a follow commit ?

@simo Can you please just do it? I spoke to @puiterwijk at Flock about it and he wants to get this merged and a new release of Ipsilon with Python 3 support out the door.

I also will be contributing a spec for building it for SUSE distributions...

Ok, I am going to merge and then follow up with any changes as needed

Pull-Request has been merged by simo

4 years ago
Metadata
Changes Summary 50
+11 -12
file changed
Makefile
+8 -7
file changed
ipsilon/admin/common.py
+2 -1
file changed
ipsilon/info/infoldap.py
+2 -3
file changed
ipsilon/info/infonss.py
+9 -6
file changed
ipsilon/install/ipsilon-client-install
+1 -1
file changed
ipsilon/install/ipsilon-db2conf
+12 -11
file changed
ipsilon/install/ipsilon-server-install
+6 -1
file changed
ipsilon/ipsilon
+3 -3
file changed
ipsilon/login/authldap.py
+7 -4
file changed
ipsilon/providers/openid/auth.py
+4 -1
file changed
ipsilon/providers/openidc/admin.py
+6 -4
file changed
ipsilon/providers/openidc/api.py
+5 -5
file changed
ipsilon/providers/openidc/auth.py
+5 -4
file changed
ipsilon/providers/openidc/provider.py
+5 -1
file changed
ipsilon/providers/openidc/store.py
+6 -5
file changed
ipsilon/providers/saml2/admin.py
+5 -3
file changed
ipsilon/providers/saml2/auth.py
+6 -6
file changed
ipsilon/providers/saml2/provider.py
+1 -1
file changed
ipsilon/providers/saml2/rest.py
+10 -5
file changed
ipsilon/providers/saml2/sessions.py
+1 -1
file changed
ipsilon/providers/saml2idp.py
+1 -2
file changed
ipsilon/tools/certs.py
+3 -2
file changed
ipsilon/tools/saml2metadata.py
+3 -2
file changed
ipsilon/util/config.py
+10 -8
file changed
ipsilon/util/data.py
+4 -9
file changed
ipsilon/util/endpoint.py
+1 -1
file changed
ipsilon/util/errors.py
+2 -1
file renamed
ipsilon/util/http.py
ipsilon/util/httputils.py
+10 -7
file changed
ipsilon/util/log.py
+1 -8
file changed
ipsilon/util/page.py
+7 -2
file changed
ipsilon/util/security.py
+5 -3
file changed
ipsilon/util/sessions.py
+2 -1
file changed
ipsilon/util/webfinger.py
+29 -1
file changed
runtests
+1 -1
file changed
tests/containers/Dockerfile-base
+1 -1
file changed
tests/containers/Dockerfile-fedora
+5 -10
file changed
tests/containers/Dockerfile-rpm
+6
file added
tests/containers/Dockerfile-rpm-py2
+10
file added
tests/containers/Dockerfile-rpm-py3
+7 -6
file changed
tests/dbupgrades.py
+4 -3
file changed
tests/fconf.py
+25 -10
file changed
tests/helpers/common.py
+2 -2
file changed
tests/helpers/control.py
+3 -4
file changed
tests/helpers/http.py
+1 -1
file changed
tests/httpd.conf
+12 -12
file changed
tests/openidc.py
+1 -0
file changed
tests/pgdb.py
+1 -1
file changed
tests/testetcd.py
+1 -0
file changed
tests/testnameid.py
+11 -1
file changed
tests/tests.py