RE: Categorize KrbOption by adding group info

["Zheng, Kai" <ka...@intel.com>]

Thanks Steve for this complete deep thought about the client side design. It looks like centralizing all kinds of APIs in a place as KrbClient does isn't going in the right way. As we're going to support more mechanisms and provide more APIs for users, it will be hard without risk of breaking existing codes and applications. I thought we may desire to break KrbClient down into smaller classes according to mechanisms? Or remove the layer at all and expose AsRequest family classes instead? Anyway, I agree it's the right direction for us to consider before the formal 1.0.0 release. 

>> As an aside, the code added for DIRKRB-450 seems to have been removed from that method.
Yes I removed the codes into KdcRequest.java because it can be shared by TgsRequest there. Please check out the change and if it breaks your codes, let me know.

>> I'm looking forward to adding kpasswd and kadmin (per the specifications of course).
It sounds great and I would help some bit. This will provide more inputs and exercises for us about how to redesign the client side library, all the layers. 

This is still a new project and it's never late to change or redesign I guess. Fortunately we have bundle of tests, so don't be afraid.

Regards,
Kai

-----Original Message-----
From: Steve Moyer [mailto:smoyer@psu.edu] 
Sent: Friday, November 20, 2015 7:43 PM
To: kerby@directory.apache.org
Subject: Re: Categorize KrbOption by adding group info

Emmanuel and Kai,

I hope I haven't done too much complaining!  If I ever try to push the project in the wrong direction, please let me know.  I intend to write a bit longer e-mail talking about what Penn State needs from the Kerberos client with some specific design questions about the Kerby way of doing things.

I will admit that understanding the structure of the code has taken me a while, and that wrapping the layers of enums was especially confusing.  Perhaps I'm a power user but I was considering building the AsRequest object myself since it avoids the ambiguity of which KrbOption ends up where (the structure looks just like a packet as seen in WireShark).

I share Emmanuel's concern about KrbOption - we're merging things in a way that's opposed to the idea of "separation-of-concerns".  Everytime something is added, it requires more code in the requestTgtTicket(KOptions requestOptions) method (https://github.com/apache/directory-kerby/blob/master/kerby-kerb/kerb-client/src/main/java/org/apache/kerby/kerberos/kerb/client/impl/AbstractInternalKrbClient.java) gets longer.  As an aside, the code added for DIRKRB-450 seems to have been removed from that method.

There's actually a third layer of enum values - the KinitTool has a set of enums that define its CLI arguments - these get translated into KrbOption and some of these get translated into KdcOption.  The KdcOption enum encapsulates the 15 values representing the 15 flags in an AsRequest which seems like the right level of encapsulation.  I don't mind enums with a lot of elements (we have one with all the ISO country codes) but it's hard when the context switches from one element to the next.

With all this said, with the recent changes, we're able to use the Kerby project for both our authentication and kvno functions.  I'm looking forward to adding kpasswd and kadmin (per the specifications of course).

Thanks again for all the work that went into this project ... I may be a newcomer but I'm deeply appreciative!

Steve

--

“The mark of the immature man is that he wants to die nobly for a cause, while the mark of the mature man is that he wants to live humbly for one.” - Wilhelm Stekel

----- Original Message -----
From: "Emmanuel Lécharny" <el...@gmail.com>
To: kerby@directory.apache.org
Sent: Friday, November 20, 2015 5:09:40 AM
Subject: Re: Categorize KrbOption by adding group info

Le 20/11/15 10:52, Zheng, Kai a écrit :
> Thanks Emmanuel!
>
>>> I just find it easier to stick to the RFC ...
> Agree. Just forgot to mention that in the core we do stick to the specs and define those types, like KdcOption. I would regard KrbOption(s) as the bridge or wrapper for the KrbClient API to frontend and interact with users' applications.

I understand. Users might be confused by the various Enum, when they are to pick the right one for the tool they use.
>
> Note I'm not thinking in the current codes the client API (including the KrbOption) is defining ideally. Instead, it would be great to have it well reviewed and discussed before a formal release. We do have enough time for that. 

It's definitively a blured area. I don't like classes/enums with tens of constants, it's a kind of a pain for users, but the alternative is not clear to me...

Definitively something that worth a discussion, I'm not sure I would stand on my position which is not perfect either...