[Freeipa-devel] [PATCH 0002] Port from python-krbV to python-gssapi

[Simo Sorce]

On Tue, 2015-08-25 at 20:45 +0200, Michael Šimáček wrote:
> 
> On 2015-08-25 18:43, Robbie Harwood wrote:
> > Jan Cholasta <jcholast at redhat.com> writes:
> >
> >> On 25.8.2015 12:46, Michael Šimáček wrote:
> >>> On 2015-08-25 12:38, Alexander Bokovoy wrote:
> >>>> On Tue, 25 Aug 2015, Michael Šimáček wrote:
> >>>>> On 2015-08-24 20:29, Robbie Harwood wrote:
> >>>>>> Michael Šimáček <msimacek at redhat.com> writes:
> >>>>>>> On 2015-08-24 17:49, Simo Sorce wrote:
> >>>>>>>> On Mon, 2015-08-24 at 17:18 +0200, Michael Šimáček wrote:
> >>>>>>>>> On 2015-08-24 14:50, Jan Cholasta wrote:
> >>>>>>>>>
> >>>>>>>>> Fixed. python-gssapi has a display_as method that could pull the
> >>>>>>>>> name
> >>>>>>>>> from it, but it doesn't work in current version, therefore using
> >>>>>>>>> partition to split on '@'
> >>>>>>
> >>>>>> It's actually a bug in MIT Krb5, as we noted in your bug[0].  So this:
> >>>>>>
> >>>>>>> -        user = api.Command.user_show(unicode(principal[0]))['result']
> >>>>>>> +        user =
> >>>>>>> api.Command.user_show(principal.partition('@')[0])['result']
> >>>>>>
> >>>>>> is working around a bug in specific Kerberos versions.  If people are
> >>>>>> okay with merging such code, then I guess this is fine; I would
> >>>>>> personally not do so because there is not a clear point at which it can
> >>>>>> be removed.  At the very least, we should wait until we see what
> >>>>>> versions of krb5 MIT is going to fix.
> >>>>>>
> >>>>>> Otherwise, looks good.
> >>>>>>
> >>>>>> [0]: https://github.com/pythongssapi/python-gssapi/issues/79
> >>>>>>
> >>>>>
> >>>>> python-krbV migration is blocking support for Python 3. The bug
> >>>>> doesn't have any fix upstream yet and there are two bugs actually, the
> >>>>> second one is in python-gssapi, which I've just reported [1]. Waiting
> >>>>> for two bugs to be fixed could be detrimental to py3 migration as we
> >>>>> don't have much time left. And I'm no longer sure that display_as
> >>>>
> >>>> I don't buy this.
> >>>>
> >>>> We have plenty of time for solving these bugs. Remember, that Samba
> >>>> DCE RPC bindings aren't migrated to Python 3 either and will not be
> >>>> before release of Samba 4.4. For Samba 4.3 it is simply too late.
> >>>>
> >>>> So we are still far away from full Python3 migration for FreeIPA and
> >>>> waiting for solving these two bugs is OK.
> >>>
> >>> If fixing them solves anything at all. I planned to use
> >>> display_as(NameType.user), but when trying it on Name object with
> >>> name_type set (which doesn't trigger the segfault), it doesn't seem to
> >>> work either. I get:
> >>> gssapi.raw.exceptions.OperationUnavailableError: Major (1048576): The
> >>> operation or option is not available or unsupported, Minor (0): Unknown
> >>> error
> >>>
> >>> Robbie, can you clarify whether display_as could be actually used to get
> >>> the first component of the principal reliably?
> >
> > display_as should behave in accordance with its docs; anything else is a
> > bug report, which you filed.  I don't know what you're asking me for
> > beyond that.
> >
> 
> Why I mentioned display_as at all is that I initially assumed it could 
> be used for this, but it was only an assumption because I couldn't get 
> around the segfault. Later on, the cause of the segfault was found and I 
> was able to try the method and I found out that it probably cannot be 
> used for this purpose (i. e. extracting the first component of the 
> principal) regardless of the two bugs. How I thought it would be used:
> import gssapi
> cred = gssapi.Credentials()
> user = cred.name.display_as(gssapi.NameType.user)
> 
> What I got:
> gssapi.raw.exceptions.OperationUnavailableError: Major (1048576): The 
> operation or option is not available or unsupported, Minor (0): Unknown 
> error
> 
> This seems more like the method is not intended to be used this way. So 
> I'm asking you whether it is a bug or whether there is another way to do 
> it. Otherwise display_as cannot be used here.
> 
> >> As I have written in the other thread, we use "principal.split('@')" in
> >> other parts of IPA, so "principal.partition('@')" should be OK as well.
> >>
> >> This patch works for me, so ACK.
> >>
> >> Unless there are any further objections, I would like to push it.
> >
> > I think the newest iteration of this
> >
> >> user = api.Command.user_show(principal.partition('@')[0].partition('/')[0])['result']
> >
> > is even worse, but if it is decided to merge, then hopefully we can be
> > rid of it quickly.
> 
> It is splitting a string of known format in a way that is used in other 
> places of freeipa. What is specifically so bad about it? What do you 
> suggest as an alternative?

Given display_as() currently does not work for you go ahead with this
code. We'll revisit display_as later once we figure out more about the
bug that makes it fail.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York