Merge pull request #77 from uc-cdis/feat/keycloak-external-oidc

HP-1361 Feat/keycloak external OIDC

.secrets.baseline

@@ -167,5 +167,5 @@
       }
     ]
   },
-  "generated_at": "2023-09-19T21:25:48Z"
+  "generated_at": "2024-05-07T14:42:22Z"
 }

README.md

@@ -76,6 +76,23 @@ Two methods exist for authenticating a request to the `/token` endpoint:
                 ...
             }
         },
+        {
+            "base_url": "https://non-fence-data.org/",
+            "oidc_client_id": "xxx",
+            "oidc_client_secret": "xxx",
+            "login_options": {
+                "externaldata-keycloak": {
+                    "name": "keycloak Login",
+                    "params": {
+                        "idp": "keycloak",
+                        "auth_url": "auth/realms/xyz/protocol/openid-connect/auth",
+                        "token_url": "auth/realms/xyz/protocol/openid-connect/token",
+                        "id_token_username": "email",
+                        "scope": "openid profile offline_access"
+                   }
+                }
+            }
+        },
         ...
     ]
 }
@@ -88,11 +105,20 @@ Note that IDP IDs (`other-google` and `other-orcid` in the example above) must b
 Also note that the OIDC clients you create must be granted `read-storage` access to all the data in the external
 Data Commons via the data-commons' `user.yaml`.
 
-Finally, the `redirect_uri` property for external OIDC providers is
+The `id_token_username` property for OIDC clients can be configured with `.` in between strings for a nested username inside a token.
+For example if the token jwt has username encoded in the json as `token["context"]["user"]["name"]`:
+We can write this in the parameters as `"id_token_username": "context.user.name"`.
+If nothing is specified, for a fence client the default is `"context.user.name"`, for a non-fence client the default is `"email"`.
+
+
+The `redirect_uri` property for external OIDC providers is
 an optional field that supports sharing OIDC client
 configuration between multiple workspace deployments
 as part of a multi-account application system.
 
+
+Finally, non fence IDPs can be provided given their auth url, token url, and necessary scope as a part part of the `params` of the external IDP.
+
 The key `aggregate_endpoint_allowlist` is an optional key which consists of a list of endpoints that are supported by the `/aggregate` api.
 
 ## Dev-Test

tests/aggregate_test.py

@@ -1,4 +1,6 @@
 import httpx
+import json
+import os
 import uuid
 
 from .conftest import (
@@ -8,6 +10,15 @@
 )
 
 
+def get_number_of_configured_idps():
+    with open(os.environ["SECRET_CONFIG"], "r") as f:
+        external_idps = json.load(f)["external_oidc"]
+    count = 0
+    for provider in external_idps:
+        count += len(provider.get("login_options", {}))
+    return count
+
+
 def test_aggregate_user_user_endpoint(
     app, client, persisted_refresh_tokens, auth_header
 ):
