Requested changes

This commit is contained in:
Gabriele C 2018-12-09 19:03:51 +01:00
parent b919c33819
commit fac3a70634
7 changed files with 18 additions and 106 deletions

View File

@ -302,13 +302,12 @@ public class CommandDescription {
} }
/** /**
* Defines if the command contains sensitive data * Defines that the command contains sensitive data
* *
* @param sensitive The sensitive data flag
* @return The builder * @return The builder
*/ */
public CommandBuilder sensitive(boolean sensitive) { public CommandBuilder sensitive() {
this.sensitive = sensitive; this.sensitive = true;
return this; return this;
} }
} }

View File

@ -94,7 +94,7 @@ public class CommandInitializer {
.detailedDescription("Command to log in using AuthMeReloaded.") .detailedDescription("Command to log in using AuthMeReloaded.")
.withArgument("password", "Login password", MANDATORY) .withArgument("password", "Login password", MANDATORY)
.permission(PlayerPermission.LOGIN) .permission(PlayerPermission.LOGIN)
.sensitive(true) .sensitive()
.executableCommand(LoginCommand.class) .executableCommand(LoginCommand.class)
.register(); .register();
@ -117,7 +117,7 @@ public class CommandInitializer {
.withArgument("password", "Password", OPTIONAL) .withArgument("password", "Password", OPTIONAL)
.withArgument("verifyPassword", "Verify password", OPTIONAL) .withArgument("verifyPassword", "Verify password", OPTIONAL)
.permission(PlayerPermission.REGISTER) .permission(PlayerPermission.REGISTER)
.sensitive(true) .sensitive()
.executableCommand(RegisterCommand.class) .executableCommand(RegisterCommand.class)
.register(); .register();
@ -141,7 +141,7 @@ public class CommandInitializer {
.withArgument("oldPassword", "Old password", MANDATORY) .withArgument("oldPassword", "Old password", MANDATORY)
.withArgument("newPassword", "New password", MANDATORY) .withArgument("newPassword", "New password", MANDATORY)
.permission(PlayerPermission.CHANGE_PASSWORD) .permission(PlayerPermission.CHANGE_PASSWORD)
.sensitive(true) .sensitive()
.executableCommand(ChangePasswordCommand.class) .executableCommand(ChangePasswordCommand.class)
.register(); .register();
@ -200,7 +200,7 @@ public class CommandInitializer {
.withArgument("player", "Player name", MANDATORY) .withArgument("player", "Player name", MANDATORY)
.withArgument("password", "Password", MANDATORY) .withArgument("password", "Password", MANDATORY)
.permission(AdminPermission.REGISTER) .permission(AdminPermission.REGISTER)
.sensitive(true) .sensitive()
.executableCommand(RegisterAdminCommand.class) .executableCommand(RegisterAdminCommand.class)
.register(); .register();
@ -235,7 +235,7 @@ public class CommandInitializer {
.withArgument("player", "Player name", MANDATORY) .withArgument("player", "Player name", MANDATORY)
.withArgument("pwd", "New password", MANDATORY) .withArgument("pwd", "New password", MANDATORY)
.permission(AdminPermission.CHANGE_PASSWORD) .permission(AdminPermission.CHANGE_PASSWORD)
.sensitive(true) .sensitive()
.executableCommand(ChangePasswordAdminCommand.class) .executableCommand(ChangePasswordAdminCommand.class)
.register(); .register();
@ -545,7 +545,7 @@ public class CommandInitializer {
.detailedDescription("Set a new password after successfully recovering your account.") .detailedDescription("Set a new password after successfully recovering your account.")
.withArgument("password", "New password", MANDATORY) .withArgument("password", "New password", MANDATORY)
.permission(PlayerPermission.RECOVER_EMAIL) .permission(PlayerPermission.RECOVER_EMAIL)
.sensitive(true) .sensitive()
.executableCommand(EmailSetPasswordCommand.class) .executableCommand(EmailSetPasswordCommand.class)
.register(); .register();
@ -574,7 +574,7 @@ public class CommandInitializer {
.description("Command for logging in") .description("Command for logging in")
.detailedDescription("Processes the two-factor authentication code during login.") .detailedDescription("Processes the two-factor authentication code during login.")
.withArgument("code", "The TOTP code to use to log in", MANDATORY) .withArgument("code", "The TOTP code to use to log in", MANDATORY)
.sensitive(true) .sensitive()
.executableCommand(TotpCodeCommand.class) .executableCommand(TotpCodeCommand.class)
.register(); .register();
@ -596,7 +596,7 @@ public class CommandInitializer {
.detailedDescription("Saves the generated TOTP secret after confirmation.") .detailedDescription("Saves the generated TOTP secret after confirmation.")
.withArgument("code", "Code from the given secret from /totp add", MANDATORY) .withArgument("code", "Code from the given secret from /totp add", MANDATORY)
.permission(PlayerPermission.ENABLE_TWO_FACTOR_AUTH) .permission(PlayerPermission.ENABLE_TWO_FACTOR_AUTH)
.sensitive(true) .sensitive()
.executableCommand(ConfirmTotpCommand.class) .executableCommand(ConfirmTotpCommand.class)
.register(); .register();
@ -608,7 +608,7 @@ public class CommandInitializer {
.detailedDescription("Disables two-factor authentication for your account.") .detailedDescription("Disables two-factor authentication for your account.")
.withArgument("code", "Current 2FA code", MANDATORY) .withArgument("code", "Current 2FA code", MANDATORY)
.permission(PlayerPermission.DISABLE_TWO_FACTOR_AUTH) .permission(PlayerPermission.DISABLE_TWO_FACTOR_AUTH)
.sensitive(true) .sensitive()
.executableCommand(RemoveTotpCommand.class) .executableCommand(RemoveTotpCommand.class)
.register(); .register();

View File

@ -7,7 +7,7 @@ import fr.xephi.authme.message.MessageKey;
import fr.xephi.authme.message.Messages; import fr.xephi.authme.message.Messages;
import fr.xephi.authme.output.ConsoleFilter; import fr.xephi.authme.output.ConsoleFilter;
import fr.xephi.authme.output.Log4JFilter; import fr.xephi.authme.output.Log4JFilter;
import fr.xephi.authme.output.LogFilterService; import fr.xephi.authme.service.LogFilterService;
import fr.xephi.authme.service.BukkitService; import fr.xephi.authme.service.BukkitService;
import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.Settings;
import fr.xephi.authme.settings.properties.DatabaseSettings; import fr.xephi.authme.settings.properties.DatabaseSettings;

View File

@ -1,5 +1,7 @@
package fr.xephi.authme.output; package fr.xephi.authme.output;
import fr.xephi.authme.service.LogFilterService;
import java.util.logging.Filter; import java.util.logging.Filter;
import java.util.logging.LogRecord; import java.util.logging.LogRecord;

View File

@ -1,5 +1,6 @@
package fr.xephi.authme.output; package fr.xephi.authme.output;
import fr.xephi.authme.service.LogFilterService;
import org.apache.logging.log4j.Level; import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.Marker; import org.apache.logging.log4j.Marker;
import org.apache.logging.log4j.core.LogEvent; import org.apache.logging.log4j.core.LogEvent;

View File

@ -1,4 +1,4 @@
package fr.xephi.authme.output; package fr.xephi.authme.service;
import fr.xephi.authme.command.CommandMapper; import fr.xephi.authme.command.CommandMapper;
import fr.xephi.authme.command.FoundCommandResult; import fr.xephi.authme.command.FoundCommandResult;
@ -36,8 +36,7 @@ public class LogFilterService {
case MISSING_BASE_COMMAND: case MISSING_BASE_COMMAND:
return false; return false;
default: default:
break;
}
return command.getCommandDescription().isSensitive(); return command.getCommandDescription().isSensitive();
} }
}
} }

View File

@ -1,89 +0,0 @@
package fr.xephi.authme.output;
import com.google.common.collect.Lists;
import fr.xephi.authme.command.CommandDescription;
import fr.xephi.authme.command.CommandInitializer;
import org.junit.Test;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.junit.Assert.assertThat;
/**
* Test for {@link LogFilterService}.
*/
public class LogFilterHelperTest {
private static final List<CommandDescription> ALL_COMMANDS = new CommandInitializer().getCommands();
/**
* Checks that {@link LogFilterService#COMMANDS_TO_SKIP} contains the entries we expect
* (commands with password argument).
*/
@Test
public void shouldBlacklistAllSensitiveCommands() {
// given
List<CommandDescription> sensitiveCommands = Arrays.asList(
getCommand("register"), getCommand("login"), getCommand("changepassword"), getCommand("unregister"),
getCommand("authme", "register"), getCommand("authme", "changepassword"),
getCommand("email", "setpassword")
);
// Build array with entries like "/register ", "/authme cp ", "/authme changepass "
String[] expectedEntries = sensitiveCommands.stream()
.map(cmd -> buildCommandSyntaxes(cmd))
.flatMap(List::stream)
.map(syntax -> syntax + " ")
.toArray(String[]::new);
// when / then
assertThat(LogFilterService.COMMANDS_TO_SKIP, containsInAnyOrder(expectedEntries));
}
private static CommandDescription getCommand(String label) {
return findCommandWithLabel(label, ALL_COMMANDS);
}
private static CommandDescription getCommand(String parentLabel, String childLabel) {
CommandDescription parent = getCommand(parentLabel);
return findCommandWithLabel(childLabel, parent.getChildren());
}
private static CommandDescription findCommandWithLabel(String label, List<CommandDescription> commands) {
return commands.stream()
.filter(cmd -> cmd.getLabels().contains(label))
.findFirst().orElseThrow(() -> new IllegalArgumentException(label));
}
/**
* Returns all "command syntaxes" from which the given command can be reached.
* For example, the result might be a List containing "/authme changepassword", "/authme changepass",
* "/authme cp", "/authme:authme changepassword" etc.
*
* @param command the command to build syntaxes for
* @return command syntaxes
*/
private static List<String> buildCommandSyntaxes(CommandDescription command) {
List<String> prefixes = getCommandPrefixes(command);
return command.getLabels()
.stream()
.map(label -> Lists.transform(prefixes, p -> p + label))
.flatMap(List::stream)
.collect(Collectors.toList());
}
private static List<String> getCommandPrefixes(CommandDescription command) {
if (command.getParent() == null) {
return Arrays.asList("/", "/authme:");
}
return command.getParent().getLabels()
.stream()
.map(label -> new String[]{"/" + label + " ", "/authme:" + label + " "})
.flatMap(Arrays::stream)
.collect(Collectors.toList());
}
}