AuthorizationService::getAuthorizationUri fixed PHP_QUERY_RFC1738

marcing commented 2 years ago

Dear Thomas,

Would it be possible to add optional encoding_type value for http_build_query call inside AuthorizationService::getAuthorizationUri? In our worflow with customer it is required that encoding type is PHP_QUERY_RFC3986, and not PHP_QUERY_RFC1738. Passing this value should be enough, but also a callback would work for query building. Perhaps there is another way I could not find.

Let me know what you think!

Thank you!!! Marcin

thomasvargiu commented 2 years ago

Hi @marcing, I'm reading the specs. Probably the fix should be to always encode it per RFC3986 spec. I'll let you know.

thomasvargiu commented 2 years ago

Update: the current implementation is correct. See section 3.1.2.1.

I will probably provide a public method to give a client the opportunity to treat parameters as the client wants.

I'm closing the issue for the moment because is something different from specifications. For your implementation I suggest you to create a function to convert it as you need.

marcing commented 2 years ago

Dear Thomas,

I was surprised too, but it looks like some OpenID implementations do not work with RFC3986. When I hardcoded RFC1738 it started to work "on the other side".

The problem is "+" character in &response_type=code+token We got a report that it should be "&response_type=code token" and it really started to work when I changed it. I will let you know what is the library on the other side once I get the information. It would be great it there was a callback of some kind to influcence the URL generation.

Thank you, Marcin

thomasvargiu commented 2 years ago

Hi @marcing, I think we'll probably don't want to provide some custom implementation in order to support IdP with wrong implementations, but you're still free to change the provided URL for your custom needs. If the IdP server doesn't accept the URL, I think it's an IdP bug, or some bug with your application that re-encode query parameters.

&response_type=code+token is correct per RFC1738, OAuth2 (RFC6749) spec and OpenID spec since space is encoded with +. Encoding the query string per RFC3986 will create a query string &response_type=code%20token that it is different from what the specification requires (a MUST).

You are still free to take the returned authorization URL, decode the query string from RFC1738, and encode it again per RFC3986 if you really need it.

marcing commented 2 years ago

Dear Thomas,

No problem. I will do as you suggested.

Thank you, Marcin

ChrisTitos commented 11 months ago

Hi,

We're starting to use your library for our OIDC integrations, and we also ran into this. The OIDC provider of one of our clients does not recognize the + in the scope parameter. It only works with %20. Our client uses an ADFS (Microsoft) instance, so this isn't some obscure provider.

In the OIDC spec I see these examples: https://openid.net/specs/openid-connect-core-1_0.html#id_tokenExample

  GET /authorize?
    response_type=code
    &client_id=s6BhdRkqt3
    &redirect_uri=https%3A%2F%2Fclient.example.org%2Fcb
    &scope=openid%20profile%20email
    &nonce=n-0S6_WzA2Mj
    &state=af0ifjsldkj HTTP/1.1
  Host: server.example.com

These examples all show %20 encoding.

It's confusing because indeed the spec also says urls must be encoded as application/x-www-form-urlencoded, which would imply +.

Given the confusion in the spec itself, would it be an idea to add this as a configuration option in the library?

facile-it / php-openid-client

AuthorizationService::getAuthorizationUri fixed PHP_QUERY_RFC1738 #21