3
\$\begingroup\$

I'm just trying to see if anyone disagrees with the way I'm handling my logic for this. Something doesn't feel right with it but I don't quite know what it is.

Just wanted to add that the new_password_key is NOT a password for the user to log in with. As of right now I was going to have them directed to a page from a link in an email where they can enter a new password.

function forgot_password_submit()
{
    $this->form_validation->set_rules('username', 'Username', 'trim|required|xss_clean');

    if (!$this->form_validation->run()) 
    {
        echo json_encode(array('error' => 'yes', 'message' => 'There was a problem submitting the form! Please refresh the window and try again!'));    
    }
    else
    {
        if (!is_null($user_data = $this->users->get_user_by_username($this->input->post('username'))))
        {
            if (!isset($user_data->new_password_key) && (!isset($user_data->new_password_requested)))
            {                
                if(!strtotime($user_data->new_password_requested) <= (time() - 172800))
                {
                    echo json_encode(array('error' => 'yes', 'message' => 'You have to wait 2 days before a new temp password can be emailed!'));
                }
                else
                {
                    if ($this->kow_auth->forgot_password($this->input->post('username')))
                    {
                        $this->kow_auth->send_email('forgot_password', 'KOW Manager Forgot Password Email', $user_data);
                        echo json_encode(array('success' => 'yes', 'message' => 'A temporary password has been emailed to you!'));
                    }
                    else
                    {
                        echo json_encode(array('error' => 'yes', 'message' => 'A temporary password could not be created for you!'));
                    }  
                }
            }
            else
            {
                echo json_encode(array('success' => 'yes', 'message' => 'Check your email for your temporary password!'));
            }
        }
        else
        {
            echo json_encode(array('error' => 'yes', 'message' => 'User does not exist in the database!'));
        }

    }          
}

EDIT: What I'm reallying wondering about how to use the functionality of this setting this temporary new password for when it expires.

Library Function:

/**
 * Generate reset code (to change password) and send it to user
 *
 * @return string
 * @return object
 */
function forgot_password($username)
{
    if (strlen($username) > 0) {
        if (!is_null($user_data = $this->ci->users->get_user_by_username($username)))
        {
            $data = new stdClass;

            $data->user_id = $user_data->user_id;
            $data->username = $user_data->username;
            $data->email = $user_data->email;
            $data->new_password_key = md5(rand().microtime());
            $data->first_name = $user_data->first_name;
            $data->last_name = $user_data->last_name;

            $this->ci->users->set_password_key($user_data->user_id, $data->new_password_key);
            return $data;
        }
    }
    return NULL;

}

Model:

/**
 * Set new password key for user.
 * This key can be used for authentication when resetting user's password.
 *
 * @param   int
 * @param   string
 * @return  bool
 */
function set_password_key($user_id, $new_password_key)
{
    $this->db->set('new_password_key', $new_password_key);
    $this->db->set('new_password_requested', date('Y-m-d H:i:s'));
    $this->db->where('user_id', $user_id);

    $this->db->update('users');
    return $this->db->affected_rows() > 0;
}

EDIT 2:

This is what I'm going to use for the controller. There just seems to be some logic issues I have with it because what if it gets down to the if statement if ($already_sent_password) and for some reason they didn't get it. Then what? Or what if it gets down to if (!strtotime($user_data->new_password_requested) <= (time() - 172800)) which is starting to sounds stupid to me because why make them have to wait two days to get a new password key.

function forgot_password_submit() 
{
    $this->form_validation->set_rules('username', 'Username', 'trim|required|xss_clean');

    if (!$this->form_validation->run()) 
    {
        $this->kow_auth->output('There was a problem submitting the form! Please refresh the window and try again!', FALSE);
        return;
    }

    $user_data = $this->users->get_user_by_username($this->input->post('username'));
    if ($user_data === NULL) 
    {
        $this->kow_auth->output('User does not exist in the database!', FALSE);
        return;
    }

    $already_sent_password = (isset($user_data->new_password_key) && isset($user_data->new_password_requested));
    if ($already_sent_password) 
    {
        $this->kow_auth->output('Check your email for your temporary password!');
        return;
    }

    if (!strtotime($user_data->new_password_requested) <= (time() - 172800)) 
    {
        $this->kow_auth->output('You have to wait 2 days before a new temp password can be emailed!', FALSE);
    } 
    else 
    {
        if ($this->kow_auth->forgot_password($this->input->post('username'))) 
        {
            $this->kow_auth->send_email('forgot_password', 'KOW Manager Forgot Password Email', $user_data);
            $this->kow_auth->output('A temporary password has been emailed to you!');
        } 
        else 
        {
            $this->kow_auth->output('A temporary password could not be created for you!', FALSE);
        }
    }
}

