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.

Sign up for GitHub

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

DEVEXP-417 Tweaked token renewal to be based on 401 instead of 403 #1562

Merged
merged 1 commit into from
May 11, 2023
Merged

DEVEXP-417 Tweaked token renewal to be based on 401 instead of 403 #1562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions ...rc/main/java/com/marklogic/client/impl/okhttp/MarkLogicCloudAuthenticationConfigurer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,8 @@ public TokenAuthenticationInterceptor(TokenGenerator tokenGenerator) {
@Override
public Response intercept(Chain chain) throws IOException {
Response response = chain.proceed(addTokenToRequest(chain));
if (response.code() == 403) {
logger.info("Received 403; will generate new token if necessary and retry request");
if (response.code() == 401) {
logger.info("Received 401; will generate new token if necessary and retry request");
response.close();
final String currentToken = this.token;
generateNewTokenIfNecessary(currentToken);
Expand All @@ -169,7 +169,7 @@ public Response intercept(Chain chain) throws IOException {

/**
* In the case of N threads using the same DatabaseClient - e.g. when using DMSDK - all of them
* may make a request at the same time and get a 403 back. Functionally, it should be fine if all
* may make a request at the same time and get a 401 back. Functionally, it should be fine if all
* make their own requests to renew the token, with the last thread being the one whose token
* value is retained on this class. But to simplify matters, this block is synchronized so only one
* thread can be in here. And if that thread finds that this.token is different from currentToken,
Expand All @@ -182,7 +182,7 @@ public Response intercept(Chain chain) throws IOException {
*/
private synchronized void generateNewTokenIfNecessary(String currentToken) {
if (currentToken.equals(this.token)) {
logger.info("Generating new token based on receiving 403");
logger.info("Generating new token based on receiving 401");
this.token = tokenGenerator.generateToken();
} else if (logger.isDebugEnabled()) {
logger.debug("This instance's token has already been updated, presumably by another thread");
Expand Down
24 changes: 12 additions & 12 deletions ...pi/src/test/java/com/marklogic/client/impl/okhttp/TokenAuthenticationInterceptorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,30 +40,30 @@ void beforeEach() {
}

@Test
void receive403() {
enqueueResponseCodes(200, 200, 403, 200);
void receive401() {
enqueueResponseCodes(200, 200, 401, 200);

verifyRequestReturnsResponseCode(200);
verifyRequestReturnsResponseCode(200);
verifyRequestReturnsResponseCode(200,
"If a 403 is received from the server, then the token should be renewed, and then the 200 should be " +
"If a 401 is received from the server, then the token should be renewed, and then the 200 should be " +
"returned to the caller.");

assertEquals(2, fakeTokenGenerator.timesInvoked,
"A token should have been generated for the first request and then again when the 403 was received.");
"A token should have been generated for the first request and then again when the 401 was received.");
}

@Test
void receive401() {
enqueueResponseCodes(200, 200, 401);
void receive403() {
enqueueResponseCodes(200, 200, 403);

verifyRequestReturnsResponseCode(200);
verifyRequestReturnsResponseCode(200);
verifyRequestReturnsResponseCode(401);
verifyRequestReturnsResponseCode(403);

assertEquals(1, fakeTokenGenerator.timesInvoked,
"A token should have been generated for the first request, and the 401 should not have resulted in the " +
"token being renewed; only a 403 should.");
"A token should have been generated for the first request, and the 403 should not have resulted in the " +
"token being renewed; only a 401 should.");
}

@Test
Expand All @@ -76,8 +76,8 @@ void multipleThreads() throws Exception {
};

// Mock up 4 responses for each of the 2 threads created below. For each thread, the first call succeeds; the
// second receives a 403 and then succeeds; and the third call succeeds.
enqueueResponseCodes(200, 200, 403, 403, 200, 200, 200, 200);
// second receives a 401 and then succeeds; and the third call succeeds.
enqueueResponseCodes(200, 200, 401, 401, 200, 200, 200, 200);

// Spawn two threads and wait for them to complete.
ExecutorService service = Executors.newFixedThreadPool(2);
Expand All @@ -88,7 +88,7 @@ void multipleThreads() throws Exception {

assertEquals(2, fakeTokenGenerator.timesInvoked,
"The fake token generator should have been invoked twice - once when the interceptor was created, and then " +
"only one more time when the two threads received 403's at almost the exact same time. The interceptor " +
"only one more time when the two threads received 401's at almost the exact same time. The interceptor " +
"is expected to synchronize the call for generating a token such that only one thread will generate a " +
"new token. The other token is expected to see that the token has changed and uses the new token " +
"instead of generating a new token itself.");
Expand Down