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

Python-Keycloak version 3.9.1 leads to Import Errors #30

Closed
waza-ari opened this issue Mar 3, 2024 · 5 comments
Closed

Python-Keycloak version 3.9.1 leads to Import Errors #30

waza-ari opened this issue Mar 3, 2024 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@waza-ari
Copy link
Owner

waza-ari commented Mar 3, 2024

python-keycloak has replaced python-jose with jwcrypto in version v3.9.1 based on this PR. While this change is meant to address security issues, it leads to ImportErrors when using this library as we're catching python-jose exceptions.

While the preferred option would be to migrate as well and update dependencies to include a recent enough version (as proposed in #29), there's a bunch of breaking changes that require more careful consideration:

Audience claim

The default behaviour of python-jose was that the aud claim was not required, but would be validated if present. Compare the part in their code base.

jwcrypto doesn't expose that much control anymore, so python-keycloak have made an effort to somehow map the old verify_aud option. This results in a change in default behaviour:

With verify_aud set to True, the authentication now fails if no aud claim is present, which used to work in the old version as require_aud was False. In fact that applies to pretty much all the claims. In my case that did break the application, I don't consider this a minor issue

JWT Decode Options

So far we've been exposing the JWT decode options to the user, see also the docs. This wouldn't be possible anymore, as it would not have any effect with the new version of python-keycloak. We'd need to rewrite the doc section and somehow hope no one is making use of these options.

Around the same topic there's still marcospereirampj/python-keycloak#503 in the upstream repo around backwards incompatibility. Especially the fact that enabling verify_aud (which is currently the default) prohibits exp and nbf validation does concern me.

To summarize, I do see the need for the change, but I'd rather have the upstream python-keycloak changes settled before releasing a new version.

Summary

As a summary we should pin python-keycloak to a version below v3.9.1 for now, until discussions in upstream repo are resolved.

@mcolladoacciona
Copy link

Any update for the usage of python-keycloak v4.0.0? (we need to use this library alongside with python-keycloak and some bug fixes that are in the v4 of the library.

@waza-ari
Copy link
Owner Author

Hi, I wanted to look into it for two weeks already, but didn't find the time yet. I'll try on the upcoming weekend, but can't give any promises at this point

@waza-ari
Copy link
Owner Author

@mcolladoacciona I've just created v1.0.0, its available on Test PyPI. Would you be willing to test it? I'll probably release it later today after some more testing myself.

@mcolladoacciona
Copy link

I've tried on the secondary repository that I have where we implemented this library and so far so good. On the main repository where we want to implement this library, one of the stoppers was to have this limitation about the version for the python-keycloak library, because we strongly use it as an middleware between the instance of keycloak and the developer platform. On next iterations of the API we'll change all the internals to implement this library.

@waza-ari
Copy link
Owner Author

After running the updated version successfully in our project and getting one more positive feedback from here I just approved the production release for v1.0.1. Should be available within a couple of minutes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants