From 20f9ccaf9fb7fcf524f6909b0e36bd2e9ed82450 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Mon, 30 May 2016 13:46:36 -0700 Subject: [PATCH 1/5] function needed to be made public and importable. --- src/oic/oauth2/provider.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/oic/oauth2/provider.py b/src/oic/oauth2/provider.py index c9ffcbd39..4de4dbcc4 100644 --- a/src/oic/oauth2/provider.py +++ b/src/oic/oauth2/provider.py @@ -117,6 +117,13 @@ def token_response(**kwargs): return aresp +def error_response(error, descr=None): + logger.error("%s" % error) + response = ErrorResponse(error=error, error_description=descr) + return Response(response.to_json(), content="application/json", + status="400 Bad Request") + + # noinspection PyUnusedLocal def none_response(**kwargs): _areq = kwargs["areq"] @@ -212,16 +219,11 @@ def input(query="", post=None): @staticmethod def _error_response(error, descr=None): - logger.error("%s" % error) - response = ErrorResponse(error=error, error_description=descr) - return Response(response.to_json(), content="application/json", - status="400 Bad Request") + return error_response(error=error, descr=descr) @staticmethod def _error(error, descr=None): - response = ErrorResponse(error=error, error_description=descr) - return Response(response.to_json(), content="application/json", - status="400 Bad Request") + return error_response(error=error, descr=descr) @staticmethod def _authz_error(error, descr=None): From 2e9769c564f674e96a4c460ed1362656d2666fe4 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 2 Jun 2016 15:10:37 -0700 Subject: [PATCH 2/5] Don't check against session, not trustworthy. --- src/oic/utils/rp/oauth2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/oic/utils/rp/oauth2.py b/src/oic/utils/rp/oauth2.py index 5356ef6cd..4cb4b574d 100644 --- a/src/oic/utils/rp/oauth2.py +++ b/src/oic/utils/rp/oauth2.py @@ -120,7 +120,7 @@ def callback(self, response, session, format='dict'): else: raise OAuth2Error("Access denied") - if session["state"] != authresp["state"]: + if authresp["state"] not in self.authz_req: self._err("Received state not the same as expected.") if self.behaviour["response_type"] == "code": From e16f122c9af86dbbd8643b1dabf341c12bddf16b Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 2 Jun 2016 15:11:36 -0700 Subject: [PATCH 3/5] If a key file is not where it's supposed to be - create it. --- src/oic/utils/keyio.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/oic/utils/keyio.py b/src/oic/utils/keyio.py index 9b04811ed..49c5de807 100644 --- a/src/oic/utils/keyio.py +++ b/src/oic/utils/keyio.py @@ -961,6 +961,8 @@ def build_keyjar(key_conf, kid_template="", keyjar=None, kidd=None): fileformat="der", keytype=typ, keyusage=spec["use"]) except FileNotFoundError: + if 'name' not in spec: + spec['name'] = spec['key'] kb = rsa_init(spec) else: kb = rsa_init(spec) From 53e59354e4d1dafda32bf08111b52220ec2cba4d Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 2 Jun 2016 15:12:56 -0700 Subject: [PATCH 4/5] Machine readable logs Use Application class Don't trust session information --- oidc_example/rp3/rp3.py | 562 +++++++++++++++++++++++----------------- 1 file changed, 320 insertions(+), 242 deletions(-) diff --git a/oidc_example/rp3/rp3.py b/oidc_example/rp3/rp3.py index 991df2771..60ca8600e 100755 --- a/oidc_example/rp3/rp3.py +++ b/oidc_example/rp3/rp3.py @@ -44,6 +44,27 @@ SERVER_ENV = {} +class JLog(object): + def __init__(self, logger, sid): + self.logger = logger + self.id = sid + + def info(self, info): + _dict = {'id': self.id} + _dict.update(info) + self.logger.info(json.dumps(_dict)) + + def error(self, info): + _dict = {'id': self.id} + _dict.update(info) + self.logger.error(json.dumps(_dict)) + + def warning(self, info): + _dict = {'id': self.id} + _dict.update(info) + self.logger.warning(json.dumps(_dict)) + + # noinspection PyUnresolvedReferences def static(environ, start_response, logger, path): logger.info("[static]sending: %s" % (path,)) @@ -83,14 +104,14 @@ def opresult(environ, start_response, **kwargs): template_lookup=LOOKUP, headers=[]) - args = {} + _args = {} for param in ['userinfo', 'userid', 'id_token']: try: - args[param] = kwargs[param] + _args[param] = kwargs[param] except KeyError: - args[param] = None + _args[param] = None - return resp(environ, start_response, **args) + return resp(environ, start_response, **_args) def operror(environ, start_response, error=None): @@ -144,245 +165,298 @@ def url_eq(a, b): return a == b -def application(environ, start_response): - session = environ['beaker.session'] - path = environ.get('PATH_INFO', '').lstrip('/') - if path == "robots.txt": - return static(environ, start_response, LOGGER, "static/robots.txt") - elif path.startswith("static/"): - return static(environ, start_response, LOGGER, path) - elif '/static/' in path: - pre, post = path.split('static') - return static(environ, start_response, LOGGER, 'static'+post) - - query = parse_qs(environ["QUERY_STRING"]) - acr_values = session._params['acrs'] - clients = session._params['clients'] - server_env = session._params['server_env'] - - LOGGER.info(50 * "=") - LOGGER.info("[{}] path: {}".format(session.id, path)) - LOGGER.info(50 * "=") - - if path == '': - if 'access_token' not in session: - return opchoice(environ, start_response, clients) - else: - client = clients[session["op"]] - # check_session_iframe_url = None - try: - # check_session_iframe_url = client.provider_info[ - # "check_session_iframe"] - - session["session_management"] = { - "session_state": query["session_state"][0], - "client_id": client.client_id, - "issuer": client.provider_info["issuer"] - } - except KeyError: - pass +KEY_MAP = {'state': 'state', 'iss': 'op'} - kwargs = dict( - [(p, session[p]) for p in ['id_token', 'userinfo', 'user_id'] if - p in session]) - return opresult(environ, start_response, **kwargs) - elif path == "rp": # After having chosen which OP to authenticate at - if "uid" in query: - try: - client = clients.dynamic_client(userid=query["uid"][0]) - except (ConnectionError, OIDCError) as err: - return operror(environ, start_response, '{}'.format(err)) - elif 'issuer' in query: - try: - client = clients[query["issuer"][0]] - except (ConnectionError, OIDCError) as err: - return operror(environ, start_response, '{}'.format(err)) - else: - client = clients[query["op"][0]] +class Application(object): + def __init__(self, acrs, clients, conf, userinfo, base, **extra_args): + self.acr_values = acrs + self.clients = clients + self.conf = conf + self.userinfo = userinfo + self.base = base + self.extra_args = extra_args + self.session = {} - client.get_userinfo = session._params['userinfo'] - try: - client.resource_server = session._params['resource_server'] - except KeyError: - pass + def find_session(self, **kwargs): + _f = 0 + _n = 0 + for _ses in self.session.values(): + for key, vals in kwargs.items(): + try: + _val = _ses[KEY_MAP[key]] + except KeyError: + pass + else: + _n += 1 + if _val in vals: + _f += 1 - try: - session['response_format'] = query["response_format"][0] - except KeyError: - session['response_format'] = 'html' + if _f and _f == _n: + return _ses - session["op"] = client.provider_info["issuer"] + return None - try: - resp = client.create_authn_request(session, acr_values) - except Exception as err: - logging.error(err) - raise - else: - return resp(environ, start_response) - elif path.endswith('authz_post'): - try: - _iss = session['op'] - except KeyError: - LOGGER.info( - '[{}] No active session with {}'.format(session.id, - environ['REMOTE_ADDR'])) - return opchoice(environ, start_response, clients) - else: - client = clients[_iss] + def application(self, environ, start_response): + b_session = environ['beaker.session'] + + jlog = JLog(LOGGER, b_session.id) - query = parse_qs(get_post(environ)) + path = environ.get('PATH_INFO', '').lstrip('/') try: - info = query["fragment"][0] + jlog.info({'cookie': environ['HTTP_COOKIE'].split(';'), + 'path': path}) except KeyError: - return sorry_response(environ, start_response, conf.BASE, - "missing fragment ?!") - if info == ['x']: - return sorry_response(environ, start_response, conf.BASE, - "Expected fragment didn't get one ?!") + jlog.info({'path': path}) - LOGGER.info('[{}] Fragment part: {}'.format(session.id, info)) + if path == "robots.txt": + return static(environ, start_response, LOGGER, "static/robots.txt") + elif path.startswith("static/"): + return static(environ, start_response, LOGGER, path) + elif '/static/' in path: + pre, post = path.split('static') + return static(environ, start_response, LOGGER, 'static' + post) + + query = parse_qs(environ["QUERY_STRING"]) try: - result = client.callback(info, session, 'urlencoded') - if isinstance(result, SeeOther): - return result(environ, start_response) - except OIDCError as err: - return operror(environ, start_response, "%s" % err) - except Exception as err: - raise - else: - session.update(result) - res = SeeOther(server_env['base_url']) - return res(environ, start_response) - elif path in clients.return_paths(): # After having authenticated at the OP - try: - _iss = session['op'] - except KeyError: - LOGGER.info( - '[{}]No active session with {}'.format(session.id, - environ['REMOTE_ADDR'])) - return opchoice(environ, start_response, clients) - - # mismatch between callback and return_uri - # ignore trailing '/' - if not url_eq(_iss, clients.path[path]): - LOGGER.warning( - '[{}]issuer mismatch: {} != {}'.format(session.id, _iss, - clients.path[path])) - return operror(environ, start_response, "%s" % 'Not allowed') - - client = clients[clients.path[path]] - - _response_type = client.behaviour["response_type"] - try: - _response_mode = client.authz_req[session['state']]['response_mode'] + session = b_session['session_info'] except KeyError: - _response_mode = '' - - LOGGER.info( - "[{}]response_type: {}, response_mode: {}".format(session.id, - _response_type, - _response_mode)) - if _response_type and _response_type != "code": - # Fall through if it's a query response anyway - if query: + session = self.find_session(**query) + if session: + b_session['session_info'] = session + else: + session = {} + b_session['session_info'] = session + self.session[b_session.id] = session + + if path == '': + if 'access_token' not in session: + return opchoice(environ, start_response, self.clients) + else: + client = self.clients[session["op"]] + # check_session_iframe_url = None + try: + # check_session_iframe_url = client.provider_info[ + # "check_session_iframe"] + + session["session_management"] = { + "session_state": query["session_state"][0], + "client_id": client.client_id, + "issuer": client.provider_info["issuer"] + } + except KeyError: + pass + + kwargs = dict( + [(p, session[p]) for p in + ['id_token', 'userinfo', 'user_id'] if + p in session]) + + return opresult(environ, start_response, **kwargs) + elif path == "rp": # After having chosen which OP to authenticate at + if "uid" in query: + try: + client = self.clients.dynamic_client(userid=query["uid"][0]) + except (ConnectionError, OIDCError) as err: + return operror(environ, start_response, '{}'.format(err)) + elif 'issuer' in query: + try: + client = self.clients[query["issuer"][0]] + except (ConnectionError, OIDCError) as err: + return operror(environ, start_response, '{}'.format(err)) + else: + client = self.clients[query["op"][0]] + + client.get_userinfo = self.userinfo + try: + client.resource_server = session['resource_server'] + except KeyError: pass - elif _response_mode: - # form_post encoded + + try: + session['response_format'] = query["response_format"][0] + except KeyError: + session['response_format'] = 'html' + + session["op"] = client.provider_info["issuer"] + + try: + resp = client.create_authn_request(session, self.acr_values) + except Exception as err: + logging.error(err) + raise + else: + return resp(environ, start_response) + elif path.endswith('authz_post'): + try: + _iss = session['op'] + except KeyError: + jlog.error({'reason': 'No active session', + 'remote_addr': environ['REMOTE_ADDR']}) + + return opchoice(environ, start_response, self.clients) + else: + client = self.clients[_iss] + + query = parse_qs(get_post(environ)) + try: + info = query["fragment"][0] + except KeyError: + return sorry_response(environ, start_response, self.base, + "missing fragment ?!") + if info == ['x']: + return sorry_response(environ, start_response, self.base, + "Expected fragment didn't get one ?!") + + jlog.info({'fragment': info}) + + try: + result = client.callback(info, session, 'urlencoded') + if isinstance(result, SeeOther): + return result(environ, start_response) + except OIDCError as err: + return operror(environ, start_response, "%s" % err) + except Exception as err: + raise + else: + session.update(result) + res = SeeOther(self.conf['base_url']) + return res(environ, start_response) + elif path in self.clients.return_paths(): # After having + # authenticated at the OP + jlog.info({'query': query}) + + _client = None + for cli in self.clients.client.values(): + if query['state'][0] in cli.authz_req: + _client = cli + break + + if not _client: + jlog.error({ + 'reason': 'No active session', + 'remote_addr': environ['REMOTE_ADDR'], + 'state': query['state'][0] + }) + return opchoice(environ, start_response, self.clients) + + try: + _iss = query['iss'][0] + except KeyError: pass else: - return opresult_fragment(environ, start_response) + if _iss != _client.provider_info['issuer']: + jlog.error({'reason': 'Got response from wrong OP'}) + return opchoice(environ, start_response, self.clients) - LOGGER.info("[{}]Query part: {}".format(session.id, query)) + _response_type = _client.behaviour["response_type"] + try: + _response_mode = _client.authz_req[session['state']][ + 'response_mode'] + except KeyError: + _response_mode = '' + + jlog.info({ + "response_type": _response_type, + "response_mode": _response_mode}) + + if _response_type and _response_type != "code": + # Fall through if it's a query response anyway + if query: + pass + elif _response_mode: + # form_post encoded + pass + else: + return opresult_fragment(environ, start_response) - try: - result = client.callback(query, session) - if isinstance(result, SeeOther): - return result(environ, start_response) - except OIDCError as err: - return operror(environ, start_response, "%s" % err) - except Exception: - raise - else: - session.update(result) - res = SeeOther(server_env['base_url']) - return res(environ, start_response) - elif path == "logout": # After the user has pressed the logout button - try: - _iss = session['op'] - except KeyError: - LOGGER.info( - '[{}]No active session with {}'.format(session.id, - environ['REMOTE_ADDR'])) - return opchoice(environ, start_response, clients) - client = clients[_iss] - try: - del client.authz_req[session['state']] - except KeyError: - pass + try: + result = _client.callback(query, session) + if isinstance(result, SeeOther): + return result(environ, start_response) + except OIDCError as err: + return operror(environ, start_response, "%s" % err) + except Exception: + raise + else: + session.update(result) + res = SeeOther(self.conf['base_url']) + return res(environ, start_response) + elif path == "logout": # After the user has pressed the logout button + try: + _iss = session['op'] + except KeyError: + jlog.error( + {'reason': 'No active session', + 'remote_addr': environ['REMOTE_ADDR']}) + return opchoice(environ, start_response, self.clients) + client = self.clients[_iss] + try: + del client.authz_req[session['state']] + except KeyError: + pass - logout_url = client.end_session_endpoint - try: - # Specify to which URL the OP should return the user after - # log out. That URL must be registered with the OP at client - # registration. - logout_url += "?" + urlencode( - {"post_logout_redirect_uri": client.registration_response[ - "post_logout_redirect_uris"][0]}) - except KeyError: - pass - else: - # If there is an ID token send it along as a id_token_hint - _idtoken = get_id_token(client, session) - if _idtoken: - logout_url += "&" + urlencode({ - "id_token_hint": id_token_as_signed_jwt(client, _idtoken, - "HS256")}) - # Also append the ACR values - logout_url += "&" + urlencode({"acr_values": acr_values}, - True) - - LOGGER.debug("[{}]Logout URL: {}".format(session.id, logout_url)) - LOGGER.debug("Logging out from session: [{}]".format(session.id)) - session.delete() - resp = SeeOther(str(logout_url)) - return resp(environ, start_response) - elif path == "logout_success": # post_logout_redirect_uri - return Response("Logout successful!")(environ, start_response) - elif path == "session_iframe": # session management - kwargs = session["session_management"] - resp = Response(mako_template="rp_session_iframe.mako", - template_lookup=LOOKUP) - return resp(environ, start_response, - session_change_url="{}session_change".format( - server_env["base_url"]), - **kwargs) - elif path == "session_change": - try: - _iss = session['op'] - except KeyError: - LOGGER.info( - '[{}]No active session with {}'.format(session.id, - environ['REMOTE_ADDR'])) - return opchoice(environ, start_response, clients) + logout_url = client.end_session_endpoint + try: + # Specify to which URL the OP should return the user after + # log out. That URL must be registered with the OP at client + # registration. + logout_url += "?" + urlencode( + {"post_logout_redirect_uri": client.registration_response[ + "post_logout_redirect_uris"][0]}) + except KeyError: + pass + else: + # If there is an ID token send it along as a id_token_hint + _idtoken = get_id_token(client, session) + if _idtoken: + logout_url += "&" + urlencode({ + "id_token_hint": id_token_as_signed_jwt(client, + _idtoken, + "HS256")}) + # Also append the ACR values + logout_url += "&" + urlencode({"acr_values": self.acr_values}, + True) + + session.delete() + resp = SeeOther(str(logout_url)) + return resp(environ, start_response) + elif path == "logout_success": # post_logout_redirect_uri + return Response("Logout successful!")(environ, start_response) + elif path == "session_iframe": # session management + kwargs = session["session_management"] + resp = Response(mako_template="rp_session_iframe.mako", + template_lookup=LOOKUP) + return resp(environ, start_response, + session_change_url="{}session_change".format( + self.conf["base_url"]), + **kwargs) + elif path == "session_change": + try: + _iss = session['op'] + except KeyError: + jlog.error({ + 'reason': 'No active session', + 'remote_addr': environ['REMOTE_ADDR']}) + return opchoice(environ, start_response, self.clients) - try: - client = clients[_iss] - except KeyError: - return Response("No valid session.")(environ, start_response) - - kwargs = {"prompt": "none"} - # If there is an ID token send it along as a id_token_hint - idt = get_id_token(client, session) - if idt: - kwargs["id_token_hint"] = id_token_as_signed_jwt(client, idt, - "HS256") - resp = client.create_authn_request(session, acr_values, **kwargs) - return resp(environ, start_response) + try: + client = self.clients[_iss] + except KeyError: + return Response("No valid session.")(environ, start_response) - return opchoice(environ, start_response, clients) + kwargs = {"prompt": "none"} + # If there is an ID token send it along as a id_token_hint + idt = get_id_token(client, session) + if idt: + kwargs["id_token_hint"] = id_token_as_signed_jwt(client, idt, + "HS256") + resp = client.create_authn_request(session, self.acr_values, + **kwargs) + return resp(environ, start_response) + + return opchoice(environ, start_response, self.clients) if __name__ == '__main__': @@ -397,29 +471,30 @@ def application(environ, start_response): parser.add_argument("-b", dest="base_url", help="base url of the RP") parser.add_argument('-k', dest='verify_ssl', action='store_false') args = parser.parse_args() - conf = importlib.import_module(args.config) + _conf = importlib.import_module(args.config) if args.base_url: - conf.BASE = args.base_url + _conf.BASE = args.base_url - _base = "{base}:{port}/".format(base=conf.BASE, port=args.port) + _base = "{base}:{port}/".format(base=_conf.BASE, port=args.port) - for _client, client_conf in six.iteritems(conf.CLIENTS): + for _client, client_conf in six.iteritems(_conf.CLIENTS): if "client_registration" in client_conf: client_reg = client_conf["client_registration"] - client_reg["redirect_uris"] = [url.format(base=conf.BASE) for url in - client_reg["redirect_uris"]] + client_reg["redirect_uris"] = [ + url.format(base=_conf.BASE) for url in + client_reg["redirect_uris"]] session_opts = { 'session.type': 'memory', 'session.cookie_expires': True, 'session.auto': True, 'session.key': "{}.beaker.session.id".format( - urlparse(conf.BASE).netloc.replace(":", ".")) + urlparse(_conf.BASE).netloc.replace(":", ".")) } try: - key_spec = conf.KEY_SPECIFICATION + key_spec = _conf.KEY_SPECIFICATION except AttributeError: jwks_info = {} else: @@ -434,37 +509,40 @@ def application(environ, start_response): f.close() try: - ctype = conf.CLIENT_TYPE + ctype = _conf.CLIENT_TYPE except KeyError: ctype = 'OIDC' if ctype == 'OIDC': - _clients = OIDCClients(conf, _base, jwks_info=jwks_info, + _clients = OIDCClients(_conf, _base, jwks_info=jwks_info, verify_ssl=args.verify_ssl) else: - _clients = OAuthClients(conf, _base, jwks_info=jwks_info, + _clients = OAuthClients(_conf, _base, jwks_info=jwks_info, verify_ssl=args.verify_ssl) SERVER_ENV.update({"template_lookup": LOOKUP, "base_url": _base}) - kwargs = {'clients': _clients, - 'acrs': conf.ACR_VALUES, - 'server_env': SERVER_ENV, - 'userinfo': conf.USERINFO} + app_args = {'clients': _clients, + 'acrs': _conf.ACR_VALUES, + 'conf': SERVER_ENV, + 'userinfo': _conf.USERINFO, + 'base': _conf.BASE} try: - kwargs['resource_server'] = conf.RESOURCE_SERVER + app_args['resource_server'] = _conf.RESOURCE_SERVER except AttributeError: pass + _app = Application(**app_args) + SRV = wsgiserver.CherryPyWSGIServer( ('0.0.0.0', int(args.port)), - SessionMiddleware(application, session_opts, **kwargs)) + SessionMiddleware(_app.application, session_opts)) - if conf.BASE.startswith("https"): + if _conf.BASE.startswith("https"): from cherrypy.wsgiserver.ssl_builtin import BuiltinSSLAdapter - SRV.ssl_adapter = BuiltinSSLAdapter(conf.SERVER_CERT, conf.SERVER_KEY, - conf.CERT_CHAIN) + SRV.ssl_adapter = BuiltinSSLAdapter(_conf.SERVER_CERT, _conf.SERVER_KEY, + _conf.CERT_CHAIN) extra = " using SSL/TLS" else: extra = "" From 03b7b8275dbff90714a53317f5131dc24287a8ce Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 2 Jun 2016 19:12:05 -0700 Subject: [PATCH 5/5] Don't trust session information --- src/oic/utils/rp/__init__.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/oic/utils/rp/__init__.py b/src/oic/utils/rp/__init__.py index 42f2461f1..ad41102ca 100644 --- a/src/oic/utils/rp/__init__.py +++ b/src/oic/utils/rp/__init__.py @@ -117,15 +117,16 @@ def callback(self, response, session, format='dict'): else: raise OIDCError("Access denied") - if session["state"] != authresp["state"]: - self._err("Received state not the same as expected.") + _state = authresp["state"] + # if session["state"] != authresp["state"]: + # self._err("Received state not the same as expected.") try: _id_token = authresp['id_token'] except KeyError: _id_token = None else: - if _id_token['nonce'] != session["nonce"]: + if _id_token['nonce'] != self.authz_req[_state]['nonce']: self._err("Received nonce not the same as expected.") if self.behaviour["response_type"] == "code": @@ -175,7 +176,7 @@ def callback(self, response, session, format='dict'): if _id_token['iss'] != self.provider_info['issuer']: self._err("Issuer mismatch") - if _id_token['nonce'] != session['nonce']: + if _id_token['nonce'] != self.authz_req[_state]['nonce']: self._err("Nonce mismatch") if not self.allow_sign_alg_none: