This is an automated email from the ASF dual-hosted git repository.
dhavalshah9131 pushed a commit to branch ds_RANGER-5861_ran210
in repository https://gitbox.apache.org/repos/asf/ranger.git
commit 245ac61228076d7e1323a3b37adaf89aef14632f
Author: Dhaval Shah <dhavalshah9131@@gmail.com>
AuthorDate: Fri Oct 30 15:51:16 2026 +0530
RANGER-5861 : Update on service type should not be allowed.
---
.../ranger/plugin/errors/ValidationErrorCode.java | 1 +
.../model/validation/RangerServiceValidator.java | 14 +++++++++
.../validation/TestRangerServiceValidator.java | 34 ++++++++++++++++++++
.../java/org/apache/ranger/biz/ServiceDBStore.java | 7 +++++
.../java/org/apache/ranger/rest/PublicAPIsv2.java | 2 +-
.../java/org/apache/ranger/rest/ServiceREST.java | 36 +++++++++++++++++++---
.../org/apache/ranger/rest/TestPublicAPIsv2.java | 4 +--
.../org/apache/ranger/rest/TestServiceREST.java | 30 +++++++++++++++---
8 files changed, 116 insertions(+), 12 deletions(-)
diff --git
a/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java
b/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java
index 5844e23c5..73fccb0ce 100644
---
a/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java
+++
b/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java
@@ -41,6 +41,7 @@ public enum ValidationErrorCode {
SERVICE_VALIDATION_ERR_MISSING_SERVICE_DEF(1009, "Missing service def"),
SERVICE_VALIDATION_ERR_INVALID_SERVICE_DEF(1010, "Service def not found:
service-def-name=[{0}]"),
SERVICE_VALIDATION_ERR_REQUIRED_PARM_MISSING(1011, "Missing required
configuration parameter(s): missing parameters={0}"),
+ SERVICE_VALIDATION_ERR_SERVICE_TYPE_NOT_UPDATABLE(1012, "Service type
cannot be changed on update: existing type=[{0}], requested type=[{1}]"),
// SERVICE-DEF VALIDATION
SERVICE_DEF_VALIDATION_ERR_UNSUPPORTED_ACTION(2001, "Internal error:
unsupported action[{0}]; isValid(Long) is only supported for DELETE"),
diff --git
a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceValidator.java
b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceValidator.java
index 5c087bdff..bcc9cef48 100644
---
a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceValidator.java
+++
b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceValidator.java
@@ -249,6 +249,20 @@ boolean isValid(RangerService service, Action action,
List<ValidationFailureDeta
.build());
valid = false;
}
+
+ if (serviceDef != null && action ==
Action.UPDATE && valid) {
+ RangerService existingForType =
getService(id);
+ if (existingForType != null &&
!StringUtils.equals(existingForType.getType(), type)) {
+ ValidationErrorCode error =
ValidationErrorCode.SERVICE_VALIDATION_ERR_SERVICE_TYPE_NOT_UPDATABLE;
+ failures.add(new
ValidationFailureDetailsBuilder()
+ .field("type")
+
.isSemanticallyIncorrect()
+
.errorCode(error.getErrorCode())
+
.becauseOf(error.getMessage(existingForType.getType(), type))
+ .build());
+ valid = false;
+ }
+ }
}
if (nameSpecified && serviceDef != null) {
// check if required parameters were specified
diff --git
a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceValidator.java
b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceValidator.java
index bb9968b4f..9f1061226 100644
---
a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceValidator.java
+++
b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceValidator.java
@@ -535,6 +535,7 @@ public void test_isValid_happyPath() throws Exception {
when(service.getId()).thenReturn(7L);
RangerService existingService = mock(RangerService.class);
when(existingService.getId()).thenReturn(Long.valueOf(7L));
+ when(existingService.getType()).thenReturn("aType");
when(_store.getService(7L)).thenReturn(existingService);
when(_store.getServiceByName("aName")).thenReturn(existingService);
Assert.assertTrue(_validator.isValid(service, Action.UPDATE,
_failures));
@@ -543,6 +544,39 @@ public void test_isValid_happyPath() throws Exception {
Assert.assertTrue(_validator.isValid(service, Action.UPDATE,
_failures));
}
+ @Test
+ public void test_isValid_update_serviceTypeNotUpdatable() throws
Exception {
+ Object[][] serviceDefInput = new Object[][] {
+ { "param1", true },
+ { "param2", true },
+ { "param3", false },
+ { "param4", false },
+ { "param5", true },
+ };
+ List<RangerServiceConfigDef> configDefs =
_utils.createServiceConditionDefs(serviceDefInput);
+ RangerServiceDef serviceDef = mock(RangerServiceDef.class);
+ when(serviceDef.getConfigs()).thenReturn(configDefs);
+
+ RangerService service = mock(RangerService.class);
+ when(service.getId()).thenReturn(7L);
+ when(service.getName()).thenReturn("aName");
+ when(service.getDisplayName()).thenReturn("aDisplayName");
+ when(service.getType()).thenReturn("aType");
+ String[] configs = new String[] { "param1", "param2", "param3",
"param5", "param6" };
+ Map<String, String> configMap = _utils.createMap(configs);
+ when(service.getConfigs()).thenReturn(configMap);
+
+
when(_store.getServiceDefByName("aType")).thenReturn(serviceDef);
+ when(_store.getServiceByName("aName")).thenReturn(null);
+
+ RangerService existingService = mock(RangerService.class);
+ when(existingService.getType()).thenReturn("storedType");
+ when(_store.getService(7L)).thenReturn(existingService);
+
+ checkFailure_isValid(_validator, service, Action.UPDATE,
_failures, "semantic", "type");
+
Assert.assertEquals(ValidationErrorCode.SERVICE_VALIDATION_ERR_SERVICE_TYPE_NOT_UPDATABLE.getErrorCode(),
_failures.get(0)._errorCode);
+ }
+
@Test
public void test_isValid_withId_errorConditions() throws Exception {
// api that takes in long is only supported for delete currently
diff --git
a/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
b/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
index d5a0f1f77..764f2cb51 100644
--- a/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
+++ b/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
@@ -1655,6 +1655,13 @@ public RangerService updateService(RangerService
service, Map<String, Object> op
RangerService existing =
svcService.getPopulatedViewObject(xExisting);
+ if (StringUtils.isNotBlank(service.getType()) &&
!StringUtils.equals(existing.getType(), service.getType())) {
+ throw restErrorUtil.createRESTException(
+ "Service type cannot be changed on
update. Existing type=[" + existing.getType() + "], requested type=[" +
service.getType() + "]",
+ MessageEnums.DATA_NOT_UPDATABLE);
+ }
+ service.setType(existing.getType());
+
String existingName = existing.getName();
boolean renamed =
!StringUtils.equalsIgnoreCase(service.getName(), existingName);
diff --git
a/security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java
b/security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java
index 924449650..ff1faa9d3 100644
--- a/security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java
+++ b/security-admin/src/main/java/org/apache/ranger/rest/PublicAPIsv2.java
@@ -469,7 +469,7 @@ public RangerService updateService(RangerService service,
@PathParam("id") Long
throw
restErrorUtil.createRESTException(HttpServletResponse.SC_BAD_REQUEST , "service
id mismatch", true);
}
- return serviceREST.updateService(service, request);
+ return serviceREST.updateService(service, id, request);
}
@PUT
diff --git
a/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
b/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
index 27615fb31..141225fd2 100644
--- a/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
+++ b/security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
@@ -792,12 +792,16 @@ public RangerService createService(RangerService service)
{
return ret;
}
+ public RangerService updateService(RangerService service,
HttpServletRequest request) {
+ return updateService(service, null, request);
+ }
+
@PUT
@Path("/services/{id}")
@Consumes({ "application/json" })
@Produces({ "application/json" })
@PreAuthorize("@rangerPreAuthSecurityHandler.isAPIAccessible(\"" +
RangerAPIList.UPDATE_SERVICE + "\")")
- public RangerService updateService(RangerService service,
+ public RangerService updateService(RangerService service,
@PathParam("id") Long reqServiceId,
@Context HttpServletRequest request) {
if(LOG.isDebugEnabled()) {
LOG.debug("==> ServiceREST.updateService(): " +
service);
@@ -812,6 +816,14 @@ public RangerService updateService(RangerService service,
perf = RangerPerfTracer.getPerfTracer(PERF_LOG,
"ServiceREST.updateService(serviceName=" + service.getName() + ")");
}
+ if (reqServiceId != null) {
+ if (service.getId() == null) {
+ service.setId(reqServiceId);
+ } else if
(!service.getId().equals(reqServiceId)) {
+ throw
restErrorUtil.createRESTException(HttpServletResponse.SC_BAD_REQUEST, "service
id mismatch", true);
+ }
+ }
+
/**
* If display name is blank (EMPTY String or NULL), use
previous display name.
*/
@@ -826,9 +838,6 @@ public RangerService updateService(RangerService service,
}
}
- RangerServiceValidator validator =
validatorFactory.getServiceValidator(svcStore);
- validator.validate(service, Action.UPDATE);
-
if(!StringUtils.isEmpty(service.getName().trim())){
service.setName(service.getName().trim());
}
@@ -841,10 +850,27 @@ public RangerService updateService(RangerService service,
// TODO: As of now we are allowing SYS_ADMIN to create
all the
// services including KMS
+ RangerService existingService =
svcStore.getService(service.getId());
+ String storedType =
existingService.getType();
+
+ if (StringUtils.isNotBlank(service.getType()) &&
!StringUtils.equals(storedType, service.getType())) {
+ throw
restErrorUtil.createRESTException("Service type cannot be changed on update",
MessageEnums.DATA_NOT_UPDATABLE);
+ }
+
+ service.setType(storedType);
+
+ XXServiceDef xxServiceDef =
daoManager.getXXServiceDef().findByName(storedType);
+
+ if (xxServiceDef == null) {
+ throw
restErrorUtil.createRESTException("Service def not found: service-def-name=" +
storedType, MessageEnums.INVALID_INPUT_DATA);
+ }
- XXServiceDef xxServiceDef =
daoManager.getXXServiceDef().findByName(service.getType());
bizUtil.hasKMSPermissions("Service",
xxServiceDef.getImplclassname());
bizUtil.blockAuditorRoleUser();
+
+ RangerServiceValidator validator =
validatorFactory.getServiceValidator(svcStore);
+ validator.validate(service, Action.UPDATE);
+
Map<String, Object> options = getOptions(request);
ret = svcStore.updateService(service, options);
diff --git
a/security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java
b/security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java
index 0fba41ee7..645d1fca5 100644
--- a/security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java
+++ b/security-admin/src/test/java/org/apache/ranger/rest/TestPublicAPIsv2.java
@@ -410,7 +410,7 @@ public void test12createService() throws Exception {
public void test13updateService() throws Exception {
RangerService rangerService = rangerService();
HttpServletRequest request = null;
- Mockito.when(serviceREST.updateService(rangerService,
request)).thenReturn(rangerService);
+ Mockito.when(serviceREST.updateService(rangerService, Id,
request)).thenReturn(rangerService);
RangerService dbRangerService =
publicAPIsv2.updateService(rangerService, Id, request);
Assert.assertNotNull(dbRangerService);
Assert.assertEquals(dbRangerService, rangerService);
@@ -418,7 +418,7 @@ public void test13updateService() throws Exception {
rangerService.getId());
Assert.assertEquals(dbRangerService.getName(),
rangerService.getName());
- Mockito.verify(serviceREST).updateService(rangerService,
request);
+ Mockito.verify(serviceREST).updateService(rangerService, Id,
request);
}
@Test
diff --git
a/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java
b/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java
index fc4043071..ad38465c6 100644
--- a/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java
+++ b/security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java
@@ -360,6 +360,14 @@ public XXServiceDef serviceDef() {
return xServiceDef;
}
+ private void stubStoredServiceForUpdate(RangerService existingService,
XXServiceDef xServiceDef) throws Exception {
+ XXServiceDefDao xServiceDefDao =
Mockito.mock(XXServiceDefDao.class);
+
+
Mockito.when(daoManager.getXXServiceDef()).thenReturn(xServiceDefDao);
+
Mockito.when(svcStore.getService(existingService.getId())).thenReturn(existingService);
+
Mockito.when(xServiceDefDao.findByName(existingService.getType())).thenReturn(xServiceDef);
+ }
+
public XXService xService() {
XXService xService = new XXService();
xService.setAddedByUserId(Id);
@@ -683,13 +691,10 @@ public void test8updateServiceDef() throws Exception {
HttpServletRequest request = null;
Map<String, Object> options = null;
- XXServiceDefDao xServiceDefDao =
Mockito.mock(XXServiceDefDao.class);
Mockito.when(validatorFactory.getServiceValidator(svcStore))
.thenReturn(serviceValidator);
-
Mockito.when(daoManager.getXXServiceDef()).thenReturn(xServiceDefDao);
- Mockito.when(xServiceDefDao.findByName(rangerService.getType()))
- .thenReturn(xServiceDef);
+ stubStoredServiceForUpdate(rangerService, xServiceDef);
Mockito.when(
svcStore.updateService((RangerService)
Mockito.any(), (Map<String, Object>) Mockito.any()))
@@ -2956,4 +2961,21 @@ public void
test156GrantBlockedWhenUnauthenticatedAndFlagFalse() throws Exceptio
Mockito.when(restErrorUtil.createRESTException(Mockito.anyString())).thenReturn(new
WebApplicationException());
Assert.assertThrows(WebApplicationException.class, () ->
serviceREST.grantAccess("HDFS_1", grantRequest, request));
}
+
+ @Test
+ public void test157UpdateServiceRejectsTypeChange() throws Exception {
+ RangerService service = rangerService();
+ service.setType("kms");
+ HttpServletRequest request =
Mockito.mock(HttpServletRequest.class);
+ RangerService storedService = rangerService();
+
+
Mockito.when(svcStore.getService(storedService.getId())).thenReturn(storedService);
+ Mockito.when(restErrorUtil.createRESTException("Service type
cannot be changed on update", MessageEnums.DATA_NOT_UPDATABLE))
+ .thenThrow(new WebApplicationException());
+
+ Assert.assertThrows(WebApplicationException.class, () ->
serviceREST.updateService(service, request));
+
+ Mockito.verify(restErrorUtil).createRESTException("Service type
cannot be changed on update", MessageEnums.DATA_NOT_UPDATABLE);
+ Mockito.verify(svcStore,
Mockito.never()).updateService(Mockito.any(), Mockito.any());
+ }
}
