diff --git a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadService.java b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadService.java index 4d20756..bacd12d 100644 --- a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadService.java +++ b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/service/JarUploadService.java @@ -47,6 +47,14 @@ public class JarUploadService { /** Allowed characters in job name and version used as path components. */ private static final Pattern SAFE_NAME = Pattern.compile("[a-zA-Z0-9._\\-]+"); + /** + * Fully-qualified Java class name pattern. + * Matches names like {@code com.fronzec.plugins.ticketpdf.TicketPdfJobPlugin}. + * Requires at least one package segment followed by a simple class name. + */ + private static final Pattern FQCN = + Pattern.compile("([a-zA-Z_$][a-zA-Z0-9_$]*\\.)+[a-zA-Z_$][a-zA-Z0-9_$]*"); + private final JobDefinitionRepository jobDefinitionRepository; private final Optional autoApproveConfig; private final JarSignatureVerifier jarSignatureVerifier; @@ -90,6 +98,7 @@ public JarUploadResponse uploadJar( try { validateJar(file); + validateMainClassName(mainClassName); String checksum = ChecksumUtil.computeSha256(file); @@ -172,6 +181,28 @@ private void validateJar(MultipartFile file) { } } + /** + * Validates that {@code mainClassName} is a fully-qualified Java class name. + * + *

Requires at least one package segment (e.g. {@code com.example.MyClass}). + * A bare simple name ({@code MyClass}) is rejected because a plugin entry-point + * without a package would conflict with the default package and is almost always + * an input error. + * + * @param mainClassName the class name to validate + * @throws InvalidJarException if the value is blank or does not match the FQCN pattern + */ + private void validateMainClassName(String mainClassName) { + if (mainClassName == null || mainClassName.isBlank()) { + throw new InvalidJarException("mainClassName must not be blank"); + } + if (!FQCN.matcher(mainClassName).matches()) { + throw new InvalidJarException( + "mainClassName must be a fully-qualified Java class name " + + "(e.g. com.example.MyClass): " + mainClassName); + } + } + /** Reads up to {@code n} bytes from the beginning of the file. */ private byte[] readHeader(MultipartFile file, int n) { try (InputStream in = file.getInputStream()) { diff --git a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/config/SecurityConfig.java b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/config/SecurityConfig.java index 8939f0e..4314a17 100644 --- a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/config/SecurityConfig.java +++ b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/config/SecurityConfig.java @@ -31,6 +31,7 @@ *

  • {@code GET /jobs/plugins} — public (plugin discovery)
    • *
  • {@code GET /jobs/**} — {@code PLUGIN_VIEWER} or {@code PLUGIN_ADMIN}
    • *
  • {@code POST /jobs/**}, {@code PUT /jobs/**}, {@code DELETE /jobs/**} — {@code PLUGIN_ADMIN}
    • + *
  • {@code POST /data/**} — {@code PLUGIN_ADMIN} (destructive data-reset; not active in production)
    • *
  • {@code /actuator/health} — public
    • * * @@ -64,6 +65,8 @@ public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Excepti .hasAnyRole("PLUGIN_VIEWER", "PLUGIN_ADMIN") .requestMatchers("/jobs/**") .hasRole("PLUGIN_ADMIN") + .requestMatchers("/data/**") + .hasRole("PLUGIN_ADMIN") .anyRequest() .authenticated()) .httpBasic(Customizer.withDefaults()); diff --git a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/config/SecurityProperties.java b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/config/SecurityProperties.java index a203979..e95b2e8 100644 --- a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/config/SecurityProperties.java +++ b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/config/SecurityProperties.java @@ -4,25 +4,35 @@ import org.springframework.boot.context.properties.ConfigurationProperties; /** - * Externalised security credentials from {@code application.properties}. + * Externalised security credentials bound from {@code app.security.*} properties. * - *

    Note: Passwords are stored in plain text ({@code {noop}}) for development. - * Migrate to {@code {bcrypt}} hashes before production deployment. + *

    Passwords have NO Java-level default. Each active profile must supply its own credentials: + *

    + * + *

    This is deliberate: a missing password is fail-safe. With no active profile (a + * misconfiguration) the password is {@code null}, {@link SecurityConfig} fails to build the + * {@code UserDetails} ({@code password cannot be null}), and the app refuses to start rather + * than booting with a known insecure default. Tests that load {@link SecurityConfig} in + * isolation must supply explicit credentials via the context runner's property values. */ @ConfigurationProperties("app.security") public class SecurityProperties { - /** Username for the admin role (PLUGIN_ADMIN). */ + /** Username for the admin role (PLUGIN_ADMIN). Defaults to {@code "admin"}. */ private String adminUser = "admin"; - /** Password for the admin user — plain text in dev, bcrypt hash in production. */ - private String adminPassword = "{noop}admin123"; + /** Password for the admin user. No default — supplied per active profile (fail-safe if absent). */ + private String adminPassword; - /** Username for the viewer role (PLUGIN_VIEWER). */ + /** Username for the viewer role (PLUGIN_VIEWER). Defaults to {@code "viewer"}. */ private String viewerUser = "viewer"; - /** Password for the viewer user — plain text in dev, bcrypt hash in production. */ - private String viewerPassword = "{noop}viewer123"; + /** Password for the viewer user. No default — supplied per active profile (fail-safe if absent). */ + private String viewerPassword; public String getAdminUser() { return adminUser; diff --git a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/web/JobManagementController.java b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/web/JobManagementController.java index bdf29f9..07666e6 100644 --- a/fr-batch-service/src/main/java/com/fronzec/frbatchservice/web/JobManagementController.java +++ b/fr-batch-service/src/main/java/com/fronzec/frbatchservice/web/JobManagementController.java @@ -203,6 +203,12 @@ public ResponseEntity disableDefinition(@PathVariable long id) { * Approves a job definition, setting its status to {@code APPROVED} and recording * the approver and timestamp. * + *

    The approver identity is always derived from the authenticated principal + * (via {@link AuditService#currentUserId()}). Any {@code approvedBy} field + * supplied in the request body is accepted for backward compatibility with existing + * scripts (e.g. {@code load-plugins.hurl}) but is intentionally ignored — it + * cannot be trusted because callers can forge any value. + * *

    Returns {@code 200 OK} with the updated definition on success. * {@code 404 Not Found} if the definition does not exist. * {@code 409 Conflict} if the definition is already approved. @@ -210,7 +216,7 @@ public ResponseEntity disableDefinition(@PathVariable long id) { @PutMapping("/definitions/{id}/approve") public ResponseEntity approveDefinition( @PathVariable long id, - @RequestBody ApprovalRequest request) { + @RequestBody(required = false) ApprovalRequest request) { Optional opt = jobDefinitionRepository.findById(id); if (opt.isEmpty()) { log.warn("Cannot approve — job definition not found: id={}", id); @@ -226,22 +232,25 @@ public ResponseEntity approveDefinition( "message", "Job definition " + id + " is already approved")); } + // Derive the approver from the authenticated principal, not from the request body. + // The body field is kept for backward-compat deserialization but its value is discarded. + String approver = AuditService.currentUserId(); entity.setApprovalStatus("APPROVED"); - entity.setApprovedBy(request.approvedBy()); + entity.setApprovedBy(approver); entity.setApprovedAt(LocalDateTime.now()); JobDefinitionEntity saved = jobDefinitionRepository.save(entity); log.info( "Job definition approved: id={}, jobName={}, approvedBy={}", saved.getId(), saved.getJobName(), - request.approvedBy()); + approver); auditService.logEvent( new AuditEvent( AuditEventType.JOB_APPROVED, saved.getJobName(), - AuditService.currentUserId(), - "Approved by " + request.approvedBy(), + approver, + "Approved by " + approver, AuditEvent.SUCCESS, LocalDateTime.now())); @@ -252,36 +261,44 @@ public ResponseEntity approveDefinition( * Rejects a job definition, setting its status to {@code REJECTED} and recording * the reviewer and timestamp. * + *

    The reviewer identity is always derived from the authenticated principal + * (via {@link AuditService#currentUserId()}). Any {@code approvedBy} field + * supplied in the request body is accepted for backward compatibility but is + * intentionally ignored — it cannot be trusted because callers can forge any value. + * *

    Returns {@code 200 OK} with the updated definition on success. * {@code 404 Not Found} if the definition does not exist. */ @PutMapping("/definitions/{id}/reject") public ResponseEntity rejectDefinition( @PathVariable long id, - @RequestBody ApprovalRequest request) { + @RequestBody(required = false) ApprovalRequest request) { Optional opt = jobDefinitionRepository.findById(id); if (opt.isEmpty()) { log.warn("Cannot reject — job definition not found: id={}", id); return ResponseEntity.notFound().build(); } + // Derive the reviewer from the authenticated principal, not from the request body. + // The body field is kept for backward-compat deserialization but its value is discarded. + String reviewer = AuditService.currentUserId(); JobDefinitionEntity entity = opt.get(); entity.setApprovalStatus("REJECTED"); - entity.setApprovedBy(request.approvedBy()); + entity.setApprovedBy(reviewer); entity.setApprovedAt(LocalDateTime.now()); JobDefinitionEntity saved = jobDefinitionRepository.save(entity); log.info( "Job definition rejected: id={}, jobName={}, rejectedBy={}", saved.getId(), saved.getJobName(), - request.approvedBy()); + reviewer); auditService.logEvent( new AuditEvent( AuditEventType.JOB_REJECTED, saved.getJobName(), - AuditService.currentUserId(), - "Rejected by " + request.approvedBy(), + reviewer, + "Rejected by " + reviewer, AuditEvent.SUCCESS, LocalDateTime.now())); diff --git a/fr-batch-service/src/main/resources/application-production.properties b/fr-batch-service/src/main/resources/application-production.properties index db23ea3..c72a3e7 100644 --- a/fr-batch-service/src/main/resources/application-production.properties +++ b/fr-batch-service/src/main/resources/application-production.properties @@ -7,7 +7,7 @@ # Spring Web Configuration #--------------------- server.servlet.context-path=/api/batch-service -management.endpoints.web.exposure.include=health,info,env,prometheus +management.endpoints.web.exposure.include=health,info,prometheus management.endpoint.health.group.plugins.include=pluginHealthIndicator # Validate that health group members exist (catches misconfiguration) management.endpoint.health.validate-group-membership=true diff --git a/fr-batch-service/src/main/resources/application.properties b/fr-batch-service/src/main/resources/application.properties index 7da0f0c..043bb82 100644 --- a/fr-batch-service/src/main/resources/application.properties +++ b/fr-batch-service/src/main/resources/application.properties @@ -2,7 +2,9 @@ # Spring Web Configuration #--------------------- server.servlet.context-path=/api/batch-service -management.endpoints.web.exposure.include=health,info,env,prometheus +# Do NOT expose `env` here: it dumps resolved properties (incl. credentials) and +# this base value is inherited by any profile that does not override it. +management.endpoints.web.exposure.include=health,info,prometheus management.endpoint.health.group.plugins.include=pluginHealthIndicator #>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> # Jackson Object Mapper Configuration @@ -99,14 +101,16 @@ fr-batch-service.jobs.job2.chunk-size=10 fr-batch-service.jobs.job2.retry.max-attempts=3 fr-batch-service.jobs.job2.retry.backoff-millis=2000 #>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> -# Security Configuration (HTTP Basic, dev plain-text passwords) -# Note: {noop} prefix documents that passwords are plain text. -# Migrate to {bcrypt} hashes before production deployment. +# Security Configuration (HTTP Basic) +# Passwords have NO defaults here — each active profile MUST supply them: +# local: application-local.properties (admin123 / viewer123, noop) +# docker: application-docker.properties (admin123 / viewer123, noop) +# production: application-production.properties (bcrypt hashes via env vars) +# Usernames default to admin/viewer as a convenience; passwords are intentionally +# absent so the app fails to start rather than running with leaked plain-text defaults. #--------------------- app.security.admin-user=admin -app.security.admin-password={noop}admin123 app.security.viewer-user=viewer -app.security.viewer-password={noop}viewer123 #>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> # Rest clients configs #--------------------- diff --git a/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoadingIntegrationTest.java b/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoadingIntegrationTest.java index 5eaba6f..72bcb53 100644 --- a/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoadingIntegrationTest.java +++ b/fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoadingIntegrationTest.java @@ -236,6 +236,10 @@ void enableDefinition_setsEnabledTrue() throws Exception { @Test @Order(3) void approveDefinition_setsApproved() throws Exception { + // The approvedBy field in the request body is accepted for backward compatibility + // but intentionally ignored — the controller derives the approver from the + // authenticated principal (AuditService.currentUserId()). In the test context + // there is no authenticated user, so the fallback "system" is recorded. mockMvc .perform( put(JOBS_BASE + "/definitions/" + definitionId + "/approve") @@ -243,7 +247,7 @@ void approveDefinition_setsApproved() throws Exception { .content("{\"approved_by\":\"test-user\"}")) .andExpect(status().isOk()) .andExpect(jsonPath("$.approval_status").value("APPROVED")) - .andExpect(jsonPath("$.approved_by").value("test-user")) + .andExpect(jsonPath("$.approved_by").value("system")) .andExpect(jsonPath("$.approved_at").exists()); } diff --git a/fr-batch-service/src/test/java/com/fronzec/frbatchservice/config/SecurityConfigPasswordEncoderProfileTest.java b/fr-batch-service/src/test/java/com/fronzec/frbatchservice/config/SecurityConfigPasswordEncoderProfileTest.java index c040e7a..1f9b1c4 100644 --- a/fr-batch-service/src/test/java/com/fronzec/frbatchservice/config/SecurityConfigPasswordEncoderProfileTest.java +++ b/fr-batch-service/src/test/java/com/fronzec/frbatchservice/config/SecurityConfigPasswordEncoderProfileTest.java @@ -24,9 +24,16 @@ class SecurityConfigPasswordEncoderProfileTest { // SecurityConfig is annotated with @EnableWebSecurity, which imports the // HttpSecurity infrastructure — no SecurityAutoConfiguration needed here. + // SecurityProperties has no Java-level password defaults (fail-safe by design), so the + // isolated web slice must supply explicit credentials or UserDetails building would fail. + private static final String[] TEST_CREDENTIALS = { + "app.security.admin-password={noop}test-admin", "app.security.viewer-password={noop}test-viewer" + }; + private WebApplicationContextRunner runnerWithProfiles(String... profiles) { return new WebApplicationContextRunner() .withUserConfiguration(SecurityConfig.class) + .withPropertyValues(TEST_CREDENTIALS) .withInitializer(ctx -> ctx.getEnvironment().setActiveProfiles(profiles)); } @@ -73,6 +80,7 @@ void defaultProfile_hasSingleNoOpEncoder() { // No profile active at all — the !production NoOp bean must still be the only one. new WebApplicationContextRunner() .withUserConfiguration(SecurityConfig.class) + .withPropertyValues(TEST_CREDENTIALS) .run( context -> { assertThat(context).hasNotFailed();