1

I had to do a basic login system to protect a page, and I have no access to database so i store the username and password hard coded in php page.

My question is, can this login system hold againts an attack? I need it to hold about 1 month.

Any sugestions to improve will be helpefull. The code is not in laravel, even if it might look like. The username and password, will be changed to something stronger of course.

Thank you in advance.

<?php
class UserController {
private $username;
private $password;
private $isLoggedIn = false;

// Credentials
public function credentials() {
    $credentials = array(
        array(
            "username" => "telekom",
            "password" => "1234"
        ),
        array(
            "username" => "telekom2",
            "password" => "1234"
        )
    );
    return $credentials;
}

// Basic login
public function login() {
    foreach ($this->credentials() as $credential) {
        if ($this->username == $credential['username'] && $this->password == $credential['password']) {
            Session::put('username', $this->username);
            Session::put('password', $this->password);
            $this->isLoggedIn = true;
        }
    }
}

// Get login status
public function isLoggedIn() {
    return $this->isLoggedIn;
}

// Logout
public function logout() {
    // Delete all sessions
    Session::all();
    redirect('/telekom/');
}

// Telekom
public function telekom() {
    $form = new Form();
    if (Input::get('logout') == 1) {
        $this->logout();
    }

    // Post Data from login form
    if (Input::has('username') || Input::has('password')) {
        if (!$form->isCsrfValid()) {
            $form->errors['CSRF'] = "CSRF Token";
        } // CSRF protection is on, comment to disable
        if (empty($form->errors)) {
            $this->username = Input::get('username');
            $this->password = Input::get('password');

            // Check Login
            $this->login();
            if (!$this->isLoggedIn()) {
                Session::put('login', 'Username and password do not match.');
            } else {
                redirect('/telekom/');
            }
        } else {
            Session::put('login', '<p class="color-dark-red"><strong>Errors:</strong></p>
                        <p>' . $form->displayErrors($form->errors) . '</p>');
        }
    // Check if session has username and password 
    } elseif (Session::has('username') && Session::has('password')) {
        $this->username = Session::get('username', false);
        $this->password = Session::get('password', false);
        // Check Login 
        $this->login();
    }
}
}// EOF Class User

// Outside class
$user = new UserController();

// Outside class
if (!$user->isLoggedIn()) {
    // display login form
} else {
    // display protected content    
}
?>
Scott Arciszewski
  • 33,610
  • 16
  • 89
  • 206
George
  • 63
  • 1
  • 8
  • 1
    If PHP ever fails to pre-pocess the page, it can dump the contents of the script... In that case, anyone could see the username/password. Not so much an attack, but something to consider. You could put it in a separate text file (inaccessible from the public) and have php load/parse it. This sounds like a good candidate for web server authentication instead. – Gary Sep 17 '15 at 13:50
  • The class UserController is in a folder includes/controllers/UserController.php with htaccesss set to be accessed only from my ip domain, and is autoloaded by composer, and accessed in a page like /telekom/index.php, if php fails to pre-process do you think that will show pages that are autoloaded by composer in plain text or just the index.php? – George Sep 17 '15 at 14:01
  • If it fails, no includes would be dumped, because those are included by the pre-processing. The only plaintext the user will see is the actual page they visited (however they would see the include paths). But the htaccess I was referring to, is you can have the webserver itself provide basic password authentication. – Gary Sep 17 '15 at 14:17
  • The path in index.php is require_once dirname(__DIR__) . '/includes/init.php'; which is protected by htacess. Order Deny,Allow Deny from all Allow from my ip – George Sep 17 '15 at 14:27
  • That's good. What I was referring to was to potentially [use Apache to authenticate access to index.php instead of PHP](https://wiki.apache.org/httpd/PasswordBasicAuth). You can still do it the way you're doing, but I don't recommend having that information in the same file. If nothing else, you could put those variables in one of your protected includes. – Gary Sep 17 '15 at 14:32
  • The username and password is in UserController.php, the file is protected by htaccess. Thank you for removing some of my concerns. – George Sep 17 '15 at 14:40
  • Oh sorry, I was being a little thick headed; I'm following now. I think your setup is fine. The only other thing I would change is not having the password in memory for so long. – Gary Sep 17 '15 at 14:45

3 Answers3

1

My comments are getting lengthy, so I'll just move them here. I would not recommend you put the username and password in the same file. If PHP ever fails to process the page, it will be dumped as plain text to the user. Even for database connections (where the un/pwd almost have to be stored plain text), most people don't put the information in the same file.

You have a couple options:

  1. Make a separate PHP file that sets your UN/PWD variables, put it somewhere that isn't accessible from outside your server, and include it in index.php. In this case, I wouldn't include the file until right when you're going to compare the variables and let the local scope dump it as soon as possible.

  2. Since this is such basic authentication, you could use Apache's built in password authentication module.

Gary
  • 13,303
  • 18
  • 49
  • 71
  • I avoid the basic authentication from apache because its not pretty :). I have the username and password stored in UserController.php protected with htaccess. If index.php will fail to process it will only show init.php location, init.php has 2 lines of code, one to require userfunctions.php and one composer/autoload.php. – George Sep 17 '15 at 14:49
  • It really is ugly, but it's secure and easy to set up. Anyway, now that I see you're pretty much already doing what I was suggesting, I think you're fine. – Gary Sep 17 '15 at 14:52
0

in my opinion, this solution is safe enough when you don't plan to use it forever. What would I check is setting of your web server - some text editors makes backup copies of edited files, like index.php~, index.php.bkp or so. Make sure whether your web server do not serve those files, if any.

Vrata Blazek
  • 464
  • 3
  • 12
  • The server does not make backups. – George Sep 17 '15 at 13:57
  • Not server but some text editors can do so. And server, when misconfigured, displays source code of your script when i.e. http://yourserver.com/index.php~ address is requested. – Vrata Blazek Sep 17 '15 at 14:00
  • I checked just to be safe, I edited the code with sublime text 3, does not seem to be any backups. I edit the php file locally, then upload to the server. Thank you for your sugestion. – George Sep 17 '15 at 14:04
0

The problem with temporary solutions is that they've never temporary.

Never hard code passwords. Some of the reasons are:

  • It is harder to keep source code secret than it is a key.
  • Any vulnerabilities in your site that allow reading of source code may reveal the password.
  • Passwords from development will end up in production.
  • It is impossible to change passwords without redeploying.
SilverlightFox
  • 32,436
  • 11
  • 76
  • 145