AccountResolver.ByEmail: Ignore inconsistent accounts if multiple accounts are matched
An email can only belong to a single account. If multiple accounts are
found for an email it means there is an inconsistency, i.e. some of the
found accounts have a preferred email set that they do not own via an
external ID. In this case we return only the one account that actually
owns the email via an external ID. Returning all matched accounts
instead of only the account that owns the email via an external ID was a
behavior change that was accidentally introduced by change I799bf1c57.
If none of the matched accounts owns the email, we return all matches to
be consistent with the behavior of Emails.getAccountFor(String) that is
used if the user can see secondary emails (Emails.getAccountFor(String)
falls back to searching accounts by preferred email if there is no
account owning the email via an external ID and all of them are
returned).
This fall-back is important for Google since internally we support a
process to link accounts, which creates such inconsistent accounts
(accounts having a preferred email that they do not own via an external
ID).
Release-Notes: skip
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Ic0002ef35899a3df8fbc2138f91619326fe38798
diff --git a/java/com/google/gerrit/server/account/AccountResolver.java b/java/com/google/gerrit/server/account/AccountResolver.java
index 2670e52..1818b1b 100644
--- a/java/com/google/gerrit/server/account/AccountResolver.java
+++ b/java/com/google/gerrit/server/account/AccountResolver.java
@@ -25,6 +25,7 @@
import com.google.common.base.Suppliers;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
@@ -447,20 +448,45 @@
}
// User cannot see secondary emails, hence search by preferred email only.
- Stream<AccountState> accountStateStream =
- accountQueryProvider.get().byPreferredEmail(input).stream();
+ List<AccountState> accountStates = accountQueryProvider.get().byPreferredEmail(input);
- // Users can always see their own secondary emails. Hence if any email of the user matches,
- // include the user into the result.
+ if (accountStates.size() == 1) {
+ return Stream.of(Iterables.getOnlyElement(accountStates));
+ }
+
+ if (accountStates.size() > 1) {
+ // An email can only belong to a single account. If multiple accounts are found it means
+ // there is an inconsistency, i.e. some of the found accounts have a preferred email set
+ // that they do not own via an external ID. Hence in this case we return only the one
+ // account that actually owns the email via an external ID.
+ for (AccountState accountState : accountStates) {
+ if (accountState.externalIds().stream()
+ .map(ExternalId::email)
+ .filter(Objects::nonNull)
+ .anyMatch(email -> email.equals(input))) {
+ return Stream.of(accountState);
+ }
+ }
+
+ // None of the matched accounts owns the email, return all matches to be consistent with
+ // the behavior of Emails.getAccountFor(String) that is used above if the user can see
+ // secondary emails.
+ return accountStates.stream();
+ }
+
+ // No match by preferred email. Since users can always see their own secondary emails, check
+ // if the input matches a secondary email of the user and if yes, return the account of the
+ // user.
if (asUser.isIdentifiedUser()
&& asUser.asIdentifiedUser().state().externalIds().stream()
.map(ExternalId::email)
.filter(Objects::nonNull)
.anyMatch(email -> email.equals(input))) {
- return Streams.concat(accountStateStream, Stream.of(asUser.asIdentifiedUser().state()));
+ return Stream.of(asUser.asIdentifiedUser().state());
}
- return accountStateStream;
+ // No match.
+ return Stream.empty();
}
@Override
