From 62c04dfe753327bfc7b3dff14abe4a2620645230 Mon Sep 17 00:00:00 2001 From: eastagiletracker <310448263+eastagiletracker@users.noreply.github.com> Date: Tue, 29 Sep 2026 11:54:59 +0700 Subject: [PATCH] Skip illegal user names when validating a list of users Previously a single empty or illegal user name in the list passed to GET /api/V2/users (and the test mode equivalent) failed the entire call. Such names are now skipped in the same way names that don't correspond to an account are omitted from the results. --- README.md | 3 +- .../us/kbase/auth2/service/api/Users.java | 12 ++++---- .../test/auth2/service/api/TestModeTest.java | 21 +++++++++----- .../auth2/service/api/UserEndpointTest.java | 28 +++++++++++++------ 4 files changed, 41 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index ec606e80..2b4d0cec 100644 --- a/README.md +++ b/README.md @@ -88,7 +88,8 @@ keys `display` and `email`. `GET /api/V2/users/?list=` Validate a set of user names and get the users' display names. Returns a map of username -> -display name. Any usernames that do not correspond to accounts will not be included in the map. +display name. Any usernames that do not correspond to accounts, as well as any empty or illegal +usernames, will not be included in the map. `GET /api/V2/users/search//?fields=` Find users based on a prefix of the username or any parts of the display name, where parts are diff --git a/src/main/java/us/kbase/auth2/service/api/Users.java b/src/main/java/us/kbase/auth2/service/api/Users.java index e4a219f6..3b8e03bd 100644 --- a/src/main/java/us/kbase/auth2/service/api/Users.java +++ b/src/main/java/us/kbase/auth2/service/api/Users.java @@ -21,7 +21,6 @@ import us.kbase.auth2.lib.DisplayName; import us.kbase.auth2.lib.UserName; import us.kbase.auth2.lib.UserSearchSpec; -import us.kbase.auth2.lib.exceptions.ErrorType; import us.kbase.auth2.lib.exceptions.IllegalParameterException; import us.kbase.auth2.lib.exceptions.InvalidTokenException; import us.kbase.auth2.lib.exceptions.MissingParameterException; @@ -54,8 +53,11 @@ public Map getUsers( Collectors.toMap(e -> e.getKey().getName(), e -> e.getValue().getName())); } - static Set processUserListString(final String users) - throws IllegalParameterException { + /* Entries that are empty or not legal user names are skipped, in the same way names that + * don't correspond to an account are omitted from the results, rather than failing the + * entire call. An illegal user name can never correspond to an account. + */ + static Set processUserListString(final String users) { final Set uns = new HashSet<>(); if (nullOrEmpty(users)) { return uns; @@ -65,9 +67,7 @@ static Set processUserListString(final String users) try { uns.add(new UserName(u.trim())); } catch (MissingParameterException | IllegalParameterException e) { - //TODO CODE this exception could use some clean up - throw new IllegalParameterException(ErrorType.ILLEGAL_USER_NAME, String.format( - "Illegal user name [%s]: %s", u, e.getMessage())); + // skip, see above } } return uns; diff --git a/src/test/java/us/kbase/test/auth2/service/api/TestModeTest.java b/src/test/java/us/kbase/test/auth2/service/api/TestModeTest.java index 0e6d8a31..5806cc02 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/TestModeTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/TestModeTest.java @@ -272,14 +272,21 @@ public void getUserDisplayNameFailInput() { new NoTokenProvidedException("No user token provided")); failGetUserDisplayName(tm, " \t \n ", "", new NoTokenProvidedException("No user token provided")); + } + + @Test + public void getUserDisplayNameSkipMissingAndIllegalNames() throws Exception { + final Authentication auth = mock(Authentication.class); + final TestMode tm = new TestMode(auth); + + when(auth.testModeGetUserDisplayNames(new IncomingToken("token"), + set(new UserName("foo"), new UserName("bar")))) + .thenReturn(ImmutableMap.of( + new UserName("foo"), new DisplayName("whee"), + new UserName("bar"), new DisplayName("whoo"))); - failGetUserDisplayName(tm, "token", "foo, , bar", - new IllegalParameterException(ErrorType.ILLEGAL_USER_NAME, - "Illegal user name [ ]: 30000 Missing input parameter: user name")); - failGetUserDisplayName(tm, "token", "foo, MyName, bar", - new IllegalParameterException(ErrorType.ILLEGAL_USER_NAME, - "Illegal user name [ MyName]: 30010 Illegal user name: " + - "Illegal character in user name MyName: M")); + assertThat("incorrect users", tm.getUsers("token", "foo, , MyName, 9baz, bar"), + is(ImmutableMap.of("foo", "whee", "bar", "whoo"))); } @Test diff --git a/src/test/java/us/kbase/test/auth2/service/api/UserEndpointTest.java b/src/test/java/us/kbase/test/auth2/service/api/UserEndpointTest.java index 0ff18ddd..3061cd52 100644 --- a/src/test/java/us/kbase/test/auth2/service/api/UserEndpointTest.java +++ b/src/test/java/us/kbase/test/auth2/service/api/UserEndpointTest.java @@ -638,18 +638,19 @@ private IncomingToken setUpUsersForTesting() throws Exception { } @Test - public void getUserListFailMissingUser() throws Exception { - failGetUserList(" u1 , , u3", "foobar", 400, "Bad Request", - new IllegalParameterException(ErrorType.ILLEGAL_USER_NAME, - "Illegal user name [ ]: 30000 Missing input parameter: user name")); + public void getUserListSkipMissingUser() throws Exception { + getUserList(" baz , , mua", ImmutableMap.of("baz", "fuz", "mua", "paz")); } @Test - public void getUserListFailIllegalUser() throws Exception { - failGetUserList(" u1 , aA , u3", "foobar", 400, "Bad Request", - new IllegalParameterException(ErrorType.ILLEGAL_USER_NAME, - "Illegal user name [ aA ]: 30010 Illegal user name: " + - "Illegal character in user name aA: A")); + public void getUserListSkipIllegalUser() throws Exception { + getUserList(" baz , aA , mua, 9foo, ba*z, u3", + ImmutableMap.of("baz", "fuz", "mua", "paz")); + } + + @Test + public void getUserListOnlyIllegalUsers() throws Exception { + getUserList(" aA , , 9foo", Collections.emptyMap()); } @Test @@ -659,6 +660,15 @@ public void getUserListFailBadToken() throws Exception { failGetUserList("u1", "boobar", 401, "Unauthorized", new InvalidTokenException()); } + @Test + public void getUserListFailBadTokenWithIllegalUsers() throws Exception { + // illegal names are skipped, so the token must still be checked + failGetUserList(" u1 , aA , ", null, 400, "Bad Request", + new NoTokenProvidedException("No user token provided")); + failGetUserList(" u1 , aA , ", "boobar", 401, "Unauthorized", + new InvalidTokenException()); + } + private void failGetUserList( final String list, final String token,