You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@isis.apache.org by da...@apache.org on 2021/07/14 07:59:41 UTC

[isis] 01/02: ISIS-2793: fixes caching in AuthenticatedWebSessionForIsis

This is an automated email from the ASF dual-hosted git repository.

danhaywood pushed a commit to branch ISIS-2793-rewrite
in repository https://gitbox.apache.org/repos/asf/isis.git

commit 041fb783e58861cf608191fd01a553191b9283c0
Author: danhaywood <da...@haywood-associates.co.uk>
AuthorDate: Wed Jul 14 08:55:50 2021 +0100

    ISIS-2793: fixes caching in AuthenticatedWebSessionForIsis
---
 .../interaction/integration/IsisRequestCycle.java  | 73 --------------------
 .../manager/AuthenticationManager.java             |  3 +
 .../keycloak/IsisModuleSecurityKeycloak.java       | 20 ++----
 .../keycloak/handler/LogoutHandlerForKeycloak.java | 60 ++++++++++++++++
 .../AuthenticatedWebSessionForIsis.java            |  8 ++-
 .../viewer/integration/WebRequestCycleForIsis.java | 79 ++++++++--------------
 6 files changed, 106 insertions(+), 137 deletions(-)

diff --git a/core/interaction/src/main/java/org/apache/isis/core/interaction/integration/IsisRequestCycle.java b/core/interaction/src/main/java/org/apache/isis/core/interaction/integration/IsisRequestCycle.java
deleted file mode 100644
index 2f6432c..0000000
--- a/core/interaction/src/main/java/org/apache/isis/core/interaction/integration/IsisRequestCycle.java
+++ /dev/null
@@ -1,73 +0,0 @@
-/*
- *  Licensed to the Apache Software Foundation (ASF) under one
- *  or more contributor license agreements.  See the NOTICE file
- *  distributed with this work for additional information
- *  regarding copyright ownership.  The ASF licenses this file
- *  to you under the Apache License, Version 2.0 (the
- *  "License"); you may not use this file except in compliance
- *  with the License.  You may obtain a copy of the License at
- *
- *        http://www.apache.org/licenses/LICENSE-2.0
- *
- *  Unless required by applicable law or agreed to in writing,
- *  software distributed under the License is distributed on an
- *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- *  KIND, either express or implied.  See the License for the
- *  specific language governing permissions and limitations
- *  under the License.
- */
-package org.apache.isis.core.interaction.integration;
-
-import org.apache.isis.applib.services.iactnlayer.InteractionContext;
-import org.apache.isis.applib.services.iactnlayer.InteractionService;
-import org.apache.isis.applib.services.user.ImpersonatedUserHolder;
-import org.apache.isis.applib.services.user.UserMemento;
-
-import lombok.RequiredArgsConstructor;
-import lombok.val;
-
-/**
- *
- * @since 2.0
- */
-@RequiredArgsConstructor(staticName = "next")
-public class IsisRequestCycle {
-
-    private final InteractionService interactionService;
-    private final ImpersonatedUserHolder impersonatedUserHolder;
-
-    // -- SUPPORTING WEB REQUEST CYCLE FOR ISIS ...
-
-    public void onBeginRequest(final InteractionContext authenticatedContext) {
-
-        val contextToUse = impersonatedUserHolder.getUserMemento()
-                .map(impersonatingUserMemento->
-                    InteractionContext
-                        .ofUserWithSystemDefaults(
-                                merge(
-                                        authenticatedContext.getUser(),
-                                        impersonatingUserMemento)))
-                .orElse(authenticatedContext);
-
-        interactionService.openInteraction(contextToUse);
-    }
-
-    public void onRequestHandlerExecuted() {
-
-    }
-
-    public void onEndRequest() {
-        interactionService.closeInteractionLayers();
-    }
-
-    // -- HELPER
-
-    // not sure if this is strictly required; idea is to preserve some state from the origin user
-    private static UserMemento merge(UserMemento origin, UserMemento fake) {
-        return fake
-                .withAuthenticationSource(origin.getAuthenticationSource())
-                .withAuthenticationCode(origin.getAuthenticationCode());
-    }
-
-
-}
diff --git a/core/security/src/main/java/org/apache/isis/core/security/authentication/manager/AuthenticationManager.java b/core/security/src/main/java/org/apache/isis/core/security/authentication/manager/AuthenticationManager.java
index 37d12df..8f03b81 100644
--- a/core/security/src/main/java/org/apache/isis/core/security/authentication/manager/AuthenticationManager.java
+++ b/core/security/src/main/java/org/apache/isis/core/security/authentication/manager/AuthenticationManager.java
@@ -145,6 +145,9 @@ public class AuthenticationManager {
         if(userMemento.getAuthenticationSource().isExternal()) {
             return true;
         }
+        if(userMemento.isImpersonating()) {
+            return true;
+        }
         final String userName = userByValidationCode.get(userMemento.getAuthenticationCode());
         return authentication.getUser().isCurrentUser(userName);
     }
