From 04d30cfe16e11140c8efb22afd61f2386c35cd87 Mon Sep 17 00:00:00 2001
From: cflip <36554078+cflip@users.noreply.github.com>
Date: Tue, 20 Jul 2021 17:25:03 -0600
Subject: Handle input validation in abstract Form class

---
 create_thread.php                  |  33 ++++-------
 includes/form/CreateThreadForm.php |  45 ++++++++++++++
 includes/form/Form.php             |  45 ++++++++++++++
 includes/form/RegisterForm.php     |  56 ++++++++++++++++++
 includes/form/SignInForm.php       |  32 ++++++++++
 includes/model/User.php            |  17 +++---
 register.php                       | 116 ++++++++++++-------------------------
 signin.php                         |  61 ++++++-------------
 8 files changed, 254 insertions(+), 151 deletions(-)
 create mode 100644 includes/form/CreateThreadForm.php
 create mode 100644 includes/form/Form.php
 create mode 100644 includes/form/RegisterForm.php
 create mode 100644 includes/form/SignInForm.php

diff --git a/create_thread.php b/create_thread.php
index d01eb26..11b278c 100755
--- a/create_thread.php
+++ b/create_thread.php
@@ -42,36 +42,23 @@ if (!Session::get()->is_signed_in()) {
 		<input type="submit" name="submit">
 	</form>
 <?php
+include_once './includes/form/CreateThreadForm.php';
 include_once './includes/model/Post.php';
 include_once './includes/model/Thread.php';
 include_once './includes/error.php';
 
 if ($_SERVER['REQUEST_METHOD'] == 'POST') {
-	$post_content = filter_input(INPUT_POST, 'post_content', FILTER_SANITIZE_STRING);
-	$thread_subject = filter_input(INPUT_POST, 'thread_subject', FILTER_SANITIZE_STRING);
-	$thread_cat = filter_input(INPUT_POST, 'thread_cat', FILTER_SANITIZE_NUMBER_INT);
+    $form = new CreateThreadForm();
+    $post_content = $form->validate_post_content($_POST['post_content']);
+    $thread_subject = $form->validate_thread_subject($_POST['thread_subject']);
+    $thread_category = $form->validate_thread_category($_POST['thread_cat']);
 
-	$errors = array();
-	if (empty($thread_subject) or !$thread_subject) {
-		$errors[] = 'Thread subject cannot be empty';
-	}
-	if (empty($post_content) or !$post_content) {
-		$errors[] = 'Thread body cannot be empty';
-	}
+	$form->on_success(function () use ($post_content, $thread_subject, $thread_category) {
+        $thread_id = Thread::create($thread_subject, $thread_category);
+        Post::create($post_content, $thread_id, $thread_category);
 
-	if (!empty($errors)) {
-		$errstr = 'Please check the following problems: <ul>';
-		foreach ($errors as $err) {
-			$errstr .= '<li>' . $err . '</li>';
-		}
-		$errstr .= '</ul>';
-		trigger_error($errstr);
-	} else {
-		$thread_id = Thread::create($thread_subject, $thread_cat);
-		Post::create($post_content, $thread_id, $thread_cat);
-
-		header("Location: viewthread.php?id=" . $thread_id);
-	}
+        header("Location: /viewthread.php?id=" . $thread_id);
+    });
 }
 ?>
 </body>
diff --git a/includes/form/CreateThreadForm.php b/includes/form/CreateThreadForm.php
new file mode 100644
index 0000000..3774f6a
--- /dev/null
+++ b/includes/form/CreateThreadForm.php
@@ -0,0 +1,45 @@
+<?php
+
+include_once './includes/form/Form.php';
+
+class CreateThreadForm extends Form
+{
+	public function validate_post_content($post_content): ?string
+	{
+		$result = null;
+
+		if (empty($post_content)) {
+			$this->report_error("Post content cannot be empty");
+		} else {
+			$result = filter_var($post_content, FILTER_SANITIZE_STRING);
+		}
+
+		return $result;
+	}
+
+	public function validate_thread_subject($thread_subject): ?string
+	{
+		$result = null;
+
+		if (empty($thread_subject)) {
+			$this->report_error("Thread subject cannot be empty");
+		} else {
+			$result = filter_var($thread_subject, FILTER_SANITIZE_STRING);
+		}
+
+		return $result;
+	}
+
+	public function validate_thread_category($thread_category): ?int
+	{
+		$result = null;
+
+		if (empty($thread_category)) {
+			$this->report_error("Invalid thread category");
+		} else {
+			$result = filter_var($thread_category, FILTER_SANITIZE_NUMBER_INT);
+		}
+
+		return $result;
+	}
+}
\ No newline at end of file
diff --git a/includes/form/Form.php b/includes/form/Form.php
new file mode 100644
index 0000000..3f1bd62
--- /dev/null
+++ b/includes/form/Form.php
@@ -0,0 +1,45 @@
+<?php
+
+abstract class Form
+{
+	private $errors = array();
+	private $success = true;
+
+	protected function report_error($error_string)
+	{
+		$this->errors[] = $error_string;
+		$this->success = false;
+	}
+
+	public function success(): bool
+	{
+		return $this->success;
+	}
+
+	public function html_error_list(): string
+	{
+		if ($this->success)
+			return "";
+
+		if (count($this->errors) > 1) {
+			$result = '<ul>';
+			foreach ($this->errors as $err) {
+				$result .= '<li>' . $err . '</li>';
+			}
+			$result .= '</ul>';
+			return $result;
+		} else {
+			return $this->errors[0];
+		}
+	}
+
+	public function on_success(Closure $param)
+	{
+		if ($this->success()) {
+			$param();
+	    } else {
+			echo '<p>Please check the following problems:</p>';
+			trigger_error($this->html_error_list());
+		}
+	}
+}
\ No newline at end of file
diff --git a/includes/form/RegisterForm.php b/includes/form/RegisterForm.php
new file mode 100644
index 0000000..4967048
--- /dev/null
+++ b/includes/form/RegisterForm.php
@@ -0,0 +1,56 @@
+<?php
+
+include_once './includes/form/Form.php';
+include_once './includes/model/User.php';
+
+class RegisterForm extends Form
+{
+	const USERNAME_REGEX = "/^[a-zA-Z0-9_]*$/";
+	const PASSWORD_REGEX = "/^[a-zA-Z0-9\W]*$/";
+
+    public function validate_username($username): ?string
+    {
+		$result = null;
+
+    	if (empty($username)) {
+			$this->report_error("You must provide a username.");
+		} else {
+			$result = filter_var($username, FILTER_SANITIZE_STRING);
+
+			if (!preg_match(self::USERNAME_REGEX, $result)) {
+				$this->report_error("Username can only contain letters, numbers and underscores.");
+			}
+
+			if (strlen($result) > 30) {
+				$this->report_error("Your username must be 30 characters or less.");
+			}
+
+			if (User::username_exists($result)) {
+				$this->report_error("The username '" . $result . "' has already been taken by another user.");
+			}
+		}
+    	return $result;
+    }
+
+    public function validate_password($password, $password_check): ?string
+    {
+		$result = null;
+
+    	if (empty($password)) {
+			$this->report_error("You must provide a password.");
+		} else {
+			$result = filter_var($password, FILTER_SANITIZE_STRING);
+			$pass_check = filter_var($password_check, FILTER_SANITIZE_STRING);
+
+			if (preg_match(self::PASSWORD_REGEX, $result) === false) {
+				$this->report_error("Password contains invalid characters!");
+			}
+
+			if ($result !== $pass_check) {
+				$this->report_error("The two passwords do not match.");
+			}
+		}
+
+    	return $result;
+    }
+}
diff --git a/includes/form/SignInForm.php b/includes/form/SignInForm.php
new file mode 100644
index 0000000..3735029
--- /dev/null
+++ b/includes/form/SignInForm.php
@@ -0,0 +1,32 @@
+<?php
+
+include_once './includes/form/Form.php';
+
+class SignInForm extends Form
+{
+	public function validate_username($username): ?string
+	{
+		$result = null;
+
+		if (empty($username)) {
+			$this->report_error('Please provide a username.');
+		} else {
+			$result = filter_var($username, FILTER_SANITIZE_STRING);
+		}
+
+		return $result;
+	}
+
+	public function validate_password($password): ?string
+	{
+		$result = null;
+
+		if (empty($password)) {
+			$this->report_error('Please provide a password.');
+		} else {
+			$result = filter_var($password, FILTER_SANITIZE_STRING);
+		}
+
+		return $result;
+	}
+}
\ No newline at end of file
diff --git a/includes/model/User.php b/includes/model/User.php
index ba475c7..785b847 100755
--- a/includes/model/User.php
+++ b/includes/model/User.php
@@ -56,9 +56,10 @@ class User
 		return $this->has_value;
 	}
 
