Add QueryParam auth plugin support

[soullivaneuh]

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Related tickets	fixes #265
Documentation	php-http/documentation#238
License	MIT

What's in this PR?

Add missing support for the QueryParam auth plugin.

Why?

Because there is no reason to not have it. :-D

[Nyholm]

Great. Thank you

[soullivaneuh]

@Nyholm I have an issue with the always empty params array.

I'm investigating, but if you have a clue to give... ;-)

[Nyholm]

I have an issue with the always empty params array.

Which line?

[soullivaneuh]

@Nyholm https://github.com/php-http/HttplugBundle/pull/267/files#diff-850942b3ba24ab03a40aaa81b6152852R544

In a nutshell, params is always present as an empty array event if not filled. But validateAuthenticationType does not except params unless for query_params.

But I'm not sure we can remove it from the config if the array is empty. 🤔

[Nyholm]

Be more pragmatic =)

Write a special case for query_params in validateAuthenticationType.

[soullivaneuh]

@Nyholm The issue is params is always present as an empty array: https://github.com/php-http/HttplugBundle/pull/267/files#diff-850942b3ba24ab03a40aaa81b6152852R544

So yes, I can just unset params if the value is empty on validateAuthenticationType, but I consider that quite ugly and params will always be present on the final configuration.

Or maybe the idea you have is different?

[soullivaneuh]

@Nyholm I did an unset on the validation. Does it seem ok to you?

The other fails are note related AFAIK.

[Nyholm]

I think it is okey. I let other maintainers give their opinion as well.

I think that error is unrelated. I check later.

[dbu]

thanks for this pull request.the risk with query parameters for credentials is that they show up in all kind of access logs from the server, proxies and the browser history. its not much of an injection risk like other things with query parameters (if i can trick you to login with somebody's account, that is not an issue - the injection problem is when i can trick you to click a link that executes some operation with your own login).

can you please provide a pull request for php-http/documentation to document this feature as well? maybe add the warning about passwords showing up in access logs and the browser history?

@Nyholm did a bunch of cleanup on master, but afaik there is still something failing for some versions. could you rebase on master nonetheless?

[soullivaneuh]

the risk with query parameters for credentials is that they show up in all kind of access logs from the server

I totally agree! But the targeted API maintainer does not seems to be agree. BTW, the maintainer is Google, f*ck yeah!

They is a risk, but I don't think we should log the emitted request on production.

I'll take a look for the documentation and the rebase.

[soullivaneuh]

Please see php-http/documentation#238.

[soullivaneuh]

Is anything missing to get this merged and released?

[Nyholm]

Thank you for the ping.
You are all good. Thank you for the work!

[soullivaneuh]

Thank you for the merge @Nyholm !

May we have a new release? :-)