This is an automated email from the ASF dual-hosted git repository. liyang-kylin pushed a commit to branch kylin5 in repository https://gitbox.apache.org/repos/asf/kylin.git
commit 854a2216e05a0caa6c8ee7ce27db440c9e75afd9 Author: Li Yang <[email protected]> AuthorDate: Mon Oct 5 18:51:03 2026 +0800 KYLIN-6119 require admin or service token on maintenance operations Several system maintenance handlers were reachable without an authorization check: roll_event_log and diag/progress were public in the security chain, while do_cleanup_garbage and metadata/reload only required an authenticated caller. Any authenticated low-privilege user could therefore trigger heavy rollup, garbage-collection, or metadata reload work, or forge diagnostic progress. Gate the handlers behind the global-admin-or-service-token check already used for the other internal RPC endpoints, and have the internal info extractor attach the deployment-scoped token so diagnostics keep reporting without extra configuration. The unit-of-work simulation helper, which writes metadata, is gated as well for consistency. --- .../kylin/rest/controller/NSystemController.java | 11 +++++--- .../rest/controller/NSystemControllerTest.java | 30 +++++++++++++++++++--- .../apache/kylin/tool/restclient/RestClient.java | 9 +++++++ .../kylin/rest/controller/OpsController.java | 5 +++- .../kylin/rest/controller/OpsControllerTest.java | 22 ++++++++++++++-- 5 files changed, 68 insertions(+), 9 deletions(-) diff --git a/src/common-server/src/main/java/org/apache/kylin/rest/controller/NSystemController.java b/src/common-server/src/main/java/org/apache/kylin/rest/controller/NSystemController.java index 6616c611af..bdd2878f26 100644 --- a/src/common-server/src/main/java/org/apache/kylin/rest/controller/NSystemController.java +++ b/src/common-server/src/main/java/org/apache/kylin/rest/controller/NSystemController.java @@ -102,7 +102,8 @@ public class NSystemController extends NBasicController { @PutMapping(value = "/roll_event_log") @ResponseBody - public EnvelopeResponse<String> rollEventLog() { + public EnvelopeResponse<String> rollEventLog(HttpServletRequest servletRequest) { + InternalRpcSecurity.requireGlobalAdminOrServiceToken(servletRequest); if (ToolUtil.waitForSparderRollUp()) { return new EnvelopeResponse<>(CODE_SUCCESS, "", ""); } @@ -119,7 +120,8 @@ public class NSystemController extends NBasicController { @ApiOperation(value = "reload metadata", tags = { "MID" }) @PostMapping(value = "/metadata/reload") @ResponseBody - public EnvelopeResponse<String> reloadMetadata() throws IOException { + public EnvelopeResponse<String> reloadMetadata(HttpServletRequest servletRequest) throws IOException { + InternalRpcSecurity.requireGlobalAdminOrServiceToken(servletRequest); systemService.reloadMetadata(); return new EnvelopeResponse<>(CODE_SUCCESS, "", ""); } @@ -146,6 +148,7 @@ public class NSystemController extends NBasicController { @PostMapping(value = "/do_cleanup_garbage") @ResponseBody public EnvelopeResponse<String> doCleanupGarbage(final HttpServletRequest request) throws Exception { + InternalRpcSecurity.requireGlobalAdminOrServiceToken(request); scheduleService.routineTask(); return new EnvelopeResponse<>(CODE_SUCCESS, "", ""); } @@ -154,7 +157,9 @@ public class NSystemController extends NBasicController { @ResponseBody public EnvelopeResponse<String> simulateInsertMeta( @RequestParam(value = "count", required = false, defaultValue = "5") int count, - @RequestParam(value = "sleepSec", required = false, defaultValue = "20") long sleepSec) { + @RequestParam(value = "sleepSec", required = false, defaultValue = "20") long sleepSec, + HttpServletRequest servletRequest) { + InternalRpcSecurity.requireGlobalAdminOrServiceToken(servletRequest); if (KylinConfig.getInstanceFromEnv().isUnitOfWorkSimulationEnabled()) { val projectList = IntStream.range(0, 5).mapToObj(i -> "simulation" + i).collect(Collectors.toList()); diff --git a/src/common-server/src/test/java/org/apache/kylin/rest/controller/NSystemControllerTest.java b/src/common-server/src/test/java/org/apache/kylin/rest/controller/NSystemControllerTest.java index 9aec46f801..daa3da15f7 100644 --- a/src/common-server/src/test/java/org/apache/kylin/rest/controller/NSystemControllerTest.java +++ b/src/common-server/src/test/java/org/apache/kylin/rest/controller/NSystemControllerTest.java @@ -21,6 +21,7 @@ import static org.apache.kylin.common.constant.HttpConstant.HTTP_VND_APACHE_KYLI import org.apache.kylin.common.KylinConfigBase; import org.apache.kylin.common.exception.KylinException; +import org.apache.kylin.common.persistence.transaction.BroadcastEventReadyNotifier; import org.apache.kylin.common.util.JsonUtil; import org.apache.kylin.common.util.NLocalFileMetadataTestCase; import org.apache.kylin.junit.rule.TransactionExceptedException; @@ -78,7 +79,30 @@ public class NSystemControllerTest extends NLocalFileMetadataTestCase { public void testRollEventLog() throws Exception { mockMvc.perform(MockMvcRequestBuilders.put("/api/system/roll_event_log").contentType(MediaType.APPLICATION_JSON) .accept(MediaType.parseMediaType(APPLICATION_JSON))).andExpect(MockMvcResultMatchers.status().isOk()); - Mockito.verify(nSystemController).rollEventLog(); + Mockito.verify(nSystemController).rollEventLog(Mockito.any()); + } + + @Test + public void testMaintenanceOperationsRejectAnonymous() { + SecurityContextHolder.clearContext(); + Assert.assertThrows(KylinException.class, + () -> nSystemController.rollEventLog(new MockHttpServletRequest())); + Assert.assertThrows(KylinException.class, + () -> nSystemController.reloadMetadata(new MockHttpServletRequest())); + Assert.assertThrows(KylinException.class, + () -> nSystemController.doCleanupGarbage(new MockHttpServletRequest())); + Assert.assertThrows(KylinException.class, + () -> nSystemController.simulateInsertMeta(1, 1, new MockHttpServletRequest())); + } + + @Test + public void testMaintenanceOperationAcceptsServiceToken() throws Exception { + SecurityContextHolder.clearContext(); + mockMvc.perform(MockMvcRequestBuilders.post("/api/system/transaction/simulation/insert_meta") + .header(BroadcastEventReadyNotifier.BROADCAST_TOKEN_HEADER, getTestConfig().getBroadcastToken()) + .contentType(MediaType.APPLICATION_JSON).param("count", "10").param("sleepSec", "100") + .accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_JSON))) + .andExpect(MockMvcResultMatchers.status().isOk()); } @Test @@ -95,7 +119,7 @@ public class NSystemControllerTest extends NLocalFileMetadataTestCase { .contentType(MediaType.APPLICATION_JSON).param("count", "10").param("sleepSec", "1") .accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_JSON))) .andExpect(MockMvcResultMatchers.status().is5xxServerError()); - Mockito.verify(nSystemController).simulateInsertMeta(Mockito.anyInt(), Mockito.anyLong()); + Mockito.verify(nSystemController).simulateInsertMeta(Mockito.anyInt(), Mockito.anyLong(), Mockito.any()); } @Test @@ -104,7 +128,7 @@ public class NSystemControllerTest extends NLocalFileMetadataTestCase { .contentType(MediaType.APPLICATION_JSON).param("count", "10").param("sleepSec", "100") .accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_JSON))) .andExpect(MockMvcResultMatchers.status().isOk()); - Mockito.verify(nSystemController).simulateInsertMeta(Mockito.anyInt(), Mockito.anyLong()); + Mockito.verify(nSystemController).simulateInsertMeta(Mockito.anyInt(), Mockito.anyLong(), Mockito.any()); } @Test diff --git a/src/core-common/src/main/java/org/apache/kylin/tool/restclient/RestClient.java b/src/core-common/src/main/java/org/apache/kylin/tool/restclient/RestClient.java index c17308b172..fc7faec927 100644 --- a/src/core-common/src/main/java/org/apache/kylin/tool/restclient/RestClient.java +++ b/src/core-common/src/main/java/org/apache/kylin/tool/restclient/RestClient.java @@ -291,6 +291,13 @@ public class RestClient { return response; } + private void addServiceToken(HttpRequestBase method) { + String broadcastToken = KylinConfig.getInstanceFromEnv().getBroadcastToken(); + if (StringUtils.isNotBlank(broadcastToken)) { + method.addHeader(BroadcastEventReadyNotifier.BROADCAST_TOKEN_HEADER, broadcastToken); + } + } + private void addHttpHeaders(HttpRequestBase method) { method.addHeader("Accept", "application/json, text/plain, */*"); method.addHeader("Content-Type", "application/json"); @@ -380,6 +387,7 @@ public class RestClient { public boolean updateDiagProgress(String diagId, String stage, float progress, long updateTime) { String url = baseUrl + "/system/diag/progress"; HttpPut put = newPut(url); + addServiceToken(put); HttpResponse response = null; try { HashMap<String, Object> paraMap = Maps.newHashMap(); @@ -406,6 +414,7 @@ public class RestClient { public boolean rollUpEventLog() { String url = baseUrl + "/system/roll_event_log"; HttpPut put = newPut(url); + addServiceToken(put); HttpResponse response = null; try { response = client.execute(put); diff --git a/src/ops-server/src/main/java/org/apache/kylin/rest/controller/OpsController.java b/src/ops-server/src/main/java/org/apache/kylin/rest/controller/OpsController.java index c7b1fb9207..2d50d481f0 100644 --- a/src/ops-server/src/main/java/org/apache/kylin/rest/controller/OpsController.java +++ b/src/ops-server/src/main/java/org/apache/kylin/rest/controller/OpsController.java @@ -53,6 +53,7 @@ import org.apache.kylin.rest.response.MaintenanceModeResponse; import org.apache.kylin.rest.response.ServerExtInfoResponse; import org.apache.kylin.rest.response.ServerInfoResponse; import org.apache.kylin.rest.response.ServersResponse; +import org.apache.kylin.rest.security.InternalRpcSecurity; import org.apache.kylin.rest.service.MetadataBackupService; import org.apache.kylin.rest.service.OpsService; import org.apache.kylin.rest.service.ScheduleService; @@ -177,7 +178,9 @@ public class OpsController extends NBasicController { @ApiOperation(value = "diagProgress", tags = { "SM" }) @PutMapping(value = "/diag/progress") @ResponseBody - public EnvelopeResponse<String> updateDiagProgress(@RequestBody DiagProgressRequest diagProgressRequest) { + public EnvelopeResponse<String> updateDiagProgress(@RequestBody DiagProgressRequest diagProgressRequest, + HttpServletRequest servletRequest) { + InternalRpcSecurity.requireGlobalAdminOrServiceToken(servletRequest); systemService.updateDiagProgress(diagProgressRequest); return new EnvelopeResponse<>(CODE_SUCCESS, "", ""); } diff --git a/src/ops-server/src/test/java/org/apache/kylin/rest/controller/OpsControllerTest.java b/src/ops-server/src/test/java/org/apache/kylin/rest/controller/OpsControllerTest.java index aa40070387..cc30c6508e 100644 --- a/src/ops-server/src/test/java/org/apache/kylin/rest/controller/OpsControllerTest.java +++ b/src/ops-server/src/test/java/org/apache/kylin/rest/controller/OpsControllerTest.java @@ -25,6 +25,8 @@ import java.time.ZoneId; import java.time.temporal.ChronoUnit; import java.util.Random; +import org.apache.kylin.common.exception.KylinException; +import org.apache.kylin.common.persistence.transaction.BroadcastEventReadyNotifier; import org.apache.kylin.common.util.AddressUtil; import org.apache.kylin.common.util.JsonUtil; import org.apache.kylin.common.util.NLocalFileMetadataTestCase; @@ -45,6 +47,7 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import org.springframework.http.MediaType; +import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.authentication.TestingAuthenticationToken; import org.springframework.security.core.context.SecurityContextHolder; @@ -205,11 +208,26 @@ public class OpsControllerTest extends NLocalFileMetadataTestCase { @Test public void testUpdateDiagProgress() throws Exception { DiagProgressRequest request = new DiagProgressRequest(); - Mockito.doAnswer(x -> null).when(opsController).updateDiagProgress(Mockito.any()); + Mockito.doAnswer(x -> null).when(opsController).updateDiagProgress(Mockito.any(), Mockito.any()); mockMvc.perform(MockMvcRequestBuilders.put("/api/system/diag/progress").contentType(MediaType.APPLICATION_JSON) .accept(MediaType.parseMediaType(HTTP_VND_APACHE_KYLIN_JSON)) .content(JsonUtil.writeValueAsString(request))).andExpect(MockMvcResultMatchers.status().isOk()); - Mockito.verify(opsController).updateDiagProgress(Mockito.any()); + Mockito.verify(opsController).updateDiagProgress(Mockito.any(), Mockito.any()); + } + + @Test + public void testUpdateDiagProgressRejectsAnonymous() { + SecurityContextHolder.clearContext(); + Assert.assertThrows(KylinException.class, + () -> opsController.updateDiagProgress(new DiagProgressRequest(), new MockHttpServletRequest())); + } + + @Test + public void testUpdateDiagProgressAcceptsServiceToken() { + SecurityContextHolder.clearContext(); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addHeader(BroadcastEventReadyNotifier.BROADCAST_TOKEN_HEADER, getTestConfig().getBroadcastToken()); + opsController.updateDiagProgress(new DiagProgressRequest(), request); } @Test
