Closed dnsv closed 1 year ago
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.
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:
- ... 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?
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?
Do you mean like if an app is only using RSA based JWTs, so they can just leave the hashing on?
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.
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.
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 thejwk_key
that's used for signing the token. It's generated with the client secret, which is being hashed sincev2.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
Setup Django (I'm using Python 3.11):
poetry install
poetry run python manage.py runserver
poetry run python manage.py migrate
poetry run python manage.py createsuperuser
.frontend/.env
:Setup Next.js (I'm using node v18.12.1, but i probably works the same with lower versions).
cd frontend
yarn install
yarn dev
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:
Expected behavior
Being able to login :)
Version
Additional context
N/A