Fix up the interface we present to mention SPNEGO #3
Conversation
Signed-off-by: Robbie Harwood <[email protected]> Resolves: pythongssapi#1
Signed-off-by: Robbie Harwood <[email protected]> Resolves: pythongssapi#1
Signed-off-by: Robbie Harwood <[email protected]>
|
I think we should deprecate principal and hostname_override and instead allow to pass in GSS-Names, although hostname_override is relatively neutral, principal applies only to kerberos as a concept, and I do not think it expresses the richness it could (does it support enterprise names as is ?) |
|
force_preemptive is a strange name, the strategy is normally called "opportunistic auth", maybe we should also deprecate that old name and introduce a better one here. |
|
I think it may also be nice to let delegate have mutliple values given we have 2 delegate flags that means two different things. |
|
another thing we may want to consider is the ability to pass in a python-gssapi credential instead of havin request-gssapi always find it itself, this is better than passing in principal as it allows to use a MEMORY creadential we just created and is not in the defaul cache collection used by the system. Otherwise we'd have to set the process ccache and this will impact other threads/functions unnecessarily. Bad especially in systems that deal with multiple users as proxies of some kind (that use delegation for example). |
GSSAPI only has |
|
GSS_C_DELEG_POLICY_FLAG |
|
GSS_C_DELEG_POLICY_FLAG
Well, one issue with that is that python-gssapi doesn't know about that
flag, but we could teach it
[python-gssapi issue #132](pythongssapi/python-gssapi#132)
|
|
Can we have conditional support in this code until python-gssapi gets it, or should we rather wait and fix it later? |
|
I'd rather wait until it at least lands in python-gssapi master, but I am working on that as well. For the interface, because python, we can reuse the What I'm not clear on is how that would behave, and moreover what behaviors we want to support. The Heimdal doc defines four behaviors based on the two flags being set or not; I'm not clear which are desirable here, and how to handle the new two cases in the code. |
|
Four new commits, hopefully addressing your API suggestions @simo5 (except delegate; see above). |
requests_gssapi/gssapi_.py
Outdated
| @@ -83,13 +83,13 @@ class HTTPSPNEGOAuth(AuthBase): | |||
| def __init__(self, mutual_authentication=REQUIRED, service="HTTP", | |||
| delegate=False, force_preemptive=False, principal=None, | |||
| hostname_override=None, sanitize_mutual_error_response=True, | |||
| creds=None): | |||
| creds=None, opportunistic_auth=False): | |||
There was a problem hiding this comment.
I was thinking we shouldn't retain force_preemptive, but provide a translation from that flag to opportunistic_auth in the wrapper that respondes to the old class name (HTTPKerberosAuth)
requests_gssapi/gssapi_.py
Outdated
| @@ -97,6 +97,9 @@ def __init__(self, mutual_authentication=REQUIRED, service="HTTP", | |||
|
|
|||
| if self.creds and self.principal: | |||
| raise RuntimeError("creds and principal are mutually exclusive") | |||
| if type(service) == gssapi.names.Name and hostname_override != None: | |||
There was a problem hiding this comment.
should we retain hostname_override at all ?
shouldn't we just deal it in the compat wrapper for the old class to translate it into a gssapi service name before calling into the HTTPSPNEGOAuth initializer ?
There was a problem hiding this comment.
If we want to make it easy to pass in a string instead of a gssapi name type then we could change the service string to allow to pass in "HTTP@hostname", it sounds silly to have a "hostname_pverride as a separate argument when you can easily use the "service", which I would perhaps rename to "target", but that may be going too far.
frozencemetery
force-pushed
the
shim
branch
2 times, most recently
from
7c701c2 to
90982b2
Compare
|
Pushed up a new version with a stricter shim that hopefully behaves more in line with your suggestion. |
Signed-off-by: Robbie Harwood <[email protected]>
Signed-off-by: Robbie Harwood <[email protected]>
Signed-off-by: Robbie Harwood <[email protected]>
Signed-off-by: Robbie Harwood <[email protected]>
Signed-off-by: Robbie Harwood <[email protected]>
|
@simo5 please re-review, thanks! |
(It's probably easiest to look at README.rst after the changes. Most of these were performed with sed and as such aren't very interesting.)
New auth provider is named "HTTPSPNEGOAuth". If there are suggestions., I am happy to modify the API to be more than a bare shim. If they're breaking backward compat, now is the time to offer them before we merge this :)