From 1d9d22cc7676fac14bfe4a5b67537ccfb4f1849d Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Wed, 26 Jun 2013 12:43:37 -0700 Subject: [PATCH 01/46] [ticket/11620] Add testable facade for sessions.php Since many functions in session.php have global variables inside the function, this exposes those functions through a testable facade that uses testable_factory's mock global variables to modify global variables used in the functions. This is using the facade pattern to provide a testable "front" to the functions in sessions.php. PHPBB3-11620 --- tests/session/testable_facade.php | 49 +++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 tests/session/testable_facade.php diff --git a/tests/session/testable_facade.php b/tests/session/testable_facade.php new file mode 100644 index 0000000000..d4c03ec8d5 --- /dev/null +++ b/tests/session/testable_facade.php @@ -0,0 +1,49 @@ +get_session($db); + global $request; + $request->overwrite('PHP_SELF', $php_self, phpbb_request_interface::SERVER); + $request->overwrite('QUERY_STRING', $query_string, phpbb_request_interface::SERVER); + $request->overwrite('REQUEST_URI', $request_uri, phpbb_request_interface::SERVER); + return phpbb_session::extract_current_page($root_path, phpbb_request_interface::SERVER); + } + + // [To be completed] + // public static function extract_current_hostname() {} + // public static function session_begin($update_session_page = true) {} + // public static function session_create($user_id = false, $set_admin = false, $persist_login = false, $viewonline = true) {} + // public static function session_kill($new_session = true) {} + // public static function session_gc() {} + // public static function set_cookie($name, $cookiedata, $cookietime) {} + // public static function check_ban($user_id = false, $user_ips = false, $user_email = false, $return = false) {} + // public static function check_dnsbl($mode, $ip = false) {} + // public static function set_login_key($user_id = false, $key = false, $user_ip = false) {} + // public static function reset_login_keys($user_id = false) {} + // public static function validate_referer($check_script_path = false) {} + // public static function update_session($session_data, $session_id = null) {} + // public static function unset_admin() {} +} + From 19a348e35990511186fc8e987b28b5f8b34c6650 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Wed, 26 Jun 2013 12:44:14 -0700 Subject: [PATCH 02/46] [ticket/11620] Add test for test_extract_current_page PHPBB3-11620 --- tests/session/class_functions_test.php | 49 ++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 tests/session/class_functions_test.php diff --git a/tests/session/class_functions_test.php b/tests/session/class_functions_test.php new file mode 100644 index 0000000000..c4ae5628f1 --- /dev/null +++ b/tests/session/class_functions_test.php @@ -0,0 +1,49 @@ +createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_empty.xml'); + } + + public function setUp() + { + $this->session_factory = new phpbb_session_testable_factory; + $this->db = $this->new_dbal(); + } + + function test_extract_current_page() + { + $expected_fields = array( + 'page_name' => "index.php", + 'script_path' => "/phpBB/" + ); + + $output = phpbb_session_testable_facade::extract_current_page( + $this->db, + $this->session_factory, + /* Root Path */ "./", + /* PHP Self */ "/phpBB/index.php", + /* Query String*/ "", + /* Request URI */ "/phpBB/" + ); + + foreach($expected_fields as $field => $expected_value) + { + $this->assertSame($expected_value, $output[$field]); + } + } +} From e1d957c3eed77ffb02eaf2c9422f09e71b12f938 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Wed, 26 Jun 2013 12:49:05 -0700 Subject: [PATCH 03/46] [ticket/11620] Remove accidental argument from testable_facade. PHPBB3-11620 --- tests/session/testable_facade.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/session/testable_facade.php b/tests/session/testable_facade.php index d4c03ec8d5..f85332c94a 100644 --- a/tests/session/testable_facade.php +++ b/tests/session/testable_facade.php @@ -28,7 +28,7 @@ class phpbb_session_testable_facade $request->overwrite('PHP_SELF', $php_self, phpbb_request_interface::SERVER); $request->overwrite('QUERY_STRING', $query_string, phpbb_request_interface::SERVER); $request->overwrite('REQUEST_URI', $request_uri, phpbb_request_interface::SERVER); - return phpbb_session::extract_current_page($root_path, phpbb_request_interface::SERVER); + return phpbb_session::extract_current_page($root_path); } // [To be completed] From 9f156e995468d322a9b90f188cb31df059b03d82 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Thu, 27 Jun 2013 15:56:19 -0700 Subject: [PATCH 04/46] [ticket/11620] Rename class_functions_test -> extract_page_test Renaming this file because it is going to contain a large data provider, so I'd rather split this test out. PHPBB3-11620 --- .../session/{class_functions_test.php => extract_page_test.php} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename tests/session/{class_functions_test.php => extract_page_test.php} (93%) diff --git a/tests/session/class_functions_test.php b/tests/session/extract_page_test.php similarity index 93% rename from tests/session/class_functions_test.php rename to tests/session/extract_page_test.php index c4ae5628f1..fca7763bc3 100644 --- a/tests/session/class_functions_test.php +++ b/tests/session/extract_page_test.php @@ -9,7 +9,7 @@ require_once dirname(__FILE__) . '/testable_facade.php'; -class phpbb_session_class_functions_test extends phpbb_database_test_case +class phpbb_session_extract_page_test extends phpbb_database_test_case { public $session_factory; public $db; From 7fd03abcab531d3e989753492ab0cce78549c1a3 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Thu, 27 Jun 2013 15:57:58 -0700 Subject: [PATCH 05/46] [ticket/11620] Add data provider to extract_page These test cases were taken from a live session, more test cases should be added to test specific functionality in this function. PHPBB3-11620 --- tests/session/extract_page_test.php | 105 ++++++++++++++++++++++++---- 1 file changed, 91 insertions(+), 14 deletions(-) diff --git a/tests/session/extract_page_test.php b/tests/session/extract_page_test.php index fca7763bc3..24fcb98164 100644 --- a/tests/session/extract_page_test.php +++ b/tests/session/extract_page_test.php @@ -14,6 +14,88 @@ class phpbb_session_extract_page_test extends phpbb_database_test_case public $session_factory; public $db; + static public function extract_current_page_data() + { + return array( + array( + './', + '/phpBB/index.php', + '', + '/phpBB/', + array( + 'page_name' => 'index.php', + 'page_dir' => '', + 'query_string' => '', + 'script_path' => '/phpBB/', + 'root_script_path' => '/phpBB/', + 'page' => 'index.php', + 'forum' => 0 + ) + ) , + array( + './', + '/phpBB/ucp.php', + 'mode=login', + '/phpBB/ucp.php?mode=login', + array( + 'page_name' => 'ucp.php', + 'page_dir' => '', + 'query_string' => 'mode=login', + 'script_path' => '/phpBB/', + 'root_script_path' => '/phpBB/', + 'page' => 'ucp.php?mode=login', + 'forum' => 0 + ) + ) , + array( + './', + '/phpBB/ucp.php', + 'mode=register', + '/phpBB/ucp.php?mode=register', + array( + 'page_name' => 'ucp.php', + 'page_dir' => '', + 'query_string' => 'mode=register', + 'script_path' => '/phpBB/', + 'root_script_path' => '/phpBB/', + 'page' => 'ucp.php?mode=register', + 'forum' => 0 + ) + ) , + array( + './', + '/phpBB/ucp.php', + 'mode=register', + '/phpBB/ucp.php?mode=register', + array( + 'page_name' => 'ucp.php', + 'page_dir' => '', + 'query_string' => 'mode=register', + 'script_path' => '/phpBB/', + 'root_script_path' => '/phpBB/', + 'page' => 'ucp.php?mode=register', + 'forum' => 0 + ) + ) , + array( + './../', + '/phpBB/adm/index.php', + 'sid=e7215d958cdd41a6fc13509bebe53e42', + '/phpBB/adm/index.php?sid=e7215d958cdd41a6fc13509bebe53e42', + array( + 'page_name' => 'index.php', + //'page_dir' => 'adm', + // ^-- Ignored because .. returns different directory in live vs testing + 'query_string' => '', + 'script_path' => '/phpBB/adm/', + 'root_script_path' => '/phpBB/', + //'page' => 'adm/index.php', + 'forum' => 0 + ) + ) + ); + } + public function getDataSet() { return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_empty.xml'); @@ -25,25 +107,20 @@ class phpbb_session_extract_page_test extends phpbb_database_test_case $this->db = $this->new_dbal(); } - function test_extract_current_page() + /** @dataProvider extract_current_page_data */ + function test_extract_current_page($root_path, $php_self, $query_string, $request_uri, $expected) { - $expected_fields = array( - 'page_name' => "index.php", - 'script_path' => "/phpBB/" - ); - $output = phpbb_session_testable_facade::extract_current_page( $this->db, $this->session_factory, - /* Root Path */ "./", - /* PHP Self */ "/phpBB/index.php", - /* Query String*/ "", - /* Request URI */ "/phpBB/" + $root_path, + $php_self, + $query_string, + $request_uri ); - foreach($expected_fields as $field => $expected_value) - { - $this->assertSame($expected_value, $output[$field]); - } + // This compares the result of the output. + // Any keys that are not in the expected array are overwritten by the output (aka not checked). + $this->assert_array_content_equals(array_merge($output, $expected), $output); } } From b8d9d7b79f98093a5870db2e3b60663ed5069d39 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Mon, 1 Jul 2013 00:11:44 -0700 Subject: [PATCH 06/46] [ticket/11620] Add extract_current_hostname Add a tests for extracting the current hostname from session. PHPBB3-11620 --- tests/session/extract_hostname_test.php | 58 +++++++++++++++++++++++++ tests/session/testable_facade.php | 11 ++++- 2 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 tests/session/extract_hostname_test.php diff --git a/tests/session/extract_hostname_test.php b/tests/session/extract_hostname_test.php new file mode 100644 index 0000000000..a126626ae3 --- /dev/null +++ b/tests/session/extract_hostname_test.php @@ -0,0 +1,58 @@ +createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_empty.xml'); + } + + public function setUp() + { + $this->session_factory = new phpbb_session_testable_factory; + $this->db = $this->new_dbal(); + } + + static public function extract_current_hostname_data() + { + return array ( + // [Input] $host, $server_name_config, $cookie_domain_config, [Expected] $output + // If host is ip use that ipv4 + array("127.0.0.1", "skipped.org", "skipped.org", "127.0.0.1"), + // If no host but server name matches cookie_domain use that + array("", "example.org", "example.org", "example.org"), + // If there is a host uri use that + array("example.org", False, False, "example.org"), + // "best approach" guessing + array("", "example.org", False, "example.org"), + array("", False, "127.0.0.1", "127.0.0.1"), + array("", False, False, php_uname('n')), + ); + } + + /** @dataProvider extract_current_hostname_data */ + function test_extract_current_hostname($host, $server_name_config, $cookie_domain_config, $expected) + { + $output = phpbb_session_testable_facade::extract_current_hostname( + $this->db, + $this->session_factory, + $host, + $server_name_config, + $cookie_domain_config + ); + + $this->assertEquals($expected, $output); + } +} diff --git a/tests/session/testable_facade.php b/tests/session/testable_facade.php index f85332c94a..a4a3d63ed4 100644 --- a/tests/session/testable_facade.php +++ b/tests/session/testable_facade.php @@ -31,8 +31,17 @@ class phpbb_session_testable_facade return phpbb_session::extract_current_page($root_path); } + public static function extract_current_hostname($db, $session_factory, $host, $server_name_config, $cookie_domain_config) { + $session = $session_factory->get_session($db); + global $config, $request; + $config['server_name'] = $server_name_config; + $config['cookie_domain'] = $cookie_domain_config; + $request->overwrite('SERVER_NAME', $host, phpbb_request_interface::SERVER); + $request->overwrite('Host', $host, phpbb_request_interface::SERVER); + // Note: There is a php_uname fallthrough in this method that this function doesn't override + return $session->extract_current_hostname(); + } // [To be completed] - // public static function extract_current_hostname() {} // public static function session_begin($update_session_page = true) {} // public static function session_create($user_id = false, $set_admin = false, $persist_login = false, $viewonline = true) {} // public static function session_kill($new_session = true) {} From 71fbe74edea4ad2618fbd9161e83ccaabafea9ac Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Mon, 1 Jul 2013 15:07:06 -0700 Subject: [PATCH 07/46] [ticket/11620] Fix quotes in extract_hostname_test PHPBB3-11620 --- tests/session/extract_hostname_test.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/session/extract_hostname_test.php b/tests/session/extract_hostname_test.php index a126626ae3..ae12a027f7 100644 --- a/tests/session/extract_hostname_test.php +++ b/tests/session/extract_hostname_test.php @@ -30,15 +30,15 @@ class phpbb_session_extract_hostname_test extends phpbb_database_test_case return array ( // [Input] $host, $server_name_config, $cookie_domain_config, [Expected] $output // If host is ip use that ipv4 - array("127.0.0.1", "skipped.org", "skipped.org", "127.0.0.1"), + array('127.0.0.1', 'skipped.org', 'skipped.org', '127.0.0.1'), // If no host but server name matches cookie_domain use that - array("", "example.org", "example.org", "example.org"), + array('', 'example.org', 'example.org', 'example.org'), // If there is a host uri use that - array("example.org", False, False, "example.org"), - // "best approach" guessing - array("", "example.org", False, "example.org"), - array("", False, "127.0.0.1", "127.0.0.1"), - array("", False, False, php_uname('n')), + array('example.org', false, false, 'example.org'), + // 'best approach' guessing + array('', 'example.org', false, 'example.org'), + array('', false, '127.0.0.1', '127.0.0.1'), + array('', false, false, php_uname('n')), ); } From e8facfc735ccc10fd106a169e2508b4c335a0e9e Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Mon, 1 Jul 2013 15:09:13 -0700 Subject: [PATCH 08/46] [ticket/11620] Add commas in extract_page_test PHPBB3-11620 --- tests/session/extract_page_test.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/session/extract_page_test.php b/tests/session/extract_page_test.php index 24fcb98164..94ed96c6d2 100644 --- a/tests/session/extract_page_test.php +++ b/tests/session/extract_page_test.php @@ -29,7 +29,7 @@ class phpbb_session_extract_page_test extends phpbb_database_test_case 'script_path' => '/phpBB/', 'root_script_path' => '/phpBB/', 'page' => 'index.php', - 'forum' => 0 + 'forum' => 0, ) ) , array( @@ -44,7 +44,7 @@ class phpbb_session_extract_page_test extends phpbb_database_test_case 'script_path' => '/phpBB/', 'root_script_path' => '/phpBB/', 'page' => 'ucp.php?mode=login', - 'forum' => 0 + 'forum' => 0, ) ) , array( @@ -59,7 +59,7 @@ class phpbb_session_extract_page_test extends phpbb_database_test_case 'script_path' => '/phpBB/', 'root_script_path' => '/phpBB/', 'page' => 'ucp.php?mode=register', - 'forum' => 0 + 'forum' => 0, ) ) , array( @@ -74,7 +74,7 @@ class phpbb_session_extract_page_test extends phpbb_database_test_case 'script_path' => '/phpBB/', 'root_script_path' => '/phpBB/', 'page' => 'ucp.php?mode=register', - 'forum' => 0 + 'forum' => 0, ) ) , array( @@ -90,7 +90,7 @@ class phpbb_session_extract_page_test extends phpbb_database_test_case 'script_path' => '/phpBB/adm/', 'root_script_path' => '/phpBB/', //'page' => 'adm/index.php', - 'forum' => 0 + 'forum' => 0, ) ) ); From 2f92c903e7f42978e01d09287e93d572f5e302c9 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Mon, 1 Jul 2013 15:16:22 -0700 Subject: [PATCH 09/46] [ticket/11620] Make testable_facade non-static, expand. Make the class functions of testable_facade no longer static methods, but a class based one and expand the methods to be filled in, in later commits. PHPBB3-11620 --- tests/session/extract_hostname_test.php | 5 +- tests/session/extract_page_test.php | 7 +- tests/session/testable_facade.php | 145 ++++++++++++++++++++---- 3 files changed, 132 insertions(+), 25 deletions(-) diff --git a/tests/session/extract_hostname_test.php b/tests/session/extract_hostname_test.php index ae12a027f7..6978b5286f 100644 --- a/tests/session/extract_hostname_test.php +++ b/tests/session/extract_hostname_test.php @@ -13,6 +13,7 @@ class phpbb_session_extract_hostname_test extends phpbb_database_test_case { public $session_factory; public $db; + public $session_facade; public function getDataSet() { @@ -23,6 +24,8 @@ class phpbb_session_extract_hostname_test extends phpbb_database_test_case { $this->session_factory = new phpbb_session_testable_factory; $this->db = $this->new_dbal(); + $this->session_facade = + new phpbb_session_testable_facade($this->db, $this->session_factory); } static public function extract_current_hostname_data() @@ -45,7 +48,7 @@ class phpbb_session_extract_hostname_test extends phpbb_database_test_case /** @dataProvider extract_current_hostname_data */ function test_extract_current_hostname($host, $server_name_config, $cookie_domain_config, $expected) { - $output = phpbb_session_testable_facade::extract_current_hostname( + $output = $this->session_facade->extract_current_hostname( $this->db, $this->session_factory, $host, diff --git a/tests/session/extract_page_test.php b/tests/session/extract_page_test.php index 94ed96c6d2..f8883dc8c9 100644 --- a/tests/session/extract_page_test.php +++ b/tests/session/extract_page_test.php @@ -13,6 +13,7 @@ class phpbb_session_extract_page_test extends phpbb_database_test_case { public $session_factory; public $db; + public $session_facade; static public function extract_current_page_data() { @@ -105,14 +106,14 @@ class phpbb_session_extract_page_test extends phpbb_database_test_case { $this->session_factory = new phpbb_session_testable_factory; $this->db = $this->new_dbal(); + $this->session_facade = + new phpbb_session_testable_facade($this->db, $this->session_factory); } /** @dataProvider extract_current_page_data */ function test_extract_current_page($root_path, $php_self, $query_string, $request_uri, $expected) { - $output = phpbb_session_testable_facade::extract_current_page( - $this->db, - $this->session_factory, + $output = $this->session_facade->extract_current_page( $root_path, $php_self, $query_string, diff --git a/tests/session/testable_facade.php b/tests/session/testable_facade.php index a4a3d63ed4..6d2c1408c9 100644 --- a/tests/session/testable_facade.php +++ b/tests/session/testable_facade.php @@ -14,16 +14,32 @@ require_once dirname(__FILE__) . '/../../phpBB/includes/session.php'; * This class exists to expose session.php's functions in a more testable way. * * Since many functions in session.php have global variables inside the function, - * this exposes those functions through a testable facade that uses testable_factory's - * mock global variables to modify global variables used in the functions. + * this exposes those functions through a testable facade that uses + * testable_factory's mock global variables to modify global variables used in + * the functions. * - * This is using the facade pattern to provide a testable "front" to the functions in sessions.php. + * This is using the facade pattern to provide a testable "front" to the + * functions in sessions.php. * */ class phpbb_session_testable_facade { - public static function extract_current_page($db, $session_factory, $root_path, $php_self, $query_string, $request_uri) { - $session_factory->get_session($db); + var $db; + var $session_factory; + + function __construct($db, $session_factory) { + $this->db = $db; + $this->session_factory = $session_factory; + } + + function extract_current_page ( + $root_path, + $php_self, + $query_string, + $request_uri + ) + { + $this->session_factory->get_session($this->db); global $request; $request->overwrite('PHP_SELF', $php_self, phpbb_request_interface::SERVER); $request->overwrite('QUERY_STRING', $query_string, phpbb_request_interface::SERVER); @@ -31,28 +47,115 @@ class phpbb_session_testable_facade return phpbb_session::extract_current_page($root_path); } - public static function extract_current_hostname($db, $session_factory, $host, $server_name_config, $cookie_domain_config) { - $session = $session_factory->get_session($db); + function extract_current_hostname ( + $host, + $server_name_config, + $cookie_domain_config + ) + { + $session = $this->session_factory->get_session($this->db); global $config, $request; $config['server_name'] = $server_name_config; $config['cookie_domain'] = $cookie_domain_config; $request->overwrite('SERVER_NAME', $host, phpbb_request_interface::SERVER); $request->overwrite('Host', $host, phpbb_request_interface::SERVER); - // Note: There is a php_uname fallthrough in this method that this function doesn't override + // Note: There is a php_uname fallthrough in this method + // that this function doesn't override return $session->extract_current_hostname(); } - // [To be completed] - // public static function session_begin($update_session_page = true) {} - // public static function session_create($user_id = false, $set_admin = false, $persist_login = false, $viewonline = true) {} - // public static function session_kill($new_session = true) {} - // public static function session_gc() {} - // public static function set_cookie($name, $cookiedata, $cookietime) {} - // public static function check_ban($user_id = false, $user_ips = false, $user_email = false, $return = false) {} - // public static function check_dnsbl($mode, $ip = false) {} - // public static function set_login_key($user_id = false, $key = false, $user_ip = false) {} - // public static function reset_login_keys($user_id = false) {} - // public static function validate_referer($check_script_path = false) {} - // public static function update_session($session_data, $session_id = null) {} - // public static function unset_admin() {} + + + /** This function has a *lot* of dependencies, so instead of naming them all, + * just ask for overrides */ + function session_begin ( + $update_session_page = true, + $config_overrides = array(), + $request_overrides = array() + ) + { + $session = $this->session_factory->get_session($this->db); + global $config, $request; + $request->merge(phpbb_request_interface::SERVER, $request_overrides); + $config = array_merge($config, $config_overrides); + return $session->session_begin($update_session_page); + } + + function session_create ( + $user_id = false, + $set_admin = false, + $persist_login = false, + $viewonline = true, + $config_overrides = array(), + $request_overrides = array(), + $bot_overrides = array(), + $uri_sid = "" + ) + { + $session = $this->session_factory->get_session($this->db); + global $config, $request, $cache; + $request->merge(phpbb_request_interface::SERVER, $request_overrides); + $config = array_merge($config, $config_overrides); + // Bots + $cache->merge_cache_data(array('_bots' => $bot_overrides)); + // Uri sid + $_GET['sid'] = $uri_sid; + return $session->session_create($user_id, $set_admin, $persist_login, $viewonline); + } + + function session_kill($new_session = true) + { + $session = $this->session_factory->get_session($this->db); + global $config, $request; + + } + + function session_gc() + { + $session = $this->session_factory->get_session($this->db); + global $config, $request; + + } + + function set_cookie($name, $cookiedata, $cookietime) + { + $session = $this->session_factory->get_session($this->db); + global $config, $request; + + } + + function check_ban($user_id = false, $user_ips = false, $user_email = false, $return = false) + { + $session = $this->session_factory->get_session($this->db); + global $config, $request; + + } + + function check_dnsbl($mode, $ip = false) + { + $session = $this->session_factory->get_session($this->db); + global $config, $request; + + } + + function set_login_key($user_id = false, $key = false, $user_ip = false) + { + $session = $this->session_factory->get_session($this->db); + global $config, $request; + + } + + function reset_login_keys($user_id = false) + { + $session = $this->session_factory->get_session($this->db); + global $config, $request; + + } + + function validate_referer($check_script_path = false) + { + $session = $this->session_factory->get_session($this->db); + global $config, $request; + return $session->validate_referer($check_script_path); + } } From 17890a308bbecd295c6ebb92d55fc39e68aae34e Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Mon, 1 Jul 2013 16:44:22 -0700 Subject: [PATCH 10/46] [ticket/11620] Add ipv6 test cases and remove extra arguments. PHPBB3-11620 --- tests/session/extract_hostname_test.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/session/extract_hostname_test.php b/tests/session/extract_hostname_test.php index 6978b5286f..cd71f82b17 100644 --- a/tests/session/extract_hostname_test.php +++ b/tests/session/extract_hostname_test.php @@ -32,8 +32,12 @@ class phpbb_session_extract_hostname_test extends phpbb_database_test_case { return array ( // [Input] $host, $server_name_config, $cookie_domain_config, [Expected] $output - // If host is ip use that ipv4 + // If host is ip use that + // ipv4 array('127.0.0.1', 'skipped.org', 'skipped.org', '127.0.0.1'), + // ipv6 + array('::1', 'skipped.org', 'skipped.org', ':'), + array('2002::3235:51f9', 'skipped.org', 'skipped.org', '2002::3235'), // If no host but server name matches cookie_domain use that array('', 'example.org', 'example.org', 'example.org'), // If there is a host uri use that @@ -49,8 +53,6 @@ class phpbb_session_extract_hostname_test extends phpbb_database_test_case function test_extract_current_hostname($host, $server_name_config, $cookie_domain_config, $expected) { $output = $this->session_facade->extract_current_hostname( - $this->db, - $this->session_factory, $host, $server_name_config, $cookie_domain_config From 30ebc03d143aaa7e3708b84b93b2e112351e70e5 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Wed, 3 Jul 2013 12:26:25 -0700 Subject: [PATCH 11/46] [ticket/11620] Remove unneeded functions from testable facade There are functions listed in testable facade that don't have a lot of dependencies, instead mostly just take the input and perform database functions on them. These can be tested without a testable facade function and so will be removed. PHPBB3-11620 --- tests/session/testable_facade.php | 49 ------------------------------- 1 file changed, 49 deletions(-) diff --git a/tests/session/testable_facade.php b/tests/session/testable_facade.php index 6d2c1408c9..02af73174f 100644 --- a/tests/session/testable_facade.php +++ b/tests/session/testable_facade.php @@ -102,55 +102,6 @@ class phpbb_session_testable_facade return $session->session_create($user_id, $set_admin, $persist_login, $viewonline); } - function session_kill($new_session = true) - { - $session = $this->session_factory->get_session($this->db); - global $config, $request; - - } - - function session_gc() - { - $session = $this->session_factory->get_session($this->db); - global $config, $request; - - } - - function set_cookie($name, $cookiedata, $cookietime) - { - $session = $this->session_factory->get_session($this->db); - global $config, $request; - - } - - function check_ban($user_id = false, $user_ips = false, $user_email = false, $return = false) - { - $session = $this->session_factory->get_session($this->db); - global $config, $request; - - } - - function check_dnsbl($mode, $ip = false) - { - $session = $this->session_factory->get_session($this->db); - global $config, $request; - - } - - function set_login_key($user_id = false, $key = false, $user_ip = false) - { - $session = $this->session_factory->get_session($this->db); - global $config, $request; - - } - - function reset_login_keys($user_id = false) - { - $session = $this->session_factory->get_session($this->db); - global $config, $request; - - } - function validate_referer($check_script_path = false) { $session = $this->session_factory->get_session($this->db); From 290533a14fbcf09caf40c88c30fc39b227f110f0 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Wed, 3 Jul 2013 12:28:37 -0700 Subject: [PATCH 12/46] [ticket/11620] Add validate_referrer test Add a test for the validate_referrer function. PHPBB3-11620 --- tests/session/testable_facade.php | 16 ++++- tests/session/validate_referrer_test.php | 80 ++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 tests/session/validate_referrer_test.php diff --git a/tests/session/testable_facade.php b/tests/session/testable_facade.php index 02af73174f..886c9b328a 100644 --- a/tests/session/testable_facade.php +++ b/tests/session/testable_facade.php @@ -102,10 +102,24 @@ class phpbb_session_testable_facade return $session->session_create($user_id, $set_admin, $persist_login, $viewonline); } - function validate_referer($check_script_path = false) + function validate_referer( + $check_script_path, + $referer, + $host, + $force_server_vars, + $server_port, + $server_name, + $root_script_path + ) { $session = $this->session_factory->get_session($this->db); global $config, $request; + $session->referer = $referer; + $session->page['root_script_path'] = $root_script_path; + $session->host = $host; + $config['force_server_vars'] = $force_server_vars; + $config['server_name'] = $server_name; + $request->overwrite('SERVER_PORT', $server_port, phpbb_request_interface::SERVER); return $session->validate_referer($check_script_path); } } diff --git a/tests/session/validate_referrer_test.php b/tests/session/validate_referrer_test.php new file mode 100644 index 0000000000..e5faf8a21f --- /dev/null +++ b/tests/session/validate_referrer_test.php @@ -0,0 +1,80 @@ +createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_empty.xml'); + } + + public function setUp() + { + $this->session_factory = new phpbb_session_testable_factory; + $this->db = $this->new_dbal(); + $this->session_facade = + new phpbb_session_testable_facade($this->db, $this->session_factory); + } + + static function referrer_inputs() { + $ex = "example.org"; + $alt = "example.com"; + return array( + // checkpath referrer host forcevars port servername rootpath pass? + // 0 Referrer or host wasn't collected, therefore should validate + array(false, "", $ex, false, 80, $ex, "", true), + array(false, $ex, "", false, 80, $ex, "", true), + // 2 Referrer doesn't match host or server_name + array(false, $alt, $ex, yes, 80, $ex, "", false), + // 3 Everything should check out + array(false, $ex, $ex, false, 80, $ex, "", true), + // 4 Check Script Path + array(true, $ex, $ex, false, 80, $ex, "", true), + array(true, "$ex/foo", $ex, false, 80, $ex, "/foo", true), + array(true, "$ex/bar", $ex, false, 80, $ex, "/foo", false), + // 7 Port (This is not checked unless path is checked) + array(true, "$ex:80/foo", "$ex:80", false, 80, "$ex:80", "/foo", true), + array(true, "$ex:80/bar", "$ex:80", false, 80, "$ex:80", "/foo", false), + array(true, "$ex:79/foo", "$ex:81", false, 81, "$ex:81", "/foo", false), + ); + } + + /** @dataProvider referrer_inputs */ + function test_failing_referrer ( + $check_script_path, + $referrer, + $host, + $force_server_vars, + $server_port, + $server_name, + $root_script_path, + $pass_or_fail + ) + { + //Referrer needs http:// because it's going to get stripped in function. + $referrer = ($referrer? 'http://'.$referrer : ''); + $this->assertEquals( + $pass_or_fail, + $this->session_facade->validate_referer( + $check_script_path, + $referrer, + $host, + $force_server_vars, + $server_port, + $server_name, + $root_script_path + ), "referrer should" . ($pass_or_fail? "" : "n't") . " be validated"); + } +} From ab1c42babf5fcbc07637940bd50ba4b2d7edca81 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Wed, 3 Jul 2013 12:41:40 -0700 Subject: [PATCH 13/46] [ticket/11620] Add indentation, change quote style. indentation is probably more important than 80 characters per line apparently. Single quotes instead of double per coding guidelines. PHPBB3-11620 --- tests/session/validate_referrer_test.php | 34 ++++++++++++------------ 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/session/validate_referrer_test.php b/tests/session/validate_referrer_test.php index e5faf8a21f..0636f04072 100644 --- a/tests/session/validate_referrer_test.php +++ b/tests/session/validate_referrer_test.php @@ -32,22 +32,22 @@ class phpbb_session_validate_referrer_test extends phpbb_database_test_case $ex = "example.org"; $alt = "example.com"; return array( - // checkpath referrer host forcevars port servername rootpath pass? - // 0 Referrer or host wasn't collected, therefore should validate - array(false, "", $ex, false, 80, $ex, "", true), - array(false, $ex, "", false, 80, $ex, "", true), - // 2 Referrer doesn't match host or server_name - array(false, $alt, $ex, yes, 80, $ex, "", false), - // 3 Everything should check out - array(false, $ex, $ex, false, 80, $ex, "", true), - // 4 Check Script Path - array(true, $ex, $ex, false, 80, $ex, "", true), - array(true, "$ex/foo", $ex, false, 80, $ex, "/foo", true), - array(true, "$ex/bar", $ex, false, 80, $ex, "/foo", false), - // 7 Port (This is not checked unless path is checked) - array(true, "$ex:80/foo", "$ex:80", false, 80, "$ex:80", "/foo", true), - array(true, "$ex:80/bar", "$ex:80", false, 80, "$ex:80", "/foo", false), - array(true, "$ex:79/foo", "$ex:81", false, 81, "$ex:81", "/foo", false), + // checkpath referrer host forcevars port servername rootpath pass? + // 0 Referrer or host wasn't collected, therefore should validate + array(false, '', $ex, false, 80, $ex, '', true), + array(false, $ex, '', false, 80, $ex, '', true), + // 2 Referrer doesn't match host or server_name + array(false, $alt, $ex, yes, 80, $ex, '', false), + // 3 Everything should check out + array(false, $ex, $ex, false, 80, $ex, '', true), + // 4 Check Script Path + array(true, $ex, $ex, false, 80, $ex, '', true), + array(true, "$ex/foo", $ex, false, 80, $ex, "/foo", true), + array(true, "$ex/bar", $ex, false, 80, $ex, "/foo", false), + // 7 Port (This is not checked unless path is checked) + array(true, "$ex:80/foo", "$ex:80", false, 80, "$ex:80", "/foo", true), + array(true, "$ex:80/bar", "$ex:80", false, 80, "$ex:80", "/foo", false), + array(true, "$ex:79/foo", "$ex:81", false, 81, "$ex:81", "/foo", false), ); } @@ -75,6 +75,6 @@ class phpbb_session_validate_referrer_test extends phpbb_database_test_case $server_port, $server_name, $root_script_path - ), "referrer should" . ($pass_or_fail? "" : "n't") . " be validated"); + ), "referrer should" . ($pass_or_fail? '' : "n't") . " be validated"); } } From 7ef95ce8ac8d189c65c3c3b27f0da9d1ac46877c Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Fri, 5 Jul 2013 11:52:40 -0700 Subject: [PATCH 14/46] [ticket/11620] Fix typo and confusingly named test PHPBB3-11620 --- tests/session/validate_referrer_test.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/session/validate_referrer_test.php b/tests/session/validate_referrer_test.php index 0636f04072..6774166132 100644 --- a/tests/session/validate_referrer_test.php +++ b/tests/session/validate_referrer_test.php @@ -37,7 +37,7 @@ class phpbb_session_validate_referrer_test extends phpbb_database_test_case array(false, '', $ex, false, 80, $ex, '', true), array(false, $ex, '', false, 80, $ex, '', true), // 2 Referrer doesn't match host or server_name - array(false, $alt, $ex, yes, 80, $ex, '', false), + array(false, $alt, $ex, false, 80, $ex, '', false), // 3 Everything should check out array(false, $ex, $ex, false, 80, $ex, '', true), // 4 Check Script Path @@ -52,7 +52,7 @@ class phpbb_session_validate_referrer_test extends phpbb_database_test_case } /** @dataProvider referrer_inputs */ - function test_failing_referrer ( + function test_referrer_inputs ( $check_script_path, $referrer, $host, From 521d35dd6eaf7a6cd8be1ebd8591e4b2b21fd99f Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Fri, 5 Jul 2013 13:10:27 -0700 Subject: [PATCH 15/46] [ticket/11620] Add create_test with test for bot detection Added a test for the creation of a session with a simple test for detecting whether a bot is present. PHPBB3-11620 --- tests/session/create_test.php | 86 +++++++++++++++++++++++++++++++ tests/session/testable_facade.php | 26 ++++++---- 2 files changed, 102 insertions(+), 10 deletions(-) create mode 100644 tests/session/create_test.php diff --git a/tests/session/create_test.php b/tests/session/create_test.php new file mode 100644 index 0000000000..773b833cf8 --- /dev/null +++ b/tests/session/create_test.php @@ -0,0 +1,86 @@ +createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_full.xml'); + } + + public function setUp() + { + $this->session_factory = new phpbb_session_testable_factory; + $this->db = $this->new_dbal(); + $this->session_facade = + new phpbb_session_testable_facade($this->db, $this->session_factory); + } + + static function bot($bot_agent, $user_id, $bot_ip) + { + return array(array( + 'bot_agent' => $bot_agent, + 'user_id' => $user_id, + 'bot_ip' => $bot_ip + )); + } + + static function create_inputs() { + return array( + array( + false, + false, + false, + false, + array(), + 'user agent', + '127.0.0.1', + self::bot('user agent', 13, '127.0.0.1'), + '', + function ($test, $output) { + $test->assertEquals($output->data['is_bot'], true, "should be a bot"); + } + ) + ); + } + + /** @dataProvider create_inputs */ + function test_session_create ( + $user_id = false, + $set_admin = false, + $persist_login = false, + $viewonline = true, + array $config_overrides = array(), + $user_agent = "", + $ip_address = "", + array $bot_overrides = array(), + $uri_sid = "", + $test_function + ) + { + $output = $this->session_facade->session_create( + $user_id, + $set_admin, + $persist_login, + $viewonline, + $config_overrides, + $user_agent, + $ip_address, + $bot_overrides, + $uri_sid + ); + $test_function($this, $output); + } +} diff --git a/tests/session/testable_facade.php b/tests/session/testable_facade.php index 886c9b328a..33175a293b 100644 --- a/tests/session/testable_facade.php +++ b/tests/session/testable_facade.php @@ -85,21 +85,27 @@ class phpbb_session_testable_facade $set_admin = false, $persist_login = false, $viewonline = true, - $config_overrides = array(), - $request_overrides = array(), - $bot_overrides = array(), + array $config_overrides = array(), + $user_agent, + $ip_address, + array $bot_overrides = array(), $uri_sid = "" ) { - $session = $this->session_factory->get_session($this->db); - global $config, $request, $cache; - $request->merge(phpbb_request_interface::SERVER, $request_overrides); - $config = array_merge($config, $config_overrides); + $this->session_factory->merge_config_data($config_overrides); // Bots - $cache->merge_cache_data(array('_bots' => $bot_overrides)); + $this->session_factory->merge_cache_data(array('_bots' => $bot_overrides)); + global $request; + $session = $this->session_factory->get_session($this->db); + $session->browser = $user_agent; + $session->ip = $ip_address; // Uri sid - $_GET['sid'] = $uri_sid; - return $session->session_create($user_id, $set_admin, $persist_login, $viewonline); + if ($uri_sid) + { + $_GET['sid'] = $uri_sid; + } + $session->session_create($user_id, $set_admin, $persist_login, $viewonline); + return $session; } function validate_referer( From 6f8187f7faadc543f3e43db278cd7239e8cf7ac7 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Fri, 5 Jul 2013 13:49:03 -0700 Subject: [PATCH 16/46] [ticket/11620] Reworked create_test without data provider PHPBB3-11620 --- tests/session/create_test.php | 53 ++++++++--------------------------- 1 file changed, 11 insertions(+), 42 deletions(-) diff --git a/tests/session/create_test.php b/tests/session/create_test.php index 773b833cf8..9d77a26f17 100644 --- a/tests/session/create_test.php +++ b/tests/session/create_test.php @@ -37,50 +37,19 @@ class phpbb_session_create_test extends phpbb_database_test_case )); } - static function create_inputs() { - return array( - array( - false, - false, - false, - false, - array(), - 'user agent', - '127.0.0.1', - self::bot('user agent', 13, '127.0.0.1'), - '', - function ($test, $output) { - $test->assertEquals($output->data['is_bot'], true, "should be a bot"); - } - ) - ); - } - - /** @dataProvider create_inputs */ - function test_session_create ( - $user_id = false, - $set_admin = false, - $persist_login = false, - $viewonline = true, - array $config_overrides = array(), - $user_agent = "", - $ip_address = "", - array $bot_overrides = array(), - $uri_sid = "", - $test_function - ) + function test_bot_session () { $output = $this->session_facade->session_create( - $user_id, - $set_admin, - $persist_login, - $viewonline, - $config_overrides, - $user_agent, - $ip_address, - $bot_overrides, - $uri_sid + false, + false, + false, + false, + array(), + 'user agent', + '127.0.0.1', + self::bot('user agent', 13, '127.0.0.1'), + '' ); - $test_function($this, $output); + $this->assertEquals($output->data['is_bot'], true, "should be a bot"); } } From 5cdcb689df37fd7cbaaa1b5475caa830e87be318 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Fri, 5 Jul 2013 14:49:30 -0700 Subject: [PATCH 17/46] [ticket/11620] Implemented a provider mock object. Due to an auth_refactor, there is a new dependency in session.php on phpbb_container and a provider. For purposes of testing, implemented a simple one. PHPBB3-11620 --- tests/mock/provider.php | 23 +++++++++++++++++++++++ tests/session/testable_factory.php | 12 +++++++++++- 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 tests/mock/provider.php diff --git a/tests/mock/provider.php b/tests/mock/provider.php new file mode 100644 index 0000000000..21ef2fc949 --- /dev/null +++ b/tests/mock/provider.php @@ -0,0 +1,23 @@ +request = new phpbb_mock_request( array(), @@ -83,6 +87,12 @@ class phpbb_session_testable_factory $cache = $this->cache = new phpbb_mock_cache($this->get_cache_data()); $SID = $_SID = null; + $phpbb_container = $this->container = new phpbb_mock_container_builder(); + $phpbb_container->set( + 'auth.provider.db', + new phpbb_provider() + ); + $session = new phpbb_mock_session_testable; return $session; } From a1168972ff4d6e5957da08c22437244c92f9696e Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Fri, 5 Jul 2013 15:00:05 -0700 Subject: [PATCH 18/46] [ticket/11620] Added validate_session to provider. PHPBB3-11620 --- tests/mock/provider.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/mock/provider.php b/tests/mock/provider.php index 21ef2fc949..1e7b4bc4ba 100644 --- a/tests/mock/provider.php +++ b/tests/mock/provider.php @@ -12,12 +12,19 @@ * sessions. */ class phpbb_provider { + function autologin() { return array(); } + function kill() { } + + function validate_session($data) + { + return true; + } } From 3999d7ec7cc0860d3f955db9088558f92d1ba497 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Mon, 8 Jul 2013 10:28:40 -0700 Subject: [PATCH 19/46] [ticket/11620] More mock provider methods The mock provider object now better matches the interface given for providers. PHPBB3-11620 --- tests/mock/provider.php | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/tests/mock/provider.php b/tests/mock/provider.php index 1e7b4bc4ba..4d0d6fc84c 100644 --- a/tests/mock/provider.php +++ b/tests/mock/provider.php @@ -10,21 +10,43 @@ /** * Mock provider class with basic functions to help test * sessions. + * + * See interface here: + * includes/auth/provider/interface.php */ class phpbb_provider { + function init() + { + return null; + } + + function login($username, $password) + { + return array( + 'status' => "", + 'error_msg' => "", + 'user_row' => "", + ); + } + function autologin() { return array(); } - function kill() + function acp($new) { - + return array(); } - function validate_session($data) + function logout($data, $new_session) { - return true; + return null; + } + + function validate_session($user) + { + return null; } } From cd1fe789d243e12330a049799818ed7b062ea347 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Mon, 8 Jul 2013 16:34:46 -0700 Subject: [PATCH 20/46] [ticket/11620] Minor changes to tests for coding standards PHPBB3-11620 --- tests/session/create_test.php | 6 +++--- tests/session/extract_page_test.php | 26 ++++++++++++------------ tests/session/testable_facade.php | 12 +++++++++-- tests/session/validate_referrer_test.php | 4 ++-- 4 files changed, 28 insertions(+), 20 deletions(-) diff --git a/tests/session/create_test.php b/tests/session/create_test.php index 9d77a26f17..4a7484321c 100644 --- a/tests/session/create_test.php +++ b/tests/session/create_test.php @@ -33,11 +33,11 @@ class phpbb_session_create_test extends phpbb_database_test_case return array(array( 'bot_agent' => $bot_agent, 'user_id' => $user_id, - 'bot_ip' => $bot_ip + 'bot_ip' => $bot_ip, )); } - function test_bot_session () + function test_bot_session() { $output = $this->session_facade->session_create( false, @@ -50,6 +50,6 @@ class phpbb_session_create_test extends phpbb_database_test_case self::bot('user agent', 13, '127.0.0.1'), '' ); - $this->assertEquals($output->data['is_bot'], true, "should be a bot"); + $this->assertEquals($output->data['is_bot'], true, 'should be a bot'); } } diff --git a/tests/session/extract_page_test.php b/tests/session/extract_page_test.php index f8883dc8c9..c17845526f 100644 --- a/tests/session/extract_page_test.php +++ b/tests/session/extract_page_test.php @@ -15,6 +15,19 @@ class phpbb_session_extract_page_test extends phpbb_database_test_case public $db; public $session_facade; + public function getDataSet() + { + return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_empty.xml'); + } + + public function setUp() + { + $this->session_factory = new phpbb_session_testable_factory; + $this->db = $this->new_dbal(); + $this->session_facade = + new phpbb_session_testable_facade($this->db, $this->session_factory); + } + static public function extract_current_page_data() { return array( @@ -97,19 +110,6 @@ class phpbb_session_extract_page_test extends phpbb_database_test_case ); } - public function getDataSet() - { - return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_empty.xml'); - } - - public function setUp() - { - $this->session_factory = new phpbb_session_testable_factory; - $this->db = $this->new_dbal(); - $this->session_facade = - new phpbb_session_testable_facade($this->db, $this->session_factory); - } - /** @dataProvider extract_current_page_data */ function test_extract_current_page($root_path, $php_self, $query_string, $request_uri, $expected) { diff --git a/tests/session/testable_facade.php b/tests/session/testable_facade.php index 33175a293b..d28201adc3 100644 --- a/tests/session/testable_facade.php +++ b/tests/session/testable_facade.php @@ -65,8 +65,16 @@ class phpbb_session_testable_facade } - /** This function has a *lot* of dependencies, so instead of naming them all, - * just ask for overrides */ + /** + * + * This function has a lot of dependencies, so instead of naming them all, + * just ask for overrides + * + * @param update_session_page Boolean of whether to set page of the session + * @param config_overrides An array of overrides for the global config object + * @param request_overrides An array of overrides for the global request object + * @return boolean False if the user is identified, otherwise true. + */ function session_begin ( $update_session_page = true, $config_overrides = array(), diff --git a/tests/session/validate_referrer_test.php b/tests/session/validate_referrer_test.php index 6774166132..1428187f27 100644 --- a/tests/session/validate_referrer_test.php +++ b/tests/session/validate_referrer_test.php @@ -63,8 +63,8 @@ class phpbb_session_validate_referrer_test extends phpbb_database_test_case $pass_or_fail ) { - //Referrer needs http:// because it's going to get stripped in function. - $referrer = ($referrer? 'http://'.$referrer : ''); + // Referrer needs http:// because it's going to get stripped in function. + $referrer = $referrer ? 'http://'.$referrer : ''; $this->assertEquals( $pass_or_fail, $this->session_facade->validate_referer( From f51721e905af68624df422889f92a069abd7b750 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Mon, 8 Jul 2013 16:38:53 -0700 Subject: [PATCH 21/46] [ticket/11620] Rename provider -> mock_auth_provider Rename the class and file name to better match what the class is mocking, as well as implement the interface of that class. PHPBB3-11620 --- tests/mock/{provider.php => auth_provider.php} | 3 ++- tests/session/testable_factory.php | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) rename tests/mock/{provider.php => auth_provider.php} (90%) diff --git a/tests/mock/provider.php b/tests/mock/auth_provider.php similarity index 90% rename from tests/mock/provider.php rename to tests/mock/auth_provider.php index 4d0d6fc84c..cca7865fa2 100644 --- a/tests/mock/provider.php +++ b/tests/mock/auth_provider.php @@ -14,7 +14,8 @@ * See interface here: * includes/auth/provider/interface.php */ -class phpbb_provider { +class phpbb_mock_auth_provider implements phpbb_auth_provider_interface +{ function init() { diff --git a/tests/session/testable_factory.php b/tests/session/testable_factory.php index d0d0dabbec..ace968eb43 100644 --- a/tests/session/testable_factory.php +++ b/tests/session/testable_factory.php @@ -8,7 +8,7 @@ */ require_once dirname(__FILE__) . '/../mock/container_builder.php'; -require_once dirname(__FILE__) . '/../mock/provider.php'; +require_once dirname(__FILE__) . '/../mock/auth_provider.php'; /** * This class exists to setup an instance of phpbb's session class for testing. @@ -90,7 +90,7 @@ class phpbb_session_testable_factory $phpbb_container = $this->container = new phpbb_mock_container_builder(); $phpbb_container->set( 'auth.provider.db', - new phpbb_provider() + new phpbb_mock_auth_provider() ); $session = new phpbb_mock_session_testable; From c96b0b1a47cef409fecc3bd30792e0907a869baa Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Tue, 9 Jul 2013 12:03:17 -0700 Subject: [PATCH 22/46] [ticket/11620] Removed unnecessary lines and whitespace PHPBB3-11620 --- tests/mock/auth_provider.php | 7 +------ tests/session/testable_facade.php | 7 +++---- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/tests/mock/auth_provider.php b/tests/mock/auth_provider.php index cca7865fa2..9674c573e3 100644 --- a/tests/mock/auth_provider.php +++ b/tests/mock/auth_provider.php @@ -8,15 +8,10 @@ */ /** - * Mock provider class with basic functions to help test - * sessions. - * - * See interface here: - * includes/auth/provider/interface.php + * Mock auth provider class with basic functions to help test sessions. */ class phpbb_mock_auth_provider implements phpbb_auth_provider_interface { - function init() { return null; diff --git a/tests/session/testable_facade.php b/tests/session/testable_facade.php index d28201adc3..b9f61b80cb 100644 --- a/tests/session/testable_facade.php +++ b/tests/session/testable_facade.php @@ -24,8 +24,8 @@ require_once dirname(__FILE__) . '/../../phpBB/includes/session.php'; */ class phpbb_session_testable_facade { - var $db; - var $session_factory; + protected $db; + protected $session_factory; function __construct($db, $session_factory) { $this->db = $db; @@ -59,12 +59,11 @@ class phpbb_session_testable_facade $config['cookie_domain'] = $cookie_domain_config; $request->overwrite('SERVER_NAME', $host, phpbb_request_interface::SERVER); $request->overwrite('Host', $host, phpbb_request_interface::SERVER); - // Note: There is a php_uname fallthrough in this method + // Note: There is a php_uname function used as a fallthrough // that this function doesn't override return $session->extract_current_hostname(); } - /** * * This function has a lot of dependencies, so instead of naming them all, From 62d7a0570071bb572fa60e502596d21eaee57376 Mon Sep 17 00:00:00 2001 From: asperous Date: Thu, 11 Jul 2013 14:58:41 -0700 Subject: [PATCH 23/46] [ticket/11620] Abstracted session setUp into a test_case class When defining a database test case with a setUp function, it is important to call the parent's setup function, because that is when the database is setup. PHPBB3-11620 --- tests/session/create_test.php | 16 ++--------- tests/session/extract_hostname_test.php | 16 ++--------- tests/session/extract_page_test.php | 16 ++--------- tests/session/validate_referrer_test.php | 16 ++--------- .../phpbb_session_test_case.php | 27 +++++++++++++++++++ 5 files changed, 35 insertions(+), 56 deletions(-) create mode 100644 tests/test_framework/phpbb_session_test_case.php diff --git a/tests/session/create_test.php b/tests/session/create_test.php index 4a7484321c..a8248ae62c 100644 --- a/tests/session/create_test.php +++ b/tests/session/create_test.php @@ -7,27 +7,15 @@ * */ -require_once dirname(__FILE__) . '/testable_facade.php'; +require_once dirname(__FILE__) . '/../test_framework/phpbb_session_test_case.php'; -class phpbb_session_create_test extends phpbb_database_test_case +class phpbb_session_create_test extends phpbb_session_test_case { - public $session_factory; - public $db; - public $session_facade; - public function getDataSet() { return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_full.xml'); } - public function setUp() - { - $this->session_factory = new phpbb_session_testable_factory; - $this->db = $this->new_dbal(); - $this->session_facade = - new phpbb_session_testable_facade($this->db, $this->session_factory); - } - static function bot($bot_agent, $user_id, $bot_ip) { return array(array( diff --git a/tests/session/extract_hostname_test.php b/tests/session/extract_hostname_test.php index cd71f82b17..5ff43cbb60 100644 --- a/tests/session/extract_hostname_test.php +++ b/tests/session/extract_hostname_test.php @@ -7,27 +7,15 @@ * */ -require_once dirname(__FILE__) . '/testable_facade.php'; +require_once dirname(__FILE__) . '/../test_framework/phpbb_session_test_case.php'; -class phpbb_session_extract_hostname_test extends phpbb_database_test_case +class phpbb_session_extract_hostname_test extends phpbb_session_test_case { - public $session_factory; - public $db; - public $session_facade; - public function getDataSet() { return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_empty.xml'); } - public function setUp() - { - $this->session_factory = new phpbb_session_testable_factory; - $this->db = $this->new_dbal(); - $this->session_facade = - new phpbb_session_testable_facade($this->db, $this->session_factory); - } - static public function extract_current_hostname_data() { return array ( diff --git a/tests/session/extract_page_test.php b/tests/session/extract_page_test.php index c17845526f..9346973bc4 100644 --- a/tests/session/extract_page_test.php +++ b/tests/session/extract_page_test.php @@ -7,27 +7,15 @@ * */ -require_once dirname(__FILE__) . '/testable_facade.php'; +require_once dirname(__FILE__) . '/../test_framework/phpbb_session_test_case.php'; -class phpbb_session_extract_page_test extends phpbb_database_test_case +class phpbb_session_extract_page_test extends phpbb_session_test_case { - public $session_factory; - public $db; - public $session_facade; - public function getDataSet() { return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_empty.xml'); } - public function setUp() - { - $this->session_factory = new phpbb_session_testable_factory; - $this->db = $this->new_dbal(); - $this->session_facade = - new phpbb_session_testable_facade($this->db, $this->session_factory); - } - static public function extract_current_page_data() { return array( diff --git a/tests/session/validate_referrer_test.php b/tests/session/validate_referrer_test.php index 1428187f27..f91ac5f1f9 100644 --- a/tests/session/validate_referrer_test.php +++ b/tests/session/validate_referrer_test.php @@ -7,27 +7,15 @@ * */ -require_once dirname(__FILE__) . '/testable_facade.php'; +require_once dirname(__FILE__) . '/../test_framework/phpbb_session_test_case.php'; -class phpbb_session_validate_referrer_test extends phpbb_database_test_case +class phpbb_session_validate_referrer_test extends phpbb_session_test_case { - public $session_factory; - public $db; - public $session_facade; - public function getDataSet() { return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_empty.xml'); } - public function setUp() - { - $this->session_factory = new phpbb_session_testable_factory; - $this->db = $this->new_dbal(); - $this->session_facade = - new phpbb_session_testable_facade($this->db, $this->session_factory); - } - static function referrer_inputs() { $ex = "example.org"; $alt = "example.com"; diff --git a/tests/test_framework/phpbb_session_test_case.php b/tests/test_framework/phpbb_session_test_case.php new file mode 100644 index 0000000000..6ff7d8e2ef --- /dev/null +++ b/tests/test_framework/phpbb_session_test_case.php @@ -0,0 +1,27 @@ +session_factory = new phpbb_session_testable_factory; + $this->db = $this->new_dbal(); + $this->session_facade = + new phpbb_session_testable_facade($this->db, $this->session_factory); + } +} From 87e65224d45c0571e90e00156abd165bd6bbe059 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Mon, 22 Jul 2013 11:07:17 -0700 Subject: [PATCH 24/46] [ticket/11620] Cherry-Pick merge tests from session-storage-cache PHPBB3-11620 --- tests/session/testable_facade.php | 16 +++++----- tests/session/unset_admin_test.php | 48 ++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 7 deletions(-) create mode 100644 tests/session/unset_admin_test.php diff --git a/tests/session/testable_facade.php b/tests/session/testable_facade.php index b9f61b80cb..1cb1c94b52 100644 --- a/tests/session/testable_facade.php +++ b/tests/session/testable_facade.php @@ -77,14 +77,16 @@ class phpbb_session_testable_facade function session_begin ( $update_session_page = true, $config_overrides = array(), - $request_overrides = array() + $request_overrides = array(), + $cookies_overrides = array() ) { + $this->session_factory->merge_config_data($config_overrides); + $this->session_factory->merge_server_data($request_overrides); + $this->session_factory->set_cookies($cookies_overrides); $session = $this->session_factory->get_session($this->db); - global $config, $request; - $request->merge(phpbb_request_interface::SERVER, $request_overrides); - $config = array_merge($config, $config_overrides); - return $session->session_begin($update_session_page); + $session->session_begin($update_session_page); + return $session; } function session_create ( @@ -93,8 +95,8 @@ class phpbb_session_testable_facade $persist_login = false, $viewonline = true, array $config_overrides = array(), - $user_agent, - $ip_address, + $user_agent = 'user agent', + $ip_address = '127.0.0.1', array $bot_overrides = array(), $uri_sid = "" ) diff --git a/tests/session/unset_admin_test.php b/tests/session/unset_admin_test.php new file mode 100644 index 0000000000..bbb5eb1439 --- /dev/null +++ b/tests/session/unset_admin_test.php @@ -0,0 +1,48 @@ +createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_full.xml'); + } + + function get_test_session() + { + return $this->session_facade->session_begin( + true, + // Config + array( + 'session_length' => time(), // need to do this to allow sessions started at time 0 + ), + // Server + array( + 'HTTP_USER_AGENT' => "user agent", + 'REMOTE_ADDR' => "127.0.0.1", + ), + // Cookies + array( + '_sid' => 'bar_session000000000000000000000', + '_u' => 4, + ) + ); + } + + public function test_unset_admin() + { + $session = $this->get_test_session(); + $this->assertEquals(1, $session->data['session_admin'], 'should be an admin before test starts'); + $session->unset_admin(); + $session = $this->get_test_session(); + $this->assertEquals(0, $session->data['session_admin'], 'should be not be an admin after unset_admin'); + } +} From f7da773c06534cd9b359bc5e6430469c2ff9a4bc Mon Sep 17 00:00:00 2001 From: asperous Date: Thu, 11 Jul 2013 20:23:12 -0700 Subject: [PATCH 25/46] [ticket/11620] Added manual key test PHPBB3-11620 --- tests/session/fixtures/sessions_key.xml | 44 +++++++++++++++++++++++++ tests/session/session_key_tests.php | 28 ++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 tests/session/fixtures/sessions_key.xml create mode 100644 tests/session/session_key_tests.php diff --git a/tests/session/fixtures/sessions_key.xml b/tests/session/fixtures/sessions_key.xml new file mode 100644 index 0000000000..246d284557 --- /dev/null +++ b/tests/session/fixtures/sessions_key.xml @@ -0,0 +1,44 @@ + + + + key_id + user_id + last_ip + last_login + + a87ff679a2f3e71d9181a67b7542122c + 4 + 127.0.0.1 + 0 + +
+ + session_id + session_user_id + session_ip + session_browser + + bar_session000000000000000000000 + 4 + 127.0.0.1 + user agent + 1 + +
+ + user_id + username_clean + user_permissions + user_sig + user_occ + user_interests + + 4 + bar + + + + + +
+ diff --git a/tests/session/session_key_tests.php b/tests/session/session_key_tests.php new file mode 100644 index 0000000000..382ed06a15 --- /dev/null +++ b/tests/session/session_key_tests.php @@ -0,0 +1,28 @@ +createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_key.xml'); + } + + public function test_set_key_manually() + { + $this->session_factory->merge_config_data(array('allow_autologin' => true)); + $session = $this->session_factory->get_session($this->db); + $session->cookie_data['u'] = 4; + $session->cookie_data['k'] = 4; + $session->session_create(4, false, 4); + $this->assertEquals(4, $session->data['user_id']); + } +} From f5a09858d044592fa027e5ce23f4060aec0c38fa Mon Sep 17 00:00:00 2001 From: asperous Date: Fri, 12 Jul 2013 07:15:46 -0700 Subject: [PATCH 26/46] [ticket/11620] Added a session key reset test PHPBB3-11620 --- tests/session/session_key_tests.php | 33 ++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/tests/session/session_key_tests.php b/tests/session/session_key_tests.php index 382ed06a15..bc3d6dd71c 100644 --- a/tests/session/session_key_tests.php +++ b/tests/session/session_key_tests.php @@ -1,4 +1,4 @@ -createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_key.xml'); @@ -18,11 +21,31 @@ class phpbb_session_login_keys_test extends phpbb_session_test_case public function test_set_key_manually() { + // With AutoLogin setup $this->session_factory->merge_config_data(array('allow_autologin' => true)); $session = $this->session_factory->get_session($this->db); - $session->cookie_data['u'] = 4; - $session->cookie_data['k'] = 4; - $session->session_create(4, false, 4); - $this->assertEquals(4, $session->data['user_id']); + // Using a user_id and key that is already in the database + $session->cookie_data['u'] = $this->user_id; + $session->cookie_data['k'] = $this->key_id; + // Try to access session + $session->session_create($this->user_id, false, $this->user_id); + + $this->assertEquals($this->user_id, $session->data['user_id'], "session should automatically login"); + } + + public function test_reset_keys() + { + // With AutoLogin setup + $this->session_factory->merge_config_data(array('allow_autologin' => true)); + $session = $this->session_factory->get_session($this->db); + // Reset of the keys for this user + $session->reset_login_keys($this->user_id); + // Using a user_id and key that was in the database (before reset) + $session->cookie_data['u'] = $this->user_id; + $session->cookie_data['k'] = $this->key_id; + // Try to access session + $session->session_create($this->user_id, false, $this->user_id); + + $this->assertNotEquals($this->user_id, $session->data['user_id'], "session should be cleared"); } } From 750ea771084d96097ea5a22c11a6659e8f39869d Mon Sep 17 00:00:00 2001 From: asperous Date: Fri, 12 Jul 2013 07:20:46 -0700 Subject: [PATCH 27/46] [ticket/11620] Typo in file name session_key_tests -> test PHPBB3-11620 --- tests/session/{session_key_tests.php => session_key_test.php} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/session/{session_key_tests.php => session_key_test.php} (100%) diff --git a/tests/session/session_key_tests.php b/tests/session/session_key_test.php similarity index 100% rename from tests/session/session_key_tests.php rename to tests/session/session_key_test.php From 016faad6682495a35566d2d0451697486a47e80c Mon Sep 17 00:00:00 2001 From: asperous Date: Fri, 12 Jul 2013 09:47:06 -0700 Subject: [PATCH 28/46] [ticket/11620] Remove typo in beginning of session_key_test PHPBB3-11620 --- tests/session/session_key_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/session/session_key_test.php b/tests/session/session_key_test.php index bc3d6dd71c..6406742168 100644 --- a/tests/session/session_key_test.php +++ b/tests/session/session_key_test.php @@ -1,4 +1,4 @@ -_ Date: Fri, 12 Jul 2013 09:54:38 -0700 Subject: [PATCH 29/46] [ticket/11620] Added a test for checking if users are banned PHPBB3-11620 --- tests/mock/session_testable.php | 4 ++ tests/session/check_ban_test.php | 51 ++++++++++++++++ tests/session/fixtures/sessions_banlist.xml | 66 +++++++++++++++++++++ 3 files changed, 121 insertions(+) create mode 100644 tests/session/check_ban_test.php create mode 100644 tests/session/fixtures/sessions_banlist.xml diff --git a/tests/mock/session_testable.php b/tests/mock/session_testable.php index 56ff8c8b32..283f9af192 100644 --- a/tests/mock/session_testable.php +++ b/tests/mock/session_testable.php @@ -58,5 +58,9 @@ class phpbb_mock_session_testable extends phpbb_session } } } + + public function setup() + { + } } diff --git a/tests/session/check_ban_test.php b/tests/session/check_ban_test.php new file mode 100644 index 0000000000..ec1f5e3aa1 --- /dev/null +++ b/tests/session/check_ban_test.php @@ -0,0 +1,51 @@ +createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_banlist.xml'); + } + + static function check_banned_data() + { + return array( + array('All false values, should not be banned', + false, false, false, false, /* ?: */ false), + array('Matching values in the database, should be banned', + 4, '127.0.0.1', 'bar@example.org', true, /* ?: */ true), + array('IP Banned, should be banned', + false, '127.1.1.1', false, falseN, /* ?: */ true), + ); + } + + /** @dataProvider check_banned_data */ + public function test_check_is_banned($test_msg, $user_id, $user_ips, $user_email, $return, $should_be_banned) + { + $session = $this->session_factory->get_session($this->db); + // Change the global cache object for this test because + // the mock cache object does not hit the database as is + // needed for this test. + global $cache; + $old_cache = $cache; + $cache = new phpbb_cache_driver_file(); + + $is_banned = + $session->check_ban($user_id, $user_ips, $user_email, $return); + $this->assertEquals($should_be_banned, $is_banned, $test_msg); + + $cache = $old_cache; + } +} diff --git a/tests/session/fixtures/sessions_banlist.xml b/tests/session/fixtures/sessions_banlist.xml new file mode 100644 index 0000000000..9422fc0665 --- /dev/null +++ b/tests/session/fixtures/sessions_banlist.xml @@ -0,0 +1,66 @@ + + + + user_id + username_clean + user_permissions + user_sig + user_occ + user_interests + + 1 + anonymous + + + + + +
+ + session_id + session_user_id + session_ip + session_browser + session_admin + + bar_session000000000000000000000 + 4 + 127.0.0.1 + user agent + 1 + +
+ + ban_id + ban_userid + ban_ip + ban_email + ban_start + ban_end + ban_exclude + ban_reason + ban_give_reason + + 2 + 4 + 127.0.0.1 + bar@example.org + 1111 + 0 + 0 + HAHAHA + 1 + + + 3 + 0 + 127.1.1.1 + + 1111 + 0 + 0 + HAHAHA + 1 + +
+ From 362480263cbad6cabbe2637edca153b27d97c493 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Tue, 25 Jun 2013 12:20:42 -0700 Subject: [PATCH 30/46] [ticket/11615] Rename continue -> check_isvalid for clarity PHPBB3-11615 --- tests/session/{continue_test.php => check_isvalid_test.php} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename tests/session/{continue_test.php => check_isvalid_test.php} (98%) diff --git a/tests/session/continue_test.php b/tests/session/check_isvalid_test.php similarity index 98% rename from tests/session/continue_test.php rename to tests/session/check_isvalid_test.php index e5a7f7a4a1..7cc6f13286 100644 --- a/tests/session/continue_test.php +++ b/tests/session/check_isvalid_test.php @@ -9,7 +9,7 @@ require_once dirname(__FILE__) . '/testable_factory.php'; -class phpbb_session_continue_test extends phpbb_database_test_case +class phpbb_session_check_isvalid_test extends phpbb_database_test_case { public function getDataSet() { From e74abfaa2c25b7c9b4f2f865fbf6800de0761a6b Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Tue, 25 Jun 2013 12:24:02 -0700 Subject: [PATCH 31/46] [ticket/11615] Refactored isvalid test to be more imperative Refactoring the continue/is_valid test to remove the confusing data provider work around, while still keeping redundancies down to a minimum. PHPBB3-11615 --- tests/session/check_isvalid_test.php | 126 +++++++++------------------ tests/session/testable_factory.php | 28 +++++- 2 files changed, 66 insertions(+), 88 deletions(-) diff --git a/tests/session/check_isvalid_test.php b/tests/session/check_isvalid_test.php index 7cc6f13286..8083e3406a 100644 --- a/tests/session/check_isvalid_test.php +++ b/tests/session/check_isvalid_test.php @@ -2,7 +2,7 @@ /** * * @package testing -* @copyright (c) 2011 phpBB Group +* @copyright (c) 2013 phpBB Group * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2 * */ @@ -16,42 +16,7 @@ class phpbb_session_check_isvalid_test extends phpbb_database_test_case return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_full.xml'); } - static public function session_begin_attempts() - { - // The session_id field is defined as CHAR(32) in the database schema. - // Thus the data we put in session_id fields has to have a length of 32 characters on stricter DBMSes. - // Thus we fill those strings up with zeroes until they have a string length of 32. - - return array( - array( - 'bar_session000000000000000000000', '4', 'user agent', '127.0.0.1', - array( - array('session_id' => 'anon_session00000000000000000000', 'session_user_id' => 1), - array('session_id' => 'bar_session000000000000000000000', 'session_user_id' => 4), - ), - array(), - 'If a request comes with a valid session id with matching user agent and IP, no new session should be created.', - ), - array( - 'anon_session00000000000000000000', '4', 'user agent', '127.0.0.1', - array( - array('session_id' => '__new_session_id__', 'session_user_id' => 1), // use generated SID - array('session_id' => 'bar_session000000000000000000000', 'session_user_id' => 4), - ), - array( - 'u' => array('1', null), - 'k' => array(null, null), - 'sid' => array('__new_session_id__', null), - ), - 'If a request comes with a valid session id and IP but different user id and user agent, a new anonymous session is created and the session matching the supplied session id is deleted.', - ), - ); - } - - /** - * @dataProvider session_begin_attempts - */ - public function test_session_begin_valid_session($session_id, $user_id, $user_agent, $ip, $expected_sessions, $expected_cookies, $message) + protected function access_with($session_id, $user_id, $user_agent, $ip) { global $phpbb_container, $phpbb_root_path, $phpEx; @@ -68,66 +33,53 @@ class phpbb_session_check_isvalid_test extends phpbb_database_test_case ->will($this->returnValue($auth_provider)); $session_factory = new phpbb_session_testable_factory; - $session_factory->set_cookies(array( - '_sid' => $session_id, - '_u' => $user_id, - )); - $session_factory->merge_config_data(array( - 'session_length' => time(), // need to do this to allow sessions started at time 0 - )); - $session_factory->merge_server_data(array( - 'HTTP_USER_AGENT' => $user_agent, - 'REMOTE_ADDR' => $ip, - )); + $session_factory->merge_test_data($session_id, $user_id, $user_agent, $ip); $session = $session_factory->get_session($db); $session->page = array('page' => 'page', 'forum' => 0); $session->session_begin(); - - $sql = 'SELECT session_id, session_user_id - FROM phpbb_sessions - ORDER BY session_user_id'; - - $expected_sessions = $this->replace_session($expected_sessions, $session->session_id); - $expected_cookies = $this->replace_session($expected_cookies, $session->session_id); - - $this->assertSqlResultEquals( - $expected_sessions, - $sql, - $message - ); - - $session->check_cookies($this, $expected_cookies); - $session_factory->check($this); + return $session; } - /** - * Replaces recursively the value __new_session_id__ with the given session - * id. - * - * @param array $array An array of data - * @param string $session_id The new session id to use instead of the - * placeholder. - * @return array The input array with all occurances of __new_session_id__ - * replaced. - */ - public function replace_session($array, $session_id) + protected function check_session_equals($expected_sessions, $message) { - foreach ($array as $key => &$value) - { - if ($value === '__new_session_id__') - { - $value = $session_id; - } + $sql = 'SELECT session_id, session_user_id + FROM phpbb_sessions + ORDER BY session_user_id'; - if (is_array($value)) - { - $value = $this->replace_session($value, $session_id); - } - } + $this->assertSqlResultEquals($expected_sessions, $sql, $message); + } - return $array; + public function test_session_valid_session_exists() + { + $session = $this->access_with('bar_session000000000000000000000', '4', 'user agent', '127.0.0.1'); + $session->check_cookies($this, array()); + + $this->check_session_equals(array( + array('session_id' => 'anon_session00000000000000000000', 'session_user_id' => 1), + array('session_id' => 'bar_session000000000000000000000', 'session_user_id' => 4), + ), + 'If a request comes with a valid session id with matching user agent and IP, no new session should be created.' + ); + } + + public function test_session_invalid_make_new_annon_session() + { + $session = $this->access_with('anon_session00000000000000000000', '4', 'user agent', '127.0.0.1'); + $session->check_cookies($this, array( + 'u' => array('1', null), + 'k' => array(null, null), + 'sid' => array($session->session_id, null), + )); + + $this->check_session_equals(array( + array('session_id' => $session->session_id, 'session_user_id' => 1), // use generated SID + array('session_id' => 'bar_session000000000000000000000', 'session_user_id' => 4), + ), + 'If a request comes with a valid session id and IP but different user id and user agent, + a new anonymous session is created and the session matching the supplied session id is deleted.' + ); } } diff --git a/tests/session/testable_factory.php b/tests/session/testable_factory.php index ace968eb43..8733ce15ef 100644 --- a/tests/session/testable_factory.php +++ b/tests/session/testable_factory.php @@ -2,7 +2,7 @@ /** * * @package testing -* @copyright (c) 2011 phpBB Group +* @copyright (c) 2013 phpBB Group * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2 * */ @@ -174,6 +174,32 @@ class phpbb_session_testable_factory return $this->server_data = array_merge($this->server_data, $server_data); } + /** + * Set cookies, merge config and server data in one step. + * + * New values overwrite old ones. + * + * @param $session_id + * @param $user_id + * @param $user_agent + * @param $ip + * @param int $time + */ + public function merge_test_data($session_id, $user_id, $user_agent, $ip, $time = 0) + { + $this->set_cookies(array( + '_sid' => $session_id, + '_u' => $user_id, + )); + $this->merge_config_data(array( + 'session_length' => time() + $time, // need to do this to allow sessions started at time 0 + )); + $this->merge_server_data(array( + 'HTTP_USER_AGENT' => $user_agent, + 'REMOTE_ADDR' => $ip, + )); + } + /** * Retrieve all server variables to be passed to the session. * From 2d850ba7a8f57dd74a23f0feaedf5c5fc409a520 Mon Sep 17 00:00:00 2001 From: asperous Date: Fri, 12 Jul 2013 11:08:16 -0700 Subject: [PATCH 32/46] [ticket/11620] Refactored check_isvalid_test to use session_test_case Since the continue->isvalid refactoring is now in a branch with the session_test_case framework, this test can be refactored to use that framework. PHPBB3-11620 --- tests/session/check_isvalid_test.php | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/tests/session/check_isvalid_test.php b/tests/session/check_isvalid_test.php index 8083e3406a..6c21359c7d 100644 --- a/tests/session/check_isvalid_test.php +++ b/tests/session/check_isvalid_test.php @@ -7,9 +7,10 @@ * */ -require_once dirname(__FILE__) . '/testable_factory.php'; +require_once dirname(__FILE__) . '/../test_framework/phpbb_session_test_case.php'; -class phpbb_session_check_isvalid_test extends phpbb_database_test_case + +class phpbb_session_check_isvalid_test extends phpbb_session_test_case { public function getDataSet() { @@ -18,28 +19,13 @@ class phpbb_session_check_isvalid_test extends phpbb_database_test_case protected function access_with($session_id, $user_id, $user_agent, $ip) { - global $phpbb_container, $phpbb_root_path, $phpEx; + $this->session_factory->merge_test_data($session_id, $user_id, $user_agent, $ip); - $db = $this->new_dbal(); - $config = new phpbb_config(array()); - $request = $this->getMock('phpbb_request'); - $user = $this->getMock('phpbb_user'); - - $auth_provider = new phpbb_auth_provider_db($db, $config, $request, $user, $phpbb_root_path, $phpEx); - $phpbb_container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface'); - $phpbb_container->expects($this->any()) - ->method('get') - ->with('auth.provider.db') - ->will($this->returnValue($auth_provider)); - - $session_factory = new phpbb_session_testable_factory; - $session_factory->merge_test_data($session_id, $user_id, $user_agent, $ip); - - $session = $session_factory->get_session($db); + $session = $this->session_factory->get_session($this->db); $session->page = array('page' => 'page', 'forum' => 0); $session->session_begin(); - $session_factory->check($this); + $this->session_factory->check($this); return $session; } From 13e4271c502152b8fa318422d808aabeb97e6c8c Mon Sep 17 00:00:00 2001 From: asperous Date: Fri, 12 Jul 2013 11:28:17 -0700 Subject: [PATCH 33/46] [ticket/11620] Fixed a typo on check_ban_test PHPBB3-11620 --- tests/session/check_ban_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/session/check_ban_test.php b/tests/session/check_ban_test.php index ec1f5e3aa1..6795338f23 100644 --- a/tests/session/check_ban_test.php +++ b/tests/session/check_ban_test.php @@ -27,7 +27,7 @@ class phpbb_session_check_ban_test extends phpbb_session_test_case array('Matching values in the database, should be banned', 4, '127.0.0.1', 'bar@example.org', true, /* ?: */ true), array('IP Banned, should be banned', - false, '127.1.1.1', false, falseN, /* ?: */ true), + false, '127.1.1.1', false, false, /* ?: */ true), ); } From af3a4ee33a148c864c30d22a2031cfc7e1b7bcf3 Mon Sep 17 00:00:00 2001 From: asperous Date: Fri, 12 Jul 2013 13:04:09 -0700 Subject: [PATCH 34/46] [ticket/11620] Fixed check_ban_test errors with cache and ban warning message PHPBB3-11620 --- tests/session/check_ban_test.php | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/tests/session/check_ban_test.php b/tests/session/check_ban_test.php index 6795338f23..6ff688ee3d 100644 --- a/tests/session/check_ban_test.php +++ b/tests/session/check_ban_test.php @@ -38,14 +38,26 @@ class phpbb_session_check_ban_test extends phpbb_session_test_case // Change the global cache object for this test because // the mock cache object does not hit the database as is // needed for this test. - global $cache; - $old_cache = $cache; - $cache = new phpbb_cache_driver_file(); + global $cache, $config, $phpbb_root_path, $php_ext; + $cache = new phpbb_cache_service( + new phpbb_cache_driver_file(), + $config, + $this->db, + $phpbb_root_path, + $php_ext + ); - $is_banned = - $session->check_ban($user_id, $user_ips, $user_email, $return); + try + { + $is_banned = + $session->check_ban($user_id, $user_ips, $user_email, $return); + } catch (PHPUnit_Framework_Error_Notice $e) + { + // User error was triggered, user must have been banned + $is_banned = true; + } $this->assertEquals($should_be_banned, $is_banned, $test_msg); - $cache = $old_cache; + $cache = new phpbb_mock_cache(); } } From 7dbd85ad02b031fc2392adfd27c66c3725ef117b Mon Sep 17 00:00:00 2001 From: asperous Date: Fri, 12 Jul 2013 13:04:37 -0700 Subject: [PATCH 35/46] [ticket/11620] Added garbage_collection_test PHPBB3-11620 --- tests/session/fixtures/sessions_garbage.xml | 58 +++++++++++++++++++ tests/session/garbage_collection_test.php | 63 +++++++++++++++++++++ 2 files changed, 121 insertions(+) create mode 100644 tests/session/fixtures/sessions_garbage.xml create mode 100644 tests/session/garbage_collection_test.php diff --git a/tests/session/fixtures/sessions_garbage.xml b/tests/session/fixtures/sessions_garbage.xml new file mode 100644 index 0000000000..23c44a975b --- /dev/null +++ b/tests/session/fixtures/sessions_garbage.xml @@ -0,0 +1,58 @@ + + + + user_id + username_clean + user_permissions + user_sig + user_occ + user_interests + + 4 + bar + + + + + +
+ + session_id + session_user_id + session_ip + session_browser + session_admin + + anon_session00000000000000000000 + 1 + 127.0.0.1 + anonymous user agent + 0 + + + bar_session000000000000000000000 + 4 + 127.0.0.1 + user agent + 1 + +
+ + attempt_ip + attempt_browser + attempt_forwarded_for + attempt_time + user_id + username + username_clean + + 127.0.0.1 + browser + + 0001 + 4 + bar + bar + +
+ diff --git a/tests/session/garbage_collection_test.php b/tests/session/garbage_collection_test.php new file mode 100644 index 0000000000..1b607dfb4f --- /dev/null +++ b/tests/session/garbage_collection_test.php @@ -0,0 +1,63 @@ +createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_garbage.xml'); + } + + public function setUp() + { + parent::setUp(); + $this->session = $this->session_factory->get_session($this->db); + } + + protected function assert_sessions_equal($expected, $msg) + { + $sql = 'SELECT session_id, session_user_id + FROM phpbb_sessions + ORDER BY session_user_id'; + + $this->assertSqlResultEquals($expected, $sql, $msg); + } + + public function test_cleanup_all() + { + $this->assert_sessions_equal( + array( + array + ( + 'session_id' => 'anon_session00000000000000000000', + 'session_user_id' => 1, + ), + array + ( + 'session_id' => 'bar_session000000000000000000000', + 'session_user_id' => 4, + )), + 'Before test, should have some sessions.' + ); + // Set session length so it clears all + global $config; + $config['session_length'] = 0; + // There is an error unless the captcha plugin is set + $config['captcha_plugin'] = 'phpbb_captcha_nogd'; + $this->session->session_gc(); + $this->assert_sessions_equal( + array(), + 'After setting session time to 0, should remove all.' + ); + } +} From 30f198c61a56deb14223a60a8ebf370cc90d9f4b Mon Sep 17 00:00:00 2001 From: asperous Date: Sat, 13 Jul 2013 07:32:49 -0700 Subject: [PATCH 36/46] [ticket/11620] Update auth_provider for new interface PHPBB3-11620 --- tests/mock/auth_provider.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/mock/auth_provider.php b/tests/mock/auth_provider.php index 9674c573e3..e0a2abd2d5 100644 --- a/tests/mock/auth_provider.php +++ b/tests/mock/auth_provider.php @@ -31,7 +31,7 @@ class phpbb_mock_auth_provider implements phpbb_auth_provider_interface return array(); } - function acp($new) + function acp() { return array(); } @@ -45,4 +45,9 @@ class phpbb_mock_auth_provider implements phpbb_auth_provider_interface { return null; } + + public function get_acp_template($new_config) + { + return null; + } } From 25b189d33bbf2b94da6f9f969e8d91ade8eba30a Mon Sep 17 00:00:00 2001 From: asperous Date: Sat, 13 Jul 2013 08:26:46 -0700 Subject: [PATCH 37/46] [ticket/11620] Cleanup creation_test that was renamed on a cherry-pick PHPBB3-11620 --- tests/session/creation_test.php | 69 --------------------------------- 1 file changed, 69 deletions(-) delete mode 100644 tests/session/creation_test.php diff --git a/tests/session/creation_test.php b/tests/session/creation_test.php deleted file mode 100644 index fde76d6b06..0000000000 --- a/tests/session/creation_test.php +++ /dev/null @@ -1,69 +0,0 @@ -createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_empty.xml'); - } - - // also see security/extract_current_page.php - - public function test_login_session_create() - { - global $phpbb_container, $phpbb_root_path, $phpEx; - - $db = $this->new_dbal(); - $config = new phpbb_config(array()); - $request = $this->getMock('phpbb_request'); - $user = $this->getMock('phpbb_user'); - - $auth_provider = new phpbb_auth_provider_db($db, $config, $request, $user, $phpbb_root_path, $phpEx); - $phpbb_container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface'); - $phpbb_container->expects($this->any()) - ->method('get') - ->with('auth.provider.db') - ->will($this->returnValue($auth_provider)); - - $session_factory = new phpbb_session_testable_factory; - - $session = $session_factory->get_session($db); - $session->page = array('page' => 'page', 'forum' => 0); - - $session->session_create(3); - - $sql = 'SELECT session_user_id - FROM phpbb_sessions'; - - $this->assertSqlResultEquals( - array(array('session_user_id' => 3)), - $sql, - 'Check if exactly one session for user id 3 was created' - ); - - $one_year_in_seconds = 365 * 24 * 60 * 60; - $cookie_expire = $session->time_now + $one_year_in_seconds; - - $session->check_cookies($this, array( - 'u' => array(null, $cookie_expire), - 'k' => array(null, $cookie_expire), - 'sid' => array($session->session_id, $cookie_expire), - )); - - global $SID, $_SID; - $this->assertEquals($session->session_id, $_SID); - $this->assertEquals('?sid=' . $session->session_id, $SID); - - $session_factory->check($this); - } -} - From de2cb595336ee49aeb4bab4ee857fbe71e249599 Mon Sep 17 00:00:00 2001 From: asperous Date: Sat, 13 Jul 2013 08:38:02 -0700 Subject: [PATCH 38/46] [ticket/11620] Fix a static calls to non-static for session captcha These changes fixes an error in certain version of php that throws an error if you call a non-static method statically. PHPBB3-11620 --- phpBB/includes/captcha/captcha_factory.php | 3 ++- phpBB/includes/session.php | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/captcha/captcha_factory.php b/phpBB/includes/captcha/captcha_factory.php index 1ed8e119b5..af79f059fe 100644 --- a/phpBB/includes/captcha/captcha_factory.php +++ b/phpBB/includes/captcha/captcha_factory.php @@ -50,7 +50,8 @@ class phpbb_captcha_factory { include($phpbb_root_path . "includes/captcha/plugins/{$name}_plugin." . $phpEx); } - call_user_func(array($name, 'garbage_collect'), 0); + $captcha = new $name; + $captcha->garbage_collect(''); } /** diff --git a/phpBB/includes/session.php b/phpBB/includes/session.php index 66bf053f7d..392a55e48f 100644 --- a/phpBB/includes/session.php +++ b/phpBB/includes/session.php @@ -1016,7 +1016,8 @@ class phpbb_session { include($phpbb_root_path . "includes/captcha/captcha_factory." . $phpEx); } - phpbb_captcha_factory::garbage_collect($config['captcha_plugin']); + $captcha_factory = new phpbb_captcha_factory(); + $captcha_factory->garbage_collect($config['captcha_plugin']); $sql = 'DELETE FROM ' . LOGIN_ATTEMPT_TABLE . ' WHERE attempt_time < ' . (time() - (int) $config['ip_login_limit_time']); From cc1aef47fb4f5d37415436c62067ad2dcde768bb Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Mon, 22 Jul 2013 11:13:31 -0700 Subject: [PATCH 39/46] [ticket/11620] Changes for code guidelines consistency PHPBB3-11620 --- tests/session/extract_page_test.php | 20 ++++++++++---------- tests/session/testable_facade.php | 3 ++- tests/session/validate_referrer_test.php | 5 +++-- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/tests/session/extract_page_test.php b/tests/session/extract_page_test.php index 9346973bc4..4dbbbc9c59 100644 --- a/tests/session/extract_page_test.php +++ b/tests/session/extract_page_test.php @@ -32,8 +32,8 @@ class phpbb_session_extract_page_test extends phpbb_session_test_case 'root_script_path' => '/phpBB/', 'page' => 'index.php', 'forum' => 0, - ) - ) , + ), + ), array( './', '/phpBB/ucp.php', @@ -47,8 +47,8 @@ class phpbb_session_extract_page_test extends phpbb_session_test_case 'root_script_path' => '/phpBB/', 'page' => 'ucp.php?mode=login', 'forum' => 0, - ) - ) , + ), + ), array( './', '/phpBB/ucp.php', @@ -62,8 +62,8 @@ class phpbb_session_extract_page_test extends phpbb_session_test_case 'root_script_path' => '/phpBB/', 'page' => 'ucp.php?mode=register', 'forum' => 0, - ) - ) , + ), + ), array( './', '/phpBB/ucp.php', @@ -77,8 +77,8 @@ class phpbb_session_extract_page_test extends phpbb_session_test_case 'root_script_path' => '/phpBB/', 'page' => 'ucp.php?mode=register', 'forum' => 0, - ) - ) , + ), + ), array( './../', '/phpBB/adm/index.php', @@ -93,8 +93,8 @@ class phpbb_session_extract_page_test extends phpbb_session_test_case 'root_script_path' => '/phpBB/', //'page' => 'adm/index.php', 'forum' => 0, - ) - ) + ), + ), ); } diff --git a/tests/session/testable_facade.php b/tests/session/testable_facade.php index 1cb1c94b52..1343b34a79 100644 --- a/tests/session/testable_facade.php +++ b/tests/session/testable_facade.php @@ -27,7 +27,8 @@ class phpbb_session_testable_facade protected $db; protected $session_factory; - function __construct($db, $session_factory) { + function __construct($db, $session_factory) + { $this->db = $db; $this->session_factory = $session_factory; } diff --git a/tests/session/validate_referrer_test.php b/tests/session/validate_referrer_test.php index f91ac5f1f9..b517b668ac 100644 --- a/tests/session/validate_referrer_test.php +++ b/tests/session/validate_referrer_test.php @@ -52,7 +52,7 @@ class phpbb_session_validate_referrer_test extends phpbb_session_test_case ) { // Referrer needs http:// because it's going to get stripped in function. - $referrer = $referrer ? 'http://'.$referrer : ''; + $referrer = $referrer ? 'http://' . $referrer : ''; $this->assertEquals( $pass_or_fail, $this->session_facade->validate_referer( @@ -63,6 +63,7 @@ class phpbb_session_validate_referrer_test extends phpbb_session_test_case $server_port, $server_name, $root_script_path - ), "referrer should" . ($pass_or_fail? '' : "n't") . " be validated"); + ), + "referrer should" . ($pass_or_fail ? '' : "n't") . " be validated"); } } From 28e98466d959875d2f5b0e917c32783c1039dc23 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Mon, 22 Jul 2013 12:26:02 -0700 Subject: [PATCH 40/46] [ticket/11620] Changes to match merge PHPBB3-11620 --- tests/session/fixtures/sessions_full.xml | 3 +++ tests/session/testable_facade.php | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/session/fixtures/sessions_full.xml b/tests/session/fixtures/sessions_full.xml index 509687f4d2..6bbaf1c9d5 100644 --- a/tests/session/fixtures/sessions_full.xml +++ b/tests/session/fixtures/sessions_full.xml @@ -37,17 +37,20 @@ session_user_id session_ip session_browser + session_admin anon_session00000000000000000000 1 127.0.0.1 anonymous user agent + 0 bar_session000000000000000000000 4 127.0.0.1 user agent + 1 diff --git a/tests/session/testable_facade.php b/tests/session/testable_facade.php index 1343b34a79..c5e58fce05 100644 --- a/tests/session/testable_facade.php +++ b/tests/session/testable_facade.php @@ -8,7 +8,7 @@ */ require_once dirname(__FILE__) . '/testable_factory.php'; -require_once dirname(__FILE__) . '/../../phpBB/includes/session.php'; +require_once dirname(__FILE__) . '/../../phpBB/phpbb/session.php'; /** * This class exists to expose session.php's functions in a more testable way. From 9dbd42e9452fe8459de905170f9031a4515cc0b2 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Mon, 22 Jul 2013 13:51:06 -0700 Subject: [PATCH 41/46] [ticket/11620] Space between . in directory import concatenation PHPBB3-11620 --- tests/session/check_ban_test.php | 2 +- tests/session/check_isvalid_test.php | 2 +- tests/session/create_test.php | 2 +- tests/session/extract_hostname_test.php | 2 +- tests/session/extract_page_test.php | 2 +- tests/session/garbage_collection_test.php | 2 +- tests/session/session_key_test.php | 2 +- tests/session/unset_admin_test.php | 2 +- tests/session/validate_referrer_test.php | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/session/check_ban_test.php b/tests/session/check_ban_test.php index 6ff688ee3d..dfe971ea27 100644 --- a/tests/session/check_ban_test.php +++ b/tests/session/check_ban_test.php @@ -16,7 +16,7 @@ class phpbb_session_check_ban_test extends phpbb_session_test_case public function getDataSet() { - return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_banlist.xml'); + return $this->createXMLDataSet(dirname(__FILE__) . '/fixtures/sessions_banlist.xml'); } static function check_banned_data() diff --git a/tests/session/check_isvalid_test.php b/tests/session/check_isvalid_test.php index 6c21359c7d..24dce7c9c4 100644 --- a/tests/session/check_isvalid_test.php +++ b/tests/session/check_isvalid_test.php @@ -14,7 +14,7 @@ class phpbb_session_check_isvalid_test extends phpbb_session_test_case { public function getDataSet() { - return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_full.xml'); + return $this->createXMLDataSet(dirname(__FILE__) . '/fixtures/sessions_full.xml'); } protected function access_with($session_id, $user_id, $user_agent, $ip) diff --git a/tests/session/create_test.php b/tests/session/create_test.php index a8248ae62c..64140f9883 100644 --- a/tests/session/create_test.php +++ b/tests/session/create_test.php @@ -13,7 +13,7 @@ class phpbb_session_create_test extends phpbb_session_test_case { public function getDataSet() { - return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_full.xml'); + return $this->createXMLDataSet(dirname(__FILE__) . '/fixtures/sessions_full.xml'); } static function bot($bot_agent, $user_id, $bot_ip) diff --git a/tests/session/extract_hostname_test.php b/tests/session/extract_hostname_test.php index 5ff43cbb60..bd183fd438 100644 --- a/tests/session/extract_hostname_test.php +++ b/tests/session/extract_hostname_test.php @@ -13,7 +13,7 @@ class phpbb_session_extract_hostname_test extends phpbb_session_test_case { public function getDataSet() { - return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_empty.xml'); + return $this->createXMLDataSet(dirname(__FILE__) . '/fixtures/sessions_empty.xml'); } static public function extract_current_hostname_data() diff --git a/tests/session/extract_page_test.php b/tests/session/extract_page_test.php index 4dbbbc9c59..f4ae8de021 100644 --- a/tests/session/extract_page_test.php +++ b/tests/session/extract_page_test.php @@ -13,7 +13,7 @@ class phpbb_session_extract_page_test extends phpbb_session_test_case { public function getDataSet() { - return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_empty.xml'); + return $this->createXMLDataSet(dirname(__FILE__) . '/fixtures/sessions_empty.xml'); } static public function extract_current_page_data() diff --git a/tests/session/garbage_collection_test.php b/tests/session/garbage_collection_test.php index 1b607dfb4f..b1be958d62 100644 --- a/tests/session/garbage_collection_test.php +++ b/tests/session/garbage_collection_test.php @@ -15,7 +15,7 @@ class phpbb_session_garbage_collection_test extends phpbb_session_test_case public function getDataSet() { - return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_garbage.xml'); + return $this->createXMLDataSet(dirname(__FILE__) . '/fixtures/sessions_garbage.xml'); } public function setUp() diff --git a/tests/session/session_key_test.php b/tests/session/session_key_test.php index 6406742168..1cf2101385 100644 --- a/tests/session/session_key_test.php +++ b/tests/session/session_key_test.php @@ -16,7 +16,7 @@ class phpbb_session_login_keys_test extends phpbb_session_test_case public function getDataSet() { - return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_key.xml'); + return $this->createXMLDataSet(dirname(__FILE__) . '/fixtures/sessions_key.xml'); } public function test_set_key_manually() diff --git a/tests/session/unset_admin_test.php b/tests/session/unset_admin_test.php index bbb5eb1439..1d5b1759ab 100644 --- a/tests/session/unset_admin_test.php +++ b/tests/session/unset_admin_test.php @@ -13,7 +13,7 @@ class phpbb_session_unset_admin_test extends phpbb_session_test_case { public function getDataSet() { - return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_full.xml'); + return $this->createXMLDataSet(dirname(__FILE__) . '/fixtures/sessions_full.xml'); } function get_test_session() diff --git a/tests/session/validate_referrer_test.php b/tests/session/validate_referrer_test.php index b517b668ac..f78bf36c1b 100644 --- a/tests/session/validate_referrer_test.php +++ b/tests/session/validate_referrer_test.php @@ -13,7 +13,7 @@ class phpbb_session_validate_referrer_test extends phpbb_session_test_case { public function getDataSet() { - return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/sessions_empty.xml'); + return $this->createXMLDataSet(dirname(__FILE__) . '/fixtures/sessions_empty.xml'); } static function referrer_inputs() { From 9d38ded22875e023d4391e7724673087f87fa481 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Mon, 22 Jul 2013 13:52:17 -0700 Subject: [PATCH 42/46] [ticket/11620] Expected and actual test conditions wrongly swapped PHPBB3-11620 --- tests/session/create_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/session/create_test.php b/tests/session/create_test.php index 64140f9883..0faedf4db2 100644 --- a/tests/session/create_test.php +++ b/tests/session/create_test.php @@ -38,6 +38,6 @@ class phpbb_session_create_test extends phpbb_session_test_case self::bot('user agent', 13, '127.0.0.1'), '' ); - $this->assertEquals($output->data['is_bot'], true, 'should be a bot'); + $this->assertEquals(true, $output->data['is_bot'] , 'should be a bot'); } } From cc6147f8769b7d70c9c48257c787ff00552beaf4 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Mon, 22 Jul 2013 15:41:51 -0700 Subject: [PATCH 43/46] [ticket/11620] Minor indentation changes and comment clarity PHPBB3-11620 --- tests/session/check_ban_test.php | 11 +++++------ tests/session/check_isvalid_test.php | 1 - 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/session/check_ban_test.php b/tests/session/check_ban_test.php index dfe971ea27..303751f54c 100644 --- a/tests/session/check_ban_test.php +++ b/tests/session/check_ban_test.php @@ -23,11 +23,11 @@ class phpbb_session_check_ban_test extends phpbb_session_test_case { return array( array('All false values, should not be banned', - false, false, false, false, /* ?: */ false), + false, false, false, false, /* should be banned? -> */ false), array('Matching values in the database, should be banned', - 4, '127.0.0.1', 'bar@example.org', true, /* ?: */ true), + 4, '127.0.0.1', 'bar@example.org', true, /* should be banned? -> */ true), array('IP Banned, should be banned', - false, '127.1.1.1', false, false, /* ?: */ true), + false, '127.1.1.1', false, false, /* should be banned? -> */ true), ); } @@ -38,7 +38,7 @@ class phpbb_session_check_ban_test extends phpbb_session_test_case // Change the global cache object for this test because // the mock cache object does not hit the database as is // needed for this test. - global $cache, $config, $phpbb_root_path, $php_ext; + global $cache, $config, $phpbb_root_path, $php_ext; $cache = new phpbb_cache_service( new phpbb_cache_driver_file(), $config, @@ -49,8 +49,7 @@ class phpbb_session_check_ban_test extends phpbb_session_test_case try { - $is_banned = - $session->check_ban($user_id, $user_ips, $user_email, $return); + $is_banned = $session->check_ban($user_id, $user_ips, $user_email, $return); } catch (PHPUnit_Framework_Error_Notice $e) { // User error was triggered, user must have been banned diff --git a/tests/session/check_isvalid_test.php b/tests/session/check_isvalid_test.php index 24dce7c9c4..c60770dad8 100644 --- a/tests/session/check_isvalid_test.php +++ b/tests/session/check_isvalid_test.php @@ -9,7 +9,6 @@ require_once dirname(__FILE__) . '/../test_framework/phpbb_session_test_case.php'; - class phpbb_session_check_isvalid_test extends phpbb_session_test_case { public function getDataSet() From 568de3b8ceb68c4d988a7e69b0d357bd43bdd25b Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Mon, 22 Jul 2013 15:44:22 -0700 Subject: [PATCH 44/46] [ticket/11620] Changed incorrect global variable PHPBB3-11620 --- tests/session/check_ban_test.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/session/check_ban_test.php b/tests/session/check_ban_test.php index 303751f54c..fe7c70575a 100644 --- a/tests/session/check_ban_test.php +++ b/tests/session/check_ban_test.php @@ -38,13 +38,13 @@ class phpbb_session_check_ban_test extends phpbb_session_test_case // Change the global cache object for this test because // the mock cache object does not hit the database as is // needed for this test. - global $cache, $config, $phpbb_root_path, $php_ext; + global $cache, $config, $phpbb_root_path, $phpEx; $cache = new phpbb_cache_service( new phpbb_cache_driver_file(), $config, $this->db, $phpbb_root_path, - $php_ext + $phpEx ); try From 0c54fb034b71cfc2bff338430acb1e2b43083dd5 Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Mon, 22 Jul 2013 17:39:14 -0700 Subject: [PATCH 45/46] [ticket/11620] Move check_ban_test functions to setUp/tearDown for clarity PHPBB3-11620 --- tests/session/check_ban_test.php | 36 +++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/tests/session/check_ban_test.php b/tests/session/check_ban_test.php index fe7c70575a..8d6c9a866d 100644 --- a/tests/session/check_ban_test.php +++ b/tests/session/check_ban_test.php @@ -13,6 +13,8 @@ class phpbb_session_check_ban_test extends phpbb_session_test_case { protected $user_id = 4; protected $key_id = 4; + protected $session; + protected $backup_cache; public function getDataSet() { @@ -31,14 +33,16 @@ class phpbb_session_check_ban_test extends phpbb_session_test_case ); } - /** @dataProvider check_banned_data */ - public function test_check_is_banned($test_msg, $user_id, $user_ips, $user_email, $return, $should_be_banned) + public function setUp() { - $session = $this->session_factory->get_session($this->db); - // Change the global cache object for this test because - // the mock cache object does not hit the database as is - // needed for this test. + parent::setUp(); + // Get session here so that config is mocked correctly + $this->session = $this->session_factory->get_session($this->db); global $cache, $config, $phpbb_root_path, $phpEx; + $this->backup_cache = $cache; + // Change the global cache object for this test because + // the mock cache object does not hit the database as is needed + // for this test. $cache = new phpbb_cache_service( new phpbb_cache_driver_file(), $config, @@ -46,17 +50,29 @@ class phpbb_session_check_ban_test extends phpbb_session_test_case $phpbb_root_path, $phpEx ); + } + public function tearDown() + { + parent::tearDown(); + // Set cache back to what it was before the test changed it + global $cache; + $cache = $this->backup_cache; + } + + /** @dataProvider check_banned_data */ + public function test_check_is_banned($test_msg, $user_id, $user_ips, $user_email, $return, $should_be_banned) + { try { - $is_banned = $session->check_ban($user_id, $user_ips, $user_email, $return); - } catch (PHPUnit_Framework_Error_Notice $e) + $is_banned = $this->session->check_ban($user_id, $user_ips, $user_email, $return); + } + catch (PHPUnit_Framework_Error_Notice $e) { // User error was triggered, user must have been banned $is_banned = true; } - $this->assertEquals($should_be_banned, $is_banned, $test_msg); - $cache = new phpbb_mock_cache(); + $this->assertEquals($should_be_banned, $is_banned, $test_msg); } } From 2fe2724e684304e1c8323c047d1dde6cd732afcd Mon Sep 17 00:00:00 2001 From: Andy Chase Date: Mon, 22 Jul 2013 17:39:45 -0700 Subject: [PATCH 46/46] [ticket/11620] Whitespace and combine function into test_case PHPBB3-11620 --- tests/mock/auth_provider.php | 8 +++---- tests/session/check_isvalid_test.php | 13 ++--------- tests/session/create_test.php | 2 +- tests/session/garbage_collection_test.php | 22 +++++-------------- tests/session/testable_facade.php | 8 +++---- tests/session/validate_referrer_test.php | 7 +++--- .../phpbb_session_test_case.php | 9 ++++++++ 7 files changed, 30 insertions(+), 39 deletions(-) diff --git a/tests/mock/auth_provider.php b/tests/mock/auth_provider.php index e0a2abd2d5..9d002334d6 100644 --- a/tests/mock/auth_provider.php +++ b/tests/mock/auth_provider.php @@ -20,10 +20,10 @@ class phpbb_mock_auth_provider implements phpbb_auth_provider_interface function login($username, $password) { return array( - 'status' => "", - 'error_msg' => "", - 'user_row' => "", - ); + 'status' => "", + 'error_msg' => "", + 'user_row' => "", + ); } function autologin() diff --git a/tests/session/check_isvalid_test.php b/tests/session/check_isvalid_test.php index c60770dad8..760e2a6f24 100644 --- a/tests/session/check_isvalid_test.php +++ b/tests/session/check_isvalid_test.php @@ -28,21 +28,12 @@ class phpbb_session_check_isvalid_test extends phpbb_session_test_case return $session; } - protected function check_session_equals($expected_sessions, $message) - { - $sql = 'SELECT session_id, session_user_id - FROM phpbb_sessions - ORDER BY session_user_id'; - - $this->assertSqlResultEquals($expected_sessions, $sql, $message); - } - public function test_session_valid_session_exists() { $session = $this->access_with('bar_session000000000000000000000', '4', 'user agent', '127.0.0.1'); $session->check_cookies($this, array()); - $this->check_session_equals(array( + $this->check_sessions_equals(array( array('session_id' => 'anon_session00000000000000000000', 'session_user_id' => 1), array('session_id' => 'bar_session000000000000000000000', 'session_user_id' => 4), ), @@ -59,7 +50,7 @@ class phpbb_session_check_isvalid_test extends phpbb_session_test_case 'sid' => array($session->session_id, null), )); - $this->check_session_equals(array( + $this->check_sessions_equals(array( array('session_id' => $session->session_id, 'session_user_id' => 1), // use generated SID array('session_id' => 'bar_session000000000000000000000', 'session_user_id' => 4), ), diff --git a/tests/session/create_test.php b/tests/session/create_test.php index 0faedf4db2..442445599b 100644 --- a/tests/session/create_test.php +++ b/tests/session/create_test.php @@ -38,6 +38,6 @@ class phpbb_session_create_test extends phpbb_session_test_case self::bot('user agent', 13, '127.0.0.1'), '' ); - $this->assertEquals(true, $output->data['is_bot'] , 'should be a bot'); + $this->assertEquals(true, $output->data['is_bot'], 'should be a bot'); } } diff --git a/tests/session/garbage_collection_test.php b/tests/session/garbage_collection_test.php index b1be958d62..e7d01785dd 100644 --- a/tests/session/garbage_collection_test.php +++ b/tests/session/garbage_collection_test.php @@ -24,29 +24,19 @@ class phpbb_session_garbage_collection_test extends phpbb_session_test_case $this->session = $this->session_factory->get_session($this->db); } - protected function assert_sessions_equal($expected, $msg) - { - $sql = 'SELECT session_id, session_user_id - FROM phpbb_sessions - ORDER BY session_user_id'; - - $this->assertSqlResultEquals($expected, $sql, $msg); - } - public function test_cleanup_all() { - $this->assert_sessions_equal( + $this->check_sessions_equals( array( - array - ( + array( 'session_id' => 'anon_session00000000000000000000', 'session_user_id' => 1, ), - array - ( + array( 'session_id' => 'bar_session000000000000000000000', 'session_user_id' => 4, - )), + ), + ), 'Before test, should have some sessions.' ); // Set session length so it clears all @@ -55,7 +45,7 @@ class phpbb_session_garbage_collection_test extends phpbb_session_test_case // There is an error unless the captcha plugin is set $config['captcha_plugin'] = 'phpbb_captcha_nogd'; $this->session->session_gc(); - $this->assert_sessions_equal( + $this->check_sessions_equals( array(), 'After setting session time to 0, should remove all.' ); diff --git a/tests/session/testable_facade.php b/tests/session/testable_facade.php index c5e58fce05..9f0a3c5f59 100644 --- a/tests/session/testable_facade.php +++ b/tests/session/testable_facade.php @@ -33,7 +33,7 @@ class phpbb_session_testable_facade $this->session_factory = $session_factory; } - function extract_current_page ( + function extract_current_page( $root_path, $php_self, $query_string, @@ -48,7 +48,7 @@ class phpbb_session_testable_facade return phpbb_session::extract_current_page($root_path); } - function extract_current_hostname ( + function extract_current_hostname( $host, $server_name_config, $cookie_domain_config @@ -75,7 +75,7 @@ class phpbb_session_testable_facade * @param request_overrides An array of overrides for the global request object * @return boolean False if the user is identified, otherwise true. */ - function session_begin ( + function session_begin( $update_session_page = true, $config_overrides = array(), $request_overrides = array(), @@ -90,7 +90,7 @@ class phpbb_session_testable_facade return $session; } - function session_create ( + function session_create( $user_id = false, $set_admin = false, $persist_login = false, diff --git a/tests/session/validate_referrer_test.php b/tests/session/validate_referrer_test.php index f78bf36c1b..a302229287 100644 --- a/tests/session/validate_referrer_test.php +++ b/tests/session/validate_referrer_test.php @@ -16,7 +16,8 @@ class phpbb_session_validate_referrer_test extends phpbb_session_test_case return $this->createXMLDataSet(dirname(__FILE__) . '/fixtures/sessions_empty.xml'); } - static function referrer_inputs() { + static function referrer_inputs() + { $ex = "example.org"; $alt = "example.com"; return array( @@ -39,8 +40,8 @@ class phpbb_session_validate_referrer_test extends phpbb_session_test_case ); } - /** @dataProvider referrer_inputs */ - function test_referrer_inputs ( + /** @dataProvider referrer_inputs */ + function test_referrer_inputs( $check_script_path, $referrer, $host, diff --git a/tests/test_framework/phpbb_session_test_case.php b/tests/test_framework/phpbb_session_test_case.php index 6ff7d8e2ef..e6a2b03bba 100644 --- a/tests/test_framework/phpbb_session_test_case.php +++ b/tests/test_framework/phpbb_session_test_case.php @@ -24,4 +24,13 @@ abstract class phpbb_session_test_case extends phpbb_database_test_case $this->session_facade = new phpbb_session_testable_facade($this->db, $this->session_factory); } + + protected function check_sessions_equals($expected_sessions, $message) + { + $sql = 'SELECT session_id, session_user_id + FROM phpbb_sessions + ORDER BY session_user_id'; + + $this->assertSqlResultEquals($expected_sessions, $sql, $message); + } }