----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67690/#review205206 -----------------------------------------------------------
Code change looks good. API "getMSentryPrivileges" which is been fixed in this path is used only when the privilege are listed and not in the authorization path. It is clear that there were no tests that were veryfying it. Please add a test for the same. - kalyan kumar kalvagadda On June 21, 2018, 2:25 p.m., Arjun Mishra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67690/ > ----------------------------------------------------------- > > (Updated June 21, 2018, 2:25 p.m.) > > > Review request for sentry, kalyan kumar kalvagadda and Sergio Pena. > > > Repository: sentry > > > Description > ------- > > In SentryStore#getMSentryPrivileges when retrieving URI's the query condition > "addCustomParam("\"" + authHierarchy.getUri() + "\".startsWith(:URI)", URI, > authHierarchy.getUri());", will always be True. This should be changed to > filter by appropriate URI. > > For example: when retrieving privileges on URI object "/x/y/z" this condition > is checked for: ""file:///x/y/z".startsWith(file:///x/y/z))", which is always > true > > > Diffs > ----- > > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > f0e373a9a > > > Diff: https://reviews.apache.org/r/67690/diff/1/ > > > Testing > ------- > > $ mvn -f sentry-service/sentry-service-server/pom.xml test > > > Thanks, > > Arjun Mishra > >
