Skip to content

Commit

Permalink
XWIKI-22430: Logging out does not unlock pages that were being edited
Browse files Browse the repository at this point in the history 
  * Provide a new UserUnauthenticatedEvent to detect whenever a user
    logged out, the same way that we have UserAuthenticatedEvent
  * Trigger that new event when processing a logout in
    MyFormAuthenticator
  * Refactor the code in XWikiHibernateStore to listen for
    UserUnauthenticatedEvent for cleaning the locks of a user, instead
of listening to an action event
  * Rename UserAuthenticatedEventNotifier and use it for both
    authenticated and unauthenticated events
  * Fix test
  • Loading branch information
@surli
surli committed Aug 29, 2024
1 parent ca47e4b commit 5994612
Show file tree
Hide file tree
Showing 7 changed files with 174 additions and 56 deletions.
47 changes: 37 additions & 10 deletions .../user/UserAuthenticatedEventNotifier.java → ...user/UserAuthenticationEventNotifier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,22 @@
package com.xpn.xwiki.internal.user;

import javax.inject.Inject;
import javax.inject.Provider;
import javax.inject.Singleton;

import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;

import org.xwiki.component.annotation.Component;
import org.xwiki.observation.ObservationManager;
import org.xwiki.observation.event.Event;
import org.xwiki.security.authentication.UserAuthenticatedEvent;
import org.xwiki.security.authentication.UserUnauthenticatedEvent;
import org.xwiki.user.UserReference;
import org.xwiki.user.UserReferenceResolver;

import com.xpn.xwiki.XWikiContext;

/**
* This notifier helps dealing with events triggered when a user is authenticated through XWiki Oldcore's
* authenticators. It wraps an {@code ObservationManager} and a {@code UserReferenceResolver} to
Expand All @@ -40,9 +46,9 @@
* @version $Id$
* @since 13.3RC1
*/
@Component(roles = UserAuthenticatedEventNotifier.class)
@Component(roles = UserAuthenticationEventNotifier.class)
@Singleton
public class UserAuthenticatedEventNotifier
public class UserAuthenticationEventNotifier
{

@Inject
Expand All @@ -54,29 +60,50 @@ public class UserAuthenticatedEventNotifier
@Inject
private UserReferenceResolver<String> userReferenceResolver;

@Inject
private Provider<XWikiContext> contextProvider;

/**
* Resolve a string as a {@code UserReference} and notify a {@code UserAuthenticatedEvent} created with that user
* reference.
*
* @param stringUserReference string form of the reference of user that will be resolved as a {@code
* UserReference} and passed to the {@code UserAuthenticatedEvent} instance creation
*/
public void notify(String stringUserReference)
public void notifyUserAuthenticated(String stringUserReference)
{
if (!StringUtils.isBlank(stringUserReference)) {
UserReference userReference = this.userReferenceResolver.resolve(stringUserReference);
if (this.logger.isDebugEnabled()) {
this.logger.debug("User [{}] authenticated", userReference);
}
this.notify(new UserAuthenticatedEvent(userReference));
}
}

/**
* Notify that the given user is now logged out.
*
* @param stringUserReference the user for whom to fire a {@link UserUnauthenticatedEvent}.
*/
public void notifyUserUnauthenticated(String stringUserReference)
{
UserReference userReference = this.userReferenceResolver.resolve(stringUserReference);
this.notify(new UserAuthenticatedEvent(userReference));
if (!StringUtils.isBlank(stringUserReference)) {
UserReference userReference = this.userReferenceResolver.resolve(stringUserReference);
if (this.logger.isDebugEnabled()) {
this.logger.debug("User [{}] unauthenticated", userReference);
}
this.notify(new UserUnauthenticatedEvent(userReference));
}
}

/**
* Notify a {@link UserAuthenticatedEvent} that has already been created.
*
* @param event {@code UserAuthenticatedEvent}
*/
private void notify(UserAuthenticatedEvent event)
private void notify(Event event)
{
if (this.logger.isDebugEnabled()) {
this.logger.debug("User authenticated for [{}]", event.getUserReference());
}
this.observationManager.notify(event, null);
this.observationManager.notify(event, null, this.contextProvider.get());
}
}
50 changes: 28 additions & 22 deletions ...rm-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/store/XWikiHibernateStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@
import org.hibernate.query.Query;
import org.slf4j.Logger;
import org.suigeneris.jrcs.rcs.Version;
import org.xwiki.bridge.event.ActionExecutingEvent;
import org.xwiki.component.annotation.Component;
import org.xwiki.component.manager.ComponentLookupException;
import org.xwiki.component.manager.ComponentManager;
Expand All @@ -86,7 +85,9 @@
import org.xwiki.observation.event.Event;
import org.xwiki.query.QueryException;
import org.xwiki.query.QueryManager;
import org.xwiki.security.authentication.UserUnauthenticatedEvent;
import org.xwiki.store.UnexpectedException;
import org.xwiki.user.UserReferenceSerializer;
import org.xwiki.wiki.descriptor.WikiDescriptorManager;
import org.xwiki.wiki.manager.WikiManagerException;

