Hydra connect fails when the client secret contains "%"

[gr-eg]

Given a client with a secret of secr%et

Using hydra connect fails because this line tries to unescape the secret but it contains an invalid escape sequence.

It looks like this is handled in the express consent app by escaping the id and secret but this isn't done with hydra connect

[aeneasr]

Ah yes, this should have been fixed with https://github.com/golang/oauth2/commit/13449ad91cb26cb47661c1b080790392170385fd and is probably not included here because the lock file is outdates. I'm currently working on a PR that will also replace glide with dep and I'll make sure to include this change as well.

Thank you for the report!

[gr-eg]

It looks like this has potentially regressed in v0.10.10.

If a % is present in the client secret then invalid_client is raised from: https://github.com/ory/fosite/blob/master/access_request_handler.go#L87

I guess the problem is largely mitigated due to % not being present in the charset for secret generation but It's still possible to manually pass a failing secret via hydra clients create.

[aeneasr]

Hm, not sure, the client secret is being unescaped here.

If your secret contains % (e.g. foo%bar) you should encode it so it becomes %25 (e.g. foo%25bar)

[ruhavingfun22]

Note I have also had problems with '%' and with '$' (on version v0.10.0-alpha.10). This bit me pretty hard, as one of my deployments was using this as the root client credential. I spent a couple hours trying a lot of permutations of escaping my secrets for the root, as well as escaping when trying to call it from the CLI, before I eventually gave up on getting these two characters to work... Would be very interested if someone figured out the right combination of escaping to make this work.

[aeneasr]

@ruhavingfun22 did you manage to get it done? IMHO it should simply work with e.g. https://meyerweb.com/eric/tools/dencoder/

[ruhavingfun22]

@arekkas I did not. I did try with the correct encodings from the site you mention, but something in the docker string parameterization, http request or how the DB was storing the special character didn't agree. I'm not saying it isn't possible to make it work, but after almost 2 hours of trying, it just wasn't worth the effort to make it work. All of the other special characters work without having to specify any encoding, so it was easier to just use those.

[aeneasr]

That sounds really frustrating, I'll take another look at this -> reopening

[aeneasr]

hydra connect has been deprecated on master. If the issue persists with flag --client-id or env var OAUTH2_CLIENT_ID let me know

[wojciechce]

Hi there. Seems like the problem mentioned in this issue is still valid. We are looking forward to hearing your opinion on that. Thank you in advance!

Summary: Special characters like '%Q', '%n' in client_secret are causing Hydra returns 400.

Steps to reproduce:

1. Follow the 5 minute tutorial
2. Create a client with password containing '%Q' or '%n':

   docker exec -it hydra_hydra_1 \
   hydra clients create \
   --endpoint http://localhost:4445 \
   --id percent-sign-client \
   --secret 'foo%Qbar' \
   --grant-types authorization_code,refresh_token \
   --response-types code,id_token \
   --scope openid,offline \
   --callbacks http://127.0.0.1:5555/callback

3. Setup home route on http://127.0.0.1:5555/ (tutorial)
4. Open http://127.0.0.1:5555/ in your browser.
5. Stop serving home route by pressing CTRL+C.
6. Authenticate, login and when your browser served an error, copy 'code' from params of URL.
7. Prepare POST request, replace {{code}} with code from URL:

   POST /oauth2/token HTTP/1.1
   Host: localhost:4444
   Authorization: Basic cGVyY2VudC1zaWduLWNsaWVudDpmb28lUWJhcg==
   Cache-Control: no-cache
   Content-Disposition: form-data; name="grant_type" authorization_code
   Content-Disposition: form-data; name="code" {{code}}
   Content-Disposition: form-data; name="redirect_uri" http://127.0.0.1:5555/callback
   Content-Disposition: form-data; name="refresh_token"

8. Execute request.
9. Hydra responds with error:

   {
   "error": "invalid_request",
   "error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed",
   "error_hint": "The client secret in the HTTP authorization header could not be decoded from \"application/x-www-form-urlencoded\".",
   "status_code": 400,
   "error_debug": "invalid URL escape \"%Qb\""
   }

[aeneasr]

