Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Config] Add password algorithm selector #9468

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
12 changes: 7 additions & 5 deletions SQL/0000-00-03-ConfigTables.sql
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ CREATE TABLE `ConfigSettings` (
`Description` varchar(255) DEFAULT NULL,
`Visible` tinyint(1) DEFAULT '0',
`AllowMultiple` tinyint(1) DEFAULT '0',
`DataType` ENUM('text','boolean','email','instrument','textarea','scan_type','date_format','lookup_center','path','web_path', 'log_level') DEFAULT NULL,
`DataType` ENUM('text','boolean','email','instrument','textarea','scan_type','date_format','lookup_center','path','web_path','log_level','password_algo') DEFAULT NULL,
`Parent` int(11) DEFAULT NULL,
`Label` varchar(255) DEFAULT NULL,
`OrderNumber` int(11) DEFAULT NULL,
Expand Down Expand Up @@ -59,10 +59,11 @@ INSERT INTO ConfigSettings (Name, Description, Visible, AllowMultiple, DataType,
INSERT INTO ConfigSettings (Name, Description, Visible, AllowMultiple, DataType, Parent, Label, OrderNumber) SELECT 'citation_policy', 'Citation Policy for Acknowledgements module', 1, 0, 'textarea', ID, 'Citation Policy', 25 FROM ConfigSettings WHERE Name="study";
INSERT INTO ConfigSettings (Name, Description, Visible, AllowMultiple, DataType, Parent, Label, OrderNumber) SELECT 'CSPAdditionalHeaders', 'Extensions to the Content-security policy allow only for self-hosted content', 1, 0, 'text', ID, 'Content-Security Extensions', 26 FROM ConfigSettings WHERE Name="study";
INSERT INTO ConfigSettings (Name, Description, Visible, AllowMultiple, DataType, Parent, Label, OrderNumber) SELECT 'usePwnedPasswordsAPI', 'Whether to query the Have I Been Pwned password API on password changes to prevent the usage of common and breached passwords', 1, 0, 'boolean', ID, 'Enable "Pwned Password" check', 27 FROM ConfigSettings WHERE Name="study";
INSERT INTO ConfigSettings (Name, Description, Visible, AllowMultiple, DataType, Parent, Label, OrderNumber) SELECT 'dateDisplayFormat', 'The date format to use throughout LORIS for displaying date information - formats for date inputs are browser- and locale-dependent.', 1, 0, 'text', ID, 'Date display format', 28 FROM ConfigSettings WHERE Name="study";
INSERT INTO ConfigSettings (Name, Description, Visible, AllowMultiple, DataType, Parent, Label, OrderNumber) SELECT 'adminContactEmail', 'An email address that users can write to in order to report issues or ask question', 1, 0, 'text', ID, 'Administrator Email', 29 FROM ConfigSettings WHERE Name="study";
INSERT INTO ConfigSettings (Name, Description, Visible, AllowMultiple, DataType, Parent, Label, OrderNumber) SELECT 'UserMaximumDaysInactive', 'The maximum number of days since last login before making a user inactive', 1, 0, 'text', ID, 'Maximum Days Before Making User Inactive', 30 FROM ConfigSettings WHERE Name="study";
INSERT INTO ConfigSettings (Name, Description, Visible, AllowMultiple, DataType, Parent, Label, OrderNumber) SELECT 'useDoB', 'Use DoB (Date of Birth)', 1, 0, 'boolean', ID, 'Use DoB', 31 FROM ConfigSettings WHERE Name="study";
INSERT INTO ConfigSettings (Name, Description, Visible, AllowMultiple, DataType, Parent, Label, OrderNumber) SELECT 'passwordAlgorithm','Which PHP password algorithm to use for hashing the passwords',1,0,'password_algo', ID,'Password Algorithm', 28 FROM ConfigSettings WHERE Name="study";
INSERT INTO ConfigSettings (Name, Description, Visible, AllowMultiple, DataType, Parent, Label, OrderNumber) SELECT 'dateDisplayFormat', 'The date format to use throughout LORIS for displaying date information - formats for date inputs are browser- and locale-dependent.', 1, 0, 'text', ID, 'Date display format', 29 FROM ConfigSettings WHERE Name="study";
INSERT INTO ConfigSettings (Name, Description, Visible, AllowMultiple, DataType, Parent, Label, OrderNumber) SELECT 'adminContactEmail', 'An email address that users can write to in order to report issues or ask question', 1, 0, 'text', ID, 'Administrator Email', 30 FROM ConfigSettings WHERE Name="study";
INSERT INTO ConfigSettings (Name, Description, Visible, AllowMultiple, DataType, Parent, Label, OrderNumber) SELECT 'UserMaximumDaysInactive', 'The maximum number of days since last login before making a user inactive', 1, 0, 'text', ID, 'Maximum Days Before Making User Inactive', 31 FROM ConfigSettings WHERE Name="study";
INSERT INTO ConfigSettings (Name, Description, Visible, AllowMultiple, DataType, Parent, Label, OrderNumber) SELECT 'useDoB', 'Use DoB (Date of Birth)', 1, 0, 'boolean', ID, 'Use DoB', 32 FROM ConfigSettings WHERE Name="study";

INSERT INTO ConfigSettings (Name, Description, Visible, AllowMultiple, Label, OrderNumber) VALUES ('paths', 'Specify directories where LORIS-related files are stored or created. Take care when editing these fields as changing them incorrectly can cause certain modules to lose functionality.', 1, 0, 'Paths', 2);
INSERT INTO ConfigSettings (Name, Description, Visible, AllowMultiple, DataType, Parent, Label, OrderNumber) SELECT 'imagePath', 'Path to images for display in Imaging Browser (e.g. /data/$project/data/) ', 1, 0, 'text', ID, 'Images', 9 FROM ConfigSettings WHERE Name="paths";
Expand Down Expand Up @@ -282,6 +283,7 @@ INSERT INTO Config (ConfigID, Value) SELECT ID, 't1' FROM ConfigSettings WHER
INSERT INTO Config (ConfigID, Value) SELECT ID, 't2' FROM ConfigSettings WHERE Name="modalities_to_deface";
INSERT INTO Config (ConfigID, Value) SELECT ID, 'pd' FROM ConfigSettings WHERE Name="modalities_to_deface";
INSERT INTO Config (ConfigID, Value) SELECT ID, 'false' FROM ConfigSettings WHERE Name="usePwnedPasswordsAPI";
INSERT INTO Config (ConfigID, Value) SELECT ID, '2y' FROM ConfigSettings WHERE Name="passwordAlgorithm";
INSERT INTO Config (ConfigID, Value) SELECT ID, 'Y-m-d H:i:s' FROM ConfigSettings WHERE Name="dateDisplayFormat";
INSERT INTO Config (ConfigID, Value) SELECT ID, '/data/issue_tracker/' FROM ConfigSettings WHERE Name="IssueTrackerDataPath";
INSERT INTO Config (ConfigID, Value) SELECT ID, '' FROM ConfigSettings WHERE Name="adminContactEmail";
Expand Down
7 changes: 7 additions & 0 deletions SQL/New_patches/2024-11-14-Add-Password-Algo-Config.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
ALTER TABLE ConfigSettings
MODIFY COLUMN DataType enum('text','boolean','email','instrument','textarea','scan_type','date_format','lookup_center','path','web_path','log_level', 'password_algo') DEFAULT NULL;

INSERT INTO `ConfigSettings` (`Name`, `Description`, `Visible`, `AllowMultiple`, `DataType`, `Parent`, `Label`, `OrderNumber`)
VALUES ('passwordAlgorithm','Which PHP password algorithm to use for hashing the passwords',1,0,'password_algo',1,'Password Algorithm',28);

INSERT INTO Config (`ConfigID`, `Value`) VALUES (LAST_INSERT_ID(), '2y');
4 changes: 3 additions & 1 deletion htdocs/postdeploy.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@
$sqls = file_get_contents($path_to_file);
$conn->exec($sqls);

$pw = password_hash($password, PASSWORD_DEFAULT);
$config =& \NDB_Config::singleton();
$password_algo = $config->getSetting("passwordAlgorithm");
$pw = password_hash($password, $password_algo);

$conn->query(
"UPDATE users SET Password_hash=" . $conn->quote($pw) .
Expand Down
10 changes: 10 additions & 0 deletions modules/configuration/php/configuration.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,16 @@ class Configuration extends \NDB_Form
'emergency' => 'Emergency',
];

$this->tpl_data['password_algos'] = array_combine(
password_algos(),
array_map(
function ($algo) {
return $algo === '2y' ? 'bcrypt' : $algo;
},
password_algos()
)
);

skarya22 marked this conversation as resolved.
Show resolved Hide resolved
$this->tpl_data['date_format'] = $date_format;
$this->tpl_data['lookup_center'] = [
'' => '',
Expand Down
10 changes: 9 additions & 1 deletion modules/configuration/templates/form_configuration.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,13 @@
</div>
{/function}


{function name=createPasswordAlgo}
<select class="form-control" name="{$k}" {if $d eq "Yes"}disabled{/if}>
{foreach from=$password_algos key=name item=label}
<option {if $v eq $name}selected{/if} value="{$name}">{$label}</option>
{/foreach}
</select>
{/function}

{function name=createScanType}
<select class="form-control" name="{$k}" {if $d eq "Yes"}disabled{/if}>
Expand Down Expand Up @@ -108,6 +114,8 @@
{call createInstrument k=$id v=$v d=$node['Disabled']}
{elseif $node['DataType'] eq 'scan_type'}
{call createScanType k=$id v=$v d=$node['Disabled']}
{elseif $node['DataType'] eq 'password_algo'}
{call createPasswordAlgo k=$id v=$v d=$node['Disabled']}
{elseif $node['DataType'] eq 'date_format'}
{call createDateFormat k=$id v=$v d=$node['Disabled']}
{elseif $node['DataType'] eq 'email'}
Expand Down
6 changes: 4 additions & 2 deletions php/installer/Installer.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -535,20 +535,22 @@ class Installer
return false;
}

$stmt = $DB->prepare(
$stmt = $DB->prepare(
"UPDATE users
SET
UserID=:q_userID,
Password_hash=:q_password,
Active='Y'
WHERE ID=1"
);
$config =& \NDB_Config::singleton();
$password_algo = $config->getSetting("passwordAlgorithm");
return $stmt->execute(
[
'q_userID' => $values['frontenduser'],
'q_password' => password_hash(
$values['frontendpassword'],
PASSWORD_DEFAULT
$password_algo
),
]
);
Expand Down
20 changes: 15 additions & 5 deletions php/libraries/Password.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -195,17 +195,27 @@ class Password
* Creates a new Password object when the $value param is well-formed.
* well-formed.
*
* @param string $value The proposed password value.
* @param string $value The proposed password value.
* @param ?NDB_Config $config The config
*
* @throws InvalidArgumentException When passsword is too short or too simple.
*/
public final function __construct(string $value)
public final function __construct(string $value, ?NDB_Config $config = null)
{
// Ensure proposed value is well-formed.
$this->_validate($value);
// Don't store the value in the object; instead use the hashed version
// This mitigates the risk of accidentally revealing the plaintext.
$this->_hash = password_hash($value, PASSWORD_DEFAULT);
$config = $config ?? \NDB_Config::singleton();
$password_algo = $config->getSetting("passwordAlgorithm");

if (empty($password_algo)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the main selling points of password_hash is that it's supposed to have sensible defaults that can be upgraded by PHP over time. I think in this case, you should use PASSWORD_DEFAULT (and that there should be an option to explicitly use the default from PHP.

throw new ConfigurationException(
"Password algorithm is not configured in the settings.
"
);
}

// Hash the password using the configured algorithm
$this->_hash = password_hash($value, $password_algo);
}

/**
Expand Down
6 changes: 4 additions & 2 deletions php/libraries/SinglePointLogin.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,10 @@ class SinglePointLogin
// Validate passsword
$oldhash = $row['Password_hash'] ?? '';
if (password_verify($password, $oldhash)) {
$config =& \NDB_Config::singleton();
$password_algo = $config->getSetting("passwordAlgorithm");
// Rehash according to latest standards if necessary.
if (password_needs_rehash($oldhash, PASSWORD_DEFAULT)) {
if (password_needs_rehash($oldhash, $password_algo)) {
try {
$newHash = new \Password($password);
\User::factory($username)->updatePassword(
Expand All @@ -302,7 +304,7 @@ class SinglePointLogin
// update the password here without using the Password class
$newHash = password_hash(
$password,
PASSWORD_DEFAULT
$password_algo
);
\NDB_Factory::singleton()->database()->update(
'users',
Expand Down
1 change: 1 addition & 0 deletions raisinbread/RB_files/RB_Config.sql
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ INSERT INTO `Config` (`ID`, `ConfigID`, `Value`) VALUES (126,129,'365');
INSERT INTO `Config` (`ID`, `ConfigID`, `Value`) VALUES (127,130,'/var/www/loris/');
INSERT INTO `Config` (`ID`, `ConfigID`, `Value`) VALUES (128,131,'/data/EEGUploadIncomingPath/');
INSERT INTO `Config` (`ID`, `ConfigID`, `Value`) VALUES (129,132,'false');
INSERT INTO `Config` (`ID`, `ConfigID`, `Value`) VALUES (130,133,'2y');

UNLOCK TABLES;
SET FOREIGN_KEY_CHECKS=1;
1 change: 1 addition & 0 deletions raisinbread/RB_files/RB_ConfigSettings.sql
Original file line number Diff line number Diff line change
Expand Up @@ -121,5 +121,6 @@ INSERT INTO `ConfigSettings` (`ID`, `Name`, `Description`, `Visible`, `AllowMult
INSERT INTO `ConfigSettings` (`ID`, `Name`, `Description`, `Visible`, `AllowMultiple`, `DataType`, `Parent`, `Label`, `OrderNumber`) VALUES (130,'DownloadPath','Where files are downloaded',1,0,'text',26,'Downloads',4);
INSERT INTO `ConfigSettings` (`ID`, `Name`, `Description`, `Visible`, `AllowMultiple`, `DataType`, `Parent`, `Label`, `OrderNumber`) VALUES (131,'EEGUploadIncomingPath', 'Path to the upload directory for incoming EEG studies', 1, 0, 'text', 26, 'EEG Incoming Directory', 15);
INSERT INTO `ConfigSettings` (`ID`, `Name`, `Description`, `Visible`, `AllowMultiple`, `DataType`, `Parent`, `Label`, `OrderNumber`) VALUES (132,'useDoB','Use DoB (Date of Birth)',1,0,'boolean',1,'Use DoB',12);
INSERT INTO `ConfigSettings` (`ID`, `Name`, `Description`, `Visible`, `AllowMultiple`, `DataType`, `Parent`, `Label`, `OrderNumber`) VALUES (133, 'passwordAlgorithm','Which PHP password algorithm to use for hashing the passwords',1,0,'password_algo',1,'Password Algorithm',28);
UNLOCK TABLES;
SET FOREIGN_KEY_CHECKS=1;
27 changes: 23 additions & 4 deletions test/unittests/PasswordTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@ class PasswordTest extends TestCase
*/
private $_factory;

private $_configInfo = [0 => ['65' => 'false']];
private $_configInfo = [
['65' => 'false'],
['133','2y',],
];

/**
* Setup
Expand Down Expand Up @@ -141,7 +144,14 @@ public function testContructorInvalidValues($invalidValue): void
*/
public function testWellFormedPassword(): void
{
$this->assertInstanceOf('Password', new \Password(self::VALID_PASSWORD));
$this->_configMock->method('getSetting')
->with('passwordAlgorithm')
->willReturn(PASSWORD_BCRYPT);

$this->assertInstanceOf(
'Password',
new \Password(self::VALID_PASSWORD, $this->_configMock)
);
}

/**
Expand All @@ -152,9 +162,18 @@ public function testWellFormedPassword(): void
*/
public function testToString(): void
{
$password = new \Password(self::VALID_PASSWORD);
// Configure the mock to return a valid password algorithm
$this->_configMock->method('getSetting')
->with('passwordAlgorithm')
->willReturn(PASSWORD_BCRYPT);

// Instantiate the Password object with the valid password and config
$password = new \Password(self::VALID_PASSWORD, $this->_configMock);

// Assert that the password can be verified with the hashed value
$this->assertTrue(
password_verify(self::VALID_PASSWORD, (string) $password)
password_verify(self::VALID_PASSWORD, (string) $password),
"Password string should correctly verify against the original value."
);
}
}
6 changes: 4 additions & 2 deletions tools/resetpassword.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@

// Don't echo the password being typed
`/bin/stty -echo`;
$newPass = trim(fgets(STDIN));
$newHash = password_hash($newPass, PASSWORD_DEFAULT);
$newPass = trim(fgets(STDIN));
$config =& \NDB_Config::singleton();
$password_algo = $config->getSetting("passwordAlgorithm");
$newHash = password_hash($newPass, $password_algo);
`/bin/stty echo`;

;
Expand Down
Loading