First of all, your code looks rock-solid, obvious in what it does and very clean. This is not something that I would expect from someone "new in development". Whoever you learned all this from, you should keep them. You got all of the major topics of a login system right: * Your code uses parameterized statements for all database access, to protect against SQL injections. * Your code saves passwords only in their hashed form, to protect against data breaches. * You use a decent password hashing algorithm. Good that PHP provides a sane default here and makes it easy to use. Throughout its long history, PHP had not always made it easy to write correct and secure code. For password hashing it succeeded to provide a good API, and you use it perfectly as well. * You use the full available character set from Unicode by connecting to the MySQL database using the `utf8mb4` encoding. This allows emojis in user names 🙂 to be saved correctly. * All HTML that is printed carefully avoids to reflect any user-provided data. This prevents cross-site scripting. Next, I'm looking at the details of the code, from top to bottom, see if there are any unnecessary stylistic variations. ### database.php For me, the comment on the `DatabaseConnection` is redundant. The name of the class already expresses clearly that the DatabaseConnection connects to the database. Therefore I would not need that comment. Since you are still new in development, it's perfectly fine to keep that comment until you (and whoever else works on your project) don't need it anymore because it's obvious. Same for the `will be inherited` part in the comment. My IDE lets me quickly see the type hierarchy of classes, which makes this comment redundant as well. Plus, `will be` refers to the future and is probably outdated by now. The constructor of `DatabaseConnection` initializes the field `$this->database`, but when looking at the class `DatabaseConnection` alone, that field is not used anywhere. This feels strange at first. I'm somehow missing the `private $database` declaration, like you wrote it in the class `SignUp`. Still in the constructor of `DatabaseConnection`, you wrote `try{` without a space in between. It is more common to write `try {` with a space in between, just like there is a space after `if` and `catch`. If you are using an IDE, it has a command to "format the source code", so you don't have to do that yourself. There are many different styles of formatting the code, it doesn't matter which one you choose. Just choose one that you like and apply it _consistently_ to all of the code. Still in the constructor of `DatabaseConnection`, when I first saw the code, I didn't see a reason for the `try catch` block at all. If you remove that block and only keep the single line `$this->database = ...` instead, the original stack trace will be preserved, including all information about the actual cause of the exception. By wrapping the `PDOException` in a plain `Exception` and only copying the message, you give away this context information. So far for my first impression. The original stack trace contains a lot of information that is useful for tracking down errors. However, it also contains the database credentials, and these must not end up in the log files of the server. Therefore stripping the exception from all unnecessary information is essential in this case. To alert future readers of the code, I would add a comment explaining this in a single line, or just referring to a Stack Overflow answer with all the details. ### registration.php The comment `This is a registration script` is redundant. Keeping the author information is good if you plan on passing the project to someone else later, so that they have someone to contact. Whether or not the `copyright 2021` is necessary or helpful is something I cannot decide for you. Declaring `strict_types=1` is good style, you should add it to `database.php` as well. The class `SignUp` has no comment above it. This leaves a reader a bit unclear about what a `SignUp` really is. It might be "all data that is collected on the sign-up page", or it might be "the data that is actually required to perform a sign-up operation". The difference between these two is that the former would include the password confirmation field, but the latter would not. You declare `SignUp extends DatabaseConnection`, which sounds strange. It's not that "every sign-up is a specialized database connection", but that's what the keyword `extends` usually means. Instead, it's more likely that the class `SignUpPage` would _use_ a database connection. Therefore I would drop the `extends DatabaseConnection` from the class `SignUp`. It's a nice trick that by declaring `private $database` in the class `SignUp`, you make that previously undeclared field of the class `DatabaseConnection` visible. Having tricks like these in the code may be confusing for readers, or they may even expect them. Instead of this trick, I would rather declare a field `private $database` of type `DatabaseConnection` and initialize this field in the constructor. I like the field name `$passwordHash` since it makes it obvious that the password is never stored in its plain text form. I hope that you use that field in exactly this way, I haven't looked at the actual code yet. But even if at some place there were some code that read `$passwordHash = $_POST['password']`, this would immediately alert any careful reader that something is wrong here, which is a good second-line defense. I'm not sure what the purpose of the field `$signUpPage` is. It could be a complete URL like `https://localhost:3000/signup`, it could be the HTML that makes up the sign-up page, or it could be the simple word `signup`, as in your code. A more precise name would be `$relativeSignUpPageUrl`, but that name is rather long, so it's understandable you chose a shorter name. In that case it might help the reader to add a small comment describing an example value for the variable. From this example value it is usually simple enough to infer all allowed values. The function `setName` has the comment `Assign the username`. First, when I let my IDE render that comment, it looks like this: [![Screenshot of rendered comment][1]][1] The word `Assign` is listed in the section of the parameter `$name` while it really describes the function, not the parameter. The comment should rather be: ~~~php /** * Assign the username * @param string $name */ ~~~ If you were to rename the function to `setUsername`, each word of the comment would be redundant, allowing you to leave out the comment completely. In the function `setPassword`, the comment contains the words `& $confirm`. The `&` should be written as `&` since the comments contain HTML, just like [Javadoc](https://www.oracle.com/de/technical-resources/articles/java/javadoc-tool.html) (different programming language, same idea). When you search for `@param` in that document, you will find in match 16/36 that each parameter has a `@param` of its own. It's the same for PHP. In the comment for `signUpPage`, I don't see a need to write `Page` with a capital P. Same for `returnWithError`. The comment for `existingName` says "Checks whether a name exists". The return type is `void`, which is unexpected to me. I would have expected `bool` here since that's the correct type for "whether". Instead, the function exits if the name does not exist. Therefore I would rename that function to `checkNameExists`. Function names starting with `check` usually behave this way, throwing either an exception on failure or exiting plainly. In the function `ValidateNameLength` you started the name with a capital `V`, that is unusual. The other function names start with a lowercase letter. In `ValidateNameLength` you check the length of the name using `mb_strlen`. This is very careful, many other programmers would have used the wrong `strlen` instead. In `addUser`, you use a `\w` in the regular expression. The [documentation for the \w](https://www.php.net/manual/en/regexp.reference.escape.php) was a bit hard to find since it is not directly referenced in the documentation of `preg_match`, but anyways. The `\w` includes all "word" characters, even outside `[A-Za-z]`. This may include Cyrillic, Hangul and several other alphabets. If you want to avoid usernames that look equal but are not (the Latin `a` and the Cyrillic `а`), you should rather restrict the allowed characters to `[A-Za-z]` first. You can always extend the character set later, restricting it is more work since it might affect existing usernames. Still in `addUser`, the SQL statement currently depends on the order of the column names in the database. If the order of the columns should change in the future, your code will put the data in the wrong columns. To prevent this, you can list the column names explicitly, as in `INSERT INTO users(name, email, password_hash, remote_addr, client_ip, forwarded_for, user_agent)`. This change, of course, will make your code dependent on the _names_ of the columns instead of their position. Choose either way, I'm not sure which change is more likely to happen. ### main.php The call to `new SignUp` with the database connection parameters looks strange. This is because a `Page` object should not need to know these connection parameters as individual values, it should rather only have a single parameter of type `DatabaseConnection`. Assume that the database connection needs more more parameter, for whatever reason. Then you would have to adjust the call to `new SignUp`, and that feels wrong because a page should be independent from this low-level change. ### Summary Even after these many detailed remarks, the code still looks very solid to me. The remarks can be fixed easily, and the overall structure of the code is very nice and easy to understand. Very well done. And to answer your initial main question: Yes, your code is secure. All user-generated data is only used in safe ways, without allowing any injection attacks. [1]: https://i.sstatic.net/02r2M.png