#653 Empty salt column causes error when retrieving password

- Handle potentially empty salt column in MySQL and SQLite
- Create unit tests reflecting these cases
This commit is contained in:
ljacqu 2016-04-14 11:48:24 +02:00
parent 3c59bb1efb
commit 3bb7ff2b85
5 changed files with 59 additions and 25 deletions

View File

@ -274,20 +274,20 @@ public class MySQL implements DataSource {
@Override
public HashedPassword getPassword(String user) {
String sql = "SELECT " + col.PASSWORD + "," + col.SALT + " FROM " + tableName
+ " WHERE " + col.NAME + "=?;";
ResultSet rs = null;
boolean useSalt = !col.SALT.isEmpty();
String sql = "SELECT " + col.PASSWORD
+ (useSalt ? ", " + col.SALT : "")
+ " FROM " + tableName + " WHERE " + col.NAME + "=?;";
try (Connection con = getConnection(); PreparedStatement pst = con.prepareStatement(sql)) {
pst.setString(1, user.toLowerCase());
rs = pst.executeQuery();
if (rs.next()) {
return new HashedPassword(rs.getString(col.PASSWORD),
!col.SALT.isEmpty() ? rs.getString(col.SALT) : null);
try (ResultSet rs = pst.executeQuery()) {
if (rs.next()) {
return new HashedPassword(rs.getString(col.PASSWORD),
useSalt ? rs.getString(col.SALT) : null);
}
}
} catch (SQLException ex) {
logSqlException(ex);
} finally {
close(rs);
}
return null;
}

View File

@ -154,22 +154,20 @@ public class SQLite implements DataSource {
@Override
public HashedPassword getPassword(String user) {
PreparedStatement pst = null;
ResultSet rs = null;
try {
pst = con.prepareStatement("SELECT " + col.PASSWORD + "," + col.SALT
+ " FROM " + tableName + " WHERE " + col.NAME + "=?");
boolean useSalt = !col.SALT.isEmpty();
String sql = "SELECT " + col.PASSWORD
+ (useSalt ? ", " + col.SALT : "")
+ " FROM " + tableName + " WHERE " + col.NAME + "=?";
try (PreparedStatement pst = con.prepareStatement(sql)) {
pst.setString(1, user);
rs = pst.executeQuery();
if (rs.next()) {
return new HashedPassword(rs.getString(col.PASSWORD),
!col.SALT.isEmpty() ? rs.getString(col.SALT) : null);
try (ResultSet rs = pst.executeQuery()) {
if (rs.next()) {
return new HashedPassword(rs.getString(col.PASSWORD),
useSalt ? rs.getString(col.SALT) : null);
}
}
} catch (SQLException ex) {
logSqlException(ex);
} finally {
close(rs);
close(pst);
}
return null;
}

View File

@ -22,7 +22,11 @@ import static org.junit.Assume.assumeThat;
*/
public abstract class AbstractDataSourceIntegrationTest {
protected abstract DataSource getDataSource();
protected DataSource getDataSource() {
return getDataSource("salt");
}
protected abstract DataSource getDataSource(String saltColumn);
@Test
public void shouldReturnIfAuthIsAvailableOrNot() {
@ -56,6 +60,22 @@ public abstract class AbstractDataSourceIntegrationTest {
assertThat(userPassword, equalToHash("b28c32f624a4eb161d6adc9acb5bfc5b", "f750ba32"));
}
@Test
public void shouldReturnPasswordWithEmptySaltColumn() {
// given
DataSource dataSource = getDataSource("");
// when
HashedPassword bobbyPassword = dataSource.getPassword("bobby");
HashedPassword invalidPassword = dataSource.getPassword("doesNotExist");
HashedPassword userPassword = dataSource.getPassword("user");
// then
assertThat(bobbyPassword, equalToHash("$SHA$11aa0706173d7272$dbba966"));
assertThat(invalidPassword, nullValue());
assertThat(userPassword, equalToHash("b28c32f624a4eb161d6adc9acb5bfc5b"));
}
@Test
public void shouldGetAuth() {
// given
@ -133,6 +153,21 @@ public abstract class AbstractDataSourceIntegrationTest {
assertThat(dataSource.getPassword("user"), equalToHash(newHash));
}
@Test
public void shouldUpdatePasswordWithNoSalt() {
// given
DataSource dataSource = getDataSource("");
HashedPassword newHash = new HashedPassword("new_hash", "1241");
// when
boolean response1 = dataSource.updatePassword("user", newHash);
boolean response2 = dataSource.updatePassword("non-existent-name", new HashedPassword("asdfasdf", "a1f34ec"));
// then
assertThat(response1 && response2, equalTo(true));
assertThat(dataSource.getPassword("user"), equalToHash("new_hash"));
}
@Test
public void shouldRemovePlayerAuth() {
// given

View File

@ -53,7 +53,6 @@ public class MySqlIntegrationTest extends AbstractDataSourceIntegrationTest {
});
set(DatabaseSettings.MYSQL_DATABASE, "h2_test");
set(DatabaseSettings.MYSQL_TABLE, "authme");
set(DatabaseSettings.MYSQL_COL_SALT, "salt");
ConsoleLoggerTestInitializer.setupLogger();
Path sqlInitFile = TestHelper.getJarPath("/datasource-integration/sql-initialize.sql");
@ -80,7 +79,8 @@ public class MySqlIntegrationTest extends AbstractDataSourceIntegrationTest {
}
@Override
protected DataSource getDataSource() {
protected DataSource getDataSource(String saltColumn) {
when(settings.getProperty(DatabaseSettings.MYSQL_COL_SALT)).thenReturn(saltColumn);
return new MySQL(settings, hikariSource);
}

View File

@ -75,7 +75,8 @@ public class SQLiteIntegrationTest extends AbstractDataSourceIntegrationTest {
}
@Override
protected DataSource getDataSource() {
protected DataSource getDataSource(String saltColumn) {
when(settings.getProperty(DatabaseSettings.MYSQL_COL_SALT)).thenReturn(saltColumn);
return new SQLite(settings, con);
}