Non-conformant error responses

[aeneasr]

Describe the bug

We recently changed the way error_description and other error fields are generated. This unfortunately caused two warnings and one failure in the OpenID Connect Conformity Test Suite, specifically the oidcc-response-type-missing test.

To Reproduce

The full test log can be seen here: test-log-oidcc-response-type-missing-client_secret_basic-discovery-code-default-dynamic_client-8XprPDGkiQ09O8v.zip

WARNING: error_hint is unexpected

I think this can be ignored

error response includes unexpected parameters. This may be because the server supports extensions the test suite is unaware of, or the server may be returning values it should not.

WARNING: error_hint is unexpected

I think this can be ignored

error response includes unexpected parameters. This may be because the server supports extensions the test suite is unaware of, or the server may be returning values it should not.

WARNING error_description has CR LF or TAB

This should be fixed

'error_description' field includes characters CR, LF or TAB, these are not recommended to include

• see: https://bitbucket.org/openid/connect/issues/1147/certification-rfc6749-must-for
• error_description: The authorization server does not support obtaining a token using this method The request is missing the "response_type"" parameter.

ERROR invalid characters in error_description

'error_description' field MUST NOT include characters outside the set %09-0A (Tab and LF) / %x0D (CR) / %x20-21 / %x23-5B / %x5D-7E

• error_description: The authorization server does not support obtaining a token using this method The request is missing the "response_type"" parameter.

Expected behavior

The test should pass.

Environment

ORY Fosite v0.34.1

Additional context

/cc @mitar could you take a look maybe?

[mitar]

Yes, I can. Should we remove error_hint as described in #488? Or it is too early? We have an excuse now. :-)

So should I use my proposed syntax with : as a delimiter between messages?

[aeneasr]

I think we can keep error_hint around for a little longer, but we should eventually deprecate it. My gut says targeting ORY Hydra v1.11.x!

So should I use my proposed syntax with : as a delimiter between messages?

I think it is more standard to have a dot as a delimiter for OAuth2 error messages but if it makes more sense (from a language point) to have : that would also be fine!

[mitar]

I can try dot. Probably in both cases there will be some error message combinations which will be strange to read for a human because of wrong way how things got combined. In go I have seen that errors get combined with : more, so this is why I am assuming it will work for us here as well.

[aeneasr]

We can of course also modify the hint messages so that they become easier to read!

[mitar]

We can of course also modify the hint messages so that they become easier to read!

That I will leave to you. :-) If you want to take a stab at it. Probably we should go over all of them and check. Also you should remove any " while you are at it, they are not allowed (together with \).

[aeneasr]

Fair enough ;)

[aeneasr]

@mitar since I am working on this anyways right now, I can also just give it a shot if that's ok for you!

[mitar]

Sure.

[aeneasr]

So it looks like newlines are ok, the only character not allowed but being used is " which is probably used quite often. However, looks like we can replace it with ' to keep readability high!