Block the removal of the Realm primary external ids

Do not allow removing e-mails associated with the primary
external id of a Realm as it would result in effectively
removing the account id in that realm.

Removing accounts is not allowed in Gerrit and therefore
should not be allowed in Realms as well, especially when
that is done unconsciously while removing an e-mail.

This problem has been in the Gerrit code-base for years
but it was hidden by the front-end check done in the GWT UI
that was checking if the realm allowed or not that deletion.
The advent of PolyGerrit UI has changed how things work
and delegated the responsibility of making the decision to
the backend, which was broken.

The attempt to remove the e-mail associated with the primary
external id scheme of the realm will now result in a
ResourceConflictException, which is one of the error conditions
that should be already known to the PolyGerrit UI.

Bug: Issue 12755
Change-Id: I06f3414b02d00cdeaacb36e22f756560d70c5b45
This commit is contained in:
Luca Milanesio
2020-05-15 01:10:16 +01:00
parent 4415cc33c7
commit 0a6e793eea
2 changed files with 84 additions and 1 deletions

View File

@@ -17,6 +17,7 @@ package com.google.gerrit.server.restapi.account;
import static java.util.stream.Collectors.toSet;
import com.google.gerrit.extensions.client.AccountFieldName;
import com.google.gerrit.extensions.client.AuthType;
import com.google.gerrit.extensions.common.Input;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
@@ -33,6 +34,7 @@ import com.google.gerrit.server.account.AccountResource;
import com.google.gerrit.server.account.Realm;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.account.externalids.ExternalIds;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -52,6 +54,7 @@ public class DeleteEmail implements RestModifyView<AccountResource.Email, Input>
private final PermissionBackend permissionBackend;
private final AccountManager accountManager;
private final ExternalIds externalIds;
private final AuthType authType;
@Inject
DeleteEmail(
@@ -59,12 +62,14 @@ public class DeleteEmail implements RestModifyView<AccountResource.Email, Input>
Realm realm,
PermissionBackend permissionBackend,
AccountManager accountManager,
ExternalIds externalIds) {
ExternalIds externalIds,
AuthConfig authConfig) {
this.self = self;
this.realm = realm;
this.permissionBackend = permissionBackend;
this.accountManager = accountManager;
this.externalIds = externalIds;
this.authType = authConfig.getAuthType();
}
@Override
@@ -95,6 +100,14 @@ public class DeleteEmail implements RestModifyView<AccountResource.Email, Input>
throw new ResourceNotFoundException(email);
}
if (realm.accountBelongsToRealm(extIds)) {
String errorMsg =
String.format(
"Cannot remove e-mail '%s' which is directly associated with %s authentication",
email, authType);
throw new ResourceConflictException(errorMsg);
}
try {
accountManager.unlink(
user.getAccountId(), extIds.stream().map(ExternalId::key).collect(toSet()));