base64decode("cGVyY2VudC1zaWduLWNsaWVudDpmb28lUWJhcg==") is percent-sign-client:foo%Qbar but should be urlencode("percent-sign-client:foo%Qbar") which is percent-sign-client%3Afoo%25Qbar and would be base64encoded cGVyY2VudC1zaWduLWNsaWVudCUzQWZvbyUyNVFiYXI=. See here for spec.

[michalwojciechowski]

@aeneasr please confirm if my understanding is correct: https://tools.ietf.org/html/rfc6749#appendix-B assumes that our credentials represent a valid octet sequence and treats all percentage sings as the special unicode signs (followed by other characters). Since this is not always true, we need to escape all special characters with e.g. urlencode.

[aeneasr]

Yes! I've worked on PRs for JS and Go OAuth2 libraries that got that wrong. Using special chars like % is uncommon for OAuth2 so most have not encountered this before. My recommendation is to skip these special chars and only have url-safe chars. This will avoid issues with other libraries that get this wrong.

[wojciechce]

@aeneasr As you wrote above, I created new client:

docker exec -it hydra_hydra_1 \
    hydra clients create \
    --endpoint http://localhost:4445 \
    --id percent-sign-client \
    --secret 'foo%Qbar' \
    --grant-types authorization_code,refresh_token \
    --response-types code,id_token \
    --scope openid,offline \
    --callbacks http://127.0.0.1:5555/callback

Then I extecuted request:

POST /oauth2/token HTTP/1.1
Host: localhost:4444
Authorization: Basic cGVyY2VudC1zaWduLWNsaWVudCUzQWZvbyUyNVFiYXI=
Cache-Control: no-cache

Content-Disposition: form-data; name="grant_type" authorization_code
Content-Disposition: form-data; name="code" {{code}}
Content-Disposition: form-data; name="redirect_uri" http://127.0.0.1:5555/callback
Content-Disposition: form-data; name="refresh_token"

And then Hydra returns:

{
    "error": "invalid_request",
    "error_description": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed",
    "error_hint": "Client credentials missing or malformed in both HTTP Authorization header and HTTP POST body.",
    "status_code": 400
}

Please confirm that is correct Hydra behaviour or I did something wrong (maybe client_secret in Hydra should be urlEncoded too).

[michalwojciechowski]

Thank you! Wasn't aware that this is such a big deal in OAuth world. We will definitely take your recommendation into account, perhaps this is the most convenient way for everyone.

[aeneasr]

Not sure what's going on at @wojciechce but here:

docker exec -it hydra_hydra_1 \
    hydra clients create \
    --endpoint http://localhost:4445 \
    --id my-client1 \
    --secret 'sec%25ret' \
    -g client_credentials

docker exec -it hydra_hydra_1 \ 
    hydra token client \
    --endpoint http://localhost:4444 \
    --client-id my-client1 \
    --client-secret 'sec%ret'
kP8IPu3us--Hkp4hMpXc42CxMI-i8uLaKSPV3MYPmnU.udw5KphRBxYIJU9wx92VD4ZIcI1XrTP5ySyGehievUE

$ curl -X POST \
>   http://localhost:4444/oauth2/token \
>   -H 'authorization: Basic bXktY2xpZW50MTpzZWMlMjVyZXQ=' \
>   -H 'cache-control: no-cache' \
>   -H 'content-type: application/x-www-form-urlencoded' \
>   -H 'postman-token: 992fd5fe-c31d-504f-b7ec-928429b7ad55' \
>   -d grant_type=client_credentials
{"access_token":"C3I_XRIK9nR4PtV_PFnGyy9WtZlI6JUv70BjCIDCtnw.vPlsAFM11pYh1XsL58Q7kql9WMTmIWnJ_iUB50nTT3M","expires_in":3599,"scope":"","token_type":"bearer"}

So everything is working as expected on our end.

[aeneasr]

I made a mistake earlier, we actually do not encode the whole username:password but instead base64(urlencode(username):urlencode(password)). Hope that settles this issue at last.

[wojciechce]

It works! Great! I confirm. Solution is base64(urlencode(client_id):urlencode(client_secret)).

@aeneasr Thank you for your time.

[aeneasr]

No problem :)