Fixed Removing connected-account when I have no password

[Rasmus Vind]

Affected version

v2.0.10

I am working towards integrating @Steve Gibson's SQRL login system with XenForo and I just came across an interesting series of steps. I just tested it with a Google account and it's the same with all kinds of connected accounts, I believe.

So I did the following:

1. Click "Register"
2. Click "Register faster using: Anything"
3. Enter username and email
4. Click your username and hit "Connected accounts"
5. Tick "Disassociate Anything account" and click "Confirm disassociation"

Now I am instantly logged out but redirected to /account/connected-accounts/ with an error "You must be logged-in to do that.".

Also, I used a bogus email (I know, my own fault) and XenForo allowed me to disassociate my connected account even though:

1. It knew my email was not validated.
2. It knew I had no password.

So I am completely locked out of my account.

You can do better, and you usually do.

The code in XF\Pub\Controller\Account::actionConnectedAccountDisassociate has the following lines:

            if (!$auth->hasPassword() && $totalConnected <= 1)
            {
                $visitor->Auth->resetPassword();
                $sendConfirmation = true;
            }
            else
            {
                $sendConfirmation = false;
            }

Which means that it resets my password in case I never used it. A nice feature because it knows I don't have a password except that $visitor->Auth->resetPassword forces a log-out. I feel like we need to do something similar to what we do in XF\ControllerPlugin\Login when changing our password:

    public function handleVisitorPasswordChange()
    {
        $visitor = \XF::visitor();
        if (!$visitor->user_id)
        {
            return;
        }
        /** @var \XF\Repository\UserRemember $rememberRepo */
        $rememberRepo = $this->repository('XF:UserRemember');
        $userRemember = $this->validateVisitorRememberKey();
        $rememberRepo->clearUserRememberRecords($visitor->user_id);
        if ($userRemember)
        {
            // had a remember key before which has been invalidated, so give another one
            $this->createVisitorRememberKey();
        }
        // this will reset the necessary details in the session (such as password date)
        $this->session->changeUser($visitor);
    }

It ensures that when changing the password, we aren't logged out. Because the session depends on the password somehow.

I feel like you should be forced to set a password (when you don't have one) when you remove your last connected account. Also, you shouldn't be instantly logged out and definitely not redirected to a page never visible to you when logged out. Also, you may wish to consider requiring email validation prior to relying on password recovery.

Thanks,

Ralle

 

[Chris D]

We will handle the password reset better, and how we do in other places.

There aren't any changes planned to handle the invalid email situation. Generally most connected account providers will have already confirmed your email and by us doing it as well only really serves to not really make the registration faster (as advertised).

There are also other tools within the software to catch bad email addresses. Notably being sent any sort of email will trigger your account to be flagged as email bouncing if the handling for email bounces is set up.

Anyone can theoretically have an issue caused by an invalid email. The most common one is users who registered with a genuine email address which later ceases to exist. If they then forget their password, they will be left in a similar situation.

 

[XF Bug Bot]

Thank you for reporting this issue. It has now been resolved and we are aiming to include it in a future XF release (2.1.0 B7/RC1).

Change log:

Handle a password reset via disassociating a connected account by ensuring the current session is not invalidated.

Any changes made as a result of this issue being resolved may not be rolled out here until later.