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

ntlm authentication #3731

Closed
wants to merge 11 commits into from
Closed

ntlm authentication #3731

wants to merge 11 commits into from

Conversation

BrownTruck
Copy link
Contributor

Adding ntlm for #1182. Other authentication libraries can be added the same way (ie, kerberos)
The import statement was placed inside the method to only import when needed, and not cause an ImportError if package isn't installed.


This was automatically migrated from #3419 to reparent it to the master branch. Please see original pull request for any previous discussion.

Original Submitter: @vmuriart

@benjimin
Copy link

@vmuriart @pfmoore It looks like this came to a standstill because existing ntlm libraries were stagnant, restrictively licensed, focused for web not proxy, have password storage security issues, or have massive compiled dependencies. Is that correct?

I recently put together (independently) a little bit of code to download through an ntlm proxy using SSPI credentials, with a ctypes alternative to pywin32. Do you want to take a look and suggest what it would take to properly solve this problem with python installers behind corporate networks?

@vmuriart
Copy link

@benjimin Your library looks promising! Couple things (I think I know the answer, but want to verify)

  • There is no function to allow user to use credentials other than his own?
  • The solution uses windows API, so Linux users wouldn't benefit ?
  • Are you on adding direct support for requests' authentication ?

@benjimin
Copy link

@vmuriart ,

  • SSPI/single-signon is most useful for networks where different workstations/proxies/servers use the same domain controller to validate credentials. I imagine this covers a substantial fraction of the use-cases for NTLM proxy authentication. I think web browsers normally try SSPI first (when the proxy demands NTLM authentication) and only prompt the user for different credentials if those are rejected. Although this solution doesn't currently replicate that feature (of user-supplied different credentials), I am tempted to add it (because to avoid vulnerabilities I think it should be done by importing a well-tested cryptography library rather than a separate implementation like python-ntlm seems to have).
  • Although I have no plans yet to do this, it would be possible to add support for integrated authentication on other platforms (SSPI is the windows-specific name for GSSAPI). I think this is more commonly seen with Kerberos rather than NTLM. (NTLM proxy authentication seems to be mainly encountered from Windows environments.)
  • I'm not sure what you mean ("Are you on adding ..")? Do you think the best place for this kind of thing would be a separate ntlm-specific package, or requests, or urllib3 (e.g. contrib/ntlmpool)?

@vmuriart
Copy link

Thanks for the reply. They are along the lines of what I was thinking, but wanted to confirm.
In regards to the last point. I think a separate package would be best since it has a different domain of use.
You are intending your library to be an alternative to python-ntlm as opposed to just a solution to this particular issue here right?

@benjimin
Copy link

benjimin commented Aug 19, 2016

My intention is for myself and others to be able to install python software without needing to separately find and download every dependency manually (through a web browser). I'm commenting here for ideas how/where best to contribute to that?

Ideally, random python software (but especially pip/conda) should just work everywhere, even alas on MS-Windows and even behind corporate firewalls (like all mainstream/opensource web browsers already do). Rather than building a separate package and patching proxy-auth support into everything separately, I personally think it would be simpler if the de facto standard python web library were updated so that the defaults would work everywhere (either by implementing system proxy autodetection and WPAD/PAC, a 407 handler, code for the different auth protocols, trying system credentials if safe, and only raising an exception if it is necessary to prompt the user; or perhaps defaulting to use the native OS higher-level HTTP/S API rather than a low-level python sockets implementation). But I'm not sure what other people think is the best solution?

@xavfernandez xavfernandez added the C: proxy Dealing with proxies and networking label Oct 27, 2016
@xavfernandez xavfernandez reopened this Nov 20, 2016
@BrownTruck
Copy link
Contributor Author

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Apr 1, 2017
@pradyunsg
Copy link
Member

Hi @vmuriart!

Is this still something that might be useful? If so, could you merge/rebase this PR on the current master and update it?

@pradyunsg pradyunsg added the S: awaiting response Waiting for a response/more information label Aug 21, 2017
@pradyunsg
Copy link
Member

Closing due to a lack of response. :)

@pradyunsg pradyunsg closed this Oct 20, 2017
@pradyunsg pradyunsg removed the S: awaiting response Waiting for a response/more information label Oct 20, 2017
@lock
Copy link

lock bot commented Jun 2, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 2, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 2, 2019
@pradyunsg pradyunsg removed the needs rebase or merge PR has conflicts with current master label Apr 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation C: proxy Dealing with proxies and networking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants