From 9e41f553566c7d7cb665f8c738ea6128783afca7 Mon Sep 17 00:00:00 2001 From: Gavin Date: Wed, 3 Apr 2024 18:44:34 -0700 Subject: [PATCH 1/2] Add isValidUsername method --- .../java/us/kbase/auth/client/AuthClient.java | 64 +++++++++++++++- .../auth/client/internal/TokenCache.java | 6 +- .../test/auth/client/AuthClientTest.java | 75 +++++++++++++++++++ test.cfg.example | 1 + 4 files changed, 139 insertions(+), 7 deletions(-) diff --git a/src/main/java/us/kbase/auth/client/AuthClient.java b/src/main/java/us/kbase/auth/client/AuthClient.java index 3b634d0..1d3d180 100644 --- a/src/main/java/us/kbase/auth/client/AuthClient.java +++ b/src/main/java/us/kbase/auth/client/AuthClient.java @@ -9,7 +9,12 @@ import java.net.URI; import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.LinkedList; +import java.util.List; import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.slf4j.LoggerFactory; @@ -17,6 +22,7 @@ import us.kbase.auth.AuthException; import us.kbase.auth.AuthToken; +import us.kbase.auth.client.internal.StringCache; import us.kbase.auth.client.internal.TokenCache; /** A client for the KBase Auth2 authentication server (https://github.com/kbase/auth2). @@ -31,8 +37,11 @@ public class AuthClient { private static final ObjectMapper MAPPER = new ObjectMapper(); - // make this configurable? Add a builder if so + // make these configurable? Add a builder if so private final TokenCache tokenCache = new TokenCache(1000, 2000); // same as old auth client + private final StringCache userCache = new StringCache(1000, 2000); // same as old auth client + + final static Pattern INVALID_USERNAME = Pattern.compile("[^a-z\\d_]+"); private final URI rootURI; @@ -164,9 +173,7 @@ public String getServerVersion() throws IOException, AuthException { * @throws AuthException if an auth exception occurs communicating with the auth service. */ public AuthToken validateToken(final String token) throws IOException, AuthException { - if (token == null || token.trim().isEmpty()) { - throw new IllegalArgumentException("token must be a non-whitespace string"); - } + checkToken(token); final AuthToken t = tokenCache.getToken(token); if (t != null) { return t; @@ -179,4 +186,53 @@ public AuthToken validateToken(final String token) throws IOException, AuthExcep return authToken; } + private void checkToken(final String token) { + if (token == null || token.trim().isEmpty()) { + throw new IllegalArgumentException("token must be a non-whitespace string"); + } + } + + /** Check if usernames are valid accounts in the auth service. + * @param users the list of usernames to check. If they contain any invalid characters + * (e.g. anything other than a-z, 0-9, or _, an exception will be thrown. + * @param token any valid auth token. + * @return a mapping of each username to whether it's valid or not. + * @throws IOException if an IOException occurs communicating with the auth service. + * @throws AuthException if an auth exception occurs communicating with the auth service. + */ + public Map isValidUserName(final List users, final String token) + throws IOException, AuthException { + // theoretically someone could submit hundreds of users and hit the url size limit + // don't worry about that for now. + checkToken(token); + if (users == null || users.isEmpty()) { + throw new IllegalArgumentException("users cannot be null or empty"); + } + final List badlist = new LinkedList(); + final Map result = new HashMap<>(); + for (String user: users) { + if (user == null || user.trim().isEmpty()) { + throw new IllegalArgumentException("each user must be a non-whitespace string"); + } + user = user.trim(); + final Matcher m = INVALID_USERNAME.matcher(user); + if (m.find()) { + // this matches the prior behavior, but later could have an validity enum + // one for bad chars, one for does not exist + throw new IllegalArgumentException( + "username " + user + " has invalid character: " + m.group(0)); + } + if (userCache.hasString(user)) { + result.put(user, true); + } else { + badlist.add(user); + } + } + final URI target = rootURI.resolve("api/V2/users/?list=" + String.join(",", badlist)); + final Map res = request(target, token.trim()); + res.keySet().stream().forEach(u -> userCache.putString(u)); + badlist.stream().forEach(u -> result.put(u, res.containsKey(u))); + return result; + } + } diff --git a/src/main/java/us/kbase/auth/client/internal/TokenCache.java b/src/main/java/us/kbase/auth/client/internal/TokenCache.java index 0af138f..a77e13b 100644 --- a/src/main/java/us/kbase/auth/client/internal/TokenCache.java +++ b/src/main/java/us/kbase/auth/client/internal/TokenCache.java @@ -116,7 +116,7 @@ public void putValidToken(AuthToken token) { } Collections.sort(dts); for(int i = size; i < dts.size(); i++) { - cache.remove(dts.get(i).token); + cache.remove(dts.get(i).tokenHash); } } } @@ -135,11 +135,11 @@ class UserDate { class DateToken implements Comparable{ - final String token; + final String tokenHash; final Date date; DateToken(long date, String token) { - this.token = token; + this.tokenHash = token; this.date = new Date(date); } diff --git a/src/test/java/us/kbase/test/auth/client/AuthClientTest.java b/src/test/java/us/kbase/test/auth/client/AuthClientTest.java index c7a7e16..69679b6 100644 --- a/src/test/java/us/kbase/test/auth/client/AuthClientTest.java +++ b/src/test/java/us/kbase/test/auth/client/AuthClientTest.java @@ -5,7 +5,12 @@ import static org.junit.Assert.fail; import java.net.URI; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedList; import java.util.List; +import java.util.Map; import org.junit.AfterClass; import org.junit.Before; @@ -179,4 +184,74 @@ private void validateTokenFail(final URI uri, final String token, final Exceptio TestCommon.assertExceptionCorrect(got, expected); } } + + @Test + public void isValidUserName() throws Exception { + final String token1 = TestCommon.getAuthToken1(); + final List goodUsers = TestCommon.getGoodUsers(); + + final AuthClient c = AuthClient.from(new URI(TestCommon.getAuthURI())); + + final List badUsers = Arrays.asList( + "superfakeuserthatdoesntexistihope", + "anothersuperfakeuserrighthereimfake"); + + final List allUsers = new LinkedList<>(badUsers); + goodUsers.stream().forEach(u -> allUsers.add(String.format(" \t %s ", u))); + + final Map expected = new HashMap<>(); + goodUsers.stream().forEach(u -> expected.put(u.trim(), true)); + badUsers.stream().forEach(u -> expected.put(u, false)); + + final Map res = c.isValidUserName(allUsers, token1); + + assertThat("incorrect users", res, is(expected)); + + // 2nd time from cache. Again no good way to test this w/o a client mock + final Map res2 = c.isValidUserName(allUsers, token1); + + assertThat("incorrect users", res2, is(expected)); + } + + @Test + public void isValidUserNameFailBadArgs() throws Exception { + final URI uri = new URI(TestCommon.getAuthURI()); + final List u = Arrays.asList("u"); + isValidUserNameFail(uri, null, "foo", + new IllegalArgumentException("users cannot be null or empty")); + isValidUserNameFail(uri, Collections.emptyList(), "foo", + new IllegalArgumentException("users cannot be null or empty")); + isValidUserNameFail(uri, Arrays.asList("a", null), "foo", + new IllegalArgumentException("each user must be a non-whitespace string")); + isValidUserNameFail(uri, Arrays.asList("a", " "), "foo", + new IllegalArgumentException("each user must be a non-whitespace string")); + isValidUserNameFail(uri, Arrays.asList("a", " foo*bar "), "foo", + new IllegalArgumentException("username foo*bar has invalid character: *")); + isValidUserNameFail(uri, u, null, + new IllegalArgumentException("token must be a non-whitespace string")); + isValidUserNameFail(uri, u, " \t ", + new IllegalArgumentException("token must be a non-whitespace string")); + } + + @Test + public void isValidUserNameFailBadToken() throws Exception { + final URI uri = new URI(TestCommon.getAuthURI()); + final List u = Arrays.asList("u"); + isValidUserNameFail(uri, u, "badtoken", new AuthException( + "Auth service returned an error: 10020 Invalid token")); + } + + private void isValidUserNameFail( + final URI uri, + final List usernames, + final String token, + final Exception expected) { + try { + AuthClient.from(uri).isValidUserName(usernames, token); + fail("expected exception"); + } catch (Exception got) { + TestCommon.assertExceptionCorrect(got, expected); + } + } + } diff --git a/test.cfg.example b/test.cfg.example index ad605be..1c0d6ee 100644 --- a/test.cfg.example +++ b/test.cfg.example @@ -8,4 +8,5 @@ auth_user1 = auth_token2 = auth_user2 = +# existing names from the auth server good_users = \ No newline at end of file From c323ad36fa6664d689bb43c1db7ca4b1b5033926 Mon Sep 17 00:00:00 2001 From: Gavin Date: Tue, 9 Apr 2024 19:25:01 -0700 Subject: [PATCH 2/2] Add codecov token --- .github/workflows/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e94d4db..ba3bab2 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -50,4 +50,5 @@ jobs: - name: Upload coverage to Codecov uses: codecov/codecov-action@v3 with: + token: ${{ secrets.CODECOV_TOKEN }} fail_ci_if_error: true