ServletUtils#createHTTPRequest loose parameters in case of POST request

[Thomas Mortagne]

Either that or AuthorizationGrant#parse does not get parameters from the right place in case of POST request.

My use case is the following:

• I sent a POST request with the following parameters: grant_type=authorization_code, code=MZGYbIc_Vrc0xXb7cRlGZx7PELdM1FWLtnIAo6WjRQQ, redirect_uri=http%3A%2F%2Ftoto
• I receive the request in a Servlet and use ServletUtils#createHTTPRequest
• I use TokenRequest#parse(HTTPRequest)

-> I get the following exception:

#!

com.nimbusds.oauth2.sdk.ParseException: Missing "grant_type" parameter
    at com.nimbusds.oauth2.sdk.AuthorizationGrant.parse(AuthorizationGrant.java:87)
    at com.nimbusds.oauth2.sdk.TokenRequest.parse(TokenRequest.java:405)
        ...

In the meantime looks like the workaround is to recreate the URI with the parameters in it (which does not make much sense for a POST request, would probably be cleaner to to the opposite and have a map of parameters extracted from the URI in case of GET request).

Note: I can provide a pull request if the choice is to recreate the URI with the parameters in it.

[Thomas Mortagne]

Ok forget that, I just forgot the part about parameters put in the request content with application/x-www-form-urlencoded...

Sorry for the noise.

[Thomas Mortagne]

Closing as invalid

[Connect2id OSS]

Hi Thomas,

If you construct the token request with the TokenRequest class, this will take care of setting the expected content-type header.

Do you think the JavaDocs need fixing / updating regarding your issue?

[Thomas Mortagne]

Actually the problem is still valid since Servlet request is consuming the content in case of application/x-www-form-urlencoded. So createHTTPRequest should serialize Servlet#getParameterMap() instead of copying the content.

[Thomas Mortagne]

Yes I agree TokenRequest is doing the right thing but then Servlet move the parameters from the content to the request parameters map.

[Thomas Mortagne]

Note in case it's not clear: I'm on provider side in a Servlet. I'm not creating a token request, I'm reading one.

[Connect2id OSS]

Hi Thomas,

We use TokenRequest.parse(ServletUtils.createHTTPRequest(servletRequest)) in our Connect2id server, and it's been working without problems.

Could you perhaps submit a test case?

Servlet.getParameterMap() is not a good idea, because there are OAuth / OIDC requests that expect JSON in the POST / PUT body.

What servlet container are you using BTW?

[Thomas Mortagne]

I'm currently using Tomcat 8.0.33 for my tests and I saw various references of this request content being emptied with application/x-www-form-urlencoded while searching on Google.

Here is my detailed use case to reproduce the issue:

• send the following request:

#!


POST http://127.0.0.1:8080/xwiki/oidc/token
Content-Type: application/x-www-form-urlencoded
grant_type=authorization_code&code=MZGYbIc_Vrc0xXb7cRlGZx7PELdM1FWLtnIAo6WjRQQ&redirect_uri=http%3A%2F%2Ftoto&client_id=clientid

• in the servlet use ServletUtils#createHTTPRequest(sr)

-> createHTTPRequest find an empty body (but I can see the parameters when calling getParameterMap())

You can see my workaround on https://github.com/xwiki-contrib/oidc/blob/master/oidc-provider/src/main/java/org/xwiki/contrib/oidc/provider/internal/util/OIDCServletUtils.java#L56.

Servlet.getParameterMap() is not a good idea, because there are OAuth / OIDC requests that expect JSON in the POST / PUT body.

I'm only talking about application/x-www-form-urlencoded requests

[Thomas Mortagne]

I created the following pull request as another way to explain the issue: https://bitbucket.org/connect2id/oauth-2.0-sdk-with-openid-connect-extensions/pull-requests/10/issue-88-servletutils-createhttprequest/diff

[Vladimir Dzhuvinov]

Your patch got merged and a new v5.8.2 got pushed to Maven Central. I wasn't aware that a servlet container may strip content like this (we use Tomcat too), so this should make the SDK more reliable across deployments.

Thanks for contributing!

Vlad

[Thomas Mortagne]

Your patch got merged and a new v5.8.2 got pushed to Maven Central

Great, thanks ! Removing my workaround :)

[Thomas Mortagne]

Note: I digged a bit more and this happen because of a generic filter (several actually) in front of my service which are calling ServletRequest#getParameter() which in turn consume the content (it has to do get the parameters). That should be true for any application server but indeed if you don't have any filter and nobody call getParameter or other method related to paraleters) you should not see this. Thing is we have a few filters like this in XWiki for various reasons...

One problem my workaround does not fix is that Jetty for example parse the content with getInputStream() and return an error when getReader() is then used in createHTTPRequest (Tomcat does not care it just behave as if the content was empty). Looking into that.

I guess the safest would be to simply always use getParameterMap() and not even try to read the content at all in case of application/x-www-form-urlencoded content. Doing more tests and will provide a new pull request.

Created #186 since #184 has been released.

[Connect2id OSS]

Thanks for keeping us posted on this.

A warning to developers about upstream filters was put in the javadocs -> commit d8d881b.

Please, keep us updated on this if something new comes out.

[Andrii Deinega]

@Vladimir Dzhuvinov , one of the major issues with getParameterMap() in

// Recreate the content based on parameters

request.setQuery(URLUtils.serializeParametersAlt(sr.getParameterMap()));

is that it returns parameters passed in the query string and this is something you don’t want to get & handle from a request to the token endpoint. Java Servlet API Specification explicitly tells about this behavior or quirk if you will.

Moreover, some Java frameworks, for example, Jersey empties getParameterMap() for POST requests so you never get intermixed parameters (I mean query and form parameters).

I understand this bug was closed a quite long ago but hope you still might find this information useful.

‌