lonnieezell / Bonfire

Jumpstart your CodeIgniter web applications with a modular, HMVC-ready, backend.
http://cibonfire.com
1.39k stars 524 forks source link

Fix bug: requires password enter when update my profile #1279

Open lesydat opened 7 years ago

lesydat commented 7 years ago

When update profile, system requires user enter the password (can't left it empty). This patch to fix this bug

lesydat commented 7 years ago

@lonnieezell Can you review this request?

Reconix commented 7 years ago

Isn't it intentional to make you enter the password twice, even on the update?

silverark commented 7 years ago

@Reconix I think he means that you can't update your profile details without having to also update your password. If you just try to update your display_name only, it shows the error "The Password field does not match the pass_confirm field." even if you left 'password' and 'pass_confirm' both empty.

I came across the same issue.

lonnieezell commented 7 years ago

If we're going to remove the validation rule to check if password and pass_confirm match, then we need to provide some other form of check there to ensure they do indeed match, or we could allow users to block themselves when they accidentally enter a typo for the password field.

Maybe a custom validation rule?

silverark commented 7 years ago

The rule is added in the Users Controller when saving the user on 'Update'.

 // If a value has been entered for the password, pass_confirm is required.
        // Otherwise, the pass_confirm field could be left blank and the form validation
        // would still pass.
        if ($type != 'insert' && $this->input->post('password')) {
            $this->form_validation->set_rules('pass_confirm', 'lang:bf_password_confirm', "required|matches[password]");
        }

Did you want a custom rule in the model instead?

lonnieezell commented 7 years ago

@silverark Oh, you're right. We don't need it in both places and the controller is where it should happen.

I'm all good with this, then. Anyone have any objections merging this? (been a while since I've been deep in the code on this project).

silverark commented 7 years ago

I've just tested it and it's working for me. Works in the admin area's Settings controller too for updating a users password.

The only issue I can see is if you forget to type in the 'password' field, but enter the 'pass_confirm', it would just ignore it, and show "saved" even though it didn't change the password. Putting in the following to pass_confirm validation would stop that happening:

       [
            'field' => 'pass_confirm',
            'label' => 'lang:bf_password_confirm',
            'rules' => 'matches[password]',
        ],

Not sure how to suggest an edit to a pull request.