From 4f8a2c5dadb1f192517a896ccce386e5191b87f7 Mon Sep 17 00:00:00 2001 From: itismadness Date: Tue, 12 Jun 2018 23:46:30 +1100 Subject: [PATCH] Implement router class (!71) Squashed commit of the following: Author: itismadness fix bug with router on ajax calls finish up initial router implementation Merge branch 'master' into router Add tests for router Fix boolean on authorized function Finish basic implementation for the router --- app/Exception/InvalidAccessException.php | 11 +++ app/Exception/RouterException.php | 11 +++ app/Router.php | 96 ++++++++++++++---- classes/g.class.php | 3 + classes/script_start.php | 15 +++ classes/util.php | 10 -- sections/forums/index.php | 120 +++++------------------ tests/RouterTest.php | 113 +++++++++++++++++++++ 8 files changed, 254 insertions(+), 125 deletions(-) create mode 100644 app/Exception/InvalidAccessException.php create mode 100644 app/Exception/RouterException.php create mode 100644 tests/RouterTest.php diff --git a/app/Exception/InvalidAccessException.php b/app/Exception/InvalidAccessException.php new file mode 100644 index 000000000..9c0cefd61 --- /dev/null +++ b/app/Exception/InvalidAccessException.php @@ -0,0 +1,11 @@ +/index.php files would + * set their specific section routes on that global Router object. script_start.php + * would then include that index.php for a given section to populate the router, + * and then call the route from within script_start.php. + * + * By default, we assume that any POST requests will require authorization and any + * GET request will not. + */ +class Router { + private $authorize = ['GET' => false, 'POST' => true]; + private $routes = ['GET' => [], 'POST' => []]; + private $auth_key = null; + + /** + * Router constructor. + * @param string $auth_key Authorization key for a user + */ + public function __construct($auth_key = '') { + $this->auth_key = $auth_key; + } + + /** + * @param string|array $methods + * @param string $action + * @param string $path + * @param bool $authorize + */ + public function addRoute($methods=['GET'], string $action, string $path, bool $authorize = false) { if (is_array($methods)) { foreach ($methods as $method) { - $this->addRoute($method, $action, $file); + $this->addRoute($method, $action, $path, $authorize); } } else { - if ($methods === 'GET') { - $this->addGet($action, $file); + if (strtoupper($methods) === 'GET') { + $this->addGet($action, $path, $authorize); } - elseif ($methods === 'POST') { - $this->addPost($action, $file); + elseif (strtoupper($methods) === 'POST') { + $this->addPost($action, $path, $authorize); } } } - public function addGet($action, $file) { - $this->get[$action] = $file; + public function addGet(string $action, string $file, bool $authorize = false) { + $this->routes['GET'][$action] = ['file' => $file, 'authorize' => $authorize]; } - public function addPost($action, $file) { - $this->post[$action] = $file; + public function addPost(string $action, string $file, bool $authorize = true) { + $this->routes['POST'][$action] = ['file' => $file, 'authorize' => $authorize]; } - public function getRoute($action) { - if ($_SERVER['REQUEST_METHOD'] === 'GET' && isset($this->get[$action])) { - return SERVER_ROOT.$this->get[$action]; + public function authorizeGet(bool $authorize = true) { + $this->authorize['GET'] = $authorize; + } + + public function authorizePost(bool $authorize = true) { + $this->authorize['POST'] = $authorize; + } + + public function authorized() { + return !empty($_REQUEST['auth']) && $_REQUEST['auth'] === $this->auth_key; + } + + public function hasRoutes() { + return array_sum(array_map("count", $this->routes)) > 0; + } + + /** + * @param string $action + * @return string path to file to load + * @throws RouterException + */ + public function getRoute(string $action) { + $request_method = strtoupper(empty($_SERVER['REQUEST_METHOD']) ? 'GET' : $_SERVER['REQUEST_METHOD']); + if (isset($this->routes[$request_method]) && isset($this->routes[$request_method][$action])) { + $method = $this->routes[$request_method][$action]; } - elseif ($_SERVER['REQUEST_METHOD'] === 'POST' && isset($this->post[$action])) { - return SERVER_ROOT.$this->post[$action]; + else { + throw new RouterException("Invalid action for '${request_method}' request method"); + } + + if (($this->authorize[$request_method] || $method['authorize']) && !$this->authorized()) { + throw new InvalidAccessException(); + } + else { + return $method['file']; } - return false; } } \ No newline at end of file diff --git a/classes/g.class.php b/classes/g.class.php index f9df0c04a..210c6b3ef 100644 --- a/classes/g.class.php +++ b/classes/g.class.php @@ -4,6 +4,9 @@ class G { public static $DB; /** @var CACHE */ public static $Cache; + /** @var \Gazelle\Router */ + public static $Router; + public static $LoggedUser; public static function initialize() { diff --git a/classes/script_start.php b/classes/script_start.php index ef5b33abe..a1bbaa495 100644 --- a/classes/script_start.php +++ b/classes/script_start.php @@ -413,6 +413,7 @@ define('STAFF_LOCKED', 1); $AllowedPages = ['staffpm', 'ajax', 'locked', 'logout', 'login']; +G::$Router = new \Gazelle\Router(G::$LoggedUser['AuthKey']); if (isset(G::$LoggedUser['LockedAccount']) && !in_array($Document, $AllowedPages)) { require(SERVER_ROOT . '/sections/locked/index.php'); } @@ -425,6 +426,20 @@ else { } } +if (G::$Router->hasRoutes()) { + $action = $_REQUEST['action'] ?? ''; + try { + /** @noinspection PhpIncludeInspection */ + require_once(G::$Router->getRoute($action)); + } + catch (\Gazelle\Exception\RouterException $exception) { + error(404); + } + catch (\Gazelle\Exception\InvalidAccessException $exception) { + error(403); + } +} + $Debug->set_flag('completed module execution'); /* Required in the absence of session_start() for providing that pages will change diff --git a/classes/util.php b/classes/util.php index 0f824a05c..4c6d8011e 100644 --- a/classes/util.php +++ b/classes/util.php @@ -215,14 +215,4 @@ function unserialize_array($array) { */ function isset_array_checked($array, $value) { return (isset($array[$value])) ? "checked" : ""; -} - -function get_route(\Gazelle\Router $router, $action) { - $route = $router->getRoute($action); - if ($route === false) { - error(-1); - } - else { - require_once($route); - } } \ No newline at end of file diff --git a/sections/forums/index.php b/sections/forums/index.php index 67e35a99c..200495862 100644 --- a/sections/forums/index.php +++ b/sections/forums/index.php @@ -9,100 +9,30 @@ if (!empty($LoggedUser['DisableForums'])) { $Forums = Forums::get_forums(); $ForumCats = Forums::get_forum_categories(); -if (!empty($_POST['action'])) { - switch ($_POST['action']) { - case 'reply': - require(SERVER_ROOT.'/sections/forums/take_reply.php'); - break; - case 'new': - require(SERVER_ROOT.'/sections/forums/take_new_thread.php'); - break; - case 'mod_thread': - require(SERVER_ROOT.'/sections/forums/mod_thread.php'); - break; - case 'poll_mod': - require(SERVER_ROOT.'/sections/forums/poll_mod.php'); - break; - case 'add_poll_option': - require(SERVER_ROOT.'/sections/forums/add_poll_option.php'); - break; - case 'warn': - require(SERVER_ROOT.'/sections/forums/warn.php'); - break; - case 'take_warn': - require(SERVER_ROOT.'/sections/forums/take_warn.php'); - break; - case 'take_topic_notes': - require(SERVER_ROOT.'/sections/forums/take_topic_notes.php'); - break; +G::$Router->addGet('', SERVER_ROOT.'/sections/forums/main.php'); - default: - error(0); - } -} elseif (!empty($_GET['action'])) { - switch ($_GET['action']) { - case 'viewforum': - // Page that lists all the topics in a forum - require(SERVER_ROOT.'/sections/forums/forum.php'); - break; - case 'viewthread': - case 'viewtopic': - // Page that displays threads - require(SERVER_ROOT.'/sections/forums/thread.php'); - break; - case 'ajax_get_edit': - // Page that switches edits for mods - require(SERVER_ROOT.'/sections/forums/ajax_get_edit.php'); - break; - case 'new': - // Create a new thread - require(SERVER_ROOT.'/sections/forums/newthread.php'); - break; - case 'takeedit': - // Edit posts - require(SERVER_ROOT.'/sections/forums/takeedit.php'); - break; - case 'get_post': - // Get posts - require(SERVER_ROOT.'/sections/forums/get_post.php'); - break; - case 'get_post2': - require(SERVER_ROOT.'/sections/forums/get_post2.php'); - break; - case 'delete': - // Delete posts - require(SERVER_ROOT.'/sections/forums/delete.php'); - break; - case 'catchup': - // Catchup - require(SERVER_ROOT.'/sections/forums/catchup.php'); - break; - case 'search': - // Search posts - require(SERVER_ROOT.'/sections/forums/search.php'); - break; - case 'change_vote': - // Change poll vote - require(SERVER_ROOT.'/sections/forums/change_vote.php'); - break; - case 'delete_poll_option': - require(SERVER_ROOT.'/sections/forums/delete_poll_option.php'); - break; - case 'sticky_post': - require(SERVER_ROOT.'/sections/forums/sticky_post.php'); - break; - case 'edit_rules': - require(SERVER_ROOT.'/sections/forums/edit_rules.php'); - break; - case 'thread_subscribe': - break; - case 'warn': - require(SERVER_ROOT.'/sections/forums/warn.php'); - break; - default: - error(404); - } -} else { - require(SERVER_ROOT.'/sections/forums/main.php'); -} +G::$Router->addPost('reply', SERVER_ROOT.'/sections/forums/take_reply.php'); +G::$Router->addPost('new', SERVER_ROOT.'/sections/forums/take_new_thread.php'); +G::$Router->addPost('mod_thread', SERVER_ROOT.'/sections/forums/mod_thread.php'); +G::$Router->addPost('poll_mod', SERVER_ROOT.'/sections/forums/poll_mod.php'); +G::$Router->addPost('add_poll_option', SERVER_ROOT.'/sections/forums/add_poll_option.php'); +G::$Router->addPost('warn', SERVER_ROOT.'/sections/forums/warn.php'); +G::$Router->addPost('take_warn', SERVER_ROOT.'/sections/forums/take_warn.php'); +G::$Router->addPost('take_topic_notes', SERVER_ROOT.'/sections/forums/take_topic_notes.php'); +G::$Router->addGet('viewforum', SERVER_ROOT.'/sections/forums/forum.php'); +G::$Router->addGet('viewthread', SERVER_ROOT.'/sections/forums/thread.php'); +G::$Router->addGet('viewtopic', SERVER_ROOT.'/sections/forums/thread.php'); +G::$Router->addGet('ajax_get_edit', SERVER_ROOT.'/sections/forums/ajax_get_edit.php'); +G::$Router->addGet('new', SERVER_ROOT.'/sections/forums/newthread.php'); +G::$Router->addGet('takeedit', SERVER_ROOT.'/sections/forums/takeedit.php'); +G::$Router->addGet('get_post', SERVER_ROOT.'/sections/forums/get_post.php'); +G::$Router->addGet('delete', SERVER_ROOT.'/sections/forums/delete.php'); +G::$Router->addGet('catchup', SERVER_ROOT.'/sections/forums/catchup.php'); +G::$Router->addGet('search', SERVER_ROOT.'/sections/forums/search.php'); +G::$Router->addGet('change_vote', SERVER_ROOT.'/sections/forums/change_vote.php'); +G::$Router->addGet('delete_poll_option', SERVER_ROOT.'/sections/forums/delete_poll_option.php'); +G::$Router->addGet('sticky_post', SERVER_ROOT.'/sections/forums/sticky_post.php'); +G::$Router->addGet('edit_rules', SERVER_ROOT.'/sections/forums/edit_rules.php'); +//G::$Router->addGet('thread_subscribe', ''); +G::$Router->addGet('warn', SERVER_ROOT.'/sections/forums/warn.php'); diff --git a/tests/RouterTest.php b/tests/RouterTest.php new file mode 100644 index 000000000..eab11bb48 --- /dev/null +++ b/tests/RouterTest.php @@ -0,0 +1,113 @@ +assertFalse($router->hasRoutes()); + $router->addGet('action1', 'path'); + $this->assertTrue($router->hasRoutes()); + $router->addPost('action2', 'path2'); + $_SERVER['REQUEST_METHOD'] = 'GET'; + $this->assertEquals('path', $router->getRoute('action1')); + $_SERVER['REQUEST_METHOD'] = 'POST'; + $_REQUEST['auth'] = 'auth'; + $this->assertEquals('path2', $router->getRoute('action2')); + $this->assertTrue($router->hasRoutes()); + } + + /** + * @throws Exception\RouterException + */ + public function testAddRoutes() { + $router = new Router(); + $router->authorizePost(false); + $router->addRoute(['GET', 'POST'], 'action', 'path'); + $_SERVER['REQUEST_METHOD'] = 'GET'; + $this->assertEquals('path', $router->getRoute('action')); + $_REQUEST['REQUEST_METHOD'] = 'POST'; + $this->assertEquals('path', $router->getRoute('action')); + } + + /** + * @throws Exception\RouterException + */ + public function testAuthorizeGet() { + $router = new Router('auth23'); + $router->addGet('action', 'path', true); + $_REQUEST['auth'] = 'auth23'; + $this->assertEquals('path', $router->getRoute('action')); + } + + public function testHasGets() { + $router = new Router(); + $this->assertFalse($router->hasRoutes()); + $router->addGet('action', ''); + $this->assertTrue($router->hasRoutes()); + } + + public function testHasPosts() { + $router = new Router(); + $this->assertFalse($router->hasRoutes()); + $router->addPost('action', ''); + $this->assertTrue($router->hasRoutes()); + } + + /** + * @expectedException \Gazelle\Exception\RouterException + * @expectedExceptionMessage Invalid action for 'GET' request method + */ + public function testInvalidRoute() { + $router = new Router(); + $_SERVER['REQUEST_METHOD'] = 'GET'; + $router->getRoute('invalid'); + } + + /** + * @expectedException \Gazelle\Exception\InvalidAccessException + * @expectedExceptionMessage You are not authorized to access this action + */ + public function testNoAuthGet() { + $router = new Router(); + $router->authorizeGet(); + $router->addGet('action', 'path'); + $router->getRoute('action'); + } + + /** + * @expectedException \Gazelle\Exception\InvalidAccessException + * @expectedExceptionMessage You are not authorized to access this action + */ + public function testNoAuthPost() { + $router = new Router(); + $router->addPost('action', 'test2'); + $_SERVER['REQUEST_METHOD'] = 'POST'; + $router->getRoute('action'); + } + + /** + * @expectedException \Gazelle\Exception\InvalidAccessException + * @expectedExceptionMessage You are not authorized to access this action + */ + public function testInvalidAuth() { + $router = new Router('auth'); + $router->addPost('action', 'test_path'); + $_SERVER['REQUEST_METHOD'] = 'POST'; + $router->getRoute('action'); + } +} \ No newline at end of file