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!