2
\$\begingroup\$

I made a simple authentication system with php for an elm webapp that will have a small number (1 - 5) of users able to log in.

I am planning to send the users their php session-id at login, and keep this in memory while the app is running in order to anthenticate all the requests sent by the app to the server.

Elm allows for single page applications, keeping state between all pages transitions.

All client/server communications will be sent as Json.

I have a few questions:

  • I think it should work even with cookies disabled, since the information needed to start the session in Php will be the stored session id in the POST request sent by Elm. Is this right? And if so how can I make sure php does not set session cookies?
  • Is there any obvious security mistake in my login, signup and logout?

login.php

<?php
include 'utils.php';
session_start();
$id = session_id();

if((getenv('REQUEST_METHOD') == 'POST')) {

  $json_data = file_get_contents("php://input");

  $php_data = json_decode($json_data);

  if (is_null($php_data)){
    logError("json data could not be decoded");
    exit();
  }

  if(!isset($php_data->username) || !isset($php_data->password)){
    logError("wrong input");
    exit();
  }

  $db = mysqli_connect($mysql_server, $mysql_user, $mysql_password, $mysql_db);

  if (mysqli_connect_errno()){
    logError('Could not connect to database');
    exit();
  }

  $stmt  = mysqli_stmt_init($db);

  $getLogInfoQuery = "SELECT password, salt FROM users WHERE name = ?";
  mysqli_stmt_prepare($stmt, $getLogInfoQuery);
  mysqli_stmt_bind_param($stmt,'s', $php_data->username);
  mysqli_stmt_execute($stmt);
  mysqli_stmt_bind_result($stmt, $hashedPass, $salt);

  if (!mysqli_stmt_fetch($stmt)){
    logError("Wrong username/password");
    mysqli_close($db);
    exit();
  }

  if (hash('sha256', $php_data->username.$php_data->password.$salt) !== $hashedPass){
    sleep (2);
    logError("Wrong username/password");
    mysqli_close($db);
    exit();
  }  

  mysqli_close($db);

  $_SESSION['logInfo']['username'] = $php_data->username;

  $result = array('username' => $php_data->username
                 ,'sessionId' => $id
                 );

  $toJson = json_encode($result);
  echo $toJson;
  exit();
} 

elseif((getenv('REQUEST_METHOD') == 'GET') && isset($_SESSION['logInfo']['username'])){
  //check if already logged in

  $result = array('username' => $_SESSION['logInfo']['username']
                 ,'sessionId' => session_id()
                 );

  $toJson = json_encode($result);
  echo $toJson;
  exit();
} 

else {
  $result = array('notLoggedIn' => 'true');
  echo (json_encode($result));
  exit();
}
?>

signup.php

<?php
include 'utils.php';
session_start();
$id = session_id();

if(getenv('REQUEST_METHOD') == 'POST') {

    if (isset($_SESSION['logInfo']['username'])){
          logError("You are already logged in!");
          exit();
    }

    $json_data = file_get_contents("php://input");

    $php_data = json_decode($json_data);

    if (is_null($php_data)){
          logError("json data could not be decoded");
      exit();
    }

    if(!isset($php_data->username)  || !isset($php_data->password)){
            logError("wrong input");
            exit();
        }

    $db = mysqli_connect($mysql_server, $mysql_user, $mysql_password, $mysql_db);

    if (mysqli_connect_errno()){
        logError('Could not connect to database');
    exit();
        }

    $username = $php_data->username;
    $password = $php_data->password;

    $salt = md5(uniqid(mt_rand(), true));
    $hash = hash('sha256', $username.$password.$salt);
    $ip   = $_SERVER['REMOTE_ADDR'];

    $stmt  = mysqli_stmt_init($db);

    $query = "SELECT name FROM users WHERE name = ?";
    mysqli_stmt_prepare($stmt, $query);
    mysqli_stmt_bind_param($stmt,'s', $username);
    mysqli_stmt_execute($stmt);

    if (mysqli_stmt_fetch($stmt)){
      logError("This username is already in use");
      mysqli_close($db);
      exit();
    }

    $query = "INSERT INTO users(name, password, salt, ip) VALUES (?, ?, ?, ?)";

    mysqli_stmt_prepare($stmt, $query);
    mysqli_stmt_bind_param($stmt,'ssss',$username, $hash, $salt, $ip);
    mysqli_stmt_execute($stmt);

    if (mysqli_stmt_affected_rows($stmt) == 0){
      logError("Data was not inserted into database");
      mysqli_close($db);
      exit();
    }

    $result = array('signUpComplete' => true);
    echo (json_encode($result));


    mysqli_close($db);
    exit();

}

logout.php

<?php
include 'utils.php';
session_start();
session_unset();
session_destroy(); 

$result = array('notLoggedIn' => 'true');
echo (json_encode($result));
exit();

?>

And here is an example of the way I plan to use it:

<?php
include 'utils.php';

if(getenv('REQUEST_METHOD') == 'POST') {
    $json_data = file_get_contents("php://input");

    $php_data = json_decode($json_data);

    if (is_null($php_data)){
    logError("json data could not be decoded");
    exit();
    }

    if(!isset($php_data->sessionId)){
       logError("wrong input");
    exit();
    }

    session_id($php_data->sessionId);
    session_start();

    if (!isset($_SESSION['logInfo']['username'])){
      logError("wrong credentials");
      exit();
    }

   # Do some stuff requiring valid credentials...

   exit();

} else {
  logError("invalid request");
}

?>

Here is a quick draft of the elm side. I removed the view code for conciseness. I also didn't do any of the error handling yet.

module Auth exposing (..)

import Http exposing (..)
import Json.Decode as Decode exposing (..)
import Json.Encode as Encode exposing (..)


type LogInfo
    = LoggedIn
        { username : String
        , sessionId : String
        }
    | LoggedOut


type alias Model =
    { username : String
    , password : String
    , logInfo : LogInfo
    , signUpComplete : Bool
    , displayMode : DisplayMode
    , files : List String
    }


type DisplayMode
    = DisplaySignUp
    | DisplayLogin


type Msg
    = SetUsername String
    | SetPassword String
    | Login
    | SignUp
    | Logout
    | ChangeDisplayMode DisplayMode
    | GetFiles
    | SetFiles (Result Http.Error (List String))
    | ConfirmSignUp (Result Http.Error Bool)
    | ProcessAuthMsg (Result Http.Error LogInfo)


update msg model =
    case msg of
        SetUsername s ->
            ( { model | username = s }
            , Cmd.none
            )

        SetPassword s ->
            ( { model | password = s }
            , Cmd.none
            )

        Login ->
            ( model
            , login model
            )

        SignUp ->
            ( model
            , signUp model
            )

        Logout ->
            ( model
            , logout
            )

        ChangeDisplayMode mode ->
            ( { model | displayMode = mode }
            , Cmd.none
            )

        GetFiles ->
            ( model
            , case model.logInfo of
                LoggedOut ->
                    Cmd.none

                LoggedIn { sessionId } ->
                    getFiles sessionId
            )

        SetFiles res ->
            case res of
                Err _ ->
                    ( model, Cmd.none )

                Ok files ->
                    ( { model | files = files }, Cmd.none )

        ConfirmSignUp res ->
            case res of
                Err _ ->
                    ( model, Cmd.none )

                Ok _ ->
                    ( { model | signUpComplete = True }
                    , Cmd.none
                    )

        ProcessAuthMsg res ->
            case res of
                Err _ ->
                    ( model, Cmd.none )

                Ok logInfo ->
                    ( { model | logInfo = logInfo }
                    , Cmd.none
                    )


login : Model -> Cmd Msg
login model =
    let
        body =
            Encode.object
                [ ( "username"
                  , Encode.string (.username model)
                  )
                , ( "password"
                  , Encode.string (.password model)
                  )
                ]
                |> Http.jsonBody

        request =
            Http.post "login.php" body decodeLoginResult
    in
    Http.send ProcessAuthMsg request


decodeLoginResult : Decoder LogInfo
decodeLoginResult =
    Decode.map2 (\a b -> LoggedIn { username = a, sessionId = b })
        (Decode.field "username" Decode.string)
        (Decode.field "sessionId" Decode.string)


signUp : Model -> Cmd Msg
signUp model =
    let
        body =
            Encode.object
                [ ( "username"
                  , Encode.string (.username model)
                  )
                , ( "password"
                  , Encode.string (.password model)
                  )
                ]
                |> Http.jsonBody

        request =
            Http.post "signup.php" body decodeSignupResult
    in
    Http.send ConfirmSignUp request


logout : Cmd Msg
logout =
    --let
    --    request =
    --        Http.get (domainAdr "logout.php") decodeRes
    --in
    --Http.send ProcessHttpResult request
    Debug.todo ""


decodeSignupResult =
    Decode.field "signUpComplete" Decode.bool


getFiles : String -> Cmd Msg
getFiles sessionId =
    let
        body =
            Encode.object
                [ ( "sessionId"
                  , Encode.string sessionId
                  )
                ]
                |> Http.jsonBody

        request =
            Http.post "getFiles.php" body decodeFiles
    in
    Http.send SetFiles request


decodeFiles =
    Decode.list Decode.string
\$\endgroup\$
  • \$\begingroup\$ I added some code for the elm side. This is an early draft though. My main concern is more about security holes in my approach server side. \$\endgroup\$ – eniac314 Oct 4 '18 at 19:37
  • \$\begingroup\$ "How can I make sure php does not set session cookies?" That part is an excellent, concrete and widely applicable question you should cross-post to Stackoverflow. \$\endgroup\$ – Thilo Oct 5 '18 at 0:00
  • \$\begingroup\$ :tongue-in-cheek: What about non-obvious errors? :) \$\endgroup\$ – 23fc9a62-56de-47fb-97b4-737890 Oct 6 '18 at 1:16
  • \$\begingroup\$ Well, I'll of course welcome any comments about non obvious errors as well :) \$\endgroup\$ – eniac314 Oct 6 '18 at 6:36
1
\$\begingroup\$

I know little to nothing about Elm, but speaking of PHP your code is awfully duplicated. Sometimes it duplicates itself, either across different files or in the same file and also it duplicates the functionality already exists in PHP. For example, PHP can log errors for you, and not a single line of code have to be written for that.

Processing input

  $json_data = file_get_contents("php://input");
  $php_data = json_decode($json_data);
  if (is_null($php_data)){
    logError("json data could not be decoded");
    exit();
  }
  if(!isset($php_data->username)  || !isset($php_data->password)){
        logError("wrong input");
        exit();
  }

This block of code is repeated across multiple files. So it's better to make a function from it and put it in utils.php

Besides, a "json data could not be decoded" error message is not very informative, whereas PHP can give you a more detailed account. So, to decode json is better to create a function on its own,

function jsonDecode($json, $assoc = false)
{
    $ret = json_decode($json, $assoc);
    if ($error = json_last_error())
    {
        throw new \Exception(json_last_error_msg(), $error);
    }
    return $ret;
}

Besides, as I just said above, PHP can log errors for you. With two simple php.ini directives, log_errors and error_log you can tell it to log all errors and where they should be stored. So, no need to log an error manually - just raise it, and PHP will do the rest.

So let's create a function that takes JSON from input, decodes it, and, optionally, checks for the required data:

function get_json_from_input ($required_fields = [])
{
  $json_data = file_get_contents("php://input");
  $php_data = jsonDecode($json_data);
  foreach ($required_fields as $var)
  if(!isset($php_data->$var) {
      throw new \Exception("wrong input");
  }
  return $json_data;
}

just put these two functions in utils.php and the amount of code in your files will be reduced considerably.

Connecting to the database.

The connection code is also duplicated across the files so it should be moved elsewhere. Here I have a small article that explains How to properly connect to Mysql database using mysqli. Just take the code, put it in a file, and include it in the every script that requires a database connection.
Note that there are many improvements in the connection code as well, for example, you don't set the connection encoding, which will result in the broken data in the database.

And of course mysqli will start to log its errors by itself as well!

Performing queries.

Surely you already noticed that every query takes you to write a lot of repeated commands. A function is to help here as well. I've got an example mysqli helper function which could perfectly fit here. So instead of this wall of code

  $stmt  = mysqli_stmt_init($db);

  $getLogInfoQuery = "SELECT password, salt FROM users WHERE name = ?";
  mysqli_stmt_prepare($stmt, $getLogInfoQuery);
  mysqli_stmt_bind_param($stmt,'s', $php_data->username);
  mysqli_stmt_execute($stmt);
  mysqli_stmt_bind_result($stmt, $hashedPass, $salt);

  if (!mysqli_stmt_fetch($stmt)){
    logError("Wrong username/password");
    mysqli_close($db);
    exit();
  }

you would have to write just two lines!

$sql = "SELECT password, salt FROM users WHERE name = ?";
$user_data = mysqli($db, $sql, [$php_data->username])->get_result->fetch_assoc();

The final refactoring

A quite important thing: you should really use the PHP's internal password_hash() function instead of anything else. So I would change your code to use the proper kind of hash:

<?php
include 'utils.php';
include 'db.php';
session_start();
$id = session_id();

if((getenv('REQUEST_METHOD') == 'POST') {

  $json_data = get_json_from_input(['username','password']);

  $sql = "SELECT password FROM users WHERE name = ?";
  $user = mysqli($db, $sql, [$php_data->username])->get_result->fetch_assoc();

  if ($user && password_verify($php_data->password, $user['password']))
  {
      $_SESSION['logInfo']['username'] = $php_data->username;
      $result = array('username' => $php_data->username
                 ,'sessionId' => $id
                 );
      echo json_encode($result);
  } else {
      sleep (2);
      throw new \Exception("Wrong username/password");
  }  
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.