[GitHub] [knox] smolnar82 commented on a diff in pull request #624: KNOX-2790 - Added a new funtion to verifier, this way the session lim…

[GitBox <gi...@apache.org>]

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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org