Skip to content

Commit

Permalink
Merge branch 'pu/ps/rt225083' into '2024.11'
Browse files Browse the repository at this point in the history
feature(Tinebase/Controller): login failures by client

See merge request tine20/tine20!4668
  • Loading branch information
pschuele committed Jan 29, 2024
2 parents 382f84f + 19325ce commit d24b0f6
Show file tree
Hide file tree
Showing 9 changed files with 107 additions and 23 deletions.
5 changes: 4 additions & 1 deletion tests/tine20/Admin/Frontend/JsonTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1246,7 +1246,10 @@ public function testBlockedUserGroupSave()
// deactivate user
$userArray = $this->_createTestUser();
$userArray['lastLoginFailure'] = Tinebase_DateTime::now()->toString();
$userArray['loginFailures'] = 10;

$accessLog = Tinebase_User::getAccessLog();
$clientType = $accessLog && $accessLog['clienttype'] ? $accessLog['clienttype'] : 'Unknown';
$userArray['loginFailures']= json_encode([$clientType => 10]);

$savedGroup = $this->_saveGroup($userArray);

Expand Down
6 changes: 5 additions & 1 deletion tine20/RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@ TINE RELEASENOTES
=====================
Release: TBA (2024.11)
Last change: 2023-12-20
Last change: 2023-11-22

# GENERAL CHANGES (Administrative)

## Support for MySQL 5.7 dropped
## Support for mariadb 10.2 / 10.3 dropped
## New: mysql 8.0 + maria 10.4 - 10.6

# GENERAL CHANGES (User Interface)

# ADMIN / OPERATION
Expand Down
4 changes: 3 additions & 1 deletion tine20/Tinebase/AccessLog.php
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,9 @@ public function getAccessLogEntry($loginName, Zend_Auth_Result $authResult, Tine
'login_name' => mb_substr($authResult->getIdentity(), 0, 63),
'user_agent' => mb_substr($userAgent, 0, 254),
), true);


Tinebase_User::setAccessLog($accessLog);

return $accessLog;
}

Expand Down
11 changes: 8 additions & 3 deletions tine20/Tinebase/Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ protected function _getLoginUser($_username, Tinebase_Model_AccessLog $_accessLo
Tinebase_Exception::log($e);
}
}

$user = $accountsController->getFullUserByLoginName($_username);

$_accessLog->account_id = $user->getId();
Expand Down Expand Up @@ -469,18 +469,23 @@ protected function _loginFailed(Zend_Auth_Result $authResult, Tinebase_Model_Acc
}

$loglevel = Zend_Log::INFO;

$loginFailureCount = null;
$clientType = $accessLog->clienttype ?? 'Unknown';

