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

PXP-6617 Add custom scopes validation and revert aud validation to default #47

Merged
merged 27 commits into from
Apr 29, 2021
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
c723d4b
feat(scope): Add class JWTScopeError(JWTError)
vpsx Sep 3, 2020
d2a4a77
fix(aud): Validate aud claim the normal way, validate custom scopes c…
vpsx Sep 3, 2020
397d062
fix(aud): allow passthrough of options arg to pyjwt
vpsx Sep 11, 2020
66dd781
fix(aud-scope): switch require_auth_header to checking scopes not aud
vpsx Sep 14, 2020
93cfe40
fix(aud-scope): Skip aud validation in require_auth_header/validate_r…
vpsx Sep 16, 2020
29f8420
test(aud-scope): Change default_audiences fixture to default_scopes; …
vpsx Sep 22, 2020
ee69572
fix(aud-scope): chg aud to scope in FastAPI access_token dependency
vpsx Sep 22, 2020
950a1aa
test(aud-scope): Upd tests to reflect new aud/scope usage
vpsx Sep 23, 2020
e404a59
fix(aud-scope): chg aud to scope in CurrentUser call to validate_request
vpsx Sep 23, 2020
c64b190
test(aud): add happy-path test for aud validation
vpsx Sep 23, 2020
99b1531
style(black): Blacken, and update black rev in precommit config
vpsx Sep 23, 2020
13d8a3b
test(aud): Explicitly pass None instead of default_audiences
vpsx Oct 9, 2020
e25302d
fix(aud): Re-enable aud claim validation in require_auth_header
vpsx Oct 12, 2020
40ef1a4
fix(aud): Expect iss in aud claim by default in token.validate_jwt...
vpsx Oct 8, 2020
71e5bba
test(app-fixture): Set app.config['BASE_URL'] as well as ['USER_API']
vpsx Oct 12, 2020
25a3162
fix(aud): Allow passing expected audience to FastAPI access_token dep…
vpsx Oct 12, 2020
57f9bd0
test(aud): Include aud claim in default claims test fixture
vpsx Oct 12, 2020
38326bf
fix(aud): Allow passing expected audience to require_auth_header and …
vpsx Oct 12, 2020
0178eb4
fix(aud): Update set_current_user proxy fn to pass in expected aud
vpsx Oct 12, 2020
e0be0f1
test(aud): Add test: no aud arg provided and no aud claim in token
vpsx Oct 13, 2020
ba5ffbf
test(aud): Rename fixture default_audiences to default_audience
vpsx Oct 13, 2020
692c975
chore(precommit): pre-commit autoupdate
vpsx Apr 22, 2021
db80a21
fix(aud): fix incorrect kwargs logic
vpsx Apr 22, 2021
2d07c84
docs(aud): add missing audience arg to docstring
vpsx Apr 22, 2021
c0dfb06
test(aud): use default_audience instead of iss in claims fixture
vpsx Apr 22, 2021
5272cfc
fix(aud-scope): error message
vpsx Apr 23, 2021
229f586
docs(aud-scope): Fix docstring
vpsx Apr 23, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ repos:
- id: no-commit-to-branch
args: [--branch, develop, --branch, master, --pattern, release/.*]
- repo: https://github.com/psf/black
rev: 19.10b0
rev: 20.8b1
paulineribeyre marked this conversation as resolved.
Show resolved Hide resolved
hooks:
- id: black
5 changes: 5 additions & 0 deletions src/authutils/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,8 @@ class JWTPurposeError(JWTError):
class JWTAudienceError(JWTError):

pass


class JWTScopeError(JWTError):

pass
100 changes: 71 additions & 29 deletions src/authutils/token/core.py
Original file line number Diff line number Diff line change
@@ -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):
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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 list".format(
vpsx marked this conversation as resolved.
Show resolved Hide resolved
token["scopes"]
)
)
missing_scopes = set(scope) - set(token_scopes)
if missing_scopes:
raise JWTScopeError(
"token is missing required scopes: " + str(missing_scopes)
)

return token
20 changes: 14 additions & 6 deletions src/authutils/token/fastapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -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::

Expand All @@ -25,7 +27,7 @@ def whoami(token=Depends(access_token("user", "openapi", purpose="access"))):
return token["iss"]

Args:
*audiences: Required, all must occur in ``aud``.
paulineribeyre marked this conversation as resolved.
Show resolved Hide resolved
*scopes: Required, all must occur in ``scope``.
issuer: 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.
Expand All @@ -34,9 +36,9 @@ def whoami(token=Depends(access_token("user", "openapi", purpose="access"))):
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]

