Commit dc27619d authored by Dimitrios Gogos's avatar Dimitrios Gogos
Browse files

feat: enhance JWT token decoding to differentiate between access and ID tokens

parent e03ed1a5
Loading
Loading
Loading
Loading
+8 −2
Changes for src/portal_service/api/auth/verification.py: 8 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -35,7 +35,7 @@ def _get_jwks_client(jwks_uri: str) -> PyJWKClient:
    return _jwks_client_cache[jwks_uri]


def _decode_token(token: str) -> TokenClaims:
def _decode_token(token: str, expected_typ: str = "Bearer") -> TokenClaims:
    """
    Decode and verify a JWT, returning its claims.

@@ -43,6 +43,10 @@ def _decode_token(token: str) -> TokenClaims:
      TokenExpiredError   — signature valid but token is past exp
      InvalidTokenError   — anything else that makes the token unacceptable
      IdentityProviderUnavailableError — signing keys unreachable; not the caller's fault

    `expected_typ` is Keycloak's own `typ` claim: "Bearer" for access tokens, "ID"
    for ID tokens. Both share issuer, audience and signing key, so without this an
    ID token would pass as an access token.
    """
    oidc = get_settings().oidc
    if oidc is None:
@@ -73,6 +77,8 @@ def _decode_token(token: str) -> TokenClaims:
        raise IdentityProviderUnavailableError("Could not fetch signing keys") from exc
    except jwt.PyJWTError as exc:
        raise InvalidTokenError(f"Token validation failed: {exc}") from exc
    if payload.get("typ") != expected_typ:
        raise InvalidTokenError(f"Expected a {expected_typ} token")
    try:
        return TokenClaims(**payload)
    except ValidationError as exc:
@@ -89,7 +95,7 @@ def _decode_id_token(token: str, expected_nonce: str) -> TokenClaims:
    copies it into the ID token, so a token replayed from another login carries a
    different one and is refused.
    """
    claims = _decode_token(token)
    claims = _decode_token(token, expected_typ="ID")
    if not claims.nonce or not compare_digest(claims.nonce, expected_nonce):
        logger.warning("id token nonce mismatch", subject=claims.sub)
        raise InvalidTokenError("ID token does not belong to this login")
+2 −0
Changes for tests/conftest.py: 2 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -75,6 +75,7 @@ def make_token(rsa_key: rsa.RSAPrivateKey) -> Callable[..., str]:
        issuer: str = ISSUER,
        audience: str | list[str] | None = CLIENT_ID,
        preferred_username: str | None = "user",
        typ: str = "Bearer",
        **extra: Any,
    ) -> str:
        now = int(time.time())
@@ -85,6 +86,7 @@ def make_token(rsa_key: rsa.RSAPrivateKey) -> Callable[..., str]:
            "iat": now,
            "scope": scope,
            "preferred_username": preferred_username,
            "typ": typ,
            **extra,
        }
        if audience is not None:
+1 −1
Changes for tests/test_callback_route.py: 1 added line, 1 removed line.
Original line number Diff line number Diff line
@@ -76,7 +76,7 @@ def _tokens(make_token: Callable[..., str], nonce: str) -> IssuedTokens:
        tokens=SessionTokens(
            access_token="the-access-token",
            refresh_token="the-refresh-token",
            id_token=make_token(nonce=nonce),
            id_token=make_token(nonce=nonce, typ="ID"),
        ),
        expires_in=300,
    )
+17 −3
Changes for tests/test_id_token.py: 17 added lines, 3 removed lines.
Original line number Diff line number Diff line
@@ -56,7 +56,7 @@ def test_audience_may_list_several_clients(
def test_id_token_with_the_expected_nonce_is_accepted(
    app_jwt: FastAPI, patch_jwks: None, make_token: Callable[..., str]
) -> None:
    claims = _decode_id_token(make_token(nonce="the-nonce"), expected_nonce="the-nonce")
    claims = _decode_id_token(make_token(nonce="the-nonce", typ="ID"), expected_nonce="the-nonce")
    assert claims.nonce == "the-nonce"


@@ -64,11 +64,25 @@ def test_id_token_from_another_login_is_refused(
    app_jwt: FastAPI, patch_jwks: None, make_token: Callable[..., str]
) -> None:
    with pytest.raises(InvalidTokenError, match="does not belong to this login"):
        _decode_id_token(make_token(nonce="another-login"), expected_nonce="the-nonce")
        _decode_id_token(make_token(nonce="another-login", typ="ID"), expected_nonce="the-nonce")


def test_id_token_without_a_nonce_is_refused(
    app_jwt: FastAPI, patch_jwks: None, make_token: Callable[..., str]
) -> None:
    with pytest.raises(InvalidTokenError, match="does not belong to this login"):
        _decode_id_token(make_token(), expected_nonce="the-nonce")
        _decode_id_token(make_token(typ="ID"), expected_nonce="the-nonce")


def test_an_id_token_is_not_accepted_as_an_access_token(
    app_jwt: FastAPI, patch_jwks: None, make_token: Callable[..., str]
) -> None:
    with pytest.raises(InvalidTokenError):
        _decode_token(make_token(typ="ID"))


def test_an_access_token_is_not_accepted_as_an_id_token(
    app_jwt: FastAPI, patch_jwks: None, make_token: Callable[..., str]
) -> None:
    with pytest.raises(InvalidTokenError):
        _decode_id_token(make_token(nonce="the-nonce"), expected_nonce="the-nonce")