django-oauth/django-oauth-toolkit

Invalid JWT signature because of hashed client secret

Closed this issue · 6 comments

dnsv commented

Describe the bug

I'm using next-auth on the frontend and the latest django-oauth-toolkit with OIDC enabled and HS256 as the algorithm on the backend. After authorizing my oauth application I get the error "failed to validate JWT signature" in next-auth.

The JWT token provided by django-oauth-toolkit seems to be incorrect. It's prepared in oauth2_provider.oauth2_validators.finalize_id_token. The culprit is the jwk_key that's used for signing the token. It's generated with the client secret, which is being hashed since v2.0.0.

https://github.com/jazzband/django-oauth-toolkit/blob/04b5b3e49020168d8decc2c536f3287ad40bcfc3/oauth2_provider/oauth2_validators.py#L845

I'm assuming that hashing was added under the assumption that the raw secret isn't needed anymore after it's created, but that doesn't seem to be the case (see line 224).

https://github.com/jazzband/django-oauth-toolkit/blob/04b5b3e49020168d8decc2c536f3287ad40bcfc3/oauth2_provider/models.py#L217-L225

To Reproduce

I've created a test repo: https://github.com/dnsv/auth-test

  1. Setup Django (I'm using Python 3.11):

    1. poetry install
    2. poetry run python manage.py runserver
    3. poetry run python manage.py migrate
    4. poetry run python manage.py createsuperuser.

  2. Login into admin.

  3. Create a new Oauth application:

  4. Create the file frontend/.env:

    OAUTH_CLIENT_ID=...
OAUTH_CLIENT_SECRET=...

  5. Setup Next.js (I'm using node v18.12.1, but i probably works the same with lower versions).

    1. cd frontend
    2. yarn install
    3. yarn dev

  6. Go to http://localhost:3000/ and try to sign it. You'll get the error shown in the console after the authorization step.

Highlights from the backend setup:

# backend/base/settings.py

OAUTH2_PROVIDER = {
    "OIDC_ENABLED": True,
    "OAUTH2_VALIDATOR_CLASS": "backend.oauth2.utils.CustomOAuth2Validator",
    "SCOPES": {
        "openid": "OpenID Connect scope",
    },
}

# backend/oauth2/utils.py

from oauth2_provider.oauth2_validators import OAuth2Validator

class CustomOAuth2Validator(OAuth2Validator):
    oidc_claim_scope = None

    def get_additional_claims(self, request):
        return {
            "id": request.user.id,
            "given_name": request.user.first_name,
            "family_name": request.user.last_name,
            "name": f"{request.user.first_name} {request.user.last_name}",
            "email": request.user.email,
        }

Expected behavior

Being able to login :)

Version

  • I have tested with the latest published release and it's still a problem.
  • I have tested with the master branch and it's still a problem.

Additional context

N/A

n2ygk commented

Ouch! We'll have to back off the hashing I guess. That will be a complicated migration. Maybe with a setting to enable/disable hashing and some code to deal with a hashed or unhashed secret.

gardenerik commented

If I understand the spec correctly, the ID token must be signed by the secret key (if using MAC algorithm such as HS256).

As stated in 3.1.3.7. ID Token Validation:

  1. ... the octets of the UTF-8 representation of the client_secret ... are used as the key to validate the signature.

So that would make DOT 2.0+ not spec complaint.

I would like to open a PR to resolve this issue, but I am not certain how should that be handled. We obviously cannot revert the changes, as hashing is a one-way operation.

Secrets for new applications should be stored in plain text. We will also need to continue to support the use of hashed secrets in requests. We should also allow a simple way to regenerate existing secrets if wanted by the user.

Is there anything I am missing?

n2ygk commented

Hashing was added erroneously by not considering the requirement to sign jwt's. Perhaps undo hashing of new secrets and retain the ability to check old hashed ones. I think this should just work for hashed or clear secrets because the password check looks for the has prefix.

Perhaps make it an option when creating an application to disable hashing if the app will never use jwts?

gardenerik commented

Do you mean like if an app is only using RSA based JWTs, so they can just leave the hashing on?

n2ygk commented

Many apps do not use jwts at all. They were obtain and provide an oauth2 Bearer access token and the backend contacts the AS to validate it.

gardenerik commented

I see.. Ok, so I will open a PR in the following days in which I will add support to use both hashed and unhashed secrets with the option to enable/disable hashing them on save.

  • 👍1