Allow to push updates of account.config file
Pushing updates of account.config files was disabled while accounts were stored in both ReviewDb and NoteDb to prevent that the accounts go out of sync between ReviewDb and NoteDb. Now since accounts only live in NoteDb we can allow pushing updates of account.config files. However pushing updates to an account.config file is still rejected if: - it cannot be parsed - the preferred email is invalid - the own account is deactivated The same checks are done on submit if updates of account.config files have been pushed for review. Pushing an account.config file with an invalid preferred email is not rejected if the preferred email didn't change. This means if an account already had an invalid preferred email updates of account.config can still be pushed without being forced to fix the preferred email. However if the preferred email is changed it is verified that is valid. When validating the account.config file on submit we don't have the old file version available and hence we can't check if the preferred email has changed. This is why in this case we always enforce the preferred email to be valid. Change-Id: Ie08e50ea92fa7101c5cfcdca78632e8fd52cb26b Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
parent
6957747c87
commit
ed7c347ddb
@ -77,6 +77,7 @@ import com.google.gerrit.gpg.Fingerprint;
|
||||
import com.google.gerrit.gpg.PublicKeyStore;
|
||||
import com.google.gerrit.gpg.testutil.TestKey;
|
||||
import com.google.gerrit.reviewdb.client.Account;
|
||||
import com.google.gerrit.reviewdb.client.AccountGroup;
|
||||
import com.google.gerrit.reviewdb.client.RefNames;
|
||||
import com.google.gerrit.server.Sequences;
|
||||
import com.google.gerrit.server.account.AccountConfig;
|
||||
@ -812,14 +813,14 @@ public class AccountIT extends AbstractDaemonTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
public void pushAccountConfigToUserBranchForReviewIsRejectedOnSubmit() throws Exception {
|
||||
String userRefName = RefNames.refsUsers(admin.id);
|
||||
public void pushAccountConfigToUserBranchForReviewAndSubmit() throws Exception {
|
||||
String userRef = RefNames.refsUsers(admin.id);
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
fetch(allUsersRepo, userRefName + ":userRef");
|
||||
fetch(allUsersRepo, userRef + ":userRef");
|
||||
allUsersRepo.reset("userRef");
|
||||
|
||||
Config ac = getAccountConfig(allUsersRepo);
|
||||
ac.setString(AccountConfig.ACCOUNT, null, AccountConfig.KEY_STATUS, "OOO");
|
||||
ac.setString(AccountConfig.ACCOUNT, null, AccountConfig.KEY_STATUS, "out-of-office");
|
||||
|
||||
PushOneCommit.Result r =
|
||||
pushFactory
|
||||
@ -830,17 +831,203 @@ public class AccountIT extends AbstractDaemonTest {
|
||||
"Update account config",
|
||||
AccountConfig.ACCOUNT_CONFIG,
|
||||
ac.toText())
|
||||
.to(MagicBranch.NEW_CHANGE + userRefName);
|
||||
.to(MagicBranch.NEW_CHANGE + userRef);
|
||||
r.assertOkStatus();
|
||||
accountIndexedCounter.assertNoReindex();
|
||||
assertThat(r.getChange().change().getDest().get()).isEqualTo(userRefName);
|
||||
assertThat(r.getChange().change().getDest().get()).isEqualTo(userRef);
|
||||
|
||||
gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
|
||||
gApi.changes().id(r.getChangeId()).current().submit();
|
||||
accountIndexedCounter.assertReindexOf(admin);
|
||||
|
||||
AccountInfo info = gApi.accounts().self().get();
|
||||
assertThat(info.email).isEqualTo(admin.email);
|
||||
assertThat(info.name).isEqualTo(admin.fullName);
|
||||
assertThat(info.status).isEqualTo("out-of-office");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void pushAccountConfigWithPrefEmailThatDoesNotExistAsExtIdToUserBranchForReviewAndSubmit()
|
||||
throws Exception {
|
||||
TestAccount foo = accountCreator.create(name("foo"));
|
||||
String userRef = RefNames.refsUsers(foo.id);
|
||||
accountIndexedCounter.clear();
|
||||
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
fetch(allUsersRepo, userRef + ":userRef");
|
||||
allUsersRepo.reset("userRef");
|
||||
|
||||
String email = "some.email@example.com";
|
||||
Config ac = getAccountConfig(allUsersRepo);
|
||||
ac.setString(AccountConfig.ACCOUNT, null, AccountConfig.KEY_PREFERRED_EMAIL, email);
|
||||
|
||||
PushOneCommit.Result r =
|
||||
pushFactory
|
||||
.create(
|
||||
db,
|
||||
admin.getIdent(),
|
||||
allUsersRepo,
|
||||
"Update account config",
|
||||
AccountConfig.ACCOUNT_CONFIG,
|
||||
ac.toText())
|
||||
.to(MagicBranch.NEW_CHANGE + userRef);
|
||||
r.assertOkStatus();
|
||||
accountIndexedCounter.assertNoReindex();
|
||||
assertThat(r.getChange().change().getDest().get()).isEqualTo(userRef);
|
||||
|
||||
setApiUser(foo);
|
||||
gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
|
||||
gApi.changes().id(r.getChangeId()).current().submit();
|
||||
|
||||
accountIndexedCounter.assertReindexOf(foo);
|
||||
|
||||
AccountInfo info = gApi.accounts().self().get();
|
||||
assertThat(info.email).isEqualTo(email);
|
||||
assertThat(info.name).isEqualTo(foo.fullName);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void pushAccountConfigToUserBranchForReviewIsRejectedOnSubmitIfConfigIsInvalid()
|
||||
throws Exception {
|
||||
String userRef = RefNames.refsUsers(admin.id);
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
fetch(allUsersRepo, userRef + ":userRef");
|
||||
allUsersRepo.reset("userRef");
|
||||
|
||||
PushOneCommit.Result r =
|
||||
pushFactory
|
||||
.create(
|
||||
db,
|
||||
admin.getIdent(),
|
||||
allUsersRepo,
|
||||
"Update account config",
|
||||
AccountConfig.ACCOUNT_CONFIG,
|
||||
"invalid config")
|
||||
.to(MagicBranch.NEW_CHANGE + userRef);
|
||||
r.assertOkStatus();
|
||||
accountIndexedCounter.assertNoReindex();
|
||||
assertThat(r.getChange().change().getDest().get()).isEqualTo(userRef);
|
||||
|
||||
gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
|
||||
exception.expect(ResourceConflictException.class);
|
||||
exception.expectMessage(
|
||||
String.format("update of %s not allowed", AccountConfig.ACCOUNT_CONFIG));
|
||||
String.format(
|
||||
"invalid account configuration: commit '%s' has an invalid '%s' file for account '%s':"
|
||||
+ " Invalid config file %s in commit %s",
|
||||
r.getCommit().name(),
|
||||
AccountConfig.ACCOUNT_CONFIG,
|
||||
admin.id,
|
||||
AccountConfig.ACCOUNT_CONFIG,
|
||||
r.getCommit().name()));
|
||||
gApi.changes().id(r.getChangeId()).current().submit();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void pushAccountConfigToUserBranchForReviewIsRejectedOnSubmitIfPreferredEmailIsInvalid()
|
||||
throws Exception {
|
||||
String userRef = RefNames.refsUsers(admin.id);
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
fetch(allUsersRepo, userRef + ":userRef");
|
||||
allUsersRepo.reset("userRef");
|
||||
|
||||
String noEmail = "no.email";
|
||||
Config ac = getAccountConfig(allUsersRepo);
|
||||
ac.setString(AccountConfig.ACCOUNT, null, AccountConfig.KEY_PREFERRED_EMAIL, noEmail);
|
||||
|
||||
PushOneCommit.Result r =
|
||||
pushFactory
|
||||
.create(
|
||||
db,
|
||||
admin.getIdent(),
|
||||
allUsersRepo,
|
||||
"Update account config",
|
||||
AccountConfig.ACCOUNT_CONFIG,
|
||||
ac.toText())
|
||||
.to(MagicBranch.NEW_CHANGE + userRef);
|
||||
r.assertOkStatus();
|
||||
accountIndexedCounter.assertNoReindex();
|
||||
assertThat(r.getChange().change().getDest().get()).isEqualTo(userRef);
|
||||
|
||||
gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
|
||||
exception.expect(ResourceConflictException.class);
|
||||
exception.expectMessage(
|
||||
String.format(
|
||||
"invalid account configuration: invalid preferred email '%s' for account '%s'",
|
||||
noEmail, admin.id));
|
||||
gApi.changes().id(r.getChangeId()).current().submit();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void pushAccountConfigToUserBranchForReviewIsRejectedOnSubmitIfOwnAccountIsDeactivated()
|
||||
throws Exception {
|
||||
String userRef = RefNames.refsUsers(admin.id);
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
fetch(allUsersRepo, userRef + ":userRef");
|
||||
allUsersRepo.reset("userRef");
|
||||
|
||||
Config ac = getAccountConfig(allUsersRepo);
|
||||
ac.setBoolean(AccountConfig.ACCOUNT, null, AccountConfig.KEY_ACTIVE, false);
|
||||
|
||||
PushOneCommit.Result r =
|
||||
pushFactory
|
||||
.create(
|
||||
db,
|
||||
admin.getIdent(),
|
||||
allUsersRepo,
|
||||
"Update account config",
|
||||
AccountConfig.ACCOUNT_CONFIG,
|
||||
ac.toText())
|
||||
.to(MagicBranch.NEW_CHANGE + userRef);
|
||||
r.assertOkStatus();
|
||||
accountIndexedCounter.assertNoReindex();
|
||||
assertThat(r.getChange().change().getDest().get()).isEqualTo(userRef);
|
||||
|
||||
gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
|
||||
exception.expect(ResourceConflictException.class);
|
||||
exception.expectMessage("invalid account configuration: cannot deactivate own account");
|
||||
gApi.changes().id(r.getChangeId()).current().submit();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void pushAccountConfigToUserBranchForReviewDeactivateOtherAccount() throws Exception {
|
||||
TestAccount foo = accountCreator.create(name("foo"));
|
||||
assertThat(gApi.accounts().id(foo.id.get()).getActive()).isTrue();
|
||||
String userRef = RefNames.refsUsers(foo.id);
|
||||
accountIndexedCounter.clear();
|
||||
|
||||
AccountGroup adminGroup = groupCache.get(new AccountGroup.NameKey("Administrators"));
|
||||
grant(allUsers, userRef, Permission.PUSH, false, adminGroup.getGroupUUID());
|
||||
grantLabel("Code-Review", -2, 2, allUsers, userRef, false, adminGroup.getGroupUUID(), false);
|
||||
grant(allUsers, userRef, Permission.SUBMIT, false, adminGroup.getGroupUUID());
|
||||
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
fetch(allUsersRepo, userRef + ":userRef");
|
||||
allUsersRepo.reset("userRef");
|
||||
|
||||
Config ac = getAccountConfig(allUsersRepo);
|
||||
ac.setBoolean(AccountConfig.ACCOUNT, null, AccountConfig.KEY_ACTIVE, false);
|
||||
|
||||
PushOneCommit.Result r =
|
||||
pushFactory
|
||||
.create(
|
||||
db,
|
||||
admin.getIdent(),
|
||||
allUsersRepo,
|
||||
"Update account config",
|
||||
AccountConfig.ACCOUNT_CONFIG,
|
||||
ac.toText())
|
||||
.to(MagicBranch.NEW_CHANGE + userRef);
|
||||
r.assertOkStatus();
|
||||
accountIndexedCounter.assertNoReindex();
|
||||
assertThat(r.getChange().change().getDest().get()).isEqualTo(userRef);
|
||||
|
||||
gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
|
||||
gApi.changes().id(r.getChangeId()).current().submit();
|
||||
accountIndexedCounter.assertReindexOf(foo);
|
||||
|
||||
assertThat(gApi.accounts().id(foo.id.get()).getActive()).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void pushWatchConfigToUserBranch() throws Exception {
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
@ -883,13 +1070,70 @@ public class AccountIT extends AbstractDaemonTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
public void pushAccountConfigToUserBranchIsRejected() throws Exception {
|
||||
public void pushAccountConfigToUserBranch() throws Exception {
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
fetch(allUsersRepo, RefNames.refsUsers(admin.id) + ":userRef");
|
||||
allUsersRepo.reset("userRef");
|
||||
|
||||
Config ac = getAccountConfig(allUsersRepo);
|
||||
ac.setString(AccountConfig.ACCOUNT, null, AccountConfig.KEY_STATUS, "OOO");
|
||||
ac.setString(AccountConfig.ACCOUNT, null, AccountConfig.KEY_STATUS, "out-of-office");
|
||||
|
||||
pushFactory
|
||||
.create(
|
||||
db,
|
||||
admin.getIdent(),
|
||||
allUsersRepo,
|
||||
"Update account config",
|
||||
AccountConfig.ACCOUNT_CONFIG,
|
||||
ac.toText())
|
||||
.to(RefNames.REFS_USERS_SELF)
|
||||
.assertOkStatus();
|
||||
accountIndexedCounter.assertReindexOf(admin);
|
||||
|
||||
AccountInfo info = gApi.accounts().self().get();
|
||||
assertThat(info.email).isEqualTo(admin.email);
|
||||
assertThat(info.name).isEqualTo(admin.fullName);
|
||||
assertThat(info.status).isEqualTo("out-of-office");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void pushAccountConfigToUserBranchIsRejectedIfConfigIsInvalid() throws Exception {
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
fetch(allUsersRepo, RefNames.refsUsers(admin.id) + ":userRef");
|
||||
allUsersRepo.reset("userRef");
|
||||
|
||||
PushOneCommit.Result r =
|
||||
pushFactory
|
||||
.create(
|
||||
db,
|
||||
admin.getIdent(),
|
||||
allUsersRepo,
|
||||
"Update account config",
|
||||
AccountConfig.ACCOUNT_CONFIG,
|
||||
"invalid config")
|
||||
.to(RefNames.REFS_USERS_SELF);
|
||||
r.assertErrorStatus("invalid account configuration");
|
||||
r.assertMessage(
|
||||
String.format(
|
||||
"commit '%s' has an invalid '%s' file for account '%s':"
|
||||
+ " Invalid config file %s in commit %s",
|
||||
r.getCommit().name(),
|
||||
AccountConfig.ACCOUNT_CONFIG,
|
||||
admin.id,
|
||||
AccountConfig.ACCOUNT_CONFIG,
|
||||
r.getCommit().name()));
|
||||
accountIndexedCounter.assertNoReindex();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void pushAccountConfigToUserBranchIsRejectedIfPreferredEmailIsInvalid() throws Exception {
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
fetch(allUsersRepo, RefNames.refsUsers(admin.id) + ":userRef");
|
||||
allUsersRepo.reset("userRef");
|
||||
|
||||
String noEmail = "no.email";
|
||||
Config ac = getAccountConfig(allUsersRepo);
|
||||
ac.setString(AccountConfig.ACCOUNT, null, AccountConfig.KEY_PREFERRED_EMAIL, noEmail);
|
||||
|
||||
PushOneCommit.Result r =
|
||||
pushFactory
|
||||
@ -901,7 +1145,138 @@ public class AccountIT extends AbstractDaemonTest {
|
||||
AccountConfig.ACCOUNT_CONFIG,
|
||||
ac.toText())
|
||||
.to(RefNames.REFS_USERS_SELF);
|
||||
r.assertErrorStatus("account update not allowed");
|
||||
r.assertErrorStatus("invalid account configuration");
|
||||
r.assertMessage(
|
||||
String.format("invalid preferred email '%s' for account '%s'", noEmail, admin.id));
|
||||
accountIndexedCounter.assertNoReindex();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void pushAccountConfigToUserBranchInvalidPreferredEmailButNotChanged() throws Exception {
|
||||
TestAccount foo = accountCreator.create(name("foo"));
|
||||
String userRef = RefNames.refsUsers(foo.id);
|
||||
|
||||
String noEmail = "no.email";
|
||||
accountsUpdate.create().update(foo.id, a -> a.setPreferredEmail(noEmail));
|
||||
accountIndexedCounter.clear();
|
||||
|
||||
AccountGroup adminGroup = groupCache.get(new AccountGroup.NameKey("Administrators"));
|
||||
grant(allUsers, userRef, Permission.PUSH, false, adminGroup.getGroupUUID());
|
||||
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
fetch(allUsersRepo, userRef + ":userRef");
|
||||
allUsersRepo.reset("userRef");
|
||||
|
||||
String status = "in vacation";
|
||||
Config ac = getAccountConfig(allUsersRepo);
|
||||
ac.setString(AccountConfig.ACCOUNT, null, AccountConfig.KEY_STATUS, status);
|
||||
|
||||
pushFactory
|
||||
.create(
|
||||
db,
|
||||
admin.getIdent(),
|
||||
allUsersRepo,
|
||||
"Update account config",
|
||||
AccountConfig.ACCOUNT_CONFIG,
|
||||
ac.toText())
|
||||
.to(userRef)
|
||||
.assertOkStatus();
|
||||
accountIndexedCounter.assertReindexOf(foo);
|
||||
|
||||
AccountInfo info = gApi.accounts().id(foo.id.get()).get();
|
||||
assertThat(info.email).isEqualTo(noEmail);
|
||||
assertThat(info.name).isEqualTo(foo.fullName);
|
||||
assertThat(info.status).isEqualTo(status);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void pushAccountConfigToUserBranchIfPreferredEmailDoesNotExistAsExtId() throws Exception {
|
||||
TestAccount foo = accountCreator.create(name("foo"));
|
||||
String userRef = RefNames.refsUsers(foo.id);
|
||||
accountIndexedCounter.clear();
|
||||
|
||||
AccountGroup adminGroup = groupCache.get(new AccountGroup.NameKey("Administrators"));
|
||||
grant(allUsers, userRef, Permission.PUSH, false, adminGroup.getGroupUUID());
|
||||
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
fetch(allUsersRepo, userRef + ":userRef");
|
||||
allUsersRepo.reset("userRef");
|
||||
|
||||
String email = "some.email@example.com";
|
||||
Config ac = getAccountConfig(allUsersRepo);
|
||||
ac.setString(AccountConfig.ACCOUNT, null, AccountConfig.KEY_PREFERRED_EMAIL, email);
|
||||
|
||||
pushFactory
|
||||
.create(
|
||||
db,
|
||||
admin.getIdent(),
|
||||
allUsersRepo,
|
||||
"Update account config",
|
||||
AccountConfig.ACCOUNT_CONFIG,
|
||||
ac.toText())
|
||||
.to(userRef)
|
||||
.assertOkStatus();
|
||||
accountIndexedCounter.assertReindexOf(foo);
|
||||
|
||||
AccountInfo info = gApi.accounts().id(foo.id.get()).get();
|
||||
assertThat(info.email).isEqualTo(email);
|
||||
assertThat(info.name).isEqualTo(foo.fullName);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void pushAccountConfigToUserBranchIsRejectedIfOwnAccountIsDeactivated() throws Exception {
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
fetch(allUsersRepo, RefNames.refsUsers(admin.id) + ":userRef");
|
||||
allUsersRepo.reset("userRef");
|
||||
|
||||
Config ac = getAccountConfig(allUsersRepo);
|
||||
ac.setBoolean(AccountConfig.ACCOUNT, null, AccountConfig.KEY_ACTIVE, false);
|
||||
|
||||
PushOneCommit.Result r =
|
||||
pushFactory
|
||||
.create(
|
||||
db,
|
||||
admin.getIdent(),
|
||||
allUsersRepo,
|
||||
"Update account config",
|
||||
AccountConfig.ACCOUNT_CONFIG,
|
||||
ac.toText())
|
||||
.to(RefNames.REFS_USERS_SELF);
|
||||
r.assertErrorStatus("invalid account configuration");
|
||||
r.assertMessage("cannot deactivate own account");
|
||||
accountIndexedCounter.assertNoReindex();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void pushAccountConfigToUserBranchDeactivateOtherAccount() throws Exception {
|
||||
TestAccount foo = accountCreator.create(name("foo"));
|
||||
assertThat(gApi.accounts().id(foo.id.get()).getActive()).isTrue();
|
||||
String userRef = RefNames.refsUsers(foo.id);
|
||||
accountIndexedCounter.clear();
|
||||
|
||||
AccountGroup adminGroup = groupCache.get(new AccountGroup.NameKey("Administrators"));
|
||||
grant(allUsers, userRef, Permission.PUSH, false, adminGroup.getGroupUUID());
|
||||
|
||||
TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
|
||||
fetch(allUsersRepo, userRef + ":userRef");
|
||||
allUsersRepo.reset("userRef");
|
||||
|
||||
Config ac = getAccountConfig(allUsersRepo);
|
||||
ac.setBoolean(AccountConfig.ACCOUNT, null, AccountConfig.KEY_ACTIVE, false);
|
||||
|
||||
pushFactory
|
||||
.create(
|
||||
db,
|
||||
admin.getIdent(),
|
||||
allUsersRepo,
|
||||
"Update account config",
|
||||
AccountConfig.ACCOUNT_CONFIG,
|
||||
ac.toText())
|
||||
.to(userRef)
|
||||
.assertOkStatus();
|
||||
accountIndexedCounter.assertReindexOf(foo);
|
||||
|
||||
assertThat(gApi.accounts().id(foo.id.get()).getActive()).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@ -127,7 +127,7 @@ import com.google.gerrit.server.git.strategy.SubmitStrategy;
|
||||
import com.google.gerrit.server.git.validators.CommitValidationListener;
|
||||
import com.google.gerrit.server.git.validators.MergeValidationListener;
|
||||
import com.google.gerrit.server.git.validators.MergeValidators;
|
||||
import com.google.gerrit.server.git.validators.MergeValidators.AccountValidator;
|
||||
import com.google.gerrit.server.git.validators.MergeValidators.AccountMergeValidator;
|
||||
import com.google.gerrit.server.git.validators.MergeValidators.ProjectConfigValidator;
|
||||
import com.google.gerrit.server.git.validators.OnSubmitValidationListener;
|
||||
import com.google.gerrit.server.git.validators.OnSubmitValidators;
|
||||
@ -393,7 +393,7 @@ public class GerritGlobalModule extends FactoryModule {
|
||||
bind(AnonymousUser.class);
|
||||
|
||||
factory(AbandonOp.Factory.class);
|
||||
factory(AccountValidator.Factory.class);
|
||||
factory(AccountMergeValidator.Factory.class);
|
||||
factory(RefOperationValidators.Factory.class);
|
||||
factory(OnSubmitValidators.Factory.class);
|
||||
factory(MergeValidators.Factory.class);
|
||||
|
||||
@ -0,0 +1,91 @@
|
||||
// Copyright (C) 2017 The Android Open Source Project
|
||||
//
|
||||
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||
// you may not use this file except in compliance with the License.
|
||||
// You may obtain a copy of the License at
|
||||
//
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing, software
|
||||
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
package com.google.gerrit.server.git.validators;
|
||||
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.gerrit.common.Nullable;
|
||||
import com.google.gerrit.reviewdb.client.Account;
|
||||
import com.google.gerrit.server.IdentifiedUser;
|
||||
import com.google.gerrit.server.account.AccountConfig;
|
||||
import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
|
||||
import com.google.inject.Provider;
|
||||
import java.io.IOException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import javax.inject.Inject;
|
||||
import org.eclipse.jgit.errors.ConfigInvalidException;
|
||||
import org.eclipse.jgit.lib.ObjectId;
|
||||
import org.eclipse.jgit.revwalk.RevWalk;
|
||||
|
||||
public class AccountValidator {
|
||||
|
||||
private final Provider<IdentifiedUser> self;
|
||||
private final OutgoingEmailValidator emailValidator;
|
||||
|
||||
@Inject
|
||||
public AccountValidator(Provider<IdentifiedUser> self, OutgoingEmailValidator emailValidator) {
|
||||
this.self = self;
|
||||
this.emailValidator = emailValidator;
|
||||
}
|
||||
|
||||
public List<String> validate(
|
||||
Account.Id accountId, RevWalk rw, @Nullable ObjectId oldId, ObjectId newId)
|
||||
throws IOException {
|
||||
Account oldAccount = null;
|
||||
if (oldId != null && !ObjectId.zeroId().equals(oldId)) {
|
||||
try {
|
||||
oldAccount = loadAccount(accountId, rw, oldId);
|
||||
} catch (ConfigInvalidException e) {
|
||||
// ignore, maybe the new commit is repairing it now
|
||||
}
|
||||
}
|
||||
|
||||
Account newAccount;
|
||||
try {
|
||||
newAccount = loadAccount(accountId, rw, newId);
|
||||
} catch (ConfigInvalidException e) {
|
||||
return ImmutableList.of(
|
||||
String.format(
|
||||
"commit '%s' has an invalid '%s' file for account '%s': %s",
|
||||
newId.name(), AccountConfig.ACCOUNT_CONFIG, accountId.get(), e.getMessage()));
|
||||
}
|
||||
|
||||
List<String> messages = new ArrayList<>();
|
||||
if (accountId.equals(self.get().getAccountId()) && !newAccount.isActive()) {
|
||||
messages.add("cannot deactivate own account");
|
||||
}
|
||||
|
||||
if (newAccount.getPreferredEmail() != null
|
||||
&& (oldAccount == null
|
||||
|| !newAccount.getPreferredEmail().equals(oldAccount.getPreferredEmail()))) {
|
||||
if (!emailValidator.isValid(newAccount.getPreferredEmail())) {
|
||||
messages.add(
|
||||
String.format(
|
||||
"invalid preferred email '%s' for account '%s'",
|
||||
newAccount.getPreferredEmail(), accountId.get()));
|
||||
}
|
||||
}
|
||||
|
||||
return ImmutableList.copyOf(messages);
|
||||
}
|
||||
|
||||
private Account loadAccount(Account.Id accountId, RevWalk rw, ObjectId commit)
|
||||
throws IOException, ConfigInvalidException {
|
||||
rw.reset();
|
||||
AccountConfig accountConfig = new AccountConfig(null, accountId);
|
||||
accountConfig.load(rw, commit);
|
||||
return accountConfig.getAccount();
|
||||
}
|
||||
}
|
||||
@ -32,7 +32,6 @@ import com.google.gerrit.reviewdb.client.Branch;
|
||||
import com.google.gerrit.reviewdb.client.RefNames;
|
||||
import com.google.gerrit.server.GerritPersonIdent;
|
||||
import com.google.gerrit.server.IdentifiedUser;
|
||||
import com.google.gerrit.server.account.AccountConfig;
|
||||
import com.google.gerrit.server.account.WatchConfig;
|
||||
import com.google.gerrit.server.account.externalids.ExternalIdsConsistencyChecker;
|
||||
import com.google.gerrit.server.config.AllUsersName;
|
||||
@ -58,11 +57,9 @@ import java.net.URL;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.Objects;
|
||||
import java.util.regex.Pattern;
|
||||
import org.eclipse.jgit.errors.ConfigInvalidException;
|
||||
import org.eclipse.jgit.lib.Config;
|
||||
import org.eclipse.jgit.lib.ObjectId;
|
||||
import org.eclipse.jgit.lib.PersonIdent;
|
||||
import org.eclipse.jgit.lib.Repository;
|
||||
import org.eclipse.jgit.notes.NoteMap;
|
||||
@ -70,7 +67,6 @@ import org.eclipse.jgit.revwalk.FooterKey;
|
||||
import org.eclipse.jgit.revwalk.FooterLine;
|
||||
import org.eclipse.jgit.revwalk.RevCommit;
|
||||
import org.eclipse.jgit.revwalk.RevWalk;
|
||||
import org.eclipse.jgit.treewalk.TreeWalk;
|
||||
import org.eclipse.jgit.util.SystemReader;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
@ -88,6 +84,7 @@ public class CommitValidators {
|
||||
private final DynamicSet<CommitValidationListener> pluginValidators;
|
||||
private final AllUsersName allUsers;
|
||||
private final ExternalIdsConsistencyChecker externalIdsConsistencyChecker;
|
||||
private final AccountValidator accountValidator;
|
||||
private final String installCommitMsgHookCommand;
|
||||
private final ProjectCache projectCache;
|
||||
|
||||
@ -99,12 +96,14 @@ public class CommitValidators {
|
||||
DynamicSet<CommitValidationListener> pluginValidators,
|
||||
AllUsersName allUsers,
|
||||
ExternalIdsConsistencyChecker externalIdsConsistencyChecker,
|
||||
AccountValidator accountValidator,
|
||||
ProjectCache projectCache) {
|
||||
this.gerritIdent = gerritIdent;
|
||||
this.canonicalWebUrl = canonicalWebUrl;
|
||||
this.pluginValidators = pluginValidators;
|
||||
this.allUsers = allUsers;
|
||||
this.externalIdsConsistencyChecker = externalIdsConsistencyChecker;
|
||||
this.accountValidator = accountValidator;
|
||||
this.installCommitMsgHookCommand =
|
||||
cfg != null ? cfg.getString("gerrit", null, "installCommitMsgHookCommand") : null;
|
||||
this.projectCache = projectCache;
|
||||
@ -133,7 +132,7 @@ public class CommitValidators {
|
||||
new BannedCommitsValidator(rejectCommits),
|
||||
new PluginCommitValidationListener(pluginValidators),
|
||||
new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker),
|
||||
new AccountValidator(allUsers)));
|
||||
new AccountCommitValidator(allUsers, accountValidator)));
|
||||
}
|
||||
|
||||
public CommitValidators forGerritCommits(
|
||||
@ -158,7 +157,7 @@ public class CommitValidators {
|
||||
new ConfigValidator(branch, user, rw, allUsers),
|
||||
new PluginCommitValidationListener(pluginValidators),
|
||||
new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker),
|
||||
new AccountValidator(allUsers)));
|
||||
new AccountCommitValidator(allUsers, accountValidator)));
|
||||
}
|
||||
|
||||
public CommitValidators forMergedCommits(PermissionBackend.ForRef perm, IdentifiedUser user) {
|
||||
@ -709,11 +708,13 @@ public class CommitValidators {
|
||||
}
|
||||
|
||||
/** Rejects updates to 'account.config' in user branches. */
|
||||
public static class AccountValidator implements CommitValidationListener {
|
||||
public static class AccountCommitValidator implements CommitValidationListener {
|
||||
private final AllUsersName allUsers;
|
||||
private final AccountValidator accountValidator;
|
||||
|
||||
public AccountValidator(AllUsersName allUsers) {
|
||||
public AccountCommitValidator(AllUsersName allUsers, AccountValidator accountValidator) {
|
||||
this.allUsers = allUsers;
|
||||
this.accountValidator = accountValidator;
|
||||
}
|
||||
|
||||
@Override
|
||||
@ -725,7 +726,7 @@ public class CommitValidators {
|
||||
|
||||
if (receiveEvent.command.getRefName().startsWith(MagicBranch.NEW_CHANGE)) {
|
||||
// no validation on push for review, will be checked on submit by
|
||||
// MergeValidators.AccountValidator
|
||||
// MergeValidators.AccountMergeValidator
|
||||
return Collections.emptyList();
|
||||
}
|
||||
|
||||
@ -735,15 +736,19 @@ public class CommitValidators {
|
||||
}
|
||||
|
||||
try {
|
||||
ObjectId newBlobId = getAccountConfigBlobId(receiveEvent.revWalk, receiveEvent.commit);
|
||||
|
||||
ObjectId oldId = receiveEvent.command.getOldId();
|
||||
ObjectId oldBlobId =
|
||||
!ObjectId.zeroId().equals(oldId)
|
||||
? getAccountConfigBlobId(receiveEvent.revWalk, oldId)
|
||||
: null;
|
||||
if (!Objects.equals(oldBlobId, newBlobId)) {
|
||||
throw new CommitValidationException("account update not allowed");
|
||||
List<String> errorMessages =
|
||||
accountValidator.validate(
|
||||
accountId,
|
||||
receiveEvent.revWalk,
|
||||
receiveEvent.command.getOldId(),
|
||||
receiveEvent.commit);
|
||||
if (!errorMessages.isEmpty()) {
|
||||
throw new CommitValidationException(
|
||||
"invalid account configuration",
|
||||
errorMessages
|
||||
.stream()
|
||||
.map(m -> new CommitValidationMessage(m, true))
|
||||
.collect(toList()));
|
||||
}
|
||||
} catch (IOException e) {
|
||||
String m = String.format("Validating update for account %s failed", accountId.get());
|
||||
@ -752,14 +757,6 @@ public class CommitValidators {
|
||||
}
|
||||
return Collections.emptyList();
|
||||
}
|
||||
|
||||
private ObjectId getAccountConfigBlobId(RevWalk rw, ObjectId id) throws IOException {
|
||||
RevCommit commit = rw.parseCommit(id);
|
||||
try (TreeWalk tw =
|
||||
TreeWalk.forPath(rw.getObjectReader(), AccountConfig.ACCOUNT_CONFIG, commit.getTree())) {
|
||||
return tw != null ? tw.getObjectId(0) : null;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private static CommitValidationMessage invalidEmail(
|
||||
|
||||
@ -14,6 +14,7 @@
|
||||
|
||||
package com.google.gerrit.server.git.validators;
|
||||
|
||||
import com.google.common.base.Joiner;
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.gerrit.extensions.api.projects.ProjectConfigEntryType;
|
||||
import com.google.gerrit.extensions.registration.DynamicMap;
|
||||
@ -47,6 +48,7 @@ import java.io.IOException;
|
||||
import java.util.List;
|
||||
import org.eclipse.jgit.errors.ConfigInvalidException;
|
||||
import org.eclipse.jgit.lib.Repository;
|
||||
import org.eclipse.jgit.revwalk.RevWalk;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
|
||||
@ -55,7 +57,7 @@ public class MergeValidators {
|
||||
|
||||
private final DynamicSet<MergeValidationListener> mergeValidationListeners;
|
||||
private final ProjectConfigValidator.Factory projectConfigValidatorFactory;
|
||||
private final AccountValidator.Factory accountValidatorFactory;
|
||||
private final AccountMergeValidator.Factory accountValidatorFactory;
|
||||
|
||||
public interface Factory {
|
||||
MergeValidators create();
|
||||
@ -65,7 +67,7 @@ public class MergeValidators {
|
||||
MergeValidators(
|
||||
DynamicSet<MergeValidationListener> mergeValidationListeners,
|
||||
ProjectConfigValidator.Factory projectConfigValidatorFactory,
|
||||
AccountValidator.Factory accountValidatorFactory) {
|
||||
AccountMergeValidator.Factory accountValidatorFactory) {
|
||||
this.mergeValidationListeners = mergeValidationListeners;
|
||||
this.projectConfigValidatorFactory = projectConfigValidatorFactory;
|
||||
this.accountValidatorFactory = accountValidatorFactory;
|
||||
@ -222,23 +224,26 @@ public class MergeValidators {
|
||||
}
|
||||
}
|
||||
|
||||
public static class AccountValidator implements MergeValidationListener {
|
||||
public static class AccountMergeValidator implements MergeValidationListener {
|
||||
public interface Factory {
|
||||
AccountValidator create();
|
||||
AccountMergeValidator create();
|
||||
}
|
||||
|
||||
private final Provider<ReviewDb> dbProvider;
|
||||
private final AllUsersName allUsersName;
|
||||
private final ChangeData.Factory changeDataFactory;
|
||||
private final AccountValidator accountValidator;
|
||||
|
||||
@Inject
|
||||
public AccountValidator(
|
||||
public AccountMergeValidator(
|
||||
Provider<ReviewDb> dbProvider,
|
||||
AllUsersName allUsersName,
|
||||
ChangeData.Factory changeDataFactory) {
|
||||
ChangeData.Factory changeDataFactory,
|
||||
AccountValidator accountValidator) {
|
||||
this.dbProvider = dbProvider;
|
||||
this.allUsersName = allUsersName;
|
||||
this.changeDataFactory = changeDataFactory;
|
||||
this.accountValidator = accountValidator;
|
||||
}
|
||||
|
||||
@Override
|
||||
@ -250,30 +255,33 @@ public class MergeValidators {
|
||||
PatchSet.Id patchSetId,
|
||||
IdentifiedUser caller)
|
||||
throws MergeValidationException {
|
||||
if (!allUsersName.equals(destProject.getProject().getNameKey())
|
||||
|| Account.Id.fromRef(destBranch.get()) == null) {
|
||||
Account.Id accountId = Account.Id.fromRef(destBranch.get());
|
||||
if (!allUsersName.equals(destProject.getProject().getNameKey()) || accountId == null) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (commit.getParentCount() > 1) {
|
||||
// for merge commits we cannot ensure that the 'account.config' file is not modified, since
|
||||
// for merge commits file modifications that come in through the merge don't appear in the
|
||||
// file list that is returned by ChangeData#currentFilePaths()
|
||||
throw new MergeValidationException("cannot submit merge commit to user branch");
|
||||
}
|
||||
|
||||
ChangeData cd =
|
||||
changeDataFactory.create(
|
||||
dbProvider.get(), destProject.getProject().getNameKey(), patchSetId.getParentKey());
|
||||
try {
|
||||
if (cd.currentFilePaths().contains(AccountConfig.ACCOUNT_CONFIG)) {
|
||||
throw new MergeValidationException(
|
||||
String.format("update of %s not allowed", AccountConfig.ACCOUNT_CONFIG));
|
||||
if (!cd.currentFilePaths().contains(AccountConfig.ACCOUNT_CONFIG)) {
|
||||
return;
|
||||
}
|
||||
} catch (OrmException e) {
|
||||
log.error("Cannot validate account update", e);
|
||||
throw new MergeValidationException("account validation unavailable");
|
||||
}
|
||||
|
||||
try (RevWalk rw = new RevWalk(repo)) {
|
||||
List<String> errorMessages = accountValidator.validate(accountId, rw, null, commit);
|
||||
if (!errorMessages.isEmpty()) {
|
||||
throw new MergeValidationException(
|
||||
"invalid account configuration: " + Joiner.on("; ").join(errorMessages));
|
||||
}
|
||||
} catch (IOException e) {
|
||||
log.error("Cannot validate account update", e);
|
||||
throw new MergeValidationException("account validation unavailable");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user