Skip to content

Ranger -5587 modified && condition to OR to resolve the issue of definition id #943

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
abhiishek26 wants to merge 1 commit into
base: master
Choose a base branch
from
+40 −1
Open

Ranger -5587 modified && condition to OR to resolve the issue of definition id #943

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ public RangerServiceDef updateServiceDef(RangerServiceDef serviceDef, @PathParam
// if serviceDef.id is null, then set param 'id' into serviceDef Object
if (serviceDef.getId() == null) {
serviceDef.setId(id);
} else if (StringUtils.isBlank(serviceDef.getName()) && !serviceDef.getId().equals(id)) {
} else if (StringUtils.isBlank(serviceDef.getName()) || !serviceDef.getId().equals(id)) {
throw restErrorUtil.createRESTException(HttpServletResponse.SC_BAD_REQUEST, "serviceDef Id mismatch", true);
}

Expand Down
39 changes: 39 additions & 0 deletions security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,20 @@ public void test1createServiceDef() throws Exception {
public void test2updateServiceDef() throws Exception {
RangerServiceDef rangerServiceDef = rangerServiceDef();

// Ensure the service def has a name to pass validation
if (StringUtils.isBlank(rangerServiceDef.getName())) {
rangerServiceDef.setName("hdfs");
}

// Mock restErrorUtil to prevent NullPointerException
WebApplicationException mockException = new WebApplicationException(
"serviceDef Id mismatch",
HttpServletResponse.SC_BAD_REQUEST);
Mockito.when(restErrorUtil.createRESTException(
Mockito.anyInt(),
Mockito.anyString(),
Mockito.anyBoolean())).thenReturn(mockException);

Mockito.when(validatorFactory.getServiceDefValidator(svcStore)).thenReturn(serviceDefValidator);
Mockito.when(svcStore.updateServiceDef(Mockito.any())).thenReturn(rangerServiceDef);

Expand Down Expand Up @@ -4505,4 +4519,29 @@ public void testValidateGrantRevokeRequest_InvalidOwnerForNonAdmin() throws Exce
}
});
}

@Test
public void testUpdateServiceDefWithValidNameButMismatchedId() throws Exception {
// Arrange
RangerServiceDef serviceDef = rangerServiceDef();
serviceDef.setName("hdfs"); // Valid name
serviceDef.setId(999L); // Mismatched ID (different from Id = 8L)

WebApplicationException mockException = new WebApplicationException();
Mockito.when(restErrorUtil.createRESTException(
Mockito.eq(HttpServletResponse.SC_BAD_REQUEST),
Mockito.eq("serviceDef Id mismatch"),
Mockito.eq(true)
)).thenReturn(mockException);

// Act & Assert
Assertions.assertThrows(WebApplicationException.class,
() -> serviceREST.updateServiceDef(serviceDef, Id));

// Verify the correct error was created
Mockito.verify(restErrorUtil).createRESTException(
HttpServletResponse.SC_BAD_REQUEST,
"serviceDef Id mismatch",
true);
}
}
Loading