[Feature/Identity] Fixes test failures in gradle check for #4798

CHANGELOG.md

@@ -18,3 +18,4 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - [Identity] Adds Basic Auth mechanism via Internal IdP ([#4798](https://github.com/opensearch-project/OpenSearch/pull/4798))
 - [Identity] Introduce Identity Module ([#5583](https://github.com/opensearch-project/OpenSearch/pull/5583))
 - [Identity] Adds Bearer Auth mechanism via internal token handling ([#5611](https://github.com/opensearch-project/OpenSearch/pull/5611))
+- [Identity] Fixes test failures in gradle check for #4798 ([#5441](https://github.com/opensearch-project/OpenSearch/pull/5441))

sandbox/libs/authn/src/main/java/org/opensearch/authn/AuthenticationTokenHandler.java

@@ -74,7 +74,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);
     }

sandbox/libs/authn/src/main/java/org/opensearch/authn/Subject.java

@@ -29,4 +29,10 @@ public interface Subject {
      * throws SubjectDisabled
      */
     void login(final AuthenticationToken token);
+
+    /**
+     * Checks the current authentication status of this subject
+     * @return true if authenticated, false otherwise
+     */
+    boolean isAuthenticated();
 }

sandbox/libs/authn/src/main/java/org/opensearch/authn/internal/InternalSubject.java

@@ -66,7 +66,27 @@ 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);
     }
+
+    /**
+     * A flag to indicate whether this subject is already authenticated
+     *
+     * @return true if authenticated, false otherwise
+     */
+    @Override
+    public boolean isAuthenticated() {
+        return shiroSubject.isAuthenticated();
+    }
+
 }

sandbox/libs/authn/src/main/java/org/opensearch/authn/noop/NoopSubject.java

@@ -47,11 +47,14 @@ public String toString() {
         return "NoopSubject(principal=" + getPrincipal() + ")";
     }
 
-    /**
-     * Logs the user in
-     */
     @Override
     public void login(AuthenticationToken authenticationToken) {
         // Do nothing as noop subject is always logged in
     }
+
+    @Override
+    public boolean isAuthenticated() {
+        // Noop subject is always authenticated
+        return true;
+    }
 }

sandbox/libs/authn/src/test/java/org/opensearch/authn/internal/InternalSubjectTests.java

@@ -0,0 +1,102 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.authn.internal;
+
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
+import org.apache.shiro.SecurityUtils;
+import org.apache.shiro.authc.AuthenticationException;
+import org.apache.shiro.subject.Subject;
+import org.junit.After;
+import org.junit.Before;
+import org.opensearch.authn.tokens.AuthenticationToken;
+import org.opensearch.authn.tokens.BasicAuthToken;
+import org.opensearch.test.OpenSearchTestCase;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
+public class InternalSubjectTests extends OpenSearchTestCase {
+    private static final AuthenticationToken AUTH_TOKEN = new BasicAuthToken("Basic YWRtaW46YWRtaW4=");
+
+    @Before
+    public void initRealm() {
+        new InternalAuthenticationManager();
+    }
+
+    @After
+    public void logoutSubject() {
+        Subject subject = SecurityUtils.getSubject();
+        if (subject == null) return;
+        if (subject.getSession() != null) subject.getSession().stop();
+        SecurityUtils.getSubject().logout();
+    }
+
+    public void testGetPrincipal() {
+        InternalSubject internalSubject = new InternalSubject(SecurityUtils.getSubject());
+        assertNull(internalSubject.getPrincipal());
+
+        internalSubject.login(AUTH_TOKEN);
+
+        assertNotNull(internalSubject.getPrincipal());
+    }
+
+    public void testLoginBehaviors() {
+        Subject subject = mock(Subject.class);
+        InternalSubject internalSubject = new InternalSubject(subject);
+
+        // 1. Verify that login() is called on first attempt
+
+        internalSubject.login(AUTH_TOKEN);
+        verify(subject, times(1)).login(any());
+
+        // 2. Verify that call to login() is skipped on 2nd attempt for same user
+
+        when(subject.isAuthenticated()).thenReturn(true);
+        when(subject.getPrincipal()).thenReturn("admin");
+
+        internalSubject.login(AUTH_TOKEN);
+        verify(subject, times(1)).login(any());
+
+        // 3. Verify that login() is called again for a different user
+
+        when(subject.isAuthenticated()).thenReturn(true);
+        when(subject.getPrincipal()).thenReturn("marvin");
+
+        // even though we are using same token, the mock stub returns different principal and hence login is called again
+        internalSubject.login(AUTH_TOKEN);
+        verify(subject, times(2)).login(any());
+    }
+
+    public void testSuccessfulLoginAndLogOut() {
+        InternalSubject internalSubject = new InternalSubject(SecurityUtils.getSubject());
+
+        internalSubject.login(AUTH_TOKEN);
+        assertTrue(internalSubject.isAuthenticated());
+    }
+
+    public void testLoginFailure() {
+        InternalSubject internalSubject = new InternalSubject(SecurityUtils.getSubject());
+
+        AuthenticationToken authToken = new BasicAuthToken("Basic bWFydmluOmdhbGF4eQ==");
+
+        assertThrows(AuthenticationException.class, () -> internalSubject.login(authToken));
+    }
+
+    public void testIsAuthenticated() {
+        InternalSubject internalSubject = new InternalSubject(SecurityUtils.getSubject());
+
+        assertFalse(internalSubject.isAuthenticated());
+        internalSubject.login(AUTH_TOKEN);
+        assertTrue(internalSubject.isAuthenticated());
+    }
+}