On 3/12/2015 8:12 AM, Fraser Tweedale wrote:
On Wed, Mar 11, 2015 at 02:04:56PM -0400, Ade Lee wrote:
Looks good in general.
I notice that your patch adds the use of the Vector class. Vector is old and synchronized - which can slow things down unnecessarily. Use ArrayList or similar instead.
Ade
Roger that; I will switch to ArrayList. For now you can all s/Vector/ArrayList/g in your heads while you review this patch :)
Quick comments: 1. The TOKEN_GROUPS probably should be a List<String> to simplify the creation and the usage of the list of groups. 2. I'm not quite clear the purpose of this enhancement. If it's meant to be a general-purpose directory-based authentication plugin, it would make sense to have a fully configurable parameters for retrieving the group information. However, if this is only to be used for Dogtag authentication, there's already a user-group subsystem that can provide the information. See PKIRealm.getRoles(). I may have more comments later. -- Endi S. Dewata