smolnar82 commented on code in PR #624:
URL: https://github.com/apache/knox/pull/624#discussion_r954564130
##########
gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java:
##########
@@ -264,9 +264,14 @@ private Response getAuthenticationToken(int statusCode) {
original = originalUrlCookies.get(0).getValue();
}
+ Principal p = request.getUserPrincipal();
+ ConcurrentSessionVerifier verifier =
services.getService(ServiceType.CONCURRENT_SESSION_VERIFIER);
+ if (!verifier.verifySessionForUser(p.getName())) {
Review Comment:
Now we have this same 3-line code block in two places; I'd create a new
private void method that throws the web app exception if needed (to avoid code
duplication).
##########
gateway-server/src/main/java/org/apache/knox/gateway/session/control/InMemoryConcurrentSessionVerifier.java:
##########
@@ -51,10 +51,16 @@ public class InMemoryConcurrentSessionVerifier implements
ConcurrentSessionVerif
private long cleaningPeriod;
private final ScheduledExecutorService expiredTokenRemover =
Executors.newSingleThreadScheduledExecutor();
+ /**
+ * This function is used after the verifySessionForUser function.
+ * It checks the session limits even though verifySessionForUser already
checked them,
+ * because in high stress situations there might be some threads which get
verified even though they are over the limit in
+ * verifySessionForUser function, so we catch them here.
+ */
@Override
- public boolean verifySessionForUser(String username, JWT jwtToken) {
+ public boolean registerToken(String username, JWT jwtToken) {
Review Comment:
This could be simplified like that:
```
if (verifySessionForUser(userName)) {
//lock
concurrentSessionCounter.putIfAbsent(username, new HashSet<>());
//unlock
return true;
}
return false;
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
