You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by GitBox <gi...@apache.org> on 2022/08/23 10:56:12 UTC
[GitHub] [knox] smolnar82 commented on a diff in pull request #622: KNOX-2789 - Changed configuration names, removed the option to config…
smolnar82 commented on code in PR #622:
URL: https://github.com/apache/knox/pull/622#discussion_r952457950
##########
gateway-server/src/test/java/org/apache/knox/gateway/session/control/InMemoryConcurrentSessionVerifierTest.java:
##########
@@ -316,25 +296,27 @@ public void testBackgroundThreadRemoveExpiredTokens() throws ServiceLifecycleExc
verifier.verifySessionForUser("admin", adminToken2);
JWT expiringAdminToken = tokenAuthority.issueToken(expiringJwtAttributesForAdmin);
verifier.verifySessionForUser("admin", expiringAdminToken);
- Assert.assertEquals(3, verifier.countValidTokensForUser("admin"));
- Thread.sleep(1100);
- Assert.assertEquals(2, verifier.countValidTokensForUser("admin"));
+ Assert.assertEquals(3, verifier.getTokenCountForUser("admin").intValue());
+ Thread.sleep(1050);
+ verifier.removeExpiredTokens();
+ Assert.assertEquals(2, verifier.getTokenCountForUser("admin").intValue());
JWTokenAttributes expiringJwtAttributesForTom = makeJwtAttribute("tom", true);
verifier.verifySessionForUser("tom", tomToken1);
verifier.verifySessionForUser("tom", tomToken2);
JWT expiringTomToken = tokenAuthority.issueToken(expiringJwtAttributesForTom);
verifier.verifySessionForUser("tom", expiringTomToken);
- Assert.assertEquals(3, verifier.countValidTokensForUser("tom"));
- Thread.sleep(1100);
- Assert.assertEquals(2, verifier.countValidTokensForUser("tom"));
+ Assert.assertEquals(3, verifier.getTokenCountForUser("tom").intValue());
+ Thread.sleep(1050);
Review Comment:
The same qq. here.
##########
gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java:
##########
@@ -822,7 +822,7 @@ public interface GatewayConfig {
Set<String> getPrivilegedUsers();
- Set<String> getNonPrivilegedUsers();
+ Set<String> getUnlimitedUsers();
Review Comment:
We may rename this to `getSessionVerificationUnlimitiedUsers` and the previous one to `getSessionVerificationPrivilegedUsers`. What do you think?
##########
gateway-server/src/test/java/org/apache/knox/gateway/session/control/InMemoryConcurrentSessionVerifierTest.java:
##########
@@ -273,41 +255,39 @@ public void testNegativeLimitMeansUnlimited() throws ServiceLifecycleException {
@Test
public void testExpiredTokensAreNotCounted() throws ServiceLifecycleException, TokenServiceException, InterruptedException {
- GatewayConfig config = mockConfig(new HashSet<>(Arrays.asList("admin")), new HashSet<>(Arrays.asList("tom", "guest")), 3, 3);
+ GatewayConfig config = mockConfig(Collections.emptySet(), new HashSet<>(Arrays.asList("admin")), 3, 3);
verifier.init(config, options);
JWTokenAttributes expiringJwtAttributesForTom = makeJwtAttribute("tom", true);
- JWT tomToken = tokenAuthority.issueToken(jwtAttributesForTom);
- verifier.verifySessionForUser("tom", tomToken);
+ verifier.verifySessionForUser("tom", tomToken1);
Assert.assertEquals(1, verifier.countValidTokensForUser("tom"));
- tomToken = tokenAuthority.issueToken(expiringJwtAttributesForTom);
- verifier.verifySessionForUser("tom", tomToken);
+ JWT expiringTomToken = tokenAuthority.issueToken(expiringJwtAttributesForTom);
+ verifier.verifySessionForUser("tom", expiringTomToken);
Assert.assertEquals(2, verifier.countValidTokensForUser("tom"));
- tomToken = tokenAuthority.issueToken(expiringJwtAttributesForTom);
- verifier.verifySessionForUser("tom", tomToken);
+ expiringTomToken = tokenAuthority.issueToken(expiringJwtAttributesForTom);
+ verifier.verifySessionForUser("tom", expiringTomToken);
Assert.assertEquals(3, verifier.countValidTokensForUser("tom"));
- Thread.sleep(1000L);
+ Thread.sleep(1100);
Review Comment:
As discussed offline, we may give a high enough cleaning period, and invoke the cleaning job manually here to avoid Thread.sleep. Have considered doing that?
##########
gateway-server/src/test/java/org/apache/knox/gateway/session/control/InMemoryConcurrentSessionVerifierTest.java:
##########
@@ -316,25 +296,27 @@ public void testBackgroundThreadRemoveExpiredTokens() throws ServiceLifecycleExc
verifier.verifySessionForUser("admin", adminToken2);
JWT expiringAdminToken = tokenAuthority.issueToken(expiringJwtAttributesForAdmin);
verifier.verifySessionForUser("admin", expiringAdminToken);
- Assert.assertEquals(3, verifier.countValidTokensForUser("admin"));
- Thread.sleep(1100);
- Assert.assertEquals(2, verifier.countValidTokensForUser("admin"));
+ Assert.assertEquals(3, verifier.getTokenCountForUser("admin").intValue());
+ Thread.sleep(1050);
Review Comment:
If you are invoking `verifier.removeExpiredTokens();` anyway, why do we need the Thread.sleep here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org