mongodb / winkerberos

A native Kerberos client implementation for Python on Windows
Apache License 2.0
54 stars 15 forks source link

Support of kerberos.GSS_MECH_OID_SPNEGO #15

Closed veklov closed 7 years ago

veklov commented 7 years ago

Hello Bernie (@behackett),

This change allows implemention of Spnego/Negotiate authentication in requests_kerberos (and potentially in urllib_kerberos after switching to winkerberos):

result, self.context[host] = kerberos.authGSSClientInit(kerb_spn, gssflags=gssflags, principal=self.principal, mech_oid=kerberos.GSS_MECH_OID_SPNEGO)

If you are fine with the change and merge it, I am going to add HttpSpnegoAuth to requests_kerberos and urllib_kerberos (migrating it to winkerberos).

Kind regrads, Alexey Veklov

behackett commented 7 years ago

Neat! I had thought about adding this myself. Thanks for the patch. I'll review and merge it soon.

behackett commented 7 years ago

Though it's not strictly necessary, maybe we should use PyCObject / PyCapsule for the OIDs, rather than strings, to match the API of PyKerberos?

https://github.com/apple/ccs-pykerberos/blob/PyKerberos-1.2.5/src/kerberos.c#L836-L841

veklov commented 7 years ago

To tell the truth I am new to Python, feel free to do whatever changes you think are necessary. If you do not have time, give me a hint and I will try to implement and update the pull request.

behackett commented 7 years ago

No worries. I can merge this and add patches after. Thanks again for the patch. This is great work.

behackett commented 7 years ago

@veklov - to add support for this option to requests-kerberos, you'll also have to get requests-kerberos to switch to https://github.com/apple/ccs-pykerberos (see https://github.com/requests/requests-kerberos/issues/63), or get this feature added to https://github.com/02strich/pykerberos.

veklov commented 7 years ago

@behackett - I have changed the code to use capsules.

Good catch about 02strich/pykerberos vs apple/ccs-pykerberos I tried several libraries and it looks like I finally ended up testing with packages after pip install requests-kerberos pip install kerberos I presume that Apple's implementation was used... I will try to create a patch for pykerberos as well.

Meanwhile, could you take a look at my attempt to integrate this into requests-kerberos: https://github.com/veklov/requests-kerberos

veklov commented 7 years ago

@behackett - Hi. I will update the code as you suggested. I adapted PyCapsule implementation used in Apple's code, and I did the same while merging Apple's fix into 02strich/pykerberos (https://github.com/02strich/pykerberos/pull/25). I literally copied it. Could you take a look at that pull request and suggest whether it makes sense to use similar macroses there?

veklov commented 7 years ago

@behackett - Hi, I have changed the code as you suggested.

behackett commented 7 years ago

Great work. I'll do some testing and merge this soon.

The issue with PyCapsule is that Python 2.6 doesn't provide it. PyKerberos currently works on 2.6, and provides aliases similar to WinKerberos. It appears that Apple's project broke support for Python 2.6.

Note that requests (and therefore requests-kerberos) claims to support Python 2.6.

veklov commented 7 years ago

ok. It turned out that PyKerberos automatically runs tests for pull requests. Mine passed https://travis-ci.org/02strich/pykerberos/builds/193902068 (including Python 2.6 build). By the way, I have not got any response for https://github.com/02strich/pykerberos/pull/25 . Maybe 02strich is out. Do you know who else can review it?

behackett commented 7 years ago

Looking at the Travis test output I'm not sure what it actually tests. Python 2.6 definitely doesn't provide PyCapsule. PyKerberos provides PyNew, PyCheck, and PyGet aliases for this purpose:

https://github.com/02strich/pykerberos/blob/v1.1.9/src/kerberos.c#L26-L28

veklov commented 7 years ago

I will update that pull request too :)

veklov commented 7 years ago

Hi, Bernie. My pull request for PyKerberos (https://github.com/02strich/pykerberos/pull/25) has been merged and released. Could you merge this one, so I could proceed with the fix for requests_kerberos and urllib_kerberos.

behackett commented 7 years ago

Thanks for your patience. I've merged this, applied a small fix, updated the docs, and given you credit in the changelog.

Thanks again! This is a great addition to the library.

behackett commented 7 years ago

I'm going to run a build through Coverity scan. Assuming there are no issues, I'll do a release after.

veklov commented 7 years ago

Thank you Bernie

behackett commented 7 years ago

I've released WinKerberos 0.6.0 - https://pypi.python.org/pypi/winkerberos/0.6.0. Thanks again for the patch!