Bugfixes and nick filter.

Add a configurable nickname ban filter, similar to the chat filter except this is defined in the properties file since it doesn't need unicode. Any user that attempts to use a nick that contains anything on this list will be denied.
Fix a bug with /kick and /ban that caused it to only work against users with entirely lower-case nicknames.
Add a check that the user's IP address hasn't changed. This probably isn't as needed now that /kick actually works against everybody.
This commit is contained in:
Andy Janata 2018-06-13 11:36:28 -07:00
parent 67c0e0e954
commit d8740258d7
7 changed files with 60 additions and 21 deletions

View File

@ -11,6 +11,8 @@ pyx.id_code_salt=
# comma-separated listed of IP addresses (v4 or v6) from which users are considered admins.
# IPv6 addresses must be fully spelt out without omitting groups of 0s with ::
pyx.admin_addrs=127.0.0.1,0:0:0:0:0:0:0:1
# comma-separated list of strings banned from appearing in nicks.
pyx.banned_nicks=xyzzy
# The name of a class that implements net.socialgamer.cah.util.ChatFilter.ShadowBannedStringProvider
# which will then be called to get the shadowbanned strings.

View File

@ -21,6 +21,7 @@ pyx.chat.global.repeated_words_unique_ratio=${pyx.global.repeated_words_unique_r
pyx.chat.game.flood_count=${pyx.game.flood_count}
pyx.chat.game.flood_time=${pyx.game.flood_time}
pyx.build=${buildNumber}
pyx.banned_nicks=${pyx.banned_nicks}
pyx.metrics.game.enabled=${pyx.metrics.game.enabled}
pyx.metrics.game.url_format=${pyx.metrics.game.url_format}

View File

@ -289,6 +289,14 @@ public class CahModule extends AbstractModule {
}
}
@Provides
@BannedNicks
Set<String> provideBannedNicks() {
synchronized (properties) {
return ImmutableSet.copyOf(properties.getProperty("pyx.banned_nicks", "").split(","));
}
}
@BindingAnnotation
@Retention(RetentionPolicy.RUNTIME)
public @interface BanList {
@ -388,4 +396,9 @@ public class CahModule extends AbstractModule {
@Retention(RetentionPolicy.RUNTIME)
public @interface Admins {
}
@BindingAnnotation
@Retention(RetentionPolicy.RUNTIME)
public @interface BannedNicks {
}
}

View File

@ -167,7 +167,7 @@ public class ConnectedUsers {
*/
public void removeUser(final User user, final DisconnectReason reason) {
synchronized (users) {
if (users.containsKey(user.getNickname())) {
if (users.containsKey(user.getNickname().toLowerCase())) {
logger.info(String.format("Removing user %s because %s", user.toString(), reason));
user.noLongerValid();
users.remove(user.getNickname().toLowerCase());

View File

@ -30,6 +30,8 @@ import java.util.concurrent.PriorityBlockingQueue;
import javax.annotation.Nullable;
import org.apache.log4j.Logger;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
@ -46,6 +48,8 @@ import net.socialgamer.cah.Constants.Sigil;
*/
public class User {
private static final Logger LOG = Logger.getLogger(User.class);
private final String nickname;
private final String idCode;
@ -289,6 +293,15 @@ public class User {
return valid;
}
public boolean isValidFromHost(final String currentHostname) {
final boolean addrValid = hostname.equals(currentHostname);
if (!addrValid) {
LOG.warn(String.format("User %s used to be from %s but is now from %s", nickname, hostname,
currentHostname));
}
return isValid() && addrValid;
}
/**
* Mark this user as no longer valid, probably because they pinged out.
*/

View File

@ -24,6 +24,7 @@
package net.socialgamer.cah.handlers;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.regex.Pattern;
@ -39,6 +40,7 @@ import com.google.inject.Provider;
import net.socialgamer.cah.CahModule.Admins;
import net.socialgamer.cah.CahModule.BanList;
import net.socialgamer.cah.CahModule.BannedNicks;
import net.socialgamer.cah.CahModule.SessionPermalinkUrlFormat;
import net.socialgamer.cah.CahModule.ShowSessionPermalink;
import net.socialgamer.cah.CahModule.ShowUserPermalink;
@ -73,6 +75,7 @@ public class RegisterHandler extends Handler {
private final ConnectedUsers users;
private final Set<String> adminList;
private final Set<String> banList;
private final Set<String> bannedNickList;
private final User.Factory userFactory;
private final Provider<String> persistentIdProvider;
private final IdCodeMangler idCodeMangler;
@ -89,7 +92,8 @@ public class RegisterHandler extends Handler {
@ShowSessionPermalink final boolean showSessionPermalink,
@SessionPermalinkUrlFormat final String sessionPermalinkFormatString,
@ShowUserPermalink final boolean showUserPermalink,
@UserPermalinkUrlFormat final String userPermalinkFormatString) {
@UserPermalinkUrlFormat final String userPermalinkFormatString,
@BannedNicks final Set<String> bannedNickList) {
this.users = users;
this.banList = banList;
this.userFactory = userFactory;
@ -100,6 +104,7 @@ public class RegisterHandler extends Handler {
this.sessionPermalinkFormatString = sessionPermalinkFormatString;
this.showUserPermalink = showUserPermalink;
this.userPermalinkFormatString = userPermalinkFormatString;
this.bannedNickList = bannedNickList;
}
@Override
@ -121,10 +126,14 @@ public class RegisterHandler extends Handler {
return error(ErrorCode.INVALID_ID_CODE);
} else {
final String nick = request.getParameter(AjaxRequest.NICKNAME).trim();
final String nickLower = nick.toLowerCase(Locale.ENGLISH);
for (final String banned : bannedNickList) {
if (nickLower.contains(banned)) {
return error(ErrorCode.RESERVED_NICK);
}
}
if (!VALID_NAME.matcher(nick).matches()) {
return error(ErrorCode.INVALID_NICK);
} else if ("xyzzy".equalsIgnoreCase(nick)) {
return error(ErrorCode.RESERVED_NICK);
} else {
String persistentId = request.getParameter(AjaxRequest.PERSISTENT_ID);
if (StringUtils.isBlank(persistentId)) {

View File

@ -1,16 +1,16 @@
/**
* Copyright (c) 2012-2018, Andy Janata
* All rights reserved.
*
*
* Redistribution and use in source and binary forms, with or without modification, are permitted
* provided that the following conditions are met:
*
*
* * Redistributions of source code must retain the above copyright notice, this list of conditions
* and the following disclaimer.
* * Redistributions in binary form must reproduce the above copyright notice, this list of
* conditions and the following disclaimer in the documentation and/or other materials provided
* with the distribution.
*
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR
* IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
* FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
@ -37,6 +37,11 @@ import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;
import org.json.simple.JSONObject;
import org.json.simple.JSONValue;
import com.google.inject.Injector;
import net.socialgamer.cah.Constants.AjaxOperation;
import net.socialgamer.cah.Constants.AjaxRequest;
import net.socialgamer.cah.Constants.AjaxResponse;
@ -46,17 +51,12 @@ import net.socialgamer.cah.Constants.SessionAttribute;
import net.socialgamer.cah.StartupUtils;
import net.socialgamer.cah.data.User;
import org.json.simple.JSONObject;
import org.json.simple.JSONValue;
import com.google.inject.Injector;
/**
* Servlet implementation class CahServlet.
*
*
* Superclass for all CAH servlets. Provides utility methods to return errors and data, and to log.
*
*
* @author Andy Janata (ajanata@socialgamer.net)
*/
public abstract class CahServlet extends HttpServlet {
@ -107,8 +107,9 @@ public abstract class CahServlet extends HttpServlet {
|| op.equals(AjaxOperation.FIRST_LOAD.toString()));
if (!skipSessionUserCheck && hSession.getAttribute(SessionAttribute.USER) == null) {
returnError(user, response.getWriter(), ErrorCode.NOT_REGISTERED, serial);
} else if (user != null && !user.isValid()) {
} else if (user != null && !user.isValidFromHost(request.getRemoteAddr())) {
// user probably pinged out, or possibly kicked by admin
// or their IP address magically changed (working around a ban?)
hSession.invalidate();
returnError(user, response.getWriter(), ErrorCode.SESSION_EXPIRED, serial);
} else {
@ -137,7 +138,7 @@ public abstract class CahServlet extends HttpServlet {
/**
* Handles a request from a CAH client. A session is guaranteed to exist at this point.
*
*
* @param request
* The request data.
* @param response
@ -153,7 +154,7 @@ public abstract class CahServlet extends HttpServlet {
/**
* Return an error to the client.
*
*
* @param user
* User that caused the error.
* @param writer
@ -175,7 +176,7 @@ public abstract class CahServlet extends HttpServlet {
/**
* Return response data to the client.
*
*
* @param user
* User this response is for.
* @param writer
@ -190,7 +191,7 @@ public abstract class CahServlet extends HttpServlet {
/**
* Return multiple response data to the client.
*
*
* @param user
* User this response is for.
* @param writer
@ -205,7 +206,7 @@ public abstract class CahServlet extends HttpServlet {
/**
* Return any response data to the client.
*
*
* @param user
* User this response is for.
* @param writer
@ -231,7 +232,7 @@ public abstract class CahServlet extends HttpServlet {
/**
* Log a message, with the user's name if {@code user} is not null.
*
*
* @param user
* The user this log message is about, or {@code null} if unknown.
* @param message