Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

ModulePersonalData: setNewPassword hook triggered even when password is not valid #5247

Closed
aschempp opened this Issue · 14 comments

2 participants

@aschempp
Collaborator

In ModulePersonalData, we are triggering the save_callback before validating input. If the field is a password field, we'll trigger setNewPassword hook afterwards.

However, in tl_member there is a save_callback on the password field which will also trigger the hook.
The solution would be to check for TL_MODE in the save_callback.

@aschempp
Collaborator

For Contao 2.11 and 3.0, ModulePersonalData on line 188 should be adjusted like this:

if (!$objWidget->hasErrors() && is_array($arrData['save_callback']))

In Contao 3.1, it should be adjusted so the save_callback is only triggered when $objWidget->submitInput() is given (see DataContainer and DC_Table classes).

@aschempp
Collaborator

Also, I just noticed that our solution still does trigger the hook twice. We need to either not trigger in save_callback when TL_MODE is FE, or remove it from ModulePersonalData.

@leofeyer
Owner

Also, I just noticed that our solution still does trigger the hook twice

Do you mean the 2.11 or the 3.0 solution?

@aschempp
Collaborator

The 2.11 solution. Because the hook will be triggered and the save_callback. So we should still check TL_MODE

@aschempp
Collaborator

hmm, actually both solutions. Except if we remove the hook in ModulePersonalData... but not sure if the parameters are identical?

@aschempp
Collaborator

for 2.11 and 3.0, change the save_callback on tl_member.password

if (TL_MODE == 'BE')
...
@leofeyer
Owner

Like this?

public function setNewPassword($strPassword, $user)
{
    // Return if there is no user (e.g. upon registration)
    if (TL_MODE != 'FE' || !$user)
    {
        return $strPassword;
    }
@aschempp
Collaborator

Correct Boss

@leofeyer
Owner

Fixed in 028b51a.

@leofeyer leofeyer closed this
@leofeyer
Owner

Can you post the code changes you have in mind for Contao 3 as well please?

@aschempp
Collaborator

The most simple change would be

if (!$objWidget->hasErrors() && is_array($arrData['save_callback']))

to

if (is_array($arrData['save_callback']) && !$objWidget->hasErrors() && $objWidget->submitInput())

@leofeyer leofeyer reopened this
@aschempp
Collaborator

Again, this should be like this (Contao 2.11):

if (TL_MODE == 'FE' || !$user)
{
    return $strPassword;
}

In the frontend, the hook is already triggered here: https://github.com/contao/core/blob/lts/system/modules/frontend/ModulePersonalData.php#L233

@leofeyer
Owner

Fixed in 6d3341f.

@leofeyer leofeyer closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.