diff --git a/README.md b/README.md index 2c92fceaa..6cecd77e0 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,7 @@ Fence is a core service of the Gen3 stack that has multiple capabilities: 1. [API Documentation](#API-documentation) 1. [Terminologies](#Terminologies) -1. [Identity Providers](#identity-provider) +1. [Identity Providers](#identity-providers) 1. [OIDC & OAuth2](#oidc--oauth2) 1. [Accessing Data](#accessing-data) 1. [Setup](#setup) @@ -95,13 +95,11 @@ Relying Party - an OAuth 2.0 Client which uses (requests) OpenID Connect. Fence can be configured to support different Identity Providers (IdPs) for AuthN. At the moment, supported IDPs include: - Google -- Shibboleth +- [Shibboleth](docs/shibboleth.md) - NIH iTrust - InCommon - eduGAIN -Note: the Shibboleth dockerfile image is at https://quay.io/repository/cdis/fence-shib and is NOT compatible with python 3/the latest fence. - ## OIDC & OAuth2 Fence acts as a central broker that supports multiple IdPs. diff --git a/docs/google_architecture.md b/docs/google_architecture.md index a5c7ff49e..e2cfee492 100644 --- a/docs/google_architecture.md +++ b/docs/google_architecture.md @@ -74,7 +74,7 @@ Google supports a bucket-level configuration called ["Requester Pays"](https://c > Without requester pays enabled, the Google project the Google Bucket is in gets billed. -The Data Access methods [Signed URLs](#signed-urls) and [Temporary Service Account Credentials](#temporary-service-account-credentials) support accessing requester pays buckets with some additional configuration and considerations (noteably: how it affects end-users). +The Data Access methods [Signed URLs](#signed-urls) and [Temporary Service Account Credentials](#temporary-service-account-credentials) support accessing requester pays buckets with some additional configuration and considerations (notably: how it affects end-users). In order to fully understand the options for requester pays support, it's important to first understand the technical steps to get any of these Google Data Access methods to work, as detailed in the library Fence uses for Google API interactions, [cirrus](https://github.com/uc-cdis/cirrus). It would also be useful to read through the access method details for [Signed URLs](#signed-urls) and [Temporary Service Account Credentials](#temporary-service-account-credentials). @@ -133,7 +133,7 @@ If you want Fence to automatically attempt to provide the necessary permissions Example for Google's Cloud Storage SDK `gsutil`: ```bash -# activate the temporary service account credentials recieved from Fence +# activate the temporary service account credentials received from Fence # this assumes the creds are saved in a file named `creds.json` gcloud auth activate-service-account --key-file ./creds.json @@ -179,7 +179,7 @@ Each Client Service Account is a member in the User's Proxy Group, meaning it ha > WARNING: By default, Google Service Account Keys have an expiration of 10 years. To create a more manageable and secure expiration you must manually "expire" the keys by deleting them with a cronjob (once they are alive longer than a configured expiration). Fence's command line tool `fence-create` has a function for expiring keys that you should run on a schedule. Check out `fence-create google-manage-keys --help` -Note that internally, we handle users themselves generating these temporary credentials differently. A user is given a **Primary Service Account** which is essentially the same thing as a Client Service Account, but is meant for use solely by the user themself (e.g. not going through a client or outside applcation but hitting the API directly). +Note that internally, we handle users themselves generating these temporary credentials differently. A user is given a **Primary Service Account** which is essentially the same thing as a Client Service Account, but is meant for use solely by the user themself (e.g. not going through a client or outside application but hitting the API directly). At the moment, we track keys generated for the Primary Service Account in our db. Client Service Accounts are expired by checking their creation time through Google's API. @@ -207,7 +207,7 @@ A user logs into fence with their eRA Commons ID. To get access to data through --- -Google Account Linking is achieved by sending the user through the beginning of the OIDC flow with Google. The user is redirected to a Google Login page and whichever account they succesfully log in to becomes linked to their fence identity. +Google Account Linking is achieved by sending the user through the beginning of the OIDC flow with Google. The user is redirected to a Google Login page and whichever account they successfully log in to becomes linked to their fence identity. ![Google Account Linking](images/g_accnt_link.png) @@ -227,7 +227,7 @@ This allows a user to create their own personal Google Cloud Project and registe This method also requires Fence to have access to that user's Google project. Fence is then able to monitor the project for any anomalies that may unintentionally provide data access to entities who should not have access. -In order to register a Service Account, *all* users in the Google Project must have already gone through Google Account Linking (as desribed above). After that, any user on the project can attempt to register a service account with fence. +In order to register a Service Account, *all* users in the Google Project must have already gone through Google Account Linking (as described above). After that, any user on the project can attempt to register a service account with fence. --- @@ -247,7 +247,7 @@ Projects are always validated against the following checks: * Checks if the Google project either has no parent organization, or if it does, it is included on the whitelist of parent organizations (defined in Fence config). (The reason for this logic is that user permissions can be inherited from a parent organization, but the Fence SA is only given permission on the project level and thus can only read project-specific IAM policies. Any inherited policies will not be available during validation. Therefore, if there is no parent org then there is nothing to worry about, but if there is, then we must trust the parent org to have properly set its permissions.) * Google Project only has valid member types * Key: `valid_member_types` - * Checks if the Google project ony has members that are User Accounts or Service Accounts. + * Checks if the Google project only has members that are User Accounts or Service Accounts. * Google Project members exist in fence * Key: `members_exist_in_fence` * Checks if the User members on the Google project exist in the fence DB diff --git a/fence/__init__.py b/fence/__init__.py index b6ce220df..43e93738e 100644 --- a/fence/__init__.py +++ b/fence/__init__.py @@ -248,7 +248,13 @@ def _set_authlib_cfgs(app): def _setup_oidc_clients(app): - enabled_idp_ids = list(config["ENABLED_IDENTITY_PROVIDERS"]["providers"].keys()) + if "LOGIN_OPTIONS" in config: + enabled_idp_ids = [option["idp"] for option in config["LOGIN_OPTIONS"]] + else: + # fall back on "providers" + enabled_idp_ids = list( + config.get("ENABLED_IDENTITY_PROVIDERS", {}).get("providers", {}).keys() + ) oidc = config.get("OPENID_CONNECT", {}) # Add OIDC client for Google if configured. diff --git a/fence/auth.py b/fence/auth.py index c89ea2633..9e7956d84 100644 --- a/fence/auth.py +++ b/fence/auth.py @@ -145,7 +145,16 @@ def wrapper(*args, **kwargs): return f(*args, **kwargs) eppn = None - enable_shib = "shibboleth" in config.get("ENABLED_IDENTITY_PROVIDERS", []) + if "LOGIN_OPTIONS" in config: + enable_shib = "shibboleth" in [ + option["idp"] for option in config["LOGIN_OPTIONS"] + ] + else: + # fall back on "providers" + enable_shib = "shibboleth" in config.get( + "ENABLED_IDENTITY_PROVIDERS", {} + ).get("providers", {}) + if enable_shib and "SHIBBOLETH_HEADER" in config: eppn = flask.request.headers.get(config["SHIBBOLETH_HEADER"]) diff --git a/fence/blueprints/login/__init__.py b/fence/blueprints/login/__init__.py index 382a3aa41..2b63a9923 100644 --- a/fence/blueprints/login/__init__.py +++ b/fence/blueprints/login/__init__.py @@ -6,7 +6,11 @@ the endpoints for each provider. """ +from authlib.common.urls import add_params_to_uri import flask +import requests + +from cdislogging import get_logger from fence.blueprints.login.fence_login import FenceLogin, FenceCallback from fence.blueprints.login.google import GoogleLogin, GoogleCallback @@ -18,8 +22,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). @@ -45,23 +47,6 @@ def make_login_blueprint(app): ValueError: if app is not amenably configured """ - try: - default_idp = config["ENABLED_IDENTITY_PROVIDERS"]["default"] - idps = config["ENABLED_IDENTITY_PROVIDERS"]["providers"] - except KeyError as e: - logger.warn( - "app not configured correctly with ENABLED_IDENTITY_PROVIDERS:" - " missing {}".format(str(e)) - ) - default_idp = None - idps = {} - - # check if google is configured as a client. we will at least need a - # a callback if it is - google_client_exists = ( - "OPENID_CONNECT" in config and "google" in config["OPENID_CONNECT"] - ) - blueprint = flask.Blueprint("login", __name__) blueprint_api = RestfulApi(blueprint) @@ -70,73 +55,224 @@ def default_login(): """ The default root login route. """ + # default login option + if "DEFAULT_LOGIN_IDP" in config: + default_idp = config["DEFAULT_LOGIN_IDP"] + elif "default" in config.get("ENABLED_IDENTITY_PROVIDERS", {}): + # fall back on ENABLED_IDENTITY_PROVIDERS.default + default_idp = config["ENABLED_IDENTITY_PROVIDERS"]["default"] + else: + logger.warn("DEFAULT_LOGIN_IDP not configured") + default_idp = None - def absolute_login_url(provider_id): - base_url = config["BASE_URL"].rstrip("/") - return base_url + "/login/{}".format(IDP_URL_MAP[provider_id]) - - def provider_info(idp_id): - if not idp_id: - return { - "id": None, - "name": None, - "url": None, - "desc": None, - "secondary": False, + # other login options + if "LOGIN_OPTIONS" in config: + login_options = config["LOGIN_OPTIONS"] + elif "providers" in config.get("ENABLED_IDENTITY_PROVIDERS", {}): + # fall back on "providers" and convert to "login_options" format + enabled_providers = config["ENABLED_IDENTITY_PROVIDERS"]["providers"] + login_options = [ + { + "name": details.get("name"), + "idp": idp, + "desc": details.get("desc"), + "secondary": details.get("secondary"), } - return { - "id": idp_id, - "name": idps[idp_id]["name"], - "url": absolute_login_url(idp_id), - "desc": idps[idp_id].get("desc", None), - "secondary": idps[idp_id].get("secondary", False), + for idp, details in enabled_providers.items() + ] + else: + logger.warn("LOGIN_OPTIONS not configured") + login_options = [] + + def absolute_login_url(provider_id, shib_idp=None): + try: + base_url = config["BASE_URL"].rstrip("/") + login_url = base_url + "/login/{}".format(IDP_URL_MAP[provider_id]) + except KeyError as e: + raise InternalError( + "identity provider misconfigured: {}".format(str(e)) + ) + + if shib_idp: + login_url = add_params_to_uri( + login_url, {"idp": "shibboleth", "shib_idp": shib_idp} + ) + return login_url + + def provider_info(login_details): + info = { + # "id" deprecated, replaced by "idp" + "id": login_details["idp"], + "idp": login_details["idp"], + "name": login_details["name"], + # "url" deprecated, replaced by "urls" + "url": absolute_login_url(login_details["idp"]), + "desc": login_details.get("desc", None), + "secondary": login_details.get("secondary", False), } + # handle Shibboleth IDPs + if login_details["idp"] == "fence" and "shib_idps" in login_details: + + # get list of all available shib IDPs + if not hasattr(app, "all_shib_idps"): + app.all_shib_idps = get_all_shib_idps() + + requested_shib_idps = login_details["shib_idps"] + if requested_shib_idps == "*": + shib_idps = app.all_shib_idps + elif isinstance(requested_shib_idps, list): + # get the display names for each requested shib IDP + shib_idps = [] + for requested_shib_idp in requested_shib_idps: + shib_idp = next( + ( + available_shib_idp + for available_shib_idp in app.all_shib_idps + if available_shib_idp["idp"] == requested_shib_idp + ), + None, + ) + if not shib_idp: + raise InternalError( + 'Requested shib_idp "{}" does not exist'.format( + requested_shib_idp + ) + ) + shib_idps.append(shib_idp) + else: + raise InternalError( + 'fence provider misconfigured: "shib_idps" must be a list or "*", got {}'.format( + requested_shib_idps + ) + ) + + info["urls"] = [ + { + "name": shib_idp["name"], + "url": absolute_login_url( + login_details["idp"], shib_idp["idp"] + ), + } + for shib_idp in shib_idps + ] + + # non-Shibboleth provider + else: + info["urls"] = [ + { + "name": login_details["name"], + "url": absolute_login_url(login_details["idp"]), + } + ] + + return info + try: - all_provider_info = [provider_info(idp_id) for idp_id in list(idps.keys())] - default_provider_info = provider_info(default_idp) + all_provider_info = [ + provider_info(login_details) for login_details in login_options + ] except KeyError as e: - raise InternalError("identity providers misconfigured: {}".format(str(e))) + raise InternalError("login options misconfigured: {}".format(e)) + + # if several login_options are defined for this default IDP, will + # default to the first one: + default_provider_info = next( + (info for info in all_provider_info if info["idp"] == default_idp), None + ) + if not default_provider_info: + raise InternalError("default provider misconfigured") return flask.jsonify( {"default_provider": default_provider_info, "providers": all_provider_info} ) # Add identity provider login routes for IDPs enabled in the config. + configured_idps = config["OPENID_CONNECT"].keys() - if "fence" in idps: + if "fence" in configured_idps: blueprint_api.add_resource(FenceLogin, "/fence", strict_slashes=False) blueprint_api.add_resource(FenceCallback, "/fence/login", strict_slashes=False) - if "google" in idps: + if "google" in configured_idps: blueprint_api.add_resource(GoogleLogin, "/google", strict_slashes=False) - - # we can use Google Client and callback here without the login endpoint - # if Google is configured as a client but not in the idps - if "google" in idps or google_client_exists: blueprint_api.add_resource( GoogleCallback, "/google/login", strict_slashes=False ) - if "orcid" in idps: + if "orcid" in configured_idps: blueprint_api.add_resource(ORCIDLogin, "/orcid", strict_slashes=False) blueprint_api.add_resource(ORCIDCallback, "/orcid/login", strict_slashes=False) - if "synapse" in idps: + if "synapse" in configured_idps: blueprint_api.add_resource(SynapseLogin, "/synapse", strict_slashes=False) blueprint_api.add_resource( SynapseCallback, "/synapse/login", strict_slashes=False ) - if "microsoft" in idps: + if "microsoft" in configured_idps: blueprint_api.add_resource(MicrosoftLogin, "/microsoft", strict_slashes=False) blueprint_api.add_resource( MicrosoftCallback, "/microsoft/login", strict_slashes=False ) - if "shibboleth" in idps: + if "shibboleth" in configured_idps: blueprint_api.add_resource(ShibbolethLogin, "/shib", strict_slashes=False) blueprint_api.add_resource( ShibbolethCallback, "/shib/login", strict_slashes=False ) return blueprint + + +def get_all_shib_idps(): + """ + Get the list of all existing Shibboleth IDPs. + This function only returns the information we need to generate login URLs. + + Returns: + list: list of {"idp": "", "name": ""} dictionaries + """ + url = config["OPENID_CONNECT"].get("fence", {}).get("shibboleth_discovery_url") + if not url: + raise InternalError( + "Unable to get list of Shibboleth IDPs: OPENID_CONNECT.fence.shibboleth_discovery_url not configured" + ) + res = requests.get(url) + assert ( + res.status_code == 200 + ), "Unable to get list of Shibboleth IDPs from {}".format(url) + return [ + { + "idp": shib_idp["entityID"], + "name": get_shib_idp_en_name(shib_idp["DisplayNames"]), + } + for shib_idp in res.json() + ] + + +def get_shib_idp_en_name(names): + """ + Returns a name in English for a Shibboleth IDP, or the first available + name if no English name was provided. + + Args: + names (list): list of {"lang": "", "value": ""} dictionaries + Example: + [ + { + "value": "University of Chicago", + "lang": "en" + }, + { + "value": "Universidad de Chicago", + "lang": "es" + } + ] + + Returns: + str: Display name to use for this Shibboleth IDP + """ + for name in names: + if name.get("lang") == "en": + return name["value"] + return names[0]["value"] diff --git a/fence/blueprints/login/fence_login.py b/fence/blueprints/login/fence_login.py index 78aa80b99..a0ff8d293 100644 --- a/fence/blueprints/login/fence_login.py +++ b/fence/blueprints/login/fence_login.py @@ -1,3 +1,4 @@ +from authlib.common.urls import add_params_to_uri import flask from flask_restful import Resource @@ -32,6 +33,17 @@ def get(self): authorization_url, state = flask.current_app.fence_client.generate_authorize_redirect( oauth2_redirect_uri, prompt="login" ) + + # if requesting to login through Shibboleth, add idp and + # shib_idp parameters to the authorization URL + idp = flask.request.args.get("idp") + if idp == "shibboleth": + shib_idp = flask.request.args.get("shib_idp") + if shib_idp: + authorization_url = add_params_to_uri( + authorization_url, {"idp": idp, "shib_idp": shib_idp} + ) + flask.session["state"] = state return flask.redirect(authorization_url) diff --git a/fence/blueprints/login/shib.py b/fence/blueprints/login/shib.py index 4702cec85..45173bd87 100644 --- a/fence/blueprints/login/shib.py +++ b/fence/blueprints/login/shib.py @@ -1,3 +1,4 @@ +from cdislogging import get_logger import flask from flask_restful import Resource @@ -7,6 +8,8 @@ from fence.models import IdentityProvider from fence.config import config +logger = get_logger(__name__) + class ShibbolethLogin(Resource): def get(self): @@ -24,8 +27,22 @@ def get(self): validate_redirect(redirect_url) if redirect_url: flask.session["redirect"] = redirect_url + + # 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") + flask.session["entityID"] = entityID actual_redirect = config["BASE_URL"] + "/login/shib/login" - return flask.redirect(config["SSO_URL"] + actual_redirect) + 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 + ) + ) class ShibbolethCallback(Resource): @@ -33,16 +50,30 @@ def get(self): """ Complete the shibboleth login. """ - if "SHIBBOLETH_HEADER" in config: - eppn = flask.request.headers.get(config["SHIBBOLETH_HEADER"]) - - else: + shib_header = config.get("SHIBBOLETH_HEADER") + if not shib_header: raise InternalError("Missing shibboleth header configuration") - 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: - raise Unauthorized("Please login") + + # eppn stands for eduPersonPrincipalName + username = flask.request.headers.get("eppn") + if not username: + persistent_id = flask.request.headers.get(shib_header) + username = persistent_id.split("!")[-1] if persistent_id else None + if not username: + # some inCommon providers are not returning eppn + # or persistent_id. See PXP-4309 + # print("shib_header", shib_header) + # print("flask.request.headers", flask.request.headers) + raise Unauthorized( + "Unable to retrieve username from Shibboleth results" + ) + + 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" diff --git a/fence/blueprints/oauth2.py b/fence/blueprints/oauth2.py index 7830b9617..e4fe51eed 100644 --- a/fence/blueprints/oauth2.py +++ b/fence/blueprints/oauth2.py @@ -77,6 +77,10 @@ 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": + shib_idp = flask.request.args.get("shib_idp") + if shib_idp: + params["shib_idp"] = shib_idp login_url = add_params_to_uri(login_url, params) return flask.redirect(login_url) diff --git a/fence/config-default.yaml b/fence/config-default.yaml index 25f832ba8..66f1715b9 100644 --- a/fence/config-default.yaml +++ b/fence/config-default.yaml @@ -80,6 +80,9 @@ ENABLE_DB_MIGRATION: true # - Fully configure at least one client so login works # - WARNING: Be careful changing the *_ALLOWED_SCOPES as you can break basic # and optional functionality +# +# See ``fence/blueprints/login/__init__.py`` IDP_URL_MAP for which identity +# providers can be configured. # ////////////////////////////////////////////////////////////////////////////////////// OPENID_CONNECT: # These Google values must be obtained from Google's Cloud Console @@ -128,6 +131,8 @@ OPENID_CONNECT: # WARNING: DO NOT ENABLE IN PRODUCTION (for testing purposes only) mock: false mock_default_user: 'test@example.com' + # this is needed to enable InCommon login, if some LOGIN_OPTIONS are configured with idp=fence and a list of shib_idps: + shibboleth_discovery_url: 'https://login.bionimbus.org/Shibboleth.sso/DiscoFeed' # you can setup up an orcid client here: https://orcid.org/developer-tools orcid: client_id: '' @@ -197,14 +202,51 @@ SESSION_ALLOWED_SCOPES: # ////////////////////////////////////////////////////////////////////////////////////// # LOGIN -# - Modify based on which OIDC client(s) you configured above +# - Modify based on which OIDC provider(s) you configured above # - NOTE: You can have multiple IDPs for users to login with, but one has to be set # as the default # ////////////////////////////////////////////////////////////////////////////////////// -# Login url for identity provider (IDP): -# Google? Use: '{{BASE_URL}}/login/google' -# Multi-tenant fence (e.g. another fence instance)? Use: '{{BASE_URL}}/login/fence' -# Sibboleth? Use: '{{BASE_URL}}/login/shib' + +# List of enabled login options (used by data-portal to display login buttons). +# Each option must be configured with a "name" and an "idp". +# "idp" must be a configured provider in OPENID_CONNECT section. +# Multiple options can be configured with the same idp. +# "desc" and "secondary" are optional. +# If the idp is fence, a list of "shib_idps" can be configured for +# InCommon login. +LOGIN_OPTIONS: + - name: 'Login from Google' + idp: google + # desc: 'description' + # secondary: True + # - name: 'NIH Login' + # idp: fence + # - name: 'Orcid Login' + # idp: orcid + # - name: 'Microsoft Login' + # idp: microsoft + # - name: 'InCommon Login' + # idp: fence + # # "shib_idps" can be '*' or a list of one or more entity IDs + # shib_idps: + # - urn:mace:incommon:nih.gov + # - urn:mace:incommon:uchicago.edu +# The following can be used for shibboleth login, simply uncomment. +# NOTE: Don't enable shibboleth if the deployment is not protected by +# shibboleth module, the shib module takes care of preventing header +# spoofing. + # - name: 'Shibboleth Login' + # idp: shibboleth + +# Default login provider: +# - must be configured in LOGIN_OPTIONS and OPENID_CONNECT +# - if several options in LOGIN_OPTIONS are defined for this IDP, will default +# to the first one. +# Default login URL: +# - Google? Use: '{{BASE_URL}}/login/google' +# - Multi-tenant fence (e.g. another fence instance)? Use: '{{BASE_URL}}/login/fence' +# - Sibboleth? Use: '{{BASE_URL}}/login/shib' +DEFAULT_LOGIN_IDP: google DEFAULT_LOGIN_URL: '{{BASE_URL}}/login/google' # `LOGIN_REDIRECT_WHITELIST` is a list of extra whitelisted URLs which can be redirected @@ -214,38 +256,9 @@ DEFAULT_LOGIN_URL: '{{BASE_URL}}/login/google' # only the domains for the additional desired redirects are necessary here). LOGIN_REDIRECT_WHITELIST: [] -# Which Identity Provider fence will/can use -# -# See ``fence/blueprints/login/__init__.py`` for which identity providers can -# be loaded. -# +### DEPRECATED and replaced by OPENID_CONNECT + LOGIN_OPTIONS configs ENABLED_IDENTITY_PROVIDERS: - # ID for which of the providers to default to - default: 'google' - # Information for identity providers - providers: -# Additional Identity Provider options, simply uncomment: -# -# orcid: -# name: 'Login from ORCID' -# -# microsoft: -# name: 'Login from Microsoft' -# -# fence: -# name: 'Fence Multi-Tenant Login' -# - google: - name: 'Login from Google' -# The following can be used for shibboleth login, simply uncomment. -# -# NOTE: Don't enable shibboleth if the deployment is not protected by -# shibboleth module, the shib module takes care of preventing header -# spoofing. -# -# shibboleth: -# name: 'NIH Login' -# + # ////////////////////////////////////////////////////////////////////////////////////// # LIBRARY CONFIGURATION (authlib & flask) @@ -343,7 +356,7 @@ SUPPORT_EMAIL_FOR_ERRORS: null # ////////////////////////////////////////////////////////////////////////////////////// # SHIBBOLETH -# - Support using `shibboleth` in ENABLED_IDENTITY_PROVIDERS +# - Support using `shibboleth` in LOGIN_OPTIONS # - Contains defaults for using NIH's Login. # ////////////////////////////////////////////////////////////////////////////////////// # assumes shibboleth is deployed under {{BASE_URL}}/shibboleth diff --git a/fence/resources/google/validity.py b/fence/resources/google/validity.py index ea372401e..1bb5d39da 100644 --- a/fence/resources/google/validity.py +++ b/fence/resources/google/validity.py @@ -245,7 +245,7 @@ def check_validity(self, early_return=True, db=None): "INVALID Fence's Monitoring service account does " "NOT have access in project id {}. Monitor needs access to continue " "checking project validity. Exiting early and determining invalid.".format( - self.user_id, self.google_project_id + self.google_project_id ) ) return diff --git a/openapis/swagger.yaml b/openapis/swagger.yaml index c69e10c72..96b6deb79 100644 --- a/openapis/swagger.yaml +++ b/openapis/swagger.yaml @@ -81,9 +81,23 @@ paths: - name: idp required: false in: query - description: Upstream identity provider to use, fallback to the default one if not provided + description: >- + Upstream identity provider to use. Specifying `idp=shibboleth` lets + us specify a `shib_idp`. If no `idp` is specified, defaults to NIH + login. + schema: + type: string + example: 'shibboleth' + - name: shib_idp + required: false + in: query + description: >- + Identifier for the shibboleth IDP. Available identifiers are what + is listed by the `login.bionimbus.org/Shibboleth.sso/DiscoFeed` + endpoint. If no `shib_idp` is specified, defaults to NIH login. schema: type: string + example: 'urn:mace:incommon:uchicago.edu' - name: scope required: false in: query @@ -216,6 +230,61 @@ paths: type: string required: - token + /login/shib: + get: + tags: + - login + summary: Initiate login via shibboleth provider + description: | + NOTE that this endpoint only exists if shibboleth is enabled as an + identity provider in the fence config. Currently this only happens for + `login.bionimbus.org`. + parameters: + - name: shib_idp + required: false + in: query + description: >- + Identifier for the shibboleth IDP. Available identifiers are what + is listed by the `login.bionimbus.org/Shibboleth.sso/DiscoFeed` + endpoint. If no `shib_idp` is specified, defaults to NIH login. + schema: + type: string + example: 'urn:mace:incommon:uchicago.edu' + responses: + 302: + description: redirect to external identity provider + /login/fence: + get: + tags: + - login + summary: Initiate login via Fence provider (multi-tenant Fence) + description: | + NOTE that this endpoint only exists if fence is enabled as an + identity provider in the fence config. + parameters: + - name: idp + required: false + in: query + description: >- + Identifier for the IDP. Specifying `idp=shibboleth` lets us + specify a `shib_idp`. If no `idp` is specified, defaults to + NIH login. + schema: + type: string + example: 'shibboleth' + - name: shib_idp + required: false + in: query + description: >- + Identifier for the shibboleth IDP. Available identifiers are what + is listed by the `login.bionimbus.org/Shibboleth.sso/DiscoFeed` + endpoint. If no `shib_idp` is specified, defaults to NIH login. + schema: + type: string + example: 'urn:mace:incommon:uchicago.edu' + responses: + 302: + description: redirect to external identity provider /jwt/keys: get: tags: diff --git a/tests/link/test_link.py b/tests/link/test_link.py index 23875f821..92cfe9e7c 100644 --- a/tests/link/test_link.py +++ b/tests/link/test_link.py @@ -73,15 +73,10 @@ def test_google_link_redirect_no_google_idp( # Don't include google in the enabled idps, but leave it configured # in the openid connect clients: override_settings = { - "ENABLED_IDENTITY_PROVIDERS": { - # ID for which of the providers to default to. - "default": "fence", - # Information for identity providers. - "providers": { - "fence": {"name": "Fence Multi-Tenant OAuth"}, - "shibboleth": {"name": "NIH Login"}, - }, - }, + "LOGIN_OPTIONS": [ + {"idp": "fence", "name": "Fence Multi-Tenant OAuth"}, + {"idp": "shibboleth", "name": "NIH Login"}, + ], "OPENID_CONNECT": { "google": { "client_id": "123", diff --git a/tests/login/test_default_login.py b/tests/login/test_default_login.py index 0b27d1708..2ef132a69 100644 --- a/tests/login/test_default_login.py +++ b/tests/login/test_default_login.py @@ -1,22 +1,64 @@ +from urllib.parse import urlencode + from fence.config import config def test_default_login(app, client): response_json = client.get("/login").json assert "default_provider" in response_json - assert "providers" in response_json response_default = response_json["default_provider"] - response_providers = response_json["providers"] - idps = config["ENABLED_IDENTITY_PROVIDERS"]["providers"] - default_idp_id = config["ENABLED_IDENTITY_PROVIDERS"]["default"] + configured_logins = config["LOGIN_OPTIONS"] + default_idp = config["DEFAULT_LOGIN_IDP"] + # Check default IDP is correct. - assert response_default["id"] == default_idp_id - assert response_default["name"] == idps[default_idp_id]["name"] - # Check all providers in response: expected ID, expected name, URL actually - # maps correctly to the endpoint on fence. + assert response_default["idp"] == default_idp + names_for_this_idp = [ + login_details["name"] + for login_details in configured_logins + if login_details["idp"] == default_idp + ] + assert response_default["name"] in names_for_this_idp + + +def test_enabled_logins(app, client): + response_json = client.get("/login").json + assert "providers" in response_json + response_providers = response_json["providers"] + configured_logins = config["LOGIN_OPTIONS"] + + # Check all providers in the response have the expected idp, name, URLs, + # desc and secondary information app_urls = [url_map_rule.rule for url_map_rule in app.url_map._rules] - for response_idp in response_providers: - assert response_idp["id"] in idps - assert response_idp["name"] == idps[response_idp["id"]]["name"] - login_url = response_idp["url"].replace(config["BASE_URL"], "") - assert login_url in app_urls + for configured in configured_logins: + # this assumes (idp, name) couples in test config are unique + response_provider = next( + ( + provider + for provider in response_providers + if provider["idp"] == configured["idp"] + and provider["name"] == configured["name"] + ), + None, + ) + assert ( + response_provider + ), 'Configured login option "{}" not in /login response: {}'.format() + if "desc" in configured: + assert response_provider["desc"] == configured["desc"] + if "secondary" in configured: + assert response_provider["secondary"] == configured["secondary"] + if "shib_idps" in configured: + for shib_idp in configured["shib_idps"]: + assert any( + urlencode({"shib_idp": shib_idp}) in url_info["url"] + for url_info in response_provider["urls"] + ), 'shib_idp param "{}", encoded "{}", is not in provider\'s login URLs: {}'.format( + shib_idp, + urlencode({"shib_idp": shib_idp}), + response_provider["urls"], + ) + login_urls = [ + url_info["url"].replace(config["BASE_URL"], "").split("?")[0] + for url_info in response_provider["urls"] + ] + assert all(url in app_urls for url in login_urls) diff --git a/tests/login/test_fence_login.py b/tests/login/test_fence_login.py index c36b44654..da59242e0 100644 --- a/tests/login/test_fence_login.py +++ b/tests/login/test_fence_login.py @@ -48,7 +48,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 @@ -59,7 +59,7 @@ def config_idp_in_client( def test_redirect_oauth2_authorize(app, client, config_idp_in_client): """ Test that the ``/oauth2/authorize`` endpoint on the client fence redirects to the - ``/login/fence`` endpoint, also on the client fence, + ``/login/fence`` endpoint, also on the client fence, in the multi-tenant setup case. """ r = client.post("/oauth2/authorize") diff --git a/tests/test-fence-config.yaml b/tests/test-fence-config.yaml index 970f1470e..a5eb5d5a1 100644 --- a/tests/test-fence-config.yaml +++ b/tests/test-fence-config.yaml @@ -88,6 +88,15 @@ OPENID_CONNECT: client_id: '' client_secret: '' redirect_url: '{{BASE_URL}}/login/microsoft/login/' + fence: + client_id: '' + client_secret: '' + redirect_url: '{{BASE_URL}}/login/fence/login' + shibboleth_discovery_url: 'https://login.bionimbus.org/Shibboleth.sso/DiscoFeed' + shibboleth: + client_id: '' + client_secret: '' + redirect_url: '{{BASE_URL}}/login/shib/login' # these are the *possible* scopes a client can be given, NOT scopes that are # given to all clients. You can be more restrictive during client creation CLIENT_ALLOWED_SCOPES: @@ -132,6 +141,7 @@ SESSION_ALLOWED_SCOPES: # Google? Use: '{{BASE_URL}}/login/google' # Multi-tenant fence (e.g. another fence instance)? Use: '{{BASE_URL}}/login/fence' # Sibboleth? Use: '{{BASE_URL}}/login/shib' +DEFAULT_LOGIN_IDP: google DEFAULT_LOGIN_URL: '{{BASE_URL}}/login/google' # `LOGIN_REDIRECT_WHITELIST` is a list of extra whitelisted URLs which can be redirected @@ -144,27 +154,24 @@ LOGIN_REDIRECT_WHITELIST: [] # See ``fence/blueprints/login/__init__.py`` for which identity providers can # be loaded. # -ENABLED_IDENTITY_PROVIDERS: - # ID for which of the providers to default to - default: 'google' - # Information for identity providers - providers: - google: - name: 'Google Login' - fence: - name: 'Fence Multi-Tenant Login' - orcid: - name: 'Orcid Login' - microsoft: - name: 'Microsoft Login' -# The following can be used for shibboleth login, simply uncomment. -# -# NOTE: Don't enable shibboleth if the deployment is not protected by -# shibboleth module, the shib module takes care of preventing header -# spoofing. -# - shibboleth: - name: 'NIH Login' +LOGIN_OPTIONS: + - name: 'Google Login' + idp: google + desc: 'description' # optional parameter + secondary: True # optional parameter + - name: 'Fence Multi-Tenant Login' + idp: fence + - name: 'InCommon Login' + idp: fence + shib_idps: + - urn:mace:incommon:nih.gov + - urn:mace:incommon:uchicago.edu + - name: 'Orcid Login' + idp: orcid + - name: 'Microsoft Login' + idp: microsoft + - name: 'NIH Login' + idp: shibboleth # ////////////////////////////////////////////////////////////////////////////////////// # LIBRARY CONFIGURATION (authlib & flask) @@ -249,7 +256,7 @@ SUPPORT_EMAIL_FOR_ERRORS: null # ////////////////////////////////////////////////////////////////////////////////////// # SHIBBOLETH -# - Support using `shibboleth` in ENABLED_IDENTITY_PROVIDERS +# - Support using `shibboleth` in LOGIN_OPTIONS # - Contains defaults for using NIH's Login. # ////////////////////////////////////////////////////////////////////////////////////// # assumes shibboleth is deployed under {{BASE_URL}}/shibboleth diff --git a/tests/test_datamodel.py b/tests/test_datamodel.py index 8f9ade077..3284ced70 100644 --- a/tests/test_datamodel.py +++ b/tests/test_datamodel.py @@ -29,7 +29,7 @@ def test_user_delete_cascade(db_session): assert db_session.query(Client).filter_by(client_id=client.client_id).count() == 0 -def test_service_account_relationsips(db_session): +def test_service_account_relationships(db_session): """ test service account tables have proper relationships/fields """