if (null !== $user) {
$accessLog->account_id = $user->getId();
$loginFailureCount = $user['loginFailures'][$clientType] ?? 0 ;
$warnLoginFailures = Tinebase_Config::getInstance()->get(Tinebase_Config::WARN_LOGIN_FAILURES, 4);
if ($user->loginFailures >= $warnLoginFailures) {
if ($loginFailureCount >= $warnLoginFailures) {
$loglevel = Zend_Log::WARN;
}
}

if (Tinebase_Core::isLogLevel($loglevel)) Tinebase_Core::getLogger()->log(
__METHOD__ . '::' . __LINE__
. " Login with username {$accessLog->login_name} from {$accessLog->ip} failed ({$accessLog->result})!"
. ($user ? ' Auth failure count: ' . $user->loginFailures : ''),
. ($loginFailureCount ? " Auth failure count for client type: $clientType" . $loginFailureCount : ''),
$loglevel);
if (Tinebase_Core::isLogLevel(Zend_Log::DEBUG)) Tinebase_Core::getLogger()->debug(
__METHOD__ . '::' . __LINE__ . ' Auth result messages: ' . print_r($authResult->getMessages(), TRUE));
Expand Down
8 changes: 5 additions & 3 deletions tine20/Tinebase/Model/FullUser.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
* @property string $configuration
* @property array $groups list of group memberships
* @property Tinebase_DateTime $lastLoginFailure time of last login failure
* @property int $loginFailures number of login failures
* @property string $loginFailures login failures by client type
* @property string $visibility displayed/hidden in/from addressbook
* @property string $type
* @property Tinebase_Model_EmailUser $emailUser
Expand Down Expand Up @@ -185,8 +185,10 @@ class Tinebase_Model_FullUser extends Tinebase_Model_User
'validators' => [Zend_Filter_Input::ALLOW_EMPTY => true],
],
'loginFailures' => [
'type' => 'integer',
'validators' => [Zend_Filter_Input::ALLOW_EMPTY => true],
self::TYPE => self::TYPE_JSON,
self::VALIDATORS => [
Zend_Filter_Input::ALLOW_EMPTY => true,
],
],
'sambaSAM' => [
'type' => 'string',
Expand Down
31 changes: 30 additions & 1 deletion tine20/Tinebase/Setup/Update/17.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class Tinebase_Setup_Update_17 extends Setup_Update_Abstract
const RELEASE017_UPDATE002 = __CLASS__ . '::update002';
const RELEASE017_UPDATE003 = __CLASS__ . '::update003';
const RELEASE017_UPDATE004 = __CLASS__ . '::update004';
const RELEASE017_UPDATE005 = __CLASS__ . '::update005';

static protected $_allUpdates = [
self::PRIO_TINEBASE_BEFORE_STRUCT => [
Expand All @@ -32,6 +33,14 @@ class Tinebase_Setup_Update_17 extends Setup_Update_Abstract
self::CLASS_CONST => self::class,
self::FUNCTION_CONST => 'update002',
],
self::RELEASE017_UPDATE003 => [
self::CLASS_CONST => self::class,
self::FUNCTION_CONST => 'update003',
],
self::RELEASE017_UPDATE005 => [
self::CLASS_CONST => self::class,
self::FUNCTION_CONST => 'update005',
],
],
self::PRIO_TINEBASE_STRUCTURE => [
self::RELEASE017_UPDATE003 => [
Expand Down Expand Up @@ -80,7 +89,7 @@ public function update002()
public function update003()
{
Tinebase_TransactionManager::getInstance()->rollBack();

Setup_SchemaTool::updateSchema([
Tinebase_Model_NumberableConfig::class,
]);
Expand Down Expand Up @@ -161,4 +170,24 @@ public function update004()

$this->addApplicationUpdate(Tinebase_Config::APP_NAME, '17.4', self::RELEASE017_UPDATE004);
}

public function update005()
{
Tinebase_TransactionManager::getInstance()->rollBack();
if ($this->getTableVersion('accounts') < 20) {
$declaration = new Setup_Backend_Schema_Field_Xml('
<field>
<name>login_failures</name>
<type>text</type>
<length>4000</length>
</field>
');
$this->_backend->alterCol('accounts', $declaration);
$this->setTableVersion('accounts', 20);
}

Tinebase_Core::getDb()->query('UPDATE ' . SQL_TABLE_PREFIX . 'accounts SET login_failures = ' .
'JSON_OBJECT("JSON-RPC", CAST(login_failures AS INTEGER)) WHERE login_failures IS NOT NULL');
$this->addApplicationUpdate(Tinebase_Config::APP_NAME, '17.5', self::RELEASE017_UPDATE005);
}
}
8 changes: 4 additions & 4 deletions tine20/Tinebase/Setup/setup.xml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<application>
<name>Tinebase</name>
<version>17.4</version>
<version>17.5</version>
<order>0</order>
<minimumRequiredVersion>12.28</minimumRequiredVersion>
<tables>
Expand Down Expand Up @@ -233,7 +233,7 @@
</table>
<table>
<name>accounts</name>
<version>19</version>
<version>20</version>
<declaration>
<field>
<name>id</name>
Expand Down Expand Up @@ -313,8 +313,8 @@
</field>
<field>
<name>login_failures</name>
<type>integer</type>
<default>0</default>
<type>text</type>
<length>4000</length>
</field>
<field>
<name>last_login_failure_at</name>
Expand Down
12 changes: 12 additions & 0 deletions tine20/Tinebase/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ private function __clone() {}
* @var array|null
*/
private static $_backendConfiguration;

private static ?Tinebase_Model_AccessLog $_accessLog = null;

/**
* Holds the backend configuration options.
Expand Down Expand Up @@ -329,6 +331,16 @@ public static function setBackendType($backendType)
self::$_backendType = $newBackendType;
}

public static function setAccessLog(Tinebase_Model_AccessLog $accessLog)
{
self::$_accessLog = $accessLog;
}

public static function getAccessLog(): ?Tinebase_Model_AccessLog
{
return self::$_accessLog;
}

/**
* Setter for {@see $_backendConfiguration}
*
Expand Down
45 changes: 36 additions & 9 deletions tine20/Tinebase/User/Sql.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class Tinebase_User_Sql extends Tinebase_User_Abstract
* @var Tinebase_Backend_Sql_Command_Interface
*/
protected $_dbCommand;

/**
* the constructor
*
Expand Down Expand Up @@ -408,16 +408,19 @@ public function getUsersByPrimaryGroup($groupId)
*/
protected function _getUserSelectObject(bool $_getDeleted = false)
{
$accessLog = Tinebase_User::getAccessLog();
$clientType = $accessLog && $accessLog['clienttype'] ? $accessLog['clienttype'] : 'Unknown';
$command = 'JSON_VALUE(' . $this->rowNameMapping['loginFailures'] . ', ' . $this->_db->quote("$.$clientType") . ')';
$interval = $this->_dbCommand->getDynamicInterval(
'SECOND',
'1',
'CASE WHEN ' . $this->_db->quoteIdentifier($this->rowNameMapping['loginFailures'])
. ' > 5 THEN 60 ELSE POWER(2, ' . $this->_db->quoteIdentifier($this->rowNameMapping['loginFailures']) . ') END');
'CASE WHEN ' . $command
. ' > 5 THEN 60 ELSE POWER(2, ' . $command . ') END');

$statusSQL = 'CASE WHEN ' . $this->_db->quoteIdentifier($this->rowNameMapping['accountStatus']) . ' = ' . $this->_db->quote('enabled') . ' THEN ('
. 'CASE WHEN '.$this->_dbCommand->setDate('NOW()') .' > ' . $this->_db->quoteIdentifier($this->rowNameMapping['accountExpires'])
. ' THEN ' . $this->_db->quote('expired')
. ' WHEN ( ' . $this->_db->quoteIdentifier($this->rowNameMapping['loginFailures']) . ' > 0 AND '
. ' WHEN ( ' . $command . ' > 0 AND '
. $this->_db->quoteIdentifier($this->rowNameMapping['lastLoginFailure']) . ' + ' . $interval . ' > NOW()) THEN ' . $this->_db->quote('blocked')
. ' ELSE ' . $this->_db->quote('enabled') . ' END)'
. ' WHEN ' . $this->_db->quoteIdentifier($this->rowNameMapping['accountStatus']) . ' = ' . $this->_db->quote('expired')
Expand Down Expand Up @@ -653,7 +656,7 @@ public function setStatus($_accountId, $_status)
$accountData['seq'] = $oldUser->seq + 1;
switch($_status) {
case Tinebase_Model_User::ACCOUNT_STATUS_ENABLED:
$accountData[$this->rowNameMapping['loginFailures']] = 0;
$accountData[$this->rowNameMapping['loginFailures']] = $this->resetLoginFailureCount();
$accountData[$this->rowNameMapping['accountExpires']] = null;
$accountData['status'] = $_status;
break;
Expand Down Expand Up @@ -748,15 +751,14 @@ public function setLastLoginFailure($_loginName)

$values = array(
'last_login_failure_at' => Tinebase_DateTime::now()->get(Tinebase_Record_Abstract::ISO8601LONG),
'login_failures' => new Zend_Db_Expr($this->_db->quoteIdentifier('login_failures') . ' + 1')
'login_failures' => $this->addLoginFailureCount(),
);

$where = array(
$this->_db->quoteInto($this->_db->quoteIdentifier('id') . ' = ?', $user->getId())
);

$this->_db->update(SQL_TABLE_PREFIX . 'accounts', $values, $where);

return $user;
}

Expand All @@ -775,7 +777,7 @@ public function setLoginTime($_accountId, $_ipAddress)

$accountData['last_login_from'] = $_ipAddress;
$accountData['last_login'] = Tinebase_DateTime::now()->get(Tinebase_Record_Abstract::ISO8601LONG);
$accountData['login_failures'] = 0;
$accountData['login_failures'] = $this->resetLoginFailureCount();

$where = array(
$this->_db->quoteInto($this->_db->quoteIdentifier('id') . ' = ?', $accountId)
Expand Down Expand Up @@ -1070,7 +1072,7 @@ public function updateUserInSqlBackend(Tinebase_Model_FullUser $_user, bool $_ge
$accountData[$this->rowNameMapping['accountStatus']] = $_user->accountStatus;

if ($oldUser->accountStatus === Tinebase_User::STATUS_BLOCKED) {
$accountData[$this->rowNameMapping['loginFailures']] = 0;
$accountData[$this->rowNameMapping['loginFailures']] = $this->resetLoginFailureCount();
} elseif ($oldUser->accountStatus === Tinebase_User::STATUS_EXPIRED) {
$accountData[$this->rowNameMapping['accountExpires']] = null;
}
Expand Down Expand Up @@ -1697,4 +1699,29 @@ public function getUsersWithoutPw(): array
$stmt = $select->query();
return $stmt->fetchAll(Zend_Db::FETCH_NUM);
}

public function addLoginFailureCount(): Zend_Db_Expr
{
$accessLog = Tinebase_User::getAccessLog();
$clientType = $accessLog && $accessLog['clienttype'] ? $accessLog['clienttype'] : 'Unknown';
$field = $this->rowNameMapping['loginFailures'];
$valueCommand = "COALESCE(JSON_VALUE($field," . $this->_db->quote("$.$clientType") . '), 0) + 1';
$loginCommand = "CASE WHEN JSON_VALID($field) " .
"THEN JSON_SET($field," . $this->_db->quote("$.$clientType") . ', ' . $valueCommand . ') ' .
'ELSE JSON_OBJECT(' . $this->_db->quote($clientType) . ', 1) ' .
'END';
return new Zend_Db_Expr($loginCommand);
}

public function resetLoginFailureCount(): Zend_Db_Expr
{
$accessLog = Tinebase_User::getAccessLog();
$clientType = $accessLog && $accessLog['clienttype'] ? $accessLog['clienttype'] : 'Unknown';
$field = $this->rowNameMapping['loginFailures'];
$loginCommand = "CASE WHEN JSON_VALID($field) " .
"THEN JSON_SET($field," . $this->_db->quote("$.$clientType") . ', 0) ' .
'ELSE JSON_OBJECT(' . $this->_db->quote($clientType) . ', 0) ' .
'END';
return new Zend_Db_Expr($loginCommand);
}
}

0 comments on commit d24b0f6

Please sign in to comment.