Expand Down Expand Up @@ -93,7 +95,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:
Expand Down
55 changes: 41 additions & 14 deletions src/authutils/token/validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,57 +67,74 @@ def get_session_token():

def validate_jwt(
encoded_token,
aud,
aud=None,
scope=None,
purpose="access",
issuers=None,
public_key=None,
attempt_refresh=True,
logger=None,
options={},
):
"""
Validate a JWT and return the claims.

Args:
encoded_token (str): the base64 encoding of the token
aud (Optional[Iterable[str]]):
list of audiences that the token must satisfy; defaults to
``{'openid'}`` (minimum expected by OpenID provider)
aud (Optional[str]):
audience as which the app identifies, which the JWT will be
expected to include in its ``aud`` claim.
Optional; will default to issuer from flask.current_app.config
if available (either BASE_URL or USER_API).
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]):
which purpose the token is supposed to be used for (access,
refresh, or id)
issuers (Iterable[str]): list of allowed token issuers
public_key (Optional[str]): public key to vaidate JWT with
attempt_refresh (Optional[bool]):
whether to attempt refresh of public keys if not found in cache
options (Optional[dict]): options to pass through to pyjwt's decode

Return:
dict: dictionary of claims from the validated JWT

Raises:
ValueError: if ``aud`` is empty
JWTError:
if auth header is missing, decoding fails, or the JWT fails to
satisfy any expectation
"""
logger = logger or get_logger(__name__, log_level="info")

if not issuers:
issuers = []
for config_var in ["OIDC_ISSUER", "USER_API", "BASE_URL"]:
value = flask.current_app.config.get(config_var)
if value:
issuers.append(value)

# Can't set arg default to config[x] in fn def, so doing it this way.
if aud is None:
aud = flask.current_app.config.get("BASE_URL")
# Some Gen3 apps use BASE_URL and some use USER_API, so fall back on USER_API
if aud is None:
aud = flask.current_app.config.get("USER_API")

if public_key is None:
public_key = get_public_key_for_token(
encoded_token, attempt_refresh=attempt_refresh, logger=logger
)
if not aud:
raise ValueError("must provide at least one audience")
aud = set(aud)
claims = core.validate_jwt(encoded_token, public_key, aud, issuers)

claims = core.validate_jwt(encoded_token, public_key, aud, scope, issuers, options)
if purpose:
core.validate_purpose(claims, purpose)
return claims


def validate_request(aud, purpose="access", logger=None):
def validate_request(scope={}, audience=None, purpose="access", logger=None):
"""
Validate a ``flask.request`` by checking the JWT contained in the request
headers.
Expand All @@ -132,13 +149,19 @@ def validate_request(aud, purpose="access", logger=None):
raise JWTError("no authorization header provided")

# Pass token to ``validate_jwt``.
return validate_jwt(encoded_token, aud, purpose, logger=logger)
return validate_jwt(
encoded_token,
aud=audience,
scope=scope,
purpose=purpose,
logger=logger,
)


def require_auth_header(aud, purpose=None, logger=None):
def require_auth_header(scope={}, audience=None, purpose=None, logger=None):
"""
Return a decorator which adds request validation to check the given
audiences and (optionally) purpose.
scopes and (optionally) purpose.
vpsx marked this conversation as resolved.
Show resolved Hide resolved
"""
logger = logger or get_logger(__name__, log_level="info")

Expand All @@ -156,7 +179,11 @@ 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(aud=aud, purpose=purpose, logger=logger))
set_current_token(
validate_request(
scope=scope, audience=audience, purpose=purpose, logger=logger
)
)
return f(*args, **kwargs)

return wrapper
Expand Down
11 changes: 8 additions & 3 deletions src/authutils/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@


def set_current_user(**kwargs):
flask.g.user = CurrentUser(**kwargs)
expected_audience = flask.current_app.config.get("USER_API")
# Gen3 services use both USER_API and BASE_URL
if not expected_audience:
expected_audience = flask.current_app.config.get("BASE_URL")

flask.g.user = CurrentUser(jwt_kwargs={"audience": expected_audience}, **kwargs)
paulineribeyre marked this conversation as resolved.
Show resolved Hide resolved
set_current_token(flask.g.user._claims)
return flask.g.user

Expand Down Expand Up @@ -41,8 +46,8 @@ class CurrentUser(object):

def __init__(self, claims=None, jwt_kwargs=None):
jwt_kwargs = jwt_kwargs or {}
if "aud" not in jwt_kwargs:
jwt_kwargs["aud"] = {"openid"}
if "scope" not in jwt_kwargs:
jwt_kwargs["scope"] = {"openid"}
self._claims = claims or validate_request(**jwt_kwargs)
self.id = self._claims["sub"]
self.username = self._get_user_info("name")
Expand Down
Loading