patternphpMinor
Admin section - Secure login and authentication
Viewed 0 times
secureadminsectionloginauthenticationand
Problem
After a lot of back and forth on various sites, reading articles, watching videos etc i still can not figure out the best way to secure my admin section.
The
Let me know in the comments if you need any more details.
My Goal:
The following code attempts to deal with authenticating the user / allowing use of the various admin pages / functions if successfully logged in, what am i missing?
Config.php
Index.php
Login.php
```
WebCodex - Please Login
WCX LOGIN
">
'.$_SESSION['errors'].'';
// CLEAR THE SESSION ERRORS AFTER DISPLAYING THEM
$_SESSION['errors'] = '';
}
?>
queryIt($query);
$stmt = $backend->bind(':username', $username);
$stmt = $backend->execute();
$dbpass = $backend->getColumn();
// VERIFY USER INPUTTED PASSWORD WITH DB PASSWORD USING PHP FUNCTION password_verify()
if(password_verify($password, $dbpass)) {
// UPDATE ACTIVE AND LASTLOGIN WHERE USERNAME =
The
session id is regenerated every page reload / action to make session hijacking / fixation more difficult.Let me know in the comments if you need any more details.
My Goal:
- Securely log the user in
- Set a session / cookie
- Authenticate the user
- Do this without
HTTPS
The following code attempts to deal with authenticating the user / allowing use of the various admin pages / functions if successfully logged in, what am i missing?
Config.php
Index.php
isLoggedIn()===true) {
if(isset($_GET['page']) && !empty($_GET['page'])) {
$page = filter_input(INPUT_GET, 'page', FILTER_SANITIZE_STRING);
}
else {
$page = 'dashboard';
}
include_once('includes/header.php');
if(in_array($page, $pagearray, TRUE) && file_exists('includes/'.$page.'.php')) {
include_once('includes/'.$page.'.php');
}
else {
include_once('includes/404.php');
}
include_once('includes/footer.php');
}
else {
include_once('login.php');
}
?>Login.php
```
WebCodex - Please Login
WCX LOGIN
">
'.$_SESSION['errors'].'';
// CLEAR THE SESSION ERRORS AFTER DISPLAYING THEM
$_SESSION['errors'] = '';
}
?>
queryIt($query);
$stmt = $backend->bind(':username', $username);
$stmt = $backend->execute();
$dbpass = $backend->getColumn();
// VERIFY USER INPUTTED PASSWORD WITH DB PASSWORD USING PHP FUNCTION password_verify()
if(password_verify($password, $dbpass)) {
// UPDATE ACTIVE AND LASTLOGIN WHERE USERNAME =
Solution
Security
You have a lot of things covered. You use prepared statements and only include files that are defined in a white-list, that's good. I just have these smaller points:
(I didn't find anything else, but that doesn't mean that there is nothing else to find.)
Apart from the security aspects, your code seems fine. Sometimes, the indentation is a bit of, and your HTML doesn't validate 100%, but I didn't see any major problems.
You have a lot of things covered. You use prepared statements and only include files that are defined in a white-list, that's good. I just have these smaller points:
- don't store your password in the php source file, but in a configuration file (outside the web root). It's not too dangerous, but it's better to be save.
action=": this would be vulnerable to XSS attacks if it were working. As you are not echoing, the action will always be"". Either set the action to a hard-coded"", or echo an escaped php self. Do not leave it as it is, in case someone will "fix" it.
- you are not filtering
$_SESSION['errors']when outputting it to the user. Right now, this is not a problem (it only contains a hard-coded string). But if you at some point change your code that this does depend on user input, this will cause problems (for example:You tried to access /cool-site/alert('xss');. We are sorry, but you do not have the right to access it. I would just filter it now to be sure.
- enable
HttpOnlycookies (for your cookie as well as the session cookie). I would do this in the PHP code as well as in the server settings, because you newer know if others remember to set the server settings. This mitigates the risks of XSS (although there are still risks).
Secure loginandDo this without HTTPS: these do not go together. If you want to prevent man in the middle attacks, you will need HTTPS.
(I didn't find anything else, but that doesn't mean that there is nothing else to find.)
Apart from the security aspects, your code seems fine. Sometimes, the indentation is a bit of, and your HTML doesn't validate 100%, but I didn't see any major problems.
Context
StackExchange Code Review Q#61101, answer score: 3
Revisions (0)
No revisions yet.