From a8e3913bad134377ee710cf4abecc349fe70c213 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 2 Apr 2025 14:08:40 -0700 Subject: [PATCH 1/4] RANGER-5178: Update callers of executeOnTransactionCommit to check for existence of objects --- .../apache/ranger/biz/PolicyRefUpdater.java | 60 +++++++------- .../org/apache/ranger/biz/RoleRefUpdater.java | 78 ++++++++++++------- 2 files changed, 82 insertions(+), 56 deletions(-) diff --git a/security-admin/src/main/java/org/apache/ranger/biz/PolicyRefUpdater.java b/security-admin/src/main/java/org/apache/ranger/biz/PolicyRefUpdater.java index 4ed8ef9105..ad42e7dacc 100644 --- a/security-admin/src/main/java/org/apache/ranger/biz/PolicyRefUpdater.java +++ b/security-admin/src/main/java/org/apache/ranger/biz/PolicyRefUpdater.java @@ -516,42 +516,50 @@ private Long createPrincipal(String user) { return ret; } + private boolean doesPolicyExist(XXPolicy xPolicy) { + return daoMgr.getXXPolicy().getById(xPolicy.getId()) != null; + } + private void createPolicyAssociation(Long id, String name) { LOG.debug("===> PolicyPrincipalAssociator.createPolicyAssociation(policyId={}, type={}, name={}, id={})", xPolicy.getId(), type.name(), name, id); - switch (type) { - case USER: { - XXPolicyRefUser xPolUser = rangerAuditFields.populateAuditFields(new XXPolicyRefUser(), xPolicy); + if (doesPolicyExist(xPolicy)) { + switch (type) { + case USER: { + XXPolicyRefUser xPolUser = rangerAuditFields.populateAuditFields(new XXPolicyRefUser(), xPolicy); - xPolUser.setPolicyId(xPolicy.getId()); - xPolUser.setUserId(id); - xPolUser.setUserName(name); + xPolUser.setPolicyId(xPolicy.getId()); + xPolUser.setUserId(id); + xPolUser.setUserName(name); - daoMgr.getXXPolicyRefUser().create(xPolUser); - } - break; - case GROUP: { - XXPolicyRefGroup xPolGroup = rangerAuditFields.populateAuditFields(new XXPolicyRefGroup(), xPolicy); + daoMgr.getXXPolicyRefUser().create(xPolUser); + } + break; + case GROUP: { + XXPolicyRefGroup xPolGroup = rangerAuditFields.populateAuditFields(new XXPolicyRefGroup(), xPolicy); - xPolGroup.setPolicyId(xPolicy.getId()); - xPolGroup.setGroupId(id); - xPolGroup.setGroupName(name); + xPolGroup.setPolicyId(xPolicy.getId()); + xPolGroup.setGroupId(id); + xPolGroup.setGroupName(name); - daoMgr.getXXPolicyRefGroup().create(xPolGroup); - } - break; - case ROLE: { - XXPolicyRefRole xPolRole = rangerAuditFields.populateAuditFields(new XXPolicyRefRole(), xPolicy); + daoMgr.getXXPolicyRefGroup().create(xPolGroup); + } + break; + case ROLE: { + XXPolicyRefRole xPolRole = rangerAuditFields.populateAuditFields(new XXPolicyRefRole(), xPolicy); - xPolRole.setPolicyId(xPolicy.getId()); - xPolRole.setRoleId(id); - xPolRole.setRoleName(name); + xPolRole.setPolicyId(xPolicy.getId()); + xPolRole.setRoleId(id); + xPolRole.setRoleName(name); - daoMgr.getXXPolicyRefRole().create(xPolRole); - } - break; - default: + daoMgr.getXXPolicyRefRole().create(xPolRole); + } break; + default: + break; + } + } else { + LOG.info("Policy with id = {} does not exist, skipping policy association!", xPolicy.getId()); } LOG.debug("<=== PolicyPrincipalAssociator.createPolicyAssociation(policyId={}, type={}, name={}, id={})", xPolicy.getId(), type.name(), name, id); diff --git a/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java b/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java index 3863f5a088..23abb74c88 100644 --- a/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java +++ b/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java @@ -345,45 +345,63 @@ private Long createPrincipal(String user) { return ret; } + private boolean doesPrincipalExist(String name, PolicyRefUpdater.PRINCIPAL_TYPE type) { + switch (type) { + case USER: + return daoMgr.getXXUser().findByUserName(name) != null; + case GROUP: + return daoMgr.getXXGroup().findByGroupName(name) != null; + case ROLE: + return daoMgr.getXXRole().findByRoleName(name) != null; + default: + break; + } + return false; + } + private void createRoleAssociation(Long id, String name) { LOG.debug("===> RolePrincipalAssociator.createRoleAssociation(roleId={}, type={}, name={}, id={})", roleId, type.name(), name, id); - switch (type) { - case USER: { - XXRoleRefUser xRoleRefUser = rangerAuditFields.populateAuditFieldsForCreate(new XXRoleRefUser()); + if (doesPrincipalExist(name, type)) { + switch (type) { + case USER: { + XXRoleRefUser xRoleRefUser = rangerAuditFields.populateAuditFieldsForCreate(new XXRoleRefUser()); - xRoleRefUser.setRoleId(roleId); - xRoleRefUser.setUserId(id); - xRoleRefUser.setUserName(name); - xRoleRefUser.setUserType(0); + xRoleRefUser.setRoleId(roleId); + xRoleRefUser.setUserId(id); + xRoleRefUser.setUserName(name); + xRoleRefUser.setUserType(0); - daoMgr.getXXRoleRefUser().create(xRoleRefUser); - } - break; - case GROUP: { - XXRoleRefGroup xRoleRefGroup = rangerAuditFields.populateAuditFieldsForCreate(new XXRoleRefGroup()); + daoMgr.getXXRoleRefUser().create(xRoleRefUser); + } + break; + case GROUP: { + XXRoleRefGroup xRoleRefGroup = rangerAuditFields.populateAuditFieldsForCreate(new XXRoleRefGroup()); - xRoleRefGroup.setRoleId(roleId); - xRoleRefGroup.setGroupId(id); - xRoleRefGroup.setGroupName(name); - xRoleRefGroup.setGroupType(0); + xRoleRefGroup.setRoleId(roleId); + xRoleRefGroup.setGroupId(id); + xRoleRefGroup.setGroupName(name); + xRoleRefGroup.setGroupType(0); - daoMgr.getXXRoleRefGroup().create(xRoleRefGroup); - } - break; - case ROLE: { - XXRoleRefRole xRoleRefRole = rangerAuditFields.populateAuditFieldsForCreate(new XXRoleRefRole()); + daoMgr.getXXRoleRefGroup().create(xRoleRefGroup); + } + break; + case ROLE: { + XXRoleRefRole xRoleRefRole = rangerAuditFields.populateAuditFieldsForCreate(new XXRoleRefRole()); - xRoleRefRole.setRoleId(roleId); - xRoleRefRole.setSubRoleId(id); - xRoleRefRole.setSubRoleName(name); - xRoleRefRole.setSubRoleType(0); + xRoleRefRole.setRoleId(roleId); + xRoleRefRole.setSubRoleId(id); + xRoleRefRole.setSubRoleName(name); + xRoleRefRole.setSubRoleType(0); - daoMgr.getXXRoleRefRole().create(xRoleRefRole); - } - break; - default: - break; + daoMgr.getXXRoleRefRole().create(xRoleRefRole); + } + break; + default: + break; + } + } else { + LOG.info("Principal with type = {}, name = {} does not exist, skipping role association!", type.name(), name); } LOG.debug("<=== RolePrincipalAssociator.createRoleAssociation(roleId={}, type={}, name={}, id={})", roleId, type.name(), name, id); From 85723ca5462488cd9532084d40fe2a66938f4158 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 2 Apr 2025 15:04:51 -0700 Subject: [PATCH 2/4] Address checkstyle errors --- .../org/apache/ranger/biz/RoleRefUpdater.java | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java b/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java index 23abb74c88..c90dcb296b 100644 --- a/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java +++ b/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java @@ -365,41 +365,41 @@ private void createRoleAssociation(Long id, String name) { if (doesPrincipalExist(name, type)) { switch (type) { case USER: { - XXRoleRefUser xRoleRefUser = rangerAuditFields.populateAuditFieldsForCreate(new XXRoleRefUser()); + XXRoleRefUser xRoleRefUser = rangerAuditFields.populateAuditFieldsForCreate(new XXRoleRefUser()); - xRoleRefUser.setRoleId(roleId); - xRoleRefUser.setUserId(id); - xRoleRefUser.setUserName(name); - xRoleRefUser.setUserType(0); + xRoleRefUser.setRoleId(roleId); + xRoleRefUser.setUserId(id); + xRoleRefUser.setUserName(name); + xRoleRefUser.setUserType(0); - daoMgr.getXXRoleRefUser().create(xRoleRefUser); - } - break; - case GROUP: { - XXRoleRefGroup xRoleRefGroup = rangerAuditFields.populateAuditFieldsForCreate(new XXRoleRefGroup()); + daoMgr.getXXRoleRefUser().create(xRoleRefUser); + } + break; + case GROUP: { + XXRoleRefGroup xRoleRefGroup = rangerAuditFields.populateAuditFieldsForCreate(new XXRoleRefGroup()); - xRoleRefGroup.setRoleId(roleId); - xRoleRefGroup.setGroupId(id); - xRoleRefGroup.setGroupName(name); - xRoleRefGroup.setGroupType(0); + xRoleRefGroup.setRoleId(roleId); + xRoleRefGroup.setGroupId(id); + xRoleRefGroup.setGroupName(name); + xRoleRefGroup.setGroupType(0); - daoMgr.getXXRoleRefGroup().create(xRoleRefGroup); - } - break; - case ROLE: { - XXRoleRefRole xRoleRefRole = rangerAuditFields.populateAuditFieldsForCreate(new XXRoleRefRole()); + daoMgr.getXXRoleRefGroup().create(xRoleRefGroup); + } + break; + case ROLE: { + XXRoleRefRole xRoleRefRole = rangerAuditFields.populateAuditFieldsForCreate(new XXRoleRefRole()); - xRoleRefRole.setRoleId(roleId); - xRoleRefRole.setSubRoleId(id); - xRoleRefRole.setSubRoleName(name); - xRoleRefRole.setSubRoleType(0); + xRoleRefRole.setRoleId(roleId); + xRoleRefRole.setSubRoleId(id); + xRoleRefRole.setSubRoleName(name); + xRoleRefRole.setSubRoleType(0); - daoMgr.getXXRoleRefRole().create(xRoleRefRole); - } - break; - default: - break; + daoMgr.getXXRoleRefRole().create(xRoleRefRole); } + break; + default: + break; + } } else { LOG.info("Principal with type = {}, name = {} does not exist, skipping role association!", type.name(), name); } From 43cf08144936be18661151b0bd80c9ae0d2a7f8d Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 4 Apr 2025 12:54:01 -0700 Subject: [PATCH 3/4] Address review comments --- .../org/apache/ranger/biz/RoleRefUpdater.java | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java b/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java index c90dcb296b..7d2ad6cc2d 100644 --- a/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java +++ b/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java @@ -345,24 +345,14 @@ private Long createPrincipal(String user) { return ret; } - private boolean doesPrincipalExist(String name, PolicyRefUpdater.PRINCIPAL_TYPE type) { - switch (type) { - case USER: - return daoMgr.getXXUser().findByUserName(name) != null; - case GROUP: - return daoMgr.getXXGroup().findByGroupName(name) != null; - case ROLE: - return daoMgr.getXXRole().findByRoleName(name) != null; - default: - break; - } - return false; + private boolean doesRoleExist(){ + return roleId != null && daoMgr.getXXRole().findByRoleId(roleId) != null; } private void createRoleAssociation(Long id, String name) { LOG.debug("===> RolePrincipalAssociator.createRoleAssociation(roleId={}, type={}, name={}, id={})", roleId, type.name(), name, id); - if (doesPrincipalExist(name, type)) { + if (doesRoleExist()) { switch (type) { case USER: { XXRoleRefUser xRoleRefUser = rangerAuditFields.populateAuditFieldsForCreate(new XXRoleRefUser()); @@ -401,7 +391,7 @@ private void createRoleAssociation(Long id, String name) { break; } } else { - LOG.info("Principal with type = {}, name = {} does not exist, skipping role association!", type.name(), name); + LOG.info("Role with id = {} does not exist, skipping role association!", roleId); } LOG.debug("<=== RolePrincipalAssociator.createRoleAssociation(roleId={}, type={}, name={}, id={})", roleId, type.name(), name, id); From a0d1992a15263098db2b52fa61fc64a98b586dba Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 4 Apr 2025 13:13:56 -0700 Subject: [PATCH 4/4] Fix checkstle error --- .../src/main/java/org/apache/ranger/biz/RoleRefUpdater.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java b/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java index 7d2ad6cc2d..04d7104d29 100644 --- a/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java +++ b/security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java @@ -345,7 +345,7 @@ private Long createPrincipal(String user) { return ret; } - private boolean doesRoleExist(){ + private boolean doesRoleExist() { return roleId != null && daoMgr.getXXRole().findByRoleId(roleId) != null; }