Meishin/LoginSystem
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

#1141 Split TOTP permissions for add/remove, refactor TOTP services

Browse Source 
- Split TotpService further into GenerateTotpService and TotpAuthenticator, which wraps the GoogleAuthenticator impl
- Add missing tests for the services
- Change GenerateTotpService's interface to behave like a collection for more intuitive method behavior
This commit is contained in:
ljacqu
2018-03-10 16:21:53 +01:00
parent e72d5d5e81
commit eb9cd31a65
17 changed files with 478 additions and 228 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/test/java/fr/xephi/authme/security/TotpServiceTest.java
-51
View File
@@ -1,51 +0,0 @@
package fr.xephi.authme.security;
import fr.xephi.authme.security.TotpService.TotpGenerationResult;
import fr.xephi.authme.service.BukkitService;
import org.bukkit.entity.Player;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
import static fr.xephi.authme.AuthMeMatchers.stringWithLength;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.Assert.assertThat;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
/**
* Test for {@link TotpService}.
*/
@RunWith(MockitoJUnitRunner.class)
public class TotpServiceTest {
@InjectMocks
private TotpService totpService;
@Mock
private BukkitService bukkitService;
@Test
public void shouldGenerateTotpKey() {
// given
Player player = mock(Player.class);
given(player.getName()).willReturn("Bobby");
given(bukkitService.getIp()).willReturn("127.48.44.4");
// when
TotpGenerationResult key1 = totpService.generateTotpKey(player);
TotpGenerationResult key2 = totpService.generateTotpKey(player);
// then
assertThat(key1.getTotpKey(), stringWithLength(16));
assertThat(key2.getTotpKey(), stringWithLength(16));
assertThat(key1.getAuthenticatorQrCodeUrl(), startsWith("https://chart.googleapis.com/chart?chs=200x200"));
assertThat(key2.getAuthenticatorQrCodeUrl(), startsWith("https://chart.googleapis.com/chart?chs=200x200"));
assertThat(key1.getTotpKey(), not(equalTo(key2.getTotpKey())));
}
}
src/test/java/fr/xephi/authme/security/totp/GenerateTotpServiceTest.java
+113
View File
@@ -0,0 +1,113 @@
package fr.xephi.authme.security.totp;
import fr.xephi.authme.ReflectionTestUtils;
import fr.xephi.authme.security.totp.TotpAuthenticator.TotpGenerationResult;
import fr.xephi.authme.util.expiring.ExpiringMap;
import org.bukkit.entity.Player;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
import java.util.concurrent.TimeUnit;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertThat;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
/**
* Test for {@link GenerateTotpService}.
*/
@RunWith(MockitoJUnitRunner.class)
public class GenerateTotpServiceTest {
@InjectMocks
private GenerateTotpService generateTotpService;
@Mock
private TotpAuthenticator totpAuthenticator;
@Test
public void shouldGenerateTotpKey() {
// given
TotpGenerationResult givenGenerationResult = new TotpGenerationResult("1234", "http://example.com/link/to/chart");
Player player = mockPlayerWithName("Spencer");
given(totpAuthenticator.generateTotpKey(player)).willReturn(givenGenerationResult);
// when
TotpGenerationResult result = generateTotpService.generateTotpKey(player);
// then
assertThat(result, equalTo(givenGenerationResult));
assertThat(generateTotpService.getGeneratedTotpKey(player), equalTo(givenGenerationResult));
}
@Test
public void shouldRemoveGeneratedTotpKey() {
// given
TotpGenerationResult givenGenerationResult = new TotpGenerationResult("1234", "http://example.com/link/to/chart");
Player player = mockPlayerWithName("Hanna");
given(totpAuthenticator.generateTotpKey(player)).willReturn(givenGenerationResult);
generateTotpService.generateTotpKey(player);
// when
generateTotpService.removeGenerateTotpKey(player);
// then
assertThat(generateTotpService.getGeneratedTotpKey(player), nullValue());
}
@Test
public void shouldCheckGeneratedTotpKey() {
// given
String generatedKey = "ASLO43KDF2J";
TotpGenerationResult givenGenerationResult = new TotpGenerationResult(generatedKey, "url");
Player player = mockPlayerWithName("Aria");
given(totpAuthenticator.generateTotpKey(player)).willReturn(givenGenerationResult);
generateTotpService.generateTotpKey(player);
String validCode = "928374";
given(totpAuthenticator.checkCode(generatedKey, validCode)).willReturn(true);
// when
boolean invalidCodeResult = generateTotpService.isTotpCodeCorrectForGeneratedTotpKey(player, "000000");
boolean validCodeResult = generateTotpService.isTotpCodeCorrectForGeneratedTotpKey(player, validCode);
boolean unknownPlayerResult = generateTotpService.isTotpCodeCorrectForGeneratedTotpKey(mockPlayerWithName("other"), "299874");
// then
assertThat(invalidCodeResult, equalTo(false));
assertThat(validCodeResult, equalTo(true));
assertThat(unknownPlayerResult, equalTo(false));
verify(totpAuthenticator).checkCode(generatedKey, "000000");
verify(totpAuthenticator).checkCode(generatedKey, validCode);
}
@Test
public void shouldRemoveExpiredEntries() throws InterruptedException {
// given
TotpGenerationResult generationResult = new TotpGenerationResult("key", "url");
ExpiringMap<String, TotpGenerationResult> generatedKeys =
ReflectionTestUtils.getFieldValue(GenerateTotpService.class, generateTotpService, "totpKeys");
generatedKeys.setExpiration(1, TimeUnit.MILLISECONDS);
generatedKeys.put("ghost", generationResult);
generatedKeys.setExpiration(5, TimeUnit.MINUTES);
generatedKeys.put("ezra", generationResult);
// when
Thread.sleep(2L);
generateTotpService.performCleanup();
// then
assertThat(generateTotpService.getGeneratedTotpKey(mockPlayerWithName("Ezra")), equalTo(generationResult));
assertThat(generateTotpService.getGeneratedTotpKey(mockPlayerWithName("ghost")), nullValue());
}
private static Player mockPlayerWithName(String name) {
Player player = mock(Player.class);
given(player.getName()).willReturn(name);
return player;
}
}
src/test/java/fr/xephi/authme/security/totp/TotpAuthenticatorTest.java
+84
View File
@@ -0,0 +1,84 @@
package fr.xephi.authme.security.totp;
import com.warrenstrange.googleauth.IGoogleAuthenticator;
import fr.xephi.authme.security.totp.TotpAuthenticator.TotpGenerationResult;
import fr.xephi.authme.service.BukkitService;
import org.bukkit.entity.Player;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
import static fr.xephi.authme.AuthMeMatchers.stringWithLength;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.startsWith;
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 TotpAuthenticator}.
*/
@RunWith(MockitoJUnitRunner.class)
public class TotpAuthenticatorTest {
@InjectMocks
private TotpAuthenticator totpAuthenticator;
@Mock
private BukkitService bukkitService;
@Mock
private IGoogleAuthenticator googleAuthenticator;
@Test
public void shouldGenerateTotpKey() {
// given
// Use the GoogleAuthenticator instance the TotpAuthenticator normally creates to test its parameters
totpAuthenticator = new TotpAuthenticator(bukkitService);
Player player = mock(Player.class);
given(player.getName()).willReturn("Bobby");
given(bukkitService.getIp()).willReturn("127.48.44.4");
// when
TotpGenerationResult key1 = totpAuthenticator.generateTotpKey(player);
TotpGenerationResult key2 = totpAuthenticator.generateTotpKey(player);
// then
assertThat(key1.getTotpKey(), stringWithLength(16));
assertThat(key2.getTotpKey(), stringWithLength(16));
assertThat(key1.getAuthenticatorQrCodeUrl(), startsWith("https://chart.googleapis.com/chart?chs=200x200"));
assertThat(key2.getAuthenticatorQrCodeUrl(), startsWith("https://chart.googleapis.com/chart?chs=200x200"));
assertThat(key1.getTotpKey(), not(equalTo(key2.getTotpKey())));
}
@Test
public void shouldCheckCode() {
// given
String secret = "the_secret";
int code = 21398;
given(googleAuthenticator.authorize(secret, code)).willReturn(true);
// when
boolean result = totpAuthenticator.checkCode(secret, Integer.toString(code));
// then
assertThat(result, equalTo(true));
verify(googleAuthenticator).authorize(secret, code);
}
@Test
public void shouldHandleInvalidNumberInput() {
// given / when
boolean result = totpAuthenticator.checkCode("Some_Secret", "123ZZ");
// then
assertThat(result, equalTo(false));
verifyZeroInteractions(googleAuthenticator);
}
}
src/test/java/fr/xephi/authme/security/totp/TotpServiceTest.java
+46
View File
@@ -0,0 +1,46 @@
package fr.xephi.authme.security.totp;
import fr.xephi.authme.data.auth.PlayerAuth;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertThat;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.verify;
/**
* Test for {@link TotpService}.
*/
@RunWith(MockitoJUnitRunner.class)
public class TotpServiceTest {
@InjectMocks
private TotpService totpService;
@Mock
private TotpAuthenticator totpAuthenticator;
@Test
public void shouldVerifyCode() {
// given
String totpKey = "ASLO43KDF2J";
PlayerAuth auth = PlayerAuth.builder()
.name("Maya")
.totpKey(totpKey)
.build();
String inputCode = "408435";
given(totpAuthenticator.checkCode(totpKey, inputCode)).willReturn(true);
// when
boolean result = totpService.verifyCode(auth, inputCode);
// then
assertThat(result, equalTo(true));
verify(totpAuthenticator).checkCode(totpKey, inputCode);
}
}
src/test/java/fr/xephi/authme/service/HelpTranslationGeneratorIntegrationTest.java
+1 -1
View File
@@ -126,7 +126,7 @@ public class HelpTranslationGeneratorIntegrationTest {
// Check /login
checkDescription(configuration.get("commands.login"), "Login command", "/login detailed desc.");
checkArgs(configuration.get("commands.login"), arg("loginArg", "Login password"));
checkArgs(configuration.get("commands.login"), arg("loginArg", "Login password"), arg("2faArg", "TOTP code"));
// Check /unregister
checkDescription(configuration.get("commands.unregister"), "unreg_desc", "unreg_detail_desc");
src/test/resources/fr/xephi/authme/message/help_test.yml
+3
View File
@@ -65,6 +65,9 @@ commands:
arg1:
label: loginArg
nonExistent: does not exist
arg2:
label: 2faArg
rhetorical: false
someProp:
label: '''someProp'' does not exist'
description: idk