----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37943/#review98638 -----------------------------------------------------------
security-admin/src/main/java/org/apache/ranger/security/context/RangerAPIMapping.java (line 44) <https://reviews.apache.org/r/37943/#comment155150> Consider using Set<String> instead of List<String>. This can greatly improve lookup performance, which will be done for every REST API call. security-admin/src/main/java/org/apache/ranger/security/context/RangerAPIMapping.java (line 464) <https://reviews.apache.org/r/37943/#comment155152> Available tabs is a constant, so there is no need to create a collection instance (ArrayList) for every call. It will be efficient to create a non-modifiable collection in init() and simply return it here. And use Set<String>, instead of List<String> - for better performance. security-admin/src/main/java/org/apache/ranger/security/context/RangerAPIMapping.java (line 513) <https://reviews.apache.org/r/37943/#comment155162> Since api=>tabs mapping can be precomputed, it will be efficient to have init() populate a map (Map<String, Set<String>> mapApiToTabs;) and have getAssociatedTabsWithAPI(apiName) to simply "return mapApiToTabs.get(apiName);". This should greatly reduce the overhead of access-check for every REST API call. For example: init() { mapResourceBasedPoliciesWithAPIs(); ... mapReportsWithAPIs(); populateApiToTabsMap(apiAssociatedWithRBPolicies); ... populateApiToTabsMap(apiAssociatedWithReports); } populateApiToTabsMap(String tabName, Set<String> apis) { for(String api : apis) { mapApiToTabs.get(api).add(tabName); } } security-admin/src/main/java/org/apache/ranger/security/context/RangerPreAuthSecurityHandler.java (line 72) <https://reviews.apache.org/r/37943/#comment155176> If the method is not associated with any tab, then the method should be accessible to anyone. Please review and update. security-admin/src/main/java/org/apache/ranger/security/context/RangerPreAuthSecurityHandler.java (line 84) <https://reviews.apache.org/r/37943/#comment155164> Instead of making multiple calls to DB (at least 4 calls here), we should have a single query to return the set of modules allowed for an user. Once 2 sets of permissions { tabsForUser, tabsForAPI } are available, simply do the following to check the accesss: return CollectionUtils.containsAny(tabsForUser, tabsForAPI); This will not perform a case-insenstive comparision; but do we need comparision to be case-insensitive? - Madhan Neethiraj On Sept. 11, 2015, 5:13 p.m., Gautam Borad wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37943/ > ----------------------------------------------------------- > > (Updated Sept. 11, 2015, 5:13 p.m.) > > > Review request for ranger, Alok Lal, Don Bosco Durai, Madhan Neethiraj, > Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy. > > > Bugs: RANGER-630 > https://issues.apache.org/jira/browse/RANGER-630 > > > Repository: ranger > > > Description > ------- > > Make data access consistent across REST API and UI. > > > Diffs > ----- > > security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java 939ddc2 > security-admin/src/main/java/org/apache/ranger/biz/XAuditMgr.java d9812f9 > security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 700caff > security-admin/src/main/java/org/apache/ranger/db/XXGroupUserDao.java > 9f5abfb > security-admin/src/main/java/org/apache/ranger/db/XXModuleDefDao.java > 611eaf8 > security-admin/src/main/java/org/apache/ranger/rest/AssetREST.java e5de160 > security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java > 059f787 > security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java > 3d2e8b0 > security-admin/src/main/java/org/apache/ranger/rest/UserREST.java a9d0059 > security-admin/src/main/java/org/apache/ranger/rest/XAuditREST.java 531f395 > security-admin/src/main/java/org/apache/ranger/rest/XKeyREST.java 1c0f9fc > security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java 93980b4 > > security-admin/src/main/java/org/apache/ranger/security/context/RangerAPIList.java > PRE-CREATION > > security-admin/src/main/java/org/apache/ranger/security/context/RangerAPIMapping.java > PRE-CREATION > > security-admin/src/main/java/org/apache/ranger/security/context/RangerPreAuthSecurityHandler.java > PRE-CREATION > > security-admin/src/main/java/org/apache/ranger/service/XAuditMapService.java > 1f48c86 > security-admin/src/main/java/org/apache/ranger/service/XPermMapService.java > 7e5eb10 > > security-admin/src/main/java/org/apache/ranger/service/XResourceService.java > fa6679a > security-admin/src/main/resources/META-INF/jpa_named_queries.xml 7761756 > security-admin/src/main/resources/conf.dist/security-applicationContext.xml > a648809 > security-admin/src/test/java/org/apache/ranger/audit/TestAuditQueue.java > 021c49a > security-admin/src/test/java/org/apache/ranger/biz/TestUserMgr.java e18e51c > security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java > bb74bb8 > security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java > e7324a1 > > Diff: https://reviews.apache.org/r/37943/diff/ > > > Testing > ------- > > 1) Tested on Ranger UI working of permission model. > 2) Test REST calls to reflect access conrol based on Permission model. > 3) Checked cases like revoking access to 'user1' (having user role) from > Audit tab (using permission model) and making curl call to Audit tab's REST > APIs. > > > Thanks, > > Gautam Borad > >
