lepture / authlib

The ultimate Python library in building OAuth, OpenID Connect clients and servers. JWS,JWE,JWK,JWA,JWT included.
https://authlib.org/
BSD 3-Clause "New" or "Revised" License
4.49k stars 448 forks source link

`PrivateKeyJWT.headers` field is not passed to `private_key_jwt_sign` function call in `PrivateKeyJWT.sign` method #515

Closed lycantropos closed 1 year ago

lycantropos commented 1 year ago

Describe the bug

In commit 49c5556d8b2c7e4b8939e502fefd816bf766dfc3 headers parameter got re-introduced (previously known as header) and it is passed to client_secret_jwt_sign function call in ClientSecretJWT.sign method, but it is not passed to private_key_jwt_sign function call in PrivateKeyJWT.sign method, why is it so?

Also both client_secret_jwt_sign & private_key_jwt_sign eventually call sign_jwt_bearer_assertion which doesn't have headers parameter, but only header, so it looks to be skipped, is it expected?

Expected behavior

headers parameter is passed to private_key_jwt_sign function call in PrivateKeyJWT.sign method

class PrivateKeyJWT(ClientSecretJWT):
    ...
    def sign(self, auth, token_endpoint):
        return private_key_jwt_sign(
            auth.client_secret,
            client_id=auth.client_id,
            token_endpoint=token_endpoint,
            claims=self.claims,
            header=self.headers,
            alg=self.alg,
        )

Environment:

dhallam commented 1 year ago

@lepture https://github.com/lepture/authlib/pull/552 is ready for review.

jmacdone commented 1 year ago

I was going nuts trying to add a x5t header for https://learn.microsoft.com/en-us/azure/active-directory/develop/certificate-credentials#assertion-format until I found this. I'm happy to see a PR ready for it. 👀

jmacdone commented 1 year ago

FWIW, @dhallam's #552 is working for me. With pip install git+https://github.com/dhallam/authlib@bug/515-rfc7523-apply-headers-while-signing I was finally able to get my access_token.

lepture commented 1 year ago

https://github.com/lepture/authlib/pull/552 is merged.