Anybody have any more ideas?

\$\endgroup\$
  • 2
    \$\begingroup\$ They can only ask for a new password every two days? I'd remove the restriction. Otherwise, it becomes fairly trivial to DOS any customers who forget their passwords. I'd probably try to break up this method into a series of smaller methods (I don't know the framework, and am out of practive with php, so I'm a bit fuzzy on exactly what though). And you're not just overwriting it in the database are you? Otherwise, you allow hostile password resets (yes, the customer gets the password, but...). Also, consider caching temp passwords per customer until expired or used. \$\endgroup\$ – Clockwork-Muse Feb 15 '12 at 21:36
1
\$\begingroup\$

I see nothing that's wrong with your code. My first reaction though is that you repeat the echo quite a bit. I would turn all the

echo json_encode(array());

into something like this:

function output($message, $success = TRUE) {
  $status = $success ? array('succes' => 'yes') : array('error' => 'yes');
  echo json_encode($status, $message);
}

The two other things I would have preferred to do differently, is avoiding the deep intendation, and moving some logic out from the if()'s. This is perhaps just a matter of taste, but I find this much easier to read and follow.

function forgot_password_submit() {
    $this->form_validation->set_rules('username', 'Username', 'trim|required|xss_clean');

    if (!$this->form_validation->run()) {
        output('There was a problem submitting the form! Please refresh the window and try again!', FALSE);
        return;
    }

    $user_data = $this->users->get_user_by_username($this->input->post('username'));
    if ($user_data === NULL) {
        output('User does not exist in the database!', FALSE);
        return;
    }

    $already_sent_password = (isset($user_data->new_password_key) && isset($user_data->new_password_requested));
    if ($already_sent_password) {
        output('Check your email for your temporary password!');
        return;
    }

    if (!strtotime($user_data->new_password_requested) <= (time() - 172800)) {
        output('You have to wait 2 days before a new temp password can be emailed!', FALSE);
    } else {
        if ($this->kow_auth->forgot_password($this->input->post('username'))) {
            $this->kow_auth->send_email('forgot_password', 'KOW Manager Forgot Password Email', $user_data);
            output('message' => 'A temporary password has been emailed to you!');
        } else {
            output('A temporary password could not be created for you!', FALSE);
        }
    }
}

function output($message, $success = TRUE) {
  $status = $success ? array('succes' => 'yes') : array('error' => 'yes');
  echo json_encode($status, $message);
}
\$\endgroup\$
  • \$\begingroup\$ This is a great response and something I will definitely use, however, why how can I handle the password issue differently because of a temporary password expiring. I'm going to update my post above with the library function and model. \$\endgroup\$ – Jeff Davidson Feb 15 '12 at 21:51
  • \$\begingroup\$ I updated it. As well as can I move the output function to my library then and send the messages inside the forgot_password_submit function to the the output in the library? \$\endgroup\$ – Jeff Davidson Feb 15 '12 at 21:58
  • \$\begingroup\$ I moved the function to my library because I"m going to use it for other controllers and it says <p>Message: json_encode() expects parameter 2 to be long, string given</p> \$\endgroup\$ – Jeff Davidson Feb 15 '12 at 22:15
  • \$\begingroup\$ Check out EDIT 2. \$\endgroup\$ – Jeff Davidson Feb 15 '12 at 22:50
  • \$\begingroup\$ Anymore ideas on this? \$\endgroup\$ – Jeff Davidson Feb 15 '12 at 23:39
1
\$\begingroup\$

You can browse over some of my code, it may/not help you out but this is generally how I would approach it.

libraries/Webs3_Auth.php

/**
     * create_token
     * generate json object token with expires
     *
     * @param   string $expires     time the token will expire
     * @return  object
     */
    public static function create_token($expires=3600) {
        return json_encode(array(
                    'token' => self::generate_rand_string((int) 9),
                    'expires' => date('h:iA', strtotime('now') + $expires)
                ));
    }

    /**
     *  _generate_rand_string
     * generate a unique string
     *
     * @param   integer the length of the string to return
     * @return  string  unique string
     */
    public static function generate_rand_string($len=8) {
        $random = '';
        $char_list = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ';
        $char_list .= 'abcdefghijklmnopqrstuvwxyz';
        $char_list .= '0123456789~%.:_-'; //numeric + permitted uri chars

        for ($i = 0; $i < $len; $i++) {
            $random .= substr($char_list, (rand() % (strlen($char_list))), 1);
        }
        return $random;
    }

