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

Enable Proxies for Token Acquisition in iControlRESTTokenAuth #172

Closed
lykinsbd opened this issue Jan 28, 2020 · 3 comments · Fixed by #173
Closed

Enable Proxies for Token Acquisition in iControlRESTTokenAuth #172

lykinsbd opened this issue Jan 28, 2020 · 3 comments · Fixed by #173

Comments

@lykinsbd
Copy link
Contributor

As of #169 "Allow proxies kwarg from sdk ManagementRoot", the proxies attribute is pushed all the way down into the iControlRESTSession object and applied to the underlying requests.Session object.

However, there remains a problem with token acquisition in iControlRESTTokenAuth as it appears to still utilize plain requests.<verb> calls and does not have access to the outer session or it's proxy configuration.

Example in iControlRESTTokenAuth.get_new_token():
https://github.com/F5Networks/f5-icontrol-rest-python/blob/1.0/icontrol/authtoken.py#L146

Example in iControlRESTTokenAuth.get_auth_providers():
https://github.com/F5Networks/f5-icontrol-rest-python/blob/1.0/icontrol/authtoken.py#L98

This results in a condition where a user is unable to actually utilize the outer proxy-enabled session object, because the acquisition of a token is not proxy-enabled.


In my testing, simply passing the proxies dict down into iControlRESTTokenAuth upon initialization, and then further passing it to the above example Requests calls as an additional attribute on the request itself seems to work.

I am happy to open a PR with an attempted fix, just wanted to raise the issue first.

lykinsbd added a commit to lykinsbd/f5-icontrol-rest-python that referenced this issue Jan 29, 2020
Issues:
Fixes F5Networks#172

Problem:

The `proxies` attribute is available on the `iControlRESTSession` object,
 however it does not get passed to the `iControlRESTTokenAuth` object.
As a result, if all communication with a BigIP device needs to take place
 via a Proxy, authentication will fail and timeout.
Analysis:

I added `proxies` as a Kwarg on the `iControlRESTTokenAuth` object.
I then passed that argument in from the `iControlRESTSession` object
 instantiations of the `iControlRESTTokenAuth` object.
I then added `proxies=self.proxies` to the two instances of
 `requests.<verb>` in the `iControlRESTTokenAuth` object.
Tests:

I am unable to run the tests as listed in the documentation as even in 1.0 branch,
 `py.test --cov ./ --cov-report=html` returns:
 `argparse.ArgumentError: argument --bigip: conflicting option string: --bigip`

I did successfully test with and without proxies, by installing my fork/branch
 and testing against a live BigIP

With Proxies:

    In [1]: paste
    from f5.bigip import ManagementRoot
    from getpass import getpass
    password = getpass()
    user = "test_user"
    host = "test_f5.local"
    mah_f5 = ManagementRoot(hostname=host, username=user, password=password, proxies={"https": "socks5://127.0.0.1:8000"}, auth_provider="tmos")
    mah_f5.tmos_version

    ## -- End pasted text --
    Password:
    Out[1]: '14.1.2.3'

Without Proxies:

    In [1]: paste
    from f5.bigip import ManagementRoot
    from getpass import getpass
    password = getpass()
    user = "test_user"
    host = "localhost"
    port = 30002
    mah_f5 = ManagementRoot(hostname=host, port=port, username=user, password=password, auth_provider="tmos")
    mah_f5.tmos_version

    ## -- End pasted text --
    Password:
    Out[1]: '14.1.2.3'
jasonrahm pushed a commit that referenced this issue Feb 7, 2020
Resolve #172, Enable HTTP Proxy Support for `iControlRESTTokenAuth` Object
@mr-grj
Copy link

mr-grj commented Apr 3, 2020

Any idea when this pull request will be approved and merged into master? I'd really like to use this withing the library and avoid monkeypatching it by myself :)

@lykinsbd
Copy link
Contributor Author

lykinsbd commented Apr 3, 2020

This PR was merged into the current prod branch of “1.0”, and a release was done on Github. However, it was not pushed/published to Pypi. I opened #174 to try and get that portion done.

@mr-grj
Copy link

mr-grj commented Apr 3, 2020

@lykinsbd awesome! I'm waiting for some good news then! Thanks a lot

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