From d4cb2b9c16d28f3d6d18c208723eb29299ac81f5 Mon Sep 17 00:00:00 2001
From: Steve Ellis <sellis@ebsco.com>
Date: Fri, 21 Feb 2025 14:00:36 -0500
Subject: [PATCH 1/2] Removed legacy endpoint - RAML - Remove usage of
 PostBlUsersLoginResponse - Revise all tests to use expiry endpoin

TODO
Module desc
---
 ramls/mod-users-bl.raml                       |  32 ----
 .../java/org/folio/rest/impl/BLUsersAPI.java  |  28 +---
 .../org/folio/rest/HeadersForwardingTest.java | 149 +++++++-----------
 3 files changed, 63 insertions(+), 146 deletions(-)

diff --git a/ramls/mod-users-bl.raml b/ramls/mod-users-bl.raml
index c0b8c05..8911678 100644
--- a/ramls/mod-users-bl.raml
+++ b/ramls/mod-users-bl.raml
@@ -186,38 +186,6 @@ resourceTypes:
         type: { compositeOpenTransactionsResource: { "typeName" : "username" } }
   /_self:
     type: { compositeUserResource: { "typeName" : "self reference" } }
-  /login:
-    post:
-      description: Allow a new user to login and return an authtoken, along with a composite user record. Deprecated and will be removed in a future release. Please use /login-with-expiry.
-      is: [permissionsExpandable, includeable]
-      headers:
-        User-Agent:
-        X-Forwarded-For:
-      body:
-        application/json:
-          type: loginCredentials
-      responses:
-        201:
-          body:
-            application/json:
-              type: compositeUser
-          headers:
-            x-okapi-token:
-        400:
-          description: "Bad request"
-          body:
-            text/plain:
-              example: "Bad request"
-        422:
-          description: "Unprocessable Entity"
-          body:
-            application/json:
-              type: errors
-        500:
-          description: "Internal server error"
-          body:
-            text/plain:
-              example: "Internal server error"
   /login-with-expiry:
     post:
       description: |
diff --git a/src/main/java/org/folio/rest/impl/BLUsersAPI.java b/src/main/java/org/folio/rest/impl/BLUsersAPI.java
index 072f23f..f58e921 100644
--- a/src/main/java/org/folio/rest/impl/BLUsersAPI.java
+++ b/src/main/java/org/folio/rest/impl/BLUsersAPI.java
@@ -139,7 +139,6 @@ public class BLUsersAPI implements BlUsers {
   private static final String UNDEFINED_USER = "UNDEFINED_USER__";
 
   private static final String LOGIN_ENDPOINT = "/authn/login-with-expiry";
-  private static final String LOGIN_ENDPOINT_LEGACY = "/authn/login";
   private static final String FOLIO_ACCESS_TOKEN = "folioAccessToken";
   private static final String SET_COOKIE_HEADER = "Set-Cookie";
 
@@ -853,14 +852,6 @@ public void postBlUsersLoginWithExpiry(boolean expandPerms, List<String> include
         LOGIN_ENDPOINT, this::loginResponse);
   }
 
-  @Override
-  public void postBlUsersLogin(boolean expandPerms, List<String> include, String userAgent, String xForwardedFor,
-      LoginCredentials entity, Map<String, String> okapiHeaders, Handler<AsyncResult<javax.ws.rs.core.Response>> asyncResultHandler,
-      Context vertxContext) {
-    doPostBlUsersLogin(expandPerms, include, userAgent, xForwardedFor, entity, okapiHeaders, asyncResultHandler,
-        LOGIN_ENDPOINT_LEGACY, this::loginResponseLegacy);
-  }
-
   @SuppressWarnings("java:S1874")
   private void doPostBlUsersLogin(boolean expandPerms, List<String> include, String userAgent, String xForwardedFor, //NOSONAR
       LoginCredentials entity, Map<String, String> okapiHeaders, Handler<AsyncResult<javax.ws.rs.core.Response>> asyncResultHandler,
@@ -879,7 +870,7 @@ private void doPostBlUsersLogin(boolean expandPerms, List<String> include, Strin
 
     if (entity == null || entity.getUsername() == null || entity.getPassword() == null) {
       asyncResultHandler.handle(Future.succeededFuture(
-        PostBlUsersLoginResponse.respond400WithTextPlain("Improperly formatted request")));
+        PostBlUsersLoginWithExpiryResponse.respond400WithTextPlain("Improperly formatted request")));
     } else {
       HttpClientInterface clientForLogin = HttpClientFactory.getHttpClient(okapiURL, okapiHeaders.get(OKAPI_TENANT_HEADER));
       String moduleURL = "/authn/login";
@@ -913,7 +904,7 @@ private void doPostBlUsersLogin(boolean expandPerms, List<String> include, Strin
             } catch (Exception e) {
               client.closeClient();
               asyncResultHandler.handle(Future.succeededFuture(
-                PostBlUsersLoginResponse.respond500WithTextPlain(e.getLocalizedMessage())));
+                PostBlUsersLoginWithExpiryResponse.respond500WithTextPlain(e.getLocalizedMessage())));
             } finally {
               clientForLogin.closeClient();
             }
@@ -921,13 +912,13 @@ private void doPostBlUsersLogin(boolean expandPerms, List<String> include, Strin
           .exceptionally(throwable -> {
             clientForLogin.closeClient();
             asyncResultHandler.handle(Future.succeededFuture(
-              PostBlUsersLoginResponse.respond500WithTextPlain(throwable.getLocalizedMessage())));
+              PostBlUsersLoginWithExpiryResponse.respond500WithTextPlain(throwable.getLocalizedMessage())));
             return null;
           });
       } catch (Exception ex) {
         clientForLogin.closeClient();
         asyncResultHandler.handle(Future.succeededFuture(
-          PostBlUsersLoginResponse.respond500WithTextPlain(ex.getLocalizedMessage())));
+          PostBlUsersLoginWithExpiryResponse.respond500WithTextPlain(ex.getLocalizedMessage())));
       }
     }
   }
@@ -999,7 +990,7 @@ okapiHeaders, null, handlePreviousResponse(false, false, false,
           } catch (Exception ex) {
             client.closeClient();
             asyncResultHandler.handle(Future.succeededFuture(
-              PostBlUsersLoginResponse.respond500WithTextPlain(ex.getLocalizedMessage())));
+              PostBlUsersLoginWithExpiryResponse.respond500WithTextPlain(ex.getLocalizedMessage())));
           }
         }
       }
@@ -1084,7 +1075,7 @@ okapiHeaders, null, handlePreviousResponse(false, false, false,
         } catch (Exception e) {
           if(!aRequestHasFailed[0]){
             asyncResultHandler.handle(Future.succeededFuture(
-              PostBlUsersLoginResponse.respond500WithTextPlain(e.getLocalizedMessage())));
+              PostBlUsersLoginWithExpiryResponse.respond500WithTextPlain(e.getLocalizedMessage())));
           }
           logger.error(e.getMessage(), e);
         } finally {
@@ -1126,13 +1117,6 @@ private static void fillCompositeUserWithServicePoint(Map<String, CompletableFut
     }
   }
 
-  @SuppressWarnings("java:S1874")
-  private javax.ws.rs.core.Response loginResponseLegacy(Response loginResponse, CompositeUser cu) {
-    String token = String.valueOf(loginResponse.getHeaders().get(OKAPI_TOKEN_HEADER));
-    return PostBlUsersLoginResponse.respond201WithApplicationJson(cu,
-      PostBlUsersLoginResponse.headersFor201().withXOkapiToken(token));
-  }
-
   @SuppressWarnings("java:S1874")
   private javax.ws.rs.core.Response loginResponse(Response loginResponse, CompositeUser cu) {
     JsonObject body = loginResponse.getBody();
diff --git a/src/test/java/org/folio/rest/HeadersForwardingTest.java b/src/test/java/org/folio/rest/HeadersForwardingTest.java
index deda990..3935e70 100644
--- a/src/test/java/org/folio/rest/HeadersForwardingTest.java
+++ b/src/test/java/org/folio/rest/HeadersForwardingTest.java
@@ -72,9 +72,7 @@ public class HeadersForwardingTest {
 
   private static final String URL_AUT_RESET_PASSWORD = "/authn/reset-password";
   private static final String URL_AUTH_UPDATE = "/authn/update";
-  private static final String URL_AUTH_LOGIN_LEGACY = "/authn/login";
   private static final String URL_AUTH_LOGIN = "/authn/login-with-expiry";
-  private static final String BL_USERS_LOGIN_LEGACY = "/bl-users/login";
   private static final String BL_USERS_LOGIN = "/bl-users/login-with-expiry";
   private static final String REFRESH_TOKEN = "folioRefreshToken";
   private static final String ACCESS_TOKEN = "folioAccessToken";
@@ -108,7 +106,7 @@ public void setUp(TestContext context) {
   }
 
   @Test
-  public void testPostBlUsersLoginLegacy() {
+  public void testPostBlUsersLogin() {
     LoginCredentials credentials = new LoginCredentials();
     credentials.setUsername(USERNAME);
     credentials.setPassword("password");
@@ -125,73 +123,10 @@ public void testPostBlUsersLoginLegacy() {
       .withQueryParam("query", equalTo("username==\"" + USERNAME + "\""))
       .willReturn(WireMock.okJson(users.encode())));
 
-    WireMock.stubFor(post(URL_AUTH_LOGIN_LEGACY)
-      .willReturn(WireMock.okJson(ObjectMapperTool.valueAsString(credentials)).withStatus(201).withHeader(OKAPI_TOKEN_HEADER, getToken(USER_ID, USERNAME, TENANT))));
-
-    JsonObject permsUsersPost = new JsonObject()
-      .put("permissionUsers", new JsonArray().add(new JsonObject()));
-
-    WireMock.stubFor(get(urlPathEqualTo("/perms/users"))
-      .withQueryParam("query", equalTo("userId==" + USER_ID))
-      .willReturn(WireMock.okJson(permsUsersPost.encode()).withStatus(201)));
-
-    JsonObject jsonObject = new JsonObject()
-      .put("servicePointsUsers", new JsonArray());
-
-    WireMock.stubFor(get(urlPathEqualTo("/service-points-users"))
-        .withQueryParam("query", equalTo("userId==" + USER_ID))
-        .withQueryParam("limit", equalTo("1000"))
-        .willReturn(WireMock.okJson(jsonObject.encode()).withStatus(201)));
-
-    RestAssured
-      .given()
-      .spec(spec)
-      .header(new Header(BLUsersAPI.X_FORWARDED_FOR_HEADER, IP))
-      .body(ObjectMapperTool.valueAsString(credentials))
-      .when()
-      .post(BL_USERS_LOGIN_LEGACY)
-      .then()
-      .statusCode(201);
-
-    WireMock.verify(1, getRequestedFor(urlPathEqualTo("/users"))
-      .withQueryParam("query", equalTo("username==\"" + USERNAME + "\"")));
-
-    WireMock.verify(1, getRequestedFor(urlPathEqualTo("/perms/users"))
-      .withQueryParam("query", equalTo("userId==" + USER_ID)));
-
-    WireMock.verify(1, postRequestedFor(urlPathEqualTo(URL_AUTH_LOGIN_LEGACY)));
-
-    WireMock.verify(1, getRequestedFor(urlPathEqualTo("/service-points-users"))
-        .withQueryParam("query", equalTo("userId==" + USER_ID))
-        .withQueryParam("limit", equalTo("1000")));
-
-    WireMock.getAllServeEvents().stream()
-      .map(ServeEvent::getRequest)
-      .forEach(this::verifyHeaders);
-  }
-
-  @Test
-  public void testPostBlUsersLogin() {
-    LoginCredentials credentials = new LoginCredentials();
-    credentials.setUsername(USERNAME);
-    credentials.setPassword("password");
-
-    JsonObject user = new JsonObject()
-      .put("username", USERNAME)
-      .put("id", USER_ID);
-
-    JsonObject users = new JsonObject()
-      .put("users", new JsonArray().add(user))
-      .put("totalRecords", 1);
-
     JsonObject tokenExpiration = new JsonObject()
       .put("refreshTokenExpiration","987")
       .put("accessTokenExpiration", "456");
 
-    WireMock.stubFor(get(urlPathEqualTo("/users"))
-      .withQueryParam("query", equalTo("username==\"" + USERNAME + "\""))
-      .willReturn(WireMock.okJson(users.encode())));
-
     var accessToken = getToken(USER_ID, USERNAME, TENANT);
     var accessTokenCookie = Cookie.cookie(ACCESS_TOKEN, accessToken)
       .setMaxAge(321)
@@ -279,7 +214,7 @@ public void testPostBlUsersLoginWithNullResponseFromLogin() {
     credentials.setUsername(USERNAME);
     credentials.setPassword("password");
 
-    WireMock.stubFor(post(URL_AUTH_LOGIN_LEGACY)
+    WireMock.stubFor(post(URL_AUTH_LOGIN)
       .willReturn(null));
 
     RestAssured
@@ -288,11 +223,11 @@ public void testPostBlUsersLoginWithNullResponseFromLogin() {
       .header(new Header(BLUsersAPI.X_FORWARDED_FOR_HEADER, IP))
       .body(ObjectMapperTool.valueAsString(credentials))
       .when()
-      .post(BL_USERS_LOGIN_LEGACY)
+      .post(BL_USERS_LOGIN)
       .then()
       .statusCode(500);
 
-    WireMock.verify(1, postRequestedFor(urlPathEqualTo(URL_AUTH_LOGIN_LEGACY)));
+    WireMock.verify(1, postRequestedFor(urlPathEqualTo(URL_AUTH_LOGIN)));
   }
 
   @Test
@@ -301,8 +236,26 @@ public void testPostBlUsersLoginWithoutTenant() {
     credentials.setUsername(USERNAME);
     credentials.setPassword("password");
 
-    WireMock.stubFor(post(URL_AUTH_LOGIN_LEGACY)
-      .willReturn(WireMock.okJson(ObjectMapperTool.valueAsString(credentials)).withStatus(201).withHeader(OKAPI_TOKEN_HEADER, getTokenWithoutTenant(USER_ID, USERNAME))));
+    JsonObject tokenExpiration = new JsonObject()
+      .put("refreshTokenExpiration","987")
+      .put("accessTokenExpiration", "456");
+
+    var accessToken = getTokenWithoutTenant(USER_ID, USERNAME);
+    var accessTokenCookie = Cookie.cookie(ACCESS_TOKEN, accessToken)
+      .setMaxAge(321)
+      .setSecure(true)
+      .setHttpOnly(true);
+    var refreshTokenCookie = Cookie.cookie(REFRESH_TOKEN, "abc123")
+      .setMaxAge(123)
+      .setSecure(true)
+      .setPath("/authn")
+      .setHttpOnly(true);
+    WireMock.stubFor(post(URL_AUTH_LOGIN)
+      .willReturn(WireMock.okJson(ObjectMapperTool.valueAsString(credentials))
+        .withStatus(201)
+        .withHeader("Set-Cookie", accessTokenCookie.encode())
+        .withHeader("Set-Cookie", refreshTokenCookie.encode())
+        .withBody(tokenExpiration.encode())));
 
     WireMock.stubFor(get(urlPathEqualTo("/perms/users"))
       .withQueryParam("query", equalTo("userId==" + USER_ID))
@@ -314,11 +267,11 @@ public void testPostBlUsersLoginWithoutTenant() {
       .header(new Header(BLUsersAPI.X_FORWARDED_FOR_HEADER, IP))
       .body(ObjectMapperTool.valueAsString(credentials))
       .when()
-      .post(BL_USERS_LOGIN_LEGACY)
+      .post(BL_USERS_LOGIN)
       .then()
       .statusCode(500);
 
-    WireMock.verify(1, postRequestedFor(urlPathEqualTo(URL_AUTH_LOGIN_LEGACY)));
+    WireMock.verify(1, postRequestedFor(urlPathEqualTo(URL_AUTH_LOGIN)));
   }
 
   @Test
@@ -327,8 +280,25 @@ public void testPostBlUsersLoginWithNullToken() {
     credentials.setUsername(USERNAME);
     credentials.setPassword("password");
 
-    WireMock.stubFor(post(URL_AUTH_LOGIN_LEGACY)
-      .willReturn(WireMock.okJson(ObjectMapperTool.valueAsString(credentials)).withStatus(201).withHeader(OKAPI_TOKEN_HEADER, "")));
+    JsonObject tokenExpiration = new JsonObject()
+      .put("refreshTokenExpiration","987")
+      .put("accessTokenExpiration", "456");
+
+    var accessTokenCookie = Cookie.cookie(ACCESS_TOKEN, "") // Empty token
+      .setMaxAge(321)
+      .setSecure(true)
+      .setHttpOnly(true);
+    var refreshTokenCookie = Cookie.cookie(REFRESH_TOKEN, "abc123")
+      .setMaxAge(123)
+      .setSecure(true)
+      .setPath("/authn")
+      .setHttpOnly(true);
+    WireMock.stubFor(post(URL_AUTH_LOGIN)
+      .willReturn(WireMock.okJson(ObjectMapperTool.valueAsString(credentials))
+        .withStatus(201)
+        .withHeader("Set-Cookie", accessTokenCookie.encode())
+        .withHeader("Set-Cookie", refreshTokenCookie.encode())
+        .withBody(tokenExpiration.encode())));
 
     WireMock.stubFor(get(urlPathEqualTo("/perms/users"))
       .withQueryParam("query", equalTo("userId==" + USER_ID))
@@ -340,11 +310,11 @@ public void testPostBlUsersLoginWithNullToken() {
       .header(new Header(BLUsersAPI.X_FORWARDED_FOR_HEADER, IP))
       .body(ObjectMapperTool.valueAsString(credentials))
       .when()
-      .post(BL_USERS_LOGIN_LEGACY)
+      .post(BL_USERS_LOGIN)
       .then()
       .statusCode(500);
 
-    WireMock.verify(1, postRequestedFor(urlPathEqualTo(URL_AUTH_LOGIN_LEGACY)));
+    WireMock.verify(1, postRequestedFor(urlPathEqualTo(URL_AUTH_LOGIN)));
   }
 
   @Test
@@ -353,7 +323,7 @@ public void testPostBlUsersLoginWithError() {
     credentials.setUsername(USERNAME);
     credentials.setPassword("password");
 
-    WireMock.stubFor(post(URL_AUTH_LOGIN_LEGACY)
+    WireMock.stubFor(post(URL_AUTH_LOGIN)
       .willReturn(WireMock.okJson(JsonObject.mapFrom(credentials).encode()).withStatus(422)));
 
     RestAssured
@@ -362,11 +332,11 @@ public void testPostBlUsersLoginWithError() {
       .header(new Header(BLUsersAPI.X_FORWARDED_FOR_HEADER, IP))
       .body(ObjectMapperTool.valueAsString(credentials))
       .when()
-      .post(BL_USERS_LOGIN_LEGACY)
+      .post(BL_USERS_LOGIN)
       .then()
       .statusCode(422);
 
-    WireMock.verify(1, postRequestedFor(urlPathEqualTo(URL_AUTH_LOGIN_LEGACY)));
+    WireMock.verify(1, postRequestedFor(urlPathEqualTo(URL_AUTH_LOGIN)));
     WireMock.verify(0, postRequestedFor(urlPathEqualTo("/perms/users")));
   }
 
@@ -376,7 +346,7 @@ public void testPostBlUsersLoginNullResponse() {
     credentials.setUsername(USERNAME);
     credentials.setPassword("password");
 
-    WireMock.stubFor(post(URL_AUTH_LOGIN_LEGACY)
+    WireMock.stubFor(post(URL_AUTH_LOGIN)
       .willReturn(null));
 
     RestAssured
@@ -385,11 +355,11 @@ public void testPostBlUsersLoginNullResponse() {
       .header(new Header(BLUsersAPI.X_FORWARDED_FOR_HEADER, IP))
       .body(ObjectMapperTool.valueAsString(credentials))
       .when()
-      .post(BL_USERS_LOGIN_LEGACY)
+      .post(BL_USERS_LOGIN)
       .then()
       .statusCode(500);
 
-    WireMock.verify(1, postRequestedFor(urlPathEqualTo(URL_AUTH_LOGIN_LEGACY)));
+    WireMock.verify(1, postRequestedFor(urlPathEqualTo(URL_AUTH_LOGIN)));
     WireMock.verify(0, postRequestedFor(urlPathEqualTo("/perms/users")));
   }
 
@@ -405,7 +375,7 @@ public void testPostBlUsersLoginWithoutUsername() {
       .header(new Header(BLUsersAPI.X_FORWARDED_FOR_HEADER, IP))
       .body(ObjectMapperTool.valueAsString(credentials))
       .when()
-      .post("/bl-users/login")
+      .post(BL_USERS_LOGIN)
       .then()
       .statusCode(400);
 
@@ -425,7 +395,7 @@ public void testPostBlUsersLoginWithoutPassword() {
       .header(new Header(BLUsersAPI.X_FORWARDED_FOR_HEADER, IP))
       .body(ObjectMapperTool.valueAsString(credentials))
       .when()
-      .post("/bl-users/login")
+      .post(BL_USERS_LOGIN)
       .then()
       .statusCode(400);
 
@@ -438,11 +408,6 @@ public void testPostBlUsersLoginIncorrectPermissions() {
     doTestPostBlUsersLoginIncorrectPermissions(URL_AUTH_LOGIN, BL_USERS_LOGIN);
   }
 
-  @Test
-  public void testPostBlUsersLoginIncorrectPermissionsLegacy() {
-    doTestPostBlUsersLoginIncorrectPermissions(URL_AUTH_LOGIN_LEGACY, BL_USERS_LOGIN_LEGACY);
-  }
-
   private void doTestPostBlUsersLoginIncorrectPermissions(String authLoginEndpoint, String blUsersLoginEndpoint) {
     LoginCredentials credentials = new LoginCredentials();
     credentials.setUsername(USERNAME);
@@ -615,8 +580,7 @@ private void verifyHeaders(LoggedRequest request) {
 
   private boolean isContainsSpecifiedUrls(String url) {
     return url.contains(URL_AUT_RESET_PASSWORD)
-      || url.contains(URL_AUTH_UPDATE)
-      || url.contains(URL_AUTH_LOGIN_LEGACY);
+      || url.contains(URL_AUTH_UPDATE);
   }
 
   private StringValuePattern passwordValidateRequestMatcher() {
@@ -629,4 +593,5 @@ private StringValuePattern passwordValidateRequestMatcher() {
   private String toJson(Object object) {
     return ObjectMapperTool.valueAsString(object);
   }
+
 }

From 0534304814b9bc1c28364838f4b58aa99ed9791d Mon Sep 17 00:00:00 2001
From: Steve Ellis <sellis@ebsco.com>
Date: Fri, 21 Feb 2025 14:03:51 -0500
Subject: [PATCH 2/2] Update mod desc

---
 descriptors/ModuleDescriptor-template.json | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json
index c2e3c47..2190960 100644
--- a/descriptors/ModuleDescriptor-template.json
+++ b/descriptors/ModuleDescriptor-template.json
@@ -4,7 +4,7 @@
   "provides": [
     {
       "id": "users-bl",
-      "version": "6.1",
+      "version": "7.0",
       "handlers": [
         {
           "methods": ["GET"],
@@ -33,21 +33,6 @@
             "inventory-storage.service-points.item.get"
           ]
         },
-        {
-          "methods" : [ "POST" ],
-          "pathPattern" : "/bl-users/login",
-          "permissionsRequired" : [],
-          "modulePermissions" : [
-            "users.item.get",
-            "users.collection.get",
-            "perms.users.get",
-            "usergroups.item.get",
-            "inventory-storage.service-points-users.collection.get",
-            "inventory-storage.service-points-users.item.get",
-            "inventory-storage.service-points.collection.get",
-            "inventory-storage.service-points.item.get"
-          ]
-        },
         {
           "methods" : [ "POST" ],
           "pathPattern" : "/bl-users/login-with-expiry",
@@ -243,7 +228,7 @@
     },
     {
       "id" : "login",
-      "version" : "7.3"
+      "version" : "8.0"
     },
     {
       "id": "authtoken",