diff --git a/security/keycloak/src/main/java/org/apache/isis/security/keycloak/IsisModuleSecurityKeycloak.java b/security/keycloak/src/main/java/org/apache/isis/security/keycloak/IsisModuleSecurityKeycloak.java
index c3637ee..fef4c70 100644
--- a/security/keycloak/src/main/java/org/apache/isis/security/keycloak/IsisModuleSecurityKeycloak.java
+++ b/security/keycloak/src/main/java/org/apache/isis/security/keycloak/IsisModuleSecurityKeycloak.java
@@ -28,7 +28,6 @@ import javax.servlet.http.HttpServletResponse;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.autoconfigure.security.oauth2.client.OAuth2ClientProperties;
 import org.springframework.context.annotation.Bean;
-import org.springframework.context.annotation.ComponentScan;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Import;
 import org.springframework.security.config.annotation.web.builders.HttpSecurity;
@@ -47,9 +46,9 @@ import static org.springframework.security.oauth2.client.web.OAuth2Authorization
 
 import org.apache.isis.core.runtimeservices.IsisModuleCoreRuntimeServices;
 import org.apache.isis.core.security.authentication.login.LoginSuccessHandler;
-import org.apache.isis.core.security.authentication.manager.AuthenticationManager;
 import org.apache.isis.core.webapp.IsisModuleCoreWebapp;
 import org.apache.isis.security.keycloak.handler.KeycloakLogoutHandler;
+import org.apache.isis.security.keycloak.handler.LogoutHandlerForKeycloak;
 import org.apache.isis.security.keycloak.services.KeycloakOauth2UserService;
 import org.apache.isis.security.spring.IsisModuleSecuritySpring;
 
@@ -57,7 +56,7 @@ import lombok.RequiredArgsConstructor;
 import lombok.val;
 
 /**
- * Configuration Bean to support Isis Security using Shiro.
+ * Configuration Bean to support Isis Security using Keycloak.
  *
  * @since 2.0 {@index}
  */
@@ -67,19 +66,21 @@ import lombok.val;
         IsisModuleCoreRuntimeServices.class,
         IsisModuleCoreWebapp.class,
 
+        // services
+        LogoutHandlerForKeycloak.class,
+
         // builds on top of Spring
         IsisModuleSecuritySpring.class,
 
 })
 @EnableWebSecurity
