Moved isAuthenticated check to InternalSubject before attempting login

Signed-off-by: Darshit Chanpura <[email protected]>
opensearch-project · Dec 7, 2022 · 5db6f1b · 5db6f1b
1 parent 5d449c0
commit 5db6f1b
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 19 deletions.
diff --git a/sandbox/libs/authn/src/main/java/org/opensearch/authn/AuthenticationTokenHandler.java b/sandbox/libs/authn/src/main/java/org/opensearch/authn/AuthenticationTokenHandler.java
@@ -69,7 +69,7 @@ private static AuthenticationToken handleBasicAuth(final BasicAuthToken token) {
             return null;
         }
 
-        logger.info("Logging in as: " + username);
+        logger.info("Attempting authentication as: " + username);
 
         return new UsernamePasswordToken(username, password);
     }

diff --git a/sandbox/libs/authn/src/main/java/org/opensearch/authn/internal/InternalSubject.java b/sandbox/libs/authn/src/main/java/org/opensearch/authn/internal/InternalSubject.java
@@ -65,6 +65,16 @@ public String toString() {
      */
     public void login(AuthenticationToken authenticationToken) {
         org.apache.shiro.authc.AuthenticationToken authToken = AuthenticationTokenHandler.extractShiroAuthToken(authenticationToken);
+
+        // If already authenticated, do not check login info again
+        /*
+            TODO: understand potential repercussions in following situations:
+             1. How to handle this in password change situations
+             2. Can two subjects in same environment have same principal name? if so the following check is invalid
+         */
+        if (this.isAuthenticated() && this.getPrincipal().getName().equals(authToken.getPrincipal())) {
+            return;
+        }
         // Login via shiro realm.
         shiroSubject.login(authToken);
     }

diff --git a/server/src/main/java/org/opensearch/rest/RestController.java b/server/src/main/java/org/opensearch/rest/RestController.java
@@ -622,7 +622,7 @@ private boolean authenticate(RestRequest request, RestChannel channel) throws IO
             try {
                 headerToken = tokenType(authHeader.get());
                 subject = Identity.getAuthManager().getSubject();
-                getShiroSessionAndLogin(subject, headerToken);
+                subject.login(headerToken);
                 logger.info("Authentication successful");
                 return true;
             } catch (final AuthenticationException ae) {
@@ -667,21 +667,4 @@ private AuthenticationToken tokenType(String authHeader) {
         // support other type of header tokens
         return null;
     }
-
-    /**
-     * Get or create a shiro session before attempting login
-     *
-     * Should prevent "{@link org.apache.shiro.session.UnknownSessionException} There is no session with id '123'"
-     *
-     * @param subject Current shiro subject
-     * @param headerToken Attempt login using this token
-     */
-    private static void getShiroSessionAndLogin(Subject subject, AuthenticationToken headerToken) {
-        // TODO: potential repercussions. i.e. How to handle this in password change situations
-        // TODO: should logout be implemented to handle this
-        if (subject.isAuthenticated()) return;
-
-        subject.login(headerToken);
-    }
-
 }