@@ -18,7 +29,7 @@ def test_aggregate_user_user_endpoint(
     """
     res = client.get("/aggregate/user/user", headers=auth_header)
     assert res.status_code == 200
-    assert len(res.json) == 2
+    assert len(res.json) == get_number_of_configured_idps()
 
     default_commons_hostname = app.config["OIDC"]["default"]["commons_hostname"]
     assert default_commons_hostname in res.json
@@ -45,7 +56,7 @@ def test_aggregate_user_user_endpoint_with_filters(
         "/aggregate/user/user?filters=authz&filters=role", headers=auth_header
     )
     assert res.status_code == 200
-    assert len(res.json) == 2
+    assert len(res.json) == get_number_of_configured_idps()
 
     default_commons_hostname = app.config["OIDC"]["default"]["commons_hostname"]
     assert default_commons_hostname in res.json
@@ -95,7 +106,7 @@ def test_aggregate_endpoint_when_one_linked_commons_returns_500(
     respx_mock.get(f"{idp_a_fence_url}/user").mock(return_value=httpx.Response(500))
     res = client.get("/aggregate/user/user", headers=auth_header)
     assert res.status_code == 200
-    assert len(res.json) == 2
+    assert len(res.json) == get_number_of_configured_idps()
 
     default_commons_hostname = app.config["OIDC"]["default"]["commons_hostname"]
     assert default_commons_hostname in res.json
@@ -152,7 +163,7 @@ def test_aggregate_authz_mapping_endpoint_with_no_connected_commons(
     """
     res = client.get("/aggregate/authz/mapping", headers=auth_header)
     assert res.status_code == 200
-    assert len(res.json) == 2
+    assert len(res.json) == get_number_of_configured_idps()
 
     default_commons_hostname = app.config["OIDC"]["default"]["commons_hostname"]
     assert default_commons_hostname in res.json
@@ -180,7 +191,7 @@ def test_aggregate_authz_mapping_endpoint_with_connected_commons(
     """
     res = client.get("/aggregate/authz/mapping", headers=auth_header)
     assert res.status_code == 200
-    assert len(res.json) == 2
+    assert len(res.json) == get_number_of_configured_idps()
 
     default_commons_hostname = app.config["OIDC"]["default"]["commons_hostname"]
     assert default_commons_hostname in res.json

tests/app_test.py

@@ -198,7 +198,13 @@ def test_app_config(app):
         app.config["OIDC"]["idp_a"]["redirect_uri"]
         == "https://workspace.planx-pla.net/wts-callback"
     )
+    assert app.config["OIDC"]["idp_a"]["username_field"] == "context.user.name"
     assert app.config["OIDC"]["idp_a"]["state_prefix"] == "test"
+    assert (
+        app.config["OIDC"]["externaldata-keycloak"]["authorize_url"]
+        == "https://external.data.repository/auth/realms/xyz/protocol/openid-connect/auth"
+    )
+    assert app.config["OIDC"]["externaldata-keycloak"]["username_field"] == "email"
     assert (
         app.config["OIDC"]["default"]["redirect_uri"]
         == "https://test.workspace.planx-pla.net/wts/oauth2/authorize"

tests/conftest.py

@@ -257,7 +257,7 @@ def mock_requests(
     - obtaining JWT keys from Fence
     - Fence's user info endpoint
     Mock POST requests for:
-    - getting an access token from Fence using a refresh token
+    - getting an access token from Fence (and others) using a refresh token
     """
     access_token_to_authz_resource = {}
     for refresh_tokens in refresh_tokens_json.values():
@@ -278,11 +278,19 @@ def post_fence_token_side_effect(request):
             )
 
         def get_fence_user_side_effect(request):
-            access_token = request.headers["Authorization"].split(" ")[1]
-            assert access_token in access_token_to_authz_resource
-            return httpx.Response(
-                200,
-                json={
+            access_token = request.headers.get("Authorization", "").split(" ")
+            access_token = access_token[1] if len(access_token) > 0 else ""
+            if access_token not in access_token_to_authz_resource:
+                print(
+                    "Mocked request '/user/user': no access token was provided. Assuming anonymous; returning empty access."
+                )
+                data = {
+                    "authz": {},
+                    "is_admin": False,
+                    "role": "user",
+                }
+            else:
+                data = {
                     "authz": {
                         access_token_to_authz_resource[access_token]: [
                             {"method": "read", "service": "*"}
@@ -291,8 +299,8 @@ def get_fence_user_side_effect(request):
                     },
                     "is_admin": True,
                     "role": "admin",
-                },
-            )
+                }
+            return httpx.Response(200, json=data)
 
         def get_arborist_authz_side_effect(request):
             access_token = (
@@ -311,13 +319,19 @@ def create_mocks_for_user(app, respx_mock):
             for idp_config in app.config["OIDC"].values():
                 fence_url = idp_config["api_base_url"].rstrip("/")
 
-                fence_token_url = f"{fence_url}/oauth2/token"
-                respx_mock.post(fence_token_url).mock(
+                # mock getting tokens
+                respx_mock.post(f"{fence_url}/oauth2/token").mock(
                     side_effect=post_fence_token_side_effect
                 )
+                respx_mock.post(
+                    "https://external.data.repository/auth/realms/xyz/protocol/openid-connect/token"
+                ).mock(side_effect=post_fence_token_side_effect)
 
-                fence_user_info_url = f"{fence_url}/user"
-                respx_mock.get(fence_user_info_url).mock(
+                # mock userinfo endpoint
+                respx_mock.get(f"{fence_url}/user").mock(
+                    side_effect=get_fence_user_side_effect
+                )
+                respx_mock.get("https://external.data.repository/user/user").mock(
                     side_effect=get_fence_user_side_effect
                 )
 

tests/test_settings.json

@@ -21,6 +21,24 @@
                     "params": {"idp": "fence", "fence_idp": "shibboleth"}
                 }
             }
+        },
+        {
+            "base_url": "https://external.data.repository",
+            "oidc_client_id": "test3",
+            "oidc_client_secret": "test3",
+            "redirect_uri": "https://workspace.planx-pla.net/wts-callback",
+            "login_options": {
+                "externaldata-keycloak": {
+                    "name": "keycloak Login",
+                    "params": {
+                        "idp": "keycloak",
+                        "auth_url": "auth/realms/xyz/protocol/openid-connect/auth",
+                        "token_url": "auth/realms/xyz/protocol/openid-connect/token",
+                        "id_token_username": "email",
+                        "scope": "openid profile offline_access"
+                   }
+                }
+            }
         }
     ]
 }

wts/api.py

@@ -4,7 +4,7 @@
 import flask
 from flask import Flask
 import json
-from urllib.parse import urlparse
+from urllib.parse import urlparse, urljoin
 from cdislogging import get_logger
 from cdiserrors import APIError
 
@@ -59,9 +59,10 @@ def load_settings(app):
         "client_secret": get_var("OIDC_CLIENT_SECRET"),
         "commons_hostname": urlparse(fence_base_url).netloc,
         "api_base_url": fence_base_url,
-        "authorize_url": fence_base_url + "oauth2/authorize",
-        "access_token_url": fence_base_url + "oauth2/token",
-        "redirect_uri": wts_base_url + "oauth2/authorize",
+        "authorize_url": urljoin(fence_base_url, "oauth2/authorize"),
+        "access_token_url": urljoin(fence_base_url, "oauth2/token"),
+        "redirect_uri": urljoin(wts_base_url, "oauth2/authorize"),
+        "username_field": "context.user.name",
         "scope": "openid data user",
         "state_prefix": "",
     }
@@ -74,23 +75,39 @@ def load_settings(app):
         redirect_uri = get_var("REDIRECT_URI", default="", secret_config=conf)
         state_prefix = wts_hostname or ""
         if not redirect_uri:
-            redirect_uri = wts_base_url + "oauth2/authorize"
+            redirect_uri = urljoin(wts_base_url, "oauth2/authorize")
             state_prefix = ""
 
         for idp, idp_conf in conf.get("login_options", {}).items():
-            authorization_url = fence_base_url + "oauth2/authorize"
-            authorization_url = add_params_to_uri(
-                authorization_url, idp_conf.get("params", {})
-            )
+            scope = "openid data user"
+            idp_params = idp_conf.get("params", {})
+
+            if "auth_url" in idp_params and "token_url" in idp_params:  # Generic OIDC
+                auth_endpoint = idp_params.get("auth_url")
+                token_endpoint = idp_params.get("token_url")
+                username_field = idp_params.get("id_token_username_field", "email")
+                scope = idp_params.get("scope", scope)
+                authorization_url = urljoin(url, auth_endpoint)
+                access_token_url = urljoin(url, token_endpoint)
+
+            else:  # Default Gen3 Fence Integration
+                authorization_url = urljoin(fence_base_url, "oauth2/authorize")
+                access_token_url = urljoin(fence_base_url, "oauth2/token")
+                username_field = "context.user.name"
+                authorization_url = add_params_to_uri(
+                    authorization_url, idp_conf.get("params", {})
+                )
+
             app.config["OIDC"][idp] = {
                 "client_id": get_var("OIDC_CLIENT_ID", secret_config=conf),
                 "client_secret": get_var("OIDC_CLIENT_SECRET", secret_config=conf),
                 "commons_hostname": urlparse(fence_base_url).netloc,
                 "api_base_url": fence_base_url,
+                "username_field": username_field,
                 "authorize_url": authorization_url,
-                "access_token_url": fence_base_url + "oauth2/token",
+                "access_token_url": access_token_url,
                 "redirect_uri": redirect_uri,
-                "scope": "openid data user",
+                "scope": scope,
                 "state_prefix": state_prefix,
             }
 

