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/17 16:07:14 UTC
[isis] 01/01: ISIS-2793: fixes impersonation
This is an automated email from the ASF dual-hosted git repository.
danhaywood pushed a commit to branch ISIS-2793-v2
in repository https://gitbox.apache.org/repos/asf/isis.git
commit 137e246e1da53ebd7ebb6ef2dd740f5e5b977bce
Author: danhaywood <da...@haywood-associates.co.uk>
AuthorDate: Sat Jul 17 16:44:07 2021 +0100
ISIS-2793: fixes impersonation
Also minor formatting changes and comments.
---
.../isis/applib/services/user/ImpersonateMenu.java | 2 +-
.../applib/id/LogicalTypeTest_valueSemantics.java | 6 +-
.../interaction/integration/IsisRequestCycle.java | 73 --------------------
.../authentication/logout/LogoutHandler.java | 10 +--
.../manager/AuthenticationManager.java | 5 +-
.../spring/webmodule/SpringSecurityFilter.java | 7 +-
.../spring/webmodule/WebModuleSpringSecurity.java | 2 +-
.../wicket/ui/app/logout/LogoutHandlerWkt.java | 13 ++--
.../AuthenticatedWebSessionForIsis.java | 8 ++-
.../viewer/integration/WebRequestCycleForIsis.java | 79 ++++++++--------------
10 files changed, 54 insertions(+), 151 deletions(-)
diff --git a/api/applib/src/main/java/org/apache/isis/applib/services/user/ImpersonateMenu.java b/api/applib/src/main/java/org/apache/isis/applib/services/user/ImpersonateMenu.java
index 360fe3a..9bb74ef 100644
--- a/api/applib/src/main/java/org/apache/isis/applib/services/user/ImpersonateMenu.java
+++ b/api/applib/src/main/java/org/apache/isis/applib/services/user/ImpersonateMenu.java
@@ -78,7 +78,7 @@ public class ImpersonateMenu {
public void impersonate(
final String userName) {
- this.userService.impersonateUser(userName, Collections.emptyList());
+ this.userService.impersonateUser(userName, Collections.singletonList("org.apache.isis.viewer.wicket.roles.USER"));
this.messageService.informUser("Now impersonating " + userName);
}
public boolean hideImpersonate() {
diff --git a/api/applib/src/test/java/org/apache/isis/applib/id/LogicalTypeTest_valueSemantics.java b/api/applib/src/test/java/org/apache/isis/applib/id/LogicalTypeTest_valueSemantics.java
index 0045823..94462f2 100644
--- a/api/applib/src/test/java/org/apache/isis/applib/id/LogicalTypeTest_valueSemantics.java
+++ b/api/applib/src/test/java/org/apache/isis/applib/id/LogicalTypeTest_valueSemantics.java
@@ -24,21 +24,21 @@ import org.apache.isis.applib.SomeDomainClass;
import org.apache.isis.commons.internal.collections._Lists;
import org.apache.isis.core.internaltestsupport.contract.ValueTypeContractTestAbstract;
-public class LogicalTypeTest_valueSemantics
+public class LogicalTypeTest_valueSemantics
extends ValueTypeContractTestAbstract<LogicalType> {
@Override
protected List<LogicalType> getObjectsWithSameValue() {
return _Lists.of(
LogicalType.fqcn(SomeDomainClass.class),
- LogicalType.lazy(SomeDomainClass.class, ()->SomeDomainClass.class.getName()));
+ LogicalType.lazy(SomeDomainClass.class, SomeDomainClass.class::getName));
}
@Override
protected List<LogicalType> getObjectsWithDifferentValue() {
return _Lists.of(
LogicalType.fqcn(Object.class),
- LogicalType.lazy(List.class, ()->List.class.getName()));
+ LogicalType.lazy(List.class, List.class::getName));
}
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/logout/LogoutHandler.java b/core/security/src/main/java/org/apache/isis/core/security/authentication/logout/LogoutHandler.java
index 31b511c..0bf0301 100644
--- a/core/security/src/main/java/org/apache/isis/core/security/authentication/logout/LogoutHandler.java
+++ b/core/security/src/main/java/org/apache/isis/core/security/authentication/logout/LogoutHandler.java
@@ -19,15 +19,7 @@
package org.apache.isis.core.security.authentication.logout;
/**
- *
- * @since Apr 9, 2020
- * TODO we are at early stages of the design, a better idea occurred:
- * actually model the SignIn page as a true ViewModel similar to how we
- * render the home-page; this should allow for the LogoutHandler to be called
- * from the framework more directly and not from within the LogoutMenu's
- * logout action, which is more complicated because, this happens within
- * the context of an IsisInteraction, where we cannot simply purge the
- * current session, when in the middle of an interaction
+ * To allow viewers to close their session when a logout is requested.
*/
public interface LogoutHandler {
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 6336693..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
@@ -113,9 +113,7 @@ public class AuthenticationManager {
}
return null;
-
});
-
}
private String getUnusedRandomCode() {
@@ -147,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/spring/src/main/java/org/apache/isis/security/spring/webmodule/SpringSecurityFilter.java b/security/spring/src/main/java/org/apache/isis/security/spring/webmodule/SpringSecurityFilter.java
index 9199df0..eef18af 100644
--- a/security/spring/src/main/java/org/apache/isis/security/spring/webmodule/SpringSecurityFilter.java
+++ b/security/spring/src/main/java/org/apache/isis/security/spring/webmodule/SpringSecurityFilter.java
@@ -47,6 +47,7 @@ import lombok.val;
public class SpringSecurityFilter implements Filter {
@Autowired private InteractionService interactionService;
+ @Inject List<AuthenticationConverter> converters;
@Override
public void doFilter(
@@ -64,14 +65,13 @@ public class SpringSecurityFilter implements Filter {
}
UserMemento userMemento = null;
- for (AuthenticationConverter converter : converters) {
+ for (final AuthenticationConverter converter : converters) {
try {
userMemento = converter.convert(springAuthentication);
if(userMemento != null) {
break;
}
- } catch(Exception ex) {
- continue;
+ } catch(final Exception ignored) {
}
}
@@ -89,5 +89,4 @@ public class SpringSecurityFilter implements Filter {
()->filterChain.doFilter(servletRequest, servletResponse));
}
- @Inject List<AuthenticationConverter> converters;
}
diff --git a/security/spring/src/main/java/org/apache/isis/security/spring/webmodule/WebModuleSpringSecurity.java b/security/spring/src/main/java/org/apache/isis/security/spring/webmodule/WebModuleSpringSecurity.java
index ddd0717..44f7d2a 100644
--- a/security/spring/src/main/java/org/apache/isis/security/spring/webmodule/WebModuleSpringSecurity.java
+++ b/security/spring/src/main/java/org/apache/isis/security/spring/webmodule/WebModuleSpringSecurity.java
@@ -24,10 +24,10 @@ import javax.servlet.ServletContext;
import javax.servlet.ServletContextListener;
import javax.servlet.ServletException;
-import org.apache.isis.applib.annotation.PriorityPrecedence;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.stereotype.Service;
+import org.apache.isis.applib.annotation.PriorityPrecedence;
import org.apache.isis.applib.services.inject.ServiceInjector;
import org.apache.isis.commons.collections.Can;
import org.apache.isis.core.webapp.modules.WebModuleAbstract;
diff --git a/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/app/logout/LogoutHandlerWkt.java b/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/app/logout/LogoutHandlerWkt.java
index 7bc4045..4dbd4be 100644
--- a/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/app/logout/LogoutHandlerWkt.java
+++ b/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/app/logout/LogoutHandlerWkt.java
@@ -18,22 +18,22 @@
*/
package org.apache.isis.viewer.wicket.ui.app.logout;
-import javax.inject.Inject;
-
import org.apache.wicket.authroles.authentication.AuthenticatedWebSession;
import org.apache.wicket.request.cycle.RequestCycle;
-import org.springframework.stereotype.Service;
import org.apache.isis.applib.services.iactnlayer.InteractionLayerTracker;
import org.apache.isis.core.interaction.session.IsisInteraction;
import org.apache.isis.core.security.authentication.logout.LogoutHandler;
+import org.springframework.stereotype.Service;
+import lombok.RequiredArgsConstructor;
import lombok.val;
@Service
+@RequiredArgsConstructor
public class LogoutHandlerWkt implements LogoutHandler {
- @Inject InteractionLayerTracker iInteractionLayerTracker;
+ final InteractionLayerTracker interactionLayerTracker;
@Override
public void logout() {
@@ -43,8 +43,8 @@ public class LogoutHandlerWkt implements LogoutHandler {
return;
}
- if(iInteractionLayerTracker.isInInteraction()) {
- iInteractionLayerTracker.currentInteraction()
+ if(interactionLayerTracker.isInInteraction()) {
+ interactionLayerTracker.currentInteraction()
.map(IsisInteraction.class::cast)
.ifPresent(interaction->
interaction.setOnClose(currentWktSession::invalidateNow));
@@ -59,5 +59,4 @@ public class LogoutHandlerWkt implements LogoutHandler {
return RequestCycle.get()!=null;
}
-
}
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
+ );
}