You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nifi.apache.org by mt...@apache.org on 2022/03/18 00:02:14 UTC
[nifi] branch main updated: NIFI-9801 Fixed error in previous correction of AccessToken.isExpired() margin calculation NIFI-9801 Stabilized shaky AccessTokenTest.
This is an automated email from the ASF dual-hosted git repository.
mthomsen pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/nifi.git
The following commit(s) were added to refs/heads/main by this push:
new fc30b64 NIFI-9801 Fixed error in previous correction of AccessToken.isExpired() margin calculation NIFI-9801 Stabilized shaky AccessTokenTest.
fc30b64 is described below
commit fc30b649cccbbecaf8b861def94b6a3a75e532ea
Author: Tamas Palfy <ta...@gmail.com>
AuthorDate: Wed Mar 16 19:28:32 2022 +0100
NIFI-9801 Fixed error in previous correction of AccessToken.isExpired() margin calculation
NIFI-9801 Stabilized shaky AccessTokenTest.
This closes #5872
Signed-off-by: Mike Thomsen <mt...@apache.org>
---
.../java/org/apache/nifi/oauth2/AccessToken.java | 15 +++++--
.../org/apache/nifi/oauth2/AccessTokenTest.java | 47 +++++++++++-----------
.../StandardOauth2AccessTokenProviderTest.java | 36 ++++++++++++++++-
3 files changed, 69 insertions(+), 29 deletions(-)
diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-oauth2-provider-api/src/main/java/org/apache/nifi/oauth2/AccessToken.java b/nifi-nar-bundles/nifi-standard-services/nifi-oauth2-provider-api/src/main/java/org/apache/nifi/oauth2/AccessToken.java
index d0a6dff..5c76c79 100644
--- a/nifi-nar-bundles/nifi-standard-services/nifi-oauth2-provider-api/src/main/java/org/apache/nifi/oauth2/AccessToken.java
+++ b/nifi-nar-bundles/nifi-standard-services/nifi-oauth2-provider-api/src/main/java/org/apache/nifi/oauth2/AccessToken.java
@@ -20,7 +20,7 @@ package org.apache.nifi.oauth2;
import java.time.Instant;
public class AccessToken {
- private static final int EXPIRY_MARGIN_SECONDS = 5;
+ private static final int EXPIRY_MARGIN_SECONDS = 300;
private String accessToken;
private String refreshToken;
@@ -31,7 +31,7 @@ public class AccessToken {
private final Instant fetchTime;
public AccessToken() {
- this.fetchTime = Instant.now();
+ this.fetchTime = now();
}
public AccessToken(String accessToken, String refreshToken, String tokenType, long expiresIn, String scopes) {
@@ -88,7 +88,14 @@ public class AccessToken {
}
public boolean isExpired() {
- final Instant expirationTime = fetchTime.plusSeconds(expiresIn).plusSeconds(EXPIRY_MARGIN_SECONDS);
- return Instant.now().isAfter(expirationTime);
+ final Instant expirationTime = fetchTime.plusSeconds(expiresIn).minusSeconds(EXPIRY_MARGIN_SECONDS);
+
+ boolean expired = now().isAfter(expirationTime);
+
+ return expired;
+ }
+
+ Instant now() {
+ return Instant.now();
}
}
diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-oauth2-provider-api/src/test/java/org/apache/nifi/oauth2/AccessTokenTest.java b/nifi-nar-bundles/nifi-standard-services/nifi-oauth2-provider-api/src/test/java/org/apache/nifi/oauth2/AccessTokenTest.java
index 6f77e62..2130c82 100644
--- a/nifi-nar-bundles/nifi-standard-services/nifi-oauth2-provider-api/src/test/java/org/apache/nifi/oauth2/AccessTokenTest.java
+++ b/nifi-nar-bundles/nifi-standard-services/nifi-oauth2-provider-api/src/test/java/org/apache/nifi/oauth2/AccessTokenTest.java
@@ -16,54 +16,55 @@
*/
package org.apache.nifi.oauth2;
+import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import java.time.Instant;
+
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
public class AccessTokenTest {
- private static final String ACCESS_TOKEN = "ACCESS";
-
- private static final String REFRESH_TOKEN = "REFRESH";
-
- private static final String TOKEN_TYPE = "Bearer";
-
- private static final String SCOPES = "default";
+ private Instant now;
- private static final long TWO_SECONDS_AGO = -2;
-
- private static final long TEN_SECONDS_AGO = -10;
-
- private static final long IN_SIXTY_SECONDS = 60;
+ @BeforeEach
+ void setUp() {
+ now = Instant.now();
+ }
@Test
- public void testIsExpiredTenSecondsAgo() {
- final AccessToken accessToken = getAccessToken(TEN_SECONDS_AGO);
+ public void testIsExpiredInLessThan5Minutes() {
+ final AccessToken accessToken = getAccessToken(299);
assertTrue(accessToken.isExpired());
}
@Test
- public void testIsExpiredTwoSecondsAgo() {
- final AccessToken accessToken = getAccessToken(TWO_SECONDS_AGO);
+ public void testIsExpiredInExactly5Minutes() {
+ final AccessToken accessToken = getAccessToken(300);
assertFalse(accessToken.isExpired());
}
@Test
- public void testIsExpiredInSixtySeconds() {
- final AccessToken accessToken = getAccessToken(IN_SIXTY_SECONDS);
+ public void testIsExpiredInMoreThan5Minutes() {
+ final AccessToken accessToken = getAccessToken(301);
assertFalse(accessToken.isExpired());
}
private AccessToken getAccessToken(final long expiresInSeconds) {
return new AccessToken(
- ACCESS_TOKEN,
- REFRESH_TOKEN,
- TOKEN_TYPE,
+ null,
+ null,
+ null,
expiresInSeconds,
- SCOPES
- );
+ null
+ ) {
+ @Override
+ protected Instant now() {
+ return now;
+ }
+ };
}
}
diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-oauth2-provider-bundle/nifi-oauth2-provider-service/src/test/java/org/apache/nifi/oauth2/StandardOauth2AccessTokenProviderTest.java b/nifi-nar-bundles/nifi-standard-services/nifi-oauth2-provider-bundle/nifi-oauth2-provider-service/src/test/java/org/apache/nifi/oauth2/StandardOauth2AccessTokenProviderTest.java
index 961bfa1..cd10f6b 100644
--- a/nifi-nar-bundles/nifi-standard-services/nifi-oauth2-provider-bundle/nifi-oauth2-provider-service/src/test/java/org/apache/nifi/oauth2/StandardOauth2AccessTokenProviderTest.java
+++ b/nifi-nar-bundles/nifi-standard-services/nifi-oauth2-provider-bundle/nifi-oauth2-provider-service/src/test/java/org/apache/nifi/oauth2/StandardOauth2AccessTokenProviderTest.java
@@ -104,6 +104,7 @@ public class StandardOauth2AccessTokenProviderTest {
@Test
public void testInvalidWhenClientCredentialsGrantTypeSetWithoutClientId() throws Exception {
+ // GIVEN
Processor processor = new NoOpProcessor();
TestRunner runner = TestRunners.newTestRunner(processor);
@@ -111,13 +112,16 @@ public class StandardOauth2AccessTokenProviderTest {
runner.setProperty(testSubject, StandardOauth2AccessTokenProvider.AUTHORIZATION_SERVER_URL, "http://unimportant");
+ // WHEN
runner.setProperty(testSubject, StandardOauth2AccessTokenProvider.GRANT_TYPE, StandardOauth2AccessTokenProvider.CLIENT_CREDENTIALS_GRANT_TYPE);
+ // THEN
runner.assertNotValid(testSubject);
}
@Test
public void testValidWhenClientCredentialsGrantTypeSetWithClientId() throws Exception {
+ // GIVEN
Processor processor = new NoOpProcessor();
TestRunner runner = TestRunners.newTestRunner(processor);
@@ -125,10 +129,12 @@ public class StandardOauth2AccessTokenProviderTest {
runner.setProperty(testSubject, StandardOauth2AccessTokenProvider.AUTHORIZATION_SERVER_URL, "http://unimportant");
+ // WHEN
runner.setProperty(testSubject, StandardOauth2AccessTokenProvider.GRANT_TYPE, StandardOauth2AccessTokenProvider.CLIENT_CREDENTIALS_GRANT_TYPE);
runner.setProperty(testSubject, StandardOauth2AccessTokenProvider.CLIENT_ID, "clientId");
runner.setProperty(testSubject, StandardOauth2AccessTokenProvider.CLIENT_SECRET, "clientSecret");
+ // THEN
runner.assertValid(testSubject);
}
@@ -136,6 +142,7 @@ public class StandardOauth2AccessTokenProviderTest {
public void testAcquireNewToken() throws Exception {
String accessTokenValue = "access_token_value";
+ // GIVEN
Response response = buildResponse(
200,
"{ \"access_token\":\"" + accessTokenValue + "\" }"
@@ -143,19 +150,22 @@ public class StandardOauth2AccessTokenProviderTest {
when(mockHttpClient.newCall(any(Request.class)).execute()).thenReturn(response);
+ // WHEN
String actual = testSubject.getAccessDetails().getAccessToken();
+ // THEN
assertEquals(accessTokenValue, actual);
}
@Test
public void testRefreshToken() throws Exception {
+ // GIVEN
String firstToken = "first_token";
String expectedToken = "second_token";
Response response1 = buildResponse(
200,
- "{ \"access_token\":\"" + firstToken + "\", \"expires_in\":\"-60\", \"refresh_token\":\"not_checking_in_this_test\" }"
+ "{ \"access_token\":\"" + firstToken + "\", \"expires_in\":\"0\", \"refresh_token\":\"not_checking_in_this_test\" }"
);
Response response2 = buildResponse(
@@ -165,14 +175,17 @@ public class StandardOauth2AccessTokenProviderTest {
when(mockHttpClient.newCall(any(Request.class)).execute()).thenReturn(response1, response2);
+ // WHEN
testSubject.getAccessDetails();
String actualToken = testSubject.getAccessDetails().getAccessToken();
+ // THEN
assertEquals(expectedToken, actualToken);
}
@Test
public void testIOExceptionDuringRefreshAndSubsequentAcquire() throws Exception {
+ // GIVEN
String refreshErrorMessage = "refresh_error";
String acquireErrorMessage = "acquire_error";
@@ -191,12 +204,16 @@ public class StandardOauth2AccessTokenProviderTest {
throw new IllegalStateException("Test improperly defined mock HTTP responses.");
});
+ // Get a good accessDetails so we can have a refresh a second time
testSubject.getAccessDetails();
+
+ // WHEN
UncheckedIOException actualException = assertThrows(
UncheckedIOException.class,
() -> testSubject.getAccessDetails()
);
+ // THEN
checkLoggedDebugWhenRefreshFails();
checkLoggedRefreshError(new UncheckedIOException("OAuth2 access token request failed", new IOException(refreshErrorMessage)));
@@ -206,6 +223,7 @@ public class StandardOauth2AccessTokenProviderTest {
@Test
public void testIOExceptionDuringRefreshSuccessfulSubsequentAcquire() throws Exception {
+ // GIVEN
String refreshErrorMessage = "refresh_error";
String expectedToken = "expected_token";
@@ -229,9 +247,13 @@ public class StandardOauth2AccessTokenProviderTest {
throw new IllegalStateException("Test improperly defined mock HTTP responses.");
});
+ // Get a good accessDetails so we can have a refresh a second time
testSubject.getAccessDetails();
+
+ // WHEN
String actualToken = testSubject.getAccessDetails().getAccessToken();
+ // THEN
checkLoggedDebugWhenRefreshFails();
checkLoggedRefreshError(new UncheckedIOException("OAuth2 access token request failed", new IOException(refreshErrorMessage)));
@@ -241,6 +263,7 @@ public class StandardOauth2AccessTokenProviderTest {
@Test
public void testHTTPErrorDuringRefreshAndSubsequentAcquire() throws Exception {
+ // GIVEN
String errorRefreshResponseBody = "{ \"error_response\":\"refresh_error\" }";
String errorAcquireResponseBody = "{ \"error_response\":\"acquire_error\" }";
@@ -267,12 +290,16 @@ public class StandardOauth2AccessTokenProviderTest {
String.format("OAuth2 access token request failed [HTTP %d], response:%n%s", 503, errorAcquireResponseBody)
);
+ // Get a good accessDetails so we can have a refresh a second time
testSubject.getAccessDetails();
+
+ // WHEN
ProcessException actualException = assertThrows(
ProcessException.class,
() -> testSubject.getAccessDetails()
);
+ // THEN
checkLoggedDebugWhenRefreshFails();
checkLoggedRefreshError(new ProcessException("OAuth2 access token request failed [HTTP 500]"));
@@ -284,6 +311,7 @@ public class StandardOauth2AccessTokenProviderTest {
@Test
public void testHTTPErrorDuringRefreshSuccessfulSubsequentAcquire() throws Exception {
+ // GIVEN
String expectedRefreshErrorResponse = "{ \"error_response\":\"refresh_error\" }";
String expectedToken = "expected_token";
@@ -310,9 +338,13 @@ public class StandardOauth2AccessTokenProviderTest {
List<String> expectedLoggedError = Collections.singletonList(String.format("OAuth2 access token request failed [HTTP %d], response:%n%s", 500, expectedRefreshErrorResponse));
+ // Get a good accessDetails so we can have a refresh a second time
testSubject.getAccessDetails();
+
+ // WHEN
String actualToken = testSubject.getAccessDetails().getAccessToken();
+ // THEN
checkLoggedDebugWhenRefreshFails();
checkLoggedRefreshError(new ProcessException("OAuth2 access token request failed [HTTP 500]"));
@@ -325,7 +357,7 @@ public class StandardOauth2AccessTokenProviderTest {
private Response buildSuccessfulInitResponse() {
return buildResponse(
200,
- "{ \"access_token\":\"exists_but_value_irrelevant\", \"expires_in\":\"-60\", \"refresh_token\":\"not_checking_in_this_test\" }"
+ "{ \"access_token\":\"exists_but_value_irrelevant\", \"expires_in\":\"0\", \"refresh_token\":\"not_checking_in_this_test\" }"
);
}