vyommani commented on code in PR #1229:
URL: https://github.com/apache/ranger/pull/1229#discussion_r4011579213
##########
security-admin/src/main/java/org/apache/ranger/rest/SecurityZoneREST.java:
##########
@@ -695,20 +697,60 @@ private void blockAdminFromKMSService(RangerSecurityZone
securityZone) {
if (serviceMap != null) {
for (String serviceName : serviceMap.keySet()) {
- XXService xService =
daoManager.getXXService().findByName(serviceName);
-
- if (xService != null) {
- XXServiceDef xServiceDef =
daoManager.getXXServiceDef().getById(xService.getType());
+ String serviceType =
daoManager.getXXServiceDef().findServiceDefTypeByServiceName(serviceName);
- if
(EmbeddedServiceDefsUtil.KMS_IMPL_CLASS_NAME.equals(xServiceDef.getImplclassname()))
{
- throw restErrorUtil.createRESTException("KMS
Services/Service-Defs are not accessible for Zone operations",
MessageEnums.OPER_NOT_ALLOWED_FOR_ENTITY);
- }
+ if
(EmbeddedServiceDefsUtil.EMBEDDED_SERVICEDEF_KMS_NAME.equals(serviceType)) {
Review Comment:
This changes what "KMS service" means for the gate. Every other
sys-admin/key-admin boundary in security-admin keys on implclassname ==
KMS_IMPL_CLASS_NAME ServiceREST (getServices filtering, policy CRUD,
validateConfig) and RangerBizUtil (the "System Admin cannot
create/update/delete KMS Service-Def" check). A key admin can legitimately
create a service-def with a different name and the KMS impl class; services
under it are key-admin-only everywhere else but would pass this gate for a sys
admin.
If the motivation was the two-step lookup / latent NPE on a null
xServiceDef, a findServiceDefImplClassByServiceName named query keeps the
single round-trip without changing semantics. If name-based is intentional,
worth stating in the JIRA since it's a behavior change in a refactor.
@mneethiraj for a second opinion.
--
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]