libraries/MY_Encrypt.php need to make the encryptions URI safe

function encode($string, $key="", $url_safe=TRUE)
    {
        $ret = parent::encode($string, $key);

        if ($url_safe)
        {
            $ret = strtr(
                    $ret,
                    array(
                        '+' => '.',
                        '=' => '-',
                        '/' => '~'
                    )
                );
        }

        return $ret;
    }


    function decode($string, $key="")
    {
        $string = strtr(
                $string,
                array(
                    '.' => '+',
                    '-' => '=',
                    '~' => '/'
                )
            );

        return parent::decode($string, $key);
    }

controllers/Auth.php

public function forgot_password_form(){
        $this->load->view('templates/public', array(
            'robots'    =>  'noindex, nofollow',
            'canonical' =>  site_url(),
            'title' =>  'Forgot Password Form',
            'content'   =>  'auth/forgot_password_form'
        ));
    }

public function forgot_password_check(){


        if($this->form_validation->run('forgot_password'))
        {
            $request = '';
            $type= $this->input->post('login');
            if (preg_match('/^.*\@.*$/i', $type) > 0) {
                $request = User::find_by_email($type);
            } else {
                $request = User::find_by_alias($type);
            }

            $token = Webs3_Auth::create_token();
            $token_bits[] = (array)json_decode($token);


            if($request->update_attributes(array(
                'password_token'    =>  $token,
                'last_ip'   =>  $this->input->ip_address()
            )))
            {
                $uid = $this->encrypt->encode($request->id, $this->config->item('encryption_key'), true); //uri safe

                $link = '<a href="'.site_url('reset_password/'.$uid.'/'.$token_bits[0]['token'].'').'" style="background:#76a233;padding:9px;text-align:center;text-decoration:none;border:1px solid #608627;color:#fafafa;display:block;margin-bottom:18px;width:20%;" target="_blank">Reset my password</a>';

                $emaildata = array(
                    'token' =>  $token_bits[0]['token'],
                    'expires'   =>  $token_bits[0]['expires'],
                    'link'  =>  $link
                );
                if(Mailer::sendIt($emaildata, 'forgot_password.html', $request->email, 'Password Reset'))
                {
                    $this->session->set_flashdata('success', 'You got mail! Follow the instructions to reset your password');
                    redirect('/');
                }
                else
                {
                    $this->session->set_flashdata('error', 'Could not send password information to you at this time, contact an admin');
                    redirect('/');
                }
            }
            else
            {
                $this->session->set_flashdata('error', 'A password request could not be made at this time, contact an admin');
                redirect('/');
            }

        }
        else
        {
            $this->forgot_password_form();
        }
    }

public function reset_password_form($uid=null, $token=null){

        $token_secured = FALSE;
        $user_id = $this->encrypt->decode($uid, $this->config->item('encryption_key'));
        $user = User::find($user_id);
        $now = date('h:iA', strtotime('now'));
        $tokens[] = (array)json_decode($user->password_token);

        $token_secured = ($tokens) ? TRUE : FALSE;
        $token_secured = ($tokens[0]['token'] != rawurldecode($token)) ? TRUE : FALSE;
        $token_secured = ($tokens[0]['expires'] >= $now) ? TRUE : FALSE;


        if($token_secured === TRUE)
        {
            $this->load->view('templates/public', array(
                'robots'    =>  'noindex, nofollow',
                'canonical' =>  site_url(),
                'title' =>  'Reset password Form',
                'content'   =>  'auth/reset_password_form',
                'user_id'   =>  $uid
            ));
        }
        else
        {
            $this->session->set_flashdata('info', 'Token is invalid or has expired');
            redirect('/');
        }

    }

    public function reset_password_check(){


        if($this->form_validation->run('reset_password'))
        {
            $user = User::find($this->encrypt->decode($this->input->post('user_id'), $this->config->item('encryption_key')));
            $data = array(
                'password'  =>  Webs3_Auth::prep_password($this->input->post('password')),
                'password_token'    =>  '',
                'last_ip'   =>  $this->input->ip_address()
            );

            if($user->update_attributes($data))
            {

                $this->session->set_flashdata('success', 'Your password has been changed! You may now login with your new password');
                redirect('login');
            }
            else
            {
                $this->session->set_flashdata('error', 'There was an error updating our records, try again shortly!');
                redirect('/');
            }


        }
        else
        {
            $this->reset_password_form();
        }
    }
\$\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.