-@ComponentScan
 public class IsisModuleSecurityKeycloak {
 
     @Bean
     public WebSecurityConfigurerAdapter webSecurityConfigurer(
             @Value("${kc.realm}") String realm,
             KeycloakOauth2UserService keycloakOidcUserService,
-            KeycloakLogoutHandler keycloakLogoutHandler,
+            // KeycloakLogoutHandler keycloakLogoutHandler,
             List<LoginSuccessHandler> loginSuccessHandlers,
             List<LogoutHandler> logoutHandlers
             ) {
@@ -99,7 +100,7 @@ public class IsisModuleSecurityKeycloak {
 
                         // Propagate logouts via /logout to Keycloak
                         .logout()
-                            .addLogoutHandler(keycloakLogoutHandler)
+//                            .addLogoutHandler(keycloakLogoutHandler)
                             .logoutRequestMatcher(new AntPathRequestMatcher("/logout"));
 
                 logoutHandlers.forEach(httpSecurityLogoutConfigurer::addLogoutHandler);
@@ -121,13 +122,6 @@ public class IsisModuleSecurityKeycloak {
         };
     }
 
-    @Bean LoginSuccessHandler loginSuccessHandler(final AuthenticationManager authenticationManager) {
-        return new LoginSuccessHandler() {
-            @Override public void onSuccess() {
-
-            }
-        };
-    }
     @RequiredArgsConstructor
     public static class AuthSuccessHandler extends SavedRequestAwareAuthenticationSuccessHandler {
 
diff --git a/security/keycloak/src/main/java/org/apache/isis/security/keycloak/handler/LogoutHandlerForKeycloak.java b/security/keycloak/src/main/java/org/apache/isis/security/keycloak/handler/LogoutHandlerForKeycloak.java
new file mode 100644
index 0000000..36513d9
--- /dev/null
+++ b/security/keycloak/src/main/java/org/apache/isis/security/keycloak/handler/LogoutHandlerForKeycloak.java
@@ -0,0 +1,60 @@
+package org.apache.isis.security.keycloak.handler;
+
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.security.oauth2.core.oidc.user.OidcUser;
+import org.springframework.stereotype.Service;
+import org.springframework.web.client.RestTemplate;
+import org.springframework.web.util.UriComponentsBuilder;
+
+import org.apache.isis.core.security.authentication.logout.LogoutHandler;
+
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import lombok.val;
+
+/**
+ * Propagates logouts to Keycloak.
+ *
+ * <p>
+ * Necessary because Spring Security 5 (currently) doesn't support
+ * end-session-endpoints.
+ * </p>
+ */
+@Service
+@Slf4j
+@RequiredArgsConstructor
+public class LogoutHandlerForKeycloak implements LogoutHandler {
+
+    private final RestTemplate restTemplate;
+
+    public LogoutHandlerForKeycloak() {
+        this(new RestTemplate());
+    }
+
+    @Override public void logout() {
+        val authentication = SecurityContextHolder.getContext().getAuthentication();
+        if (authentication != null) {
+            propagateLogoutToKeycloak((OidcUser) authentication.getPrincipal());
+        }
+
+    }
+    private void propagateLogoutToKeycloak(OidcUser user) {
+
+        val endSessionEndpoint = String.format("%s/protocol/openid-connect/logout", user.getIssuer());
+
+        val builder = UriComponentsBuilder
+                .fromUriString(endSessionEndpoint)
+                .queryParam("id_token_hint", user.getIdToken().getTokenValue());
+
+        val logoutResponse = restTemplate.getForEntity(builder.toUriString(), String.class);
+        if (logoutResponse.getStatusCode().is2xxSuccessful()) {
+            log.info("Successfully logged out in Keycloak");
+        } else {
+            log.info("Could not propagate logout to Keycloak");
+        }
+    }
+
+    @Override public boolean isHandlingCurrentThread() {
+        return true;
+    }
+}
diff --git a/viewers/wicket/viewer/src/main/java/org/apache/isis/viewer/wicket/viewer/integration/AuthenticatedWebSessionForIsis.java b/viewers/wicket/viewer/src/main/java/org/apache/isis/viewer/wicket/viewer/integration/AuthenticatedWebSessionForIsis.java
index b5937ee..77760a0 100644
--- a/viewers/wicket/viewer/src/main/java/org/apache/isis/viewer/wicket/viewer/integration/AuthenticatedWebSessionForIsis.java
+++ b/viewers/wicket/viewer/src/main/java/org/apache/isis/viewer/wicket/viewer/integration/AuthenticatedWebSessionForIsis.java
@@ -140,6 +140,12 @@ implements BreadcrumbModelProvider, BookmarkedPagesModelProvider, HasCommonConte
         log(SessionLoggingService.Type.LOGOUT, userName, causedBy);
     }
 
+    //
+    // TODO: this seems overly complicated; we appear to be caching the InteractionContext
+    //  in the this.authentication field, and updating it if we ever find it is out of sync.
+    //  A spike to just delegate down to InteractionService#currentInteractionContext() didn't work, so there's
+    //  some hidden subtlety here; but it feels as though it could be simplified...
+    //
     public synchronized InteractionContext getAuthentication() {
 
         commonContext.getInteractionLayerTracker().currentInteractionContext()
@@ -160,7 +166,7 @@ implements BreadcrumbModelProvider, BookmarkedPagesModelProvider, HasCommonConte
                         }
                     } else {
                         // different user name
-                        if (isSignedIn()) {
+                        if (isSignedIn() && !currentAuthentication.getUser().isImpersonating() ){
                             // invalidate previous session
                             super.invalidate();
                         }
diff --git a/viewers/wicket/viewer/src/main/java/org/apache/isis/viewer/wicket/viewer/integration/WebRequestCycleForIsis.java b/viewers/wicket/viewer/src/main/java/org/apache/isis/viewer/wicket/viewer/integration/WebRequestCycleForIsis.java
index d1b53c9..b7dd6fd 100644
--- a/viewers/wicket/viewer/src/main/java/org/apache/isis/viewer/wicket/viewer/integration/WebRequestCycleForIsis.java
+++ b/viewers/wicket/viewer/src/main/java/org/apache/isis/viewer/wicket/viewer/integration/WebRequestCycleForIsis.java
@@ -30,7 +30,6 @@ import org.apache.wicket.Application;
 import org.apache.wicket.IPageFactory;
 import org.apache.wicket.MetaDataKey;
 import org.apache.wicket.Page;
-import org.apache.wicket.RestartResponseException;
 import org.apache.wicket.Session;
 import org.apache.wicket.WicketRuntimeException;
 import org.apache.wicket.authroles.authentication.AuthenticatedWebSession;
@@ -52,12 +51,12 @@ import org.apache.isis.applib.services.exceprecog.ExceptionRecognizerService;
 import org.apache.isis.applib.services.exceprecog.Recognition;
 import org.apache.isis.applib.services.i18n.TranslationContext;
 import org.apache.isis.applib.services.iactn.Interaction;
+import org.apache.isis.applib.services.iactnlayer.InteractionContext;
 import org.apache.isis.applib.services.iactnlayer.InteractionService;
 import org.apache.isis.applib.services.user.ImpersonatedUserHolder;
 import org.apache.isis.commons.collections.Can;
 import org.apache.isis.commons.internal.base._Strings;
 import org.apache.isis.commons.internal.exceptions._Exceptions;
-import org.apache.isis.core.interaction.integration.IsisRequestCycle;
 import org.apache.isis.core.interaction.session.MessageBroker;
 import org.apache.isis.core.metamodel.spec.feature.ObjectMember;
 import org.apache.isis.core.metamodel.specloader.validator.MetaModelInvalidException;
@@ -71,8 +70,8 @@ import org.apache.isis.viewer.wicket.ui.pages.login.WicketSignInPage;
 import org.apache.isis.viewer.wicket.ui.pages.mmverror.MmvErrorPage;
 import org.apache.isis.viewer.wicket.ui.panels.PromptFormAbstract;
 
-import lombok.val;
 import lombok.extern.log4j.Log4j2;
+import lombok.val;
 
 /**
  * Isis-specific implementation of the Wicket's {@link RequestCycle},
@@ -113,9 +112,6 @@ public class WebRequestCycleForIsis implements IRequestCycleListener {
 
     }
 
-    public static final MetaDataKey<IsisRequestCycle> REQ_CYCLE_HANDLE_KEY =
-            new MetaDataKey<IsisRequestCycle>() {private static final long serialVersionUID = 1L; };
-
     private static final MetaDataKey<SessionLifecyclePhase> SESSION_LIFECYCLE_PHASE_KEY =
             new MetaDataKey<SessionLifecyclePhase>() { private static final long serialVersionUID = 1L; };
 
@@ -139,22 +135,32 @@ public class WebRequestCycleForIsis implements IRequestCycleListener {
         }
 
         val commonContext = getCommonContext();
-        val authentication = AuthenticatedWebSessionForIsis.get().getAuthentication();
-
-        if (authentication == null) {
-            log.debug("onBeginRequest out - session was not opened (because no authentication)");
-            return;
+        val interactionService = commonContext.lookupServiceElseFail(InteractionService.class);
+
+        // if there is an interactionContext already (as some authentication mechanisms setup in filters, eg
+        // SpringSecurityFilter), then just use it.
+        // otherwise, take the value cached on AuthenticatedWebSessionForIsis.
+        val interactionContextIfAny = interactionService.currentInteractionContext();
+        val impersonatedUserHolder = commonContext.lookupServiceElseFail(ImpersonatedUserHolder.class);
+        val userMementoImpersonatedIfAny = impersonatedUserHolder.getUserMemento();
+        if(userMementoImpersonatedIfAny.isPresent()) {
+            val userMementoImpersonated = userMementoImpersonatedIfAny.get();
+            interactionService.openInteraction(
+                    InteractionContext.ofUserWithSystemDefaults(userMementoImpersonated));
+
+            // as a side-effect, sync with Wicket viewer
+            AuthenticatedWebSessionForIsis.get().getAuthentication();
+
+        } else {
+            // fallback to using that cached by Wicket viewer
+            val interactionContext = AuthenticatedWebSessionForIsis.get().getAuthentication();
+            if (interactionContext == null) {
+                log.debug("onBeginRequest out - session was not opened (because no authentication)");
+                return;
+            }
+            interactionService.openInteraction(interactionContext);
         }
 
-        val isisRequestCycle = IsisRequestCycle.next(
-                commonContext.lookupServiceElseFail(InteractionService.class),
-                commonContext.lookupServiceElseFail(ImpersonatedUserHolder.class)
-                );
-
-        requestCycle.setMetaData(REQ_CYCLE_HANDLE_KEY, isisRequestCycle);
-
-        isisRequestCycle.onBeginRequest(authentication);
-
         log.debug("onBeginRequest out - session was opened");
     }
 
@@ -222,30 +228,6 @@ public class WebRequestCycleForIsis implements IRequestCycleListener {
     public void onRequestHandlerExecuted(RequestCycle requestCycle, IRequestHandler handler) {
         log.debug("onRequestHandlerExecuted: handler: {}", handler.getClass().getName());
 
-        try {
-
-            val isisRequestCycle = requestCycle.getMetaData(REQ_CYCLE_HANDLE_KEY);
-
-            if(isisRequestCycle!=null) {
-                isisRequestCycle.onRequestHandlerExecuted();
-            }
-
-        } catch(Exception ex) {
-
-            if(handler instanceof RenderPageRequestHandler) {
-                RenderPageRequestHandler requestHandler = (RenderPageRequestHandler) handler;
-                if(requestHandler.getPage() instanceof ErrorPage) {
-                    // do nothing
-                    return;
-                }
-            }
-
-            log.debug("onRequestHandlerExecuted: isisRequestCycle.onRequestHandlerExecuted threw {}",
-                    ex.getClass().getName());
-
-            // shouldn't return null given that we're in a session ...
-            throw new RestartResponseException(errorPageProviderFor(ex), RedirectPolicy.ALWAYS_REDIRECT);
-        }
     }
 
     /**
@@ -256,12 +238,9 @@ public class WebRequestCycleForIsis implements IRequestCycleListener {
 
         log.debug("onEndRequest");
 
-        val isisRequestCycle = requestCycle.getMetaData(REQ_CYCLE_HANDLE_KEY);
-        requestCycle.setMetaData(REQ_CYCLE_HANDLE_KEY, null);
-
-        if(isisRequestCycle!=null) {
-            isisRequestCycle.onEndRequest();
-        }
+        getCommonContext().lookupService(InteractionService.class).ifPresent(
+            InteractionService::closeInteractionLayers
+        );
 
     }