[Enhancement] Input Validation for generate obo token endpoint

src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java

@@ -42,6 +42,7 @@
 import static org.hamcrest.Matchers.allOf;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasKey;
+import static org.junit.Assert.assertTrue;
 import static org.opensearch.security.support.ConfigConstants.SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX;
 import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED;
 import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
@@ -68,9 +69,16 @@ public class OnBehalfOfJwtAuthenticationTest {
     public static final String OBO_USER_NAME_NO_PERM = "obo_user_no_perm";
     public static final String DEFAULT_PASSWORD = "secret";
     public static final String NEW_PASSWORD = "testPassword123!!";
-    public static final String OBO_TOKEN_REASON = "{\"reason\":\"Test generation\"}";
+    public static final String OBO_TOKEN_REASON = "{\"description\":\"Test generation\"}";
     public static final String OBO_ENDPOINT_PREFIX = "_plugins/_security/api/generateonbehalfoftoken";
     public static final String OBO_DESCRIPTION = "{\"description\":\"Testing\", \"service\":\"self-issued\"}";
+
+    public static final String OBO_DESCRIPTION_WITH_INVALID_DURIATIONSECONDS =
+        "{\"description\":\"Testing\", \"service\":\"self-issued\", \"durationSeconds\":\"invalid-seconds\"}";
+
+    public static final String OBO_DESCRIPTION_WITH_INVALID_PARAMETERS =
+        "{\"description\":\"Testing\", \"service\":\"self-issued\", \"invalidParameter\":\"invalid-parameter\"}";
+
     public static final String HOST_MAPPING_IP = "127.0.0.1";
     public static final String OBO_USER_NAME_WITH_HOST_MAPPING = "obo_user_with_ip_role_mapping";
     public static final String CURRENT_AND_NEW_PASSWORDS = "{ \"current_password\": \""
@@ -178,11 +186,33 @@ public void shouldNotIncludeRolesFromHostMappingInOBOToken() {
         Assert.assertFalse(roles.contains("host_mapping_role"));
     }
 
+    @Test
+    public void shouldNotAuthenticateWithInvalidDurationSeconds() {
+        try (TestRestClient client = cluster.getRestClient(ADMIN_USER_NAME, DEFAULT_PASSWORD)) {
+            client.assertCorrectCredentials(ADMIN_USER_NAME);
+            TestRestClient.HttpResponse response = client.postJson(OBO_ENDPOINT_PREFIX, OBO_DESCRIPTION_WITH_INVALID_DURIATIONSECONDS);
+            response.assertStatusCode(HttpStatus.SC_BAD_REQUEST);
+            Map<String, Object> oboEndPointResponse = (Map<String, Object>) response.getBodyAs(Map.class);
+            assertTrue(oboEndPointResponse.containsValue("durationSeconds must be an integer."));
+        }
+    }
+
+    @Test
+    public void shouldNotAuthenticateWithInvalidAPIParameter() {
+        try (TestRestClient client = cluster.getRestClient(ADMIN_USER_NAME, DEFAULT_PASSWORD)) {
+            client.assertCorrectCredentials(ADMIN_USER_NAME);
+            TestRestClient.HttpResponse response = client.postJson(OBO_ENDPOINT_PREFIX, OBO_DESCRIPTION_WITH_INVALID_PARAMETERS);
+            response.assertStatusCode(HttpStatus.SC_BAD_REQUEST);
+            Map<String, Object> oboEndPointResponse = (Map<String, Object>) response.getBodyAs(Map.class);
+            assertTrue(oboEndPointResponse.containsValue("Unrecognized parameter: invalidParameter"));
+        }
+    }
+
     private String generateOboToken(String username, String password) {
         try (TestRestClient client = cluster.getRestClient(username, password)) {
             client.assertCorrectCredentials(username);
             TestRestClient.HttpResponse response = client.postJson(OBO_ENDPOINT_PREFIX, OBO_TOKEN_REASON);
-            response.assertStatusCode(200);
+            response.assertStatusCode(HttpStatus.SC_OK);
             Map<String, Object> oboEndPointResponse = (Map<String, Object>) response.getBodyAs(Map.class);
             assertThat(
                 oboEndPointResponse,

src/main/java/org/opensearch/security/action/onbehalf/CreateOnBehalfOfTokenAction.java

@@ -12,13 +12,17 @@
 package org.opensearch.security.action.onbehalf;
 
 import java.io.IOException;
+import java.util.Arrays;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
 import com.google.common.collect.ImmutableList;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.greenrobot.eventbus.Subscribe;
 
 import org.opensearch.client.node.NodeClient;
@@ -61,6 +65,12 @@
 
     public static final String DEFAULT_SERVICE = "self-issued";
 
+    protected final Logger log = LogManager.getLogger(this.getClass());
+
+    private static final Set<String> RECOGNIZED_PARAMS = new HashSet<>(
+        Arrays.asList("durationSeconds", "description", "roleSecurityMode", "service")
+    );
+
     @Subscribe
     public void onConfigModelChanged(ConfigModel configModel) {
         this.configModel = configModel;
@@ -128,13 +138,13 @@
                     final String clusterIdentifier = clusterService.getClusterName().value();
 
                     final Map<String, Object> requestBody = request.contentOrSourceParamParser().map();
-                    final String description = (String) requestBody.getOrDefault("description", null);
 
-                    final Integer tokenDuration = Optional.ofNullable(requestBody.get("durationSeconds"))
-                        .map(value -> (String) value)
-                        .map(Integer::parseInt)
-                        .map(value -> Math.min(value, OBO_MAX_EXPIRY_SECONDS)) // Max duration seconds are 600
-                        .orElse(OBO_DEFAULT_EXPIRY_SECONDS); // Fallback to default
+                    validateRequestParameters(requestBody);
+
+                    Integer tokenDuration = parseAndValidateDurationSeconds(requestBody.get("durationSeconds"));
+                    tokenDuration = Math.min(tokenDuration, OBO_MAX_EXPIRY_SECONDS);
+
+                    final String description = (String) requestBody.getOrDefault("description", null);
 
                     final Boolean roleSecurityMode = Optional.ofNullable(requestBody.get("roleSecurityMode"))
                         .map(value -> (Boolean) value)
@@ -161,8 +171,13 @@
                     builder.endObject();
 
                     response = new BytesRestResponse(RestStatus.OK, builder);
+                } catch (IllegalArgumentException iae) {
+                    builder.startObject().field("error", iae.getMessage()).endObject();
+                    response = new BytesRestResponse(RestStatus.BAD_REQUEST, builder);
                 } catch (final Exception exception) {
-                    builder.startObject().field("error", exception.toString()).endObject();
+                    log.error("Unexpected error occurred: ", exception);
+
+                    builder.startObject().field("error", "An unexpected error occurred. Please check the input and try again.").endObject();
 
                     response = new BytesRestResponse(RestStatus.INTERNAL_SERVER_ERROR, builder);
                 }
@@ -176,4 +191,29 @@
         return this.configModel.mapSecurityRoles(user, null);
     }
 
+    private void validateRequestParameters(Map<String, Object> requestBody) throws IllegalArgumentException {
+        for (String key : requestBody.keySet()) {
+            if (!RECOGNIZED_PARAMS.contains(key)) {
+                throw new IllegalArgumentException("Unrecognized parameter: " + key);
+            }
+        }
+    }
+
+    private Integer parseAndValidateDurationSeconds(Object durationObj) throws IllegalArgumentException {
+        if (durationObj == null) {
+            return OBO_DEFAULT_EXPIRY_SECONDS;
+        }
+
+        if (durationObj instanceof Integer) {
+            return (Integer) durationObj;
+        } else if (durationObj instanceof String) {
+            try {
+                return Integer.parseInt((String) durationObj);
+            } catch (NumberFormatException e) {
+                throw new IllegalArgumentException("durationSeconds must be an integer.");
+            }
+        } else {
+            throw new IllegalArgumentException("durationSeconds must be an integer.");
+        }
+    }
 }