Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat/incommon: specify downstream IDP for shibboleth login #652

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

rudyardrichter
Copy link
Contributor

@rudyardrichter rudyardrichter commented Jun 25, 2019

New Features

  • Add the ?shib_idp parameter to /login/shib (and also /oauth2/authorize when the shibboleth IDP is in use), which allows the client to specify which downstream identity provider to use for login.
    • The shib_idp parameter should be an entityID corresponding to the IDP in Shibboleth; for example, shib_idp=urn:mace:incommon:uchicago.edu or shib_idp=urn:mace:incommon:nih.gov.
    • The available logins are listed by Shibboleth, at /Shibboleth.sso/DiscoFeed.

@PlanXCyborg
Copy link
Contributor

PlanXCyborg commented Jun 25, 2019

This PR contains code that is not formatted correctly according to black. Run black on your code before merging.

Expand the full diff to see formatting changes
--- fence/blueprints/login/__init__.py
+++ blackened
@@ -9,11 +9,14 @@
 from cdislogging import get_logger
 import flask
 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
 from fence.blueprints.login.microsoft import MicrosoftLogin, MicrosoftCallback
 from fence.blueprints.login.orcid import ORCIDLogin, ORCIDCallback

--- fence/blueprints/login/fence_login.py
+++ blackened
@@ -99,10 +99,11 @@
             )
         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),
     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
@@ -132,19 +133,21 @@
     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)
+                "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
 

--- fence/blueprints/login/shib.py
+++ blackened
@@ -33,12 +33,14 @@
         actual_redirect = config["BASE_URL"] + "/login/shib/login"
         if not entityID or entityID == "urn:mace:incommon:nih.gov":
             # default to SSO_URL from the config which should be NIH login
             return flask.redirect(config["SSO_URL"] + actual_redirect)
         return flask.redirect(
-            config["BASE_URL"] + "/Shibboleth.sso/Login?entityID={}&target={}"
-            .format(entityID, actual_redirect)
+            config["BASE_URL"]
+            + "/Shibboleth.sso/Login?entityID={}&target={}".format(
+                entityID, actual_redirect
+            )
         )
 
 
 class ShibbolethCallback(Resource):
     def get(self):

--- tests/login/test_fence_login.py
+++ blackened
@@ -117,39 +117,36 @@
     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"
+        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",
                     }
                 ],
-            "Descriptions": [
-                {
-                    "value": "The University of Chicago Web Single Sign-On servce",
-                    "lang": "en"
+                "PrivacyStatementURLs": [
+                    {
+                        "value": "https://its.uchicago.edu/acceptable-use-policy/",
+                        "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",
                     }
                 ],
-            "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

This formatting comment was generated automatically by a script in uc-cdis/wool.

@coveralls
Copy link

coveralls commented Jun 25, 2019

Pull Request Test Coverage Report for Build 7201

  • 32 of 54 (59.26%) changed or added relevant lines in 4 files are covered.
  • 6 unchanged lines in 2 files lost coverage.
  • Overall coverage remained the same at ?%

Changes Missing Coverage Covered Lines Changed/Added Lines %
fence/blueprints/oauth2.py 0 2 0.0%
fence/blueprints/login/fence_login.py 24 33 72.73%
fence/blueprints/login/shib.py 4 15 26.67%
Files with Coverage Reduction New Missed Lines %
fence/blueprints/well_known.py 1 95.0%
fence/jwt/keys.py 5 83.08%
Totals Coverage Status
Change from base Build 7179: 0.0%
Covered Lines:
Relevant Lines: 0

💛 - Coveralls

@rudyardrichter rudyardrichter force-pushed the feat/incommon branch 2 times, most recently from 6d7b83f to d954728 Compare June 26, 2019 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants