[PLAT-14672] getSessionInfo should return a valid api token

Summary: With D35345 change, YBA does not store the api token in plain text anymore. Only the hash of the api token is saved and used for auth verification. As part of D35345 the getSessionInfo was returning the hashed api token in the response. This is not useful for the end user. The workflow for getting started with YBA API is to first /api_login. This returns a SessionInfo response that contains a apiToken, customer uuid, and user uuid. This information can then used for subsequent API calls, with apiToken placed in X-AUTH-YW-API-TOKEN header. The API token will be refreshed when calling any of the unauthenticated APIs /login, /api_login, or the authenticated APIs /api_token, and /session_info. With this diff, the change in behaviour is: Old: getSessionInfo used to return the previously generated api token Now: getSessionInfo generates a new api token and returns it Clients (like YBM) that could invoke concurrent requests to fetch the api token should use the `/api_token??apiTokenVersion=$lastApiTokenVersion` to make sure that the latest valid api token is used in the current thread. Invoking /session_info will always generate a new api token, and if invoked concurrently, will leave only one of the invoking threads with a valid api token. So for example, this method in YBM needs to be updated to use `/api_token??apiTokenVersion=$lastApiTokenVersion`: ``` https://github.com/yugabyte/yugabyte-cloud/blob/acb47b1c01e138a726d339888652eebf45fb7df5/common/src/main/java/clients/platform/PlatformClientImpl.java#L436-L444 @OverRide public String generateApiKey(UUID customerUUID, boolean withRetries) throws RetryableException, InterruptedException { // Call the session_info endpoint which uses "user.getOrCreateApiToken()", and hence is safe for // out-of-order generate requests // Creates the apiToken if it isn't already created, else just returns it String url = String.format("%s/api/v1/session_info", hostname); JsonNode responseJson = get(url, true /* useAuthToken */, withRetries); return responseJson.get("apiToken").asText(); } ``` Test Plan: Manually tested the /session_info returns a valid api token that then works for a subsequent API call Reviewers: dkumar, amalyshev, cwang, anijhawan, #yba-api-review, amindrov, daniel, dnolan Reviewed By: dkumar, #yba-api-review Subscribers: yugaware Differential Revision: https://phorge.dev.yugabyte.com/D36560
yugabyte · Jul 17, 2024 · c8e7530 · c8e7530
1 parent 56d2d2d
commit c8e7530
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 34 deletions.
diff --git a/managed/src/main/java/com/yugabyte/yw/controllers/SessionController.java b/managed/src/main/java/com/yugabyte/yw/controllers/SessionController.java
@@ -193,7 +193,7 @@ public Result getSessionInfo(Http.Request request) {
     SessionInfo sessionInfo =
         new SessionInfo(
             authCookie.isPresent() ? authCookie.get().value() : null,
-            user.getOrCreateApiToken(),
+            user.upsertApiToken(),
             user.getApiTokenVersion(),
             cust.getUuid(),
             user.getUuid());
@@ -545,7 +545,7 @@ public Result insecure_login(Http.Request request) {
       if (user == null) {
         throw new PlatformServiceException(FORBIDDEN, "Invalid User saved.");
       }
-      String apiToken = user.getOrCreateApiToken();
+      String apiToken = user.upsertApiToken();
 
       SessionInfo sessionInfo =
           new SessionInfo(
@@ -589,7 +589,7 @@ public Result set_security(UUID customerUUID, Http.Request request) {
     configHelper.loadConfigToDB(Security, ImmutableMap.of("level", data.level));
     if (data.level.equals("insecure")) {
       Users user = CommonUtils.getUserFromContext();
-      user.getOrCreateApiToken();
+      user.upsertApiToken();
 
       try {
         InputStream featureStream = environment.resourceAsStream("ossFeatureConfig.json");

diff --git a/managed/src/main/java/com/yugabyte/yw/models/Users.java b/managed/src/main/java/com/yugabyte/yw/models/Users.java
@@ -35,7 +35,6 @@
 import lombok.RequiredArgsConstructor;
 import lombok.Setter;
 import lombok.extern.slf4j.Slf4j;
-import org.apache.commons.lang3.StringUtils;
 import org.joda.time.DateTime;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -382,14 +381,17 @@ public void updateAuthToken(String authToken) {
    * @return apiToken
    */
   public String upsertApiToken() {
-    return upsertApiToken(null);
+    return upsertApiToken(apiTokenVersion);
   }
 
   public String upsertApiToken(Long version) {
     UUID uuidToLock = uuid != null ? uuid : NULL_UUID;
     usersLock.acquireLock(uuidToLock);
     try {
-      if (version != null && apiTokenVersion != null && !version.equals(apiTokenVersion)) {
+      if (version != null
+          && version != -1
+          && apiTokenVersion != null
+          && !version.equals(apiTokenVersion)) {
         throw new PlatformServiceException(BAD_REQUEST, "API token version has changed");
       }
       String apiTokenUnhashed = UUID.randomUUID().toString();
@@ -403,25 +405,6 @@ public String upsertApiToken(Long version) {
     }
   }
 
-  /**
-   * Get current apiToken or create a new one if not exists.
-   *
-   * @return apiToken
-   */
-  @JsonIgnore
-  public String getOrCreateApiToken() {
-    UUID uuidToLock = uuid != null ? uuid : NULL_UUID;
-    usersLock.acquireLock(uuidToLock);
-    try {
-      if (StringUtils.isEmpty(apiToken)) {
-        upsertApiToken();
-      }
-      return apiToken;
-    } finally {
-      usersLock.releaseLock(uuidToLock);
-    }
-  }
-
   /**
    * Authenticate with Token, would check if the authToken is valid.
    *

diff --git a/managed/src/main/resources/swagger-strict.json b/managed/src/main/resources/swagger-strict.json
@@ -2819,7 +2819,7 @@
         "type" : "object"
       } ]
     },
-    "BootstarpBackupParams" : {
+    "BootstrapBackupParams" : {
       "description" : "Backup parameters for bootstrapping",
       "properties" : {
         "parallelism" : {
@@ -2844,7 +2844,7 @@
           "type" : "boolean"
         },
         "backupRequestParams" : {
-          "$ref" : "#/definitions/BootstarpBackupParams",
+          "$ref" : "#/definitions/BootstrapBackupParams",
           "description" : "Parameters used to do Backup/restore"
         },
         "tables" : {
@@ -10076,7 +10076,7 @@
       "description" : "Bootstrap parameters for restarting",
       "properties" : {
         "backupRequestParams" : {
-          "$ref" : "#/definitions/BootstarpBackupParams",
+          "$ref" : "#/definitions/BootstrapBackupParams",
           "description" : "Parameters used to do Backup/restore"
         }
       },
@@ -17831,7 +17831,7 @@
           "required" : true,
           "type" : "string"
         }, {
-          "default" : "null",
+          "default" : -1,
           "format" : "int64",
           "in" : "query",
           "name" : "apiTokenVersion",

diff --git a/managed/src/main/resources/swagger.json b/managed/src/main/resources/swagger.json
@@ -2835,7 +2835,7 @@
         "type" : "object"
       } ]
     },
-    "BootstarpBackupParams" : {
+    "BootstrapBackupParams" : {
       "description" : "Backup parameters for bootstrapping",
       "properties" : {
         "parallelism" : {
@@ -2860,7 +2860,7 @@
           "type" : "boolean"
         },
         "backupRequestParams" : {
-          "$ref" : "#/definitions/BootstarpBackupParams",
+          "$ref" : "#/definitions/BootstrapBackupParams",
           "description" : "Parameters used to do Backup/restore"
         },
         "tables" : {
@@ -10180,7 +10180,7 @@
       "description" : "Bootstrap parameters for restarting",
       "properties" : {
         "backupRequestParams" : {
-          "$ref" : "#/definitions/BootstarpBackupParams",
+          "$ref" : "#/definitions/BootstrapBackupParams",
           "description" : "Parameters used to do Backup/restore"
         }
       },
@@ -18209,7 +18209,7 @@
           "required" : true,
           "type" : "string"
         }, {
-          "default" : "null",
+          "default" : -1,
           "format" : "int64",
           "in" : "query",
           "name" : "apiTokenVersion",

diff --git a/managed/src/main/resources/v1.routes b/managed/src/main/resources/v1.routes
@@ -17,7 +17,7 @@ GET     /fetch_jwt_token                                                       c
 + forceAudit
 GET     /insecure_login                                                        com.yugabyte.yw.controllers.SessionController.insecure_login(request: Request)
 PUT     /customers/:cUUID/security                                             com.yugabyte.yw.controllers.SessionController.set_security(cUUID: java.util.UUID, request: Request)
-PUT     /customers/:cUUID/api_token                                            com.yugabyte.yw.controllers.SessionController.api_token(cUUID: java.util.UUID, apiTokenVersion: java.lang.Long ?= null, request: Request)
+PUT     /customers/:cUUID/api_token                                            com.yugabyte.yw.controllers.SessionController.api_token(cUUID: java.util.UUID, apiTokenVersion: java.lang.Long ?= -1, request: Request)
 GET     /logout                                                                com.yugabyte.yw.controllers.SessionController.logout()
 + nocsrf
 POST    /register                                                              com.yugabyte.yw.controllers.SessionController.register(generateApiToken: java.lang.Boolean ?= false, request: Request)