search

nodeSolidServer / oidc-op

OpenID Connect Provider for Node.js
MIT License
5 stars 7 forks source link

Fixed not completing refresh tokens #30

Closed jaxoncreed closed 3 years ago

jaxoncreed commented 3 years ago

A fix for https://github.com/solid/node-solid-server/issues/1533.

Note this has not been thoroughly tested

matthieubosquet commented 3 years ago

LGTM! :)

bourgeoa commented 3 years ago

https://github.com/solid/oidc-op/blob/72e4cfa7870aab7913314cbbe5277d0bb559dcf8/test/handlers/TokenRequestSpec.js#L1366 Is there a possibility to fill the test ?

jaxoncreed commented 3 years ago

Okay added tests

bourgeoa commented 3 years ago

Wonderful

matthieubosquet commented 3 years ago

@bourgeoa do you think you could cut a patch release in order to test the fix on NSS on our side?

bourgeoa commented 3 years ago

@matthieubosquet Ok I shall do it first on the https://solicommunity.net:8443 test server.

matthieubosquet commented 3 years ago

Thank you @bourgeoa! :)

bourgeoa commented 3 years ago

@jaxoncreed @michielbdejong Can one of you add me to npm. I cannot publish. If not can you publish 0.11.2 I have updated the package.json to 0.11.2 and dependencies.

bourgeoa commented 3 years ago

@matthieubosquet

Note this has not been thoroughly tested

Could you check that refresh token is working as expected on the test-server https://solidcommunity.net:8443 ?

@jeff-zucker Where you also expecting this to work ? If yes could you test it ?

matthieubosquet commented 3 years ago 
POST /token HTTP/1.1
Host: solidcommunity.net:8443
Content-Type: application/x-www-form-urlencoded
Content-Length: 161
grant_type=refresh_token&refresh_token=XXX&client_id=XXX&client_secret=XXX

Still gives a 500 error I'm afraid.

bourgeoa commented 3 years ago

@matthieubosquet Sorry I missed the update should be correct now. Could you retry

matthieubosquet commented 3 years ago

@bourgeoa @jaxoncreed apparently, we're still missing 1 thing: returning an ID token. @NSeydoux suggested a fix there: https://github.com/solid/oidc-op/pull/32

Could you see if it makes sense/push/deploy?

bourgeoa commented 3 years ago

@matthieubosquet I am fully ignorant in this area so : I manually patched https://solidcommunity.net:8443 with @NSeydoux fix. Could you check the result.

If OK I could push/deploy

matthieubosquet commented 3 years ago

@NSeydoux 's fix works! ;)

@bourgeoa it would be fabulous if you could get https://github.com/solid/oidc-op/pull/32 merged!