Skip to content
Permalink
Browse files Browse the repository at this point in the history
XWIKI-19559: Refactor handling of users in oldcore
  * Add property for inactive users in XWikiContext
  * Ensure that all calls to checkAuth in XS goes through
    XWiki#checkAuth so that inactive users are handled
  * Improve code of XWiki#prepareDocuments to simplify it
  * Ensure that the login page is not displayed if a inactive user
    logged in, in XWikiCachingRightService
  * Improve a bit the code of the drawer to display the logout link in
    case of logged in inactive user
  • Loading branch information
surli committed Mar 31, 2022
1 parent 70c64c2 commit e074d22
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 59 deletions.
Expand Up @@ -31,9 +31,15 @@
#largeUserAvatar($xcontext.user)
</a>
<div class="brand-links">
#define ($logoutLink)
<a href="$xwiki.getURL('XWiki.XWikiLogout', 'logout', "xredirect=$escapetool.url($xwiki.relativeRequestURL)")" id="tmLogout" rel="nofollow">$services.icon.renderHTML('log-out') $escapetool.xml($services.localization.render('logout'))</a>
#end
#if ($xcontext.user != 'XWiki.XWikiGuest')
<a href="$xwiki.getURL($xcontext.user, 'view')" class="brand-user" id="tmUser">$!xwiki.getUserName($xcontext.user, false)</a>
<a href="$xwiki.getURL('XWiki.XWikiLogout', 'logout', "xredirect=$escapetool.url($xwiki.relativeRequestURL)")" id="tmLogout" rel="nofollow">$services.icon.renderHTML('log-out') $escapetool.xml($services.localization.render('logout'))</a>
$logoutLink
#elseif($xcontext.user == 'XWiki.XWikiGuest' && $xcontext.inactiveUserReference)
<a href="$xwiki.getURL($xcontext.inactiveUserReference, 'view')" class="brand-user" id="tmUser">$!xwiki.getUserName($xcontext.inactiveUserReference, false)</a>
$logoutLink
#else
<a href="$xwiki.getURL('XWiki.XWikiLogin', 'login', "xredirect=$escapetool.url($xwiki.relativeRequestURL)&loginLink=1")" id="tmLogin" rel="nofollow">$services.icon.renderHTML('log-in') $escapetool.xml($services.localization.render('login'))</a>
#if ($xwiki.hasAccessLevel('register', 'XWiki.XWikiPreferences'))
Expand Down
Expand Up @@ -185,7 +185,7 @@ public Response getNotificationsCount(String useUserPreferences, String userId,
{
// Build the response
Response.ResponseBuilder response;
XWikiUser xWikiUser = getXWikiContext().getWiki().getAuthService().checkAuth(getXWikiContext());
XWikiUser xWikiUser = getXWikiContext().getWiki().checkAuth(getXWikiContext());
if (xWikiUser == null) {
response = Response.status(Status.UNAUTHORIZED);
} else {
Expand Down Expand Up @@ -215,7 +215,7 @@ public String getNotificationsRSS(String useUserPreferences, String userId, Stri
String tags, String currentWiki) throws Exception
{
// Build the response
XWikiUser xWikiUser = getXWikiContext().getWiki().getAuthService().checkAuth(getXWikiContext());
XWikiUser xWikiUser = getXWikiContext().getWiki().checkAuth(getXWikiContext());
DocumentReference userIdDoc = this.documentReferenceResolver.resolve(userId);
if (xWikiUser == null || !userIdDoc.equals(xWikiUser.getUserReference())) {
getXWikiContext().getResponse().sendError(HttpServletResponse.SC_UNAUTHORIZED);
Expand Down
Expand Up @@ -4322,9 +4322,23 @@ public void prepareResources(XWikiContext context)
}
}

/**
* Authenticate the user from the context and check if the user is disabled or not.
* If the user is disabled, the method returns {@code null} but set the reference of the authenticated user in the
* context with the {@link XWikiContext#INACTIVE_USER_REFERENCE} property.
*
* @param context the context used to authenticate the user.
* @return an {@link XWikiUser} if the user is authenticated and enabled, else {@code null}.
* @throws XWikiException in case of problem when dealing with the authentication.
*/
public XWikiUser checkAuth(XWikiContext context) throws XWikiException
{
return getAuthService().checkAuth(context);
XWikiUser user = getAuthService().checkAuth(context);
if (user != null && user.isDisabled(context)) {
context.put(XWikiContext.INACTIVE_USER_REFERENCE, user.getUserReference());
user = null;
}
return user;
}

public boolean checkAccess(String action, XWikiDocument doc, XWikiContext context) throws XWikiException
Expand Down Expand Up @@ -5864,70 +5878,26 @@ public boolean prepareDocuments(XWikiRequest request, XWikiContext context, Velo
boolean hasAccess = checkAccess(context.getAction(), doc, context);

XWikiUser user;
XWikiUser inactiveUser = null;
if (context.getUserReference() == null && context.get(XWikiContext.INACTIVE_USER_REFERENCE) != null) {
inactiveUser = new XWikiUser((DocumentReference) context.get(XWikiContext.INACTIVE_USER_REFERENCE));
}
if (context.getUserReference() != null) {
user = new XWikiUser(context.getUserReference());
} else {
user = new XWikiUser(context.getUser());
}

if (inactiveUser != null && context.getAction().equals("view")) {
this.handleInactiveUserViewAction(inactiveUser, context, reference, vcontext);
}
// We need to check rights before we look for translations
// Otherwise we don't have the user language
if (!hasAccess) {
else if (!hasAccess) {
Object[] args = { doc.getFullName(), user.getUser() };
setPhonyDocument(reference, context, vcontext);
throw new XWikiException(XWikiException.MODULE_XWIKI_ACCESS, XWikiException.ERROR_XWIKI_ACCESS_DENIED,
"Access to document {0} has been denied to user {1}", null, args);
// User is disabled: the mail address is marked as checked
} else if (user.isDisabled(context) && user.isEmailChecked(context)) {
String action = context.getAction();
/*
* Allow inactive users to see skins, ressources, SSX, JSX and downloads they could have seen as guest. The
* rational behind this behaviour is that inactive users should be able to access the same UI that guests
* are used to see, including custom icons, panels, and so on...
*/
if (!((action.equals("skin") && (doc.getSpace().equals("skins") || doc.getSpace().equals("resources")))
|| ((action.equals("skin") || action.equals("download") || action.equals("ssx") || action.equals("jsx"))
&& getRightService().hasAccessLevel("view", XWikiRightService.GUEST_USER_FULLNAME,
doc.getPrefixedFullName(), context))
|| ((action.equals("view") && doc.getFullName().equals("XWiki.AccountValidation"))))) {
Object[] args = { user.getUser() };
setPhonyDocument(reference, context, vcontext);
throw new XWikiException(XWikiException.MODULE_XWIKI_USER, XWikiException.ERROR_XWIKI_USER_DISABLED,
"User {0} account is disabled", null, args);
}
// User actually needs to activate his mail address.
} else if (user.isDisabled(context) && !user.isEmailChecked(context)) {
boolean allow = false;
String action = context.getAction();
/*
* Allow inactive users to see skins, ressources, SSX, JSX and downloads they could have seen as guest. The
* rational behind this behaviour is that inactive users should be able to access the same UI that guests
* are used to see, including custom icons, panels, and so on...
*/
if ((action.equals("skin") && (doc.getSpace().equals("skins") || doc.getSpace().equals("resources")))
|| ((action.equals("skin") || action.equals("download") || action.equals("ssx") || action.equals("jsx"))
&& getRightService().hasAccessLevel("view", XWikiRightService.GUEST_USER_FULLNAME,
doc.getPrefixedFullName(), context))
|| ((action.equals("view") && doc.getFullName().equals("XWiki.AccountValidation")))) {
allow = true;
} else {
String allowed = getConfiguration().getProperty("xwiki.inactiveuser.allowedpages", "");
if (context.getAction().equals("view") && !allowed.equals("")) {
String[] allowedList = StringUtils.split(allowed, " ,");
for (String element : allowedList) {
if (element.equals(doc.getFullName())) {
allow = true;
break;
}
}
}
}
if (!allow) {
Object[] args = { context.getUser() };
setPhonyDocument(reference, context, vcontext);
throw new XWikiException(XWikiException.MODULE_XWIKI_USER, XWikiException.ERROR_XWIKI_USER_INACTIVE,
"User {0} account is inactive", null, args);
}
}

if (!"skin".equals(context.getAction())
Expand Down Expand Up @@ -5969,6 +5939,37 @@ && getRightService().hasAccessLevel("view", XWikiRightService.GUEST_USER_FULLNAM
return true;
}

private void handleInactiveUserViewAction(XWikiUser inactiveUser, XWikiContext context, DocumentReference reference,
VelocityContext vcontext) throws XWikiException
{
if (inactiveUser.isEmailChecked(context)) {
Object[] args = { inactiveUser.getUser() };
setPhonyDocument(reference, context, vcontext);
throw new XWikiException(XWikiException.MODULE_XWIKI_USER, XWikiException.ERROR_XWIKI_USER_DISABLED,
"User {0} account is disabled", null, args);
} else if (!reference.getLocalDocumentReference().equals(XWikiUser.ACCOUNT_VALIDATION_DOCUMENT_REFERENCE)) {
String allowed = getConfiguration().getProperty("xwiki.inactiveuser.allowedpages", "");
boolean allow = false;
if (!StringUtils.isEmpty(allowed)) {
XWikiDocument doc = this.getDocument(reference, context);
String[] allowedList = StringUtils.split(allowed, " ,");
for (String element : allowedList) {
if (element.equals(doc.getFullName())) {
allow = true;
break;
}
}
}

if (!allow) {
Object[] args = { inactiveUser.getUser() };
setPhonyDocument(reference, context, vcontext);
throw new XWikiException(XWikiException.MODULE_XWIKI_USER, XWikiException.ERROR_XWIKI_USER_INACTIVE,
"User {0} account is inactive", null, args);
}
}
}

/**
* @since 8.3M1
*/
Expand Down
Expand Up @@ -101,6 +101,16 @@ public class XWikiContext extends Hashtable<Object, Object>
@Deprecated
public static final String KEY_LEGACY_VELOCITYCONTEXT = "vcontext";

/**
* The reference of a logged-in inactive user: in such case the context user reference is guest, so we store
* the actual logged-in user with that key.
*
* @since 14.3RC1
* @since 13.10.5
*/
@Unstable
public static final String INACTIVE_USER_REFERENCE = "inactiveUserReference";

/** Logging helper object. */
protected static final Logger LOGGER = LoggerFactory.getLogger(XWikiContext.class);

Expand Down
Expand Up @@ -2910,7 +2910,7 @@ public String getCounter(String name)
*/
public XWikiUser checkAuth() throws XWikiException
{
return this.context.getWiki().getAuthService().checkAuth(this.context);
return this.context.getWiki().checkAuth(this.context);
}

/**
Expand All @@ -2925,7 +2925,13 @@ public XWikiUser checkAuth() throws XWikiException
*/
public XWikiUser checkAuth(String username, String password, String rememberme) throws XWikiException
{
return this.context.getWiki().getAuthService().checkAuth(username, password, rememberme, this.context);
XWikiUser user =
this.context.getWiki().getAuthService().checkAuth(username, password, rememberme, this.context);
if (user.isDisabled(this.context)) {
this.context.put(XWikiContext.INACTIVE_USER_REFERENCE, user.getUserReference());
user = null;
}
return user;
}

/**
Expand Down
Expand Up @@ -29,6 +29,7 @@
import org.xwiki.model.reference.DocumentReference;
import org.xwiki.model.reference.DocumentReferenceResolver;
import org.xwiki.model.reference.EntityReferenceSerializer;
import org.xwiki.model.reference.LocalDocumentReference;
import org.xwiki.model.reference.WikiReference;

import com.xpn.xwiki.XWiki;
Expand Down Expand Up @@ -58,6 +59,9 @@
*/
public static final String EMAIL_CHECKED_PROPERTY = "email_checked";

public static final LocalDocumentReference ACCOUNT_VALIDATION_DOCUMENT_REFERENCE =
new LocalDocumentReference(XWiki.SYSTEM_SPACE, "AccountValidation");

/**
* @see com.xpn.xwiki.internal.model.reference.CurrentMixedStringDocumentReferenceResolver
*/
Expand Down
Expand Up @@ -280,7 +280,9 @@ public boolean checkAccess(String action, XWikiDocument doc, XWikiContext contex
// implementation, so code that simply want to verify if a user can delete (but is not actually deleting)
// has to call checkAccess. This happen really often, and this why we should not redirect to login on failed
// delete, since it would prevent most user to do anything.
if (context.getUserReference() == null && !DELETE_ACTION.equals(action) && !LOGIN_ACTION.equals(action)) {
// Also we don't show the login if an inactive user logged in.
if (context.getUserReference() == null && !DELETE_ACTION.equals(action) && !LOGIN_ACTION.equals(action)
&& context.get(XWikiContext.INACTIVE_USER_REFERENCE) == null) {
LOGGER.debug("Redirecting unauthenticated user to login, since it have been denied [{}] on [{}].",
right, entityReference);
showLogin(context);
Expand Down

0 comments on commit e074d22

Please sign in to comment.