mneethiraj commented on code in PR #1000:
URL: https://github.com/apache/ranger/pull/1000#discussion_r3369049665
##########
security-admin/src/main/java/org/apache/ranger/common/UserSessionBase.java:
##########
@@ -37,6 +37,8 @@ public class UserSessionBase implements Serializable {
private boolean userAuditAdmin;
private boolean auditKeyAdmin;
private boolean keyAdmin;
+ /** Set at login when login matches {@code ranger.admin.super.users} /
super.groups. */
+ private boolean configSuperUser;
Review Comment:
- I suggest renaming `configSuperUser` to `isSuperUser`
- I suggest removing method `isEffectiveRangerAdmin()`; instead:
- update `isUserAdmin()` as: `return isSuperUser || userAdmin;`
- update `isAuditUserAdmin()` as: `return isSuperUser || userAuditAdmin;`
- update `isKeyAdmin()` as: `return isSuperUser || keyAdmin;`
- update `isAuditKeyAdmin() as: `return isSuperUser || auditKeyAdmin;`
##########
security-admin/src/main/java/org/apache/ranger/biz/RangerBizUtil.java:
##########
Review Comment:
For better readability, I suggest replacing lines 1043 to 1052 with the
following:
```
if (!session.isSuperUser()) {
boolean isKmsServiceType =
EmbeddedServiceDefsUtil.KMS_IMPL_CLASS_NAME.equals(implClassName);
if (session.isKeyAdmin() && !isKmsServiceType) {
throw restErrorUtil.createRESTException("KeyAdmin can
create/update/delete only KMS " + objType, MessageEnums.OPER_NO_PERMISSION);
}
if (session.isUserAdmin() && isKmsServiceType &&
"Service-Def".equalsIgnoreCase(objType)) {
throw restErrorUtil.createRESTException("System Admin cannot
create/update/delete KMS " + objType, MessageEnums.OPER_NO_PERMISSION);
}
}
```
##########
security-admin/scripts/ranger-admin-site-template.xml:
##########
@@ -248,4 +248,14 @@
<name>ranger.ldap.ad.url</name>
<value></value>
</property>
+ <property>
+ <name>ranger.admin.super.users</name>
+ <value></value>
+ <description>Comma-separated list of externally authenticated
users granted full Ranger administrative privileges at login (system admin and
key admin capabilities), independent of database roles. Empty or absent
disables config-based super users.</description>
Review Comment:
I suggest removing "externally authenticated", since 'authenticated' users
is implied and this is not restricted to "external authenticated users".
##########
security-admin/scripts/ranger-admin-site-template.xml:
##########
@@ -248,4 +248,14 @@
<name>ranger.ldap.ad.url</name>
<value></value>
</property>
+ <property>
+ <name>ranger.admin.super.users</name>
+ <value></value>
+ <description>Comma-separated list of externally authenticated
users granted full Ranger administrative privileges at login (system admin and
key admin capabilities), independent of database roles. Empty or absent
disables config-based super users.</description>
+ </property>
+ <property>
+ <name>ranger.admin.super.groups</name>
+ <value></value>
+ <description>Comma-separated list of groups whose members are
granted full Ranger administrative privileges at login (system admin and key
admin capabilities), independent of database roles. Group membership is
resolved from the Ranger user store (usersync). Empty or absent disables
config-based super groups.</description>
Review Comment:
I suggest removing "Group membership is resolved from the Ranger user store
(usersync)", as the group membership is obtained during authentication which
can be from LDAP or other sources i.e., not from Ranger user store.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
