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

Make typ optional #644

Merged
merged 13 commits into from Aug 8, 2021
1 change: 1 addition & 0 deletions CHANGELOG.rst
Expand Up @@ -18,6 +18,7 @@ Fixed
- Remove padding from JWK test data. `#628 <https://github.com/jpadilla/pyjwt/pull/628>`__
- Make `kty` mandatory in JWK to be compliant with RFC7517. `#624 <https://github.com/jpadilla/pyjwt/pull/624>`__
- Allow JWK without `alg` to be compliant with RFC7517. `#624 <https://github.com/jpadilla/pyjwt/pull/624>`__
- Make `typ` optional in JWT to be compliant with RFC7519. `#644 <https://github.com/jpadilla/pyjwt/pull/644>`__
- Allow to verify with private key on ECAlgorithm, as well as on Ed25519Algorithm. `#645 <https://github.com/jpadilla/pyjwt/pull/645>`__

Added
Expand Down
3 changes: 2 additions & 1 deletion docs/api.rst
Expand Up @@ -3,7 +3,7 @@ API Reference

.. module:: jwt

.. function:: encode(payload, key, algorithm="HS256", headers=None, json_encoder=None)
.. function:: encode(payload, key, algorithm="HS256", headers=None, json_encoder=None, typ="JWT")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dajiaji how about instead of adding typ as a top level kwarg, we instead allow changing it through headers?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for my late reply.

we instead allow changing it through headers?

To my understanding, we can already change "typ" through "headers", but the problem here is that you can't leave "typ" out. To solve this, you mean headers={"typ":""} is better than typ="" ?

From my point of view, since there are now several standards for typ other than JWT (e.g., dpop+jwt and scevent+jwt), it is not a bad idea to be able to change typ as a top-level argument like alg.

Anyway, if you think headers={"typ":""} is better, I can update my PR for that. No problem. Feel free to request me.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jpadilla can you please provide your feedback on the previous comment. Currently this is a blocker for me, and I'm waiting for this PR to be merged into master. Thank you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, @jpadilla. I fixed it as you requested. The changes are much less. Could you check and merge it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jpadilla I resolved a conflict on CHANGELOG.


Encode the ``payload`` as JSON Web Token.

Expand All @@ -16,6 +16,7 @@ API Reference
:param str algorithm: algorithm to sign the token with, e.g. ``"ES256"``
:param dict headers: additional JWT header fields, e.g. ``dict(kid="my-key-id")``
:param json.JSONEncoder json_encoder: custom JSON encoder for ``payload`` and ``headers``
:param str typ: media type of the payload, e.g. ``"secevent+jwt"``
:rtype: str
:returns: a JSON Web Token

Expand Down
6 changes: 3 additions & 3 deletions jwt/api_jws.py
Expand Up @@ -19,8 +19,6 @@


class PyJWS:
header_typ = "JWT"

