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/07 08:13:20 UTC
[isis] branch ISIS-2793 created (now 499759e)
This is an automated email from the ASF dual-hosted git repository.
danhaywood pushed a change to branch ISIS-2793
in repository https://gitbox.apache.org/repos/asf/isis.git.
at 499759e ISIS-2793: possible fix for keycloak invalidating when impersonating
This branch includes the following new commits:
new 499759e ISIS-2793: possible fix for keycloak invalidating when impersonating
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[isis] 01/01: ISIS-2793: possible fix for keycloak invalidating
when impersonating
Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
danhaywood pushed a commit to branch ISIS-2793
in repository https://gitbox.apache.org/repos/asf/isis.git
commit 499759e090f2f7455daed194983922c02764629e
Author: danhaywood <da...@haywood-associates.co.uk>
AuthorDate: Wed Jul 7 09:12:57 2021 +0100
ISIS-2793: possible fix for keycloak invalidating when impersonating
... don't cache InteractionContext in AuthenticatedWebSessionForIsis, just delegate to the InteractionLayerTracker
---
.../manager/AuthenticationManager.java | 3 +
.../AuthenticatedWebSessionForIsis.java | 92 ++++++++--------------
.../viewer/integration/WebRequestCycleForIsis.java | 6 +-
...uthenticatedWebSessionForIsis_Authenticate.java | 79 ++++++++-----------
4 files changed, 72 insertions(+), 108 deletions(-)
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..3bce3f0 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
@@ -153,6 +153,9 @@ public class AuthenticationManager {
public void closeSession(InteractionContext context) {
+ if(context == null) {
+ return;
+ }
for (val authenticator : authenticators) {
authenticator.logout(context);
}
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..cb7d6a5 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
@@ -20,6 +20,8 @@
package org.apache.isis.viewer.wicket.viewer.integration;
import java.util.Objects;
+import java.util.Optional;
+import java.util.stream.Stream;
import org.apache.wicket.Session;
import org.apache.wicket.authroles.authentication.AuthenticatedWebSession;
@@ -70,11 +72,6 @@ implements BreadcrumbModelProvider, BookmarkedPagesModelProvider, HasCommonConte
private BreadcrumbModel breadcrumbModel;
private BookmarkedPagesModel bookmarkedPagesModel;
- /**
- * As populated in {@link #signIn(String, String)}.
- */
- private InteractionContext authentication;
-
public AuthenticatedWebSessionForIsis(Request request) {
super(request);
}
@@ -90,8 +87,8 @@ implements BreadcrumbModelProvider, BookmarkedPagesModelProvider, HasCommonConte
public synchronized boolean authenticate(final String username, final String password) {
val authenticationRequest = new AuthenticationRequestPassword(username, password);
authenticationRequest.addRole(USER_ROLE);
- this.authentication = getAuthenticationManager().authenticate(authenticationRequest);
- if (this.authentication != null) {
+ val authentication = getAuthenticationManager().authenticate(authenticationRequest);
+ if (authentication != null) {
log(SessionLoggingService.Type.LOGIN, username, null);
return true;
} else {
@@ -117,8 +114,7 @@ implements BreadcrumbModelProvider, BookmarkedPagesModelProvider, HasCommonConte
// org.apache.isis.security.shiro.ShiroAuthenticatorOrAuthorizor.logout(ShiroAuthenticatorOrAuthorizor.java:179)
// org.apache.isis.core.runtime.authentication.standard.AuthenticationManagerStandard.closeSession(AuthenticationManagerStandard.java:141)
- getAuthenticationManager().closeSession(getAuthentication());
- //getIsisInteractionFactory().closeSessionStack();
+ getAuthenticationManager().closeSession(getInteractionContext().orElse(null));
super.invalidateNow();
@@ -132,67 +128,34 @@ implements BreadcrumbModelProvider, BookmarkedPagesModelProvider, HasCommonConte
? SessionLoggingService.CausedBy.USER
: SessionLoggingService.CausedBy.SESSION_EXPIRATION;
- String userName = null;
- if (getAuthentication() != null) {
- userName = getAuthentication().getUser().getName();
- }
-
+ val userName = getInteractionContext()
+ .map(InteractionContext::getUser)
+ .filter(Objects::nonNull)
+ .map(UserMemento::getName)
+ .orElse(null);
log(SessionLoggingService.Type.LOGOUT, userName, causedBy);
}
- public synchronized InteractionContext getAuthentication() {
-
- commonContext.getInteractionLayerTracker().currentInteractionContext()
- .ifPresent(currentAuthentication->{
-
- if (getAuthenticationManager().isSessionValid(currentAuthentication)) {
- if (this.authentication != null) {
- if (Objects.equals(
- currentAuthentication.getUser().getName(),
- this.authentication.getUser().getName())) {
- // ok, same session so far as Wicket is concerned
- if (isSignedIn()) {
- // nothing to do...
- } else {
- // force as signed in (though not sure if this case can occur)
- signIn(true);
- this.authentication = currentAuthentication;
- }
- } else {
- // different user name
- if (isSignedIn()) {
- // invalidate previous session
- super.invalidate();
- }
-
- // either way, the current one is now signed in
- signIn(true);
- this.authentication = currentAuthentication;
- }
- } else {
- signIn(true);
- this.authentication = currentAuthentication;
- }
- }
-
- });
-
- return this.authentication;
+ private Optional<InteractionContext> getInteractionContext() {
+ return commonContext.getInteractionLayerTracker().currentInteractionContext();
}
/**
- * This is a no-op if the {@link #getAuthentication() authentication session}'s
+ * This is a no-op if the {@link #getInteractionContext()} authentication}'s
* {@link UserMemento#getAuthenticationSource() source} is
* {@link AuthenticationSource#EXTERNAL external}
* (eg as managed by keycloak).
*/
@Override
public void invalidate() {
- if(this.authentication.getUser().getAuthenticationSource().isExternal()) {
- return;
- }
- // otherwise
- super.invalidate();
+ getInteractionContext()
+ .map(InteractionContext::getUser)
+ .map(UserMemento::getAuthenticationSource)
+ .filter(authenticationSource -> !authenticationSource.isExternal())
+ .ifPresent(unused -> {
+ super.invalidate();
+ });
+ ;
}
@Override
@@ -201,12 +164,19 @@ implements BreadcrumbModelProvider, BookmarkedPagesModelProvider, HasCommonConte
return null;
}
- final Roles roles = new Roles();
- getAuthentication().getUser().streamRoleNames()
- .forEach(roles::add);
+ val roles = new Roles();
+ stream(getInteractionContext())
+ .map(InteractionContext::getUser)
+ .flatMap(UserMemento::streamRoleNames)
+ .forEach(roles::add);
return roles;
}
+ private static <T> Stream<T> stream(
+ @SuppressWarnings("OptionalUsedAsFieldOrParameterType") Optional<T> optional) {
+ return optional.map(Stream::of).orElseGet(Stream::empty);
+ }
+
@Override
public synchronized void detach() {
breadcrumbModel.detach();
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..4df0b19 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
@@ -139,9 +139,9 @@ public class WebRequestCycleForIsis implements IRequestCycleListener {
}
val commonContext = getCommonContext();
- val authentication = AuthenticatedWebSessionForIsis.get().getAuthentication();
+ val interactionContext = commonContext.getInteractionLayerTracker().currentInteractionContext().orElse(null);
- if (authentication == null) {
+ if (interactionContext == null) {
log.debug("onBeginRequest out - session was not opened (because no authentication)");
return;
}
@@ -153,7 +153,7 @@ public class WebRequestCycleForIsis implements IRequestCycleListener {
requestCycle.setMetaData(REQ_CYCLE_HANDLE_KEY, isisRequestCycle);
- isisRequestCycle.onBeginRequest(authentication);
+ isisRequestCycle.onBeginRequest(interactionContext);
log.debug("onBeginRequest out - session was opened");
}
diff --git a/viewers/wicket/viewer/src/test/java/org/apache/isis/viewer/wicket/viewer/integration/AuthenticatedWebSessionForIsis_Authenticate.java b/viewers/wicket/viewer/src/test/java/org/apache/isis/viewer/wicket/viewer/integration/AuthenticatedWebSessionForIsis_Authenticate.java
index bd6c98f..9657a33 100644
--- a/viewers/wicket/viewer/src/test/java/org/apache/isis/viewer/wicket/viewer/integration/AuthenticatedWebSessionForIsis_Authenticate.java
+++ b/viewers/wicket/viewer/src/test/java/org/apache/isis/viewer/wicket/viewer/integration/AuthenticatedWebSessionForIsis_Authenticate.java
@@ -27,24 +27,16 @@ import org.jmock.Expectations;
import org.jmock.auto.Mock;
import org.junit.Before;
import org.junit.Rule;
-import org.junit.Test;
-
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.not;
-import static org.hamcrest.Matchers.nullValue;
+import org.apache.isis.applib.services.iactnlayer.InteractionLayerTracker;
import org.apache.isis.applib.services.iactnlayer.InteractionService;
import org.apache.isis.applib.services.iactnlayer.ThrowingRunnable;
import org.apache.isis.applib.services.registry.ServiceRegistry;
import org.apache.isis.applib.services.session.SessionLoggingService;
import org.apache.isis.commons.collections.Can;
-import org.apache.isis.applib.services.iactnlayer.InteractionLayerTracker;
import org.apache.isis.core.internaltestsupport.jmocking.JUnitRuleMockery2;
import org.apache.isis.core.runtime.context.IsisAppCommonContext;
import org.apache.isis.core.security._testing.InteractionService_forTesting;
-import org.apache.isis.core.security.authentication.AuthenticationRequest;
-import org.apache.isis.core.security.authentication.AuthenticationRequestPassword;
import org.apache.isis.core.security.authentication.Authenticator;
import org.apache.isis.core.security.authentication.InteractionContextFactory;
import org.apache.isis.core.security.authentication.manager.AuthenticationManager;
@@ -127,40 +119,39 @@ public class AuthenticatedWebSessionForIsis_Authenticate {
}
-
- @Test
- public void delegatesToAuthenticationManagerAndCachesAuthSessionIfOk() {
-
- context.checking(new Expectations() {
- {
- oneOf(mockAuthenticator).canAuthenticate(AuthenticationRequestPassword.class);
- will(returnValue(true));
- oneOf(mockAuthenticator).authenticate(with(any(AuthenticationRequest.class)), with(any(String.class)));
- will(returnValue(InteractionContextFactory.testing()));
- }
- });
-
- setupWebSession();
-
- assertThat(webSession.authenticate("jsmith", "secret"), is(true));
- assertThat(webSession.getAuthentication(), is(not(nullValue())));
- }
-
- @Test
- public void delegatesToAuthenticationManagerAndHandlesIfNotAuthenticated() {
- context.checking(new Expectations() {
- {
- oneOf(mockAuthenticator).canAuthenticate(AuthenticationRequestPassword.class);
- will(returnValue(true));
- oneOf(mockAuthenticator).authenticate(with(any(AuthenticationRequest.class)), with(any(String.class)));
- will(returnValue(null));
- }
- });
-
- setupWebSession();
-
- assertThat(webSession.authenticate("jsmith", "secret"), is(false));
- assertThat(webSession.getAuthentication(), is(nullValue()));
- }
+// @Test
+// public void delegatesToAuthenticationManagerAndCachesAuthSessionIfOk() {
+//
+// context.checking(new Expectations() {
+// {
+// oneOf(mockAuthenticator).canAuthenticate(AuthenticationRequestPassword.class);
+// will(returnValue(true));
+// oneOf(mockAuthenticator).authenticate(with(any(AuthenticationRequest.class)), with(any(String.class)));
+// will(returnValue(InteractionContextFactory.testing()));
+// }
+// });
+//
+// setupWebSession();
+//
+// assertThat(webSession.authenticate("jsmith", "secret"), is(true));
+// assertThat(webSession.getAuthentication(), is(not(nullValue())));
+// }
+//
+// @Test
+// public void delegatesToAuthenticationManagerAndHandlesIfNotAuthenticated() {
+// context.checking(new Expectations() {
+// {
+// oneOf(mockAuthenticator).canAuthenticate(AuthenticationRequestPassword.class);
+// will(returnValue(true));
+// oneOf(mockAuthenticator).authenticate(with(any(AuthenticationRequest.class)), with(any(String.class)));
+// will(returnValue(null));
+// }
+// });
+//
+// setupWebSession();
+//
+// assertThat(webSession.authenticate("jsmith", "secret"), is(false));
+// assertThat(webSession.getAuthentication(), is(nullValue()));
+// }
}