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