Skip to content
This repository has been archived by the owner on Jul 14, 2021. It is now read-only.

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

lesydat
Copy link
Contributor

@lesydat lesydat commented Jul 27, 2017

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

@lesydat
Copy link
Contributor Author

lesydat commented Sep 13, 2017

@lonnieezell Can you review this request?

@Reconix
Copy link
Contributor

Reconix commented Sep 27, 2017

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

@silverark
Copy link
Contributor

@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
Copy link
Owner

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
Copy link
Contributor

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
Copy link
Owner

@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
Copy link
Contributor

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants