You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@knox.apache.org by sm...@apache.org on 2024/03/13 12:18:47 UTC
(knox) branch master updated: KNOX-3019 - Allow token renewal without upper bound for non-expired tokens (#880)
This is an automated email from the ASF dual-hosted git repository.
smolnar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git
The following commit(s) were added to refs/heads/master by this push:
new c098afaec KNOX-3019 - Allow token renewal without upper bound for non-expired tokens (#880)
c098afaec is described below
commit c098afaec3a181d8a5d8d5f25a61526d4b608a8b
Author: Sandor Molnar <sm...@apache.org>
AuthorDate: Wed Mar 13 13:18:43 2024 +0100
KNOX-3019 - Allow token renewal without upper bound for non-expired tokens (#880)
---
.../token/impl/DefaultTokenStateService.java | 10 ++--
.../services/token/impl/JDBCTokenStateService.java | 4 +-
.../services/token/impl/TokenStateDatabase.java | 2 +-
.../token/impl/JDBCTokenStateServiceTest.java | 9 ++++
.../gateway/service/knoxtoken/TokenResource.java | 26 +++++-----
.../knoxtoken/TokenServiceResourceTest.java | 56 ++++++++++++++++------
.../gateway/services/security/token/KnoxToken.java | 2 +-
7 files changed, 74 insertions(+), 35 deletions(-)
diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenStateService.java b/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenStateService.java
index 18c7ea24f..afca435bd 100644
--- a/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenStateService.java
+++ b/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenStateService.java
@@ -270,12 +270,13 @@ public class DefaultTokenStateService implements TokenStateService {
return getTokenExpiration(token) <= System.currentTimeMillis();
}
- protected void setMaxLifetime(final String token, long parsedMaxLifeTime) {
- maxTokenLifetimes.put(token, parsedMaxLifeTime);
+ protected void setMaxLifetime(final String token, long maxLifeTime) {
+ maxTokenLifetimes.put(token, maxLifeTime);
}
protected void setMaxLifetime(final String token, long issueTime, long maxLifetimeDuration) {
- maxTokenLifetimes.put(token, issueTime + maxLifetimeDuration);
+ final long maxLifetime = maxLifetimeDuration < 0 ? maxLifetimeDuration : issueTime + maxLifetimeDuration;
+ setMaxLifetime(token, maxLifetime);
}
/**
@@ -313,8 +314,9 @@ public class DefaultTokenStateService implements TokenStateService {
}
protected boolean hasRemainingRenewals(final String tokenId, long renewInterval) {
+ final long maximumTokenLifetime = getMaxLifetime(tokenId);
// If the current time + buffer + the renewal interval is less than the max lifetime for the token?
- return ((System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(30) + renewInterval) < getMaxLifetime(tokenId));
+ return maximumTokenLifetime < 0 ? true : ((System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(30) + renewInterval) < maximumTokenLifetime);
}
protected long getMaxLifetime(final String tokenId) {
diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/JDBCTokenStateService.java b/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/JDBCTokenStateService.java
index 3887b117a..2a4ed5d80 100644
--- a/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/JDBCTokenStateService.java
+++ b/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/JDBCTokenStateService.java
@@ -196,10 +196,10 @@ public class JDBCTokenStateService extends AbstractPersistentTokenStateService i
@Override
protected long getMaxLifetime(String tokenId) {
- long maxLifetime = super.getMaxLifetime(tokenId);
+ long maxLifetime = super.getMaxLifetime(tokenId); // returns 0, if not found in memory
// If there is no result from the in-memory collection, proceed to check the Database
- if (maxLifetime < 1L) {
+ if (maxLifetime == 0L) {
try {
maxLifetime = tokenDatabase.getMaxLifetime(tokenId);
log.fetchedMaxLifetimeFromDatabase(Tokens.getTokenIDDisplayText(tokenId), maxLifetime);
diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/TokenStateDatabase.java b/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/TokenStateDatabase.java
index b17b7c93b..43f8c394c 100644
--- a/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/TokenStateDatabase.java
+++ b/gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/TokenStateDatabase.java
@@ -78,7 +78,7 @@ public class TokenStateDatabase {
addTokenStatement.setString(1, tokenId);
addTokenStatement.setLong(2, issueTime);
addTokenStatement.setLong(3, expiration);
- addTokenStatement.setLong(4, issueTime + maxLifetimeDuration);
+ addTokenStatement.setLong(4, maxLifetimeDuration < 0 ? maxLifetimeDuration : issueTime + maxLifetimeDuration);
return addTokenStatement.executeUpdate() == 1;
}
}
diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/services/token/impl/JDBCTokenStateServiceTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/services/token/impl/JDBCTokenStateServiceTest.java
index c30b53545..eba965b55 100644
--- a/gateway-server/src/test/java/org/apache/knox/gateway/services/token/impl/JDBCTokenStateServiceTest.java
+++ b/gateway-server/src/test/java/org/apache/knox/gateway/services/token/impl/JDBCTokenStateServiceTest.java
@@ -111,6 +111,15 @@ public class JDBCTokenStateServiceTest {
assertEquals(issueTime + maxLifetimeDuration, getLongTokenAttributeFromDatabase(tokenId, TokenStateDatabase.GET_MAX_LIFETIME_SQL));
}
+ @Test
+ public void testAddTokenThatCanAlwaysBeRenewed() throws Exception {
+ final String tokenId = UUID.randomUUID().toString();
+ jdbcTokenStateService.addToken(tokenId, System.currentTimeMillis(), System.currentTimeMillis() + 30, -1);
+
+ assertEquals(-1L, jdbcTokenStateService.getMaxLifetime(tokenId));
+ assertEquals(-1L, getLongTokenAttributeFromDatabase(tokenId, TokenStateDatabase.GET_MAX_LIFETIME_SQL));
+ }
+
@Test
public void testAddTokensForMultipleUsers() throws Exception {
String user1 = "user1";
diff --git a/gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java b/gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java
index 5c98d9fba..8698ce842 100644
--- a/gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java
+++ b/gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java
@@ -97,14 +97,12 @@ import static javax.ws.rs.core.MediaType.APPLICATION_JSON;
import static javax.ws.rs.core.MediaType.APPLICATION_XML;
/**
- * @deprecated The public REST API endpoints in this class (bound to
- * '/knoxtoken/v1/api/token/...') are no longer acceptable for
- * token-related operations. Please use the
- * '/knoxtoken/v2/api/token/...' path instead.
+ * Some of the public REST API endpoints in this class (bound to
+ * '/knoxtoken/v1/api/token/...') are no longer acceptable for token-related
+ * operations. Please use the '/knoxtoken/v2/api/token/...' path instead.
*
* @see TokenResourceV2
*/
-@Deprecated
@Singleton
@Path(TokenResource.RESOURCE_PATH)
public class TokenResource {
@@ -206,7 +204,8 @@ public class TokenResource {
UNKNOWN_TOKEN(50),
ALREADY_DISABLED(60),
ALREADY_ENABLED(70),
- DISABLED_KNOXSSO_COOKIE(80);
+ DISABLED_KNOXSSO_COOKIE(80),
+ TOKEN_EXPIRED(90);
private final int code;
@@ -549,13 +548,14 @@ public class TokenResource {
if (allowedRenewers.contains(renewer)) {
try {
JWTToken jwt = new JWTToken(token);
- // If renewal fails, it should be an exception
- expiration = tokenStateService.renewToken(jwt,
- renewInterval.orElse(tokenStateService.getDefaultRenewInterval()));
- log.renewedToken(getTopologyName(),
- Tokens.getTokenDisplayText(token),
- Tokens.getTokenIDDisplayText(TokenUtils.getTokenId(jwt)),
- renewer);
+ if (tokenStateService.isExpired(jwt)) {
+ errorCode = ErrorCode.TOKEN_EXPIRED;
+ error = "Expired tokens must not be renewed.";
+ } else {
+ // If renewal fails, it should be an exception
+ expiration = tokenStateService.renewToken(jwt, renewInterval.orElse(tokenStateService.getDefaultRenewInterval()));
+ log.renewedToken(getTopologyName(), Tokens.getTokenDisplayText(token), Tokens.getTokenIDDisplayText(TokenUtils.getTokenId(jwt)), renewer);
+ }
} catch (ParseException e) {
log.invalidToken(getTopologyName(), Tokens.getTokenDisplayText(token), e);
errorCode = ErrorCode.INVALID_TOKEN;
diff --git a/gateway-service-knoxtoken/src/test/java/org/apache/knox/gateway/service/knoxtoken/TokenServiceResourceTest.java b/gateway-service-knoxtoken/src/test/java/org/apache/knox/gateway/service/knoxtoken/TokenServiceResourceTest.java
index 332d2ce1e..5dec1162b 100644
--- a/gateway-service-knoxtoken/src/test/java/org/apache/knox/gateway/service/knoxtoken/TokenServiceResourceTest.java
+++ b/gateway-service-knoxtoken/src/test/java/org/apache/knox/gateway/service/knoxtoken/TokenServiceResourceTest.java
@@ -89,6 +89,7 @@ import org.apache.knox.gateway.services.GatewayServices;
import org.apache.knox.gateway.services.ServiceLifecycleException;
import org.apache.knox.gateway.services.ServiceType;
import org.apache.knox.gateway.services.security.AliasService;
+import org.apache.knox.gateway.services.security.AliasServiceException;
import org.apache.knox.gateway.services.security.token.JWTokenAttributes;
import org.apache.knox.gateway.services.security.token.JWTokenAuthority;
import org.apache.knox.gateway.services.security.token.KnoxToken;
@@ -784,6 +785,24 @@ public class TokenServiceResourceTest {
assertEquals(10L, tss.getMaxLifetime(token) - tss.getIssueTime(token));
}
+ @Test
+ public void testTokenRenewalShouldFailOnExpiredTokens() throws Exception {
+ final long tokenTTL = 1;
+ final String renewer = "yarn";
+ final Map<String, String> contextExpectations = new HashMap<>();
+ contextExpectations.put(TokenStateService.CONFIG_SERVER_MANAGED, "true");
+ contextExpectations.put("knox.token.ttl", String.valueOf(tokenTTL));
+ contextExpectations.put("knox.token.renewer.whitelist", renewer);
+ Thread.sleep(tokenTTL + 10); // so that the token is expired
+ configureCommonExpectations(contextExpectations);
+ final TokenResource tokenResource = new TokenResource();
+ final String accessToken = getAccessToken(tokenResource);
+
+ final Response renewalResponse = requestTokenRenewal(tokenResource, accessToken, createTestSubject(renewer));
+ assertEquals(Response.Status.BAD_REQUEST, renewalResponse.getStatusInfo());
+ assertTrue(renewalResponse.getEntity().toString().contains("Expired tokens must not be renewed."));
+ assertTrue(renewalResponse.getEntity().toString().contains("\"code\": " + TokenResource.ErrorCode.TOKEN_EXPIRED.toInt()));
+ }
@Test
public void testTokenRevocation_ServerManagedStateNotConfigured() throws Exception {
@@ -1497,19 +1516,8 @@ public class TokenServiceResourceTest {
configureCommonExpectations(contextExpectations, gatewayLevelConfig);
- TokenResource tr = new TokenResource();
- tr.request = request;
- tr.context = context;
- tr.init();
-
- // Request a token
- Response retResponse = tr.doGet();
- assertEquals(200, retResponse.getStatus());
-
- // Parse the response
- String retString = retResponse.getEntity().toString();
- String accessToken = getTagValue(retString, "access_token");
- assertNotNull(accessToken);
+ final TokenResource tr = new TokenResource();
+ final String accessToken = getAccessToken(tr);
Response response;
switch (operation) {
@@ -1526,6 +1534,22 @@ public class TokenServiceResourceTest {
return new AbstractMap.SimpleEntry<>(tss, response);
}
+ private String getAccessToken(TokenResource tokenResource) throws KeyLengthException, AliasServiceException, ServiceLifecycleException {
+ tokenResource.request = request;
+ tokenResource.context = context;
+ tokenResource.init();
+
+ // Request a token
+ final Response retResponse = tokenResource.doGet();
+ assertEquals(200, retResponse.getStatus());
+
+ // Parse the response
+ final String retString = retResponse.getEntity().toString();
+ final String accessToken = getTagValue(retString, "access_token");
+ assertNotNull(accessToken);
+ return accessToken;
+ }
+
private static Response requestTokenRenewal(final TokenResource tr, final String tokenData, final Subject caller) {
Response response;
if (caller != null) {
@@ -1681,6 +1705,10 @@ public class TokenServiceResourceTest {
@Override
public boolean isExpired(JWTToken token) {
+ try {
+ return getTokenExpiration(token) <= System.currentTimeMillis();
+ } catch (UnknownTokenException e) {
+ }
return false;
}
@@ -1719,7 +1747,7 @@ public class TokenServiceResourceTest {
@Override
public long getTokenExpiration(JWT token) throws UnknownTokenException {
- return 0;
+ return getTokenExpiration(TokenUtils.getTokenId(token));
}
@Override
diff --git a/gateway-spi/src/main/java/org/apache/knox/gateway/services/security/token/KnoxToken.java b/gateway-spi/src/main/java/org/apache/knox/gateway/services/security/token/KnoxToken.java
index 21359ca88..cd4b982c1 100644
--- a/gateway-spi/src/main/java/org/apache/knox/gateway/services/security/token/KnoxToken.java
+++ b/gateway-spi/src/main/java/org/apache/knox/gateway/services/security/token/KnoxToken.java
@@ -68,7 +68,7 @@ public class KnoxToken implements Comparable<KnoxToken>{
}
public String getMaxLifetime() {
- return KNOX_TOKEN_TS_FORMAT.get().format(new Date(maxLifetime));
+ return maxLifetime < 0 ? "Unbounded" : KNOX_TOKEN_TS_FORMAT.get().format(new Date(maxLifetime));
}
public long getMaxLifetimeLong() {