Expand Down Expand Up @@ -176,6 +177,10 @@ public class XWikiHibernateStore extends XWikiHibernateBaseStore implements XWik
@Named("local")
private EntityReferenceSerializer<String> localEntityReferenceSerializer;

@Inject
@Named("document")
private UserReferenceSerializer<DocumentReference> userReferenceSerializer;

@Inject
private ComponentManager componentManager;

Expand Down Expand Up @@ -2055,7 +2060,7 @@ private void registerLogoutListener()
{
this.observationManager.addListener(new EventListener()
{
private final Event ev = new ActionExecutingEvent();
private static final List<Event> EVENT_LIST = List.of(new UserUnauthenticatedEvent());

@Override
public String getName()
Expand All @@ -2066,59 +2071,60 @@ public String getName()
@Override
public List<Event> getEvents()
{
return Collections.<Event>singletonList(this.ev);
return EVENT_LIST;
}

@Override
public void onEvent(Event event, Object source, Object data)
{
if ("logout".equals(((ActionExecutingEvent) event).getActionName())) {
final XWikiContext ctx = (XWikiContext) data;
if (ctx.getUserReference() != null) {
releaseAllLocksForCurrentUser(ctx);
}
UserUnauthenticatedEvent userUnauthenticatedEvent = (UserUnauthenticatedEvent) event;
if (userUnauthenticatedEvent.getUserReference() != null) {
DocumentReference userDoc = XWikiHibernateStore.this.userReferenceSerializer.serialize(
userUnauthenticatedEvent.getUserReference());
releaseAllLocksForUser(userDoc, (XWikiContext) data);
}
}
});
}

/**
* Release all of the locks held by the currently logged in user.
*
* @param ctx the XWikiContext, used to start the connection and get the user name.
*/
private void releaseAllLocksForCurrentUser(final XWikiContext ctx)
private void releaseAllLocksForUser(final DocumentReference userDoc, final XWikiContext context)
{
try {
executeWrite(ctx, session -> {
executeWrite(context, session -> {
final Query query = session.createQuery("delete from XWikiLock as lock where lock.userName=:userName");
// Using deprecated getUser() because this is how locks are created.
// It would be a maintainibility disaster to use different code paths
// for calculating names when creating and removing.
query.setParameter("userName", ctx.getUser());
query.setParameter("userName", this.compactWikiEntityReferenceSerializer.serialize(userDoc));
query.executeUpdate();

return null;
});
} catch (Exception e) {
String msg = "Error while deleting active locks held by user.";
try {
this.endTransaction(ctx, false);
this.endTransaction(context, false);
} catch (Exception utoh) {
msg += " Failed to commit OR rollback [" + utoh.getMessage() + "]";
}
throw new UnexpectedException(msg, e);
}

// If we're in a non-main wiki & the user is global,
// switch to the global wiki and delete locks held there.
if (!ctx.isMainWiki() && ctx.isMainWiki(ctx.getUserReference().getWikiReference().getName())) {
final String cdb = ctx.getWikiId();
if (this.wikiDescriptorManager.isMainWiki(userDoc.getWikiReference().getName())) {
try {
ctx.setWikiId(ctx.getMainXWiki());
this.releaseAllLocksForCurrentUser(ctx);
} finally {
ctx.setWikiId(cdb);
String currentWiki = context.getWikiId();
for (String wikiId : this.wikiDescriptorManager.getAllIds()) {
if (!currentWiki.equals(wikiId)) {
context.setWikiReference(new WikiReference(wikiId));
this.releaseAllLocksForUser(userDoc, context);
context.setWikiReference(new WikiReference(currentWiki));
}
}
} catch (WikiManagerException e) {
this.logger.error("Error for getting list of wikis to release locks for user [{}]", userDoc, e);
}
}
}
Expand Down
18 changes: 9 additions & 9 deletions ...ki-platform-oldcore/src/main/java/com/xpn/xwiki/user/impl/xwiki/MyBasicAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,20 @@

import com.xpn.xwiki.XWikiContext;
import com.xpn.xwiki.XWikiException;
import com.xpn.xwiki.internal.user.UserAuthenticatedEventNotifier;
import com.xpn.xwiki.internal.user.UserAuthenticationEventNotifier;
import com.xpn.xwiki.web.Utils;

public class MyBasicAuthenticator extends BasicAuthenticator implements XWikiAuthenticator
{

private UserAuthenticatedEventNotifier userAuthenticatedEventNotifier;
private UserAuthenticationEventNotifier userAuthenticationEventNotifier;

private UserAuthenticatedEventNotifier getUserAuthenticatedEventNotifier()
private UserAuthenticationEventNotifier getUserAuthenticatedEventNotifier()
{
if ( this.userAuthenticatedEventNotifier == null ) {
this.userAuthenticatedEventNotifier = Utils.getComponent(UserAuthenticatedEventNotifier.class);
if ( this.userAuthenticationEventNotifier == null ) {
this.userAuthenticationEventNotifier = Utils.getComponent(UserAuthenticationEventNotifier.class);
}
return this.userAuthenticatedEventNotifier;
return this.userAuthenticationEventNotifier;
}

@Override
Expand Down Expand Up @@ -89,7 +89,7 @@ public boolean processLogin(String username, String password, String rememberme,

request.setUserPrincipal(principal);

this.getUserAuthenticatedEventNotifier().notify(principal.getName());
this.getUserAuthenticatedEventNotifier().notifyUserAuthenticated(principal.getName());

return false;
} else {
Expand Down Expand Up @@ -135,8 +135,8 @@ public static Principal checkLogin(SecurityRequestWrapper request, HttpServletRe

// Since this scope is static, no UserAuthenticatedEventNotifier is available
// So we create one here
UserAuthenticatedEventNotifier notifier = Utils.getComponent(UserAuthenticatedEventNotifier.class);
notifier.notify(principal.getName());
UserAuthenticationEventNotifier notifier = Utils.getComponent(UserAuthenticationEventNotifier.class);
notifier.notifyUserAuthenticated(principal.getName());

return principal;
} else {
Expand Down
19 changes: 10 additions & 9 deletions ...iki-platform-oldcore/src/main/java/com/xpn/xwiki/user/impl/xwiki/MyFormAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,21 @@

import com.xpn.xwiki.XWikiContext;
import com.xpn.xwiki.XWikiException;
import com.xpn.xwiki.internal.user.UserAuthenticatedEventNotifier;
import com.xpn.xwiki.internal.user.UserAuthenticationEventNotifier;
import com.xpn.xwiki.web.Utils;

public class MyFormAuthenticator extends FormAuthenticator implements XWikiAuthenticator
{
private static final Logger LOGGER = LoggerFactory.getLogger(MyFormAuthenticator.class);

private UserAuthenticatedEventNotifier userAuthenticatedEventNotifier;
private UserAuthenticationEventNotifier userAuthenticationEventNotifier;

private UserAuthenticatedEventNotifier getUserAuthenticatedEventNotifier()
private UserAuthenticationEventNotifier getUserAuthenticatedEventNotifier()
{
if ( this.userAuthenticatedEventNotifier == null ) {
this.userAuthenticatedEventNotifier = Utils.getComponent(UserAuthenticatedEventNotifier.class);
if ( this.userAuthenticationEventNotifier == null ) {
this.userAuthenticationEventNotifier = Utils.getComponent(UserAuthenticationEventNotifier.class);
}
return this.userAuthenticatedEventNotifier;
return this.userAuthenticationEventNotifier;
}

/**
Expand Down Expand Up @@ -168,7 +168,7 @@ public boolean processLogin(SecurityRequestWrapper request, HttpServletResponse

request.setUserPrincipal(principal);

this.getUserAuthenticatedEventNotifier().notify(principal.getName());
this.getUserAuthenticatedEventNotifier().notifyUserAuthenticated(principal.getName());

} else {
// Failed to authenticate, better cleanup the user stored in the session
Expand Down Expand Up @@ -240,7 +240,7 @@ public boolean processLogin(String username, String password, String rememberme,

request.setUserPrincipal(principal);

this.getUserAuthenticatedEventNotifier().notify(principal.getName());
this.getUserAuthenticatedEventNotifier().notifyUserAuthenticated(principal.getName());

Boolean bAjax = (Boolean) context.get("ajax");
if ((bAjax == null) || (!bAjax.booleanValue())) {
Expand Down Expand Up @@ -292,7 +292,8 @@ public boolean processLogout(SecurityRequestWrapper securityRequestWrapper,
HttpServletResponse httpServletResponse, URLPatternMatcher urlPatternMatcher) throws Exception
{
boolean result = super.processLogout(securityRequestWrapper, httpServletResponse, urlPatternMatcher);
if (result == true) {
if (result) {
this.getUserAuthenticatedEventNotifier().notifyUserUnauthenticated(securityRequestWrapper.getRemoteUser());
if (this.persistentLoginManager != null) {
this.persistentLoginManager.forgetLogin(securityRequestWrapper, httpServletResponse);
}
Expand Down
2 changes: 1 addition & 1 deletion xwiki-platform-core/xwiki-platform-oldcore/src/main/resources/META-INF/components.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ com.xpn.xwiki.internal.render.DefaultOldRendering
com.xpn.xwiki.internal.render.OldRenderingProvider
com.xpn.xwiki.internal.render.groovy.ParseGroovyFromString
com.xpn.xwiki.internal.user.MyPersistentLoginManagerProvider
com.xpn.xwiki.internal.user.UserAuthenticatedEventNotifier
com.xpn.xwiki.internal.user.UserAuthenticationEventNotifier
com.xpn.xwiki.internal.user.UserCreatedEventListener
com.xpn.xwiki.internal.velocity.DefaultVelocityEvaluator
org.xwiki.internal.web.EffectiveAuthorSetterListener
Expand Down
19 changes: 14 additions & 5 deletions ...ore/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/store/XWikiHibernateStoreTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import org.junit.jupiter.api.extension.RegisterExtension;
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.xwiki.bridge.event.ActionExecutingEvent;
import org.xwiki.context.Execution;
import org.xwiki.context.ExecutionContext;
import org.xwiki.model.reference.DocumentReference;
Expand All @@ -51,12 +50,15 @@
import org.xwiki.observation.EventListener;
import org.xwiki.observation.ObservationManager;
import org.xwiki.query.QueryManager;
import org.xwiki.security.authentication.UserUnauthenticatedEvent;
import org.xwiki.test.LogLevel;
import org.xwiki.test.junit5.LogCaptureExtension;
import org.xwiki.test.junit5.mockito.ComponentTest;
import org.xwiki.test.junit5.mockito.InjectMockComponents;
import org.xwiki.test.junit5.mockito.MockComponent;
import org.xwiki.test.mockito.MockitoComponentManager;
import org.xwiki.user.UserReference;
import org.xwiki.user.UserReferenceSerializer;
import org.xwiki.wiki.descriptor.WikiDescriptorManager;
import org.xwiki.wiki.manager.WikiManagerException;

Expand Down Expand Up @@ -143,6 +145,10 @@ public class XWikiHibernateStoreTest
@Named("compactwiki")
private EntityReferenceSerializer<String> compactWikiEntityReferenceSerializer;

@MockComponent
@Named("document")
private UserReferenceSerializer<DocumentReference> userReferenceSerializer;

@MockComponent
private WikiDescriptorManager wikiDescriptorManager;

Expand Down Expand Up @@ -240,14 +246,17 @@ void locksAreReleasedOnLogout() throws Exception

Query query = mock(Query.class);
when(session.createQuery("delete from XWikiLock as lock where lock.userName=:userName")).thenReturn(query);
when(xcontext.getUserReference()).thenReturn(new DocumentReference("xwiki", "XWiki", "LoggerOutter"));
when(xcontext.getUser()).thenReturn("XWiki.LoggerOutter");
UserReference userReference = mock(UserReference.class);
String username = "XWiki.LoggerOutter";
DocumentReference userDoc = new DocumentReference("xwiki", "XWiki", "LoggerOutter");
when(this.userReferenceSerializer.serialize(userReference)).thenReturn(userDoc);
when(this.compactWikiEntityReferenceSerializer.serialize(userDoc)).thenReturn(username);
when(this.hibernateStore.beginTransaction()).thenReturn(true);

// Fire the logout event.
eventListenerCaptor.getValue().onEvent(new ActionExecutingEvent("logout"), null, xcontext);
eventListenerCaptor.getValue().onEvent(new UserUnauthenticatedEvent(userReference), null, xcontext);

verify(query).setParameter("userName", "XWiki.LoggerOutter");
verify(query).setParameter("userName", username);
verify(query).executeUpdate();
verify(this.hibernateStore).beginTransaction();
verify(this.hibernateStore).endTransaction(true);
Expand Down
Loading

0 comments on commit 5994612

Please sign in to comment.