From 046a1bcaa50a3fe41efaacc659985b0c9226e68b Mon Sep 17 00:00:00 2001 From: MorganaFuture Date: Wed, 29 Apr 2026 16:21:45 +0300 Subject: [PATCH] restrict agent tool proxy requests to own device only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit agents with a valid JWT could request data for any device by specifying an arbitrary agentId in the path. added ownership check via tool_connections before proxying — returns 403 if the jwt machine_id doesn't match. --- .../ReactiveToolConnectionRepository.java | 15 ++ openframe-gateway-service-core/pom.xml | 12 ++ .../controller/IntegrationController.java | 14 +- .../service/AgentDeviceAccessValidator.java | 37 +++++ .../gateway/service/RestProxyService.java | 76 ++++++---- .../AgentDeviceAccessValidatorTest.java | 109 ++++++++++++++ .../gateway/service/RestProxyServiceTest.java | 140 ++++++++++++++++++ 7 files changed, 370 insertions(+), 33 deletions(-) create mode 100644 openframe-data-mongo-reactive/src/main/java/com/openframe/data/reactive/repository/tool/ReactiveToolConnectionRepository.java create mode 100644 openframe-gateway-service-core/src/main/java/com/openframe/gateway/service/AgentDeviceAccessValidator.java create mode 100644 openframe-gateway-service-core/src/test/java/com/openframe/gateway/service/AgentDeviceAccessValidatorTest.java create mode 100644 openframe-gateway-service-core/src/test/java/com/openframe/gateway/service/RestProxyServiceTest.java diff --git a/openframe-data-mongo-reactive/src/main/java/com/openframe/data/reactive/repository/tool/ReactiveToolConnectionRepository.java b/openframe-data-mongo-reactive/src/main/java/com/openframe/data/reactive/repository/tool/ReactiveToolConnectionRepository.java new file mode 100644 index 000000000..ca0a24c56 --- /dev/null +++ b/openframe-data-mongo-reactive/src/main/java/com/openframe/data/reactive/repository/tool/ReactiveToolConnectionRepository.java @@ -0,0 +1,15 @@ +package com.openframe.data.reactive.repository.tool; + +import com.openframe.data.document.tool.ToolConnection; +import com.openframe.data.document.tool.ToolType; +import org.springframework.data.mongodb.repository.ReactiveMongoRepository; +import org.springframework.stereotype.Repository; +import reactor.core.publisher.Mono; + +@Repository +public interface ReactiveToolConnectionRepository extends ReactiveMongoRepository { + + Mono findByMachineIdAndToolType(String machineId, ToolType toolType); + + Mono findFirstByAgentToolIdOrderByConnectedAtDesc(String agentToolId); +} diff --git a/openframe-gateway-service-core/pom.xml b/openframe-gateway-service-core/pom.xml index 942bfcaa1..96c246ed7 100644 --- a/openframe-gateway-service-core/pom.xml +++ b/openframe-gateway-service-core/pom.xml @@ -80,6 +80,18 @@ io.micrometer micrometer-core + + + + org.springframework.boot + spring-boot-starter-test + test + + + io.projectreactor + reactor-test + test + diff --git a/openframe-gateway-service-core/src/main/java/com/openframe/gateway/controller/IntegrationController.java b/openframe-gateway-service-core/src/main/java/com/openframe/gateway/controller/IntegrationController.java index 695d06e62..2ee2a611d 100644 --- a/openframe-gateway-service-core/src/main/java/com/openframe/gateway/controller/IntegrationController.java +++ b/openframe-gateway-service-core/src/main/java/com/openframe/gateway/controller/IntegrationController.java @@ -5,6 +5,7 @@ import org.springframework.http.ResponseEntity; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.security.core.Authentication; +import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationToken; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -73,11 +74,20 @@ public Mono> proxyApiRequest( public Mono> proxyAgentRequest( @PathVariable String toolId, ServerHttpRequest request, - @RequestBody(required = false) String body + @RequestBody(required = false) String body, + Authentication authentication ) { String path = request.getPath().toString(); log.debug("Proxying agent request for tool: {}, path: {}", toolId, path); - return restProxyService.proxyAgentRequest(toolId, request, body); + String jwtMachineId = extractMachineId(authentication); + return restProxyService.proxyAgentRequest(toolId, request, body, jwtMachineId); + } + + private String extractMachineId(Authentication authentication) { + if (authentication instanceof JwtAuthenticationToken jwtAuth) { + return jwtAuth.getToken().getClaimAsString("machine_id"); + } + return null; } } diff --git a/openframe-gateway-service-core/src/main/java/com/openframe/gateway/service/AgentDeviceAccessValidator.java b/openframe-gateway-service-core/src/main/java/com/openframe/gateway/service/AgentDeviceAccessValidator.java new file mode 100644 index 000000000..185dfd154 --- /dev/null +++ b/openframe-gateway-service-core/src/main/java/com/openframe/gateway/service/AgentDeviceAccessValidator.java @@ -0,0 +1,37 @@ +package com.openframe.gateway.service; + +import com.openframe.data.reactive.repository.tool.ReactiveToolConnectionRepository; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.stereotype.Service; +import reactor.core.publisher.Mono; + +@Service +@RequiredArgsConstructor +@Slf4j +public class AgentDeviceAccessValidator { + + private final ReactiveToolConnectionRepository toolConnectionRepository; + + /** + * Validates that the JWT machine_id is the owner of the requested agentToolId + * via the tool_connections collection. + */ + public Mono canAccess(String jwtMachineId, String agentToolId) { + if (jwtMachineId == null || agentToolId == null || agentToolId.isBlank()) { + log.warn("Device access denied: missing jwtMachineId={} or agentToolId={}", jwtMachineId, agentToolId); + return Mono.just(false); + } + return toolConnectionRepository + .findFirstByAgentToolIdOrderByConnectedAtDesc(agentToolId) + .map(conn -> { + boolean allowed = jwtMachineId.equals(conn.getMachineId()); + if (!allowed) { + log.warn("Device access denied: jwtMachineId={} tried to access agentToolId={} owned by machineId={}", + jwtMachineId, agentToolId, conn.getMachineId()); + } + return allowed; + }) + .defaultIfEmpty(false); + } +} diff --git a/openframe-gateway-service-core/src/main/java/com/openframe/gateway/service/RestProxyService.java b/openframe-gateway-service-core/src/main/java/com/openframe/gateway/service/RestProxyService.java index d31eb72ea..50946ff95 100644 --- a/openframe-gateway-service-core/src/main/java/com/openframe/gateway/service/RestProxyService.java +++ b/openframe-gateway-service-core/src/main/java/com/openframe/gateway/service/RestProxyService.java @@ -38,6 +38,7 @@ public class RestProxyService { private final ReactiveIntegratedToolRepository toolRepository; private final ToolUpstreamResolverRegistry upstreamRegistry; private final ToolApiKeyHeadersResolver apiKeyHeadersResolver; + private final AgentDeviceAccessValidator agentDeviceAccessValidator; public Mono> proxyApiRequest(String toolId, ServerHttpRequest request, String body) { return toolRepository.findById(toolId) @@ -69,39 +70,52 @@ private Map buildApiRequestHeaders(IntegratedTool tool) { return headers; } - /* - * Currently if one device have valid open-frame machine JWT token, it can send - * API request for other device. - * TODO: implement device access validation after tool connection feature is - * implemented. - * - * Tactical RMM request format: - * - GET /{agentId}/** - * - POST /** - * { - * "agentId": "*", - * } - * } - */ - public Mono> proxyAgentRequest(String toolId, ServerHttpRequest request, String body) { - return toolRepository.findById(toolId) - .flatMap(tool -> { - if (!tool.isEnabled()) { - ResponseEntity response = ResponseEntity.badRequest() - .body("Tool " + tool.getName() + " is not enabled"); - return Mono.just(response); + public Mono> proxyAgentRequest( + String toolId, ServerHttpRequest request, String body, String jwtMachineId) { + String agentToolId = extractAgentToolId(toolId, request).orElse(null); + return agentDeviceAccessValidator.canAccess(jwtMachineId, agentToolId) + .flatMap(allowed -> { + if (!allowed) { + log.warn("Blocked agent request: machineId={} to agentToolId={} via tool={}", + jwtMachineId, agentToolId, toolId); + return Mono.just(ResponseEntity.status(HttpStatus.FORBIDDEN) + .build()); } + return toolRepository.findById(toolId) + .flatMap(tool -> { + if (!tool.isEnabled()) { + ResponseEntity response = ResponseEntity.badRequest() + .body("Tool " + tool.getName() + " is not enabled"); + return Mono.just(response); + } + + URI targetUri = upstreamRegistry.resolve(toolId) + .resolveRest(tool, request, "/tools/agent"); + log.debug("Proxying agent request for tool: {}, url: {}", toolId, targetUri); + + HttpMethod method = request.getMethod(); + Map headers = buildAgentRequestHeaders(request); + + return proxy(tool, targetUri, method, headers, body); + }) + .switchIfEmpty(Mono.just( + ResponseEntity.status(HttpStatus.NOT_FOUND).body("Tool not found: " + toolId))); + }); + } - URI targetUri = upstreamRegistry.resolve(toolId).resolveRest(tool, request, "/tools/agent"); - log.debug("Proxying agent request for tool: {}, url: {}", toolId, targetUri); - - HttpMethod method = request.getMethod(); - Map headers = buildAgentRequestHeaders(request); - - return proxy(tool, targetUri, method, headers, body); - }) - .switchIfEmpty( - Mono.just(ResponseEntity.status(HttpStatus.NOT_FOUND).body("Tool not found: " + toolId))); + private java.util.Optional extractAgentToolId(String toolId, ServerHttpRequest request) { + String path = request.getPath().value(); + String prefix = "/tools/agent/" + toolId + "/"; + if (!path.startsWith(prefix)) { + return java.util.Optional.empty(); + } + String remaining = path.substring(prefix.length()); + if (remaining.isBlank()) { + return java.util.Optional.empty(); + } + int slashIdx = remaining.indexOf('/'); + String agentToolId = slashIdx == -1 ? remaining : remaining.substring(0, slashIdx); + return agentToolId.isBlank() ? java.util.Optional.empty() : java.util.Optional.of(agentToolId); } private Map buildAgentRequestHeaders(ServerHttpRequest request) { diff --git a/openframe-gateway-service-core/src/test/java/com/openframe/gateway/service/AgentDeviceAccessValidatorTest.java b/openframe-gateway-service-core/src/test/java/com/openframe/gateway/service/AgentDeviceAccessValidatorTest.java new file mode 100644 index 000000000..9187b04ef --- /dev/null +++ b/openframe-gateway-service-core/src/test/java/com/openframe/gateway/service/AgentDeviceAccessValidatorTest.java @@ -0,0 +1,109 @@ +package com.openframe.gateway.service; + +import com.openframe.data.document.tool.ToolConnection; +import com.openframe.data.reactive.repository.tool.ReactiveToolConnectionRepository; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; + +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class AgentDeviceAccessValidatorTest { + + @Mock + private ReactiveToolConnectionRepository toolConnectionRepository; + + private AgentDeviceAccessValidator validator; + + private static final String MACHINE_ID = "machine-abc123"; + private static final String AGENT_TOOL_ID = "tactical-agent-xyz"; + private static final String OTHER_MACHINE_ID = "machine-other999"; + + @BeforeEach + void setUp() { + validator = new AgentDeviceAccessValidator(toolConnectionRepository); + } + + @Nested + @DisplayName("canAccess — happy path") + class AllowedAccess { + + @Test + @DisplayName("returns true when JWT machineId matches ToolConnection owner") + void allowsWhenMachineIdMatches() { + ToolConnection conn = toolConnection(MACHINE_ID, AGENT_TOOL_ID); + when(toolConnectionRepository.findFirstByAgentToolIdOrderByConnectedAtDesc(AGENT_TOOL_ID)) + .thenReturn(Mono.just(conn)); + + StepVerifier.create(validator.canAccess(MACHINE_ID, AGENT_TOOL_ID)) + .expectNext(true) + .verifyComplete(); + } + } + + @Nested + @DisplayName("canAccess — denied") + class DeniedAccess { + + @Test + @DisplayName("returns false when JWT machineId does NOT match ToolConnection owner — cross-device attack") + void deniesWhenMachineIdMismatch() { + ToolConnection conn = toolConnection(OTHER_MACHINE_ID, AGENT_TOOL_ID); + when(toolConnectionRepository.findFirstByAgentToolIdOrderByConnectedAtDesc(AGENT_TOOL_ID)) + .thenReturn(Mono.just(conn)); + + StepVerifier.create(validator.canAccess(MACHINE_ID, AGENT_TOOL_ID)) + .expectNext(false) + .verifyComplete(); + } + + @Test + @DisplayName("returns false when no ToolConnection exists for agentToolId") + void deniesWhenNoConnectionFound() { + when(toolConnectionRepository.findFirstByAgentToolIdOrderByConnectedAtDesc(AGENT_TOOL_ID)) + .thenReturn(Mono.empty()); + + StepVerifier.create(validator.canAccess(MACHINE_ID, AGENT_TOOL_ID)) + .expectNext(false) + .verifyComplete(); + } + + @Test + @DisplayName("returns false when jwtMachineId is null — no machine_id claim in JWT") + void deniesWhenJwtMachineIdIsNull() { + StepVerifier.create(validator.canAccess(null, AGENT_TOOL_ID)) + .expectNext(false) + .verifyComplete(); + } + + @Test + @DisplayName("returns false when agentToolId is null — no target in path") + void deniesWhenAgentToolIdIsNull() { + StepVerifier.create(validator.canAccess(MACHINE_ID, null)) + .expectNext(false) + .verifyComplete(); + } + + @Test + @DisplayName("returns false when agentToolId is blank") + void deniesWhenAgentToolIdIsBlank() { + StepVerifier.create(validator.canAccess(MACHINE_ID, " ")) + .expectNext(false) + .verifyComplete(); + } + } + + private ToolConnection toolConnection(String machineId, String agentToolId) { + ToolConnection conn = new ToolConnection(); + conn.setMachineId(machineId); + conn.setAgentToolId(agentToolId); + return conn; + } +} diff --git a/openframe-gateway-service-core/src/test/java/com/openframe/gateway/service/RestProxyServiceTest.java b/openframe-gateway-service-core/src/test/java/com/openframe/gateway/service/RestProxyServiceTest.java new file mode 100644 index 000000000..c8230f410 --- /dev/null +++ b/openframe-gateway-service-core/src/test/java/com/openframe/gateway/service/RestProxyServiceTest.java @@ -0,0 +1,140 @@ +package com.openframe.gateway.service; + +import com.openframe.data.reactive.repository.tool.ReactiveIntegratedToolRepository; +import com.openframe.gateway.upstream.ToolUpstreamResolverRegistry; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.http.server.reactive.ServerHttpRequest; +import org.springframework.web.util.pattern.PathPatternParser; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.*; + +@ExtendWith(MockitoExtension.class) +class RestProxyServiceTest { + + @Mock + private ReactiveIntegratedToolRepository toolRepository; + @Mock + private ToolUpstreamResolverRegistry upstreamRegistry; + @Mock + private ToolApiKeyHeadersResolver apiKeyHeadersResolver; + @Mock + private AgentDeviceAccessValidator agentDeviceAccessValidator; + @Mock + private ServerHttpRequest request; + + private RestProxyService service; + + private static final String TOOL_ID = "tactical-rmm"; + private static final String MACHINE_ID = "machine-abc123"; + private static final String AGENT_TOOL_ID = "tactical-agent-xyz"; + + @BeforeEach + void setUp() { + service = new RestProxyService(toolRepository, upstreamRegistry, apiKeyHeadersResolver, agentDeviceAccessValidator); + } + + @Nested + @DisplayName("proxyAgentRequest — device access validation") + class DeviceAccessValidation { + + @Test + @DisplayName("returns 403 Forbidden when validator denies access — cross-device attack blocked") + void returns403WhenAccessDenied() { + mockRequestPath("/tools/agent/" + TOOL_ID + "/" + AGENT_TOOL_ID + "/api/v3/agents/"); + when(agentDeviceAccessValidator.canAccess(MACHINE_ID, AGENT_TOOL_ID)) + .thenReturn(Mono.just(false)); + + Mono> result = service.proxyAgentRequest(TOOL_ID, request, null, MACHINE_ID); + + StepVerifier.create(result) + .assertNext(response -> assertThat(response.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN)) + .verifyComplete(); + + verify(toolRepository, never()).findById(anyString()); + } + + @Test + @DisplayName("returns 403 when path has no agentToolId segment (validator receives null)") + void returns403WhenNoAgentToolIdInPath() { + mockRequestPath("/tools/agent/" + TOOL_ID); + when(agentDeviceAccessValidator.canAccess(eq(MACHINE_ID), eq(null))) + .thenReturn(Mono.just(false)); + + StepVerifier.create(service.proxyAgentRequest(TOOL_ID, request, null, MACHINE_ID)) + .assertNext(response -> assertThat(response.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN)) + .verifyComplete(); + } + + @Test + @DisplayName("returns 403 when jwtMachineId is null — no machine_id in JWT") + void returns403WhenJwtMachineIdIsNull() { + mockRequestPath("/tools/agent/" + TOOL_ID + "/" + AGENT_TOOL_ID + "/api/v3/agents/"); + when(agentDeviceAccessValidator.canAccess(null, AGENT_TOOL_ID)) + .thenReturn(Mono.just(false)); + + StepVerifier.create(service.proxyAgentRequest(TOOL_ID, request, null, null)) + .assertNext(response -> assertThat(response.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN)) + .verifyComplete(); + } + + @Test + @DisplayName("returns 404 when validator passes but tool is not found") + void returns404WhenToolNotFound() { + mockRequestPath("/tools/agent/" + TOOL_ID + "/" + AGENT_TOOL_ID + "/api/v3/agents/"); + when(agentDeviceAccessValidator.canAccess(MACHINE_ID, AGENT_TOOL_ID)) + .thenReturn(Mono.just(true)); + when(toolRepository.findById(TOOL_ID)).thenReturn(Mono.empty()); + + StepVerifier.create(service.proxyAgentRequest(TOOL_ID, request, null, MACHINE_ID)) + .assertNext(response -> assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND)) + .verifyComplete(); + } + } + + @Nested + @DisplayName("extractAgentToolId — path parsing") + class AgentToolIdExtraction { + + @Test + @DisplayName("extracts agentToolId from deep path correctly") + void extractsFromDeepPath() { + mockRequestPath("/tools/agent/" + TOOL_ID + "/" + AGENT_TOOL_ID + "/api/v3/agents/"); + when(agentDeviceAccessValidator.canAccess(any(), any())).thenReturn(Mono.just(false)); + + service.proxyAgentRequest(TOOL_ID, request, null, MACHINE_ID).block(); + + verify(agentDeviceAccessValidator).canAccess(MACHINE_ID, AGENT_TOOL_ID); + } + + @Test + @DisplayName("extracts agentToolId when it is the last path segment") + void extractsWhenLastSegment() { + mockRequestPath("/tools/agent/" + TOOL_ID + "/" + AGENT_TOOL_ID); + when(agentDeviceAccessValidator.canAccess(any(), any())).thenReturn(Mono.just(false)); + + service.proxyAgentRequest(TOOL_ID, request, null, MACHINE_ID).block(); + + verify(agentDeviceAccessValidator).canAccess(MACHINE_ID, AGENT_TOOL_ID); + } + } + + private void mockRequestPath(String path) { + org.springframework.http.server.RequestPath requestPath = + org.springframework.http.server.RequestPath.parse(path, null); + when(request.getPath()).thenReturn(requestPath); + } +}