Merge pull request from GHSA-m8v7-469p-5x89

MODRS-174: Hard-coded system user credentials

README.md

@@ -15,6 +15,7 @@ The mod-remote-storage module provides API for:
   * adapter modules
 
 ## Additional information
+The `system-user` system user for running tasks is created in the post tenant API controller. The password must be set using the `SYSTEM_USER_PASSWORD` environment variable. Permissions are defined in `src/main/resources/permissions/system-user-permissions.csv`.
 
 API provides the following URLs for working with remote storage configurations:
 
@@ -66,24 +67,26 @@ API provides the following URL to mark item as missing:
 
 ### Environment variables:
 
-| Name                          | Default value             | Description                                                       |
-| :-----------------------------| :------------------------:|:------------------------------------------------------------------|
-| JAVA_OPTIONS                  | -XX:MaxRAMPercentage=75.0 | Java options                                                 |
-| DB_HOST                       | postgres                  | Postgres hostname                                                 |
-| DB_PORT                       | 5432                      | Postgres port                                                     |
-| DB_USERNAME                   | folio_admin               | Postgres username                                                 |
-| DB_PASSWORD                   | -                         | Postgres username password                                        |
-| DB_DATABASE                   | okapi_modules             | Postgres database name                                            |
-| DB_QUERYTIMEOUT               | 60000                     | Database query timeout. |
-| DB_CHARSET                    | UTF-8                     | Database charset. |
-| DB_MAXPOOLSIZE                | 5                         | Database max pool size. |
-| KAFKA_HOST                    | kafka                     | Kafka broker hostname                                             |
-| KAFKA_PORT                    | 9092                      | Kafka broker port                                                 |
-| KAFKA_SECURITY_PROTOCOL       | PLAINTEXT                 | Kafka security protocol used to communicate with brokers (SSL or PLAINTEXT) |
-| KAFKA_SSL_KEYSTORE_LOCATION   | -                         | The location of the Kafka key store file. This is optional for client and can be used for two-way authentication for client. |
-| KAFKA_SSL_KEYSTORE_PASSWORD   | -                         | The store password for the Kafka key store file. This is optional for client and only needed if 'ssl.keystore.location' is configured. |
-| KAFKA_SSL_TRUSTSTORE_LOCATION | -                         | The location of the Kafka trust store file. |
-| KAFKA_SSL_TRUSTSTORE_PASSWORD | -                         | The password for the Kafka trust store file. If a password is not set, trust store file configured will still be used, but integrity checking is disabled. |
+| Name                          |       Default value       | Description                                                                                                                                                |
+|:------------------------------|:-------------------------:|:-----------------------------------------------------------------------------------------------------------------------------------------------------------|
+| JAVA_OPTIONS                  | -XX:MaxRAMPercentage=75.0 | Java options                                                                                                                                               |
+| DB_HOST                       |         postgres          | Postgres hostname                                                                                                                                          |
+| DB_PORT                       |           5432            | Postgres port                                                                                                                                              |
+| DB_USERNAME                   |        folio_admin        | Postgres username                                                                                                                                          |
+| DB_PASSWORD                   |             -             | Postgres username password                                                                                                                                 |
+| DB_DATABASE                   |       okapi_modules       | Postgres database name                                                                                                                                     |
+| DB_QUERYTIMEOUT               |           60000           | Database query timeout.                                                                                                                                    |
+| DB_CHARSET                    |           UTF-8           | Database charset.                                                                                                                                          |
+| DB_MAXPOOLSIZE                |             5             | Database max pool size.                                                                                                                                    |
+| KAFKA_HOST                    |           kafka           | Kafka broker hostname                                                                                                                                      |
+| KAFKA_PORT                    |           9092            | Kafka broker port                                                                                                                                          |
+| KAFKA_SECURITY_PROTOCOL       |         PLAINTEXT         | Kafka security protocol used to communicate with brokers (SSL or PLAINTEXT)                                                                                |
+| KAFKA_SSL_KEYSTORE_LOCATION   |             -             | The location of the Kafka key store file. This is optional for client and can be used for two-way authentication for client.                               |
+| KAFKA_SSL_KEYSTORE_PASSWORD   |             -             | The store password for the Kafka key store file. This is optional for client and only needed if 'ssl.keystore.location' is configured.                     |
+| KAFKA_SSL_TRUSTSTORE_LOCATION |             -             | The location of the Kafka trust store file.                                                                                                                |
+| KAFKA_SSL_TRUSTSTORE_PASSWORD |             -             | The password for the Kafka trust store file. If a password is not set, trust store file configured will still be used, but integrity checking is disabled. |
+| SYSTEM\_USER\_NAME            |        system-user        | Username of the system user.                                                                                                                               |
+| SYSTEM\_USER\_PASSWORD        |             -             | Password of the system user.                                                                                                                               |
 
 ### Required Permissions
 Institutional users should be granted the following permissions in order to use this remote storage API:

descriptors/ModuleDescriptor-template.json

@@ -319,6 +319,7 @@
             "users.item.post",
             "users.item.put",
             "login.item.post",
+            "login.item.delete",
             "perms.users.item.post",
             "perms.users.assign.immutable",
             "pubsub.event-types.post",

src/main/java/org/folio/rs/ModRemoteStorageApplication.java

@@ -1,5 +1,6 @@
 package org.folio.rs;
 
+import org.apache.commons.lang3.StringUtils;
 import org.springframework.boot.SpringApplication;
 import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
 import org.springframework.boot.autoconfigure.SpringBootApplication;
@@ -11,7 +12,11 @@
 @EnableFeignClients
 @EnableAutoConfiguration
 public class ModRemoteStorageApplication {
+  public static final String SYSTEM_USER_PASSWORD = "SYSTEM_USER_PASSWORD";
   public static void main(String[] args) {
+    if (StringUtils.isEmpty(System.getenv(SYSTEM_USER_PASSWORD))) {
+      throw new IllegalArgumentException("Required environment variable is missing: " + SYSTEM_USER_PASSWORD);
+    }
     SpringApplication.run(ModRemoteStorageApplication.class, args);
   }
 }

src/main/java/org/folio/rs/client/AuthnClient.java

@@ -4,9 +4,11 @@
 import org.springframework.cloud.openfeign.FeignClient;
 import org.springframework.http.MediaType;
 import org.springframework.http.ResponseEntity;
+import org.springframework.web.bind.annotation.DeleteMapping;
 import org.springframework.web.bind.annotation.PostMapping;
 import org.springframework.web.bind.annotation.RequestBody;
 import org.springframework.web.bind.annotation.RequestHeader;
+import org.springframework.web.bind.annotation.RequestParam;
 
 import static org.folio.spring.integration.XOkapiHeaders.TENANT;
 
@@ -19,4 +21,7 @@ public interface AuthnClient {
   @PostMapping(value = "/credentials", consumes = MediaType.APPLICATION_JSON_VALUE)
   void saveCredentials(@RequestBody SystemUserParameters systemUserParameters);
 
+  @DeleteMapping(value = "/credentials", consumes = MediaType.APPLICATION_JSON_VALUE)
+  void deleteCredentials(@RequestParam("userId") String userId);
+
 }

src/main/java/org/folio/rs/controller/TenantController.java

@@ -32,6 +32,7 @@
 import org.folio.tenant.domain.dto.Parameter;
 import org.folio.tenant.domain.dto.TenantAttributes;
 import org.folio.tenant.rest.resource.TenantApi;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.http.ResponseEntity;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RestController;
@@ -62,7 +63,10 @@ public class TenantController implements TenantApi {
   private final List<String> retrievalQueueSamples = List.of("retrieval_queue_record.json", "retrieval_queue_record_for_caia_soft.json");
   private final List<String> accessionQueueSamples = Collections.singletonList("accession_queue_record.json");
 
-  public static final String SYSTEM_USER = "system-user";
+  @Value("${remote-storage.system-user.username}")
+  private String username;
+  @Value("${remote-storage.system-user.password}")
+  private String password;
 
 
 
@@ -115,7 +119,7 @@ public ResponseEntity<Void> deleteTenant(String operationId) {
 
   private void initializeSystemUser(String tenantId) {
     try {
-      securityManagerService.prepareOrUpdateSystemUser(SYSTEM_USER, SYSTEM_USER, context.getOkapiUrl(), tenantId);
+      securityManagerService.prepareOrUpdateSystemUser(username, password, context.getOkapiUrl(), tenantId);
     } catch (Exception e) {
       log.error("Error initializing System User", e);
     }

src/main/java/org/folio/rs/integration/KafkaMessageListener.java

@@ -1,7 +1,5 @@
 package org.folio.rs.integration;
 
-import static org.folio.rs.controller.TenantController.SYSTEM_USER;
-
 import java.util.List;
 
 import org.folio.HttpStatus;
@@ -10,6 +8,7 @@
 import org.folio.rs.service.KafkaService;
 import org.folio.rs.service.SecurityManagerService;
 import org.folio.spring.FolioExecutionContext;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.kafka.annotation.KafkaListener;
 import org.springframework.stereotype.Component;
 
@@ -26,6 +25,12 @@ public class KafkaMessageListener {
   private final SecurityManagerService securityManagerService;
   private final FolioExecutionContext folioExecutionContext;
 
+  @Value("${remote-storage.system-user.username}")
+  private String username;
+
+  @Value("${remote-storage.system-user.password}")
+  private String password;
+
   @KafkaListener(id = KafkaService.EVENT_LISTENER_ID, containerFactory = "kafkaListenerContainerFactory", topicPattern = "${application.kafka.listener.events.topic-pattern}", groupId = "${application.kafka.listener.events.group-id}", concurrency = "${application.kafka.listener.events.concurrency}")
   public void handleEvents(List<DomainEvent> events) {
     log.info("Processing resource events from kafka [eventsCount: {}]", events.size());
@@ -34,7 +39,7 @@ public void handleEvents(List<DomainEvent> events) {
     } catch (FeignException fe) {
       if (fe.status() == HttpStatus.HTTP_UNAUTHORIZED.toInt()) {
         log.warn("Re-authorization attempt due to: {}", fe.getMessage());
-        securityManagerService.refreshSystemUserApiKey(SYSTEM_USER, SYSTEM_USER, folioExecutionContext.getOkapiUrl(), folioExecutionContext.getTenantId());
+        securityManagerService.refreshSystemUserApiKey(username, password, folioExecutionContext.getOkapiUrl(), folioExecutionContext.getTenantId());
         accessionQueueService.processAccessionQueueRecord(events);
       } else {
         log.error("Error processing Kafka event", fe);

src/main/java/org/folio/rs/service/SecurityManagerService.java

@@ -53,15 +53,21 @@ public void prepareOrUpdateSystemUser(String username, String password, String o
     var systemUserParameters = buildDefaultSystemUserParameters(username, password, okapiUrl, tenantId);
 
     var folioUser = getFolioUser(username);
-
+    String userId = null;
     if (folioUser.isPresent()) {
       updateUser(folioUser.get());
-      addPermissions(folioUser.get().getId());
+      userId = folioUser.get().getId();
+      addPermissions(userId);
     } else {
-      var userId = createFolioUser(username);
-      saveCredentials(systemUserParameters);
+      userId = createFolioUser(username);
       assignPermissions(userId);
     }
+    try {
+      deleteCredentials(userId);
+    } catch (feign.FeignException.NotFound e) {
+      // ignore if not exist
+    }
+    saveCredentials(systemUserParameters);
     updateApiKey(systemUserParameters);
   }
 
@@ -141,6 +147,12 @@ private void saveCredentials(SystemUserParameters systemUserParameters) {
 
     log.info("Saved credentials for user: [{}]", systemUserParameters.getUsername());
   }
+  public void deleteCredentials(String userId) {
+    authnClient.deleteCredentials(userId);
+
+    log.info("Removed credentials for user {}.", userId);
+  }
+
 
   private boolean assignPermissions(String userId) {
     List<String> perms = readPermissionsFromResource(PERMISSIONS_FILE_PATH);

src/main/resources/application.yml

@@ -79,3 +79,7 @@ application:
 
 server.port: 8081
 folio.tenant.validation.enabled: false
+remote-storage:
+  system-user:
+    username: ${SYSTEM_USER_NAME:system-user}
+    password: ${SYSTEM_USER_PASSWORD}

src/test/java/org/folio/rs/ModRemoteStorageApplicationTest.java

@@ -0,0 +1,16 @@
+package org.folio.rs;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertThrows;
+
+import org.junit.jupiter.api.Test;
+
+class ModRemoteStorageApplicationTest {
+
+  @Test
+  void exceptionOnMissingSystemUserPassword() {
+    var e = assertThrows(IllegalArgumentException.class, () -> ModRemoteStorageApplication.main(null));
+    assertThat(e.getMessage(), containsString(ModRemoteStorageApplication.SYSTEM_USER_PASSWORD));
+  }
+
+}

src/test/java/org/folio/rs/TestBase.java

@@ -35,7 +35,7 @@
 
 @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
 @TestPropertySource("classpath:application-test.yml")
-@ActiveProfiles("TestDB")
+@ActiveProfiles("test")
 @EnableTransactionManagement
 @AutoConfigureEmbeddedDatabase(beanName = "dataSource")
 @Log4j2
@@ -64,7 +64,6 @@ void setUp() {
         .addParametersItem(new Parameter().key(PARAMETER_LOAD_SAMPLE).value("true")));
     }
   }
-
   public static String getOkapiUrl() {
     return String.format("http://localhost:%s", WIRE_MOCK_PORT);
   }

src/test/java/org/folio/rs/controller/TenantControllerTest.java

@@ -15,11 +15,13 @@
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.boot.test.mock.mockito.SpyBean;
 import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.test.context.ActiveProfiles;
 import org.springframework.test.context.TestPropertySource;
 
 @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
 @TestPropertySource("classpath:application-test.yml")
 @AutoConfigureEmbeddedDatabase(beanName = "dataSource")
+@ActiveProfiles("test")
 public class TenantControllerTest {
   private final static String DROP_SCHEMA_QUERY = "DROP SCHEMA IF EXISTS %1$s CASCADE; DROP ROLE IF EXISTS %1$s";
 

src/test/java/org/folio/rs/service/SecurityManagerServiceTest.java

@@ -1,17 +1,26 @@
 package org.folio.rs.service;
 
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.deleteRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
 import static java.util.stream.Collectors.toList;
-import static org.folio.rs.controller.TenantController.SYSTEM_USER;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasItems;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import com.github.tomakehurst.wiremock.client.WireMock;
 import java.util.List;
 
 import org.folio.rs.TestBase;
 import org.folio.spring.FolioModuleMetadata;
+import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.MediaType;
 
 public class SecurityManagerServiceTest extends TestBase {
 
@@ -27,6 +36,11 @@ public class SecurityManagerServiceTest extends TestBase {
   public static final String NON_EXISTED_USER = "non_existed_user";
   public static final String NON_PRESENTED_USER = "non_presented_user";
 
+  @Value("${remote-storage.system-user.username}")
+  private String username;
+
+  @Value("${remote-storage.system-user.password}")
+  private String password;
   @Test
   void testCreateDefaultSystemUser() {
     try (var context = getFolioExecutionContextSetter()) {
@@ -37,12 +51,12 @@ void testCreateDefaultSystemUser() {
   }
 
   @Test
-  void testOverrideDefaultSystemUser() {
+  void testOverrideDefaultSystemUser() throws NoSuchFieldException, IllegalAccessException {
     try (var context = getFolioExecutionContextSetter()) {
       var originalSystemUserParameters = securityManagerService.getSystemUserParameters(TEST_TENANT);
 
       final var newOkapiUrl = "http://new-okapi-url";
-      securityManagerService.prepareOrUpdateSystemUser(SYSTEM_USER, SYSTEM_USER, newOkapiUrl, TEST_TENANT);
+      securityManagerService.prepareOrUpdateSystemUser(username, password, newOkapiUrl, TEST_TENANT);
       var updatedSystemUserParameters = securityManagerService.getSystemUserParameters(TEST_TENANT);
 
       assertEquals(originalSystemUserParameters.getId(), updatedSystemUserParameters.getId());
@@ -82,4 +96,22 @@ void testRefreshSystemUserApiKey() {
       assertThat(paths, hasItems("/authn/login"));
     }
   }
+
+  @Test
+  @DisplayName("Update user without previous password")
+  void prepareOrUpdateSystemUserWithoutPreviousPassword() {
+    wireMockServer.stubFor(
+      WireMock.delete(urlEqualTo("/authn/credentials?userId=c78aa9ec-b7d3-4d53-9e43-20296f39b496"))
+        .willReturn(
+          aResponse()
+            .withStatus(HttpStatus.NOT_FOUND.value())));
+    try (var context = getFolioExecutionContextSetter()) {
+      final var newOkapiUrl = "http://new-okapi-url";
+      securityManagerService.prepareOrUpdateSystemUser(EXISTED_USER, password, newOkapiUrl, TEST_TENANT);
+    }
+    wireMockServer.verify(
+      deleteRequestedFor(urlEqualTo("/authn/credentials?userId=c78aa9ec-b7d3-4d53-9e43-20296f39b496")));
+    wireMockServer.verify(
+      postRequestedFor(urlEqualTo("/authn/credentials")));
+  }
 }

src/test/resources/application-test.yml

@@ -32,3 +32,7 @@ logging:
 
 server.port: 8081
 folio.tenant.validation.enabled: true
+remote-storage:
+  system-user:
+    username: system-user
+    password: testpassword

src/test/resources/mappings/authn.json

@@ -62,6 +62,15 @@
         }
       }
     },
+    {
+      "request": {
+        "method": "DELETE",
+        "url": "/authn/credentials"
+      },
+      "response": {
+        "status": 204
+      }
+    },
     {
     "request": {
       "method": "POST",