Michael Šimáček <msima...@redhat.com> writes:

> On 2015-08-20 12:32, Michael Šimáček wrote:
>
>>>>> Michael Šimáček <msima...@redhat.com> writes:
>>>>>
>>>>>> Attaching new revision of the patch. Changes from the previous:
>>>>>> - ldap2's connect now chooses the bind type same way as in ipaldap
>>>>>> - get_default_realm usages replaced by api.env.realm
>>>>>> - fixed missing third kinit attempt in trust-fetch-domains
>>>>>> - removed rewrapping gssapi errors to ccache errors in krb_utils
>>>>>> - updated some parts of exception handling
>>
>> Rebased on top of current master.
>
> One of the commits reintroduced krbV dependency that I didn't notice. 
> Attaching updated revision. Only changes against previous revision are 
> in files daemons/dnssec/ipa-dnskeysync-replica and 
> daemons/dnssec/ipa-ods-exporter.

This is much better, thanks!  I've got some comments inline.

> +except gssapi.exceptions.GSSError:
>      # If there was failure on using keytab, assume it is stale and retrieve 
> again
>      retrieve_keytab(api, ccache_name, oneway_keytab_name, oneway_principal)

This code still bothers me a bit, but I think fixing it is probably
beyond the scope of a python-gssapi port.

> +    try:
> +        creds = get_credentials(name=name, ccache_name=ccache_name)
> +        # property access would raise exception if expired
> +        if creds.lifetime > 0:
> +            return creds
> +    except gssapi.exceptions.ExpiredCredentialsError:
> +        return None

Per rfc2744, lifetime is unsigned.  It's not immediately clear what will
happen when `creds.lifetime == 0`; perhaps an explicit `return Nune` in
that case?

>          # Setup LDAP connection
>          try:
> -            ctx = krbV.default_context()
> -            ccache = ctx.default_ccache()
> -            api.Backend.ldap2.connect(ccache)
> +            api.Backend.ldap2.connect()
>              cls.ldap = api.Backend.ldap2
> -        except krbV.Krb5Error as e:
> +        except gssapi.exceptions.GSSError:
>              sys.exit("Must have Kerberos credentials to migrate Winsync 
> users.")

Can you log the error here?  The other places GSSError is being caught
are doing a great job of either filtering-and-raising or
logging-and-exiting, so thanks for fixing those.

> +# Ugly hack for test purposes only. GSSAPI has no way to get default ccache
> +# name, but we don't need it outside test server
> +def get_default_ccache_name():
> +    try:
> +        out = check_output(['klist'])
> +    except CalledProcessError:
> +        raise RuntimeError("Default ccache not found. Did you kinit?")
> +    match = re.match(r'^Ticket cache:\s*(\S+)', out)
> +    if not match:
> +        raise RuntimeError("Cannot obtain ccache name")
> +    return match.group(1)

Yup, this is still ugly.  Ah well, it's only test code.

Attachment: signature.asc
Description: PGP signature

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to