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

Asymmetric JWT key decoding and encoding not work. #269

Open
Tracked by #5
LinnaViljami opened this issue Jan 13, 2023 · 5 comments
Open
Tracked by #5

Asymmetric JWT key decoding and encoding not work. #269

LinnaViljami opened this issue Jan 13, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@LinnaViljami
Copy link

Description

There are multiple problems

  • By default Django secret key is used to decode and encode JWT keys, When using asymmetric keys, we should be able to use different key to verify token (public key) and another key to generate token (private key)
  • Documentation and project examples is lacking asymmetric key usage
  • Documentation does not tell anything about the JWT_DECODE_HANDLER
@nrbnlulu
Copy link
Owner

nrbnlulu commented Jan 14, 2023

Hey, docs are not the strongest part of this lib yet 😄, I am still expirimenting with the API due to lack of support either from strawberry (extensions for subscriptions, and real on_request hook) or from graphql that you can't cleanly return error types (@oneOf, Unions are too ugly)...

As for Asymmetric JWT this would be great to support this built-in, I have no experience with it and currentlly in my project I didn't found a need for that so this is low-priority on my end, though PR's are allways welcomed 😄 .

Documentation does not tell anything about the JWT_DECODE_HANDLER

As you can see from the type hints (or from the default hook) you should return TokenType from str which is the token you got from the headers

JWT_DECODE_HANDLER: Callable[[str], "TokenType"] = decode_jwt

@nrbnlulu nrbnlulu added the enhancement New feature or request label Jan 14, 2023
@LinnaViljami
Copy link
Author

Yes.. I already solved the problem by defining custom JWT_PAYLOAD handler, JWT_TOKEN_FINDER and JWT_DECODE_HANDLER.

The basic reason and misunderstanding in the library's code, is that you assume JWT generating string and JWT verifying string be the one same string (by default DJANGO_SECRET_KEY) To deal with asymmetric keys we should add an extra setting, and use different key to generate and verify JWT.

Asymmetric keys enable single-sing-on (SSO) architecture implementation using this library. So I use strawberry-django-auth in my centralized identity provider server and generate JWT tokens using the private key.

To verify tokens we should instead use a public key, that can be handed over to the other servers, so they can not generate new tokens but instead just validate existing tokens

@LinnaViljami
Copy link
Author

So jwt verifying and generating functions should be use public and private key settings by default if JWT_ALGORIHTM parameter is asymmetric

def custom_decode_jwt(token: str) -> "TokenType":
    from gqlauth.core.utils import app_settings
    from django.conf import settings
    from gqlauth.jwt.types_ import TokenPayloadType, TokenType

    decoded = json.loads(
        jwt.decode(
            token,
            key=cast(str, settings.JWT_PUBLIC_KEY),
            algorithms=[
                app_settings.JWT_ALGORITHM,
            ],
        )["payload"]
    )
    return TokenType(token=token, payload=TokenPayloadType.from_dict(decoded))

@LinnaViljami
Copy link
Author

LinnaViljami commented Jan 17, 2023

Solution proposal

  1. JWT_SECRET_KEY should be separated into two different settings (for example JWT_ENCRYPT_KEY & JWT_VERIFY_KEY) To make it more understandable we can even split the setting into three different settings, and use one when the algorithm is not asymmetric and other ones when it is asymmetric.
  2. By default we can set all settings to set DJANGO_SECRET_KEY to make it simple to use without asymmetric keys. On the other hand we could leave two of the three settings empty by default, and throw an error if user defines an asymmetric algorithm to be used, but does not provide the required settings.
  3. In jwt functions we should use JWT_VERIFY_KEY and JWT_ENCRYPT_KEY accordingly. Encrypt key should be used only if we generate new tokens. It should be a private key. Verify key should be a public key paired with the private key.

What else

  • We could name the new settings to JWT_PUBLIC_KEY and JWT_PRIVATE_KEY to help users understand how to use these settings accordingly
  • Documentation and example should be provided into repo
  • Unit tests

@nrbnlulu
Copy link
Owner

I like your suggestion, Hopefully by the end of the week i'll have time to get into it, feel free to submit a PR yourself.

@nrbnlulu nrbnlulu mentioned this issue Jan 17, 2023
6 tasks
@nrbnlulu nrbnlulu added this to the v1.0 milestone Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants