From ad27f3eec3705c9a3fd3b4e8ac13a6654c7ae980 Mon Sep 17 00:00:00 2001 From: Rutuja Surve Date: Fri, 25 Nov 2022 15:38:29 +0530 Subject: [PATCH] username validation for special chars Signed-off-by: Rutuja Surve --- .../security/dlic/rest/api/InternalUsersApiAction.java | 7 +++++++ .../security/dlic/rest/api/AccountApiTest.java | 4 ++-- .../opensearch/security/dlic/rest/api/UserApiTest.java | 9 +++++++-- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java index dbf1a0e800..e7f3989f33 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java @@ -14,6 +14,7 @@ import java.io.IOException; import java.nio.file.Path; import java.util.List; +import java.util.regex.Pattern; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ObjectNode; @@ -93,6 +94,12 @@ protected void handlePut(RestChannel channel, final RestRequest request, final C return; } + Pattern usernamePattern = Pattern.compile("[$&+,:;=\\\\?@#|/'<>^*()%!-]"); + if (usernamePattern.matcher(username).find()) { + badRequestResponse(channel, "Username has special characters, not permitted."); + return; + } + // TODO it might be sensible to consolidate this with the overridden method in // order to minimize duplicated logic diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/AccountApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/AccountApiTest.java index 0b91aa35af..afac1699d4 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/AccountApiTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/AccountApiTest.java @@ -44,7 +44,7 @@ public AccountApiTest(){ public void testGetAccount() throws Exception { // arrange setup(); - final String testUser = "test-user"; + final String testUser = "testuser"; final String testPass = "test-pass"; addUserWithPassword(testUser, testPass, HttpStatus.SC_CREATED); @@ -79,7 +79,7 @@ public void testGetAccount() throws Exception { public void testPutAccount() throws Exception { // arrange setup(); - final String testUser = "test-user"; + final String testUser = "testuser"; final String testPass = "test-old-pass"; final String testPassHash = "$2y$12$b7TNPn2hgl0nS7gXJ.beuOd8JGl6Nz5NsTyxofglGCItGNyDdwivK"; // hash for test-old-pass final String testNewPass = "test-new-pass"; diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java index 715c256cb7..e8654182ce 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java @@ -133,6 +133,10 @@ public void testUserApi() throws Exception { response = rh.executePutRequest(ENDPOINT + "/internalusers/", "{\"hash\": \"123\"}", new Header[0]); Assert.assertEquals(HttpStatus.SC_METHOD_NOT_ALLOWED, response.getStatusCode()); + // username has special characters + response = rh.executePutRequest(ENDPOINT + "/internalusers/n@ag:ilum", "{\"hash\": \"456\"}", new Header[0]); + Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); + // Faulty JSON payload response = rh.executePutRequest(ENDPOINT + "/internalusers/nagilum", "{some: \"thing\" asd other: \"thing\"}", new Header[0]); @@ -401,7 +405,8 @@ public void testUserApi() throws Exception { Assert.assertTrue(roles.contains("starfleet")); Assert.assertTrue(roles.contains("captains")); - addUserWithPassword("$1aAAAAAAAAC", "$1aAAAAAAAAC", HttpStatus.SC_CREATED); + addUserWithPassword("$1aAAAAAAAAC", "$1aAAAAAAAAC", HttpStatus.SC_BAD_REQUEST); + addUserWithPassword("1aAAAAAAAAC", "$1aAAAAAAAAC", HttpStatus.SC_CREATED); addUserWithPassword("abc", "abc", HttpStatus.SC_CREATED); @@ -468,7 +473,7 @@ public void testPasswordRules() throws Exception { addUserWithPassword("$1aAAAAAAAac", "$1aAAAAAAAAC", HttpStatus.SC_BAD_REQUEST); addUserWithPassword(URLEncoder.encode("$1aAAAAAAAac%", "UTF-8"), "$1aAAAAAAAAC%", HttpStatus.SC_BAD_REQUEST); addUserWithPassword(URLEncoder.encode("$1aAAAAAAAac%!=\"/\\;:test&~@^", "UTF-8").replace("+", "%2B"), "$1aAAAAAAAac%!=\\\"/\\\\;:test&~@^", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword(URLEncoder.encode("$1aAAAAAAAac%!=\"/\\;: test&", "UTF-8"), "$1aAAAAAAAac%!=\\\"/\\\\;: test&123", HttpStatus.SC_CREATED); + addUserWithPassword(URLEncoder.encode("$1aAAAAAAAac%!=\"/\\;: test&", "UTF-8"), "$1aAAAAAAAac%!=\\\"/\\\\;: test&123", HttpStatus.SC_BAD_REQUEST); response = rh.executeGetRequest(PLUGINS_PREFIX + "/api/internalusers/nothinghthere?pretty", new Header[0]); Assert.assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatusCode());