sasanrose / phpredmin

Yet another web interface for Redis
BSD 3-Clause "New" or "Revised" License
404 stars 96 forks source link

add basic auth with configurable username and password #69

Closed luongvm closed 7 years ago

luongvm commented 8 years ago

I add the basic authentication with configurable username and password in config.php file.

sasanrose commented 7 years ago

@joturako can you please fix your tab width and commit again. Besides we do not have config.php any more and it is renamed to config.dist.php in 46c11309129accd78de6f168ef1eb15a1c5fd7df

luongvm commented 7 years ago

Ok I'm working on it.

luongvm commented 7 years ago

@sasanrose please check again, I've tried to fix the spaces.

sasanrose commented 7 years ago

@joturako Please see the following diff:

catdiff --git a/config.dist.php b/config.dist.php
index ae63ff6..793f3cd 100644
--- a/config.dist.php
+++ b/config.dist.php
@@ -7,7 +7,7 @@ $config = array(
     'timezone'           => 'Europe/Amsterdam',
     'auth' => array(
        'username' => 'admin',
-        'password' => 'admin'
+        'password' => password_hash('admin', PASSWORD_DEFAULT)
    ),
     'log' => array(
         'driver'    => 'file',
diff --git a/public/index.php b/public/index.php
index e05732f..14a9b72 100644
--- a/public/index.php
+++ b/public/index.php
@@ -16,31 +16,34 @@ function __autoload($class)
     }
     include_once($path.$dir.'/'.(strtolower($class)).'.php');
 }
+
 if (isset(App::instance()->config['timezone'])) {
     date_default_timezone_set(App::instance()->config['timezone']);
 }

-$username = null;
-$password = null;
+$authenticated = true;
+
+if (isset(App::instance()->config['auth'])) {
+   $username = null;
+   $password = null;

-$auth = array(
-    'username' => App::instance()->config['auth']['username'],
-    'password' => App::instance()->config['auth']['password']
-);
-// mod_php
-if (isset($_SERVER['PHP_AUTH_USER'])) {
-    $username = $_SERVER['PHP_AUTH_USER'];
-    $password = $_SERVER['PHP_AUTH_PW'];
-  // most other servers
-} elseif (isset($_SERVER['HTTP_AUTHORIZATION'])) {
-    if (strpos(strtolower($_SERVER['HTTP_AUTHORIZATION']), 'basic') === 0)
-        {
-           list($username, $password) = explode(':', base64_decode(substr($_SERVER['HTTP_AUTHORIZATION'], 6)));
-       }
+   $auth = App::instance()->config['auth'];
+
+   // mod_php
+   if (isset($_SERVER['PHP_AUTH_USER'])) {
+       $username = $_SERVER['PHP_AUTH_USER'];
+       $password = $_SERVER['PHP_AUTH_PW'];
+     // most other servers
+   } elseif (isset($_SERVER['HTTP_AUTHORIZATION']) && strpos(strtolower($_SERVER['HTTP_AUTHORIZATION']), 'basic') === 0) {
+       list($username, $password) = explode(':', base64_decode(substr($_SERVER['HTTP_AUTHORIZATION'], 6)));
    }
-   
-if($username == $auth['username'] && $password == $auth['password'])
-{
+
+   if ($username != $auth['username'] || !password_verify($password, $auth['password'])) {
+       $authenticated = false;
+   }
+}
+
+if ($authenticated) {
     $error = new Error();
     Router::instance()->route();
 } else {
  1. I fixed some coding styles and if blocks
  2. I made 'auth' optional
  3. I used password_hash and password_verify, so people can put the hash generated from their passwords in the config file instead of a plain text password (We should probably do the same thing in the future for other passwords presented the in config file)
luongvm commented 7 years ago

@sasanrose I've updated the code based on your diff. Btw, do you have a guide on coding styles? I let my IDE format it for me so I never know what's appropriate.

sasanrose commented 7 years ago

@joturako No unfortunately. However, maybe we can use something like https://github.com/FriendsOfPHP/PHP-CS-Fixer in future

luongvm commented 7 years ago

@sasanrose I see, also should I write the documentation for setting up this optional authentication?

sasanrose commented 7 years ago

@joturako That would be appreciated