Re: [Pki-devel] [PATCH] 0027..0029 support external authorization LDAP server
On 3/13/2015 2:00 PM, Fraser Tweedale wrote:
>> 4. When constructing a DN the attribute values should be
escaped with
>> LDAPUtil.escapeRDNValue() to prevent problems with DN special characters.
>>
>> 5. When constructing LDAP filter the attribute values should be escaped
>> with LDAPUtil.escapeFilter() to prevent problems with LDAP filter
>> special characters.
>>
I have escaped end-user-supplied values, but not config values.
Do you mean these lines?
1. return mGroups + "," + mGroupsBaseDN;
2. "(&(objectclass=" + mGroupObjectClass + ")(" + filter +
"))",
3. filter = k + "=" + userdn;
I think line #1 is fine because the values in those variables are
already in the DN format, not because they are config values.
Line #2 is fine too because the mGroupObjectClass is probably
filter-safe due to object class naming restrictions.
For line #3 I think the userdn still needs to be escaped. DNs and
filters have different sets of special characters (see LDAPUtil.java) so
a DN may not necessarily be filter-safe regardless how the value was
obtained.
> 9. I see the "gid" attribute being set in
TokenAuthentication. Probably it
> should be changed to IAuthToken.GID so we know where it's set and used.
> Alternatively we can replace it with IAuthToken.GROUPS that contains just a
> single group, and remove IAuthToken.GID.
>
Good suggestion; implemented.
Just one more thing, the "no gid in authToken" message probably should
be replaced with "no groups in authToken".
Once these are addressed, ACK. Thanks!
--
Endi S. Dewata