From 535e8bf3ce4886046c4af4763e7855e9860a1f44 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Oct 05 2016 12:31:55 +0000 Subject: [PATCH 1/6] Throw new FieldError to indicate which option field is problematic Signed-off-by: Patrick Uiterwijk Reviewed-by: Pierre-Yves Chibon Reviewed-by: Howard Johnson --- diff --git a/ipsilon/util/config.py b/ipsilon/util/config.py index 3607e79..f8f2864 100644 --- a/ipsilon/util/config.py +++ b/ipsilon/util/config.py @@ -30,6 +30,16 @@ def url_from_image(image): ) +class FieldValueError(ValueError): + + def __init__(self, field, *args): + super(FieldValueError, self).__init__(*args) + self.field = field + + def __str__(self): + return ValueError.__str__(self) + ', field: %s' % self.field + + class Config(Log): def __init__(self, name, *args): @@ -38,7 +48,8 @@ class Config(Log): self._dict = dict() for item in args: if not isinstance(item, Option): - raise ValueError('Invalid option type for %s' % repr(item)) + raise FieldValueError(self.name, 'Invalid option type for %s' + % repr(item)) self._list.append(item.name) self._dict[item.name] = item self.debug('Config(%s) %s' % (self.name, self._dict)) @@ -57,7 +68,7 @@ class Config(Log): def __setitem__(self, key, value): if not isinstance(value, Option): - raise ValueError('Invalid type for %s' % value) + raise FieldValueError(self.name, 'Invalid type for %s' % value) if key != value.name: raise NameError('Name mismatch, key=%s but value.name=%s' % ( key, value.name)) @@ -139,7 +150,7 @@ class Option(Log): def _str_import_value(self, value): if not isinstance(value, str): - raise ValueError('Value must be string') + raise FieldValueError(self.name, 'Value must be string') self._assigned_value = value def is_readonly(self): @@ -248,7 +259,8 @@ class Template(Option): def templatize(self, args): if not args: - raise ValueError('Templatized called w/o arguments') + raise FieldValueError(self.name, + 'Templatized called w/o arguments') return self.get_value() % args @@ -278,7 +290,8 @@ class List(Option): def import_value(self, value): if not isinstance(value, str): - raise ValueError('Value (type: %s) must be string' % type(value)) + raise FieldValueError(self.name, 'Value (type: %s) must be string' + % type(value)) self._assigned_value = [x.strip() for x in value.split(',')] @@ -288,8 +301,9 @@ class ComplexList(List): if value is None: return if not isinstance(value, list): - raise ValueError('The value type must be a list, not "%s"' % - type(value)) + raise FieldValueError(self.name, + 'The value type must be a list, not "%s"' % + type(value)) def set_value(self, value): self._check_value(value) @@ -302,8 +316,9 @@ class ComplexList(List): def import_value(self, value): if not isinstance(value, str): - raise ValueError('The value type must be a string, not "%s"' % - type(value)) + raise FieldValueError(self.name, + 'The value type must be a string, not "%s"' % + type(value)) jsonval = json.loads(value) self.set_value(jsonval) @@ -314,19 +329,24 @@ class MappingList(ComplexList): if value is None: return if not isinstance(value, list): - raise ValueError('The value type must be a list, not "%s"' % - type(value)) + raise FieldValueError(self.name, + 'The value type must be a list, not "%s"' % + type(value)) for v in value: if not isinstance(v, list): - raise ValueError('Each element must be a list, not "%s"' % - type(v)) + raise FieldValueError(self.name, + 'Each element must be a list, not "%s"' % + type(v)) if len(v) != 2: - raise ValueError('Each element must contain 2 values,' - ' not %d' % len(v)) + raise FieldValueError(self.name, + 'Each element must contain 2 values,' + ' not %d' % len(v)) def import_value(self, value): if not isinstance(value, str): - raise ValueError('Value (type: %s) must be string' % type(value)) + raise FieldValueError(self.name, + 'Value (type: %s) must be string' + % type(value)) jsonval = json.loads(value) self.set_value(jsonval) @@ -345,7 +365,8 @@ class Choice(Option): default = [] for name in default: if name not in self._allowed_values: - raise ValueError( + raise FieldValueError( + self.name, 'item [%s] is not in allowed [%s]' % (name, allowed)) self._default_value.append(name) @@ -366,7 +387,8 @@ class Choice(Option): self._assigned_value = list() for val in value: if val not in self._allowed_values: - raise ValueError( + raise FieldValueError( + self.name, 'Value "%s" not allowed [%s]' % (val, self._allowed_values)) self._assigned_value.append(val) @@ -408,12 +430,15 @@ class Pick(Option): super(Pick, self).__init__(name, description, readonly=readonly) self._allowed_values = list(allowed) if default_value not in self._allowed_values: - raise ValueError('The default value is not in the allowed list') + raise FieldValueError( + self.name, + 'The default value is not in the allowed list') self._default_value = default_value def set_value(self, value): if value not in self._allowed_values: - raise ValueError( + raise FieldValueError( + self.name, 'Value "%s" not allowed [%s]' % (value, self._allowed_values)) self._assigned_value = value From 1713de9fe1670ade14dc736211c2d5515b6b2c9f Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Oct 05 2016 12:31:57 +0000 Subject: [PATCH 2/6] Add Integer configuration field type Signed-off-by: Patrick Uiterwijk Reviewed-by: Pierre-Yves Chibon Reviewed-by: Howard Johnson --- diff --git a/ipsilon/util/config.py b/ipsilon/util/config.py index f8f2864..9e21d19 100644 --- a/ipsilon/util/config.py +++ b/ipsilon/util/config.py @@ -175,6 +175,26 @@ class String(Option): self._str_import_value(value) +class Integer(String): + + def __init__(self, name, description, default_value=None, readonly=False): + super(Integer, self).__init__(name, description, readonly=readonly) + self._default_value = int(default_value) + + def _check_value(self, value): + if not value: + return + try: + int(value) + except ValueError: + raise FieldValueError(self.name, 'The value must be an integer') + + def set_value(self, value): + self._check_value(value) + if value: + self._assigned_value = int(value) + + class Image(Option): """ An image has two components: the binary blob of the image itself and diff --git a/templates/admin/option_config.html b/templates/admin/option_config.html index 7c7c52b..3e0c779 100644 --- a/templates/admin/option_config.html +++ b/templates/admin/option_config.html @@ -78,7 +78,7 @@ {% set value = v.get_value() -%} {% if v.__class__.__name__ == 'String' and v.multiline -%} - {% elif v.__class__.__name__ in ['String', 'Template'] -%} + {% elif v.__class__.__name__ in ['String', 'Template', 'Integer'] -%} Date: Oct 05 2016 12:31:59 +0000 Subject: [PATCH 3/6] Disable the cherrypy dispatch method translation for dot and hyphen This will prevent cherrypy from automatically translating dots and hyphens to underscores. This is useful so we can support client IDs and service provider names with dots or hyphens in them while still keeping them uniquely identifiable. Signed-off-by: Patrick Uiterwijk Reviewed-by: Pierre-Yves Chibon Reviewed-by: Howard Johnson --- diff --git a/ipsilon/ipsilon b/ipsilon/ipsilon index 1831805..8f52966 100755 --- a/ipsilon/ipsilon +++ b/ipsilon/ipsilon @@ -13,6 +13,7 @@ sys.stdout = sys.stderr import glob import os import atexit +import string import cherrypy from ipsilon import find_config from ipsilon.util.data import AdminStore @@ -67,9 +68,13 @@ template_env = Environment(loader=ChoiceLoader(template_loaders), autoescape=True, extensions=['jinja2.ext.autoescape']) +transchars = string.punctuation.replace('-', '').replace('.', '') +trans = string.maketrans(transchars, '_' * len(transchars)) if __name__ == "__main__": conf = {'global': {'server.socket_host': '0.0.0.0'}, - '/': {'tools.staticdir.root': os.getcwd()}, + '/': {'tools.staticdir.root': os.getcwd(), + 'request.dispatch': + cherrypy.dispatch.Dispatcher(translate=trans)}, '/ui': {'tools.staticdir.on': True, 'tools.staticdir.dir': 'ui'}, '/cache': {'tools.staticdir.on': True, @@ -80,9 +85,12 @@ if __name__ == "__main__": else: cherrypy.config['environment'] = 'embedded' + conf = {'/': {'request.dispatch': + cherrypy.dispatch.Dispatcher(translate=trans)}} + if cherrypy.__version__.startswith('3.0') and cherrypy.engine.state == 0: cherrypy.engine.start(blocking=False) atexit.register(cherrypy.engine.stop) application = cherrypy.Application(Root('default', template_env), - script_name=None, config=None) + script_name=None, config=conf) From 310f0f386b4455e85c914f04256d0719e5bd0035 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Oct 05 2016 12:32:02 +0000 Subject: [PATCH 4/6] Implement OpenID Connect Client configuration as ConfigHelper Signed-off-by: Patrick Uiterwijk Reviewed-by: Pierre-Yves Chibon Reviewed-by: Howard Johnson --- diff --git a/ipsilon/providers/openidc/auth.py b/ipsilon/providers/openidc/auth.py index 3ea0865..0386fa6 100644 --- a/ipsilon/providers/openidc/auth.py +++ b/ipsilon/providers/openidc/auth.py @@ -228,7 +228,7 @@ class Authorization(AuthenticateRequest): try: # FIXME: Implement decryption decoded = JWT(jwt=jwt_object) - if 'request_object_signing_alg' in client: + if client['request_object_signing_alg'] != 'none': # Client told us we need to check signature if decoded.token.jose_header['alg'] != \ client['request_object_signing_alg']: @@ -240,7 +240,7 @@ class Authorization(AuthenticateRequest): decoded.token.objects['payload']) else: keyset = None - if 'jkws' in client: + if client['jwks']: keys = json.loads(client['jkws']) else: keys = requests.get(client['jwks_uri']).json() @@ -375,7 +375,7 @@ class Authorization(AuthenticateRequest): needs_auth = True if not user.is_anonymous: - if request_data['max_age'] is None: + if request_data['max_age'] in [None, 0]: needs_auth = False else: auth_time = us.get_user_attrs()['_auth_time'] @@ -431,7 +431,7 @@ class Continue(AuthenticateRequest): userinfo['sub'] = user.name else: h = hashlib.sha256() - if 'sector_identifier_uri' in client: + if client['sector_identifier_uri']: domain = get_url_hostpart( client['sector_identifier_uri']) else: diff --git a/ipsilon/providers/openidc/provider.py b/ipsilon/providers/openidc/provider.py index 3305f28..ae5023c 100644 --- a/ipsilon/providers/openidc/provider.py +++ b/ipsilon/providers/openidc/provider.py @@ -2,6 +2,7 @@ from ipsilon.providers.openidc.api import APIError, APIRequest from ipsilon.util.security import generate_random_secure_string +import ipsilon.util.config as pconfig import cherrypy @@ -19,108 +20,6 @@ def get_url_hostpart(url): return url -def validate_client_metadata(client_metadata): - # Fill in defaults for optional arguments - client_metadata['response_types'] = client_metadata.get( - 'response_types', ['code']) - client_metadata['grant_types'] = client_metadata.get( - 'grant_types', ['authorization_code']) - client_metadata['application_type'] = client_metadata.get( - 'application_type', 'web') - client_metadata['contacts'] = client_metadata.get('contacts', []) - client_metadata['subject_type'] = client_metadata.get('subject_type', - 'pairwise') - client_metadata['id_token_signed_response_alg'] = client_metadata.get( - 'id_token_signed_response_alg', 'RS256') - client_metadata['token_endpoint_auth_method'] = client_metadata.get( - 'token_endpoint_auth_method', 'client_secret_basic') - client_metadata['require_auth_time'] = client_metadata.get( - 'require_auth_time', False) - - # Check the client metadata received - if 'redirect_uris' not in client_metadata: - raise APIError(400, 'invalid_client_metadata', - 'missing redirect_uris') - - if client_metadata['application_type'] not in ['web', 'native']: - raise APIError(400, 'invalid_client_metadata', - 'application_type invalid') - - for redirect_uri in client_metadata['redirect_uris']: - if '#' in redirect_uri: - raise APIError(400, 'invalid_redirect_uri', - 'redirect_uri contains fragment') - - if client_metadata['application_type'] == 'web': - # In this case, it must be https:// and not https://localhost - if (not redirect_uri.startswith('https://') or - redirect_uri.startswith('https://localhost')): - raise APIError(400, 'invalid_redirect_uri', - 'redirect_uri %s not valid' % redirect_uri) - - elif client_metadata['application_type'] == 'native': - # In this case, it must be http://localhost, or something - # that is not http:// or https:// - if (redirect_uri.startswith('https://') or - (redirect_uri.startswith('http://') and - not redirect_uri.startswith('http://localhost'))): - raise APIError(400, 'invalid_redirect_uri', - 'redirect_uri %s not valid' % redirect_uri) - - if 'initiate_login_uri' in client_metadata: - if not client_metadata['initiate_login_uri'].startswith( - 'https://'): - raise APIError(400, 'invalid_client_metadata', - 'initiate_login_uri must be https') - - if 'sector_identifier_uri' not in client_metadata: - hostname = None - for redir_uri in client_metadata['redirect_uris']: - cur_host = get_url_hostpart(redir_uri) - if not cur_host: - raise APIError(400, 'invalid_client_metadata', - 'Unable to parse hostname from ' + - 'redirect_uri %s' % redir_uri) - if hostname is not None and cur_host != hostname: - raise APIError(400, 'invalid_client_metadata', - 'Multiple redirect_uri hostnames without ' + - 'sector_identifier_uri') - hostname = cur_host - else: - if not client_metadata['sector_identifier_uri'].startswith( - 'https://'): - raise APIError(400, 'invalid_client_metadata', - 'sector_identifier_uri must be https') - - try: - resp = requests.get(client_metadata['sector_identifier_uri']) - resp = resp.json() - for redirect_uri in client_metadata['redirect_uris']: - if redirect_uri not in resp: - raise APIError(400, 'invalid_client_metadata', - 'redirect_uri %s not in ' + - 'sector_identifier_uri document' % - redirect_uri) - except Exception as ex: - raise APIError(400, 'invalid_client_metadata', - 'unable to process sector_identifier_uri: %s' % ex) - - if 'code' in client_metadata['response_types']: - if 'authorization_code' not in client_metadata['grant_types']: - raise APIError(400, 'invalid_client_metadata', - 'authorization_code missing with code') - - if ('token' in client_metadata['response_types'] or - 'id_token' in client_metadata['response_types']): - if 'implicit' not in client_metadata['grant_types']: - raise APIError(400, 'invalid_client_metadata', - 'implicit missing with token or id_token') - - if 'jwks' in client_metadata and 'jwks_uri' in client_metadata: - raise APIError(400, 'invalid_client_metadata', - 'both jwks and jwks_uri provided') - - class Registration(APIRequest): def POST(self, *args, **kwargs): @@ -136,17 +35,25 @@ class Registration(APIRequest): self.debug('Received client registration request: %s' % client_metadata) - validate_client_metadata(client_metadata) + if 'ipsilon_internal' in client_metadata: + raise APIError(400, 'invalid_client_metadata', + 'Internal information provided') - client_metadata['client_secret'] = \ - generate_random_secure_string() - client_metadata['client_secret_expires_at'] = 0 # FIXME: Expire? - client_metadata['client_id_issued_at'] = int(time.time()) + try: + clt = Client(client_metadata, trusted=False) + clt.validate() + clt.generate_secret() + except InvalidMetadata as ex: + raise APIError(400, 'invalid_client_metadata', + ex.message) + except InvalidRedirectURI as ex: + raise APIError(400, 'invalid_redirect_uri', + ex.message) + except pconfig.FieldValueError as ex: + raise APIError(400, 'invalid_request', + 'invalid field value for %s' % ex.field) - # Store some internal data - client_metadata['ipsilon_internal'] = { - 'trusted': False - } + client_metadata = clt.generate() # Store and add reg uri client_id = self.cfg.datastore.registerDynamicClient(client_metadata) @@ -162,3 +69,300 @@ class Registration(APIRequest): # self.cfg.endpoint_url, 'ClientConfiguration') return self._respond(client_metadata) + + +class Client(pconfig.ConfigHelper): + def __init__(self, client_info=None, trusted=True): + super(Client, self).__init__() + if client_info is None: + client_info = {} + self.client_info = client_info + self.readonly = self.client_info.get('type', 'new') == 'dynamic' + if 'ipsilon_internal' in client_info: + self.client_id = client_info['ipsilon_internal']['client_id'] + else: + self.client_id = None + self.client_info['ipsilon_internal'] = {'trusted': trusted} + + self.load_config() + + def generate_secret(self, force=False): + if 'client_secret' not in self.client_info or force: + self.client_info['client_secret'] = \ + generate_random_secure_string() + self.client_info['client_secret_expires_at'] = 0 # FIXME: Expire? + self.client_info['client_id_issued_at'] = int(time.time()) + + def generate(self): + metadata = self.generate_public() + + self.generate_secret() + metadata['client_secret'] = self.client_info['client_secret'] + metadata['client_secret_expires_at'] = \ + self.client_info['client_secret_expires_at'] + metadata['client_id_issued_at'] = \ + self.client_info['client_id_issued_at'] + metadata['ipsilon_internal'] = self.client_info['ipsilon_internal'] + return metadata + + def generate_public(self): + metadata = {} + for option, value in self.get_config_obj().iteritems(): + name = option.replace(' ', '_').lower() + metadata[name] = value.get_value() + return metadata + + def validate(self): + conf = self.get_config_obj() + if len(conf['Redirect URIs'].get_value()) == 0: + raise InvalidMetadata('No Redirect URIs') + + if conf['Redirect URIs'].get_value() == ['']: + raise InvalidMetadata('No Redirect URIs') + + for redirect_uri in conf['Redirect URIs'].get_value(): + if '#' in redirect_uri: + raise InvalidRedirectURI('redirect_uri contains fragment') + + if conf['Application Type'].get_value() == 'web': + # In this case, it must be https:// and not https://localhost + if (not redirect_uri.startswith('https://') or + redirect_uri.startswith('https://localhost')): + raise InvalidRedirectURI('non-https or localhost with web') + + elif conf['Application Type'].get_value() == 'native': + # In this case, it must be http://localhost, or something + # that is not http:// or https:// + if (redirect_uri.startswith('https://') or + (redirect_uri.startswith('http://') and + not redirect_uri.startswith('http://localhost'))): + raise InvalidRedirectURI('http or https with native') + + if conf['Initiate Login URI'].get_value(): + ilu = conf['Initiate Login URI'].get_value() + if not ilu.startswith('https://'): + raise InvalidMetadata('Initiate Login URI does not start with ' + 'https') + + if not conf['Sector Identifier URI'].get_value(): + hostname = None + for redir_uri in conf['Redirect URIs'].get_value(): + cur_host = get_url_hostpart(redir_uri) + if not cur_host: + raise InvalidRedirectURI('Unable to parse hostname from %s' + % redir_uri) + if hostname is not None and cur_host != hostname: + raise InvalidMetadata('Multiple redirect_uri hostnames ' + 'without sector identifier') + hostname = cur_host + else: + si_uri = conf['Sector Identifier URI'].get_value() + if not si_uri.startswith('https://'): + raise InvalidMetadata('Sector identifier URI must be https') + + try: + resp = requests.get(si_uri) + resp = resp.json() + for redirect_uri in conf['Redirect URIs'].get_value(): + if redirect_uri not in resp: + raise InvalidMetadata('Redirect URI %s not in sector ' + 'identifier document' + % redirect_uri) + except Exception as ex: + self.debug('Unable to retrieve sector identifiers: %s' + % repr(ex)) + raise InvalidMetadata('Unable to retrieve sector identifier') + + gtypes = conf['Grant Types'].get_value() + for rtype in conf['Response Types'].get_value(): + if 'code' in rtype and 'authorization_code' not in gtypes: + raise InvalidMetadata('authorization_code grant type missing ' + 'for response type code') + + if 'token' in rtype and 'implicit' not in gtypes: + raise InvalidMetadata('implicit grant type missing for ' + 'response type token or id_token') + + if conf['JWKS'].get_value() and conf['JWKS URI'].get_value(): + raise InvalidMetadata('Both JWKs and JWKs URI are provided') + + def get_current_info(self, option): + if option == 'client_id': + return self.client_id or '' + elif option in self.client_info: + return self.client_info[option] + elif option in ['redirect_uris', 'contacts', 'request_uris']: + return [] + elif option == 'response_types': + return ['code'] + elif option == 'grant_types': + return ['authorization_code'] + elif option == 'application_type': + return 'web' + elif option == 'subject_type': + return 'pairwise' + elif option == 'require_auth_time': + return False + elif option == 'token_endpoint_auth_method': + return 'client_secret_basic' + elif option == 'id_token_signed_response_alg': + return 'RS256' + elif option == 'client_secret': + if 'client_secret' in self.client_info: + return self.client_info['client_secret'] + else: + return '*Autogenerated*' + elif option == 'default_max_age': + return 0 + else: + self.error('Unknown') + return '' + + def load_config(self): + self.new_config( + self.client_id, + pconfig.String( + 'Client ID', + 'Client Identifier used in the protocol.', + self.get_current_info('client_id'), + readonly=self.client_id is not None), + pconfig.String( + 'Client Secret', + 'Client secret used to authenticate.', + self.get_current_info('client_secret'), + readonly=True), + pconfig.String( + 'Client Name', + 'A nickname shown to the user to identify the client.', + self.get_current_info('client_name'), + readonly=self.readonly), + pconfig.List( + 'Redirect URIs', + 'URIs to be used by the client as redirect URIs. Must all ' + 'start with https if application type is web, must all start ' + 'with http://localhost/ if application type is native.', + self.get_current_info('redirect_uris'), + readonly=self.readonly), + pconfig.Pick( + 'Application Type', + 'Application type of the client.', + ['web', 'native'], + self.get_current_info('application_type'), + readonly=self.readonly), + pconfig.String( + 'Client URI', + 'URI of the home page of the client.', + self.get_current_info('client_uri'), + readonly=self.readonly), + pconfig.List( + 'Contacts', + 'List of contacts email addressess for this client.', + self.get_current_info('contacts'), + readonly=self.readonly), + pconfig.String( + 'Logo URI', + 'URI of the a logo for the client.', + self.get_current_info('logo_uri'), + readonly=self.readonly), + pconfig.String( + 'Policy URI', + 'URI to the client privacy policy.', + self.get_current_info('policy_uri'), + readonly=self.readonly), + pconfig.String( + 'TOS URI', + 'URI to the client Terms of Service.', + self.get_current_info('tos_uri'), + readonly=self.readonly), + pconfig.String( + 'JWKS URI', + 'URI to the client JSON Web Key Set document.', + self.get_current_info('jwks_uri'), + readonly=self.readonly), + pconfig.String( + 'JWKS', + 'Document with client JSON Web Key Set', + self.get_current_info('jwks'), + readonly=self.readonly, + multiline=True), + pconfig.String( + 'Sector Identifier URI', + 'URI identifying the pairwise subject value sector.', + self.get_current_info('sector_identifier_uri'), + readonly=self.readonly), + pconfig.Pick( + 'Subject type', + 'Subject type to be used for this client', + ['pairwise', 'public'], + self.get_current_info('subject_type'), + readonly=self.readonly), + pconfig.Choice( + 'Response Types', + 'Response types that will be used', + ['code', 'id_token', 'id_token token', 'code id_token', + 'code token', 'code id_token token'], + self.get_current_info('response_types'), + readonly=self.readonly), + pconfig.Choice( + 'Grant Types', + 'Grant types this client will use.', + ['authorization_code', 'implicit', 'refresh_token'], + self.get_current_info('grant_types'), + readonly=self.readonly), + pconfig.List( + 'Request URIs', + 'URIs used by the client containing request objects.', + self.get_current_info('request_uris'), + readonly=self.readonly), + pconfig.Condition( + 'Require Auth Time', + 'Whether the client requires the last auth time.', + self.get_current_info('require_auth_time'), + readonly=self.readonly), + pconfig.Pick( + 'Token Endpoint Auth Method', + 'Auth method used by the client to the token endpoint.', + ['client_secret_post', 'client_secret_basic', + 'client_secret_jwt', 'private_key_jwt', 'none'], + self.get_current_info('token_endpoint_auth_method'), + readonly=self.readonly), + pconfig.Pick( + 'ID Token Signed Response Alg', + 'Algorithm used to sign ID Tokens', + ['RS256'], + self.get_current_info('id_token_signed_response_alg'), + readonly=self.readonly), + pconfig.String( + 'Initiate Login URI', + 'URI that third party can use to initiate login at client.', + self.get_current_info('initiate_login_uri'), + readonly=self.readonly), + pconfig.Integer( + 'Default Max Age', + 'Default maximum age for authentication timeout', + self.get_current_info('default_max_age'), + readonly=self.readonly), + pconfig.List( + 'Default ACR values', + 'Default Authentication Context Class requested by client.', + self.get_current_info('default_acr_values'), + readonly=self.readonly), + # TODO: + # id_token_encrypted_response_alg + # id_token_encrypted_response_enc + # userinfo_signed_response_alg + # userinfo_encrypted_response_alg + # userinfo_encrypted_response_enc + # request_object_signing_alg (defualt none) + # request_object_encryption_alg + # request_object_encryption_enc + # token_endpoint_auth_signing_alg + ) + + +class InvalidMetadata(ValueError): + pass + + +class InvalidRedirectURI(ValueError): + pass From f7eadadeed0d4bca9f6fe6986dbc9f565e7538e5 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Oct 05 2016 12:32:05 +0000 Subject: [PATCH 5/6] Implement OpenID Connect static clients This implements static clients, which can be put in a different database than the normal OpenID Connect data, so that they can be put into a configuration file. Signed-off-by: Patrick Uiterwijk Reviewed-by: Pierre-Yves Chibon Reviewed-by: Howard Johnson --- diff --git a/ipsilon/providers/openidc/admin.py b/ipsilon/providers/openidc/admin.py new file mode 100644 index 0000000..fb4bf5d --- /dev/null +++ b/ipsilon/providers/openidc/admin.py @@ -0,0 +1,242 @@ +# Copyright (C) 2016 Ipsilon project Contributors, for license see COPYING + +import cherrypy +from ipsilon.util import config as pconfig +from ipsilon.admin.common import AdminPage +from ipsilon.admin.common import ADMIN_STATUS_OK +from ipsilon.admin.common import ADMIN_STATUS_ERROR +from ipsilon.admin.common import ADMIN_STATUS_WARN +from ipsilon.admin.common import get_mapping_list_value +from ipsilon.admin.common import get_complex_list_value +from ipsilon.providers.openidc.provider import (Client, + InvalidMetadata, + InvalidRedirectURI) +from copy import deepcopy +import logging +import re + + +INVALID_IN_CLIENT_ID = r'[^a-zA-Z0-9\-\.]' + + +class ClientAdminPage(AdminPage): + + def __init__(self, client, site, parent): + super(ClientAdminPage, self).__init__(site, form=True) + self.parent = parent + self.client = Client(client) + self.title = self.client.client_id or 'New client' + if self.client.client_id: + self.new_client = False + self.url = '%s/client/%s' % (parent.url, + self.client.client_id) + else: + self.new_client = True + self.url = '%s/new' % parent.url + self.menu = [parent] + self.back = parent.url + + def root_with_msg(self, message=None, message_type=None): + return self._template('admin/option_config.html', title=self.title, + menu=self.menu, action=self.url, back=self.back, + message=message, message_type=message_type, + name='openidc_client_form', + config=self.client.get_config_obj()) + + def GET(self, *args, **kwargs): + if not self.user.is_admin: + raise cherrypy.HTTPError(403) + + return self.root_with_msg() + + def POST(self, *args, **kwargs): + if not self.user.is_admin: + raise cherrypy.HTTPError(403) + + message = "Nothing was modified." + message_type = "info" + new_db_values = dict() + + conf = self.client.get_config_obj() + + for name, option in conf.iteritems(): + if name in kwargs: + value = kwargs[name] + if isinstance(option, pconfig.List): + value = [x.strip() for x in value.split('\n')] + # for normal lists we want unordered comparison + if set(value) == set(option.get_value()): + continue + elif isinstance(option, pconfig.Condition): + value = True + else: + if isinstance(option, pconfig.Condition): + value = False + elif isinstance(option, pconfig.Choice): + value = list() + for a in option.get_allowed(): + aname = '%s_%s' % (name, a) + if aname in kwargs: + value.append(a) + elif isinstance(option, pconfig.MappingList): + current = deepcopy(option.get_value()) + value = get_mapping_list_value(name, + current, + **kwargs) + # if current value is None do nothing + if value is None: + if option.get_value() is None: + continue + # else pass and let it continue as None + elif isinstance(option, pconfig.ComplexList): + current = deepcopy(option.get_value()) + value = get_complex_list_value(name, + current, + **kwargs) + # if current value is None do nothing + if value is None: + if option.get_value() is None: + continue + # else pass and let it continue as None + else: + continue + + if value != option.get_value() and name not in ['Client ID']: + cherrypy.log.error("Storing %s = %s" % + (name, value), severity=logging.DEBUG) + new_db_values[name] = value + + client_id = kwargs['Client ID'] + if self.new_client and client_id: + if re.search(INVALID_IN_CLIENT_ID, client_id): + message = 'Invalid character in client ID' + message_type = ADMIN_STATUS_WARN + return self.root_with_msg(message, message_type) + elif client_id.startswith('D-'): + # This is not allowed, as the D- is the internal indicator that + # this is a client registered via dynamic registration + message = 'Client ID cannot start with D-' + message_type = ADMIN_STATUS_WARN + return self.root_with_msg(message, message_type) + elif self.parent.cfg.datastore.getClient(client_id): + message = 'Client with this client ID already exists' + message_type = ADMIN_STATUS_WARN + return self.root_with_msg(message, message_type) + + if self.new_client or len(new_db_values) != 0: + try: + for key in new_db_values: + conf[key].set_value(new_db_values[key]) + self.client.validate() + except InvalidMetadata as e: + message = 'Value error: %s' % str(e) + message_type = ADMIN_STATUS_WARN + return self.root_with_msg(message, message_type) + except pconfig.FieldValueError as e: + message = 'Field %s incorrect: %s' % (e.field, str(e)) + message_type = ADMIN_STATUS_WARN + return self.root_with_msg(message, message_type) + except InvalidRedirectURI as e: + message = 'Redirect URI incorrect: %s' % str(e) + message_type = ADMIN_STATUS_WARN + return self.root_with_msg(message, message_type) + except Exception as e: # pylint: disable=broad-except + self.debug("Error: %s" % repr(e)) + message = "Internal Error: %s" % repr(e) + message_type = ADMIN_STATUS_ERROR + return self.root_with_msg(message, message_type) + + try: + metadata = self.client.generate() + if self.new_client: + cid = self.parent.cfg.datastore.registerStaticClient( + client_id, metadata) + message = "Client created" + else: + self.parent.cfg.datastore.updateClient( + self.client.client_id, metadata) + message = "Properties successfully changed" + message_type = ADMIN_STATUS_OK + except Exception as e: # pylint: disable=broad-except + self.error('Failed to save data: %s' % e) + message = "Failed to save data!" + message_type = ADMIN_STATUS_ERROR + return self.root_with_msg(message=message, + message_type=message_type) + + if self.new_client: + raise cherrypy.HTTPRedirect('%s/client/%s' + % (self.parent.url, cid)) + else: + return self.root_with_msg(message=message, + message_type=message_type) + + def delete(self): + if not self.user.is_admin: + raise cherrypy.HTTPError(403) + + if not self.parent.cfg.datastore.deleteClient(self.client.client_id): + raise Exception('Deleting the client did not work') + raise cherrypy.HTTPRedirect(self.parent.url) + delete.public_function = True + + +class DynamicAdminPage(AdminPage): + def __init__(self, site, main): + super(DynamicAdminPage, self).__init__(site) + self.name = 'client' + self.main = main + + def index(self): + return self.unknown_client() + + def root(self, *args, **kwargs): + return self.unknown_client() + + def mount(self, page): + pass + + def unknown_client(self): + raise cherrypy.HTTPRedirect('%s/admin/providers/openidc/admin' + % self.basepath) + unknown_client.exposed = True + + def __getattr__(self, attr): + client = self.main.cfg.datastore.getClient(attr) + if client is None: + return self.unknown_client() + # pylint: disable=protected-access + return ClientAdminPage(client, self.main._site, self.main) + + +class OpenIDCAdminPage(AdminPage): + def __init__(self, site, config): + super(OpenIDCAdminPage, self).__init__(site) + self.name = 'admin' + self.cfg = config + self.menu = [] + self.url = None + self.client = DynamicAdminPage(self._site, self) + + def mount(self, page): + self.menu = page.menu + self.url = '%s/%s' % (page.url, self.name) + self.add_subtree('new', ClientAdminPage({}, self._site, self)) + page.add_subtree(self.name, self) + + @property + def clients(self): + stc_clients = self.cfg.datastore.getStaticClients() + dyn_clients = self.cfg.datastore.getDynamicClients() + # Since all dynamic clients start with D-, and all static clients start + # with something else, it is safe to just update one with the other. + all_clients = stc_clients + all_clients.update(dyn_clients) + return all_clients + + def root(self, *args, **kwargs): + return self._template('admin/providers/openidc.html', + title='OpenID Connect Administration', + clients=self.clients, + baseurl=self.url, + menu=self.menu) diff --git a/ipsilon/providers/openidc/store.py b/ipsilon/providers/openidc/store.py index 7617bc2..de5bc12 100644 --- a/ipsilon/providers/openidc/store.py +++ b/ipsilon/providers/openidc/store.py @@ -2,16 +2,36 @@ from ipsilon.util.security import (generate_random_secure_string, constant_time_string_comparison) -from ipsilon.util.data import Store, UNIQUE_DATA_TABLE +from ipsilon.util.data import Store, UNIQUE_DATA_TABLE, OPTIONS_TABLE +from uuid import uuid4 import json import time -class OpenIDCStore(Store): +# This is a different store, since this can be a configuration file if the +# static OpenIDC clients are stored in a configuration file. +class OpenIDCStaticStore(Store): + _should_cleanup = False + def __init__(self, database_url): Store.__init__(self, database_url=database_url) + def _initialize_schema(self): + q = self._query(self._db, 'client', OPTIONS_TABLE, + trans=False) + q.create() + q._con.close() # pylint: disable=protected-access + + def _upgrade_schema(self, old_version): + raise NotImplementedError() + + +class OpenIDCStore(Store): + def __init__(self, database_url, static_store): + Store.__init__(self, database_url=database_url) + self.static_store = static_store + def registerDynamicClient(self, client): data = {} @@ -23,39 +43,102 @@ class OpenIDCStore(Store): # Prepend client ID with D- to indicate that this is a dynamic client return 'D-%s' % client_id - def registerStaticClient(self, client): - # TODO: Implement static client + def registerStaticClient(self, client_id, client): + if not client_id: + client_id = uuid4().hex + + data = {} + for key in client: + data[key] = json.dumps(client[key]) - client_id = None + self.static_store.save_options('client', client_id, data) - # Prepend client ID with S- to indicate that this is a static client - return 'S-%s' % client_id + return client_id + + def updateClient(self, client_id, client): + if 'type' in client['ipsilon_internal']: + del client['ipsilon_internal']['type'] + if 'client_id' in client['ipsilon_internal']: + del client['ipsilon_internal']['client_id'] + + info = {} + for key, datum in client: + info[key] = json.loads(datum) - def getClient(self, client_id): if client_id.startswith('D-'): # This is a dynamically registered client client_id = client_id[2:] - data = self.get_unique_data('client', client_id) - elif client_id.startswith('S-'): + self.save_unique_data('client', {client_id: info}) + else: # This is a statically configured client - client_id = client_id[2:] - # TODO: Get the configured client data - return None + self.static_store.save_options('client', {client_id: info}) + + def getDynamicClients(self): + clients = {} + results = self.get_unique_data('client') + for cid in results: + info = {} + for key in results[cid]: + info[key] = json.loads(results[cid][key]) + + info['ipsilon_internal']['type'] = 'dynamic' + info['ipsilon_internal']['client_id'] = 'D-%s' % cid + clients['D-%s' % cid] = info + return clients + + def getStaticClients(self): + clients = {} + results = self.static_store.load_options('client') + for cid in results: + info = {} + for key in results[cid]: + info[key] = json.loads(results[cid][key]) + + info['ipsilon_internal']['type'] = 'static' + info['ipsilon_internal']['client_id'] = cid + clients[cid] = info + return clients + + def getClient(self, client_id): + if client_id.startswith('D-'): + # This is a dynamically registered client + ctype = 'dynamic' + data = self.get_unique_data('client', client_id[2:]) else: - # No idea what this is - self.debug('Invalid client ID request: %s' % client_id) - return None + # This is a statically configured client + ctype = 'static' + data = self.static_store.load_options('client', client_id) if len(data) < 1: return None - - datum = data[client_id] + elif len(data) == 1: + datum = data[client_id[2:]] + else: + datum = data for key in datum: datum[key] = json.loads(datum[key]) + datum['ipsilon_internal']['type'] = ctype + datum['ipsilon_internal']['client_id'] = client_id + return datum + def deleteClient(self, client_id): + if not self.getClient(client_id): + return False + + if client_id.startswith('D-'): + # This is a dynamically registered client + self.del_unique_data('client', client_id[2:]) + else: + # This is a statically configured client + self.static_store.delete_options('client', client_id) + + if self.getClient(client_id): + return False + return True + def lookupToken(self, token, expected_type, return_expired=False): if '_' not in token: return None diff --git a/ipsilon/providers/openidcp.py b/ipsilon/providers/openidcp.py index f1706f7..3902d46 100644 --- a/ipsilon/providers/openidcp.py +++ b/ipsilon/providers/openidcp.py @@ -4,8 +4,9 @@ from __future__ import absolute_import from ipsilon.providers.common import ProviderBase, ProviderInstaller from ipsilon.providers.openidc.plugins.common import LoadExtensions -from ipsilon.providers.openidc.store import OpenIDCStore +from ipsilon.providers.openidc.store import OpenIDCStore, OpenIDCStaticStore from ipsilon.providers.openidc.auth import OpenIDC +from ipsilon.providers.openidc.admin import OpenIDCAdminPage from ipsilon.util.plugin import PluginObject from ipsilon.util import config as pconfig from ipsilon.info.common import InfoMapping @@ -23,6 +24,7 @@ class IdpProvider(ProviderBase): super(IdpProvider, self).__init__('openidc', 'openidc', *pargs) self.mapping = InfoMapping() self.keyset = None + self.admin = None self.page = None self.datastore = None self.server = None @@ -37,6 +39,10 @@ Provides OpenID Connect authentication infrastructure. """ 'database url', 'Database URL for OpenID Connect storage', 'openidc.sqlite'), + pconfig.String( + 'static database url', + 'Database URL for OpenID Connect static client configuration', + 'openidc.static.sqlite'), pconfig.Choice( 'enabled extensions', 'Choose the extensions to enable', @@ -154,12 +160,12 @@ Provides OpenID Connect authentication infrastructure. """ def get_tree(self, site): self.page = OpenIDC(site, self) - # self.admin = AdminPage(site, self) + self.admin = OpenIDCAdminPage(site, self) return self.page def used_datastores(self): - return [self.datastore] + return [self.datastore, self.datastore.static_store] def init_idp(self): self.keyset = JWKSet() @@ -168,7 +174,10 @@ Provides OpenID Connect authentication infrastructure. """ for key in loaded_keys['keys']: self.keyset.add(JWK(**key)) - self.datastore = OpenIDCStore(self.get_config_value('database url')) + static_store = OpenIDCStaticStore( + self.get_config_value('static database url')) + self.datastore = OpenIDCStore(self.get_config_value('database url'), + static_store) def openid_connect_issuer_wf_rel(self, resource): link = { @@ -206,6 +215,8 @@ class Installer(ProviderInstaller): help='Configure OpenID Connect Provider') group.add_argument('--openidc-dburi', help='OpenID Connect database URI') + group.add_argument('--openidc-static-dburi', + help='OpenID Connect static client database URI') group.add_argument('--openidc-subject-salt', default=None, help='Salt to use for pairwise subject subjects') group.add_argument('--openidc-extensions', default='', @@ -251,6 +262,9 @@ class Installer(ProviderInstaller): 'database url': opts['openidc_dburi'] or opts['database_url'] % { 'datadir': opts['data_dir'], 'dbname': 'openidc'}, + 'static database url': opts['openidc_static_dburi'] or + opts['database_url'] % { + 'datadir': opts['data_dir'], 'dbname': 'openidc.static'}, 'enabled extensions': opts['openidc_extensions'], 'idp key file': keyfile, 'idp sig key id': '%s-sig' % keyid, diff --git a/templates/admin/providers/openidc.html b/templates/admin/providers/openidc.html new file mode 100644 index 0000000..c027c62 --- /dev/null +++ b/templates/admin/providers/openidc.html @@ -0,0 +1,23 @@ +{% extends "master-admin.html" %} +{% block main %} +

Clients

+ +
+
+ Add New +
+
+
+{% for cid in clients %} +
+
+ {{ cid }} +
+
+ {{ cid }} + Delete +
+
+{% endfor %} +{% endblock %} From a604822297596da4f324daf4048ebbe8bd7efc4e Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Oct 05 2016 12:32:07 +0000 Subject: [PATCH 6/6] Test basic OpenIDC administration code Signed-off-by: Patrick Uiterwijk Reviewed-by: Pierre-Yves Chibon Reviewed-by: Howard Johnson --- diff --git a/tests/helpers/http.py b/tests/helpers/http.py index 2bd1dd8..1f8ab47 100755 --- a/tests/helpers/http.py +++ b/tests/helpers/http.py @@ -523,6 +523,23 @@ class HttpSessions(object): if r.status_code != 200: raise ValueError('Failed to post IDP data [%s]' % repr(r)) + def delete_oidc_client(self, idp, client_id): + """ + Delete the specified client from the OpenID client list. + """ + idpsrv = self.servers[idp] + idpuri = idpsrv['baseuri'] + + url = '%s/%s/admin/providers/openidc/admin/client/%s/delete' % ( + idpuri, idp, client_id) + headers = {'referer': url} + headers['content-type'] = 'application/x-www-form-urlencoded' + r = idpsrv['session'].get(url, headers=headers) + if r.status_code != 200: + raise ValueError('Failed to delete client [%s]' % repr(r)) + if client_id in r.text: + raise ValueError('Client was not gone after deletion') + def fetch_rest_page(self, idpname, uri): """ idpname - the name of the IDP to fetch the page from diff --git a/tests/openidc.py b/tests/openidc.py index 05c015a..8f06a4d 100755 --- a/tests/openidc.py +++ b/tests/openidc.py @@ -315,6 +315,15 @@ if __name__ == '__main__': info['scope'].remove(scope) if len(info['scope']) != 0: raise Exception('Unexpected scopes found: %s' % info['scope']) + + # Delete test client and then try to use it + sess.delete_oidc_client(idpname, reg_resp['client_id']) + r = requests.post('https://127.0.0.10:45080/idp1/openidc/TokenInfo', + data={'token': token['access_token'], + 'client_id': reg_resp['client_id'], + 'client_secret': reg_resp['client_secret']}) + if r.status_code != 400: + raise Exception('Deleted client accepted') except ValueError, e: print >> sys.stderr, " ERROR: %s" % repr(e) sys.exit(1)