From 8b301e6de44b7f5724165f539520c463429916f2 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Thu, 13 Jun 2019 11:40:25 -0500 Subject: [PATCH 1/7] feat(incommon): specify downstream shib idp --- fence/blueprints/login/shib.py | 33 ++++++++++++++++++++------------- fence/blueprints/oauth2.py | 2 ++ 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/fence/blueprints/login/shib.py b/fence/blueprints/login/shib.py index 570c6f6bad..5fe633ba9b 100644 --- a/fence/blueprints/login/shib.py +++ b/fence/blueprints/login/shib.py @@ -22,8 +22,19 @@ def get(self): redirect_url = flask.request.args.get("redirect") if redirect_url: flask.session["redirect"] = redirect_url - actual_redirect = config["BASE_URL"] + "/login/shib/login" - return flask.redirect(config["SSO_URL"] + actual_redirect) + + # figure out which IDP to target with shibboleth + # check out shibboleth docs here for more info: + # https://wiki.shibboleth.net/confluence/display/SP3/SSO + entityID = flask.request.args.get("shib_idp") + if not entityID: + # default to SSO_URL from the config which should be NIH login + actual_redirect = config["BASE_URL"] + "/login/shib/login" + return flask.redirect(config["SSO_URL"] + actual_redirect) + return flask.redirect( + config["BASE_URL"] + + "/Shibboleth.sso/Login?entityID={}".format(entityID) + ) class ShibbolethLoginFinish(Resource): @@ -31,17 +42,13 @@ def get(self): """ Complete the shibboleth login. """ - - if "SHIBBOLETH_HEADER" in config: - eppn = flask.request.headers.get(config["SHIBBOLETH_HEADER"]) - - else: + if "SHIBBOLETH_HEADER" not in config: raise InternalError("Missing shibboleth header configuration") + eppn = flask.request.headers.get(config["SHIBBOLETH_HEADER"]) username = eppn.split("!")[-1] if eppn else None - if username: - login_user(flask.request, username, IdentityProvider.itrust) - if flask.session.get("redirect"): - return flask.redirect(flask.session.get("redirect")) - return "logged in" - else: + if not username: raise Unauthorized("Please login") + login_user(flask.request, username, IdentityProvider.itrust) + if flask.session.get("redirect"): + return flask.redirect(flask.session.get("redirect")) + return "logged in" diff --git a/fence/blueprints/oauth2.py b/fence/blueprints/oauth2.py index 99f82dbc88..64abaa3256 100644 --- a/fence/blueprints/oauth2.py +++ b/fence/blueprints/oauth2.py @@ -77,6 +77,8 @@ def authorize(*args, **kwargs): raise UserError("idp {} is not supported".format(idp)) idp_url = IDP_URL_MAP[idp] login_url = "{}/login/{}".format(config.get("BASE_URL"), idp_url) + if idp == "shibboleth": + params["shib_idp"] = flask.request.args.get("shib_idp") login_url = add_params_to_uri(login_url, params) return flask.redirect(login_url) From 6ad5628d743c0e8a651c29067605da4441ad7cee Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Thu, 20 Jun 2019 15:56:00 -0500 Subject: [PATCH 2/7] feat(incommon): save idp --- fence/blueprints/login/shib.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/fence/blueprints/login/shib.py b/fence/blueprints/login/shib.py index 5fe633ba9b..d5435f2edf 100644 --- a/fence/blueprints/login/shib.py +++ b/fence/blueprints/login/shib.py @@ -27,6 +27,7 @@ def get(self): # check out shibboleth docs here for more info: # https://wiki.shibboleth.net/confluence/display/SP3/SSO entityID = flask.request.args.get("shib_idp") + flask.session["entityID"] = entityID if not entityID: # default to SSO_URL from the config which should be NIH login actual_redirect = config["BASE_URL"] + "/login/shib/login" @@ -48,7 +49,10 @@ def get(self): username = eppn.split("!")[-1] if eppn else None if not username: raise Unauthorized("Please login") - login_user(flask.request, username, IdentityProvider.itrust) + idp = IdentityProvider.itrust + if flask.session.get("entityID"): + idp = flask.session.get("entityID") + login_user(flask.request, username, idp) if flask.session.get("redirect"): return flask.redirect(flask.session.get("redirect")) return "logged in" From 899709474a085fc8feddba5a319c5a9fad40705e Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Wed, 26 Jun 2019 14:03:24 -0500 Subject: [PATCH 3/7] feat(incommon): add endpoint to list idps --- fence/blueprints/login/__init__.py | 18 ++++++-- fence/blueprints/login/fence_login.py | 62 +++++++++++++++++++++++++++ tests/login/test_fence_login.py | 6 ++- 3 files changed, 81 insertions(+), 5 deletions(-) diff --git a/fence/blueprints/login/__init__.py b/fence/blueprints/login/__init__.py index ac0484aa7f..cff768d7a9 100644 --- a/fence/blueprints/login/__init__.py +++ b/fence/blueprints/login/__init__.py @@ -6,9 +6,13 @@ the endpoints for each provider. """ +from cdislogging import get_logger import flask +import requests -from fence.blueprints.login.fence_login import FenceLogin, FenceCallback +from fence.blueprints.login.fence_login import ( + FenceLogin, FenceCallback, FenceDownstreamIDPs, get_disco_feed +) from fence.blueprints.login.google import GoogleLogin, GoogleCallback from fence.blueprints.login.shib import ShibbolethLogin, ShibbolethCallback from fence.blueprints.login.microsoft import MicrosoftLogin, MicrosoftCallback @@ -17,8 +21,6 @@ from fence.restful import RestfulApi from fence.config import config -from cdislogging import get_logger - logger = get_logger(__name__) # Mapping from IDP ID to the name in the URL on the blueprint (see below). @@ -97,6 +99,15 @@ def provider_info(idp_id): if "fence" in idps: blueprint_api.add_resource(FenceLogin, "/fence", strict_slashes=False) blueprint_api.add_resource(FenceCallback, "/fence/login", strict_slashes=False) + fence_idp_url = config["OPENID_CONNECT"].get("fence", {}).get("api_base_url") + # Check if the fence IDP is a shibboleth provider, in which case we want to add + # an endpoint on this fence which forwards to the Shibboleth discovery feed + # endpoint ("DiscoFeed") on the IDP fence. + if fence_idp_url: + if get_disco_feed(): + blueprint_api.add_resource( + FenceDownstreamIDPs, "/downstream-idps", strict_slashes=False + ) if "google" in idps: blueprint_api.add_resource(GoogleLogin, "/google", strict_slashes=False) @@ -123,4 +134,5 @@ def provider_info(idp_id): blueprint_api.add_resource( ShibbolethCallback, "/shib/login", strict_slashes=False ) + return blueprint diff --git a/fence/blueprints/login/fence_login.py b/fence/blueprints/login/fence_login.py index 78aa80b997..7e9daef116 100644 --- a/fence/blueprints/login/fence_login.py +++ b/fence/blueprints/login/fence_login.py @@ -1,13 +1,19 @@ +from cdislogging import get_logger import flask from flask_restful import Resource +import requests from fence.auth import login_user from fence.blueprints.login.redirect import validate_redirect +from fence.config import config from fence.errors import Unauthorized from fence.jwt.validate import validate_jwt from fence.models import IdentityProvider +logger = get_logger(__name__) + + class FenceLogin(Resource): """ For ``/login/fence`` endpoint. @@ -71,3 +77,59 @@ def get(self): if "redirect" in flask.session: return flask.redirect(flask.session.get("redirect")) return flask.jsonify({"username": username}) + + +class FenceDownstreamIDPs(Resource): + """ + For ``/login/downstream-idps`` endpoint. + + Should only be enabled if the fence IDP is using shibboleth. + """ + + def get(self): + """Handle ``GET /login/downstream-idps``.""" + content = get_disco_feed() + if not content: + response = flask.jsonify( + {"error": "couldn't reach endpoint on shibboleth provider"} + ) + return response, 500 + return flask.jsonify(content) + + +def get_disco_feed(): + """ + For fence instances which point to a fence instance deployed with shibboleth IDP(s), + we want to list the available downstream IDPs that could be used for shibboleth + login. The `entityID` from the DiscoFeed can be provided to the /login/shib + endpoint, e.g. (without urlencoding): + + /login/shib?shib_idp=urn:mace:incommon:uchicago.edu + + where `urn:mace:incommon:uchicago.edu` is the `entityID` according to shibboleth. + + Return: + dict: json response from the /Shibboleth.sso/DiscoFeed endpoint on the IDP fence + """ + fence_idp_url = config["OPENID_CONNECT"].get("fence", {}).get("api_base_url") + if not fence_idp_url: + return None + disco_feed_url = fence_idp_url.rstrip("/") + "/Shibboleth.sso/DiscoFeed" + response = requests.get(disco_feed_url, timeout=3) + if response.status_code != 200: + # if it's 404 that's fine---just no shibboleth. otherwise there could be an + # actual problem + if response.status_code != 404: + logger.error( + "got weird response ({}) from the IDP fence shibboleth disco feed ({})" + .format(response.status_code, disco_feed_url) + ) + return None + try: + return response.json() + except ValueError: + logger.error( + "didn't get JSON in response from IDP fence shibboleth disco feed ({})" + .format(disco_feed_url) + ) + return None diff --git a/tests/login/test_fence_login.py b/tests/login/test_fence_login.py index c36b446549..6c17143cc4 100644 --- a/tests/login/test_fence_login.py +++ b/tests/login/test_fence_login.py @@ -1,6 +1,8 @@ +from collections import OrderedDict + from addict import Dict from authutils.oauth2.client import OAuthClient -from collections import OrderedDict +import mock import pytest import fence @@ -48,7 +50,7 @@ def config_idp_in_client( yield Dict( client_id=config["OPENID_CONNECT"]["fence"]["client_id"], - client_secret=config["OPENID_CONNECT"]["fence"]["client_id"], + client_secret=config["OPENID_CONNECT"]["fence"]["client_secret"], ) app.keypairs = saved_keypairs From dd97237afd7f000a2290d125e29fa6fb7e73bf39 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Wed, 26 Jun 2019 14:04:47 -0500 Subject: [PATCH 4/7] feat(incommon): apply style --- fence/blueprints/login/__init__.py | 5 ++++- fence/blueprints/login/fence_login.py | 10 ++++++---- fence/blueprints/login/shib.py | 3 +-- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/fence/blueprints/login/__init__.py b/fence/blueprints/login/__init__.py index cff768d7a9..85cc14aaa2 100644 --- a/fence/blueprints/login/__init__.py +++ b/fence/blueprints/login/__init__.py @@ -11,7 +11,10 @@ import requests from fence.blueprints.login.fence_login import ( - FenceLogin, FenceCallback, FenceDownstreamIDPs, get_disco_feed + FenceLogin, + FenceCallback, + FenceDownstreamIDPs, + get_disco_feed, ) from fence.blueprints.login.google import GoogleLogin, GoogleCallback from fence.blueprints.login.shib import ShibbolethLogin, ShibbolethCallback diff --git a/fence/blueprints/login/fence_login.py b/fence/blueprints/login/fence_login.py index 7e9daef116..e64606aac1 100644 --- a/fence/blueprints/login/fence_login.py +++ b/fence/blueprints/login/fence_login.py @@ -121,15 +121,17 @@ def get_disco_feed(): # actual problem if response.status_code != 404: logger.error( - "got weird response ({}) from the IDP fence shibboleth disco feed ({})" - .format(response.status_code, disco_feed_url) + "got weird response ({}) from the IDP fence shibboleth disco feed ({})".format( + response.status_code, disco_feed_url + ) ) return None try: return response.json() except ValueError: logger.error( - "didn't get JSON in response from IDP fence shibboleth disco feed ({})" - .format(disco_feed_url) + "didn't get JSON in response from IDP fence shibboleth disco feed ({})".format( + disco_feed_url + ) ) return None diff --git a/fence/blueprints/login/shib.py b/fence/blueprints/login/shib.py index a01592738c..ae04058762 100644 --- a/fence/blueprints/login/shib.py +++ b/fence/blueprints/login/shib.py @@ -35,8 +35,7 @@ def get(self): actual_redirect = config["BASE_URL"] + "/login/shib/login" return flask.redirect(config["SSO_URL"] + actual_redirect) return flask.redirect( - config["BASE_URL"] - + "/Shibboleth.sso/Login?entityID={}".format(entityID) + config["BASE_URL"] + "/Shibboleth.sso/Login?entityID={}".format(entityID) ) From 480df9091bc7ee9ca75a69b894617626c06bfd38 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Wed, 26 Jun 2019 14:29:51 -0500 Subject: [PATCH 5/7] feat(incommon): improve logic to handle idps list --- fence/blueprints/login/__init__.py | 19 ++++++------------ fence/blueprints/login/fence_login.py | 29 +++++++++++++++++---------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/fence/blueprints/login/__init__.py b/fence/blueprints/login/__init__.py index 85cc14aaa2..0cf5d4fc7d 100644 --- a/fence/blueprints/login/__init__.py +++ b/fence/blueprints/login/__init__.py @@ -11,10 +11,7 @@ import requests from fence.blueprints.login.fence_login import ( - FenceLogin, - FenceCallback, - FenceDownstreamIDPs, - get_disco_feed, + FenceLogin, FenceCallback, FenceDownstreamIDPs, get_disco_feed ) from fence.blueprints.login.google import GoogleLogin, GoogleCallback from fence.blueprints.login.shib import ShibbolethLogin, ShibbolethCallback @@ -102,15 +99,11 @@ def provider_info(idp_id): if "fence" in idps: blueprint_api.add_resource(FenceLogin, "/fence", strict_slashes=False) blueprint_api.add_resource(FenceCallback, "/fence/login", strict_slashes=False) - fence_idp_url = config["OPENID_CONNECT"].get("fence", {}).get("api_base_url") - # Check if the fence IDP is a shibboleth provider, in which case we want to add - # an endpoint on this fence which forwards to the Shibboleth discovery feed - # endpoint ("DiscoFeed") on the IDP fence. - if fence_idp_url: - if get_disco_feed(): - blueprint_api.add_resource( - FenceDownstreamIDPs, "/downstream-idps", strict_slashes=False - ) + # `/downstream-idps` will forward to the `/Shibboleth.sso/DiscoFeed` endpoint on + # the fence IDP if it's available. otherwise it will just 404 + blueprint_api.add_resource( + FenceDownstreamIDPs, "/downstream-idps", strict_slashes=False + ) if "google" in idps: blueprint_api.add_resource(GoogleLogin, "/google", strict_slashes=False) diff --git a/fence/blueprints/login/fence_login.py b/fence/blueprints/login/fence_login.py index e64606aac1..d9292aba7c 100644 --- a/fence/blueprints/login/fence_login.py +++ b/fence/blueprints/login/fence_login.py @@ -6,7 +6,7 @@ from fence.auth import login_user from fence.blueprints.login.redirect import validate_redirect from fence.config import config -from fence.errors import Unauthorized +from fence.errors import Unauthorized, NotFound from fence.jwt.validate import validate_jwt from fence.models import IdentityProvider @@ -88,15 +88,17 @@ class FenceDownstreamIDPs(Resource): def get(self): """Handle ``GET /login/downstream-idps``.""" - content = get_disco_feed() - if not content: + try: + content = get_disco_feed() + except EnvironmentError: response = flask.jsonify( {"error": "couldn't reach endpoint on shibboleth provider"} ) return response, 500 + if not content: + raise NotFound("this endpoint is unavailable") return flask.jsonify(content) - def get_disco_feed(): """ For fence instances which point to a fence instance deployed with shibboleth IDP(s), @@ -109,8 +111,14 @@ def get_disco_feed(): where `urn:mace:incommon:uchicago.edu` is the `entityID` according to shibboleth. Return: - dict: json response from the /Shibboleth.sso/DiscoFeed endpoint on the IDP fence + Optional[dict]: + json response from the /Shibboleth.sso/DiscoFeed endpoint on the IDP fence; + or None if there is no fence IDP or if it returns 404 for DiscoFeed + + Raises: + EnvironmentError: if the response is bad """ + # must be configured for fence IDP fence_idp_url = config["OPENID_CONNECT"].get("fence", {}).get("api_base_url") if not fence_idp_url: return None @@ -121,17 +129,16 @@ def get_disco_feed(): # actual problem if response.status_code != 404: logger.error( - "got weird response ({}) from the IDP fence shibboleth disco feed ({})".format( - response.status_code, disco_feed_url - ) + "got weird response ({}) from the IDP fence shibboleth disco feed ({})" + .format(response.status_code, disco_feed_url) ) + raise EnvironmentError("unexpected response from fence IDP") return None try: return response.json() except ValueError: logger.error( - "didn't get JSON in response from IDP fence shibboleth disco feed ({})".format( - disco_feed_url - ) + "didn't get JSON in response from IDP fence shibboleth disco feed ({})" + .format(disco_feed_url) ) return None From d95472892559007c9f7e70e0d60ed77fbdaa8d95 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Wed, 26 Jun 2019 15:52:41 -0500 Subject: [PATCH 6/7] feat(incommon): tests --- fence/blueprints/login/fence_login.py | 13 +++-- tests/login/test_fence_login.py | 75 +++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/fence/blueprints/login/fence_login.py b/fence/blueprints/login/fence_login.py index d9292aba7c..c63479cb5e 100644 --- a/fence/blueprints/login/fence_login.py +++ b/fence/blueprints/login/fence_login.py @@ -91,10 +91,12 @@ def get(self): try: content = get_disco_feed() except EnvironmentError: - response = flask.jsonify( - {"error": "couldn't reach endpoint on shibboleth provider"} + return flask.Response( + response=flask.jsonify( + {"error": "couldn't reach endpoint on shibboleth provider"} + ), + status=500, ) - return response, 500 if not content: raise NotFound("this endpoint is unavailable") return flask.jsonify(content) @@ -123,7 +125,10 @@ def get_disco_feed(): if not fence_idp_url: return None disco_feed_url = fence_idp_url.rstrip("/") + "/Shibboleth.sso/DiscoFeed" - response = requests.get(disco_feed_url, timeout=3) + try: + response = requests.get(disco_feed_url, timeout=3) + except requests.RequestException: + raise EnvironmentError("couldn't reach fence IDP") if response.status_code != 200: # if it's 404 that's fine---just no shibboleth. otherwise there could be an # actual problem diff --git a/tests/login/test_fence_login.py b/tests/login/test_fence_login.py index 6c17143cc4..fe702617dd 100644 --- a/tests/login/test_fence_login.py +++ b/tests/login/test_fence_login.py @@ -4,6 +4,7 @@ from authutils.oauth2.client import OAuthClient import mock import pytest +import requests import fence from fence.config import config @@ -80,3 +81,77 @@ def test_redirect_login_fence(app, client, config_idp_in_client): assert r.status_code == 302 assert "/oauth2/authorize" in r.location assert config["OPENID_CONNECT"]["fence"]["api_base_url"] in r.location + + +def test_downstream_idps_no_idp(app, client): + """ + If we don't include the config here, then the client doesn't have any fence IDP, so + this endpoint should return 404. + """ + response = client.get("/login/downstream-idps") + assert response.status_code == 404 + + +def test_downstream_idps_no_shibboleth(app, client, config_idp_in_client): + """ + If we include the config pointing to a fence IDP but the IDP fence doesn't have + shibboleth, that request will 404, and this request should also return 404. + """ + + def mock_get_404(*args, **kwargs): + mocked_response = mock.MagicMock(requests.Response) + mocked_response.status_code = 404 + return mocked_response + + with mock.patch("fence.blueprints.login.fence_login.requests.get", mock_get_404): + response = client.get("/login/downstream-idps") + assert response.status_code == 404 + + +def test_downstream_idps(app, client, config_idp_in_client): + """ + Test that if we mock the request to `/Shibboleth.sso/DiscoFeed` on the IDP fence, + this client fence will correctly return the same response from + `/login-downstream-idps`. + """ + entityID = "urn:mace:incommon:uchicago.edu" + + def mock_get(*args, **kwargs): + mocked_response = mock.MagicMock(requests.Response) + mocked_response.status_code = 200 + mocked_response.json.return_value = [{ + "entityID": entityID, + "DisplayNames": [ + { + "value": "University of Chicago", + "lang": "en" + } + ], + "Descriptions": [ + { + "value": "The University of Chicago Web Single Sign-On servce", + "lang": "en" + } + ], + "PrivacyStatementURLs": [ + { + "value": "https://its.uchicago.edu/acceptable-use-policy/", + "lang": "en" + } + ], + "Logos": [ + { + "value": "https://shibboleth2.uchicago.edu/idp/shib_img/idplogo.png", + "height": "83", + "width": "350", + "lang": "en" + } + ] + }] + return mocked_response + + with mock.patch("fence.blueprints.login.fence_login.requests.get", mock_get): + response = client.get("/login/downstream-idps") + assert len(response.json) == 1 + assert [entity for entity in response.json if entity["entityID"] == entityID] + assert response.status_code == 200 From 6d7b83f73c1652fe061d3bf31ef56997e2eae535 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Wed, 26 Jun 2019 16:41:41 -0500 Subject: [PATCH 7/7] feat(incommon): proxy pass DiscoFeed --- DockerfileShib | 3 +++ dockerrunshib.bash | 1 + 2 files changed, 4 insertions(+) diff --git a/DockerfileShib b/DockerfileShib index 821afa930c..77b79f0e49 100644 --- a/DockerfileShib +++ b/DockerfileShib @@ -64,6 +64,9 @@ RUN ln -s /fence/wsgi.py /var/www/fence/wsgi.py \ require shibboleth\n\ \n\ \n\ + \n\ + ProxyPass "SHIBBOLETH_PROVIDER/Shibboleth.sso/DiscoFeed" timeout=5\n\ + \n\ ErrorLog ${APACHE_LOG_DIR}/error.log\n\ LogLevel info\n\ LogFormat "%{X-Forwarded-For}i %l %{X-UserId}i %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-agent}i\"" aws\n\ diff --git a/dockerrunshib.bash b/dockerrunshib.bash index 1ad35e65b7..74d6ffb724 100755 --- a/dockerrunshib.bash +++ b/dockerrunshib.bash @@ -20,5 +20,6 @@ if [ -f /fence/jwt-keys.tar ]; then fi service shibd start sed -i "s/ServerName SERVERNAME/ServerName https:\/\/$HOSTNAME/g" /etc/apache2/sites-available/fence.conf +sed -i "s/SHIBBOLETH_PROVIDER/$SHIBBOLETH_PROVIDER/g" /etc/apache2/sites-available/fence.conf rm -rf /var/run/apache2/apache2.pid /usr/sbin/apache2ctl -D FOREGROUND