Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add validateToken method to auth client #10

Merged
merged 1 commit into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,18 @@ jobs:
java-version: ${{matrix.java}}

- name: Run tests
run: ./gradlew test
env:
KBASE_CI_TOKEN: ${{ secrets.KBASE_CI_TOKEN }}
KBASE_CI_TOKEN2: ${{ secrets.KBASE_CI_TOKEN2 }}
run: |
cp test.cfg.example test.cfg
sed -i "s#^auth_token1 =.*#auth_token1 = $KBASE_CI_TOKEN#" test.cfg
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You test stuff live? Do these tokens expire? You might want to make a calendar event to update once in a while.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're managed by devops and last for 100 years

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point I want to stand up the auth server in testmode rather than use tokens, but... later

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how the old tests ran so I didn't want to change too much

sed -i "s#^auth_token2 =.*#auth_token2 = $KBASE_CI_TOKEN2#" test.cfg
sed -i "s#^auth_user1 =.*#auth_user1 = kbase_bot#" test.cfg
sed -i "s#^auth_user2 =.*#auth_user2 = sychan168#" test.cfg
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...and maybe make a new user here... :(

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah. :/ I mentioned it to devops

sed -i "s#^good_users =.*#good_users = kbasetest2 , kbasetest7 , kbasehelp#" test.cfg

./gradlew test

- name: Upload coverage to Codecov
uses: codecov/codecov-action@v3
Expand Down
3 changes: 2 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ compileJava {
}

test {
systemProperty "AUTH2_TEST_CONFIG", "./test.cfg"
systemProperty "test.cfg", "./test.cfg"
testLogging {
exceptionFormat = 'full'
showStandardStreams = true
Expand Down Expand Up @@ -57,6 +57,7 @@ dependencies {
implementation 'com.fasterxml.jackson.core:jackson-databind:2.5.4'
implementation 'org.slf4j:slf4j-api:1.7.25'

testImplementation 'org.ini4j:ini4j:0.5.2'
testImplementation 'junit:junit:4.12'
testImplementation 'nl.jqno.equalsverifier:equalsverifier:3.1.10'
testImplementation 'org.apache.commons:commons-lang3:3.1'
Expand Down
53 changes: 48 additions & 5 deletions src/main/java/us/kbase/auth/client/AuthClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.io.Reader;
import java.net.HttpURLConnection;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.charset.StandardCharsets;
import java.util.Map;

Expand All @@ -15,6 +16,8 @@
import com.fasterxml.jackson.databind.ObjectMapper;

import us.kbase.auth.AuthException;
import us.kbase.auth.AuthToken;
import us.kbase.auth.client.internal.TokenCache;

/** A client for the KBase Auth2 authentication server (https://github.com/kbase/auth2).
*
Expand All @@ -27,6 +30,9 @@
// TODO CODE use the built in client in Java 11 when we drop java 8

private static final ObjectMapper MAPPER = new ObjectMapper();

// make this configurable? Add a builder if so
private final TokenCache tokenCache = new TokenCache(1000, 2000); // same as old auth client

private final URI rootURI;

Expand All @@ -47,19 +53,31 @@
if (!"https".equals(auth2RootURI.getScheme())) {
LoggerFactory.getLogger(getClass()).warn("auth root URI is insecure");
}
rootURI = auth2RootURI;
final Map<String, Object> doc = request(rootURI);
final Map<String, Object> doc = request(auth2RootURI);
if (!"Authentication Service".equals(doc.get("servicename"))) {
throw new AuthException(String.format(
"Service at %s is not the authentication service", rootURI));
"Service at %s is not the authentication service", auth2RootURI));
}
try {
rootURI = new URI(auth2RootURI.toString() + "/").normalize();
} catch (URISyntaxException e) {
throw new RuntimeException("this should be impossible", e);

Check warning on line 64 in src/main/java/us/kbase/auth/client/AuthClient.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/us/kbase/auth/client/AuthClient.java#L63-L64

Added lines #L63 - L64 were not covered by tests
}
}

private Map<String, Object> request(final URI target) throws IOException, AuthException {
// tried to use the Jersey client here but kept getting ssl handshake errors if I used
// it more than once
return request(target, null);
}

private Map<String, Object> request(final URI target, final String token)
throws IOException, AuthException {
// tried to use the Jersey client here but kept getting ssl handshake errors if I made
// more than one request
final HttpURLConnection conn = (HttpURLConnection) target.toURL().openConnection();
conn.addRequestProperty("Accept", "application/json");
if (token != null) {
conn.addRequestProperty("Authorization", token);
}
try {
final int code = conn.getResponseCode();
final String res = readResponse(conn, code != 200);
Expand Down Expand Up @@ -135,5 +153,30 @@
final Map<String, Object> doc = request(rootURI);
return (String) doc.get("version");
}

// TODO CODE could do a lot more here later w/ the return data from the auth server
// - token type, custom expiration time, etc.

/** Validate a token and get name of the user that owns the token.
* @param token the token.
* @return an authtoken containing the token and the username.
* @throws IOException if an IOException occurs communicating with the auth service.
* @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");
}
final AuthToken t = tokenCache.getToken(token);
if (t != null) {
return t;
}
final URI target = rootURI.resolve("api/V2/token");
Copy link

@Xiangs18 Xiangs18 Apr 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to setRequestMethod here when calling target url?
api/v2/token could be either a GET or POST based on auth2 doc

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a GET by default and the method we want to use for both tokens, and later users, are GETs

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

final Map<String, Object> res = request(target, token.trim());
// assume we're good at this point
final AuthToken authToken = new AuthToken(token, (String) res.get("user"));
tokenCache.putValidToken(authToken);
return authToken;
}

}
58 changes: 58 additions & 0 deletions src/test/java/us/kbase/test/auth/client/AuthClientTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.core.read.ListAppender;
import us.kbase.auth.AuthException;
import us.kbase.auth.AuthToken;
import us.kbase.auth.client.AuthClient;
import us.kbase.test.common.TestCommon;

Expand Down Expand Up @@ -121,4 +122,61 @@ public void getServerVersion() throws Exception {
// don't anchor right side to allow for pre-release strings
assertThat("not a semver", ver.matches("^\\d+\\.\\d+\\.\\d+"), is(true));
}

@Test
public void validateToken() throws Exception {
final String token1 = TestCommon.getAuthToken1();
final String user1 = TestCommon.getAuthUser1();
final String token2 = TestCommon.getAuthToken2();
final String user2 = TestCommon.getAuthUser2();
assertThat("both tokens are the same", token1.equals(token2), is(false));
assertThat("both users are the same", user1.equals(user2), is(false));

final AuthClient c = AuthClient.from(new URI(TestCommon.getAuthURI()));

// First time from service
final AuthToken t = c.validateToken(token1);
assertThat("incorrect user", t.getUserName(), is(user1)); // for easier debugging
assertThat("incorrect auth token", t, is(new AuthToken(token1, user1)));

// Second time from cache. No way to actually verify that's what's happening though.
// If we refactor to use the same client for every request, can inject the client
// and mock it
final AuthToken t2 = c.validateToken(token1);
assertThat("incorrect auth token", t2, is(new AuthToken(token1, user1)));

// First time from service
final AuthToken t3 = c.validateToken(token2);
assertThat("incorrect user", t3.getUserName(), is(user2)); // for easier debugging
assertThat("incorrect auth token", t3, is(new AuthToken(token2, user2)));

// Second time from cache.
final AuthToken t4 = c.validateToken(token2);
assertThat("incorrect auth token", t4, is(new AuthToken(token2, user2)));
}

@Test
public void validateTokenFailEmptyToken() throws Exception {
final URI uri = new URI("https://ci.kbase.us/services/auth");
validateTokenFail(uri, null, new IllegalArgumentException(
"token must be a non-whitespace string"));
validateTokenFail(uri, " \t ", new IllegalArgumentException(
"token must be a non-whitespace string"));
}

@Test
public void validateTokenFailInvalidToken() throws Exception {
final URI uri = new URI("https://ci.kbase.us/services/auth");
validateTokenFail(uri, "faketoken", new AuthException(
"Auth service returned an error: 10020 Invalid token"));
}

private void validateTokenFail(final URI uri, final String token, final Exception expected) {
try {
AuthClient.from(uri).validateToken(token);
fail("expected exception");
} catch (Exception got) {
TestCommon.assertExceptionCorrect(got, expected);
}
}
}
104 changes: 104 additions & 0 deletions src/test/java/us/kbase/test/common/TestCommon.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,100 @@
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.junit.Assert.assertThat;

import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.List;
import java.util.Map;

import org.apache.commons.lang3.exception.ExceptionUtils;
import org.ini4j.Ini;


public class TestCommon {

public static final String AUTH_URL = "auth_url";
public static final String TOKEN1 = "auth_token1";
public static final String TOKEN2 = "auth_token2";
public static final String USER1 = "auth_user1";
public static final String USER2 = "auth_user2";
public static final String GOOD_USERS = "good_users";

public static final String TEST_CONFIG_FILE_PROP_NAME = "test.cfg";
public static final String TEST_CONFIG_FILE_SECTION = "auth_client_test";

private static Map<String, String> testConfig;

public static String getAuthURI() {
return getTestProperty(AUTH_URL);
}

public static String getAuthToken1() {
return getTestProperty(TOKEN1);
}

public static String getAuthToken2() {
return getTestProperty(TOKEN2);
}

public static String getAuthUser1() {
return getTestProperty(USER1);
}

public static String getAuthUser2() {
return getTestProperty(USER2);
}

public static List<String> getGoodUsers() {
return Arrays.asList(getTestProperty(GOOD_USERS).split(","));
}

public static String getTestProperty(final String propertyKey, final boolean allowNull) {
getTestConfig();
final String prop = testConfig.get(propertyKey);
if (!allowNull && (prop == null || prop.trim().isEmpty())) {
throw new TestException(String.format(
"Property %s in section %s of test file %s is missing",
propertyKey, TEST_CONFIG_FILE_SECTION, getConfigFilePath()));
}
return prop;
}

public static String getTestProperty(final String propertyKey) {
return getTestProperty(propertyKey, false);
}

private static void getTestConfig() {
if (testConfig != null) {
return;
}
final Path testCfgFilePath = getConfigFilePath();
final Ini ini;
try {
ini = new Ini(testCfgFilePath.toFile());
} catch (IOException ioe) {
throw new TestException(String.format(
"IO Error reading the test configuration file %s: %s",
testCfgFilePath, ioe.getMessage()), ioe);
}
testConfig = ini.get(TEST_CONFIG_FILE_SECTION);
if (testConfig == null) {
throw new TestException(String.format("No section %s found in test config file %s",
TEST_CONFIG_FILE_SECTION, testCfgFilePath));
}
}

private static Path getConfigFilePath() {
final String testCfgFilePathStr = System.getProperty(TEST_CONFIG_FILE_PROP_NAME);
if (testCfgFilePathStr == null || testCfgFilePathStr.trim().isEmpty()) {
throw new TestException(String.format("Cannot get the test config file path." +
" Ensure the java system property %s is set to the test config file location.",
TEST_CONFIG_FILE_PROP_NAME));
}
return Paths.get(testCfgFilePathStr).toAbsolutePath().normalize();
}

public static void assertExceptionCorrect(
final Throwable got,
final Throwable expected) {
Expand All @@ -17,5 +107,19 @@ public static void assertExceptionCorrect(
is(expected.getLocalizedMessage()));
assertThat("incorrect exception type", got, instanceOf(expected.getClass()));
}

public static class TestException extends RuntimeException {

private static final long serialVersionUID = 1L;


public TestException(final String message) {
super(message);
}

public TestException(final String message, final Throwable cause) {
super(message, cause);
}
}

}
11 changes: 11 additions & 0 deletions test.cfg.example
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[auth_client_test]

auth_url = https://ci.kbase.us/services/auth/

auth_token1 = <token goes here>
auth_user1 = <user name goes here>

auth_token2 = <token goes here>
auth_user2 = <user name goes here>

good_users = <comma separated list of user names>
Loading