Closed MFAshby closed 4 years ago
I agree it should be handled, but i'd like to point out that as-is it's not declared as supported, both docs and types declare only GET and POST are supported.
I'm fine with adding more supported http methods but that said we're also only supporting x-www-form-urlencoded
bodies and that'll likely have to be extended too.
Ah thanks for the response. I hadn't noticed that only GET and POST were defined in the types https://github.com/panva/node-openid-client/blob/0ab81fa0fae20533996953dd1a278fe6ebcc93ef/types/index.d.ts#L416
I think it would be valuable to support the full range of HTTP verbs, since some REST APIs make use of all of them.
Regarding the content encoding, I seem to be able to send a JSON payload, and set the Content-type header no problem. I am sending the authorization in the header not the body though, so I guess it won't work if I had to send the access_token as part of the body.
Please see d713512 and come back with feedback. Thank you.
Hello, @panva
I've been working with @MFAshby in the same project. After applying the patch from your commit we have a problem related to one of the project dependencies - got
.
The problems occurs in the specific case of a 204 No Content response with its body empty and if the json
option is set to true. In that case we hit https://github.com/sindresorhus/got/blob/v9.6.0/source/as-promise.js#L60. Debugging it, I've checked that response.body
is a truthy empty Buffer
and when JSON-parsing it in the following instruction, it just throws a ParseError: Unexpected end of JSON input
.
Upgrading the dependency to the latest version should solve the problem but we hit something else when doing Issuer.discover
because openid-client
is passing json: true
in a GET method and got
won't like it.
Any suggestions here?
Thanks!
The problems occurs in the specific case of a 204 No Content response with its body empty and if the
json
option is set to true. In that case we hit sindresorhus/got:source/as-promise.js@v9.6.0
#L60 . Debugging it, I've checked thatresponse.body
is a truthy emptyBuffer
and when JSON-parsing it in the following instruction, it just throws aParseError: Unexpected end of JSON input
.
Yeah, you should be able to pass json: false
, form: false
and body
as string
or Buffer
which is already the right serialization.
Upgrading the dependency to the latest version should solve the problem but we hit something else when doing
Issuer.discover
becauseopenid-client
is passingjson: true
in a GET method andgot
won't like it.
This is don't follow at all. ^9.6.0 is the only got
version openid-client depends on, you can't upgrade that because there weren't any more 9.x updates and 10.x would be a breaking dependency update.
Bottom line, tho, i don't want to deal with any of this body / response encoding, so i'll be removing the form
and json
options and instead expect body to be a string or a buffer. The developer will take care of serializing his object to a body payload.
Bottom line, tho, i don't want to deal with any of this body / response encoding, so i'll be removing the form and json options and instead expect body to be a string or a buffer. The developer will take care of serializing his object to a body payload.
This seems reasonable, the only value added when the openid-client library manipulates the body is adding the access token in the correct parameter. I don't think it's worth it for the added complexity.
This is don't follow at all. ^9.6.0 is the only got version openid-client depends on, you can't upgrade that because there weren't any more 9.x updates and 10.x would be a breaking dependency update.
We were hoping that it wasn't a very breaking upgrade, but it was.
I took a stab at this in b4ec0d8, i have to deprecate the resource method because it was just too messy to fix it.
One can now pass headers as an object via options, query as part of the first argument which can now be a URL
instance, body as a buffer or string via options. Return got response body
is now consistently a Buffer.
Thanks @panva , I hope to take a look at this later in the week.
Describe the bug When making an HTTP PATCH request using the
resource()
method of theClient
object, the request params are transmitted as query parameters instead of being included in the request body. MSDN guidelines indicate that POST, PATCH and PUT should all have a request body. This is causing my request to fail, since the resource server in question is expecting a request body.One can see that in the
resource()
method, only a POST request will have a body, and for the other verbs the params are transmitted as query parameters, which I believe is incorrect. https://github.com/panva/node-openid-client/blob/0ab81fa0fae20533996953dd1a278fe6ebcc93ef/lib/client.js#L1020To Reproduce I'll try to publish a minimal reproduction of the issue shortly
Issuer and Client configuration: (inline or gist) - Don't forget to redact your secrets.
Steps to reproduce the behaviour:
Expected behaviour HTTP PATCH and PUT requests made to a resource server using the
resource()
method should have a request body.Environment:
Additional context