Minor: Use CommonService for permission lookup

- Some changes found in a very old patch :) - drop injection of PermissionsManager in favor of CommonService
- Rename IsEqualByReflectionMatcher's method to something more specific to differentiate it better from Hamcrest's equalTo() matcher
This commit is contained in:
ljacqu 2018-02-23 23:37:24 +01:00
parent 329657bd5f
commit 8d5afa7fbc
9 changed files with 28 additions and 34 deletions

View File

@ -5,7 +5,6 @@ import fr.xephi.authme.data.auth.PlayerAuth;
import fr.xephi.authme.data.auth.PlayerCache; import fr.xephi.authme.data.auth.PlayerCache;
import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.datasource.DataSource;
import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.message.MessageKey;
import fr.xephi.authme.permission.PermissionsManager;
import fr.xephi.authme.process.AsynchronousProcess; import fr.xephi.authme.process.AsynchronousProcess;
import fr.xephi.authme.process.register.executors.RegistrationExecutor; import fr.xephi.authme.process.register.executors.RegistrationExecutor;
import fr.xephi.authme.process.register.executors.RegistrationMethod; import fr.xephi.authme.process.register.executors.RegistrationMethod;
@ -35,8 +34,6 @@ public class AsyncRegister implements AsynchronousProcess {
@Inject @Inject
private CommonService service; private CommonService service;
@Inject @Inject
private PermissionsManager permissionsManager;
@Inject
private SingletonStore<RegistrationExecutor> registrationExecutorFactory; private SingletonStore<RegistrationExecutor> registrationExecutorFactory;
@Inject @Inject
private BungeeSender bungeeSender; private BungeeSender bungeeSender;
@ -106,7 +103,7 @@ public class AsyncRegister implements AsynchronousProcess {
if (maxRegPerIp > 0 if (maxRegPerIp > 0
&& !"127.0.0.1".equalsIgnoreCase(ip) && !"127.0.0.1".equalsIgnoreCase(ip)
&& !"localhost".equalsIgnoreCase(ip) && !"localhost".equalsIgnoreCase(ip)
&& !permissionsManager.hasPermission(player, ALLOW_MULTIPLE_ACCOUNTS)) { && !service.hasPermission(player, ALLOW_MULTIPLE_ACCOUNTS)) {
List<String> otherAccounts = database.getAllAuthsByIp(ip); List<String> otherAccounts = database.getAllAuthsByIp(ip);
if (otherAccounts.size() >= maxRegPerIp) { if (otherAccounts.size() >= maxRegPerIp) {
service.send(player, MessageKey.MAX_REGISTER_EXCEEDED, Integer.toString(maxRegPerIp), service.send(player, MessageKey.MAX_REGISTER_EXCEEDED, Integer.toString(maxRegPerIp),

View File

@ -4,7 +4,6 @@ import fr.xephi.authme.data.auth.PlayerAuth;
import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.datasource.DataSource;
import fr.xephi.authme.mail.EmailService; import fr.xephi.authme.mail.EmailService;
import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.message.MessageKey;
import fr.xephi.authme.permission.PermissionsManager;
import fr.xephi.authme.process.SyncProcessManager; import fr.xephi.authme.process.SyncProcessManager;
import fr.xephi.authme.security.PasswordSecurity; import fr.xephi.authme.security.PasswordSecurity;
import fr.xephi.authme.security.crypts.HashedPassword; import fr.xephi.authme.security.crypts.HashedPassword;
@ -25,9 +24,6 @@ import static fr.xephi.authme.settings.properties.EmailSettings.RECOVERY_PASSWOR
*/ */
class EmailRegisterExecutor implements RegistrationExecutor<EmailRegisterParams> { class EmailRegisterExecutor implements RegistrationExecutor<EmailRegisterParams> {
@Inject
private PermissionsManager permissionsManager;
@Inject @Inject
private DataSource dataSource; private DataSource dataSource;
@ -46,7 +42,7 @@ class EmailRegisterExecutor implements RegistrationExecutor<EmailRegisterParams>
@Override @Override
public boolean isRegistrationAdmitted(EmailRegisterParams params) { public boolean isRegistrationAdmitted(EmailRegisterParams params) {
final int maxRegPerEmail = commonService.getProperty(EmailSettings.MAX_REG_PER_EMAIL); final int maxRegPerEmail = commonService.getProperty(EmailSettings.MAX_REG_PER_EMAIL);
if (maxRegPerEmail > 0 && !permissionsManager.hasPermission(params.getPlayer(), ALLOW_MULTIPLE_ACCOUNTS)) { if (maxRegPerEmail > 0 && !commonService.hasPermission(params.getPlayer(), ALLOW_MULTIPLE_ACCOUNTS)) {
int otherAccounts = dataSource.countAuthsByEmail(params.getEmail()); int otherAccounts = dataSource.countAuthsByEmail(params.getEmail());
if (otherAccounts >= maxRegPerEmail) { if (otherAccounts >= maxRegPerEmail) {
commonService.send(params.getPlayer(), MessageKey.MAX_REGISTER_EXCEEDED, commonService.send(params.getPlayer(), MessageKey.MAX_REGISTER_EXCEEDED,

View File

@ -31,7 +31,7 @@ public final class IsEqualByReflectionMatcher<T> extends TypeSafeMatcher<T> {
* @param <T> the object's type * @param <T> the object's type
* @return the matcher for the expected object * @return the matcher for the expected object
*/ */
public static <T> Matcher<T> isEqualTo(T expected) { public static <T> Matcher<T> hasEqualValuesOnAllFields(T expected) {
return new IsEqualByReflectionMatcher<>(expected); return new IsEqualByReflectionMatcher<>(expected);
} }
@ -70,7 +70,7 @@ public final class IsEqualByReflectionMatcher<T> extends TypeSafeMatcher<T> {
Class<?> currentClass = object.getClass(); Class<?> currentClass = object.getClass();
while (currentClass != null) { while (currentClass != null) {
for (Field f : currentClass.getDeclaredFields()) { for (Field f : currentClass.getDeclaredFields()) {
if (!Modifier.isStatic(f.getModifiers())) { if (!Modifier.isStatic(f.getModifiers()) && !f.isSynthetic()) {
fields.add(f); fields.add(f);
} }
} }

View File

@ -27,7 +27,7 @@ import java.util.Arrays;
import java.util.List; import java.util.List;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import static fr.xephi.authme.IsEqualByReflectionMatcher.isEqualTo; import static fr.xephi.authme.IsEqualByReflectionMatcher.hasEqualValuesOnAllFields;
import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.not;
@ -305,7 +305,7 @@ public class NewAPITest {
// then // then
verify(management).performRegister(eq(RegistrationMethod.API_REGISTRATION), verify(management).performRegister(eq(RegistrationMethod.API_REGISTRATION),
argThat(isEqualTo(ApiPasswordRegisterParams.of(player, pass, true)))); argThat(hasEqualValuesOnAllFields(ApiPasswordRegisterParams.of(player, pass, true))));
} }
@Test @Test
@ -319,7 +319,7 @@ public class NewAPITest {
// then // then
verify(management).performRegister(eq(RegistrationMethod.API_REGISTRATION), verify(management).performRegister(eq(RegistrationMethod.API_REGISTRATION),
argThat(isEqualTo(ApiPasswordRegisterParams.of(player, pass, false)))); argThat(hasEqualValuesOnAllFields(ApiPasswordRegisterParams.of(player, pass, false))));
} }
@Test @Test

View File

@ -30,7 +30,7 @@ import java.util.List;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import java.time.Instant; import java.time.Instant;
import static fr.xephi.authme.IsEqualByReflectionMatcher.isEqualTo; import static fr.xephi.authme.IsEqualByReflectionMatcher.hasEqualValuesOnAllFields;
import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.not;
@ -427,7 +427,7 @@ public class AuthMeApiTest {
// then // then
verify(management).performRegister(eq(RegistrationMethod.API_REGISTRATION), verify(management).performRegister(eq(RegistrationMethod.API_REGISTRATION),
argThat(isEqualTo(ApiPasswordRegisterParams.of(player, pass, true)))); argThat(hasEqualValuesOnAllFields(ApiPasswordRegisterParams.of(player, pass, true))));
} }
@Test @Test
@ -441,7 +441,7 @@ public class AuthMeApiTest {
// then // then
verify(management).performRegister(eq(RegistrationMethod.API_REGISTRATION), verify(management).performRegister(eq(RegistrationMethod.API_REGISTRATION),
argThat(isEqualTo(ApiPasswordRegisterParams.of(player, pass, false)))); argThat(hasEqualValuesOnAllFields(ApiPasswordRegisterParams.of(player, pass, false))));
} }
@Test @Test

View File

@ -30,7 +30,7 @@ import org.mockito.junit.MockitoJUnitRunner;
import java.util.Arrays; import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import static fr.xephi.authme.IsEqualByReflectionMatcher.isEqualTo; import static fr.xephi.authme.IsEqualByReflectionMatcher.hasEqualValuesOnAllFields;
import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.containsString;
import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.eq;
@ -101,7 +101,7 @@ public class RegisterCommandTest {
// then // then
verify(registrationCaptchaManager).isCaptchaRequired("test2"); verify(registrationCaptchaManager).isCaptchaRequired("test2");
verify(management).performRegister(eq(RegistrationMethod.TWO_FACTOR_REGISTRATION), verify(management).performRegister(eq(RegistrationMethod.TWO_FACTOR_REGISTRATION),
argThat(isEqualTo(TwoFactorRegisterParams.of(player)))); argThat(hasEqualValuesOnAllFields(TwoFactorRegisterParams.of(player))));
verifyZeroInteractions(emailService); verifyZeroInteractions(emailService);
} }
@ -221,7 +221,7 @@ public class RegisterCommandTest {
verify(validationService).validateEmail(playerMail); verify(validationService).validateEmail(playerMail);
verify(emailService).hasAllInformation(); verify(emailService).hasAllInformation();
verify(management).performRegister(eq(RegistrationMethod.EMAIL_REGISTRATION), verify(management).performRegister(eq(RegistrationMethod.EMAIL_REGISTRATION),
argThat(isEqualTo(EmailRegisterParams.of(player, playerMail)))); argThat(hasEqualValuesOnAllFields(EmailRegisterParams.of(player, playerMail))));
} }
@Test @Test
@ -250,7 +250,7 @@ public class RegisterCommandTest {
// then // then
verify(registrationCaptchaManager).isCaptchaRequired("newPlayer"); verify(registrationCaptchaManager).isCaptchaRequired("newPlayer");
verify(management).performRegister(eq(RegistrationMethod.PASSWORD_REGISTRATION), verify(management).performRegister(eq(RegistrationMethod.PASSWORD_REGISTRATION),
argThat(isEqualTo(PasswordRegisterParams.of(player, "myPass", null)))); argThat(hasEqualValuesOnAllFields(PasswordRegisterParams.of(player, "myPass", null))));
} }
@Test @Test
@ -268,7 +268,7 @@ public class RegisterCommandTest {
// then // then
verify(validationService).validateEmail(email); verify(validationService).validateEmail(email);
verify(management).performRegister(eq(RegistrationMethod.PASSWORD_REGISTRATION), verify(management).performRegister(eq(RegistrationMethod.PASSWORD_REGISTRATION),
argThat(isEqualTo(PasswordRegisterParams.of(player, "myPass", email)))); argThat(hasEqualValuesOnAllFields(PasswordRegisterParams.of(player, "myPass", email))));
} }
@Test @Test
@ -303,7 +303,7 @@ public class RegisterCommandTest {
// then // then
verify(registrationCaptchaManager).isCaptchaRequired("Doa"); verify(registrationCaptchaManager).isCaptchaRequired("Doa");
verify(management).performRegister(eq(RegistrationMethod.PASSWORD_REGISTRATION), verify(management).performRegister(eq(RegistrationMethod.PASSWORD_REGISTRATION),
argThat(isEqualTo(PasswordRegisterParams.of(player, "myPass", null)))); argThat(hasEqualValuesOnAllFields(PasswordRegisterParams.of(player, "myPass", null))));
} }
@Test @Test

View File

@ -5,7 +5,6 @@ import fr.xephi.authme.TestHelper;
import fr.xephi.authme.data.auth.PlayerCache; import fr.xephi.authme.data.auth.PlayerCache;
import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.datasource.DataSource;
import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.message.MessageKey;
import fr.xephi.authme.permission.PermissionsManager;
import fr.xephi.authme.process.register.executors.PasswordRegisterParams; import fr.xephi.authme.process.register.executors.PasswordRegisterParams;
import fr.xephi.authme.process.register.executors.RegistrationExecutor; import fr.xephi.authme.process.register.executors.RegistrationExecutor;
import fr.xephi.authme.process.register.executors.RegistrationMethod; import fr.xephi.authme.process.register.executors.RegistrationMethod;
@ -39,8 +38,6 @@ public class AsyncRegisterTest {
@Mock @Mock
private PlayerCache playerCache; private PlayerCache playerCache;
@Mock @Mock
private PermissionsManager permissionsManager;
@Mock
private CommonService commonService; private CommonService commonService;
@Mock @Mock
private DataSource dataSource; private DataSource dataSource;

View File

@ -5,7 +5,6 @@ import fr.xephi.authme.data.auth.PlayerAuth;
import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.datasource.DataSource;
import fr.xephi.authme.mail.EmailService; import fr.xephi.authme.mail.EmailService;
import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.message.MessageKey;
import fr.xephi.authme.permission.PermissionsManager;
import fr.xephi.authme.permission.PlayerStatePermission; import fr.xephi.authme.permission.PlayerStatePermission;
import fr.xephi.authme.process.SyncProcessManager; import fr.xephi.authme.process.SyncProcessManager;
import fr.xephi.authme.security.PasswordSecurity; import fr.xephi.authme.security.PasswordSecurity;
@ -35,13 +34,11 @@ import static org.mockito.Mockito.verifyZeroInteractions;
* Test for {@link EmailRegisterExecutor}. * Test for {@link EmailRegisterExecutor}.
*/ */
@RunWith(MockitoJUnitRunner.class) @RunWith(MockitoJUnitRunner.class)
public class EmailRegisterExecutorProviderTest { public class EmailRegisterExecutorTest {
@InjectMocks @InjectMocks
private EmailRegisterExecutor executor; private EmailRegisterExecutor executor;
@Mock
private PermissionsManager permissionsManager;
@Mock @Mock
private DataSource dataSource; private DataSource dataSource;
@Mock @Mock
@ -68,7 +65,7 @@ public class EmailRegisterExecutorProviderTest {
// then // then
assertThat(result, equalTo(false)); assertThat(result, equalTo(false));
verify(dataSource).countAuthsByEmail(email); verify(dataSource).countAuthsByEmail(email);
verify(permissionsManager).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS); verify(commonService).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS);
verify(commonService).send(player, MessageKey.MAX_REGISTER_EXCEEDED, "3", "4", "@"); verify(commonService).send(player, MessageKey.MAX_REGISTER_EXCEEDED, "3", "4", "@");
} }
@ -77,7 +74,7 @@ public class EmailRegisterExecutorProviderTest {
// given // given
given(commonService.getProperty(EmailSettings.MAX_REG_PER_EMAIL)).willReturn(3); given(commonService.getProperty(EmailSettings.MAX_REG_PER_EMAIL)).willReturn(3);
Player player = mock(Player.class); Player player = mock(Player.class);
given(permissionsManager.hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS)).willReturn(true); given(commonService.hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS)).willReturn(true);
EmailRegisterParams params = EmailRegisterParams.of(player, "test@example.com"); EmailRegisterParams params = EmailRegisterParams.of(player, "test@example.com");
// when // when
@ -85,7 +82,7 @@ public class EmailRegisterExecutorProviderTest {
// then // then
assertThat(result, equalTo(true)); assertThat(result, equalTo(true));
verify(permissionsManager).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS); verify(commonService).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS);
} }
@Test @Test
@ -102,7 +99,7 @@ public class EmailRegisterExecutorProviderTest {
// then // then
assertThat(result, equalTo(true)); assertThat(result, equalTo(true));
verify(permissionsManager).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS); verify(commonService).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS);
verify(dataSource).countAuthsByEmail(email); verify(dataSource).countAuthsByEmail(email);
} }

View File

@ -1,6 +1,7 @@
package fr.xephi.authme.util; package fr.xephi.authme.util;
import fr.xephi.authme.ReflectionTestUtils; import fr.xephi.authme.ReflectionTestUtils;
import fr.xephi.authme.TestHelper;
import org.junit.Test; import org.junit.Test;
import java.util.ConcurrentModificationException; import java.util.ConcurrentModificationException;
@ -51,4 +52,10 @@ public class ExceptionUtilsTest {
assertThat(resultNpe, nullValue()); assertThat(resultNpe, nullValue());
assertThat(resultUoe, sameInstance(uoe)); assertThat(resultUoe, sameInstance(uoe));
} }
@Test
public void shouldHaveHiddenConstructor() {
// given / when / then
TestHelper.validateHasOnlyPrivateEmptyConstructor(ExceptionUtils.class);
}
} }