From 31d0cdb8ad67746863b98626cdf1624bc3c91df0 Mon Sep 17 00:00:00 2001 From: Clemens Schwaighofer Date: Wed, 22 Jun 2022 19:38:03 +0900 Subject: [PATCH] Fix revalidate after flow in ACL\Login After revalidate time was reached, it was never reset because it used the original loginUserId set date. A new column has been added that gets reset every time the user logs in with username and password if a loginUserId is set in the database --- 4dev/database/database_create_data.sql | 2 + .../edit_user_set_login_user_id_set_date.sql | 2 + 4dev/database/table/edit_user.sql | 2 + .../20220617-edit_user_login_user_id_add.sql | 3 + 4dev/tests/CoreLibsACLLoginTest.php | 67 ++++++++++--------- .../CoreLibsACLLogin_database_create_data.sql | 2 + www/lib/CoreLibs/ACL/Login.php | 15 ++++- 7 files changed, 58 insertions(+), 35 deletions(-) diff --git a/4dev/database/database_create_data.sql b/4dev/database/database_create_data.sql index a19959f6..3e810cca 100644 --- a/4dev/database/database_create_data.sql +++ b/4dev/database/database_create_data.sql @@ -597,6 +597,7 @@ CREATE TABLE edit_user ( -- _GET login id for direct login login_user_id VARCHAR UNIQUE, -- the login uid, at least 32 chars login_user_id_set_date TIMESTAMP WITHOUT TIME ZONE, -- when above uid was set + login_user_id_last_login TIMESTAMP WITHOUT TIME ZONE, -- when the last login was done with user name and password login_user_id_valid_from TIMESTAMP WITHOUT TIME ZONE, -- if set, from when the above uid is valid login_user_id_valid_until TIMESTAMP WITHOUT TIME ZONE, -- if set, until when the above uid is valid login_user_id_revalidate_after INTERVAL, -- user must login to revalidated login id after set days, 0 for forever @@ -630,6 +631,7 @@ COMMENT ON COLUMN edit_user.password_reset_time IS 'When the password reset was COMMENT ON COLUMN edit_user.password_reset_uid IS 'Password reset page uid, one time, invalid after reset successful or time out'; COMMENT ON COLUMN edit_user.login_user_id IS 'Min 32 character UID to be used to login without password. Via GET/POST parameter'; COMMENT ON COLUMN edit_user.login_user_id_set_date IS 'login id was set at what date'; +COMMENT ON COLUMN edit_user.login_user_id_last_login IS 'set when username/password login is done'; COMMENT ON COLUMN edit_user.login_user_id_valid_from IS 'login id is valid from this date, >='; COMMENT ON COLUMN edit_user.login_user_id_valid_until IS 'login id is valid until this date, <='; COMMENT ON COLUMN edit_user.login_user_id_revalidate_after IS 'If set to a number greater 0 then user must login after given amount of days to revalidate, set to 0 for valid forver'; diff --git a/4dev/database/function/edit_user_set_login_user_id_set_date.sql b/4dev/database/function/edit_user_set_login_user_id_set_date.sql index 6f276287..f6b15f0a 100644 --- a/4dev/database/function/edit_user_set_login_user_id_set_date.sql +++ b/4dev/database/function/edit_user_set_login_user_id_set_date.sql @@ -15,8 +15,10 @@ BEGIN (OLD.login_user_id IS NULL OR NEW.login_user_id <> OLD.login_user_id) THEN NEW.login_user_id_set_date = NOW(); + NEW.login_user_id_revalidate_after = NOW(); ELSIF NEW.login_user_id IS NULL OR NEW.login_user_id = '' THEN NEW.login_user_id_set_date = NULL; + NEW.login_user_id_revalidate_after = NULL; END IF; RETURN NEW; END; diff --git a/4dev/database/table/edit_user.sql b/4dev/database/table/edit_user.sql index 9cdd7c8c..18703156 100644 --- a/4dev/database/table/edit_user.sql +++ b/4dev/database/table/edit_user.sql @@ -57,6 +57,7 @@ CREATE TABLE edit_user ( -- _GET login id for direct login login_user_id VARCHAR UNIQUE, -- the login uid, at least 32 chars login_user_id_set_date TIMESTAMP WITHOUT TIME ZONE, -- when above uid was set + login_user_id_last_login TIMESTAMP WITHOUT TIME ZONE, -- when the last login was done with user name and password login_user_id_valid_from TIMESTAMP WITHOUT TIME ZONE, -- if set, from when the above uid is valid login_user_id_valid_until TIMESTAMP WITHOUT TIME ZONE, -- if set, until when the above uid is valid login_user_id_revalidate_after INTERVAL, -- user must login to revalidated login id after set days, 0 for forever @@ -90,6 +91,7 @@ COMMENT ON COLUMN edit_user.password_reset_time IS 'When the password reset was COMMENT ON COLUMN edit_user.password_reset_uid IS 'Password reset page uid, one time, invalid after reset successful or time out'; COMMENT ON COLUMN edit_user.login_user_id IS 'Min 32 character UID to be used to login without password. Via GET/POST parameter'; COMMENT ON COLUMN edit_user.login_user_id_set_date IS 'login id was set at what date'; +COMMENT ON COLUMN edit_user.login_user_id_last_login IS 'set when username/password login is done'; COMMENT ON COLUMN edit_user.login_user_id_valid_from IS 'login id is valid from this date, >='; COMMENT ON COLUMN edit_user.login_user_id_valid_until IS 'login id is valid until this date, <='; COMMENT ON COLUMN edit_user.login_user_id_revalidate_after IS 'If set to a number greater 0 then user must login after given amount of days to revalidate, set to 0 for valid forver'; diff --git a/4dev/database/update/20220617-edit_user_login_user_id_add.sql b/4dev/database/update/20220617-edit_user_login_user_id_add.sql index 6117ce5a..801afb3d 100644 --- a/4dev/database/update/20220617-edit_user_login_user_id_add.sql +++ b/4dev/database/update/20220617-edit_user_login_user_id_add.sql @@ -6,6 +6,7 @@ ALTER TABLE edit_user ADD login_user_id VARCHAR UNIQUE; -- ALTER TABLE edit_user ADD CONSTRAINT edit_user_login_user_id_key UNIQUE (login_user_id); -- when above uid was set ALTER TABLE edit_user ADD login_user_id_set_date TIMESTAMP WITHOUT TIME ZONE; +ALTER TABLE edit_user ADD login_user_id_last_login TIMESTAMP WITHOUT TIME ZONE; -- if set, from/until when the above uid is valid ALTER TABLE edit_user ADD login_user_id_valid_from TIMESTAMP WITHOUT TIME ZONE; ALTER TABLE edit_user ADD login_user_id_valid_until TIMESTAMP WITHOUT TIME ZONE; @@ -33,8 +34,10 @@ BEGIN (OLD.login_user_id IS NULL OR NEW.login_user_id <> OLD.login_user_id) THEN NEW.login_user_id_set_date = NOW(); + NEW.login_user_id_revalidate_after = NOW(); ELSIF NEW.login_user_id IS NULL OR NEW.login_user_id = '' THEN NEW.login_user_id_set_date = NULL; + NEW.login_user_id_revalidate_after = NULL; END IF; RETURN NEW; END; diff --git a/4dev/tests/CoreLibsACLLoginTest.php b/4dev/tests/CoreLibsACLLoginTest.php index 0dd5e2ae..a35cd8d9 100644 --- a/4dev/tests/CoreLibsACLLoginTest.php +++ b/4dev/tests/CoreLibsACLLoginTest.php @@ -994,6 +994,7 @@ final class CoreLibsACLLoginTest extends TestCase . 'Login Failed - Login User ID is outside valid date range' ] ], + // TODO: Test that if we have n day check with login, that after login we can use parameter login again // // other: // login check edit access id of ID not null and not in array @@ -1196,7 +1197,6 @@ final class CoreLibsACLLoginTest extends TestCase if (!empty($mock_settings['test_login_user_id'])) { self::$db->dbExec( "UPDATE edit_user SET " - . "login_user_id_set_date = NOW(), " . "login_user_id = " . self::$db->dbEscapeLiteral($mock_settings['loginUserId']) . " " @@ -1207,10 +1207,10 @@ final class CoreLibsACLLoginTest extends TestCase if (!empty($mock_settings['test_login_user_id_revalidate_after'])) { $q_sub = ''; if ($mock_settings['test_login_user_id_revalidate_after'] == 'on') { - $q_sub = "login_user_id_set_date = NOW() - '1 day'::interval, " + $q_sub = "login_user_id_last_login = NOW() - '1 day'::interval, " . "login_user_id_revalidate_after = '1 day'::interval "; } else { - $q_sub = "login_user_id_set_date = NOW(), " + $q_sub = "login_user_id_last_login = NOW(), " . "login_user_id_revalidate_after = '6 day'::interval "; } self::$db->dbExec( @@ -1540,36 +1540,37 @@ final class CoreLibsACLLoginTest extends TestCase . self::$db->dbEscapeLiteral($post['login_username']) ); } - // if (!empty($mock_settings['test_login_user_id'])) { - // self::$db->dbExec( - // "UPDATE edit_user SET " - // . "login_user_id = NULL, " - // . "login_user_id_set_date = NULL " - // . "WHERE LOWER(username) = " - // . self::$db->dbEscapeLiteral($mock_settings['test_username']) - // ); - // } - // if (!empty($mock_settings['test_login_user_id_revalidate_after'])) { - // self::$db->dbExec( - // "UPDATE edit_user SET " - // . "login_user_id_set_date = NULL, " - // . "login_user_id_revalidate_after = NULL " - // . "WHERE LOWER(username) = " - // . self::$db->dbEscapeLiteral($mock_settings['test_username']) - // ); - // } - // if ( - // !empty($mock_settings['test_login_user_id_valid_from']) || - // !empty($mock_settings['test_login_user_id_valid_until']) - // ) { - // self::$db->dbExec( - // "UPDATE edit_user SET " - // . "login_user_id_valid_from = NULL, " - // . "login_user_id_valid_until = NULL " - // . "WHERE LOWER(username) = " - // . self::$db->dbEscapeLiteral($mock_settings['test_username']) - // ); - // } + if (!empty($mock_settings['test_login_user_id'])) { + self::$db->dbExec( + "UPDATE edit_user SET " + . "login_user_id = NULL, " + . "login_user_id_set_date = NULL, " + . "login_user_id_last_login = NULL " + . "WHERE LOWER(username) = " + . self::$db->dbEscapeLiteral($mock_settings['test_username']) + ); + } + if (!empty($mock_settings['test_login_user_id_revalidate_after'])) { + self::$db->dbExec( + "UPDATE edit_user SET " + . "login_user_id_last_login = NULL, " + . "login_user_id_revalidate_after = NULL " + . "WHERE LOWER(username) = " + . self::$db->dbEscapeLiteral($mock_settings['test_username']) + ); + } + if ( + !empty($mock_settings['test_login_user_id_valid_from']) || + !empty($mock_settings['test_login_user_id_valid_until']) + ) { + self::$db->dbExec( + "UPDATE edit_user SET " + . "login_user_id_valid_from = NULL, " + . "login_user_id_valid_until = NULL " + . "WHERE LOWER(username) = " + . self::$db->dbEscapeLiteral($mock_settings['test_username']) + ); + } } // - loginGetAclList (null, invalid,) diff --git a/4dev/tests/database/CoreLibsACLLogin_database_create_data.sql b/4dev/tests/database/CoreLibsACLLogin_database_create_data.sql index a19959f6..3e810cca 100644 --- a/4dev/tests/database/CoreLibsACLLogin_database_create_data.sql +++ b/4dev/tests/database/CoreLibsACLLogin_database_create_data.sql @@ -597,6 +597,7 @@ CREATE TABLE edit_user ( -- _GET login id for direct login login_user_id VARCHAR UNIQUE, -- the login uid, at least 32 chars login_user_id_set_date TIMESTAMP WITHOUT TIME ZONE, -- when above uid was set + login_user_id_last_login TIMESTAMP WITHOUT TIME ZONE, -- when the last login was done with user name and password login_user_id_valid_from TIMESTAMP WITHOUT TIME ZONE, -- if set, from when the above uid is valid login_user_id_valid_until TIMESTAMP WITHOUT TIME ZONE, -- if set, until when the above uid is valid login_user_id_revalidate_after INTERVAL, -- user must login to revalidated login id after set days, 0 for forever @@ -630,6 +631,7 @@ COMMENT ON COLUMN edit_user.password_reset_time IS 'When the password reset was COMMENT ON COLUMN edit_user.password_reset_uid IS 'Password reset page uid, one time, invalid after reset successful or time out'; COMMENT ON COLUMN edit_user.login_user_id IS 'Min 32 character UID to be used to login without password. Via GET/POST parameter'; COMMENT ON COLUMN edit_user.login_user_id_set_date IS 'login id was set at what date'; +COMMENT ON COLUMN edit_user.login_user_id_last_login IS 'set when username/password login is done'; COMMENT ON COLUMN edit_user.login_user_id_valid_from IS 'login id is valid from this date, >='; COMMENT ON COLUMN edit_user.login_user_id_valid_until IS 'login id is valid until this date, <='; COMMENT ON COLUMN edit_user.login_user_id_revalidate_after IS 'If set to a number greater 0 then user must login after given amount of days to revalidate, set to 0 for valid forver'; diff --git a/www/lib/CoreLibs/ACL/Login.php b/www/lib/CoreLibs/ACL/Login.php index b2ae2c7d..be69e722 100644 --- a/www/lib/CoreLibs/ACL/Login.php +++ b/www/lib/CoreLibs/ACL/Login.php @@ -540,6 +540,8 @@ class Login . "eu.debug, eu.db_debug, " // enabled . "eu.enabled, eu.deleted, " + // for checks only + . "eu.login_user_id, " // login id validation . "CASE WHEN (" . "(eu.login_user_id_valid_from IS NULL " @@ -550,7 +552,7 @@ class Login // check if user must login . "CASE WHEN eu.login_user_id_revalidate_after IS NOT NULL " . "AND eu.login_user_id_revalidate_after > '0 days'::INTERVAL " - . "AND (eu.login_user_id_set_date + eu.login_user_id_revalidate_after)::DATE " + . "AND (eu.login_user_id_last_login + eu.login_user_id_revalidate_after)::DATE " . "<= NOW()::DATE " . "THEN 1::INT ELSE 0::INT END AS login_user_id_revalidate, " . "eu.login_user_id_locked, " @@ -653,6 +655,15 @@ class Login // check if user is okay $this->loginCheckPermissions(); if ($this->login_error == 0) { + if ( + !empty($res['login_user_id']) && + !empty($this->username) && !empty($this->password) + ) { + $q = "UPDATE edit_user SET " + . "login_user_id_last_login = NOW() " + . "WHERE edit_user_id = " . $this->euid; + $this->db->dbExec($q); + } // now set all session vars and read page permissions $_SESSION['DEBUG_ALL'] = $this->db->dbBoolean($res['debug']); $_SESSION['DB_DEBUG'] = $this->db->dbBoolean($res['db_debug']); @@ -1891,7 +1902,7 @@ EOM; // check if user must login . "CASE WHEN eu.login_user_id_revalidate_after IS NOT NULL " . "AND eu.login_user_id_revalidate_after > '0 days'::INTERVAL " - . "AND eu.login_user_id_set_date + eu.login_user_id_revalidate_after <= NOW()::DATE " + . "AND eu.login_user_id_last_login + eu.login_user_id_revalidate_after <= NOW()::DATE " . "THEN 1::INT ELSE 0::INT END AS login_user_id_revalidate, " . "eu.login_user_id_locked " //