diff --git a/src/main/java/fr/xephi/authme/listener/OnJoinVerifier.java b/src/main/java/fr/xephi/authme/listener/OnJoinVerifier.java index 50ee67b2f..745a1908c 100644 --- a/src/main/java/fr/xephi/authme/listener/OnJoinVerifier.java +++ b/src/main/java/fr/xephi/authme/listener/OnJoinVerifier.java @@ -157,15 +157,15 @@ class OnJoinVerifier implements Reloadable { } /** - * Checks that the player's country is admitted if he is not registered. + * Checks that the player's country is admitted. * * @param isAuthAvailable whether or not the user is registered - * @param event the login event of the player + * @param playerIp the ip address of the player */ public void checkPlayerCountry(boolean isAuthAvailable, - PlayerLoginEvent event) throws FailedVerificationException { - if ((!isAuthAvailable || settings.getProperty(ProtectionSettings.ENABLE_PROTECTION_REGISTERED) ) && settings.getProperty(ProtectionSettings.ENABLE_PROTECTION)) { - String playerIp = event.getAddress().getHostAddress(); + String playerIp) throws FailedVerificationException { + if ((!isAuthAvailable || settings.getProperty(ProtectionSettings.ENABLE_PROTECTION_REGISTERED)) + && settings.getProperty(ProtectionSettings.ENABLE_PROTECTION)) { if (!validationService.isCountryAdmitted(playerIp)) { throw new FailedVerificationException(MessageKey.COUNTRY_BANNED_ERROR); } diff --git a/src/main/java/fr/xephi/authme/listener/PlayerListener.java b/src/main/java/fr/xephi/authme/listener/PlayerListener.java index 720a4f71c..148635675 100644 --- a/src/main/java/fr/xephi/authme/listener/PlayerListener.java +++ b/src/main/java/fr/xephi/authme/listener/PlayerListener.java @@ -231,7 +231,7 @@ public class PlayerListener implements Listener { onJoinVerifier.checkAntibot(lowerName, isAuthAvailable); onJoinVerifier.checkKickNonRegistered(isAuthAvailable); onJoinVerifier.checkNameCasing(player, auth); - onJoinVerifier.checkPlayerCountry(isAuthAvailable, event); + onJoinVerifier.checkPlayerCountry(isAuthAvailable, event.getAddress().getHostAddress()); } catch (FailedVerificationException e) { event.setKickMessage(m.retrieveSingle(e.getReason(), e.getArgs())); event.setResult(PlayerLoginEvent.Result.KICK_OTHER); diff --git a/src/test/java/fr/xephi/authme/listener/OnJoinVerifierTest.java b/src/test/java/fr/xephi/authme/listener/OnJoinVerifierTest.java index f2e9ee9eb..19685ff8c 100644 --- a/src/test/java/fr/xephi/authme/listener/OnJoinVerifierTest.java +++ b/src/test/java/fr/xephi/authme/listener/OnJoinVerifierTest.java @@ -9,6 +9,7 @@ import fr.xephi.authme.output.Messages; import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.permission.PlayerStatePermission; import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.settings.properties.ProtectionSettings; import fr.xephi.authme.settings.properties.RegistrationSettings; import fr.xephi.authme.settings.properties.RestrictionSettings; import fr.xephi.authme.util.BukkitService; @@ -90,9 +91,7 @@ public class OnJoinVerifierTest { assertThat(result, equalTo(false)); verify(event).getResult(); verifyNoMoreInteractions(event); - verifyZeroInteractions(bukkitService); - verifyZeroInteractions(dataSource); - verifyZeroInteractions(permissionsManager); + verifyZeroInteractions(bukkitService, dataSource, permissionsManager); } @Test @@ -112,8 +111,7 @@ public class OnJoinVerifierTest { assertThat(result, equalTo(true)); assertThat(event.getResult(), equalTo(PlayerLoginEvent.Result.KICK_FULL)); assertThat(event.getKickMessage(), equalTo(serverFullMessage)); - verifyZeroInteractions(bukkitService); - verifyZeroInteractions(dataSource); + verifyZeroInteractions(bukkitService, dataSource); } @Test @@ -423,6 +421,68 @@ public class OnJoinVerifierTest { } } + /** + * Tests various scenarios in which the country check should not take place. + */ + @Test + public void shouldNotCheckCountry() throws FailedVerificationException { + // protection setting disabled + given(settings.getProperty(ProtectionSettings.ENABLE_PROTECTION)).willReturn(false); + given(settings.getProperty(ProtectionSettings.ENABLE_PROTECTION_REGISTERED)).willReturn(true); + onJoinVerifier.checkPlayerCountry(false, "127.0.0.1"); + verifyZeroInteractions(validationService); + + // protection for registered players disabled + given(settings.getProperty(ProtectionSettings.ENABLE_PROTECTION_REGISTERED)).willReturn(false); + onJoinVerifier.checkPlayerCountry(true, "127.0.0.1"); + verifyZeroInteractions(validationService); + } + + @Test + public void shouldCheckAndAcceptUnregisteredPlayerCountry() throws FailedVerificationException { + // given + String ip = "192.168.0.1"; + given(settings.getProperty(ProtectionSettings.ENABLE_PROTECTION)).willReturn(true); + given(settings.getProperty(ProtectionSettings.ENABLE_PROTECTION_REGISTERED)).willReturn(false); + given(validationService.isCountryAdmitted(ip)).willReturn(true); + + // when + onJoinVerifier.checkPlayerCountry(false, ip); + + // then + verify(validationService).isCountryAdmitted(ip); + } + + @Test + public void shouldCheckAndAcceptRegisteredPlayerCountry() throws FailedVerificationException { + // given + String ip = "192.168.10.24"; + given(settings.getProperty(ProtectionSettings.ENABLE_PROTECTION)).willReturn(true); + given(settings.getProperty(ProtectionSettings.ENABLE_PROTECTION_REGISTERED)).willReturn(true); + given(validationService.isCountryAdmitted(ip)).willReturn(true); + + // when + onJoinVerifier.checkPlayerCountry(true, ip); + + // then + verify(validationService).isCountryAdmitted(ip); + } + + @Test + public void shouldThrowForBannedCountry() throws FailedVerificationException { + // given + String ip = "192.168.40.0"; + given(settings.getProperty(ProtectionSettings.ENABLE_PROTECTION)).willReturn(true); + given(settings.getProperty(ProtectionSettings.ENABLE_PROTECTION_REGISTERED)).willReturn(true); + given(validationService.isCountryAdmitted(ip)).willReturn(false); + + // expect + expectValidationExceptionWith(MessageKey.COUNTRY_BANNED_ERROR); + + // when + onJoinVerifier.checkPlayerCountry(false, ip); + } + private static Player newPlayerWithName(String name) { Player player = mock(Player.class); given(player.getName()).willReturn(name); diff --git a/src/test/java/fr/xephi/authme/util/ValidationServiceTest.java b/src/test/java/fr/xephi/authme/util/ValidationServiceTest.java index d90275212..b61f833db 100644 --- a/src/test/java/fr/xephi/authme/util/ValidationServiceTest.java +++ b/src/test/java/fr/xephi/authme/util/ValidationServiceTest.java @@ -10,6 +10,7 @@ import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.permission.PlayerStatePermission; import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.properties.EmailSettings; +import fr.xephi.authme.settings.properties.ProtectionSettings; import fr.xephi.authme.settings.properties.RestrictionSettings; import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.util.ValidationService.ValidationResult; @@ -18,13 +19,15 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; -import java.util.Arrays; import java.util.Collections; +import static java.util.Arrays.asList; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertThat; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; /** * Test for {@link ValidationService}. @@ -49,9 +52,9 @@ public class ValidationServiceTest { given(settings.getProperty(SecuritySettings.MIN_PASSWORD_LENGTH)).willReturn(3); given(settings.getProperty(SecuritySettings.MAX_PASSWORD_LENGTH)).willReturn(20); given(settings.getProperty(SecuritySettings.UNSAFE_PASSWORDS)) - .willReturn(Arrays.asList("unsafe", "other-unsafe")); + .willReturn(asList("unsafe", "other-unsafe")); given(settings.getProperty(EmailSettings.MAX_REG_PER_EMAIL)).willReturn(3); - given(settings.getProperty(RestrictionSettings.UNRESTRICTED_NAMES)).willReturn(Arrays.asList("name01", "npc")); + given(settings.getProperty(RestrictionSettings.UNRESTRICTED_NAMES)).willReturn(asList("name01", "npc")); } @Test @@ -126,7 +129,7 @@ public class ValidationServiceTest { public void shouldAcceptEmailWithWhitelist() { // given given(settings.getProperty(EmailSettings.DOMAIN_WHITELIST)) - .willReturn(Arrays.asList("domain.tld", "example.com")); + .willReturn(asList("domain.tld", "example.com")); given(settings.getProperty(EmailSettings.DOMAIN_BLACKLIST)).willReturn(Collections.emptyList()); // when @@ -140,7 +143,7 @@ public class ValidationServiceTest { public void shouldRejectEmailNotInWhitelist() { // given given(settings.getProperty(EmailSettings.DOMAIN_WHITELIST)) - .willReturn(Arrays.asList("domain.tld", "example.com")); + .willReturn(asList("domain.tld", "example.com")); given(settings.getProperty(EmailSettings.DOMAIN_BLACKLIST)).willReturn(Collections.emptyList()); // when @@ -155,7 +158,7 @@ public class ValidationServiceTest { // given given(settings.getProperty(EmailSettings.DOMAIN_WHITELIST)).willReturn(Collections.emptyList()); given(settings.getProperty(EmailSettings.DOMAIN_BLACKLIST)) - .willReturn(Arrays.asList("Example.org", "a-test-name.tld")); + .willReturn(asList("Example.org", "a-test-name.tld")); // when boolean result = validationService.validateEmail("sample@valid-name.tld"); @@ -169,7 +172,7 @@ public class ValidationServiceTest { // given given(settings.getProperty(EmailSettings.DOMAIN_WHITELIST)).willReturn(Collections.emptyList()); given(settings.getProperty(EmailSettings.DOMAIN_BLACKLIST)) - .willReturn(Arrays.asList("Example.org", "a-test-name.tld")); + .willReturn(asList("Example.org", "a-test-name.tld")); // when boolean result = validationService.validateEmail("sample@a-Test-name.tld"); @@ -245,12 +248,90 @@ public class ValidationServiceTest { assertThat(validationService.isUnrestricted("NAME01"), equalTo(true)); // Check reloading - given(settings.getProperty(RestrictionSettings.UNRESTRICTED_NAMES)).willReturn(Arrays.asList("new", "names")); + given(settings.getProperty(RestrictionSettings.UNRESTRICTED_NAMES)).willReturn(asList("new", "names")); validationService.reload(); assertThat(validationService.isUnrestricted("npc"), equalTo(false)); assertThat(validationService.isUnrestricted("New"), equalTo(true)); } + @Test + public void shouldNotInvokeGeoLiteApiIfCountryListsAreEmpty() { + // given + given(settings.getProperty(ProtectionSettings.COUNTRIES_WHITELIST)).willReturn(Collections.emptyList()); + given(settings.getProperty(ProtectionSettings.COUNTRIES_BLACKLIST)).willReturn(Collections.emptyList()); + + // when + boolean result = validationService.isCountryAdmitted("addr"); + + // then + assertThat(result, equalTo(true)); + verifyZeroInteractions(geoLiteApi); + } + + @Test + public void shouldAcceptCountryInWhitelist() { + // given + given(settings.getProperty(ProtectionSettings.COUNTRIES_WHITELIST)).willReturn(asList("ch", "it")); + given(settings.getProperty(ProtectionSettings.COUNTRIES_BLACKLIST)).willReturn(Collections.emptyList()); + String ip = "127.0.0.1"; + given(geoLiteApi.getCountryCode(ip)).willReturn("CH"); + + // when + boolean result = validationService.isCountryAdmitted(ip); + + // then + assertThat(result, equalTo(true)); + verify(geoLiteApi).getCountryCode(ip); + } + + @Test + public void shouldRejectCountryMissingFromWhitelist() { + // given + given(settings.getProperty(ProtectionSettings.COUNTRIES_WHITELIST)).willReturn(asList("ch", "it")); + given(settings.getProperty(ProtectionSettings.COUNTRIES_BLACKLIST)).willReturn(Collections.emptyList()); + String ip = "123.45.67.89"; + given(geoLiteApi.getCountryCode(ip)).willReturn("BR"); + + // when + boolean result = validationService.isCountryAdmitted(ip); + + // then + assertThat(result, equalTo(false)); + verify(geoLiteApi).getCountryCode(ip); + } + + @Test + public void shouldAcceptCountryAbsentFromBlacklist() { + // given + given(settings.getProperty(ProtectionSettings.COUNTRIES_WHITELIST)).willReturn(Collections.emptyList()); + given(settings.getProperty(ProtectionSettings.COUNTRIES_BLACKLIST)).willReturn(asList("ch", "it")); + String ip = "127.0.0.1"; + given(geoLiteApi.getCountryCode(ip)).willReturn("BR"); + + // when + boolean result = validationService.isCountryAdmitted(ip); + + // then + assertThat(result, equalTo(true)); + verify(geoLiteApi).getCountryCode(ip); + } + + @Test + public void shouldRejectCountryInBlacklist() { + // given + given(settings.getProperty(ProtectionSettings.COUNTRIES_WHITELIST)).willReturn(Collections.emptyList()); + given(settings.getProperty(ProtectionSettings.COUNTRIES_BLACKLIST)).willReturn(asList("ch", "it")); + String ip = "123.45.67.89"; + given(geoLiteApi.getCountryCode(ip)).willReturn("IT"); + + // when + boolean result = validationService.isCountryAdmitted(ip); + + // then + assertThat(result, equalTo(false)); + verify(geoLiteApi).getCountryCode(ip); + } + private static void assertErrorEquals(ValidationResult validationResult, MessageKey messageKey, String... args) { assertThat(validationResult.hasError(), equalTo(true)); assertThat(validationResult.getMessageKey(), equalTo(messageKey));