-	public static function register(string $username, string $pass_hash)
+	public static function register(string $username, string $password)
 	{
 		$sql = "INSERT INTO users(user_name, user_pass, user_date, user_level) VALUES(?, ?, NOW(), 0);";
+		$pass_hash = password_hash($password, PASSWORD_DEFAULT);
 		Database::get()->query($sql, "ss", $username, $pass_hash);
 	}
 
@@ -92,12 +93,12 @@ class User
 
 		return $threads;
 	}
-}
 
-function username_exists(string $username): bool
-{
-	$sql = "SELECT * FROM users WHERE user_name = ?;";
-	$result = Database::get()->query($sql, "s", $username);
+	public static function username_exists(string $username): bool
+	{
+		$sql = "SELECT * FROM users WHERE user_name = ?;";
+		$result = Database::get()->query($sql, "s", $username);
 
-	return !empty($result);
-}
\ No newline at end of file
+		return !empty($result);
+	}
+}
diff --git a/register.php b/register.php
index 5128f10..34e43a0 100755
--- a/register.php
+++ b/register.php
@@ -1,78 +1,38 @@
-<?php session_start() ?>
-<!DOCTYPE html>
-<html lang="en">
-<head>
-	<title>Register an account - cflip.net forum</title>
-<?php include_once 'includes/templates/head.php'; ?>
-</head>
-<body>
-<?php include './includes/templates/header.php' ?>
-	<h2>Register an account</h2>
-	<form action="register.php" method="post">
-		<label for="user_name">Username: </label><br>
-		<input type="text" name="user_name"><br>
-		<label for="user_pass">Password: </label><br>
-		<input type="password" name="user_pass"><br>
-		<label for="user_pass_check">Re-enter password: </label><br>
-		<input type="password" name="user_pass_check"><br>
-		<input type="submit" name="submit">
-	</form>
-	<br>
-<?php
-include_once './includes/model/User.php';
-include_once './includes/error.php';
-
-if ($_SERVER['REQUEST_METHOD'] == 'POST') {
-	$errors = array();
-
-	$user_name = "";
-	$user_pass = "";
-
-	if (empty($_POST['user_name'])) {
-		$errors[] = "You must provide a username.";
-	} else {
-		$user_name = $_POST['user_name'];
-		if (!preg_match("/^[a-zA-Z0-9_]*$/", $user_name)) {
-			$errors[] = "Username can only contain letters, numbers and underscores.";
-		}
-
-		if (strlen($user_name) > 30) {
-			$errors[] = "Your username must be 30 characters or less.";
-		}
-
-		if (username_exists($user_name) !== false) {
-			$errors[] = "The username '" . $user_name . "' has already been taken by another user.";
-		}
-	}
-
-	if (empty($_POST['user_pass'])) {
-		$errors[] = "You must provide a password.";
-	} else {
-		$user_pass = $_POST['user_pass'];
-		$pass_check = $_POST['user_pass_check'];
-
-		if (preg_match("/^[a-zA-Z0-9\W]*$/", $user_pass) === false) {
-			$errors[] = "Password contains invalid characters!";
-		}
-
-		if ($user_pass !== $pass_check) {
-			$errors[] = "The two passwords do not match.";
-		}
-	}
-
-	if (!empty($errors)) {
-		$errstr = 'Please check the following problems: <ul>';
-		foreach ($errors as $err) {
-			$errstr .= '<li>' . $err . '</li>';
-		}
-		$errstr .= '</ul>';
-		trigger_error($errstr);
-	} else {
-		$pass_hash = password_hash($user_pass, PASSWORD_DEFAULT);
-		User::register($user_name, $pass_hash);
-		echo '<p class="success">Account successfully registered! You can now <a href="signin.php">sign in</a></p>';
-	}
-}
-?>
-</body>
-</html>
+<?php session_start() ?>
+<!DOCTYPE html>
+<html lang="en">
+<head>
+	<title>Register an account - cflip.net forum</title>
+<?php include_once 'includes/templates/head.php'; ?>
+</head>
+<body>
+<?php include './includes/templates/header.php' ?>
+	<h2>Register an account</h2>
+	<form action="register.php" method="post">
+		<label for="user_name">Username: </label><br>
+		<input type="text" name="user_name"><br>
+		<label for="user_pass">Password: </label><br>
+		<input type="password" name="user_pass"><br>
+		<label for="user_pass_check">Re-enter password: </label><br>
+		<input type="password" name="user_pass_check"><br>
+		<input type="submit" name="submit">
+	</form>
+	<br>
+<?php
+include_once './includes/form/RegisterForm.php';
+include_once './includes/model/User.php';
+include_once './includes/error.php';
+
+if ($_SERVER['REQUEST_METHOD'] == 'POST') {
+	$form = new RegisterForm();
+	$username = $form->validate_username($_POST['user_pass']);
+	$password = $form->validate_password($_POST['user_pass'], $_POST['user_pass_check']);
+
+    $form->on_success(function () use ($username, $password) {
+        User::register($username, $password);
+        echo '<p class="success">Account successfully registered! You can now <a href="/signin.php">sign in</a></p>';
+    });
+}
+?>
+</body>
+</html>
diff --git a/signin.php b/signin.php
index c2ee5a6..4f76d13 100755
--- a/signin.php
+++ b/signin.php
@@ -16,52 +16,29 @@
 			<input type="submit" name="submit">
 		</form>
 <?php
