0

I have a login page with two radio button(buyer or seller)

For example, I chose buyer the I will get a login field

Mobile
Password

Or, I chose seller the I will get a login field

Email
Password

I am using the below function for the login code.

function login($pdo){
  $account_type=sanitize_data($_POST['account_type']);
  $password =sanitize_data($_POST['password']);
 

 if (empty($account_type)) {
   $errorMsg=  "Please select account type";
   $code= "1" ;

 }
  elseif ($account_type==1) {
        $mobileno=sanitize_data($_POST['mobileno']);

        if(empty($mobileno)) {
            $errorMsg=  "Please enter mobile number.";
            $code= "2";
          }
          elseif(is_numeric(trim($mobileno)) == false){
            $errorMsg=  "Please enter numeric value.";
            $code= "2";
          }elseif(strlen($mobileno)<10){
            $errorMsg=  "Number should be ten digits.";
            $code= "2";
          }
          else{
            // echo "<pre>";
            echo "4";// getting issue here


          }

  }
  elseif ($account_type==2) {

        if(empty($email)){
            $errorMsg="You did not enter a email.";
            $code="2";
          } //check for valid email 
        elseif(filter_var($email, FILTER_VALIDATE_EMAIL) === false){
           $errorMsg= "You did not enter a valid email.";
            $code="2";
          }
          else{

          }

  }

elseif( empty($password)) {
    $errorMsg= "Please enter the password";
    $code="3";
  }

else{
        try{ 
              //query here
        
         } catch(PDOExecption $e) { 
                $dbh->rollback(); 
                print "Error!: " . $e->getMessage() . "</br>"; 
            } 

   } 

}

Now, I am getting issues in nested if condition. For example, I choose buyer and I added 10 digits mobile number and submitted the form.

According to my code, it should check the server side validation for the field like the password is entered or not. right?

But it stops. I mean if the server-side validation is clear for mobile then it should check the next one but it's not checking.

I mean it's not checking the password. I think my execution is stopped once reach the else part.

elseif ($account_type==1) {
        $mobileno=sanitize_data($_POST['mobileno']);

        if(empty($mobileno)) {
            $errorMsg=  "Please enter mobile number.";
            $code= "2";
          }
          elseif(is_numeric(trim($mobileno)) == false){
            $errorMsg=  "Please enter numeric value.";
            $code= "2";
          }elseif(strlen($mobileno)<10){
            $errorMsg=  "Number should be ten digits.";
            $code= "2";
          }
          else{
            // echo "<pre>";
            echo "4"; // getting issue here

          }

  }

are the right way to use the code for login and server-side validation?

user9437856
  • 2,360
  • 2
  • 33
  • 92

1 Answers1

0

Actually, it's the outer clause that causes your issues. It reads like this:

if (empty($account_type)) {

    // We don't have an account type.
}
elseif ($account_type==1) {

    // Account type is '1'. Proceed in here. 
}
elseif ($account_type==2) {

    // Account type is '2'.  
}
elseif(empty($password)) {
    
    // We don't have a password.
}
else {

    // We have a password and an account type,
    // and it's neither 1 nor 2.
     
    try { 
        
        ...

    } catch(PDOExecption $e) { 
        
        ...
    } 
}

I'd suggest splitting your if chains - to be able to do this, though, you might also need to re-structure your sanitation, validation and error handling.

Wrapping your logic in a class might be worth a thought, for example:

// Mock db 
class DbConnection {}

class Login {

    /**
     * @var Array
     */
    protected $error;

    /**
     * @var Array
     */
    protected $data;

    /**
     * @var DbConnection
     */
    protected $db;

    /**
     * Mock sanitizing
     */
    protected function sanitize(array $data): array {

        return $data;
    }

    protected function validate(): bool {

        // In our sanitize() method, we already made sure
        // that everything we need is set, so we don't have
        // to do it here.
        $account_type = $this->data['account_type'];
        $password     = $this->data['password'];

        // Return early if we don't have an account type or 
        // a password.
        
        if (!$account_type) {

            $this->error = [1, 'Please select account type'];
            return false;
        }

        if(!$password) {
        
            $this->error = [3, 'Please provide a password'];
            return false;
        }

        if ($account_type == 1) {

            $mobileno = $this->data['mobileno'];

            // We might already have stripped everything that's not a number
            // from our mobile number string when sanitizing it, so we already
            // made sure it's either empty or numeric.
            // 
            // To validate it, we could either use a regex, or one of the PHP
            // ports of Google's libphonenumber library:
            // - https://stackoverflow.com/questions/123559/how-to-validate-phone-numbers-using-regex
            // - https://stackoverflow.com/questions/22378736/regex-for-mobile-number-validation/
            // - https://github.com/giggsey/libphonenumber-for-php    
            // 
            // Let's assume we already used one of those methods, so the value of
            // $mobileno would be either valid or false.
            
            if (!$mobileno) {

                $this->error = [2, 'Please enter a valid mobile number'];
                return false;
            }

            return true;
        } 

        if ($account_type==2) {

            // Some validation logic. If nothing fails, we'll finally return true.  
             
            return true;
        }
    }

    /**
     * @return Mixed - Boolean|User (object, array...)
     */
    public function login()  {

        if (!$this->validate()) {

            return false;
        } 

        $password = $this->data['password'];

        try { 
              
            // Query db for password (and maybe a username, too)
            return ['User' => '...'];
        
         } 
         catch(PDOExecption $e) { 
         
            // Exception handling; 
        }
    }

    public function getError() {

        return $this->error;
    }

    public function __construct(array $data, DbConnection $db) {

        $this->data = $this->sanitize($data);
        $this->db   = $db;
    } 
}

You're login process would then be:

// Mock POST data

$_POST = [

    'account_type' => '1',
    'password'     => 'PwG2c?4tyUzEtD!9',
    'mobileno'     => '012345678986'
];

// Evaluate login attempt

$attempt = new Login($_POST, new DbConnection());
$user    = $attempt->login();

if (!$user) {

    var_dump($attempt->getError());
}
else {

    var_dump($user);
}

Sources used:

nosurs
  • 680
  • 6
  • 13
  • Thanks for the answer, I can't use the class as my project is already done. and now I am getting issues only on the login code. Can you help me with my code? – user9437856 Apr 08 '21 at 03:21
  • You don't have to define a class, but you do have to re-structure your code - ideally by separating sanitation, validation, and error-handling. I understand that it's "only the login code" you're having issues with, but you have those issues because of those messy `if-elseif`-chains. Like I said in my answer, you're currently query the database only if you have an _invalid_ account type (neither '1' nor '2'). Have a look at those two class methods handling login & data validation. They're doing what you want to do - splitting up the logic, thus breaking apart the `if`-chains. – nosurs Apr 08 '21 at 11:00
  • For example and just to get the hang of it, define some `$errors` variable inside your login function. Make it an array, and push any errors you encounter to it. Make separate `if`s for `empty($account_type)` and `empty($password)`. Make separate `if`s for investigating both of your account types. Make another `if` for handling `$account_type != 1 && $account_type != 2`. Query your db only if `$errors` is empty. – nosurs Apr 08 '21 at 11:09