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.

Sign up for GitHub

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

Jump to bottom

Replace deprecated Ntlm() with NtlmContext() #116

Closed
wants to merge 8 commits into from
Closed

Replace deprecated Ntlm() with NtlmContext() #116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c2d8c59
Replace deprecated Ntlm() with NtlmContext()
ecederstrand May 23, 2019
cc32522
Bump ntlm_auth version
ecederstrand May 23, 2019
76a5717
Bump version here also
ecederstrand May 23, 2019
1c15c4c
Pin pytest-cov to <2.6
ecederstrand May 24, 2019
3479bd5
Replace unknown method usage
ecederstrand May 24, 2019
6433c70
Replace more usage of Ntlm() methods
May 27, 2019
6a911b8
Only test currently supported Python versions
May 27, 2019
bec2706
Default distro doesn't have Python 3.7
May 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
language: python

os: linux
dist: xenial

python:
- "2.6"
- "2.7"
- "3.3"
- "3.4"
- "3.5"
- "3.6"
- "3.7"

install:
- pip install -U pip setuptools
Expand Down
22 changes: 11 additions & 11 deletions requests_ntlm/requests_ntlm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import base64
import binascii
import sys
import warnings
Expand Down Expand Up @@ -71,8 +72,12 @@ def retry_using_http_NTLM_auth(self, auth_header_field, auth_header,

# ntlm returns the headers as a base64 encoded bytestring. Convert to
# a string.
context = ntlm.Ntlm()
negotiate_message = context.create_negotiate_message(self.domain).decode('ascii')
context = ntlm.NtlmContext(
username=self.username,
password=self.password,
domain=self.domain or None,
)
negotiate_message = base64.b64encode(context.step()).decode('ascii')
auth = u'%s %s' % (auth_type, negotiate_message)
request.headers[auth_header] = auth

Expand Down Expand Up @@ -110,17 +115,12 @@ def retry_using_http_NTLM_auth(self, auth_header_field, auth_header,
).strip()

# Parse the challenge in the ntlm context
context.parse_challenge_message(ntlm_header_value[len(auth_strip):])
challenge_token = base64.b64decode(ntlm_header_value[len(auth_strip):])

# build response
# Get the response based on the challenge message
authenticate_message = context.create_authenticate_message(
self.username,
self.password,
self.domain,
server_certificate_hash=server_certificate_hash
)
authenticate_message = authenticate_message.decode('ascii')
context._server_certificate_hash = server_certificate_hash
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of setting this we should be passing in a CBT object to NtlmContext on initialisation through the cbt_data kwarg. The _server_certificate_hash attribute is only used for backwards compat with Ntlm() in ntlm-auth and will be removed if I ever get to removing Ntlm().

What you need to do instead is change _get_certificate_hash() to return certificate_hash_bytes and don't worry about using hexlify there. Then you can create the CBT struct with:

from ntlm_auth.gss_channel_bindings import GssChannelBindingsStruct

cbt_data = GssChannelBindingsStruct()
cbt_data[cbt_data.APPLICATION_DATA] = b'tls-server-end-point:' + server_certificate_hash

It might be a good idea to change the variable name server_certificate_hash to b_server_cert_hash to make sure people are aware that it is a byte string.

authenticate_message = base64.b64encode(context.step(challenge_token)).decode('ascii')
auth = u'%s %s' % (auth_type, authenticate_message)
request.headers[auth_header] = auth

Expand All @@ -131,7 +131,7 @@ def retry_using_http_NTLM_auth(self, auth_header_field, auth_header,
response3.history.append(response2)

# Get the session_security object created by ntlm-auth for signing and sealing of messages
self.session_security = context.session_security
self.session_security = context._session_security
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is a tricky one, technically the wrap and unwrap methods of NtlmContext() should be used instead but unfortunately unwrap on NtlmContext has a different signature than the one of session_security. While this works I don't really want to rely on _ prefixed attributes because they aren't really guaranteed to stay stable over different releases.

What I recommend is to add a session_security property to this class like so:

@property
def session_security(self):
    warnings.warn("Deprecated property session_security, the wrap and unwrap methods should be accessed using the host context ntlm.contexts['hostname'].wrap/unwrap instead.", DeprecationWarning)
    return next(iter(self.contexts))._session_security

This also requires us to store the NtlmContext as a property in requests-ntlm. This is done in a similar way with requests-kerberos where the context is stored in a dict that's indexed with the hostname of the request. You can see that requests-kerberos uses urlparse from requests.compat import urlparse to get the hostname from the request and that is passed into the retry method. From there the context that is initialized is stored in self.contexts[hostname].

Doing this means that other libraries that rely on the wrapping and unwrapping functions provided by session_security aren't broken in the next release, are warned about it being deprecated here and are offered the alternative.


return response3

Expand Down
4 changes: 2 additions & 2 deletions requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
requests>=2.0.0
ntlm-auth>=1.0.2
ntlm-auth>=1.2.0
cryptography>=1.3
flask
pytest
pytest-cov
pytest-cov<2.6
wheel
2 changes: 1 addition & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
name='requests_ntlm',
version='1.1.0',
packages=[ 'requests_ntlm' ],
install_requires=[ 'requests>=2.0.0', 'ntlm-auth>=1.0.2', 'cryptography>=1.3' ],
install_requires=[ 'requests>=2.0.0', 'ntlm-auth>=1.2.0', 'cryptography>=1.3' ],
provides=[ 'requests_ntlm' ],
author='Ben Toews',
author_email='mastahyeti@gmail.com',
Expand Down