wts/blueprints/aggregate.py

@@ -50,6 +50,8 @@ async def get_aggregate_response(endpoint):
     # Initialzing refresh tokens with the keys of all the commons.
     # This is needed to treat requests to un-connected commons as open access requests
 
+    # /!\ This does not support users being logged into more than 1 IDP for each Commons!!
+    # https://ctds-planx.atlassian.net/browse/PXP-11324
     refresh_tokens = {
         commons: None for commons in flask.current_app.config["COMMONS_HOSTNAMES"]
     }
@@ -144,6 +146,11 @@ async def make_request(commons_hostname, endpoint, headers, parameters, filters)
             )
         )
         return failure_indicator
+    except Exception as e:
+        flask.current_app.logger.error(
+            "Failed to get response from {}.".format(endpoint_url)
+        )
+        return failure_indicator
 
     data = endpoint_response.json()
     if filters:

wts/blueprints/oauth2.py

@@ -50,7 +50,7 @@ def get_authorization_url():
 
     requested_idp = flask.request.args.get("idp", "default")
     client = get_oauth_client(idp=requested_idp)
-    # This will be the value that was put in the ``metadata`` in config.
+    # This will be the value is in /wts/api.py
     state_prefix = client.metadata.get("state_prefix")
     authorize_url = client.metadata.get("authorize_url")
     state = generate_token()

wts/resources/oauth2.py

@@ -14,16 +14,22 @@ def client_do_authorize():
     requested_idp = flask.session.get("idp", "default")
     client = get_oauth_client(idp=requested_idp)
     token_url = client.metadata["access_token_url"]
+
+    # username_field is defined for external oidc clients but it defaults
+    # to context.user.name for fence clients
+    username_field = client.metadata["username_field"]
+
     mismatched_state = (
         "state" not in flask.request.args
         or "state" not in flask.session
         or flask.request.args["state"] != flask.session.pop("state")
     )
+
     if mismatched_state:
         raise AuthError("could not authorize; state did not match across auth requests")
     try:
         tokens = client.fetch_token(token_url, **flask.request.args.to_dict())
-        refresh_refresh_token(tokens, requested_idp)
+        refresh_refresh_token(tokens, requested_idp, username_field)
     except KeyError as e:
         raise AuthError("error in token response: {}".format(tokens))
     except AuthlibBaseError as e:
@@ -43,7 +49,7 @@ def find_valid_refresh_token(username, idp):
     return has_valid
 
 
-def refresh_refresh_token(tokens, idp):
+def refresh_refresh_token(tokens, idp, username_field):
     """
     store new refresh token in db and purge all old tokens for the user
     """
@@ -59,6 +65,7 @@ def refresh_refresh_token(tokens, idp):
         "verify_at_hash": False,
         "leeway": 0,
     }
+
     refresh_token = tokens["refresh_token"]
     id_token = tokens["id_token"]
     # TODO: verify signature with authutils
@@ -84,9 +91,14 @@ def refresh_refresh_token(tokens, idp):
     user = current_user
     username = user.username
 
+    idp_username = id_token
+    # username field is written like "context.user.name" so we split it and loop through the segments
+    for field in username_field.split("."):
+        idp_username = idp_username.get(field)
+
     flask.current_app.logger.info(
         'Linking username "{}" for IdP "{}" to current user "{}"'.format(
-            id_token["context"]["user"]["name"], idp, username
+            idp_username, idp, username
         )
     )
     new_token = RefreshToken(