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

SimpleJWT integration doesn't take into account the settings AUTH_HEADER_NAME #474

Closed
Smixi opened this issue Aug 2, 2021 · 14 comments
Closed
Labels
bug Something isn't working fix confirmation pending issue has been fixed and confirmation from issue reporter is pending

Comments

@Smixi
Copy link

Smixi commented Aug 2, 2021

Hi, just figuring this out today because I had to use a custom header for simplejwt auth

Describe the bug
SimpleJwt can be configured by using a dict in the settings.py of a django project.
SIMPLE_JWT = { 'AUTH_HEADER_NAME' : "HTTP_X_TOKEN" # translate to X-token as header. }
But the current implementation doesn't take this settings into account :

`class SimpleJWTScheme(OpenApiAuthenticationExtension):
target_class = 'rest_framework_simplejwt.authentication.JWTAuthentication'
name = 'jwtAuth'

def get_security_definition(self, auto_schema):
    from rest_framework_simplejwt.settings import api_settings

    if len(api_settings.AUTH_HEADER_TYPES) > 1:
        warn(
            f'OpenAPI3 can only have one "bearerFormat". JWT Settings specify '
            f'{api_settings.AUTH_HEADER_TYPES}. Using the first one.'
        )
    return {
        'type': 'http',
        'scheme': 'bearer',
        'bearerFormat': api_settings.AUTH_HEADER_TYPES[0],
    }`

Where the return should become something I guess like
{'type':'apiKey', 'in': 'header', 'name': api_settings.SIMPLE_JWT['AUTH_HEADER_NAME']}

To Reproduce

Change simplejwt header setting.

Expected behavior

Authentication should scheme should follow simplejwt settings.

I may have time to make a PR if needed.

@tfranzel
Copy link
Owner

tfranzel commented Aug 2, 2021

Hi, this should actually already be fixed in 0.17.3 i believe. i fixed that along with #467. can you confirm it is not working for the most recent version?

@Smixi
Copy link
Author

Smixi commented Aug 2, 2021

Hi, this should actually already be fixed in 0.17.3 i believe. i fixed that along with #467. can you confirm it is not working for the most recent version?

I think it fixed only the type (JWT, Bearer), not the header name : While using a custom name :
image

But using this (custom class for my project):

 return {
            'type': 'apiKey',
            'in': 'header',
            'name': getattr(settings, 'SIMPLE_JWT')['AUTH_HEADER_NAME'],
        }
(using a higher priority)

image

Note that as its not the Authorization header, it cannot append Bearer, and client will need to do that manually.

@Smixi
Copy link
Author

Smixi commented Aug 2, 2021

Hi, this should actually already be fixed in 0.17.3 i believe. i fixed that along with #467. can you confirm it is not working for the most recent version?

Wait, lemme check with the most updated version, I may an older one.

@tfranzel
Copy link
Owner

tfranzel commented Aug 2, 2021

the code snippet you posted is not the current state of the code:

header_name = getattr(api_settings, 'AUTH_HEADER_NAME', 'HTTP_AUTHORIZATION')

this was changed prior to 0.17.3 release

@Smixi
Copy link
Author

Smixi commented Aug 2, 2021

It is half working for the last 0.17.3👍 . But I have an error with the same, my key is the following : HTTP_IOC_ACCESS_TOKEN, but it translates to :
image

Which give me an error when trying to pass the token (as the header is not recognized by simplejwt) :

image

@tfranzel
Copy link
Owner

tfranzel commented Aug 2, 2021

Due to limitations in the OpenAPI 3.0.3 specification, you need to actually prefix the value with AUTH_HEADER_TYPES.

That means you need to put Bearer XYZTOKEN into that field. that is what that comment is trying to tell you. does it work then?

@Smixi
Copy link
Author

Smixi commented Aug 2, 2021

I may need to check out but right know the header is not recognized (other way it would also say wrong credential), even with Bearer. I'll try out by changing the name

@tfranzel
Copy link
Owner

tfranzel commented Aug 2, 2021

i suggest trying to construct a curl command that works and work back from there.

all i can think of at this point is maybe the casing of the header name, but that should in theory work as it does for HTTP_AUTHENTICATON and Authentication: ....

@Smixi
Copy link
Author

Smixi commented Aug 2, 2021

The header is using underscore instead of dash, using :
image
it works, but using
image
it does not.

@Smixi
Copy link
Author

Smixi commented Aug 2, 2021

Using Curl : ➜ ioc git:(develop) ✗ curl -X 'GET' \

'http://127.0.0.1:8000/ioc-manager/api/iocs/'
-H 'accept: application/json'
-H 'Ioc_access_token: Bearer toto'
-H 'X-CSRFToken: u0dGquwTML5cL3GRTUT6ZXZUqEtVzKLN81nrorTURgUAqYeWadFrWbPQt3EgEFTK'
{"detail":"Authentication credentials were not provided."}%
➜ ioc git:(develop) ✗ curl -X 'GET'
'http://127.0.0.1:8000/ioc-manager/api/iocs/'
-H 'accept: application/json'
-H 'Ioc-access-token: Bearer toto'
-H 'X-CSRFToken: u0dGquwTML5cL3GRTUT6ZXZUqEtVzKLN81nrorTURgUAqYeWadFrWbPQt3EgEFTK'
{"detail":"Given token not valid for any token type","code":"token_not_valid","messages":[{"tokenClass":"AccessToken","tokenType":"access","message":"Token is invalid or expired"}]}%

@tfranzel
Copy link
Owner

tfranzel commented Aug 2, 2021

ok so this is a django/nginx gotcha:

https://github.com/django/django/blob/ca9872905559026af82000e46cde6f7dedc897b6/django/core/servers/basehttp.py#L181

headers containing underscores are discarded apparently. we would need to convert AUTH_HEADER_TYPES underscores to dashes in addition to the capitalize() call. django then convert dashes to underscores and everything should work

@Smixi
Copy link
Author

Smixi commented Aug 2, 2021

Exactly (I was surpised by not being able to see the header in the meta field ^^'). !

@tfranzel
Copy link
Owner

tfranzel commented Aug 2, 2021

that should do it. release will take circa 2 weeks as i like to collect a few issues per release. please test if the fix works as expected.

@tfranzel tfranzel added bug Something isn't working fix confirmation pending issue has been fixed and confirmation from issue reporter is pending labels Aug 2, 2021
tfranzel added a commit that referenced this issue Aug 2, 2021
@Smixi
Copy link
Author

Smixi commented Aug 2, 2021

Works fine for me with this code :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fix confirmation pending issue has been fixed and confirmation from issue reporter is pending
Projects
None yet
Development

No branches or pull requests

2 participants