[MODDATAIMP-947] Fix improper system user account management (#308)

* Add more messages surrounding system user

* Properly handle asynchronous authentication failures on creation

* Properly handle async token errors

src/main/java/org/folio/rest/impl/ModTenantAPI.java

@@ -46,10 +46,14 @@ public ModTenantAPI() {
 
   @Override
   Future<Integer> loadData(TenantAttributes attributes, String tenantId, Map<String, String> headers, Context context) {
-    if (fileSplittingEnabled) {
-      systemUserAuthService.initializeSystemUser(headers);
-    }
-    return super.loadData(attributes, tenantId, headers, context)
+    return Future.succeededFuture()
+      .compose(v -> {
+        if (fileSplittingEnabled) {
+          return systemUserAuthService.initializeSystemUser(headers);
+        }
+        return Future.succeededFuture();
+      })
+      .compose(v -> super.loadData(attributes, tenantId, headers, context))
       .compose(num -> {
         initStorageCleanupService(headers, context);
         return setupDefaultFileExtensions(headers).map(num);

src/main/java/org/folio/service/auth/AuthClient.java

@@ -7,6 +7,7 @@
 import lombok.Builder;
 import lombok.Data;
 import lombok.NoArgsConstructor;
+import lombok.ToString;
 import org.apache.http.client.methods.HttpDelete;
 import org.apache.http.client.methods.HttpPost;
 import org.apache.logging.log4j.LogManager;
@@ -99,7 +100,10 @@ public void deleteCredentials(OkapiConnectionParams params, String userId) {
   public static class LoginCredentials {
 
     private String username;
+
+    @ToString.Exclude
     private String password;
+
     private String tenant;
   }
 }

src/main/java/org/folio/service/auth/SystemUserAuthService.java

@@ -104,7 +104,7 @@ private void ensureSplittingEnabled() {
     }
   }
 
-  public void initializeSystemUser(Map<String, String> headers) {
+  public Future<Void> initializeSystemUser(Map<String, String> headers) {
     ensureSplittingEnabled();
 
     OkapiConnectionParams okapiConnectionParams = new OkapiConnectionParams(
@@ -114,22 +114,24 @@ public void initializeSystemUser(Map<String, String> headers) {
 
     User user = getOrCreateSystemUserFromApi(okapiConnectionParams);
     validatePermissions(okapiConnectionParams, user);
-    try {
-      getAuthToken(okapiConnectionParams);
-    } catch (NoSuchElementException e) {
-      LOGGER.error("Could not get auth token: ", e);
+    return getAuthToken(okapiConnectionParams)
+      .<Void>mapEmpty()
+      .recover(err -> {
+        LOGGER.error("Could not get auth token: ", err);
 
-      LOGGER.info(
-        "This may be due to a password change...resetting system user password"
-      );
-
-      recoverSystemUserAfterPasswordChange(okapiConnectionParams, user);
-    }
+        LOGGER.info(
+          "This may be due to a password change...resetting system user password"
+        );
 
-    LOGGER.info("System user created/found successfully!");
+        return recoverSystemUserAfterPasswordChange(
+          okapiConnectionParams,
+          user
+        );
+      })
+      .onSuccess(v -> LOGGER.info("System user created/found successfully!"));
   }
 
-  protected void recoverSystemUserAfterPasswordChange(
+  protected Future<Void> recoverSystemUserAfterPasswordChange(
     OkapiConnectionParams params,
     User user
   ) {
@@ -147,7 +149,7 @@ protected void recoverSystemUserAfterPasswordChange(
     usersClient.updateUser(params, user);
 
     LOGGER.info("Verifying we can login...");
-    getAuthToken(params);
+    return getAuthToken(params).mapEmpty();
   }
 
   public Future<String> getAuthToken(
@@ -157,10 +159,12 @@ public Future<String> getAuthToken(
 
     LOGGER.info("Attempting {}", getLoginCredentials(okapiConnectionParams));
 
-    return authClient.login(
-      okapiConnectionParams,
-      getLoginCredentials(okapiConnectionParams)
-    );
+    return authClient
+      .login(okapiConnectionParams, getLoginCredentials(okapiConnectionParams))
+      .onFailure((Throwable err) ->
+        LOGGER.error("Unable to login as system user", err)
+      )
+      .onSuccess((String v) -> LOGGER.info("Logged in successfully!"));
   }
 
   protected User getOrCreateSystemUserFromApi(

src/test/java/org/folio/rest/AbstractRestTest.java

@@ -415,9 +415,9 @@ private static WireMockServer mockTenantUserCalls(String realUrl) {
     );
 
     tenantMockServer.stubFor(
-      post(urlPathEqualTo("/authn/login"))
+      post(urlPathEqualTo("/authn/login-with-expiry"))
         .willReturn(
-          okJson(JsonObject.of("okapiToken", "token").toString())
+          WireMock.created().withHeader("Set-Cookie", "folioAccessToken=result")
         )
     );
 

src/test/java/org/folio/service/auth/SystemUserAuthServiceTest.java

@@ -15,6 +15,8 @@
 import static org.mockito.Mockito.when;
 
 import io.vertx.core.Future;
+import io.vertx.ext.unit.TestContext;
+import io.vertx.ext.unit.junit.VertxUnitRunner;
 import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.Optional;
@@ -25,11 +27,12 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.springframework.core.io.ClassPathResource;
 import org.springframework.core.io.Resource;
 
-@RunWith(MockitoJUnitRunner.class)
+@RunWith(VertxUnitRunner.class)
 public class SystemUserAuthServiceTest {
 
   @Mock
@@ -52,6 +55,9 @@ public class SystemUserAuthServiceTest {
 
   @Before
   public void setup() {
+    // open mocks for class
+    MockitoAnnotations.openMocks(this);
+
     service =
       new SystemUserAuthServiceTestProxy(
         authClient,
@@ -320,7 +326,7 @@ public void testInvalidPermissionFileLoading() {
   }
 
   @Test
-  public void testChangedCredentials() {
+  public void testChangedCredentials(TestContext context) {
     User response = new User();
     response.setId("user-id");
 
@@ -335,9 +341,11 @@ public void testChangedCredentials() {
     // first attempt: invalid
     // second attempt (after reset): valid
     when(authClient.login(any(), any()))
-      .thenThrow(
-        new NoSuchElementException(
-          "test simulating the credentials were invalid"
+      .thenReturn(
+        Future.failedFuture(
+          new NoSuchElementException(
+            "test simulating the credentials were invalid"
+          )
         )
       )
       .thenReturn(Future.succeededFuture("test token"));
@@ -352,18 +360,23 @@ public void testChangedCredentials() {
       .when(usersClient)
       .updateUser(any(), any());
 
-    service.initializeSystemUser(Map.of("x-okapi-tenant", "tenant"));
-
-    verify(usersClient, times(1)).getUserByUsername(any(), eq("username"));
-    verify(permissionsClient, times(1))
-      .getPermissionsUserByUserId(any(), eq("user-id"));
-    verify(authClient, times(1)).deleteCredentials(any(), eq("user-id"));
-    verify(authClient, times(1)).saveCredentials(any(), any());
-    verify(usersClient, times(1)).updateUser(any(), any());
-    verify(authClient, times(2)).login(any(), any());
-    verifyNoMoreInteractions(authClient);
-    verifyNoMoreInteractions(permissionsClient);
-    verifyNoMoreInteractions(usersClient);
+    service
+      .initializeSystemUser(Map.of("x-okapi-tenant", "tenant"))
+      .onComplete(
+        context.asyncAssertSuccess(v -> {
+          verify(usersClient, times(1))
+            .getUserByUsername(any(), eq("username"));
+          verify(permissionsClient, times(1))
+            .getPermissionsUserByUserId(any(), eq("user-id"));
+          verify(authClient, times(1)).deleteCredentials(any(), eq("user-id"));
+          verify(authClient, times(1)).saveCredentials(any(), any());
+          verify(usersClient, times(1)).updateUser(any(), any());
+          verify(authClient, times(2)).login(any(), any());
+          verifyNoMoreInteractions(authClient);
+          verifyNoMoreInteractions(permissionsClient);
+          verifyNoMoreInteractions(usersClient);
+        })
+      );
   }
 
   // allow access to private methods