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,