Prefer headers['alg'] to algorithm parameter in encode().

[dajiaji]

Fix #659.

There is an alternate solution that returns error if both headers[’alg’] and algorithm are set and they are different, I adopted the way to prefer headers['alg'] to algorithm if alg is in headers.

It can support the following case mentioned in #659 and all of existing tests passed.

key = ...
header = {"typ": "JWT", "alg": "PS256", "kid": "my-key-id"}
claims = {"foo": "bar"}

jws = jwt.encode(payload=claims, key=key, headers=header)

Close #659

[jpadilla]

thanks! could you update the changelog as well?

[jpadilla]

Would type definitions for algorithm need updating?

[De117]

If header["alg"] is always preferred to algorithm, then no. It can remain a str.

But lines 86-87 (if algorithm is None: ...) are unneeded. I'd remove that.

[dajiaji]

Since algorithm parameter is practically optional and for keeping backward-compat, I don't think its type definition should be updated at least for now.

But lines 86-87 (if algorithm is None: ...) are unneeded. I'd remove that.

As the current spec, algorithm=None is interpreted to {"alg": "none"} and related tests exist. So it should not be changed for keeping backward-compat for now. It needs another PR if needed.

[De117]

Shouldn't it then already be of type Optional[str]? 🤔 Passing algorithm=None fails the typecheck with the current annotation. Keeping it annotated as a str only makes sense if that option will (eventually) be removed.

[dajiaji]

Shouldn't it then already be of type Optional[str]? Passing algorithm=None fails the typecheck with the current annotation.

I agree with you and I have recognized this problem. I doubt this fix need to be done in this PR but I'll fix it.

[dajiaji]

@jpadilla I fixed the type definitions. I'm glad if you check and merge it.