Gfrktyl

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

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");

  }  

}