From 71e5a6a307e02e66e6b0ca4dc23804617ab6ea89 Mon Sep 17 00:00:00 2001 From: krishnamuttevi Date: Tue, 2 Jun 2026 11:47:16 +0530 Subject: [PATCH 1/4] RANGER-5623: Updated XUserMgr.java to solve UI consistency in groupusers --- .../java/org/apache/ranger/biz/XUserMgr.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java b/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java index 429da84f3df..0fa0b60f92d 100755 --- a/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java +++ b/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java @@ -1653,6 +1653,32 @@ public VXGroupUser updateXGroupUser(VXGroupUser vXGroupUser) { xaBizUtil.blockAuditorRoleUser(); + XXGroupUser xxGroupUser = null; + + if (vXGroupUser.getId() != null) { + xxGroupUser = daoManager.getXXGroupUser().getById(vXGroupUser.getId()); + if (xxGroupUser == null) { + throw restErrorUtil.createRESTException( + "Group-User mapping not found with ID: " + vXGroupUser.getId(), + MessageEnums.DATA_NOT_FOUND); + } + } + + XXGroup xGroup = daoManager.getXXGroup().findByGroupName(vXGroupUser.getName()); + if (xGroup == null) { + throw restErrorUtil.createRESTException("Group not found: " + vXGroupUser.getName(), + MessageEnums.DATA_NOT_FOUND); + } + + XXUser xUser = daoManager.getXXUser().getById(vXGroupUser.getUserId()); + if (xUser == null) { + throw restErrorUtil.createRESTException("User not found with ID: " + vXGroupUser.getUserId(), + MessageEnums.DATA_NOT_FOUND); + } + + vXGroupUser.setParentGroupId(xGroup.getId()); + + return super.updateXGroupUser(vXGroupUser); } From 32359ee2ca358c31e7e2502c5f475220796af954 Mon Sep 17 00:00:00 2001 From: krishnamuttevi Date: Tue, 2 Jun 2026 19:08:11 +0530 Subject: [PATCH 2/4] RANGER-5623: Updated the TextXUserMgr and resolved the checkstyle error --- .../java/org/apache/ranger/biz/XUserMgr.java | 1 - .../org/apache/ranger/biz/TestXUserMgr.java | 34 ++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java b/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java index 0fa0b60f92d..47b8a081683 100755 --- a/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java +++ b/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java @@ -1678,7 +1678,6 @@ public VXGroupUser updateXGroupUser(VXGroupUser vXGroupUser) { vXGroupUser.setParentGroupId(xGroup.getId()); - return super.updateXGroupUser(vXGroupUser); } diff --git a/security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java b/security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java index 54b37319f2b..0f375921392 100644 --- a/security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java +++ b/security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java @@ -2031,6 +2031,22 @@ public void test55updateXGroupUser() { VXUser vxUser = vxUser(); vxUser.setUserSource(RangerCommonEnums.USER_EXTERNAL); VXGroupUser vxGroupUser = vxGroupUser(); + // Mock XXGroupUser lookup + XXGroupUserDao xxGroupUserDao = Mockito.mock(XXGroupUserDao.class); + XXGroupUser xxGroupUser = new XXGroupUser(); + Mockito.when(daoManager.getXXGroupUser()).thenReturn(xxGroupUserDao); + Mockito.when(xxGroupUserDao.getById(vxGroupUser.getId())).thenReturn(xxGroupUser); + // Mock XXGroup lookup + XXGroupDao xxGroupDao = Mockito.mock(XXGroupDao.class); + XXGroup xxGroup = new XXGroup(); + xxGroup.setId(1L); + Mockito.when(daoManager.getXXGroup()).thenReturn(xxGroupDao); + Mockito.when(xxGroupDao.findByGroupName(vxGroupUser.getName())).thenReturn(xxGroup); + // Mock XXUser lookup + XXUserDao xxUserDao = Mockito.mock(XXUserDao.class); + XXUser xxUser = new XXUser(); + Mockito.when(daoManager.getXXUser()).thenReturn(xxUserDao); + Mockito.when(xxUserDao.getById(vxGroupUser.getUserId())).thenReturn(xxUser); Mockito.when(xGroupUserService.updateResource(Mockito.any())).thenReturn(vxGroupUser); VXGroupUser dbvxUser = xUserMgr.updateXGroupUser(vxGroupUser); Assertions.assertNotNull(dbvxUser); @@ -2492,9 +2508,25 @@ public void test82updateXgroupUserForGroupUpdate() { xXGroupUserList.add(xXGroupUser); Mockito.when(daoManager.getXXGroupUser()).thenReturn(xxGroupUserDao); Mockito.when(xxGroupUserDao.findByGroupId(vXGroup.getId())).thenReturn(xXGroupUserList); + // CHANGED: Use anyLong() so it matches regardless of internal ID state + Mockito.when(xxGroupUserDao.getById(Mockito.anyLong())).thenReturn(xXGroupUser); + // Mock XXGroup lookup + XXGroupDao xxGroupDao = Mockito.mock(XXGroupDao.class); + XXGroup xxGroup = new XXGroup(); + xxGroup.setId(vXGroup.getId()); + Mockito.when(daoManager.getXXGroup()).thenReturn(xxGroupDao); + // CHANGED: Use anyString() because the group name likely gets altered internally during the loop + Mockito.when(xxGroupDao.findByGroupName(Mockito.anyString())).thenReturn(xxGroup); + // Mock XXUser lookup + XXUserDao xxUserDao = Mockito.mock(XXUserDao.class); + XXUser xxUser = new XXUser(); + Mockito.when(daoManager.getXXUser()).thenReturn(xxUserDao); + // CHANGED: Use anyLong() for user ID lookup + Mockito.when(xxUserDao.getById(Mockito.anyLong())).thenReturn(xxUser); Mockito.when(xGroupUserService.populateViewBean(xXGroupUser)).thenReturn(vxGroupUser); xUserMgr.updateXgroupUserForGroupUpdate(vXGroup); - Mockito.verify(daoManager).getXXGroupUser(); + // Verify changes + Mockito.verify(daoManager, Mockito.atLeastOnce()).getXXGroupUser(); Mockito.verify(xxGroupUserDao).findByGroupId(vXGroup.getId()); } From f69bb31002584d7b5ceb526cbef983bb5102d8db Mon Sep 17 00:00:00 2001 From: krishnamuttevi Date: Wed, 3 Jun 2026 11:37:15 +0530 Subject: [PATCH 3/4] RANGER-5623: Added the validation check for the invalid vXGroupUser input --- .../src/main/java/org/apache/ranger/biz/XUserMgr.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java b/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java index 47b8a081683..6fbb1b9bc19 100755 --- a/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java +++ b/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java @@ -1653,6 +1653,10 @@ public VXGroupUser updateXGroupUser(VXGroupUser vXGroupUser) { xaBizUtil.blockAuditorRoleUser(); + if (vXGroupUser == null) { + throw restErrorUtil.createRESTException(HttpServletResponse.SC_BAD_REQUEST, "Groupuser not found", true); + } + XXGroupUser xxGroupUser = null; if (vXGroupUser.getId() != null) { From 5b4b5784d4c1ced17accdbdb928331cc3efb9dc9 Mon Sep 17 00:00:00 2001 From: krishnamuttevi Date: Mon, 8 Jun 2026 13:13:52 +0530 Subject: [PATCH 4/4] RANGER-5623: Addressed the review comments to optimise and add proper validation --- .../java/org/apache/ranger/biz/XUserMgr.java | 12 +- .../org/apache/ranger/biz/TestXUserMgr.java | 148 +++++++++++++----- 2 files changed, 109 insertions(+), 51 deletions(-) diff --git a/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java b/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java index 6fbb1b9bc19..466dbcf1d33 100755 --- a/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java +++ b/security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java @@ -1657,15 +1657,9 @@ public VXGroupUser updateXGroupUser(VXGroupUser vXGroupUser) { throw restErrorUtil.createRESTException(HttpServletResponse.SC_BAD_REQUEST, "Groupuser not found", true); } - XXGroupUser xxGroupUser = null; - - if (vXGroupUser.getId() != null) { - xxGroupUser = daoManager.getXXGroupUser().getById(vXGroupUser.getId()); - if (xxGroupUser == null) { - throw restErrorUtil.createRESTException( - "Group-User mapping not found with ID: " + vXGroupUser.getId(), - MessageEnums.DATA_NOT_FOUND); - } + if (vXGroupUser.getId() != null && daoManager.getXXGroupUser().getById(vXGroupUser.getId()) == null) { + throw restErrorUtil.createRESTException("Group-User mapping not found with ID: " + vXGroupUser.getId(), + MessageEnums.DATA_NOT_FOUND); } XXGroup xGroup = daoManager.getXXGroup().findByGroupName(vXGroupUser.getName()); diff --git a/security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java b/security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java index 0f375921392..1cd362ecd31 100644 --- a/security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java +++ b/security-admin/src/test/java/org/apache/ranger/biz/TestXUserMgr.java @@ -2028,30 +2028,47 @@ public void test54modifyUserActiveStatus() { @Test public void test55updateXGroupUser() { setup(); - VXUser vxUser = vxUser(); - vxUser.setUserSource(RangerCommonEnums.USER_EXTERNAL); + VXGroupUser vxGroupUser = vxGroupUser(); - // Mock XXGroupUser lookup + + // Mock GroupUser XXGroupUserDao xxGroupUserDao = Mockito.mock(XXGroupUserDao.class); XXGroupUser xxGroupUser = new XXGroupUser(); + Mockito.when(daoManager.getXXGroupUser()).thenReturn(xxGroupUserDao); Mockito.when(xxGroupUserDao.getById(vxGroupUser.getId())).thenReturn(xxGroupUser); - // Mock XXGroup lookup + + // Mock Group XXGroupDao xxGroupDao = Mockito.mock(XXGroupDao.class); XXGroup xxGroup = new XXGroup(); xxGroup.setId(1L); + Mockito.when(daoManager.getXXGroup()).thenReturn(xxGroupDao); Mockito.when(xxGroupDao.findByGroupName(vxGroupUser.getName())).thenReturn(xxGroup); - // Mock XXUser lookup + + // Mock User XXUserDao xxUserDao = Mockito.mock(XXUserDao.class); XXUser xxUser = new XXUser(); + Mockito.when(daoManager.getXXUser()).thenReturn(xxUserDao); Mockito.when(xxUserDao.getById(vxGroupUser.getUserId())).thenReturn(xxUser); + + // Mock update Mockito.when(xGroupUserService.updateResource(Mockito.any())).thenReturn(vxGroupUser); - VXGroupUser dbvxUser = xUserMgr.updateXGroupUser(vxGroupUser); - Assertions.assertNotNull(dbvxUser); - Assertions.assertEquals(dbvxUser.getId(), vxGroupUser.getId()); - Assertions.assertEquals(dbvxUser.getName(), vxGroupUser.getName()); + + VXGroupUser result = xUserMgr.updateXGroupUser(vxGroupUser); + + Assertions.assertNotNull(result); + Assertions.assertEquals(vxGroupUser.getId(), result.getId()); + Assertions.assertEquals(vxGroupUser.getName(), result.getName()); + + // addition of parent group id + Assertions.assertEquals(1L, result.getParentGroupId()); + + Mockito.verify(xxGroupUserDao).getById(vxGroupUser.getId()); + Mockito.verify(xxGroupDao).findByGroupName(vxGroupUser.getName()); + Mockito.verify(xxUserDao).getById(vxGroupUser.getUserId()); + Mockito.verify(xGroupUserService).updateResource(Mockito.any()); } @@ -2494,40 +2511,14 @@ public void test81checkAdminAccess() { } @Test - public void test82updateXgroupUserForGroupUpdate() { + public void test82updateXGroupUser_nullVXGroupUser() { setup(); - XXGroupUserDao xxGroupUserDao = Mockito.mock(XXGroupUserDao.class); - VXGroup vXGroup = vxGroup(); - List xXGroupUserList = new ArrayList<>(); - VXGroupUser vxGroupUser = vxGroupUser(); - XXGroupUser xXGroupUser = new XXGroupUser(); - xXGroupUser.setId(vxGroupUser.getId()); - xXGroupUser.setName(vxGroupUser.getName()); - xXGroupUser.setParentGroupId(vxGroupUser.getParentGroupId()); - xXGroupUser.setUserId(vxGroupUser.getUserId()); - xXGroupUserList.add(xXGroupUser); - Mockito.when(daoManager.getXXGroupUser()).thenReturn(xxGroupUserDao); - Mockito.when(xxGroupUserDao.findByGroupId(vXGroup.getId())).thenReturn(xXGroupUserList); - // CHANGED: Use anyLong() so it matches regardless of internal ID state - Mockito.when(xxGroupUserDao.getById(Mockito.anyLong())).thenReturn(xXGroupUser); - // Mock XXGroup lookup - XXGroupDao xxGroupDao = Mockito.mock(XXGroupDao.class); - XXGroup xxGroup = new XXGroup(); - xxGroup.setId(vXGroup.getId()); - Mockito.when(daoManager.getXXGroup()).thenReturn(xxGroupDao); - // CHANGED: Use anyString() because the group name likely gets altered internally during the loop - Mockito.when(xxGroupDao.findByGroupName(Mockito.anyString())).thenReturn(xxGroup); - // Mock XXUser lookup - XXUserDao xxUserDao = Mockito.mock(XXUserDao.class); - XXUser xxUser = new XXUser(); - Mockito.when(daoManager.getXXUser()).thenReturn(xxUserDao); - // CHANGED: Use anyLong() for user ID lookup - Mockito.when(xxUserDao.getById(Mockito.anyLong())).thenReturn(xxUser); - Mockito.when(xGroupUserService.populateViewBean(xXGroupUser)).thenReturn(vxGroupUser); - xUserMgr.updateXgroupUserForGroupUpdate(vXGroup); - // Verify changes - Mockito.verify(daoManager, Mockito.atLeastOnce()).getXXGroupUser(); - Mockito.verify(xxGroupUserDao).findByGroupId(vXGroup.getId()); + + Mockito.when(restErrorUtil.createRESTException(HttpServletResponse.SC_BAD_REQUEST, "Groupuser not found", true)).thenThrow(new WebApplicationException()); + + Assertions.assertThrows(WebApplicationException.class, () -> { + xUserMgr.updateXGroupUser(null); + }); } @Test @@ -4489,6 +4480,79 @@ public void test01CreateXUser_federated() { }); } + @Test + public void test133updateXGroupUser_groupUserMappingNotFound() { + setup(); + + VXGroupUser vxGroupUser = vxGroupUser(); + + XXGroupUserDao xxGroupUserDao = Mockito.mock(XXGroupUserDao.class); + + Mockito.when(daoManager.getXXGroupUser()).thenReturn(xxGroupUserDao); + Mockito.when(xxGroupUserDao.getById(vxGroupUser.getId())).thenReturn(null); + + Mockito.when(restErrorUtil.createRESTException(Mockito.contains("Group-User mapping not found"), Mockito.eq(MessageEnums.DATA_NOT_FOUND))).thenThrow(new WebApplicationException()); + + Assertions.assertThrows(WebApplicationException.class, () -> { + xUserMgr.updateXGroupUser(vxGroupUser); + }); + } + + @Test + public void test134updateXGroupUser_groupNotFound() { + setup(); + + VXGroupUser vxGroupUser = vxGroupUser(); + + XXGroupUserDao xxGroupUserDao = Mockito.mock(XXGroupUserDao.class); + XXGroupUser xxGroupUser = new XXGroupUser(); + + Mockito.when(daoManager.getXXGroupUser()).thenReturn(xxGroupUserDao); + Mockito.when(xxGroupUserDao.getById(vxGroupUser.getId())).thenReturn(xxGroupUser); + + XXGroupDao xxGroupDao = Mockito.mock(XXGroupDao.class); + + Mockito.when(daoManager.getXXGroup()).thenReturn(xxGroupDao); + Mockito.when(xxGroupDao.findByGroupName(vxGroupUser.getName())).thenReturn(null); + + Mockito.when(restErrorUtil.createRESTException(Mockito.contains("Group not found"), Mockito.eq(MessageEnums.DATA_NOT_FOUND))).thenThrow(new WebApplicationException()); + + Assertions.assertThrows(WebApplicationException.class, () -> { + xUserMgr.updateXGroupUser(vxGroupUser); + }); + } + + @Test + public void test135updateXGroupUser_userNotFound() { + setup(); + + VXGroupUser vxGroupUser = vxGroupUser(); + + XXGroupUserDao xxGroupUserDao = Mockito.mock(XXGroupUserDao.class); + XXGroupUser xxGroupUser = new XXGroupUser(); + + Mockito.when(daoManager.getXXGroupUser()).thenReturn(xxGroupUserDao); + Mockito.when(xxGroupUserDao.getById(vxGroupUser.getId())).thenReturn(xxGroupUser); + + XXGroupDao xxGroupDao = Mockito.mock(XXGroupDao.class); + XXGroup xxGroup = new XXGroup(); + xxGroup.setId(1L); + + Mockito.when(daoManager.getXXGroup()).thenReturn(xxGroupDao); + Mockito.when(xxGroupDao.findByGroupName(vxGroupUser.getName())).thenReturn(xxGroup); + + XXUserDao xxUserDao = Mockito.mock(XXUserDao.class); + + Mockito.when(daoManager.getXXUser()).thenReturn(xxUserDao); + Mockito.when(xxUserDao.getById(vxGroupUser.getUserId())).thenReturn(null); + + Mockito.when(restErrorUtil.createRESTException(Mockito.contains("User not found"), Mockito.eq(MessageEnums.DATA_NOT_FOUND))).thenThrow(new WebApplicationException()); + + Assertions.assertThrows(WebApplicationException.class, () -> { + xUserMgr.updateXGroupUser(vxGroupUser); + }); + } + private VXUser vxUser() { Collection userRoleList = new ArrayList<>(); userRoleList.add("ROLE_USER");