`aud` is using `client_id` by default

[hassaanalansary]

Hello, we are implementing keycloak at my job, and we decided to use your awesome library.

I have an issue/question.
I created a client called django-client in keycloak and left all the default configurations as is.
When setting LOCAL_DECODE=True the JWT authorization fails because the keycloak sends aud=['account'] however,
you have

if audience is None:
    audience = self.client_id # "django-client" in my case
....
payload = jwt.decode(token, key=key, algorithms=['RS256'], audience=audience, options=options)

This results in an error because the django-client is not in the intended aud coming from keycloak

I was able to add a custom Dedicated Scope to my Client with the same name (django-client), following the answers here. making aud=['account', 'django-client'] which makes it pass the JWT check.
but I think this is not a real solution.

from my research all created Clients have aud='account' by default from here
and you can remove this aud if you want and assign custom scopes instead.

I think that using audience = self.client_id by default is not correct here.

if audience is None:
    audience = self.client_id

• It should be either skipped if None

if audience is None:
    audience = None

• OR should use "account"

`aud` should be either skipped if None 
```python 
if audience is None:
    audience = 'account'

• OR we can add LOCAL_DECODE_AUD in the config that should indicate that the developer wants to enforce a specific aud, which could be anything "account", CLIENT_ID, "www.supersecretwebapp.com", etc..

I am not sure if my issue is very coherent and up to code. please reach out if you need more info about this.

P.S. A useful resource about JWT aud vs client-id here

[marcelo225]

@hassaanalansary, I agree with the second option, in other words:

@hassaanalansary and @chmoder, I created a PR to fix it.

Could you review it?

Thanks

[hassaanalansary]

Thank you gor your response, I think that is the easiest solution.
I did exactly that in my codebase

Left a trivial comment

[chmoder]

Thanks @marcelo225,

I am not 100% sure what the default should be (if any) based on my research in PyJWT. I think any default is okay. But it seems that different configurations will have different audiences. What are your thoughts on exposing the audience parameter to the middleware and public methods?

For example, we use this method to authenticate a user and the middleware for authorization. So the user_info method could take pass through options and then any audience would be usable.

django-keycloak-auth/django_keycloak_auth/keycloak.py

Line 259 in 5aa6676

def userinfo(self, token, raise_exception=True):

[marcelo225]

@chmoder, could you verify this PR? I've been swamped lately. I can't fix it at the moment. I wanna advise you all it doesn't look like I'm not working on the project

[chmoder]

@chmoder, could you verify this PR? I've been swamped lately. I can't fix it at the moment. I wanna advise you all it doesn't look like I'm not working on the project

Thank you for the awareness. I will make time for it this week.

[chmoder]

Hi @hassaanalansary, thank you for the question and suggestions. I think I understand what you are asking here. From the stack overflow article you referred to:

In Keycloak, by default, the claim aud already contains a reference to the client account. This happens because by default:

1. a newly created client comes with an unfiltered scope (i.e., has scope on all roles)
2. users are assigned with client roles from the account client, which provides access to the account console of Keycloak.

So if I understand correctly, this library has a setting to set what the client_id or ("account client" from above) is. It is the KEYCLOAK_CLIENT_ID field. Described in the usage section of the documentation.

Setting that field to account should give you the behavior you expect. That conditional in the decode method will always use the value in settings.py because there are no function references that pass an audience argument. It is a place holder for future development to allow for overriding the value.

I was going to write some changes to support your request but it doesn't look like that will be necessary. Sorry for the long post. Please let me know if that makes sense to you as well. ✋

[hassaanalansary]

I am not sure that I am 100% following.
But from my understanding,
If I use account, everything should work fine.
I agree, but in my case I prefered to create a specific client dedicated to my django backend.

Later I had to use admin-cli for other reasons and I don't think that it will pass the aud check as well

I believe that audshould not be tied to client.

In other words, the client used by the backend doesn't need to be related to the user's client.

Yes keycloak has a default behavior (sometimes not documented) but that doesn't mean that I need to stick to the default behavior.

Right now I am inheriting and overriding this code to make it work for my use case.
I think it should configurable in an easier way.

[chmoder]

I am sorry for the trouble this caused you. The underlying jwt.decode method requires an audience value. It will raise an exception if the value is not an instance of str, Iterable, or None. If client_id is None and there is an aud key in the payload, it will also raise an exception. RFC 7519.

The use of the claim is optional, and verifying it can be disabled by setting the option verify_aud: False. However that is not implemented.

The creator of the token does not enforce that the aud is validated. That is the responsibility of the system decoding the token. So, you may setKEYCLOAK_CLIENT_ID to any value your aud needs to be. Or we can add a little implementation to cover the final case of the RFC to optionally disable verifying the aud field in the token payload.

[chmoder]

@marcelo225, I think this pr is able to be closed after this discussion. If we want we can open a new issue with a label, "feature request". To

add a little implementation to cover the final case of the RFC to optionally disable verifying the aud field in the token payload.

This pr appears to have some related work. Maybe we can review that one first.

[marcelo225]

@marcelo225, I think this pr is able to be closed after this discussion. If we want we can open a new issue with a label, "feature request". To

add a little implementation to cover the final case of the RFC to optionally disable verifying the aud field in the token payload.

This pr appears to have some related work. Maybe we can review that one first.

Ok, @chmoder I'll close this pr
Sure! Could you review this pr?

Thank you for helping!