def __init__(self, algorithms=None, options=None):
self._algorithms = get_default_algorithms()
self._valid_algs = (
Expand Down Expand Up @@ -80,6 +78,7 @@ def encode(
algorithm: str = "HS256",
headers: Optional[Dict] = None,
json_encoder: Optional[Type[json.JSONEncoder]] = None,
typ: Optional[str] = "JWT",
) -> str:
segments = []

Expand All @@ -90,7 +89,8 @@ def encode(
pass

# Header
header = {"typ": self.header_typ, "alg": algorithm}
header = {"typ": typ} if typ else {}
header["alg"] = algorithm

if headers:
self._validate_headers(headers)
Expand Down
3 changes: 2 additions & 1 deletion jwt/api_jwt.py
Expand Up @@ -41,6 +41,7 @@ def encode(
algorithm: str = "HS256",
headers: Optional[Dict] = None,
json_encoder: Optional[Type[json.JSONEncoder]] = None,
typ: Optional[str] = "JWT",
) -> str:
# Check that we get a mapping
if not isinstance(payload, Mapping):
Expand All @@ -60,7 +61,7 @@ def encode(
payload, separators=(",", ":"), cls=json_encoder
).encode("utf-8")

return api_jws.encode(json_payload, key, algorithm, headers, json_encoder)
return api_jws.encode(json_payload, key, algorithm, headers, json_encoder, typ)

def decode_complete(
self,
Expand Down
62 changes: 62 additions & 0 deletions tests/test_api_jws.py
Expand Up @@ -624,6 +624,68 @@ def test_encode_headers_parameter_adds_headers(self, jws, payload):
assert "testheader" in header_obj
assert header_obj["testheader"] == headers["testheader"]

def test_encode_with_typ(self, jws):
payload = """
{
"iss": "https://scim.example.com",
"iat": 1458496404,
"jti": "4d3559ec67504aaba65d40b0363faad8",
"aud": [
"https://scim.example.com/Feeds/98d52461fa5bbc879593b7754",
"https://scim.example.com/Feeds/5d7604516b1d08641d7676ee7"
],
"events": {
"urn:ietf:params:scim:event:create": {
"ref":
"https://scim.example.com/Users/44f6142df96bd6ab61e7521d9",
"attributes": ["id", "name", "userName", "password", "emails"]
}
}
}
"""
token = jws.encode(payload.encode("utf-8"), "secret", typ="secevent+jwt")

header = token[0 : token.index(".")].encode()
header = base64url_decode(header)
header_obj = json.loads(header)

assert "typ" in header_obj
assert header_obj["typ"] == "secevent+jwt"

def test_encode_with_typ_empty_string(self, jws, payload):
token = jws.encode(payload, "secret", typ="")

header = token[0 : token.index(".")].encode()
header = base64url_decode(header)
header_obj = json.loads(header)

assert "typ" not in header_obj

def test_encode_with_typ_and_headers_include_typ(self, jws, payload):
headers = {"typ": "a"}
token = jws.encode(payload, "secret", typ="b", headers=headers)

header = token[0 : token.index(".")].encode()
header = base64url_decode(header)
header_obj = json.loads(header)

assert "typ" in header_obj
# typ in headers overwrites typ parameter.
assert header_obj["typ"] == "a"

def test_encode_with_typ_without_keywords(self, jws, payload):
headers = {"foo": "bar"}
token = jws.encode(payload, "secret", "HS256", headers, None, "baz")

header = token[0 : token.index(".")].encode()
header = base64url_decode(header)
header_obj = json.loads(header)

assert "foo" in header_obj
assert header_obj["foo"] == "bar"
assert "typ" in header_obj
assert header_obj["typ"] == "baz"

def test_encode_fails_on_invalid_kid_types(self, jws, payload):
with pytest.raises(InvalidTokenError) as exc:
jws.encode(payload, "secret", headers={"kid": 123})
Expand Down
25 changes: 25 additions & 0 deletions tests/test_api_jwt.py
Expand Up @@ -16,6 +16,7 @@
InvalidIssuerError,
MissingRequiredClaimError,
)
from jwt.utils import base64url_decode

from .utils import crypto_required, key_path, utc_timestamp

Expand Down Expand Up @@ -167,6 +168,30 @@ def test_encode_bad_type(self, jwt):
lambda: jwt.encode(t, "secret", algorithms=["HS256"]),
)

def test_encode_with_typ(self, jwt):
payload = {
"iss": "https://scim.example.com",
"iat": 1458496404,
"jti": "4d3559ec67504aaba65d40b0363faad8",
"aud": [
"https://scim.example.com/Feeds/98d52461fa5bbc879593b7754",
"https://scim.example.com/Feeds/5d7604516b1d08641d7676ee7",
],
"events": {
"urn:ietf:params:scim:event:create": {
"ref": "https://scim.example.com/Users/44f6142df96bd6ab61e7521d9",
"attributes": ["id", "name", "userName", "password", "emails"],
}
},
}
token = jwt.encode(payload, "secret", algorithm="HS256", typ="secevent+jwt")
header = token[0 : token.index(".")].encode()
header = base64url_decode(header)
header_obj = json.loads(header)

assert "typ" in header_obj
assert header_obj["typ"] == "secevent+jwt"

def test_decode_raises_exception_if_exp_is_not_int(self, jwt):
# >>> jwt.encode({'exp': 'not-an-int'}, 'secret')
example_jwt = (
Expand Down