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

Add optional parameter domain to HttpNtlmAuth __init__ #86

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
27 changes: 18 additions & 9 deletions requests_ntlm/requests_ntlm.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,34 @@ class HttpNtlmAuth(AuthBase):
Supports pass-the-hash.
"""

def __init__(self, username, password, session=None):
def __init__(self, username, password, session=None, domain=None):
Copy link
Author

@slukovic slukovic Dec 13, 2016

Choose a reason for hiding this comment

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

I'd prefer to have domain before session but I'm not sure if that would break backwards compatibility and how important that is in this case.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, we can't do that really. Sadly, positional arguments are still a thing.

"""Create an authentication handler for NTLM over HTTP.
Copy link
Member

@nitzmahone nitzmahone Dec 14, 2016

Choose a reason for hiding this comment

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

Prefacing this with r to make it a raw string will get rid of the need forone set of escaping... Have to do this all the time in Ansible module docs for Windows paths (YAML embedded in python here-string)...


:param str username: Username in 'domain\\username' format
:param str username: Username. If in either 'domain\\\\username'
Copy link
Author

Choose a reason for hiding this comment

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

This looks ugly in code but 4 slashes are necessary for doc gen (at least that's how PyCharm works).

Copy link
Member

Choose a reason for hiding this comment

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

Wait, you need four backslashes for PyCharm to render one?

Copy link
Author

@slukovic slukovic Dec 14, 2016

Choose a reason for hiding this comment

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

Yes. I guess that's because both Python and reStructuredText treat backslash as an escape character. If HTML docs aren't generated anywhere I'm happy to leave two backslashes. This way printing __doc__ will display it in the correct format.

Copy link
Member

Choose a reason for hiding this comment

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

Currently we don't render HTML docs, so it might be better to just leave it in the double form.

Copy link
Author

Choose a reason for hiding this comment

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

Done. I also made docstring raw as suggested by @nitzmahone.

format or 'username@domain' format, username and domain will be
parsed out of it. Otherwise, domain is set to '.' and username
is left intact. If the domain parameter is set no parsing is done
regardless of the username format.
:param str password: Password
:param str session: Unused. Kept for backwards-compatibility.
:param str domain: Domain. If None will be parsed out of username.
"""
if ntlm is None:
raise Exception("NTLM libraries unavailable")

# parse the username
try:
self.domain, self.username = username.split('\\', 1)
except ValueError:
if domain is not None:
self.username = username
self.domain = domain
else:
# parse the username
try:
self.username, self.domain = username.split('@', 1)
self.domain, self.username = username.split('\\', 1)
except ValueError:
self.username = username
self.domain = '.'
try:
self.username, self.domain = username.split('@', 1)
except ValueError:
self.username = username
self.domain = '.'

self.domain = self.domain.upper()
self.password = password
Expand Down
15 changes: 15 additions & 0 deletions tests/unit/test_requests_ntlm.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,18 @@ def test_username_parse_no_domain(self):

assert actual_domain == expected_domain
assert actual_user == expected_user

def test_username_with_explicit_domain(self):
test_user = 'user@server.com'
test_domain = 'domain'
expected_domain = 'DOMAIN'
expected_user = 'user@server.com'

context = requests_ntlm.HttpNtlmAuth(
test_user, 'pass', domain=test_domain)

actual_domain = context.domain
actual_user = context.username

assert actual_domain == expected_domain
assert actual_user == expected_user