Clear external group cache on explicit logout or user deletion (#1819)

Clears the external group cache whenever a user gets logged out by the logout rest method or the user gets deleted.

Co-authored-by: Eduard Heimbuch <eduard.heimbuch@cloudogu.com>

gradle/changelog/clear_external_group_cache.yaml

@@ -0,0 +1,2 @@
+- type: Changed
+  description: Clear external group cache on explicit user logout ([#1819](https://github.com/scm-manager/scm-manager/pull/1819))

scm-core/src/main/java/sonia/scm/security/LogoutEvent.java

@@ -0,0 +1,41 @@
+/*
+ * MIT License
+ *
+ * Copyright (c) 2020-present Cloudogu GmbH and Contributors
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+package sonia.scm.security;
+
+import lombok.Value;
+import sonia.scm.event.Event;
+
+/**
+ * Event is fired whenever a user explicitly logs out.
+ * The event is not fired if a session expires or a token is blacklisted,
+ * the event is only fired if the user calls the rest method for the logout.
+ *
+ * @since 2.24.0
+ */
+@Value
+@Event
+public class LogoutEvent {
+  String primaryPrincipal;
+}

scm-webapp/src/main/java/sonia/scm/api/v2/resources/AuthenticationResource.java

@@ -40,15 +40,18 @@ import io.swagger.v3.oas.annotations.security.SecuritySchemes;
 import io.swagger.v3.oas.annotations.tags.Tag;
 import org.apache.shiro.SecurityUtils;
 import org.apache.shiro.authc.AuthenticationException;
+import org.apache.shiro.subject.PrincipalCollection;
 import org.apache.shiro.subject.Subject;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import sonia.scm.event.ScmEventBus;
 import sonia.scm.metrics.AuthenticationMetrics;
 import sonia.scm.security.AccessToken;
 import sonia.scm.security.AccessTokenBuilder;
 import sonia.scm.security.AccessTokenBuilderFactory;
 import sonia.scm.security.AccessTokenCookieIssuer;
 import sonia.scm.security.AllowAnonymousAccess;
+import sonia.scm.security.LogoutEvent;
 import sonia.scm.security.Scope;
 import sonia.scm.security.Tokens;
 import sonia.scm.web.VndMediaType;
@@ -86,7 +89,6 @@ import java.util.Optional;
   @Tag(name = "Authentication", description = "Authentication related endpoints")
 })
 @Path(AuthenticationResource.PATH)
-@AllowAnonymousAccess
 public class AuthenticationResource {
 
   private static final Logger LOG = LoggerFactory.getLogger(AuthenticationResource.class);
@@ -99,17 +101,19 @@ public class AuthenticationResource {
   private final Counter loginAttemptsCounter;
   private final Counter loginFailedCounter;
   private final Counter logoutCounter;
+  private final ScmEventBus eventBus;
 
   @Inject(optional = true)
   private LogoutRedirection logoutRedirection;
 
   @Inject
-  public AuthenticationResource(AccessTokenBuilderFactory tokenBuilderFactory, AccessTokenCookieIssuer cookieIssuer, MeterRegistry meterRegistry) {
+  public AuthenticationResource(AccessTokenBuilderFactory tokenBuilderFactory, AccessTokenCookieIssuer cookieIssuer, MeterRegistry meterRegistry, ScmEventBus eventBus) {
     this.tokenBuilderFactory = tokenBuilderFactory;
     this.cookieIssuer = cookieIssuer;
     this.loginAttemptsCounter = AuthenticationMetrics.loginAttempts(meterRegistry, AUTH_METRIC_TYPE);
     this.loginFailedCounter = AuthenticationMetrics.loginFailed(meterRegistry, AUTH_METRIC_TYPE);
     this.logoutCounter = AuthenticationMetrics.logout(meterRegistry, AUTH_METRIC_TYPE);
+    this.eventBus = eventBus;
   }
 
   @POST
@@ -134,6 +138,7 @@ public class AuthenticationResource {
       schema = @Schema(implementation = ErrorDto.class)
     )
   )
+  @AllowAnonymousAccess
   public Response authenticateViaForm(
     @Context HttpServletRequest request,
     @Context HttpServletResponse response,
@@ -174,6 +179,7 @@ public class AuthenticationResource {
       schema = @Schema(implementation = ErrorDto.class)
     )
   )
+  @AllowAnonymousAccess
   public Response authenticateViaJSONBody(
     @Context HttpServletRequest request,
     @Context HttpServletResponse response,
@@ -238,7 +244,12 @@ public class AuthenticationResource {
   public Response logout(@Context HttpServletRequest request, @Context HttpServletResponse response) {
     logoutCounter.increment();
 
-    SecurityUtils.getSubject().logout();
+    Subject subject = SecurityUtils.getSubject();
+    String primaryPrincipal = subject.getPrincipals().getPrimaryPrincipal().toString();
+    subject.logout();
+
+    eventBus.post(new LogoutEvent(primaryPrincipal));
+
     // remove authentication cookie
     cookieIssuer.invalidate(request, response);
 

scm-webapp/src/main/java/sonia/scm/group/DefaultGroupCollector.java

@@ -25,12 +25,16 @@
 package sonia.scm.group;
 
 import com.cronutils.utils.VisibleForTesting;
+import com.github.legman.Subscribe;
 import com.google.common.collect.ImmutableSet;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import sonia.scm.HandlerEventType;
 import sonia.scm.cache.Cache;
 import sonia.scm.cache.CacheManager;
 import sonia.scm.security.Authentications;
+import sonia.scm.security.LogoutEvent;
+import sonia.scm.user.UserEvent;
 
 import javax.inject.Inject;
 import javax.inject.Singleton;
@@ -78,6 +82,19 @@ public class DefaultGroupCollector implements GroupCollector {
     return groups;
   }
 
+  @Subscribe(async = false)
+  public void clearCacheOnLogOut(LogoutEvent event) {
+    String principal = event.getPrimaryPrincipal();
+    cache.remove(principal);
+  }
+
+  @Subscribe(async = false)
+  public void clearCacheOnUserDeletion(UserEvent event) {
+    if (event.getEventType().equals(HandlerEventType.DELETE)) {
+      cache.remove(event.getItem().getName());
+    }
+  }
+
   private void appendInternalGroups(String principal, ImmutableSet.Builder<String> builder) {
     for (Group group : groupDAO.getAll()) {
       if (group.isMember(principal)) {

scm-webapp/src/main/java/sonia/scm/security/SecurityRequests.java

@@ -21,7 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-    
+
 package sonia.scm.security;
 
 import javax.servlet.http.HttpServletRequest;
@@ -41,7 +41,11 @@ public final class SecurityRequests {
 
   public static boolean isAuthenticationRequest(HttpServletRequest request) {
     String uri = request.getRequestURI().substring(request.getContextPath().length());
-    return isAuthenticationRequest(uri);
+    return isAuthenticationRequest(uri) && !isLogoutMethod(request);
+  }
+
+  private static boolean isLogoutMethod(HttpServletRequest request) {
+    return "DELETE".equals(request.getMethod());
   }
 
   public static boolean isAuthenticationRequest(String uri) {

scm-webapp/src/test/java/sonia/scm/api/v2/resources/AuthenticationResourceTest.java

@@ -21,7 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-    
+
 package sonia.scm.api.v2.resources;
 
 import com.github.sdorra.shiro.ShiroRule;
@@ -35,14 +35,17 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 import sonia.scm.config.ScmConfiguration;
+import sonia.scm.event.ScmEventBus;
 import sonia.scm.security.AccessToken;
 import sonia.scm.security.AccessTokenBuilder;
 import sonia.scm.security.AccessTokenBuilderFactory;
 import sonia.scm.security.AccessTokenCookieIssuer;
 import sonia.scm.security.DefaultAccessTokenCookieIssuer;
+import sonia.scm.security.LogoutEvent;
 import sonia.scm.web.RestDispatcher;
 
 import javax.servlet.http.HttpServletRequest;
@@ -52,14 +55,14 @@ import java.io.UnsupportedEncodingException;
 import java.net.URISyntaxException;
 import java.util.List;
 import java.util.Optional;
-import java.util.stream.Stream;
 
 import static java.net.URI.create;
 import static java.util.Optional.of;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 @SubjectAware(
@@ -79,6 +82,9 @@ public class AuthenticationResourceTest {
   @Mock
   private AccessTokenBuilder accessTokenBuilder;
 
+  @Mock
+  private ScmEventBus eventBus;
+
   private MeterRegistry meterRegistry;
 
   private final AccessTokenCookieIssuer cookieIssuer = new DefaultAccessTokenCookieIssuer(mock(ScmConfiguration.class));
@@ -142,7 +148,7 @@ public class AuthenticationResourceTest {
   @Before
   public void prepareEnvironment() {
     meterRegistry = new SimpleMeterRegistry();
-    authenticationResource = new AuthenticationResource(accessTokenBuilderFactory, cookieIssuer, meterRegistry);
+    authenticationResource = new AuthenticationResource(accessTokenBuilderFactory, cookieIssuer, meterRegistry, eventBus);
     dispatcher.addSingletonResource(authenticationResource);
 
     AccessToken accessToken = mock(AccessToken.class);
@@ -248,9 +254,29 @@ public class AuthenticationResourceTest {
     Optional<Meter> logoutMeter = meters.stream().filter(m -> m.getId().getName().equals("scm.auth.logout")).findFirst();
     assertThat(logoutMeter).isPresent();
     assertThat(logoutMeter.get().measure().iterator().next().getValue()).isEqualTo(1);
+
+    verify(eventBus).post(any(LogoutEvent.class));
   }
 
   @Test
+  @SubjectAware(username = "trillian", password = "secret")
+  public void shouldFireLogoutEvent() throws URISyntaxException {
+    MockHttpRequest request = MockHttpRequest.delete("/" + AuthenticationResource.PATH + "/access_token");
+
+    dispatcher.invoke(request, response);
+    assertEquals(HttpServletResponse.SC_NO_CONTENT, response.getStatus());
+
+    ArgumentCaptor<Object> captor = ArgumentCaptor.forClass(Object.class);
+    verify(eventBus).post(captor.capture());
+
+    Object event = captor.getValue();
+    assertThat(event).isInstanceOfSatisfying(LogoutEvent.class, logoutEvent -> {
+      assertThat(logoutEvent.getPrimaryPrincipal()).isEqualTo("trillian");
+    });
+  }
+
+  @Test
+  @SubjectAware(username = "trillian", password = "secret")
   public void shouldHandleLogoutRedirection() throws URISyntaxException, UnsupportedEncodingException {
     authenticationResource.setLogoutRedirection(() -> of(create("http://example.com/cas/logout")));
 
@@ -263,6 +289,7 @@ public class AuthenticationResourceTest {
   }
 
   @Test
+  @SubjectAware(username = "trillian", password = "secret")
   public void shouldHandleDisabledLogoutRedirection() throws URISyntaxException {
     authenticationResource.setLogoutRedirection(Optional::empty);
 

scm-webapp/src/test/java/sonia/scm/group/DefaultGroupCollectorTest.java

@@ -32,10 +32,12 @@ import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
-import sonia.scm.SCMContext;
+import sonia.scm.HandlerEventType;
 import sonia.scm.cache.MapCache;
 import sonia.scm.cache.MapCacheManager;
-import sonia.scm.security.Authentications;
+import sonia.scm.security.LogoutEvent;
+import sonia.scm.user.User;
+import sonia.scm.user.UserEvent;
 
 import java.util.HashSet;
 import java.util.List;
@@ -98,6 +100,27 @@ class DefaultGroupCollectorTest {
     verify(groupResolver, never()).resolve("trillian");
   }
 
+  @Test
+  void shouldClearCacheOnLogout() {
+    MapCache<String, Set<String>> cache = mapCacheManager.getCache(DefaultGroupCollector.CACHE_NAME);
+    cache.put("trillian", ImmutableSet.of("awesome", "incredible"));
+
+    collector.clearCacheOnLogOut(new LogoutEvent("trillian"));
+
+    assertThat(cache.get("trillian")).isNull();
+  }
+
+
+  @Test
+  void shouldClearCacheOnUserDeletion() {
+    MapCache<String, Set<String>> cache = mapCacheManager.getCache(DefaultGroupCollector.CACHE_NAME);
+    cache.put("trillian", ImmutableSet.of("awesome", "incredible"));
+
+    collector.clearCacheOnUserDeletion(new UserEvent(HandlerEventType.DELETE, new User("trillian")));
+
+    assertThat(cache.get("trillian")).isNull();
+  }
+
   @Test
   void shouldNotCallResolverForAnonymous() {
     groupResolvers.add(groupResolver);

scm-webapp/src/test/java/sonia/scm/security/SecurityRequestsTest.java

@@ -21,41 +21,51 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-    
+
 package sonia.scm.security;
 
-import org.junit.Test;
-import org.junit.runner.RunWith;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.Mock;
-import org.mockito.junit.MockitoJUnitRunner;
+import org.mockito.junit.jupiter.MockitoExtension;
 
 import javax.servlet.http.HttpServletRequest;
 
-import static org.junit.Assert.*;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.when;
 
 /**
  * Created by masuewer on 04.07.18.
  */
-@RunWith(MockitoJUnitRunner.class)
-public class SecurityRequestsTest {
+@ExtendWith(MockitoExtension.class)
+class SecurityRequestsTest {
 
   @Mock
   private HttpServletRequest request;
 
   @Test
-  public void testIsAuthenticationRequestWithContextPath() {
+  void shouldReturnTrueWithContextPath() {
     when(request.getRequestURI()).thenReturn("/scm/api/auth/access_token");
     when(request.getContextPath()).thenReturn("/scm");
 
-    assertTrue(SecurityRequests.isAuthenticationRequest(request));
+    assertThat(SecurityRequests.isAuthenticationRequest(request)).isTrue();
   }
 
   @Test
-  public void testIsAuthenticationRequest() throws Exception {
-    assertTrue(SecurityRequests.isAuthenticationRequest("/api/auth/access_token"));
-    assertTrue(SecurityRequests.isAuthenticationRequest("/api/v2/auth/access_token"));
-    assertFalse(SecurityRequests.isAuthenticationRequest("/api/repositories"));
-    assertFalse(SecurityRequests.isAuthenticationRequest("/api/v2/repositories"));
+  void shouldDetectAuthenticationResource() {
+    assertThat(SecurityRequests.isAuthenticationRequest("/api/auth/access_token")).isTrue();
+    assertThat(SecurityRequests.isAuthenticationRequest("/api/v2/auth/access_token")).isTrue();
+    assertThat(SecurityRequests.isAuthenticationRequest("/api/repositories")).isFalse();
+    assertThat(SecurityRequests.isAuthenticationRequest("/api/v2/repositories")).isFalse();
   }
+
+  @Test
+  void shouldReturnFalseForLogout() {
+    when(request.getRequestURI()).thenReturn("/scm/api/auth/access_token");
+    when(request.getContextPath()).thenReturn("/scm");
+    when(request.getMethod()).thenReturn("DELETE");
+
+    assertThat(SecurityRequests.isAuthenticationRequest(request)).isFalse();
+  }
+
 }