PXP-6617 Add custom scopes validation and revert aud validation to de…

…fault (#47)

* feat(scope): Add class JWTScopeError(JWTError)

* fix(aud): Validate aud claim the normal way, validate custom scopes claim

* import new JWTScopeError
* add new scope arg to core.validate_jwt
* add new scope validation to core.validate_jwt
* remove custom aud validation from core.validate_jwt
* remove random_aud hack in core.validate_jwt; type(aud) now string-or-None, not set-or-list
* pass aud through to PyJWT for normal validation
* update docstring for core.validate_jwt
* allow empty aud arg in validate.validate_jwt; cease raising ValueError
* add new optional scope arg in validate.validate_jwt; pass through to core.validate_jwt
* update docstring for validate.validate_jwt

* fix(aud): allow passthrough of options arg to pyjwt

* fix(aud-scope): switch require_auth_header to checking scopes not aud

* fix(aud-scope): Skip aud validation in require_auth_header/validate_request

* test(aud-scope): Change default_audiences fixture to default_scopes; rm aud from generic claims

* fix(aud-scope): chg aud to scope in FastAPI access_token dependency

* test(aud-scope): Upd tests to reflect new aud/scope usage

* fix(aud-scope): chg aud to scope in CurrentUser call to validate_request

* test(aud): add happy-path test for aud validation

* style(black): Blacken, and update black rev in precommit config

* test(aud): Explicitly pass None instead of default_audiences

* because default_audience may change to not None in future
* and because this better reflects the intention of the test

* fix(aud): Re-enable aud claim validation in require_auth_header

* fix(aud): Expect iss in aud claim by default in token.validate_jwt...

* ...if a value for iss is avbl, from app cfg BASE_URL or USER_API.
* Also clarify core.validate_jwt docstring.

* test(app-fixture): Set app.config['BASE_URL'] as well as ['USER_API']

* fix(aud): Allow passing expected audience to FastAPI access_token dependency

* test(aud): Include aud claim in default claims test fixture

* Update default_audience fixture accordingly
* Update tests to account for new default claims

* fix(aud): Allow passing expected audience to require_auth_header and validate_request

* Also let scope={} by default
* Update calls to require_auth_header

* fix(aud): Update set_current_user proxy fn to pass in expected aud

* based on flask.current_app.config
* Since this already assumes Flask request ctx, I think OK to look in Flask app cfg in this case

* test(aud): Add test: no aud arg provided and no aud claim in token

* test(aud): Rename fixture default_audiences to default_audience

* chore(precommit): pre-commit autoupdate

* fix(aud): fix incorrect kwargs logic

* docs(aud): add missing audience arg to docstring

* test(aud): use default_audience instead of iss in claims fixture

* fix(aud-scope): error message

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

* docs(aud-scope): Fix docstring

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

Co-authored-by: Pauline Ribeyre <ribeyre@uchicago.edu>

.pre-commit-config.yaml

@@ -1,16 +1,16 @@
 repos:
 -   repo: git@github.com:Yelp/detect-secrets
-    rev: v0.13.1
+    rev: v1.1.0
     hooks:
     -   id: detect-secrets
         args: ['--baseline', '.secrets.baseline']
 -   repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v2.5.0
+    rev: v3.4.0
     hooks:
     -   id: end-of-file-fixer
     -   id: no-commit-to-branch
         args: [--branch, develop, --branch, master, --pattern, release/.*]
 -   repo: https://github.com/psf/black
-    rev: 19.10b0
+    rev: 20.8b1
     hooks:
     -   id: black

.secrets.baseline

@@ -1,8 +1,4 @@
 {
-  "exclude": {
-    "files": null,
-    "lines": null
-  },
   "generated_at": "2021-01-19T16:35:59Z",
   "plugins_used": [
     {
@@ -12,8 +8,8 @@
       "name": "ArtifactoryDetector"
     },
     {
-      "base64_limit": 4.5,
-      "name": "Base64HighEntropyString"
+      "name": "Base64HighEntropyString",
+      "limit": 4.5
     },
     {
       "name": "BasicAuthDetector"
@@ -22,8 +18,8 @@
       "name": "CloudantDetector"
     },
     {
-      "hex_limit": 3,
-      "name": "HexHighEntropyString"
+      "name": "HexHighEntropyString",
+      "limit": 3
     },
     {
       "name": "IbmCloudIamDetector"
@@ -60,26 +56,60 @@
   "results": {
     "src/authutils/oauth2/client/blueprint.py": [
       {
+        "type": "Secret Keyword",
+        "filename": "src/authutils/oauth2/client/blueprint.py",
         "hashed_secret": "6eae3a5b062c6d0d79f070c26e6d62486b40cb46",
-        "is_secret": false,
         "is_verified": false,
         "line_number": 15,
-        "type": "Secret Keyword"
+        "is_secret": false
       }
     ],
     "src/authutils/testing/fixtures/keys.py": [
       {
+        "type": "Private Key",
+        "filename": "src/authutils/testing/fixtures/keys.py",
         "hashed_secret": "be4fc4886bd949b369d5e092eb87494f12e57e5b",
-        "is_secret": false,
         "is_verified": false,
         "line_number": 83,
-        "type": "Private Key"
+        "is_secret": false
       }
     ]
   },
-  "version": "0.13.1",
-  "word_list": {
-    "file": null,
-    "hash": null
-  }
+  "version": "1.1.0",
+  "filters_used": [
+    {
+      "path": "detect_secrets.filters.allowlist.is_line_allowlisted"
+    },
+    {
+      "path": "detect_secrets.filters.heuristic.is_sequential_string"
+    },
+    {
+      "path": "detect_secrets.filters.heuristic.is_potential_uuid"
+    },
+    {
+      "path": "detect_secrets.filters.heuristic.is_likely_id_string"
+    },
+    {
+      "path": "detect_secrets.filters.heuristic.is_templated_secret"
+    },
+    {
+      "path": "detect_secrets.filters.heuristic.is_prefixed_with_dollar_sign"
+    },
+    {
+      "path": "detect_secrets.filters.heuristic.is_indirect_reference"
+    },
+    {
+      "path": "detect_secrets.filters.common.is_ignored_due_to_verification_policies",
+      "min_level": 2
+    },
+    {
+      "path": "detect_secrets.filters.heuristic.is_lock_file"
+    },
+    {
+      "path": "detect_secrets.filters.heuristic.is_not_alphanumeric_string"
+    },
+    {
+      "path": "detect_secrets.filters.heuristic.is_swagger_file"
+    }
+  ]
 }

src/authutils/errors.py

@@ -24,3 +24,8 @@ class JWTPurposeError(JWTError):
 class JWTAudienceError(JWTError):
 
     pass
+
+
+class JWTScopeError(JWTError):
+
+    pass

src/authutils/token/core.py

@@ -1,6 +1,12 @@
 import jwt
 
-from ..errors import JWTAudienceError, JWTExpiredError, JWTPurposeError, JWTError
+from ..errors import (
+    JWTAudienceError,
+    JWTExpiredError,
+    JWTPurposeError,
+    JWTScopeError,
+    JWTError,
+)
 
 
 def get_keys_url(issuer):
@@ -47,50 +53,76 @@ def validate_purpose(claims, pur):
         )
 
 
-def validate_jwt(encoded_token, public_key, aud, issuers):
+def validate_jwt(encoded_token, public_key, aud, scope, issuers, options={}):
     """
     Validate the encoded JWT ``encoded_token``, which must satisfy the
-    audiences ``aud``.
+    scopes ``scope``.
 
     This is just a slightly lower-level function to decode the token and
     perform the most basic checks on the token.
 
     - Decode JWT using public key; PyJWT will fail if iat or exp fields are
       invalid
-    - Check audiences: token audiences must be a superset of required audiences
-      (the ``aud`` argument); fail if not satisfied
+    - 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
+    - Check issuers: token iss field must match one of the items in the
+      ``issuers`` arg
+    - Check scopes: token scopes must be a superset of required scopes
+      (the ``scope`` argument); fail if not satisfied
 
     Args:
         encoded_token (str): encoded JWT
         public_key (str): public key to validate the JWT signature
-        aud (set): non-empty set of audiences the JWT must satisfy
+        aud (Optional[str]):
+          audience with which the app identifies, usually an OIDC
+          client id, which the JWT will be expected to include in its ``aud``
+          claim. Optional; if no ``aud`` argument given, then the JWT must
+          not have an ``aud`` claim, or validation will fail.
+        scope (Optional[Iterable[str]]):
+          set of scopes, each of which the JWT must satisfy in its
+          ``scope`` claim. Optional.
         issuers (list or set): allowed issuers whitelist
+        options (Optional[dict]): options to pass through to pyjwt's decode
 
     Return:
         dict: the decoded and validated JWT
 
     Raises:
         ValueError: if receiving an incorrectly-typed argument
-        JWTValidationError: if any step of the validation fails
+        JWTExpiredError: if token is expired
+        JWTAudienceError: if aud validation fails
+        JWTScopeError: if scope validation fails
+        JWTError: if some other token validation step fails
     """
+
     # Typecheck arguments.
-    if not isinstance(aud, set) and not isinstance(aud, list):
-        raise ValueError("aud must be set or list")
+    if not isinstance(aud, str) and not aud is None:
+        raise ValueError(
+            "aud must be string 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(
+                type(scope)
+            )
+        )
     if not isinstance(issuers, set) and not isinstance(issuers, list):
-        raise ValueError("issuers must be set or list")
-
-    # To satisfy PyJWT, since the token will contain an aud field, decode has
-    # to be passed one of the audiences to check here (so PyJWT doesn't raise
-    # an InvalidAudienceError). Per the JWT specification, if the token
-    # contains an aud field, the validator MUST identify with one of the
-    # audiences listed in that field. This implementation is more strict, and
-    # allows the validator to demand multiple audiences which must all be
-    # satisfied by the token (see below).
-    aud = set(aud)
-    random_aud = list(aud)[0]
+        raise ValueError(
+            "issuers must be set or list. Instead received issuers of type {}".format(
+                type(issuers)
+            )
+        )
+
     try:
         token = jwt.decode(
-            encoded_token, key=public_key, algorithms=["RS256"], audience=random_aud
+            encoded_token,
+            key=public_key,
+            algorithms=["RS256"],
+            audience=aud,
+            options=options,
         )
     except jwt.InvalidAudienceError as e:
         raise JWTAudienceError(e)
@@ -99,7 +131,7 @@ def validate_jwt(encoded_token, public_key, aud, issuers):
     except jwt.InvalidTokenError as e:
         raise JWTError(e)
 
-    # PyJWT validates iat and exp fields (and aud...sort of); everything else
+    # PyJWT validates iat, exp, and aud fields; everything else
     # must happen here.
 
     # iss
@@ -108,12 +140,22 @@ def validate_jwt(encoded_token, public_key, aud, issuers):
         msg = "invalid issuer {}; expected: {}".format(token["iss"], issuers)
         raise JWTError(msg)
 
-    # aud
-    # The audiences listed in the token must completely satisfy all the
-    # required audiences provided. Note that this is stricter than the
-    # specification suggested in RFC 7519.
-    missing = aud - set(token["aud"])
-    if missing:
-        raise JWTAudienceError("missing audiences: " + str(missing))
+    # scope
+    # Check that if scope arg was non-empty then the token includes each given scope in its scope claim
+    if scope:
+        token_scopes = token.get("scope", [])
+        if isinstance(token_scopes, str):
+            token_scopes = [token_scopes]
+        if not isinstance(token_scopes, list):
+            raise JWTError(
+                "invalid format in scope claim: {}; expected string or list".format(
+                    token["scopes"]
+                )
+            )
+        missing_scopes = set(scope) - set(token_scopes)
+        if missing_scopes:
+            raise JWTScopeError(
+                "token is missing required scopes: " + str(missing_scopes)
+            )
 
     return token

src/authutils/token/fastapi.py

@@ -14,7 +14,9 @@
 _jwt_public_keys = {}
 
 
-def access_token(*audiences, issuer=None, allowed_issuers=None, purpose=None):
+def access_token(
+    *scopes, audience=None, issuer=None, allowed_issuers=None, purpose=None
+):
     """
     Validate and return the JWT bearer token in HTTP header::
 
@@ -25,18 +27,21 @@ def whoami(token=Depends(access_token("user", "openapi", purpose="access"))):
             return token["iss"]
 
     Args:
-        *audiences: Required, all must occur in ``aud``.
-        issuer: Force to use this issuer to validate the token if provided.
+        *scopes: Required, all must occur in ``scope``.
+        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.
 
     Returns:
         Decoded JWT claims as a :class:`dict`.
     """
 
-    if not audiences:
-        raise ValueError("Missing parameter: audiences")
-    audiences = set(audiences)
+    if not scopes:
+        raise ValueError("Missing parameter: scopes")
+    scopes = set(scopes)
     if not allowed_issuers and issuer:
         allowed_issuers = [issuer]
 
@@ -93,7 +98,13 @@ async def getter(token: HTTPAuthorizationCredentials = Security(bearer)):
         # decode and validate the token
         try:
             claims = await loop.run_in_executor(
-                None, core.validate_jwt, token, pub_key, audiences, allowed_issuers
+                None,
+                core.validate_jwt,
+                token,
+                pub_key,
+                audience,
+                scopes,
+                allowed_issuers,
             )
 
             if purpose: