diff --git a/pom.xml b/pom.xml index 934c266..3917f8f 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ org.springframework.boot spring-boot-starter-parent - 3.5.11 + 3.5.13 it.cnr.isti diff --git a/src/main/java/it/cnr/isti/workflow/manager/HumainFlowApplication.java b/src/main/java/it/cnr/isti/workflow/manager/HumainFlowApplication.java index fde3b3c..c6e52b1 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/HumainFlowApplication.java +++ b/src/main/java/it/cnr/isti/workflow/manager/HumainFlowApplication.java @@ -8,11 +8,13 @@ import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.context.annotation.Bean; +import org.springframework.scheduling.annotation.EnableScheduling; import it.cnr.isti.workflow.manager.auth.services.UserImportComponent; import it.cnr.isti.workflow.manager.flows.FlowImportComponent; @SpringBootApplication +@EnableScheduling public class HumainFlowApplication { static { diff --git a/src/main/java/it/cnr/isti/workflow/manager/app/config/WebConfig.java b/src/main/java/it/cnr/isti/workflow/manager/app/config/WebConfig.java index 1640d00..ce92113 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/app/config/WebConfig.java +++ b/src/main/java/it/cnr/isti/workflow/manager/app/config/WebConfig.java @@ -33,7 +33,7 @@ public class WebConfig implements WebMvcConfigurer { registry.addMapping("/**") .allowedOrigins(origins) .allowedMethods("GET", "POST", "PUT", "DELETE", "OPTIONS") - .allowedHeaders("*") + .allowedHeaders("Content-Type", "Authorization", "X-Requested-With", "Accept") .allowCredentials(true); } diff --git a/src/main/java/it/cnr/isti/workflow/manager/app/health/MCPBridgeHealthIndicator.java b/src/main/java/it/cnr/isti/workflow/manager/app/health/MCPBridgeHealthIndicator.java new file mode 100644 index 0000000..6408310 --- /dev/null +++ b/src/main/java/it/cnr/isti/workflow/manager/app/health/MCPBridgeHealthIndicator.java @@ -0,0 +1,41 @@ +package it.cnr.isti.workflow.manager.app.health; + +import java.time.Duration; + +import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.actuate.health.Health; +import org.springframework.boot.actuate.health.HealthIndicator; +import org.springframework.stereotype.Component; +import org.springframework.web.reactive.function.client.WebClient; + +@Component +public class MCPBridgeHealthIndicator implements HealthIndicator { + + private final WebClient.Builder webClientBuilder; + private final String mcpBridgeUrl; + + public MCPBridgeHealthIndicator(WebClient.Builder webClientBuilder, + @Value("${app.mcp.bridge.url}") String mcpBridgeUrl) { + this.webClientBuilder = webClientBuilder; + this.mcpBridgeUrl = mcpBridgeUrl; + } + + @Override + public Health health() { + try { + webClientBuilder.build() + .get() + .uri(mcpBridgeUrl + "/health") + .retrieve() + .toBodilessEntity() + .timeout(Duration.ofSeconds(5)) + .block(); + return Health.up().build(); + } catch (Exception e) { + return Health.down() + .withDetail("url", mcpBridgeUrl) + .withDetail("error", e.getMessage()) + .build(); + } + } +} diff --git a/src/main/java/it/cnr/isti/workflow/manager/app/health/OllamaHealthIndicator.java b/src/main/java/it/cnr/isti/workflow/manager/app/health/OllamaHealthIndicator.java new file mode 100644 index 0000000..015fc3b --- /dev/null +++ b/src/main/java/it/cnr/isti/workflow/manager/app/health/OllamaHealthIndicator.java @@ -0,0 +1,46 @@ +package it.cnr.isti.workflow.manager.app.health; + +import java.time.Duration; + +import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.actuate.health.Health; +import org.springframework.boot.actuate.health.HealthIndicator; +import org.springframework.stereotype.Component; +import org.springframework.web.reactive.function.client.WebClient; + +@Component +public class OllamaHealthIndicator implements HealthIndicator { + + private final WebClient webClient; + private final String ollamaUrl; + private final String ollamaKey; + + public OllamaHealthIndicator(WebClient.Builder webClientBuilder, + @Value("${app.ollama.internal.url}") String ollamaUrl, + @Value("${app.ollama.internal.key}") String ollamaKey) { + this.ollamaUrl = ollamaUrl; + this.ollamaKey = ollamaKey; + this.webClient = webClientBuilder.baseUrl(ollamaUrl).build(); + } + + @Override + public Health health() { + try { + webClient.get() + .uri("/tags") + .header("Authorization", "Bearer " + ollamaKey) + .retrieve() + .bodyToMono(String.class) + .timeout(Duration.ofSeconds(5)) + .block(); + return Health.up() + .withDetail("url", ollamaUrl) + .build(); + } catch (Exception e) { + return Health.down() + .withDetail("url", ollamaUrl) + .withDetail("error", e.getMessage()) + .build(); + } + } +} diff --git a/src/main/java/it/cnr/isti/workflow/manager/auth/config/JwtAuthenticationFilter.java b/src/main/java/it/cnr/isti/workflow/manager/auth/config/JwtAuthenticationFilter.java index 89765ef..5db944d 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/auth/config/JwtAuthenticationFilter.java +++ b/src/main/java/it/cnr/isti/workflow/manager/auth/config/JwtAuthenticationFilter.java @@ -18,6 +18,7 @@ import it.cnr.isti.workflow.manager.auth.repo.AuthRepository; import it.cnr.isti.workflow.manager.auth.repo.LoginEntity; import jakarta.servlet.FilterChain; import jakarta.servlet.ServletException; +import jakarta.servlet.DispatcherType; import jakarta.servlet.http.Cookie; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; @@ -57,6 +58,20 @@ public class JwtAuthenticationFilter extends OncePerRequestFilter { return; } + if (request.getDispatcherType() == DispatcherType.ERROR) { + logger.warn("Error dispatch reached security filter: {} {} | originalUri={} | originalStatus={} | exceptionType={} | exceptionMessage={} | authCookiePresent={} | authHeaderPresent={} | origin={} | remote={}", + request.getMethod(), + path, + request.getAttribute("jakarta.servlet.error.request_uri"), + request.getAttribute("jakarta.servlet.error.status_code"), + request.getAttribute("jakarta.servlet.error.exception_type"), + request.getAttribute("jakarta.servlet.error.message"), + extractJwtFromCookie(request) != null, + hasBearerHeader(request), + request.getHeader("Origin"), + request.getRemoteAddr()); + } + String jwt = resolveJwt(request); if (StringUtils.hasText(jwt)) { @@ -73,18 +88,33 @@ public class JwtAuthenticationFilter extends OncePerRequestFilter { authToken.setDetails(new WebAuthenticationDetailsSource().buildDetails(request)); SecurityContextHolder.getContext().setAuthentication(authToken); + logger.debug("JWT authentication established for user '{}' on {} {} | dispatcher={} | tokenSource={} | origin={} | remote={}", + username, request.getMethod(), path, request.getDispatcherType(), resolveJwtSource(request), + request.getHeader("Origin"), request.getRemoteAddr()); } else { - logger.warn("JWT validation failed for user '{}' on {} {}", username, request.getMethod(), path); + logger.warn("JWT validation failed for user '{}' on {} {} | dispatcher={} | tokenSource={} | origin={} | remote={}", + username, request.getMethod(), path, request.getDispatcherType(), resolveJwtSource(request), + request.getHeader("Origin"), request.getRemoteAddr()); } } } catch (Exception e) { - logger.warn("JWT processing failed on {} {}: {}", request.getMethod(), path, e.getMessage()); + logger.warn("JWT processing failed on {} {} | dispatcher={} | tokenSource={} | origin={} | remote={} | error={}", + request.getMethod(), path, request.getDispatcherType(), resolveJwtSource(request), request.getHeader("Origin"), + request.getRemoteAddr(), e.getMessage()); } + } else if (!isExcludedPath(path)) { + logger.debug("No JWT resolved for protected request {} {} | dispatcher={} | origin={} | remote={} | cookiesPresent={} | authHeaderPresent={}", + request.getMethod(), path, request.getDispatcherType(), request.getHeader("Origin"), request.getRemoteAddr(), + request.getCookies() != null && request.getCookies().length > 0, hasBearerHeader(request)); } filterChain.doFilter(request, response); } + private boolean isExcludedPath(String path) { + return EXCLUDED_PATHS.stream().anyMatch(path::startsWith); + } + private String resolveJwt(HttpServletRequest request) { String jwtFromCookie = extractJwtFromCookie(request); if (StringUtils.hasText(jwtFromCookie)) { @@ -97,6 +127,19 @@ public class JwtAuthenticationFilter extends OncePerRequestFilter { return null; } + private String resolveJwtSource(HttpServletRequest request) { + String jwtFromCookie = extractJwtFromCookie(request); + if (StringUtils.hasText(jwtFromCookie)) { + return "cookie"; + } + return hasBearerHeader(request) ? "authorization-header" : "none"; + } + + private boolean hasBearerHeader(HttpServletRequest request) { + String authHeader = request.getHeader("Authorization"); + return StringUtils.hasText(authHeader) && authHeader.startsWith("Bearer "); + } + private String extractJwtFromCookie(HttpServletRequest request) { Cookie[] cookies = request.getCookies(); if (cookies == null) { diff --git a/src/main/java/it/cnr/isti/workflow/manager/auth/config/SecurityConfig.java b/src/main/java/it/cnr/isti/workflow/manager/auth/config/SecurityConfig.java index 12de347..70dfd88 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/auth/config/SecurityConfig.java +++ b/src/main/java/it/cnr/isti/workflow/manager/auth/config/SecurityConfig.java @@ -1,6 +1,10 @@ package it.cnr.isti.workflow.manager.auth.config; +import java.util.Arrays; + +import org.slf4j.Logger; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.security.authentication.AuthenticationManager; @@ -10,8 +14,16 @@ import org.springframework.security.config.annotation.method.configuration.Enabl import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.config.http.SessionCreationPolicy; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; +import org.springframework.security.web.csrf.CookieCsrfTokenRepository; +import org.springframework.security.web.csrf.CsrfTokenRequestAttributeHandler; + +import jakarta.servlet.http.Cookie; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.DispatcherType; @Configuration @@ -19,14 +31,21 @@ import org.springframework.security.web.authentication.UsernamePasswordAuthentic @EnableMethodSecurity public class SecurityConfig { + private static final Logger logger = org.slf4j.LoggerFactory.getLogger(SecurityConfig.class); + @Autowired private JwtAuthenticationFilter jwtFilter; + @Value("${app.auth.cookie.name:auth_token}") + private String authCookieName; + + @Value("${app.security.csrf.enabled:false}") + private boolean csrfEnabled; + @Bean SecurityFilterChain filterChain(HttpSecurity http) throws Exception { - return http + http .cors(Customizer.withDefaults()) - .csrf(csrf -> csrf.disable()) .authorizeHttpRequests(auth -> auth .requestMatchers("/auth/login").permitAll() .requestMatchers("/auth/register").permitAll() @@ -36,12 +55,57 @@ public class SecurityConfig { .requestMatchers("/swagger-ui/**").permitAll() .requestMatchers("/v3/api-docs/**").permitAll() .requestMatchers("/actuator/**").permitAll() + .requestMatchers("/error").permitAll() .requestMatchers("/blocks/types").permitAll() .requestMatchers("/blocks/types/**").permitAll() .requestMatchers("/containers/types").permitAll() .requestMatchers("/containers/types/**").permitAll() .requestMatchers("/retriever/**").permitAll() - .anyRequest().authenticated()) + .anyRequest().authenticated()); + + if (csrfEnabled) { + logger.info("CSRF protection is ENABLED"); + CsrfTokenRequestAttributeHandler requestHandler = new CsrfTokenRequestAttributeHandler(); + http.csrf(csrf -> csrf + .csrfTokenRepository(CookieCsrfTokenRepository.withHttpOnlyFalse()) + .csrfTokenRequestHandler(requestHandler) + .ignoringRequestMatchers( + "/auth/login", "/auth/register", "/auth/logout", + "/auth/change-password", "/actuator/**")); + } else { + logger.warn("CSRF protection is DISABLED. Set app.security.csrf.enabled=true for production."); + http.csrf(csrf -> csrf.disable()); + } + + return http + .exceptionHandling(exceptions -> exceptions + .authenticationEntryPoint((request, response, authException) -> { + logger.warn( + "Rejecting unauthenticated request as 403: {} {} | authHeaderPresent={} | authCookiePresent={} | origin={} | remote={} | reason={}", + request.getMethod(), + request.getRequestURI(), + hasBearerHeader(request), + hasAuthCookie(request, authCookieName), + request.getHeader("Origin"), + request.getRemoteAddr(), + authException == null ? "n/a" : authException.getMessage()); + response.sendError(403, "Forbidden"); + }) + .accessDeniedHandler((request, response, accessDeniedException) -> { + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + logger.warn( + "Rejecting authenticated request with real 403: {} {} | principal={} | authorities={} | authHeaderPresent={} | authCookiePresent={} | origin={} | remote={} | reason={}", + request.getMethod(), + request.getRequestURI(), + authentication == null ? "anonymous" : authentication.getName(), + authentication == null ? "[]" : authentication.getAuthorities(), + hasBearerHeader(request), + hasAuthCookie(request, authCookieName), + request.getHeader("Origin"), + request.getRemoteAddr(), + accessDeniedException == null ? "n/a" : accessDeniedException.getMessage()); + response.sendError(403, "Forbidden"); + })) .sessionManagement(sess -> sess.sessionCreationPolicy(SessionCreationPolicy.STATELESS)) .addFilterBefore(jwtFilter, UsernamePasswordAuthenticationFilter.class) .build(); @@ -51,4 +115,17 @@ public class SecurityConfig { AuthenticationManager authenticationManager(AuthenticationConfiguration config) throws Exception { return config.getAuthenticationManager(); } + + private static boolean hasBearerHeader(HttpServletRequest request) { + String authHeader = request.getHeader("Authorization"); + return authHeader != null && authHeader.startsWith("Bearer "); + } + + private static boolean hasAuthCookie(HttpServletRequest request, String authCookieName) { + Cookie[] cookies = request.getCookies(); + if (cookies == null) { + return false; + } + return Arrays.stream(cookies).anyMatch(cookie -> authCookieName.equals(cookie.getName())); + } } diff --git a/src/main/java/it/cnr/isti/workflow/manager/auth/services/AuthService.java b/src/main/java/it/cnr/isti/workflow/manager/auth/services/AuthService.java index 82e97d7..00548d0 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/auth/services/AuthService.java +++ b/src/main/java/it/cnr/isti/workflow/manager/auth/services/AuthService.java @@ -139,8 +139,19 @@ public class AuthService { return authRepository.findByUsernameAndActiveTrue(username).orElse(null); } + private static final java.util.Set COMMON_PASSWORDS = java.util.Set.of( + "password", "12345678", "123456789", "1234567890", "qwerty", + "letmein", "welcome", "admin", "trustno1", "iloveyou", + "sunshine", "princess", "football", "charlie", "access", + "master", "monkey", "dragon", "shadow", "michael", + "password1", "password123", "abc123", "changeme"); + private boolean isValidPassword(String password) { - if (password == null || password.length() < 8) { + if (password == null || password.length() < 10) { + return false; + } + String lower = password.toLowerCase(java.util.Locale.ROOT); + if (COMMON_PASSWORDS.stream().anyMatch(lower::contains)) { return false; } boolean hasUppercase = false; diff --git a/src/main/java/it/cnr/isti/workflow/manager/auth/services/LoginRateLimiter.java b/src/main/java/it/cnr/isti/workflow/manager/auth/services/LoginRateLimiter.java new file mode 100644 index 0000000..ddee5d3 --- /dev/null +++ b/src/main/java/it/cnr/isti/workflow/manager/auth/services/LoginRateLimiter.java @@ -0,0 +1,42 @@ +package it.cnr.isti.workflow.manager.auth.services; + +import java.util.Deque; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentLinkedDeque; + +import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.stereotype.Component; + +@Component +public class LoginRateLimiter { + + private static final int MAX_ATTEMPTS = 10; + private static final long WINDOW_MS = 60_000; + + private final ConcurrentHashMap> requestLog = new ConcurrentHashMap<>(); + + public boolean isAllowed(String key) { + long now = System.currentTimeMillis(); + Deque timestamps = requestLog.computeIfAbsent(key, k -> new ConcurrentLinkedDeque<>()); + while (!timestamps.isEmpty() && timestamps.peekFirst() < now - WINDOW_MS) { + timestamps.pollFirst(); + } + if (timestamps.size() >= MAX_ATTEMPTS) { + return false; + } + timestamps.addLast(now); + return true; + } + + @Scheduled(fixedRate = 300_000) + public void cleanup() { + long now = System.currentTimeMillis(); + requestLog.entrySet().removeIf(entry -> { + Deque ts = entry.getValue(); + while (!ts.isEmpty() && ts.peekFirst() < now - WINDOW_MS) { + ts.pollFirst(); + } + return ts.isEmpty(); + }); + } +} diff --git a/src/main/java/it/cnr/isti/workflow/manager/controllers/ApiExceptionHandler.java b/src/main/java/it/cnr/isti/workflow/manager/controllers/ApiExceptionHandler.java index ce8ed2b..756e64a 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/controllers/ApiExceptionHandler.java +++ b/src/main/java/it/cnr/isti/workflow/manager/controllers/ApiExceptionHandler.java @@ -6,8 +6,10 @@ import java.util.Map; import java.util.stream.Collectors; import org.slf4j.Logger; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; import org.springframework.http.ProblemDetail; +import org.springframework.http.converter.HttpMessageNotReadableException; import org.springframework.validation.FieldError; import org.springframework.validation.ObjectError; import org.springframework.web.bind.MethodArgumentNotValidException; @@ -15,6 +17,7 @@ import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.RestControllerAdvice; import org.springframework.web.server.ResponseStatusException; +import jakarta.servlet.http.HttpServletRequest; import it.cnr.isti.workflow.manager.flows.validation.ValidationError; import it.cnr.isti.workflow.manager.flows.validation.ValidationErrorCode; import it.cnr.isti.workflow.manager.flows.validation.ValidationErrorCodec; @@ -24,6 +27,9 @@ public class ApiExceptionHandler { private static final Logger logger = org.slf4j.LoggerFactory.getLogger(ApiExceptionHandler.class); + @Autowired + private HttpServletRequest request; + @ExceptionHandler(MethodArgumentNotValidException.class) public ProblemDetail handleMethodArgumentNotValid(MethodArgumentNotValidException e) { List> errors = e.getBindingResult().getAllErrors().stream() @@ -34,7 +40,8 @@ public class ApiExceptionHandler { .map(error -> String.valueOf(error.getOrDefault("message", "Validation failed"))) .collect(Collectors.joining(", ")); - logger.warn("Request validation failed with status 400 BAD_REQUEST: {}", detail); + logger.warn("Request validation failed with status 400 BAD_REQUEST on {} {}: {}", + request.getMethod(), request.getRequestURI(), detail); ProblemDetail problem = ProblemDetail.forStatus(HttpStatus.BAD_REQUEST); problem.setTitle("Bad Request"); @@ -43,9 +50,22 @@ public class ApiExceptionHandler { return problem; } + @ExceptionHandler(HttpMessageNotReadableException.class) + public ProblemDetail handleHttpMessageNotReadable(HttpMessageNotReadableException e) { + String detail = resolveReadableMessage(e); + logger.warn("Request body unreadable on {} {}: {}", + request.getMethod(), request.getRequestURI(), detail); + + ProblemDetail problem = ProblemDetail.forStatus(HttpStatus.BAD_REQUEST); + problem.setTitle("Bad Request"); + problem.setDetail(detail); + return problem; + } + @ExceptionHandler(ResponseStatusException.class) public ProblemDetail handleResponseStatus(ResponseStatusException e) { - logger.warn("Request failed with status {}: {}", e.getStatusCode(), e.getReason()); + logger.warn("Request failed on {} {} with status {}: {}", + request.getMethod(), request.getRequestURI(), e.getStatusCode(), e.getReason()); ProblemDetail problem = ProblemDetail.forStatus(e.getStatusCode()); problem.setTitle(e.getStatusCode().toString()); @@ -54,6 +74,31 @@ public class ApiExceptionHandler { return problem; } + @ExceptionHandler(Exception.class) + public ProblemDetail handleGenericException(Exception e) { + logger.error("Unhandled exception on {} {}: {}", + request.getMethod(), request.getRequestURI(), e.getMessage(), e); + + ProblemDetail problem = ProblemDetail.forStatus(HttpStatus.INTERNAL_SERVER_ERROR); + problem.setTitle(HttpStatus.INTERNAL_SERVER_ERROR.toString()); + problem.setDetail(e.getMessage() == null || e.getMessage().isBlank() ? "Internal server error" : e.getMessage()); + return problem; + } + + private String resolveReadableMessage(HttpMessageNotReadableException e) { + Throwable cause = e.getMostSpecificCause(); + String message = cause != null && cause.getMessage() != null && !cause.getMessage().isBlank() + ? cause.getMessage() + : e.getMessage(); + if (message == null || message.isBlank()) { + return "Request body is invalid"; + } + if (message.contains("missing type id property 'type'")) { + return "Request body is invalid: missing required property 'type' for container configuration"; + } + return message; + } + private List toValidationErrors(ObjectError error) { List decoded = ValidationErrorCodec.decode(error.getDefaultMessage()); if (decoded.isEmpty()) { diff --git a/src/main/java/it/cnr/isti/workflow/manager/controllers/AuthController.java b/src/main/java/it/cnr/isti/workflow/manager/controllers/AuthController.java index be4b594..6bd3ed7 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/controllers/AuthController.java +++ b/src/main/java/it/cnr/isti/workflow/manager/controllers/AuthController.java @@ -15,6 +15,7 @@ import it.cnr.isti.workflow.manager.auth.model.ChangePasswordRequest; import it.cnr.isti.workflow.manager.auth.model.CreateUserResult; import it.cnr.isti.workflow.manager.auth.model.DeleteUserResult; import it.cnr.isti.workflow.manager.auth.services.AuthService; +import it.cnr.isti.workflow.manager.auth.services.LoginRateLimiter; import it.cnr.isti.workflow.manager.auth.services.TurnstileService; import org.slf4j.Logger; @@ -36,6 +37,7 @@ import org.eclipse.microprofile.openapi.annotations.Operation; import java.util.LinkedHashMap; import java.util.Map; +import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; @RestController @@ -53,6 +55,9 @@ public class AuthController { @Autowired private TurnstileService turnstileService; + @Autowired + private LoginRateLimiter loginRateLimiter; + @Value("${app.auth.cookie.name:auth_token}") private String authCookieName; @@ -64,8 +69,11 @@ public class AuthController { @PostMapping("/login") @Operation(summary = "Login", description = "Authenticates a user and returns a JWT token on success.") - public ResponseEntity login(@RequestBody AuthRequest request, HttpServletResponse servletResponse) { - logger.info("Login attempt for user: {}", request.getUsername()); + public ResponseEntity login(@RequestBody AuthRequest request, HttpServletRequest servletRequest, HttpServletResponse servletResponse) { + logger.debug("Login attempt for user: {}", request.getUsername()); + if (!loginRateLimiter.isAllowed(servletRequest.getRemoteAddr())) { + return ResponseEntity.status(HttpStatus.TOO_MANY_REQUESTS).body("Too many login attempts. Try again later."); + } if (request.getUsername() == null || request.getPassword() == null) { return ResponseEntity.badRequest().body("Username and password are required"); } @@ -89,7 +97,7 @@ public class AuthController { @PostMapping("/register") @Operation(summary = "Register user", description = "Registers a new user account with username and password.") public ResponseEntity register(@RequestBody AuthRequest request, HttpServletResponse servletResponse) { - logger.info("Register attempt for user {}", request.getUsername()); + logger.debug("Register attempt for user {}", request.getUsername()); if (request.getUsername() == null || request.getPassword() == null || request.getEmail() == null) { return ResponseEntity.badRequest().body("Username, password and email are required"); } @@ -168,6 +176,7 @@ public class AuthController { return ResponseEntity.badRequest().body("username, password and email are required"); } CreateUserResult result = authService.createUser(request.username(), request.password(), request.email(), request.role()); + logger.info("AUDIT: admin createUser username={} result={}", request.username(), result); return switch (result) { case SUCCESS -> ResponseEntity.ok().build(); case USER_ALREADY_EXISTS -> ResponseEntity.badRequest().body("USER_ALREADY_EXISTS"); @@ -186,6 +195,7 @@ public class AuthController { return ResponseEntity.badRequest().body("username and newPassword are required"); } AdminChangePasswordResult result = authService.adminChangePassword(username, request.newPassword()); + logger.info("AUDIT: admin changePassword username={} result={}", username, result); return switch (result) { case SUCCESS -> ResponseEntity.ok().build(); case USER_NOT_FOUND -> ResponseEntity.status(HttpStatus.NOT_FOUND).body("User " + username + " not found"); @@ -202,6 +212,7 @@ public class AuthController { return ResponseEntity.badRequest().body("username and role are required"); } ChangeUserRoleResult result = authService.changeUserRole(username, request.role()); + logger.info("AUDIT: admin changeRole username={} newRole={} result={}", username, request.role(), result); return switch (result) { case SUCCESS -> ResponseEntity.ok().build(); case USER_NOT_FOUND -> ResponseEntity.status(HttpStatus.NOT_FOUND).body("User " + username + " not found"); @@ -218,6 +229,7 @@ public class AuthController { return ResponseEntity.badRequest().body("username is required"); } DeleteUserResult result = authService.deleteUser(username); + logger.info("AUDIT: admin deleteUser username={} result={}", username, result); return switch (result) { case SUCCESS -> ResponseEntity.ok().build(); case USER_NOT_FOUND -> ResponseEntity.status(HttpStatus.NOT_FOUND).body("User " + username + " not found"); diff --git a/src/main/java/it/cnr/isti/workflow/manager/controllers/ContainersController.java b/src/main/java/it/cnr/isti/workflow/manager/controllers/ContainersController.java index 653e2ee..ad4c388 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/controllers/ContainersController.java +++ b/src/main/java/it/cnr/isti/workflow/manager/controllers/ContainersController.java @@ -2,6 +2,8 @@ package it.cnr.isti.workflow.manager.controllers; import java.util.List; import java.util.Map; + +import org.slf4j.Logger; import org.eclipse.microprofile.openapi.annotations.Operation; import org.eclipse.microprofile.openapi.annotations.security.SecurityRequirement; import org.springframework.http.HttpStatus; @@ -28,6 +30,8 @@ import it.cnr.isti.workflow.manager.ios.IODescriptor; @RequestMapping("/containers") public class ContainersController { + private static final Logger logger = org.slf4j.LoggerFactory.getLogger(ContainersController.class); + Map containerTypes; List> containerFactories; @@ -115,11 +119,24 @@ public class ContainersController { @SecurityRequirement(name = "bearerAuth") @Operation(summary = "Create container", description = "Creates a container from the provided container configuration.") public > Container create(@RequestBody C configuration) { + logger.debug("Create container request received | payloadClass={} | containerType={} | containerName={}", + configuration == null ? "null" : configuration.getClass().getName(), + configuration == null || configuration.getContainerType() == null ? "null" + : configuration.getContainerType().getSimpleName(), + configuration == null ? "null" : configuration.getName()); ContainerFactory factory = (ContainerFactory) containerFactories.stream() .filter(f -> f.getContainerType().equals(configuration.getContainerType())) .findFirst() .orElseThrow(() -> new IllegalArgumentException("Container factory not found for type: " + configuration.getContainerType())); - return factory.create(configuration); + logger.debug("Resolved container factory {} for type {}", + factory.getClass().getName(), + configuration.getContainerType().getSimpleName()); + Container container = factory.create(configuration); + logger.debug("Container created successfully | containerId={} | containerType={} | containerName={}", + container == null ? "null" : container.getId(), + container == null || container.getType() == null ? "null" : container.getType().getName(), + container == null ? "null" : container.getName()); + return container; } @PostMapping("/validate-subflow") diff --git a/src/main/java/it/cnr/isti/workflow/manager/controllers/ExecutionsController.java b/src/main/java/it/cnr/isti/workflow/manager/controllers/ExecutionsController.java index 6d9f080..50e0de7 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/controllers/ExecutionsController.java +++ b/src/main/java/it/cnr/isti/workflow/manager/controllers/ExecutionsController.java @@ -11,6 +11,8 @@ import org.eclipse.microprofile.openapi.annotations.Operation; import org.eclipse.microprofile.openapi.annotations.security.SecurityRequirement; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.web.server.WebServerException; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.PageRequest; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.security.core.annotation.AuthenticationPrincipal; @@ -132,6 +134,22 @@ public class ExecutionsController { return visibleExecutions(userDetails).stream().map(ExecutionView::fromExecution).toList(); } + @Operation(summary = "Retrieves executions (paginated)", description = "Retrieves a paginated list of executions for the authenticated user") + @GetMapping(path = "paged") + public Page getAllPaged( + @RequestParam(defaultValue = "0") int page, + @RequestParam(defaultValue = "50") int size, + @AuthenticationPrincipal LoginEntity userDetails) { + if (userDetails == null) { + throw new ResponseStatusException(HttpStatus.FORBIDDEN, "Authentication required"); + } + if (size > 200) { + size = 200; + } + return executionService.getExecutionsByOwner(userDetails.getUsername(), PageRequest.of(page, size)) + .map(ExecutionView::fromExecution); + } + /** @@ -160,11 +178,10 @@ public class ExecutionsController { } private List visibleExecutions(LoginEntity userDetails) { - List allExecutions = executionService.getAllExecutions(); if (userDetails == null) { - return allExecutions; + throw new ResponseStatusException(HttpStatus.FORBIDDEN, "Authentication required"); } - return allExecutions.stream() + return executionService.getAllExecutions().stream() .filter(execution -> userDetails.getUsername().equals(execution.getOwner())) .toList(); } @@ -335,10 +352,10 @@ public class ExecutionsController { } private ExecutionObject visibleExecution(String id, LoginEntity userDetails) { - ExecutionObject execution = executionService.getExecution(id); if (userDetails == null) { - return execution; + throw new ResponseStatusException(HttpStatus.FORBIDDEN, "Authentication required"); } + ExecutionObject execution = executionService.getExecution(id); if (!userDetails.getUsername().equals(execution.getOwner())) { throw new ResponseStatusException(HttpStatus.FORBIDDEN, "Execution with id " + id + " is not accessible"); } diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/ExecutionObject.java b/src/main/java/it/cnr/isti/workflow/manager/executions/ExecutionObject.java index 3799d2a..31c35e3 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/executions/ExecutionObject.java +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/ExecutionObject.java @@ -7,6 +7,7 @@ import java.util.Map; import java.util.UUID; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import java.util.function.Function; import java.util.stream.Collectors; @@ -253,6 +254,20 @@ public class ExecutionObject { return resumedStatus; } + public void shutdown() { + if (this.executorService != null && !this.executorService.isShutdown()) { + this.executorService.shutdown(); + try { + if (!this.executorService.awaitTermination(5, TimeUnit.SECONDS)) { + this.executorService.shutdownNow(); + } + } catch (InterruptedException e) { + this.executorService.shutdownNow(); + Thread.currentThread().interrupt(); + } + } + } + protected void cancel() { if (this.executorService != null) { this.executorService.shutdownNow(); diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/ExecutionTemplateResolver.java b/src/main/java/it/cnr/isti/workflow/manager/executions/ExecutionTemplateResolver.java index 4784c34..f0b30e9 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/executions/ExecutionTemplateResolver.java +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/ExecutionTemplateResolver.java @@ -3,6 +3,9 @@ package it.cnr.isti.workflow.manager.executions; import java.util.Collection; import java.util.LinkedHashMap; import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Collectors; import org.springframework.util.StringUtils; @@ -27,10 +30,14 @@ public final class ExecutionTemplateResolver { if (!StringUtils.hasText(template)) { return template; } - String resolved = template; + // Build full substitution map first, then apply in a single pass. + // Single-pass prevents chaining injection (a value containing ${{...}} + // cannot resolve further placeholders) and Matcher.quoteReplacement + // guards against regex metacharacters inside replacement values. + Map substitutions = new LinkedHashMap<>(); if (values != null) { for (Map.Entry entry : values.entrySet()) { - resolved = resolved.replace("${{" + entry.getKey() + "}}", formatValue(entry.getValue())); + substitutions.put("${{" + entry.getKey() + "}}", formatValue(entry.getValue())); } } if (executionVariables != null) { @@ -38,18 +45,25 @@ public final class ExecutionTemplateResolver { if (entry.getKey() == null) { continue; } - if (entry.getKey().startsWith(ExecutionRuntimeContextSupport.GLOBAL_PREFIX)) { - resolved = resolved.replace("${{" + entry.getKey() + "}}", formatValue(entry.getValue())); - continue; + String placeholder; + if (entry.getKey().startsWith(ExecutionRuntimeContextSupport.GLOBAL_PREFIX) + || entry.getKey().startsWith(ExecutionRuntimeContextSupport.CONTEXT_PREFIX)) { + placeholder = "${{" + entry.getKey() + "}}"; + } else { + placeholder = "${{vars." + entry.getKey() + "}}"; } - if (entry.getKey().startsWith(ExecutionRuntimeContextSupport.CONTEXT_PREFIX)) { - resolved = resolved.replace("${{" + entry.getKey() + "}}", formatValue(entry.getValue())); - continue; - } - resolved = resolved.replace("${{vars." + entry.getKey() + "}}", formatValue(entry.getValue())); + substitutions.put(placeholder, formatValue(entry.getValue())); } } - return resolved; + if (substitutions.isEmpty()) { + return template; + } + Pattern pattern = Pattern.compile( + substitutions.keySet().stream() + .map(Pattern::quote) + .collect(Collectors.joining("|"))); + return pattern.matcher(template).replaceAll( + match -> Matcher.quoteReplacement(substitutions.get(match.group()))); } public static String formatValue(Object value) { diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/ExecutionsService.java b/src/main/java/it/cnr/isti/workflow/manager/executions/ExecutionsService.java index 31468df..fe04b0e 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/executions/ExecutionsService.java +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/ExecutionsService.java @@ -1,12 +1,14 @@ package it.cnr.isti.workflow.manager.executions; -import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable; import org.springframework.stereotype.Service; import org.springframework.web.server.ResponseStatusException; import org.springframework.http.HttpStatus; @@ -36,7 +38,7 @@ import it.cnr.isti.workflow.manager.mcp.MCPSharedSessionRegistry; @Service public class ExecutionsService { - private static Map executions = new HashMap<>(); + private final Map executions = new ConcurrentHashMap<>(); @Autowired FlowExecutionValidator flowExecutionValidator; @@ -108,6 +110,11 @@ public class ExecutionsService { .toList(); } + public Page getExecutionsByOwner(String owner, Pageable pageable) { + return executionRepository.findByOwner(owner, pageable) + .map(entity -> executions.computeIfAbsent(entity.getId(), ignored -> rebuildExecution(entity))); + } + public void removeExecution(String id) { ExecutionObject execution = executions.get(id); if (execution == null && executionRepository.existsById(id)) { @@ -118,6 +125,7 @@ public class ExecutionsService { } if (execution.getContext().getStatus() == ExecutionStatus.RUNNING) throw new IllegalStateException("Execution with id " + id + " is still running"); + execution.shutdown(); executions.remove(id); executionRepository.deleteById(id); } diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/repo/ExecutionRepository.java b/src/main/java/it/cnr/isti/workflow/manager/executions/repo/ExecutionRepository.java index 886fab4..1821988 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/executions/repo/ExecutionRepository.java +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/repo/ExecutionRepository.java @@ -1,10 +1,13 @@ package it.cnr.isti.workflow.manager.executions.repo; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable; import org.springframework.data.jpa.repository.JpaRepository; import java.util.List; public interface ExecutionRepository extends JpaRepository { List findByOwner(String owner); + Page findByOwner(String owner, Pageable pageable); void deleteByOwner(String owner); } diff --git a/src/main/java/it/cnr/isti/workflow/manager/http/HTTPServerCallService.java b/src/main/java/it/cnr/isti/workflow/manager/http/HTTPServerCallService.java index 7b91d5f..e39afce 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/http/HTTPServerCallService.java +++ b/src/main/java/it/cnr/isti/workflow/manager/http/HTTPServerCallService.java @@ -1,6 +1,7 @@ package it.cnr.isti.workflow.manager.http; import java.nio.charset.StandardCharsets; +import java.time.Duration; import java.util.Base64; import java.util.Objects; @@ -51,6 +52,7 @@ public class HTTPServerCallService { return requestSpec.retrieve() .bodyToMono(String.class) + .timeout(Duration.ofSeconds(30)) .block(); } diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 74b00bd..82f1734 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -6,9 +6,10 @@ spring.datasource.username=${DB_USER:lucio} spring.datasource.password=${DB_PASSWORD:password} spring.datasource.driver-class-name=org.postgresql.Driver spring.jpa.properties.hibernate.dialect=org.hibernate.dialect.PostgreSQLDialect -spring.jpa.hibernate.ddl-auto=create-drop +spring.jpa.hibernate.ddl-auto=${ddl-auto:update} management.endpoints.web.exposure.include=health,info +management.endpoint.health.show-details=always # Keycloak #keycloak.realm=${REALM_NAME:wf-editor} diff --git a/src/test/java/it/cnr/isti/workflow/manager/controllers/AuthControllerTest.java b/src/test/java/it/cnr/isti/workflow/manager/controllers/AuthControllerTest.java index 935439b..bd9b394 100644 --- a/src/test/java/it/cnr/isti/workflow/manager/controllers/AuthControllerTest.java +++ b/src/test/java/it/cnr/isti/workflow/manager/controllers/AuthControllerTest.java @@ -17,6 +17,7 @@ import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.test.context.bean.override.mockito.MockitoBean; import org.springframework.test.context.TestPropertySource; import org.springframework.test.web.servlet.MockMvc; +import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import it.cnr.isti.workflow.manager.auth.config.JwtUtil; @@ -50,6 +51,12 @@ public class AuthControllerTest { return new MockHttpServletResponse(); } + private static MockHttpServletRequest request() { + MockHttpServletRequest req = new MockHttpServletRequest(); + req.setRemoteAddr("127.0.0.1"); + return req; + } + @Autowired private AuthController authController; @@ -80,7 +87,7 @@ public class AuthControllerTest { public void testLogin() { ResponseEntity loginResponse = authController.login( new AuthRequest("testuser", "testpassword", "testuser@example.com"), - response()); + request(), response()); assert loginResponse.getStatusCode().is2xxSuccessful(); assert loginResponse.getBody() instanceof java.util.Map; assert "USER".equals(((java.util.Map) loginResponse.getBody()).get("role")); @@ -92,7 +99,7 @@ public class AuthControllerTest { String suffix = uniqueSuffix(); String username = "changepwd-" + suffix; String oldPassword = "Startpass1!"; - String newPassword = "Newpassword1!"; + String newPassword = "Freshcr3d!X"; authController.register(new AuthRequest(username, oldPassword, username + "@example.com"), response()); ChangePasswordRequest request = new ChangePasswordRequest(); @@ -105,7 +112,7 @@ public class AuthControllerTest { ResponseEntity loginResponse = authController.login( new AuthRequest(username, newPassword, username + "@example.com"), - response()); + request(), response()); assert loginResponse.getStatusCode().is2xxSuccessful(); } @@ -194,7 +201,7 @@ public class AuthControllerTest { ResponseEntity loginResponse = authController.login( new AuthRequest(managedUsername, "Changedpass1!", managedEmail), - response()); + request(), response()); assert loginResponse.getStatusCode().is2xxSuccessful(); assert loginResponse.getBody() instanceof java.util.Map; assert "ADMIN".equals(((java.util.Map) loginResponse.getBody()).get("role")); @@ -238,7 +245,7 @@ public class AuthControllerTest { ResponseEntity deletedLoginResponse = authController.login( new AuthRequest(managedUsername, "Changedpass1!", managedEmail), - response()); + request(), response()); assert deletedLoginResponse.getStatusCode() == HttpStatus.NOT_FOUND; ResponseEntity listAfterDelete = authController.listUsers(); diff --git a/src/test/java/it/cnr/isti/workflow/manager/executions/AuthServicePasswordTest.java b/src/test/java/it/cnr/isti/workflow/manager/executions/AuthServicePasswordTest.java new file mode 100644 index 0000000..407f700 --- /dev/null +++ b/src/test/java/it/cnr/isti/workflow/manager/executions/AuthServicePasswordTest.java @@ -0,0 +1,75 @@ +package it.cnr.isti.workflow.manager.executions; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.TestPropertySource; + +import it.cnr.isti.workflow.manager.auth.model.CreateUserResult; +import it.cnr.isti.workflow.manager.auth.services.AuthService; + +@SpringBootTest +@TestPropertySource(locations = "classpath:test.properties") +public class AuthServicePasswordTest { + + @Autowired + private AuthService authService; + + @Test + public void rejectsShortPassword() { + CreateUserResult result = authService.registerUser("pwtest1", "Ab1!xxxxx", "pwtest1@test.com"); + assertEquals(CreateUserResult.INVALID_PASSWORD, result); + } + + @Test + public void rejectsPasswordWithoutUppercase() { + CreateUserResult result = authService.registerUser("pwtest2", "abcdefgh1!", "pwtest2@test.com"); + assertEquals(CreateUserResult.INVALID_PASSWORD, result); + } + + @Test + public void rejectsPasswordWithoutLowercase() { + CreateUserResult result = authService.registerUser("pwtest3", "ABCDEFGH1!", "pwtest3@test.com"); + assertEquals(CreateUserResult.INVALID_PASSWORD, result); + } + + @Test + public void rejectsPasswordWithoutDigit() { + CreateUserResult result = authService.registerUser("pwtest4", "Abcdefghij!", "pwtest4@test.com"); + assertEquals(CreateUserResult.INVALID_PASSWORD, result); + } + + @Test + public void rejectsPasswordWithoutSpecialChar() { + CreateUserResult result = authService.registerUser("pwtest5", "Abcdefgh12", "pwtest5@test.com"); + assertEquals(CreateUserResult.INVALID_PASSWORD, result); + } + + @Test + public void rejectsCommonPassword() { + CreateUserResult result = authService.registerUser("pwtest6", "Password1!", "pwtest6@test.com"); + assertEquals(CreateUserResult.INVALID_PASSWORD, result); + } + + @Test + public void rejectsPasswordContainingCommonWord() { + CreateUserResult result = authService.registerUser("pwtest7", "myLetmein1!", "pwtest7@test.com"); + assertEquals(CreateUserResult.INVALID_PASSWORD, result); + } + + @Test + public void acceptsStrongPassword() { + CreateUserResult result = authService.registerUser("pwtest8", "Str0ng!Uniq", "pwtest8@test.com"); + assertEquals(CreateUserResult.SUCCESS, result); + } + + @Test + public void rejectsPasswordWithWhitespace() { + CreateUserResult result = authService.registerUser("pwtest9", "Str0ng! Unq", "pwtest9@test.com"); + assertEquals(CreateUserResult.INVALID_PASSWORD, result); + } +} diff --git a/src/test/java/it/cnr/isti/workflow/manager/executions/ExecutionTemplateResolverTest.java b/src/test/java/it/cnr/isti/workflow/manager/executions/ExecutionTemplateResolverTest.java new file mode 100644 index 0000000..86633cb --- /dev/null +++ b/src/test/java/it/cnr/isti/workflow/manager/executions/ExecutionTemplateResolverTest.java @@ -0,0 +1,123 @@ +package it.cnr.isti.workflow.manager.executions; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.Test; + +public class ExecutionTemplateResolverTest { + + @Test + public void resolvesSimplePlaceholder() { + String result = ExecutionTemplateResolver.resolve( + "Hello ${{name}}!", + Map.of("name", "World"), + null); + assertEquals("Hello World!", result); + } + + @Test + public void resolvesMultiplePlaceholders() { + String result = ExecutionTemplateResolver.resolve( + "${{greeting}} ${{name}}!", + Map.of("greeting", "Hi", "name", "Alice"), + null); + assertEquals("Hi Alice!", result); + } + + @Test + public void resolvesExecutionVariablesWithVarsPrefix() { + Map execVars = new LinkedHashMap<>(); + execVars.put("color", "blue"); + String result = ExecutionTemplateResolver.resolve( + "Color is ${{vars.color}}", + Map.of(), + execVars); + assertEquals("Color is blue", result); + } + + @Test + public void resolvesContextVariables() { + Map execVars = new LinkedHashMap<>(); + execVars.put("context.executionId", "abc-123"); + String result = ExecutionTemplateResolver.resolve( + "Execution: ${{context.executionId}}", + Map.of(), + execVars); + assertEquals("Execution: abc-123", result); + } + + @Test + public void resolvesGlobalVariables() { + Map execVars = new LinkedHashMap<>(); + execVars.put("global.apiUrl", "https://example.com"); + String result = ExecutionTemplateResolver.resolve( + "URL: ${{global.apiUrl}}", + Map.of(), + execVars); + assertEquals("URL: https://example.com", result); + } + + @Test + public void nullTemplateReturnsNull() { + String result = ExecutionTemplateResolver.resolve( + null, Map.of(), null); + assertEquals(null, result); + } + + @Test + public void emptyTemplateReturnsEmpty() { + String result = ExecutionTemplateResolver.resolve( + "", Map.of(), null); + assertEquals("", result); + } + + @Test + public void unresolvedPlaceholderRemainsIntact() { + String result = ExecutionTemplateResolver.resolve( + "Hello ${{unknown}}!", + Map.of(), + null); + assertEquals("Hello ${{unknown}}!", result); + } + + @Test + public void valueContainingPlaceholderSyntaxIsNotReResolved() { + // This is the template injection test: + // A value that itself contains ${{...}} should NOT be resolved further. + String result = ExecutionTemplateResolver.resolve( + "Cmd: ${{cmd}}", + Map.of("cmd", "${{secret}}"), + Map.of("secret", "LEAKED")); + // The value "${{secret}}" should appear literally, NOT "LEAKED" + assertEquals("Cmd: ${{secret}}", result); + } + + @Test + public void valueWithRegexMetacharactersIsSafe() { + String result = ExecutionTemplateResolver.resolve( + "Pattern: ${{pat}}", + Map.of("pat", "$1 \\n (group)"), + null); + assertEquals("Pattern: $1 \\n (group)", result); + } + + @Test + public void formatValueHandlesCollection() { + String result = ExecutionTemplateResolver.formatValue(List.of("a", "b", "c")); + assertNotNull(result); + assertTrue(result.contains("a")); + assertTrue(result.contains("b")); + assertTrue(result.contains("c")); + } + + @Test + public void formatValueHandlesNull() { + assertEquals("null", ExecutionTemplateResolver.formatValue(null)); + } +}