GEODE-9811: Turn UnavailableSecurityManagerException into CacheClosed…

…Exception (apache#7148)
rnsasg · Nov 30, 2021 · 607c30c · 607c30c
1 parent 2b523e1
commit 607c30c
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 3 deletions.
diff --git a/geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java b/geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
@@ -29,13 +29,16 @@
 import org.apache.logging.log4j.Logger;
 import org.apache.shiro.SecurityUtils;
 import org.apache.shiro.ShiroException;
+import org.apache.shiro.UnavailableSecurityManagerException;
 import org.apache.shiro.session.Session;
 import org.apache.shiro.subject.Subject;
 import org.apache.shiro.subject.support.SubjectThreadState;
 import org.apache.shiro.util.ThreadContext;
 import org.apache.shiro.util.ThreadState;
 
 import org.apache.geode.GemFireIOException;
+import org.apache.geode.annotations.VisibleForTesting;
+import org.apache.geode.cache.CacheClosedException;
 import org.apache.geode.internal.cache.EntryEventImpl;
 import org.apache.geode.internal.security.shiro.GeodeAuthenticationToken;
 import org.apache.geode.internal.security.shiro.SecurityManagerProvider;
@@ -117,8 +120,12 @@ public Subject getSubject() {
       }
     }
 
-    // in other cases like rest call, client operations, we get it from the current thread
-    currentUser = SecurityUtils.getSubject();
+    try {
+      // in other cases like rest call, client operations, we get it from the current thread
+      currentUser = getCurrentUser();
+    } catch (UnavailableSecurityManagerException e) {
+      throw new CacheClosedException("Cache is closed.", e);
+    }
 
     if (currentUser == null || currentUser.getPrincipal() == null) {
       throw new AuthenticationRequiredException("Failed to find the authenticated user.");
@@ -127,6 +134,11 @@ public Subject getSubject() {
     return currentUser;
   }
 
+  @VisibleForTesting
+  Subject getCurrentUser() {
+    return SecurityUtils.getSubject();
+  }
+
   /**
    * Returns the current principal if one exists or null.
    *
@@ -153,7 +165,7 @@ public Subject login(final Properties credentials) {
     // this makes sure it starts with a clean user object
     ThreadContext.remove();
 
-    Subject currentUser = SecurityUtils.getSubject();
+    Subject currentUser = getCurrentUser();
     GeodeAuthenticationToken token = new GeodeAuthenticationToken(credentials);
     try {
       logger.debug("Logging in " + token.getPrincipal());

diff --git a/...-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceTest.java b/...-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceTest.java
@@ -19,11 +19,13 @@
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
 
 import java.util.Properties;
 
 import org.apache.shiro.ShiroException;
+import org.apache.shiro.UnavailableSecurityManagerException;
 import org.apache.shiro.session.Session;
 import org.apache.shiro.subject.Subject;
 import org.apache.shiro.subject.SubjectContext;
@@ -34,6 +36,7 @@
 import org.junit.Before;
 import org.junit.Test;
 
+import org.apache.geode.cache.CacheClosedException;
 import org.apache.geode.internal.security.shiro.GeodeAuthenticationToken;
 import org.apache.geode.internal.security.shiro.SecurityManagerProvider;
 import org.apache.geode.security.AuthenticationExpiredException;
@@ -219,4 +222,13 @@ public void login_when_ShiroException_causedBy_OtherExceptions() throws Exceptio
         .hasCauseInstanceOf(RuntimeException.class)
         .hasMessageContaining("Authentication error. Please check your credentials.");
   }
+
+  @Test
+  public void getSubjectShouldThrowCacheClosedExceptionIfSecurityManagerUnavailable() {
+    IntegratedSecurityService spy = spy(securityService);
+    doThrow(new UnavailableSecurityManagerException("test")).when(spy).getCurrentUser();
+    assertThatThrownBy(() -> spy.getSubject())
+        .isInstanceOf(CacheClosedException.class)
+        .hasMessageContaining("Cache is closed");
+  }
 }