MIDRC-1285 Validate token audience

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "authutils"
-version = "7.2.8"
+version = "8.0.0"
 description = "Gen3 auth utility functions"
 authors = ["CTDS UChicago <cdis@uchicago.edu>"]
 license = "Apache-2.0"

src/authutils/token/core.py

@@ -1,8 +1,6 @@
 import httpx
 import jwt
 
-from cdislogging import get_logger
-
 from ..errors import (
     JWTAudienceError,
     JWTExpiredError,
@@ -26,7 +24,7 @@ def get_keys_url(issuer, force_issuer=None):
     try:
         jwks_uri = httpx.get(openid_cfg_path).json().get("jwks_uri", "")
         return jwks_uri
-    except:
+    except Exception:
         return jwt_keys_url
 
 
@@ -84,7 +82,7 @@ def validate_jwt(
 
     - Decode JWT using public key; PyJWT will fail if iat or exp fields are
       invalid
-    - PyJWT will NOT fail if the aud field is present in the JWT but no
+    - PyJWT will also fail if the aud field is present in the JWT but no
       ``aud`` arg is passed, or if the ``aud`` arg does not match one of
       the items in the token aud field, because the audience is not validated
       anymore
@@ -96,8 +94,10 @@ def validate_jwt(
     Args:
         encoded_token (str): encoded JWT
         public_key (str): public key to validate the JWT signature
-        aud (Optional[str]): parameter present for backwards compatibility; the
-          audience is not validated anymore
+        aud (Optional[str|list]):
+          if provided, JWT validation will require that the token's ``aud`` value
+          contains the arg value; if not provided, validation will require that
+          the token not have an aud field.
         scope (Optional[Iterable[str]]):
           set of scopes, each of which the JWT must satisfy in its
           ``scope`` claim. Optional.
@@ -114,9 +114,14 @@ def validate_jwt(
         JWTScopeError: if scope validation fails
         JWTError: if some other token validation step fails
     """
-    logger = logger or get_logger(__name__, log_level="info")
 
     # Typecheck arguments.
+    if not isinstance(aud, str) and not isinstance(aud, list) and not aud is None:
+        raise ValueError(
+            "aud must be string, list or None. Instead received aud of type {}".format(
+                type(aud)
+            )
+        )
     if not isinstance(scope, set) and not isinstance(scope, list) and not scope is None:
         raise ValueError(
             "scope must be set or list or None. Instead received scope of type {}".format(
@@ -130,24 +135,12 @@ def validate_jwt(
             )
         )
 
-    # Skip audience validation.
-    # Background: authutils is used by internal Gen3 services asking if they can use a Gen3 Fence
-    # token. Each Gen3 service was setting `aud=<Fence URL>` which is not the way the audience
-    # field is supposed to be used: we were checking that fence was in the list, which it always
-    # was if fence generated it, so this provided no further protection beyond general JWT / public
-    # key verification and validation. The validation of which Gen3 instance the token is meant for
-    # is already done by using the issuer (`iss` field) to get public keys and verify the signature.
-    if aud is not None:
-        logger.warning(
-            f"Authutils no longer validates the token's `aud` field. Received {aud=} which will be ignored."
-        )
-    options["verify_aud"] = False
-
     try:
         token = jwt.decode(
             encoded_token,
             key=public_key,
             algorithms=["RS256"],
+            audience=aud,
             options=options,
         )
     except jwt.InvalidAudienceError as e:

src/authutils/token/fastapi.py

@@ -16,7 +16,12 @@
 
 
 def access_token(
-    *scopes, audience=None, issuer=None, allowed_issuers=None, purpose=None, force_issuer=None
+    *scopes,
+    audience=None,
+    issuer=None,
+    allowed_issuers=None,
+    purpose=None,
+    force_issuer=None
 ):
     """
     Validate and return the JWT bearer token in HTTP header::
@@ -29,11 +34,13 @@ def whoami(token=Depends(access_token("user", "openapi", purpose="access"))):
 
     Args:
         *scopes: Required, all must occur in ``scope``.
-        audience: Optional; parameter present for backwards compatibility; the
-          audience is not validated anymore
+        audience: Optional; if provided, JWT validation will require that the token's
+          ``aud`` value contains the arg value; if not provided, validation will require
+          that the token not have an aud field.
         issuer: Optional; force to use this issuer to validate the token if provided.
         allowed_issuers: Optional allowed issuers whitelist, default: allow all.
         purpose: Optional, must match ``pur`` if provided.
+        force_issuer: Optional
 
     Returns:
         Decoded JWT claims as a :class:`dict`.

src/authutils/token/validate.py

@@ -82,8 +82,11 @@ def validate_jwt(
     Args:
         encoded_token (str): the base64 encoding of the token
         aud (Optional[str]):
-            parameter present for backwards compatibility; the audience
-            is not validated anymore
+            if provided, JWT validation will require that the token's ``aud`` value
+            contains the arg value; if not provided, validation will require that
+            the token not have an aud field.
+            To skip aud validation, pass the following in the options arg:
+              options={"verify_aud": False}
         scope (Optional[Iterable[str]]):
             scopes that the token must satisfy
         purpose (Optional[str]):
@@ -125,7 +128,7 @@ def validate_jwt(
     return claims
 
 
-def validate_request(scope={}, audience=None, purpose="access", logger=None):
+def validate_request(scope=set(), audience=None, purpose="access", logger=None):
     """
     Validate a ``flask.request`` by checking the JWT contained in the request
     headers.
@@ -152,10 +155,16 @@ def validate_request(scope={}, audience=None, purpose="access", logger=None):
     )
 
 
-def require_auth_header(scope={}, audience=None, purpose=None, logger=None):
+def require_auth_header(scope=set(), audience=None, purpose=None, logger=None):
     """
     Return a decorator which adds request validation to check the given
     scopes, audience and purpose (all optional).
+
+    Args:
+        scope (Optional[str|set])
+        audience (Optional[str|list])
+        purpose (Optional[str])
+        logger (Optional)
     """
     logger = logger or get_logger(__name__, log_level="info")
 
@@ -173,11 +182,19 @@ def wrapper(*args, **kwargs):
             the code inside the function can use the ``LocalProxy`` for the
             token (see top of this file).
             """
-            set_current_token(
-                validate_request(
-                    scope=scope, audience=audience, purpose=purpose, logger=logger
+            try:
+                set_current_token(
+                    validate_request(
+                        scope=scope, audience=audience, purpose=purpose, logger=logger
+                    )
+                )
+            except Exception as e:
+                # since this is used as a decorator directly on API routes, the raw stack trace
+                # can be difficult to interpret since it does not include `require_auth_header`
+                logger.error(
+                    f"Error during `authutils.require_auth_header at set_current_token(validate_request)`: {e}"
                 )
-            )
+                raise
             return f(*args, **kwargs)
 
         return wrapper

src/authutils/user.py

@@ -9,8 +9,13 @@
 from authutils.errors import AuthError
 from authutils.token.validate import set_current_token, validate_request
 
+DEFAULT_TOKEN_AUDIENCE = "gen3"
+
 
 def set_current_user(**kwargs):
+    # Fallback to DEFAULT_TOKEN_AUDIENCE if no JWT audience is provided.
+    kwargs.setdefault("jwt_kwargs", {}).setdefault("audience", DEFAULT_TOKEN_AUDIENCE)
+
     flask.g.user = CurrentUser(**kwargs)
     set_current_token(flask.g.user._claims)
     return flask.g.user
@@ -19,7 +24,7 @@ def set_current_user(**kwargs):
 # Proxy for the current user.
 #
 # Other modules importing authutils can import ``current_user`` from here,
-# which will use ``_get_or_set_current_user`` to look up the user.
+# which will use ``set_current_user`` to look up the user.
 current_user = LocalProxy(set_current_user)
 
 

tests/conftest.py

@@ -7,6 +7,7 @@
 import os
 import uuid
 
+from authutils.user import DEFAULT_TOKEN_AUDIENCE
 import flask
 import jwt
 import mock
@@ -42,7 +43,7 @@ def default_audience():
     """
     Return default audience to pass to core.validate_jwt calls.
     """
-    return USER_API
+    return DEFAULT_TOKEN_AUDIENCE
 
 
 @pytest.fixture(scope="session")
@@ -142,7 +143,7 @@ def auth_header(encoded_jwt):
 
 
 @pytest.fixture(scope="function")
-def app():
+def app(default_audience):
     """
     Set up a basic flask app for testing.
     """
@@ -153,7 +154,7 @@ def app():
     app.config["BASE_URL"] = USER_API
 
     @app.route("/test")
-    @require_auth_header({"test_scope"}, USER_API, "access")
+    @require_auth_header({"test_scope"}, default_audience, "access")
     def test_endpoint():
         """
         Define a simple endpoint for testing which requires a JWT header for

tests/test_fastapi.py

@@ -7,23 +7,25 @@
 
 
 @pytest.fixture(scope="function")
-def async_client(default_scopes, mock_async_get, iss):
+def async_client(default_scopes, default_audience, mock_async_get, iss):
     mock_async_get()
 
     app = fastapi.FastAPI()
 
     @app.get("/whoami")
     def whoami(
         token=fastapi.Depends(
-            access_token(*default_scopes, audience=iss, purpose="access")
+            access_token(*default_scopes, audience=default_audience, purpose="access")
         )
     ):
         return token
 
     @app.get("/force_issuer")
     def force_issuer(
         token=fastapi.Depends(
-            access_token(*default_scopes, audience=iss, issuer=iss, purpose="access")
+            access_token(
+                *default_scopes, audience=default_audience, issuer=iss, purpose="access"
+            )
         )
     ):
         return token

tests/test_jwt.py

@@ -66,27 +66,27 @@ def test_invalid_scope_rejected(encoded_jwt, rsa_public_key, default_audience, i
         )
 
 
-def test_missing_aud_not_rejected(encoded_jwt, rsa_public_key, default_scopes, iss):
+def test_missing_aud_rejected(encoded_jwt, rsa_public_key, default_scopes, iss):
     """
     Test that if ``validate_jwt`` is passed a value for ``aud`` which does not
-    appear in the token, a ``JWTError`` is NOT raised, because the audience is not
-    validated anymore.
+    appear in the token, a ``JWTError`` is raised.
     """
-    validate_jwt(encoded_jwt, rsa_public_key, "not-in-aud", default_scopes, [iss])
+    with pytest.raises(JWTError):
+        validate_jwt(encoded_jwt, rsa_public_key, "not-in-aud", default_scopes, [iss])
 
 
-def test_unexpected_aud_not_rejected(
+def test_unexpected_aud_rejected(
     encoded_jwt,
     rsa_public_key,
     default_scopes,
     iss,
 ):
     """
     Test that if the token contains an ``aud`` claim and no ``aud`` arg is passed
-    to ``validate_jwt``, a ``JWTAudienceError`` is NOT raised, because the audience is
-    not validated anymore.
+    to ``validate_jwt``, a ``JWTAudienceError`` is raised.
     """
-    validate_jwt(encoded_jwt, rsa_public_key, None, default_scopes, [iss])
+    with pytest.raises(JWTAudienceError):
+        validate_jwt(encoded_jwt, rsa_public_key, None, default_scopes, [iss])
 
 
 def test_expected_missing_aud_accepted(