----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72828/#review221786 -----------------------------------------------------------
plugin-atlas/src/main/java/org/apache/ranger/authorization/atlas/authorizer/RangerAtlasAuthorizer.java Line 179 (original), 183 (patched) <https://reviews.apache.org/r/72828/#comment310856> Variable 'action' is not modified after initialzation here. Consider retaining 'final' marker. plugin-atlas/src/main/java/org/apache/ranger/services/atlas/RangerServiceAtlas.java Lines 79 (patched) <https://reviews.apache.org/r/72828/#comment310857> ACCESS_TYPE_READ => ACCESS_TYPE_TYPE_READ plugin-atlas/src/main/java/org/apache/ranger/services/atlas/RangerServiceAtlas.java Lines 210 (patched) <https://reviews.apache.org/r/72828/#comment310867> Given group=public is given type-read permission here, there is no need to explictly add user=tagSyncUser. I suggest to: - remove #210 - rename policyItemForTagSyncUser => policyItemTypeReadForAll security-admin/src/main/java/org/apache/ranger/patch/PatchForAtlasToAddTypeRead_J10039.java Lines 201 (patched) <https://reviews.apache.org/r/72828/#comment310858> Instead of over writting with all type-* accesses, consider adding type-read access to existing accesses. security-admin/src/main/java/org/apache/ranger/patch/PatchForAtlasToAddTypeRead_J10039.java Lines 205 (patched) <https://reviews.apache.org/r/72828/#comment310859> Couple of issues here: - public group is added only if item.getGroups() is not null. I think the condition should be: if (item.getGroups() == null || !item.getGroups.contains(GROUP_PUBLIC)) - should group=public be added to every policy-item? I suggest to replace #205 - #209 by adding following policy-item to this policy: group=public accesses=type-read - Madhan Neethiraj On Sept. 2, 2020, 5:47 p.m., Nixon Rodrigues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72828/ > ----------------------------------------------------------- > > (Updated Sept. 2, 2020, 5:47 p.m.) > > > Review request for ranger, Jayendra Parab, Madhan Neethiraj, Mehul Parikh, > Ramesh Mani, and Sarath Subramanian. > > > Bugs: RANGER-2929 > https://issues.apache.org/jira/browse/RANGER-2929 > > > Repository: ranger > > > Description > ------- > > Update Atlas Ranger Authorizer for "type-read" accessType changes done in > ATLAS-3898. > > Currently in the Atlas-Ranger plugin for types resource READ permission is > not available and read access is available by default to all types of any > category. > > This patch updates service-def with "type-read" permission and updates > authorizer for read of all typedefs and also filters typesdefs based on > access provided. > > > Diffs > ----- > > agents-common/src/main/resources/service-defs/ranger-servicedef-atlas.json > 7672be05a > > plugin-atlas/src/main/java/org/apache/ranger/authorization/atlas/authorizer/RangerAtlasAuthorizer.java > 28d71de21 > > plugin-atlas/src/main/java/org/apache/ranger/services/atlas/RangerServiceAtlas.java > 7c89ffef5 > pom.xml 1f88b27e4 > > ranger-atlas-plugin-shim/src/main/java/org/apache/ranger/authorization/atlas/authorizer/RangerAtlasAuthorizer.java > 0e220f132 > security-admin/db/mysql/optimized/current/ranger_core_db_mysql.sql > dfaf3c987 > security-admin/db/oracle/optimized/current/ranger_core_db_oracle.sql > 21626f6dc > security-admin/db/postgres/optimized/current/ranger_core_db_postgres.sql > 5cd2cc798 > > security-admin/db/sqlanywhere/optimized/current/ranger_core_db_sqlanywhere.sql > 081b153a3 > security-admin/db/sqlserver/optimized/current/ranger_core_db_sqlserver.sql > 642d6c151 > > security-admin/src/main/java/org/apache/ranger/patch/PatchForAtlasToAddTypeRead_J10039.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/72828/diff/3/ > > > Testing > ------- > > Tested Atlas UI and typedefs API functionality by setting policies in ranger > Admin for type-category/type resources . > > > Thanks, > > Nixon Rodrigues > >
