1
\$\begingroup\$

I'm trying to let users log in with multiple columns (e.g. ID, email, phone). If one fails, it checks the next column.

I would like to minimise my if and else statements. Is there better way of doing it?

$email = $request->only('email', 'password');
        $ssn='';$badgenumber='';$ophone='';$fphone='';$pager='';


        if(preg_match("/^[0-9]+$/",$email['email']) == 1 && strlen($email['email']) < 6){
            $ssn = $request->only('ssn', 'password');
            $badgenumber = $request->only('badgenumber', 'password');
        }

        if(preg_match("/^[0-9]+$/",$email['email']) == 1 && strlen($email['email']) > 6){
            $ophone = $request->only('ophone', 'password');
            $fphone = $request->only('fphone', 'password');
            $pager = $request->only('pager', 'password');
        }

        if(filter_var($email['email'],FILTER_VALIDATE_EMAIL) === false)
            $email ='';

        $error = 1;

        try {
            // verify the $email and create a token for the user

            if ($error == 1 && $email) {
                $error = 0;
                if (!$token = JWTAuth::attempt($email)) {
                    $error = 1;
                }
            }
            if ($error == 1 && $ssn) {
                $error = 0;
                if (!$token = JWTAuth::attempt($ssn)) {
                    $error = 1;
                }
            }
            if ($error == 1 && $badgenumber) {
                $error = 0;
                if (!$token = JWTAuth::attempt($badgenumber)) {
                    $error = 1;
                }
            }
            if ($error == 1 && $ophone) {
                $error = 0;
                if (!$token = JWTAuth::attempt($ophone)) {
                    $error = 1;
                }
            }
            if ($error == 1 && $fphone) {
                $error = 0;
                if (!$token = JWTAuth::attempt($fphone)) {
                    $error = 1;
                }
            }
            if ($error == 1 && $pager) {
                $error = 0;
                if (!$token = JWTAuth::attempt($pager)) {
                    $error = 1;
                }
            }

            if($error == 1){
                return response()->json(['error' => 'invalid_credentials'], 401);
            }

        } catch (JWTException $e) {
            // something went wrong
            return response()->json(['error' => 'could_not_create_token'], 500);
        }
\$\endgroup\$
0
2
\$\begingroup\$

Since this is the backend to a login request, I assume that you expect just 2 inputs, the password and one of the others. So why not use the following:

//gets an array of all inputs: email & pass or ssn & pass or phone & pass...    
$credentials = $request->all(); 

try{
    if(!$token = JWTAuth::attempt($credentials)){
        return response()->json(['error' => 'invalid_credentials'], 401);
    }

} catch (JWTException $e){
    return response()->json(['error' => 'could_not_create_token'], 500);
}

i would like to minimise my if and else what would you guys recommend?

Hint: If you find yourself doing something over and over and over, you probably need a loop. Since this is a login form and that you expect just 1 variable besides the password, don't have so many vars in your script: $email, $ssn, $pager, badgenumber, ophone, fphone, ssn... just use $identifier. If you really need to cycle through all possibilities build an array and loop until you find the one the user submitted:

$possibilities = ['email','ssn','badgenumber','ophone'...];
$identifier = '';
foreach ($possibilities as $i){
    if($request->has($i)){
        $identifier = $request->input($i);
        break; //exit the foreach loop b/c we've found the identifier
    }
}
$credentials = [$identifier, $request->input('password')];

Then you can attempt to create the token with those credentials. Beware I've never used Laravel, but I wrote my answer after going over this helpful page on how to access requests.

Quick sidenote: I notice that you have a check that can be true only if email has length < 6, and another that can be true only if email has length > 6. What if email has length===6 ?

\$\endgroup\$
2
  • \$\begingroup\$ check my question i've made changes thanks to your example :) \$\endgroup\$
    – Jah
    Jun 20 '16 at 15:48
  • \$\begingroup\$ check my answer, this is what i did from your example. \$\endgroup\$
    – Jah
    Jun 20 '16 at 16:12
0
\$\begingroup\$

here is what i've done.

$rules = [
            'email' => 'email',
            'ssn' => 'integer|digits_between:3,4',
            'badgenumber' => 'integer|digits_between:3,4',
            'ophone' => 'integer|digits_between:9,11',
            'fphone' => 'integer|digits_between:9,11',
            'pager' => 'integer|digits_between:9,11',
        ];

        $token ='';
        try {

            foreach($rules as $k => $r){

                $validator = \Validator::make($request->all(), [
                    $k => $r,
                ]);

                if (!$validator->fails() && $request->has($k) && 
                    $token = JWTAuth::attempt($request->only($k, 'password'))) {
                    break;
                }

            }


            if(!$token){
                return response()->json(['error' => 'invalid_credentials'], 401);
            }

        } catch (JWTException $e) {
            // something went wrong
            return response()->json(['error' => 'could_not_create_token'], 500);
        }
\$\endgroup\$
1
  • \$\begingroup\$ I don't think you should create the Validator once for every rule. Why not call Validator::make($request->all(),$rules) one time before the foreach loop? Then you would use if to stop processing a bad form: if($validator->fails()), respond with invalid form input error. Below that you can have the foreach($rules)... without worrying about validation. This way you create the validator and you check its fail-state only once. \$\endgroup\$ Jun 20 '16 at 21:41

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.