Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Escaping of user info breaks NTLM authentication #219

Closed
sunkan opened this issue Mar 31, 2023 · 9 comments · Fixed by #221
Closed

Escaping of user info breaks NTLM authentication #219

sunkan opened this issue Mar 31, 2023 · 9 comments · Fixed by #221

Comments

@sunkan
Copy link
Contributor

sunkan commented Mar 31, 2023

The change in #213 breaks ntml based authentication when specifying a domain.

When using ntml auth it's common to have a domain that is part of the username like this

curl_setopt($ch, CURLOPT_USERPWD, 'DOMAIN\username:password');

The slash is not suppose to be escaped and if it's the authentication fails

@nicolas-grekas
Copy link
Collaborator

Did you try other implementations of PSR-7? AFAIK, they all encode the \, and this matches https://www.rfc-editor.org/rfc/rfc3986#appendix-A

I feel like the correct course of action is to pass rawurldecode($request->getUserInfo()) to curl here.

@sunkan
Copy link
Contributor Author

sunkan commented Mar 31, 2023

I've tried laminas and realized it didn't work but it has worked fine with the library.

So your solution is for me to go and modify buzz?

It's a bit annoying that with this change there is no popular psr-7 implementation that can be used to talk to Microsoft's stuff.

@nicolas-grekas
Copy link
Collaborator

Buzz is clearly relying on an unspecified behavior yes...
My reco is to replace buzz by Symfony HttpClient at this stage.
And to fix Buzz indeed!

@nicolas-grekas
Copy link
Collaborator

(you can pin nyholm/psr7 to ~1.15.0 if you want also, but then you know you'll have some technical debt to fix at some point)

@nicolas-grekas
Copy link
Collaborator

Please link back if you do anything on another repo!

@sunkan
Copy link
Contributor Author

sunkan commented Mar 31, 2023

My reason for using this library was because it was following the psr and that says nothing about escaping the username and password.

And the psr say that the UriInterface Is a Value object so in my eyes it shouldn't deal with escaping since as this prove the escaping is domain specific.

What you say is that ofc it should escape it and if I want to use it in any other way I need to know how it's been escaped and revers it instead of getting what I passed in. I feel like that's the definition of insanity.

@nicolas-grekas
Copy link
Collaborator

nicolas-grekas commented Mar 31, 2023

I'm sorry for the confusion. You might want to check php-fig/fig-standards#1298 as a follow up.

The previous behavior wasn't correct either, as shown when doing $uri->withUserInfo('a#b'), which produces a broken URI string.

@nicolas-grekas
Copy link
Collaborator

I figured out a more seamless way to produce non-broken URLs, see #221

@sunkan
Copy link
Contributor Author

sunkan commented Mar 31, 2023

Thank you for improving and fixing this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants