Page MenuHomePhabricator

'Attaching' two external accounts of the same type results in AphrontCountQueryException
Closed, ResolvedPublic

Description

This is a clone of https://phabricator.wikimedia.org/T75720 , filed after @epriestley's comment at T6703#86537.

I can't seem to visit /u/fbstj could this be something to do with me 'attaching' two mediawiki accounts to my account?

I get the following errors when I try to visit my page:

Unhandled Exception ("AphrontCountQueryException")
More than 1 result from loadOneWhere()!

Event Timeline

qgil raised the priority of this task from to Needs Triage.
qgil updated the task description. (Show Details)
qgil added a project: OAuthServer.
qgil added subscribers: keir, epriestley, indygreg, qgil.
epriestley added a project: Support Impact.

Expected behavior is that this is prevented until T2549, which will eventually make this work properly.

Support Impact This is buggy, confusing, and a bad UX.

In particular, this is very unlikely to be particular to Mediawiki, and should reproduce with other providers (like GitHub). (If it doesn't, there's just an issue we need to fix with the MW adapter.)

epriestley renamed this task from 'Attaching' two mediawiki accounts results in AphrontCountQueryException to 'Attaching' two external accounts from the same provider results in AphrontCountQueryException.Dec 9 2014, 5:32 PM
epriestley renamed this task from 'Attaching' two external accounts from the same provider results in AphrontCountQueryException to 'Attaching' two Mediawiki accounts results in AphrontCountQueryException.

Well, maybe I'm wrong. I can't reproduce this with other account types. For example, I have no option to attach a second Facebook account:

Screen_Shot_2014-12-09_at_9.33.43_AM.png (992×1 px, 214 KB)

If I manually go to the URI, I also can't attach a second account:

Screen_Shot_2014-12-09_at_9.32.58_AM.png (992×1 px, 117 KB)

Do you see different options with an attached MW account (i.e., an option to attach a second account, and/or no exception)?

I don't see any option to add a second MediaWiki account... In Wikimedia Phabricator I see MediaWiki and LDAP linked to one account each, then "Your account is linked with all available providers." under "Add External Account".

Hey, im the one who reported/suffers from this.

How I caused it was fairly involved, I logged out of the original connected account and then logged in to the second account. Then I tried to refresh the connection through the refresh button, which linked to the second account without removing the first, and now I get the exception if I try to view my profile or edit or remove the connection

Aaaaah, okay. I'm guessing that will probably repro across other providers. Thanks!

FWIW today we got a second report.

qgil moved this task from Backlog to Details on the Wikimedia board.

Facing the similar problem, but with two Google accounts.

pasted_file (371×854 px, 70 KB)

Refresh or delete any of them will lead AphrontCountQueryException:

pasted_file (103×425 px, 13 KB)

I was also able to reproduce this with my Google accounts. I have a personal and business Gmail accounts. I mistakenly linked the personal account but I can't delete it because it's the "last" account. So I "refreshed" it and selected the business Gmail account thinking it would replace the old one, but instead, it created another one and now I'm also stuck with two linked account and the query error.

How can we safely clean the database?

So some code is written assuming that the linked accounts will be unique per provider because the configuration ui implies that there is only one linked account per provider. This isn't enforced by the database, however, and it's not really well enforced anywhere.

So should we remove the assumption of uniqueness or enforce uniqueness? I'm willing to help clean up the problem if @epriestley has any guidance on which way to go with it.

We should enforce uniqueness. The fix is probably something like:

if ($remote_account->getAccountID() != $local_account->getAccountID()) {
  throw new Exception(
    pht(
      'You are logged in to a different external account than the one connected to '.
      'your Phabricator account. Log out of the external service, then log back in '.
      'with the right account before refreshing the account link.'));
}

We should eventually remove the assumption of uniqueness (T2549), and the fact that the database does not have a unique key anticipates this, but this is very complicated, and we should never accept a different account on the "Refresh" action. That is, "Refresh" should never mean "link a different account": it should throw if the account we find doesn't match the account we expect.

The way the logic actually works, I think the fix is really in PhabricatorAuthLoginController near line 117:

  } else {
+   $existing_account = /* load existing account from provider */;
+   if ($existing_account) {
+      throw new Exception(/* as above */);
+   }
    if ($provider->shouldAllowAccountLink()) {
      return $this->processLinkUser($account);

This logic is used on both the "link" and "refresh" pathways and just doesn't account for this case.

epriestley renamed this task from 'Attaching' two Mediawiki accounts results in AphrontCountQueryException to 'Attaching' two external accounts of the same type results in AphrontCountQueryException.May 3 2015, 11:15 PM
epriestley added a subscriber: gp.

The way the logic actually works, I think the fix is really in PhabricatorAuthLoginController near line 117:
[...]

So would it be fine to prepare that patch?
And would applying that patch also "magically" retroactively fix the problem with the affected downstream account, or what would be recommended to fix that case? :-/

We had a similar error while using LDAP as an auth mechanism.

We had initially allowed logins by using one particular LDAP search attribute, which we later had to change. Two accounts were registered using the first attribute and after changing the attribute, they could no longer log in. Selecting "refresh" in the authentication settings of the users let them login/register again, and now they have two LDAP accounts under "Linked Accounts and Authentication". Attempting to remove the old one throws the AphrontCountQueryException.

Would love to have a fix for this.

And the easiest fix turned out to be just deleting the row from the database, as mentioned in T2549#88614.