From d9ba1cdd44a7eec9e7f4d429087c6ba838ad473e Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Tue, 17 Jul 2018 14:13:37 +0200 Subject: Do not check the IP address with session protection disabled This allows the user to stay logged in if his IP changes. Fixes #1106 --- application/security/LoginManager.php | 3 +++ 1 file changed, 3 insertions(+) (limited to 'application/security/LoginManager.php') diff --git a/application/security/LoginManager.php b/application/security/LoginManager.php index d6784d6d..5a58926d 100644 --- a/application/security/LoginManager.php +++ b/application/security/LoginManager.php @@ -58,6 +58,9 @@ class LoginManager */ public function generateStaySignedInToken($clientIpAddress) { + if ($this->configManager->get('security.session_protection_disabled') === true) { + $clientIpAddress = ''; + } $this->staySignedInToken = sha1( $this->configManager->get('credentials.hash') . $clientIpAddress -- cgit v1.2.3 From b49a04f796b9ad8533c53fee541132a4c2214909 Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Sat, 9 Feb 2019 16:44:48 +0100 Subject: Rewrite IP ban management This adds a dedicated manager class to handle all ban interactions, which is instantiated and handled by LoginManager. IPs are now stored in the same format as the datastore, through FileUtils. Fixes #1032 #587 --- application/security/LoginManager.php | 100 +++++----------------------------- 1 file changed, 14 insertions(+), 86 deletions(-) (limited to 'application/security/LoginManager.php') diff --git a/application/security/LoginManager.php b/application/security/LoginManager.php index 1ff3d0be..0b0ce0b1 100644 --- a/application/security/LoginManager.php +++ b/application/security/LoginManager.php @@ -20,8 +20,8 @@ class LoginManager /** @var SessionManager Session Manager instance **/ protected $sessionManager = null; - /** @var string Path to the file containing IP bans */ - protected $banFile = ''; + /** @var BanManager Ban Manager instance **/ + protected $banManager; /** @var bool Whether the user is logged in **/ protected $isLoggedIn = false; @@ -35,17 +35,21 @@ class LoginManager /** * Constructor * - * @param array $globals The $GLOBALS array (reference) * @param ConfigManager $configManager Configuration Manager instance * @param SessionManager $sessionManager SessionManager instance */ - public function __construct(& $globals, $configManager, $sessionManager) + public function __construct($configManager, $sessionManager) { - $this->globals = &$globals; $this->configManager = $configManager; $this->sessionManager = $sessionManager; - $this->banFile = $this->configManager->get('resource.ban_file', 'data/ipbans.php'); - $this->readBanFile(); + $this->banManager = new BanManager( + $this->configManager->get('security.trusted_proxies', []), + $this->configManager->get('security.ban_after'), + $this->configManager->get('security.ban_duration'), + $this->configManager->get('resource.ban_file', 'data/ipbans.php'), + $this->configManager->get('resource.log') + ); + if ($this->configManager->get('security.open_shaarli') === true) { $this->openShaarli = true; } @@ -157,31 +161,6 @@ class LoginManager return true; } - /** - * Read a file containing banned IPs - */ - protected function readBanFile() - { - if (! file_exists($this->banFile)) { - return; - } - include $this->banFile; - } - - /** - * Write the banned IPs to a file - */ - protected function writeBanFile() - { - if (! array_key_exists('IPBANS', $this->globals)) { - return; - } - file_put_contents( - $this->banFile, - "globals['IPBANS'], true) . ";\n?>" - ); - } - /** * Handle a failed login and ban the IP after too many failed attempts * @@ -189,34 +168,7 @@ class LoginManager */ public function handleFailedLogin($server) { - $ip = $server['REMOTE_ADDR']; - $trusted = $this->configManager->get('security.trusted_proxies', []); - - if (in_array($ip, $trusted)) { - $ip = getIpAddressFromProxy($server, $trusted); - if (! $ip) { - // the IP is behind a trusted forward proxy, but is not forwarded - // in the HTTP headers, so we do nothing - return; - } - } - - // increment the fail count for this IP - if (isset($this->globals['IPBANS']['FAILURES'][$ip])) { - $this->globals['IPBANS']['FAILURES'][$ip]++; - } else { - $this->globals['IPBANS']['FAILURES'][$ip] = 1; - } - - if ($this->globals['IPBANS']['FAILURES'][$ip] >= $this->configManager->get('security.ban_after')) { - $this->globals['IPBANS']['BANS'][$ip] = time() + $this->configManager->get('security.ban_duration', 1800); - logm( - $this->configManager->get('resource.log'), - $server['REMOTE_ADDR'], - 'IP address banned from login' - ); - } - $this->writeBanFile(); + $this->banManager->handleFailedAttempt($server); } /** @@ -226,13 +178,7 @@ class LoginManager */ public function handleSuccessfulLogin($server) { - $ip = $server['REMOTE_ADDR']; - // FIXME unban when behind a trusted proxy? - - unset($this->globals['IPBANS']['FAILURES'][$ip]); - unset($this->globals['IPBANS']['BANS'][$ip]); - - $this->writeBanFile(); + $this->banManager->clearFailures($server); } /** @@ -244,24 +190,6 @@ class LoginManager */ public function canLogin($server) { - $ip = $server['REMOTE_ADDR']; - - if (! isset($this->globals['IPBANS']['BANS'][$ip])) { - // the user is not banned - return true; - } - - if ($this->globals['IPBANS']['BANS'][$ip] > time()) { - // the user is still banned - return false; - } - - // the ban has expired, the user can attempt to log in again - logm($this->configManager->get('resource.log'), $server['REMOTE_ADDR'], 'Ban lifted.'); - unset($this->globals['IPBANS']['FAILURES'][$ip]); - unset($this->globals['IPBANS']['BANS'][$ip]); - - $this->writeBanFile(); - return true; + return ! $this->banManager->isBanned($server); } } -- cgit v1.2.3