-include_once 'includes/error.php';
-
-function validate($data): string
-{
-	$data = trim($data);
-	$data = stripslashes($data);
-    return htmlspecialchars($data);
-}
+include_once './includes/form/SignInForm.php';
+include_once './includes/error.php';
 
 if ($_SERVER['REQUEST_METHOD'] == 'POST') {
-	$errors = array();
-
-	if (empty($_POST['user_name'])) {
-		$errors[] = 'Please provide a username.';
-	} else {
-		$user_name = validate($_POST['user_name']);
-	}
-
-	if (empty($_POST['user_pass'])) {
-		$errors[] = 'Please provide a password.';
-	} else {
-		$user_pass = $_POST['user_pass'];
-	}
+    $form = new SignInForm();
+    $username = $form->validate_username($_POST['user_name']);
+	$password = $form->validate_password($_POST['user_pass']);
 
-	if (!empty($errors)) {
-		$errstr = 'Please check the following problems: <ul>';
-		foreach ($errors as $err) {
-			$errstr .= '<li>' . $err . '</li>';
-		}
-		$errstr .= '</ul>';
-		trigger_error($errstr);
-	} else {
-		$user = new User();
-		$user->get_by_name($user_name);
+    $form->on_success(function () use ($username, $password) {
+        $user = new User();
+        $user->get_by_name($username);
 
-		if (!$user->has_value()) {
-			trigger_error('There is no user with that name. Did you mean to <a href="register.php">create a new account?</a>');
-		} else {
-			if (!password_verify($user_pass, $user->password)) {
-				echo 'Password does not match!';
-			} else {
-				Session::get()->sign_in($user);
-				header("Location: index.php");
-			}
-		}
-	}
+        if (!$user->has_value()) {
+            trigger_error('There is no user with that name. Did you mean to <a href="register.php">create a new account?</a>');
+        } else {
+            if (!password_verify($password, $user->password)) {
+                echo 'Password does not match!';
+            } else {
+                Session::get()->sign_in($user);
+                header("Location: /");
+            }
+        }
+    });
 }
 ?>
 	</body>
-- 
cgit v1.2.3