0
\$\begingroup\$

Using cakePHP, I would love to have your point of view about this large source code, i am not sure if its enought secure. sha1() will be removed with another script. I found this large script can be optimized but how ?

i have many times

$this->Session->setFlash("You hav to complete each fiedls", "error");

and

$this->Request->redirect(SITE . "users/account");

I wonder if I can optmize this also:

class UsersController extends Controller {
    function account($Req){
        if(isset($Req->post->login)){
            $login = addslashes($Req->post->login);
            $password = sha1(addslashes($Req->post->password));
            $pass_confirm = sha1(addslashes($Req->post->pass_confirm));
            $email = addslashes($Req->post->email);
            $signature = addslashes($Req->post->signature);

            if(empty($login) || empty($email)){
                $this->Session->setFlash("You hav to complete each fiedls", "error");
                $this->Request->redirect(SITE . "users/account");
            }
            elseif($pass_confirm != $password) {
                $this->Session->setFlash("You gave two differents password", "error");
                $Req->redirect(SITE . "users/account");
            }

            $this->loadModel("Users");

            $dispoLogin = $this->Users->findCount(array(
                "login" => $login
            ));
            if($dispoLogin === 0){
                $this->Session->setFlash("The login is already use by someone else", "error");
                $this->Request->redirect(SITE . "users/account");
            }

            $dispoEmail = $this->Users->findCount(array(
                "email" => $email
            ));
            if($dispoEmail === 0){
                $this->Session->setFlash("Email adress already use by someone else", "error");
                $this->Request->redirect(SITE . "users/account");
            }

            if(empty($password)){
                $q = $this->Users->findFirst(array(
                    "fields" => "password",
                    "conditions" => array(
                        "id" => $this->User->id
                    )
                ));
                $password = sha1($q->password);
            }

            $this->Users->save(array(
                "id" => $this->User->id,
                "login" => $login,
                "password" => $password,
                "email" => $email,
                "signature" => $signature
            )); 
            $this->user->setData(array(
                "login" => $login,
                "password" => $password,
                "email" => $email,
                "signature" => $signature
            ));

            $this->Session->setFlash("Your profile page is updated");
            $this->Request->redirect(SITE);
        }
    }
}
\$\endgroup\$
2
\$\begingroup\$

I answered you on SO but I'll repeat it here too.

Please read the CakePHP Documentation, preferably from the start because you really are getting a lot wrong here.

  • There is no need to addslashes() everything, (or anything ever)
  • CakePHP has it's own AuthComponent, so no need to roll your own
  • It also has a validation engine, so no need to validate anything here
  • You're also passing some Request object to the method? I don't even want to ask...

This action should basically be about 6 lines long. TL;DR: Read the CakePHP Authentication docs, and start again.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you for your answer i really see i have a lake on my knowledge. I'll fix that \$\endgroup\$
    – Zeroth
    Oct 21 '11 at 12:01

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.