Merge "Merge branch 'stable-2.15' into master"
diff --git a/java/com/google/gerrit/httpd/GitOverHttpServlet.java b/java/com/google/gerrit/httpd/GitOverHttpServlet.java
index 05c1d58..739726e 100644
--- a/java/com/google/gerrit/httpd/GitOverHttpServlet.java
+++ b/java/com/google/gerrit/httpd/GitOverHttpServlet.java
@@ -184,7 +184,7 @@
try {
Project.NameKey nameKey = new Project.NameKey(projectName);
ProjectState state = projectCache.checkedGet(nameKey);
- if (state == null) {
+ if (state == null || !state.statePermitsRead()) {
throw new RepositoryNotFoundException(nameKey.get());
}
req.setAttribute(ATT_STATE, state);
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 28fbb7d..515624f 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -839,10 +839,10 @@
}
Object obj = createInstance(type);
- Field[] fields = obj.getClass().getDeclaredFields();
- if (fields.length == 0 && Strings.isNullOrEmpty(value)) {
+ if (Strings.isNullOrEmpty(value)) {
return obj;
}
+ Field[] fields = obj.getClass().getDeclaredFields();
for (Field f : fields) {
if (f.getAnnotation(DefaultInput.class) != null && f.getType() == String.class) {
f.setAccessible(true);
diff --git a/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java b/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java
index c51fb9b..f117b24 100644
--- a/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java
+++ b/java/com/google/gerrit/httpd/rpc/project/ProjectAccessFactory.java
@@ -259,8 +259,14 @@
throws NoSuchProjectException, IOException, PermissionBackendException,
ResourceConflictException {
ProjectState state = projectCache.checkedGet(projectName);
+ // Hidden projects(permitsRead = false) should only be accessible by the project owners.
+ // READ_CONFIG is checked here because it's only allowed to project owners(ACCESS may also
+ // be allowed for other users). Allowing project owners to access here will help them to view
+ // and update the config of hidden projects easily.
+ ProjectPermission permissionToCheck =
+ state.statePermitsRead() ? ProjectPermission.ACCESS : ProjectPermission.READ_CONFIG;
try {
- permissionBackend.currentUser().project(projectName).check(ProjectPermission.ACCESS);
+ permissionBackend.currentUser().project(projectName).check(permissionToCheck);
} catch (AuthException e) {
throw new NoSuchProjectException(projectName);
}
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalId.java b/java/com/google/gerrit/server/account/externalids/ExternalId.java
index ffd413a..442bc2a 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalId.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalId.java
@@ -278,8 +278,6 @@
*/
public static ExternalId parse(String noteId, byte[] raw, ObjectId blobId)
throws ConfigInvalidException {
- checkNotNull(blobId);
-
Config externalIdConfig = new Config();
try {
externalIdConfig.fromText(new String(raw, UTF_8));
@@ -287,6 +285,13 @@
throw invalidConfig(noteId, e.getMessage());
}
+ return parse(noteId, externalIdConfig, blobId);
+ }
+
+ public static ExternalId parse(String noteId, Config externalIdConfig, ObjectId blobId)
+ throws ConfigInvalidException {
+ checkNotNull(blobId);
+
Set<String> externalIdKeys = externalIdConfig.getSubsections(EXTERNAL_ID_SECTION);
if (externalIdKeys.size() != 1) {
throw invalidConfig(
@@ -439,11 +444,17 @@
// c.setString(...) ensures that account IDs are human readable.
c.setString(
EXTERNAL_ID_SECTION, externalIdKey, ACCOUNT_ID_KEY, Integer.toString(accountId().get()));
+
if (email() != null) {
c.setString(EXTERNAL_ID_SECTION, externalIdKey, EMAIL_KEY, email());
+ } else {
+ c.unset(EXTERNAL_ID_SECTION, externalIdKey, EMAIL_KEY);
}
+
if (password() != null) {
c.setString(EXTERNAL_ID_SECTION, externalIdKey, PASSWORD_KEY, password());
+ } else {
+ c.unset(EXTERNAL_ID_SECTION, externalIdKey, PASSWORD_KEY);
}
}
}
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdNotes.java b/java/com/google/gerrit/server/account/externalids/ExternalIdNotes.java
index 972dfbd..ecb201f 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalIdNotes.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalIdNotes.java
@@ -17,6 +17,7 @@
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toSet;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
@@ -371,9 +372,9 @@
Set<ExternalId> newExtIds = new HashSet<>();
noteMapUpdates.add(
- (rw, n) -> {
+ (rw, n, f) -> {
for (ExternalId extId : extIds) {
- ExternalId insertedExtId = upsert(rw, inserter, noteMap, extId);
+ ExternalId insertedExtId = upsert(rw, inserter, noteMap, f, extId);
newExtIds.add(insertedExtId);
}
});
@@ -399,9 +400,9 @@
Set<ExternalId> removedExtIds = get(ExternalId.Key.from(extIds));
Set<ExternalId> updatedExtIds = new HashSet<>();
noteMapUpdates.add(
- (rw, n) -> {
+ (rw, n, f) -> {
for (ExternalId extId : extIds) {
- ExternalId updatedExtId = upsert(rw, inserter, noteMap, extId);
+ ExternalId updatedExtId = upsert(rw, inserter, noteMap, f, extId);
updatedExtIds.add(updatedExtId);
}
});
@@ -429,9 +430,9 @@
checkLoaded();
Set<ExternalId> removedExtIds = new HashSet<>();
noteMapUpdates.add(
- (rw, n) -> {
+ (rw, n, f) -> {
for (ExternalId extId : extIds) {
- remove(rw, noteMap, extId);
+ remove(rw, noteMap, f, extId);
removedExtIds.add(extId);
}
});
@@ -458,9 +459,9 @@
checkLoaded();
Set<ExternalId> removedExtIds = new HashSet<>();
noteMapUpdates.add(
- (rw, n) -> {
+ (rw, n, f) -> {
for (ExternalId.Key extIdKey : extIdKeys) {
- ExternalId removedExtId = remove(rw, noteMap, extIdKey, accountId);
+ ExternalId removedExtId = remove(rw, noteMap, f, extIdKey, accountId);
removedExtIds.add(removedExtId);
}
});
@@ -476,9 +477,9 @@
checkLoaded();
Set<ExternalId> removedExtIds = new HashSet<>();
noteMapUpdates.add(
- (rw, n) -> {
+ (rw, n, f) -> {
for (ExternalId.Key extIdKey : extIdKeys) {
- ExternalId extId = remove(rw, noteMap, extIdKey, null);
+ ExternalId extId = remove(rw, noteMap, f, extIdKey, null);
removedExtIds.add(extId);
}
});
@@ -506,16 +507,16 @@
Set<ExternalId> removedExtIds = new HashSet<>();
Set<ExternalId> updatedExtIds = new HashSet<>();
noteMapUpdates.add(
- (rw, n) -> {
+ (rw, n, f) -> {
for (ExternalId.Key extIdKey : toDelete) {
- ExternalId removedExtId = remove(rw, noteMap, extIdKey, accountId);
+ ExternalId removedExtId = remove(rw, noteMap, f, extIdKey, accountId);
if (removedExtId != null) {
removedExtIds.add(removedExtId);
}
}
for (ExternalId extId : toAdd) {
- ExternalId insertedExtId = upsert(rw, inserter, noteMap, extId);
+ ExternalId insertedExtId = upsert(rw, inserter, noteMap, f, extId);
updatedExtIds.add(insertedExtId);
}
});
@@ -540,14 +541,14 @@
Set<ExternalId> removedExtIds = new HashSet<>();
Set<ExternalId> updatedExtIds = new HashSet<>();
noteMapUpdates.add(
- (rw, n) -> {
+ (rw, n, f) -> {
for (ExternalId.Key extIdKey : toDelete) {
- ExternalId removedExtId = remove(rw, noteMap, extIdKey, null);
+ ExternalId removedExtId = remove(rw, noteMap, f, extIdKey, null);
removedExtIds.add(removedExtId);
}
for (ExternalId extId : toAdd) {
- ExternalId insertedExtId = upsert(rw, inserter, noteMap, extId);
+ ExternalId insertedExtId = upsert(rw, inserter, noteMap, f, extId);
updatedExtIds.add(insertedExtId);
}
});
@@ -660,14 +661,22 @@
}
try (RevWalk rw = new RevWalk(repo)) {
+ Set<String> footers = new HashSet<>();
for (NoteMapUpdate noteMapUpdate : noteMapUpdates) {
try {
- noteMapUpdate.execute(rw, noteMap);
+ noteMapUpdate.execute(rw, noteMap, footers);
} catch (DuplicateExternalIdKeyException e) {
throw new IOException(e);
}
}
noteMapUpdates.clear();
+ if (!footers.isEmpty()) {
+ commit.setMessage(
+ footers
+ .stream()
+ .sorted()
+ .collect(joining("\n", commit.getMessage().trim() + "\n\n", "")));
+ }
RevTree oldTree = revision != null ? rw.parseTree(revision) : null;
ObjectId newTreeId = noteMap.writeTree(inserter);
@@ -718,14 +727,17 @@
* <p>If the external ID already exists it is overwritten.
*/
private static ExternalId upsert(
- RevWalk rw, ObjectInserter ins, NoteMap noteMap, ExternalId extId)
+ RevWalk rw, ObjectInserter ins, NoteMap noteMap, Set<String> footers, ExternalId extId)
throws IOException, ConfigInvalidException {
ObjectId noteId = extId.key().sha1();
Config c = new Config();
if (noteMap.contains(extId.key().sha1())) {
- byte[] raw = readNoteData(rw, noteMap.get(noteId));
+ ObjectId noteDataId = noteMap.get(noteId);
+ byte[] raw = readNoteData(rw, noteDataId);
try {
c = new BlobBasedConfig(null, raw);
+ ExternalId oldExtId = ExternalId.parse(noteId.name(), c, noteDataId);
+ addFooters(footers, oldExtId);
} catch (ConfigInvalidException e) {
throw new ConfigInvalidException(
String.format("Invalid external id config for note %s: %s", noteId, e.getMessage()));
@@ -735,7 +747,9 @@
byte[] raw = c.toText().getBytes(UTF_8);
ObjectId noteData = ins.insert(OBJ_BLOB, raw);
noteMap.set(noteId, noteData);
- return ExternalId.create(extId, noteData);
+ ExternalId newExtId = ExternalId.create(extId, noteData);
+ addFooters(footers, newExtId);
+ return newExtId;
}
/**
@@ -744,7 +758,8 @@
* @throws IllegalStateException is thrown if there is an existing external ID that has the same
* key, but otherwise doesn't match the specified external ID.
*/
- private static ExternalId remove(RevWalk rw, NoteMap noteMap, ExternalId extId)
+ private static ExternalId remove(
+ RevWalk rw, NoteMap noteMap, Set<String> footers, ExternalId extId)
throws IOException, ConfigInvalidException {
ObjectId noteId = extId.key().sha1();
if (!noteMap.contains(noteId)) {
@@ -760,6 +775,7 @@
extId.toString(),
actualExtId.toString());
noteMap.remove(noteId);
+ addFooters(footers, actualExtId);
return actualExtId;
}
@@ -772,7 +788,11 @@
* exists
*/
private static ExternalId remove(
- RevWalk rw, NoteMap noteMap, ExternalId.Key extIdKey, Account.Id expectedAccountId)
+ RevWalk rw,
+ NoteMap noteMap,
+ Set<String> footers,
+ ExternalId.Key extIdKey,
+ Account.Id expectedAccountId)
throws IOException, ConfigInvalidException {
ObjectId noteId = extIdKey.sha1();
if (!noteMap.contains(noteId)) {
@@ -792,9 +812,17 @@
extId.accountId().get());
}
noteMap.remove(noteId);
+ addFooters(footers, extId);
return extId;
}
+ private static void addFooters(Set<String> footers, ExternalId extId) {
+ footers.add("Account: " + extId.accountId().get());
+ if (extId.email() != null) {
+ footers.add("Email: " + extId.email());
+ }
+ }
+
private void checkExternalIdsDontExist(Collection<ExternalId> extIds)
throws DuplicateExternalIdKeyException, IOException {
checkExternalIdKeysDontExist(ExternalId.Key.from(extIds));
@@ -823,7 +851,7 @@
@FunctionalInterface
private interface NoteMapUpdate {
- void execute(RevWalk rw, NoteMap noteMap)
+ void execute(RevWalk rw, NoteMap noteMap, Set<String> footers)
throws IOException, ConfigInvalidException, DuplicateExternalIdKeyException;
}
diff --git a/java/com/google/gerrit/server/args4j/ProjectHandler.java b/java/com/google/gerrit/server/args4j/ProjectHandler.java
index e97c171..1d40b53 100644
--- a/java/com/google/gerrit/server/args4j/ProjectHandler.java
+++ b/java/com/google/gerrit/server/args4j/ProjectHandler.java
@@ -79,7 +79,13 @@
if (state == null) {
throw new CmdLineException(owner, String.format("project %s not found", nameWithoutSuffix));
}
- permissionBackend.currentUser().project(nameKey).check(ProjectPermission.ACCESS);
+ // Hidden projects(permitsRead = false) should only be accessible by the project owners.
+ // READ_CONFIG is checked here because it's only allowed to project owners(ACCESS may also
+ // be allowed for other users). Allowing project owners to access here will help them to view
+ // and update the config of hidden projects easily.
+ ProjectPermission permissionToCheck =
+ state.statePermitsRead() ? ProjectPermission.ACCESS : ProjectPermission.READ_CONFIG;
+ permissionBackend.currentUser().project(nameKey).check(permissionToCheck);
} catch (AuthException e) {
throw new CmdLineException(owner, new NoSuchProjectException(nameKey).getMessage());
} catch (PermissionBackendException | IOException e) {
diff --git a/java/com/google/gerrit/server/events/EventBroker.java b/java/com/google/gerrit/server/events/EventBroker.java
index 8f12cb3..62e8d12 100644
--- a/java/com/google/gerrit/server/events/EventBroker.java
+++ b/java/com/google/gerrit/server/events/EventBroker.java
@@ -150,6 +150,11 @@
protected boolean isVisibleTo(Project.NameKey project, CurrentUser user) {
try {
+ ProjectState state = projectCache.get(project);
+ if (state == null || !state.statePermitsRead()) {
+ return false;
+ }
+
permissionBackend.user(user).project(project).check(ProjectPermission.ACCESS);
return true;
} catch (AuthException | PermissionBackendException e) {
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 262c732..6dbad6a 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -517,7 +517,6 @@
/** Number of unresolved comments of the change. */
public static final FieldDef<ChangeData, Integer> UNRESOLVED_COMMENT_COUNT =
intRange(ChangeQueryBuilder.FIELD_UNRESOLVED_COMMENT_COUNT)
- .stored()
.build(ChangeData::unresolvedCommentCount);
/** Whether the change is mergeable. */
@@ -536,13 +535,11 @@
/** The number of inserted lines in this change. */
public static final FieldDef<ChangeData, Integer> ADDED =
intRange(ChangeQueryBuilder.FIELD_ADDED)
- .stored()
.build(cd -> cd.changedLines().isPresent() ? cd.changedLines().get().insertions : null);
/** The number of deleted lines in this change. */
public static final FieldDef<ChangeData, Integer> DELETED =
intRange(ChangeQueryBuilder.FIELD_DELETED)
- .stored()
.build(cd -> cd.changedLines().isPresent() ? cd.changedLines().get().deletions : null);
/** The total number of modified lines in this change. */
diff --git a/java/com/google/gerrit/server/mail/send/CreateChangeSender.java b/java/com/google/gerrit/server/mail/send/CreateChangeSender.java
index 10d14a8..a12e226 100644
--- a/java/com/google/gerrit/server/mail/send/CreateChangeSender.java
+++ b/java/com/google/gerrit/server/mail/send/CreateChangeSender.java
@@ -19,7 +19,6 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.ProjectWatches.NotifyType;
import com.google.gerrit.server.mail.send.ProjectWatch.Watchers;
import com.google.gerrit.server.permissions.PermissionBackend;
@@ -39,19 +38,16 @@
CreateChangeSender create(Project.NameKey project, Change.Id id);
}
- private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final PermissionBackend permissionBackend;
@Inject
public CreateChangeSender(
EmailArguments ea,
- IdentifiedUser.GenericFactory identifiedUserFactory,
PermissionBackend permissionBackend,
@Assisted Project.NameKey project,
@Assisted Change.Id id)
throws OrmException {
super(ea, newChangeData(ea, project, id));
- this.identifiedUserFactory = identifiedUserFactory;
this.permissionBackend = permissionBackend;
}
@@ -83,7 +79,7 @@
private boolean isOwnerOfProjectOrBranch(Account.Id userId) {
return permissionBackend
- .user(identifiedUserFactory.create(userId))
+ .absentUser(userId)
.ref(change.getDest())
.testOrFalse(RefPermission.WRITE_CONFIG);
}
diff --git a/java/com/google/gerrit/server/permissions/ChangeControl.java b/java/com/google/gerrit/server/permissions/ChangeControl.java
index 47c8543..e49686a 100644
--- a/java/com/google/gerrit/server/permissions/ChangeControl.java
+++ b/java/com/google/gerrit/server/permissions/ChangeControl.java
@@ -138,19 +138,6 @@
&& !isPatchSetLocked(db);
}
- /** Can this user delete this change? */
- private boolean canDelete(Change.Status status) {
- switch (status) {
- case NEW:
- case ABANDONED:
- return (isOwner() && refControl.canPerform(Permission.DELETE_OWN_CHANGES))
- || getProjectControl().isAdmin();
- case MERGED:
- default:
- return false;
- }
- }
-
/** Can this user rebase this change? */
private boolean canRebase(ReviewDb db) throws OrmException {
return (isOwner() || refControl.canSubmit(isOwner()) || refControl.canRebase())
@@ -369,7 +356,8 @@
case ABANDON:
return canAbandon(db());
case DELETE:
- return canDelete(getChange().getStatus());
+ return (isOwner() && refControl.canPerform(Permission.DELETE_OWN_CHANGES))
+ || getProjectControl().isAdmin();
case ADD_PATCH_SET:
return canAddPatchSet(db());
case EDIT_ASSIGNEE:
diff --git a/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java b/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
index d8e01f9..2c8c951 100644
--- a/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
@@ -23,9 +23,11 @@
import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission;
import com.google.gerrit.extensions.api.access.PluginPermission;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PeerDaemonUser;
import com.google.gerrit.server.account.CapabilityCollection;
import com.google.gerrit.server.project.NoSuchProjectException;
@@ -48,15 +50,18 @@
private final Provider<CurrentUser> currentUser;
private final ProjectCache projectCache;
private final ProjectControl.Factory projectControlFactory;
+ private final IdentifiedUser.GenericFactory identifiedUserFactory;
@Inject
DefaultPermissionBackend(
Provider<CurrentUser> currentUser,
ProjectCache projectCache,
- ProjectControl.Factory projectControlFactory) {
+ ProjectControl.Factory projectControlFactory,
+ IdentifiedUser.GenericFactory identifiedUserFactory) {
this.currentUser = currentUser;
this.projectCache = projectCache;
this.projectControlFactory = projectControlFactory;
+ this.identifiedUserFactory = identifiedUserFactory;
}
private CapabilityCollection capabilities() {
@@ -73,6 +78,12 @@
return new WithUserImpl(checkNotNull(user, "user"));
}
+ @Override
+ public WithUser absentUser(Account.Id user) {
+ IdentifiedUser identifiedUser = identifiedUserFactory.create(checkNotNull(user, "user"));
+ return new WithUserImpl(identifiedUser);
+ }
+
class WithUserImpl extends WithUser {
private final CurrentUser user;
private Boolean admin;
diff --git a/java/com/google/gerrit/server/permissions/PermissionBackend.java b/java/com/google/gerrit/server/permissions/PermissionBackend.java
index a15aa3a..ed10184 100644
--- a/java/com/google/gerrit/server/permissions/PermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/PermissionBackend.java
@@ -23,6 +23,7 @@
import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission;
import com.google.gerrit.extensions.conditions.BooleanCondition;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -94,12 +95,24 @@
public abstract WithUser currentUser();
/**
- * Returns an instance scoped to the specified user. If an instance scoped to the current user is
- * desired, use {@code currentUser()} instead.
+ * Returns an instance scoped to the specified user. Should be used in cases where the user could
+ * either be the issuer of the current request or an impersonated user. PermissionBackends that do
+ * not support impersonation can fail with an {@code IllegalStateException}.
+ *
+ * <p>If an instance scoped to the current user is desired, use {@code currentUser()} instead.
*/
public abstract WithUser user(CurrentUser user);
/**
+ * Returns an instance scoped to the provided user. Should be used in cases where the caller wants
+ * to check the permissions of a user who is not the issuer of the current request and not the
+ * target of impersonation.
+ *
+ * <p>Usage should be very limited as this can expose a group-oracle.
+ */
+ public abstract WithUser absentUser(Account.Id user);
+
+ /**
* Bulk evaluate a set of {@link PermissionBackendCondition} for view handling.
*
* <p>Overridden implementations should call {@link PermissionBackendCondition#set(boolean)} to
diff --git a/java/com/google/gerrit/server/permissions/ProjectControl.java b/java/com/google/gerrit/server/permissions/ProjectControl.java
index 30ed180..dbd60ea 100644
--- a/java/com/google/gerrit/server/permissions/ProjectControl.java
+++ b/java/com/google/gerrit/server/permissions/ProjectControl.java
@@ -202,11 +202,6 @@
return false;
}
- /** Returns whether the project is hidden. */
- private boolean isHidden() {
- return getProject().getState().equals(com.google.gerrit.extensions.client.ProjectState.HIDDEN);
- }
-
private boolean canAddRefs() {
return (canPerformOnAnyRef(Permission.CREATE) || isAdmin());
}
@@ -400,8 +395,7 @@
private boolean can(ProjectPermission perm) throws PermissionBackendException {
switch (perm) {
case ACCESS:
- return (!isHidden() && (user.isInternalUser() || canPerformOnAnyRef(Permission.READ)))
- || isOwner();
+ return user.isInternalUser() || isOwner() || canPerformOnAnyRef(Permission.READ);
case READ:
return allRefsAreVisible(Collections.emptySet());
diff --git a/java/com/google/gerrit/server/project/ChildProjects.java b/java/com/google/gerrit/server/project/ChildProjects.java
index a29a152..868d0af 100644
--- a/java/com/google/gerrit/server/project/ChildProjects.java
+++ b/java/com/google/gerrit/server/project/ChildProjects.java
@@ -53,7 +53,7 @@
/** Gets all child projects recursively. */
public List<ProjectInfo> list(Project.NameKey parent) throws PermissionBackendException {
- Map<Project.NameKey, Project> projects = readAllProjects();
+ Map<Project.NameKey, Project> projects = readAllReadableProjects();
Multimap<Project.NameKey, Project.NameKey> children = parentToChildren(projects);
PermissionBackend.WithUser perm = permissionBackend.currentUser();
@@ -62,11 +62,11 @@
return results;
}
- private Map<Project.NameKey, Project> readAllProjects() {
+ private Map<Project.NameKey, Project> readAllReadableProjects() {
Map<Project.NameKey, Project> projects = new HashMap<>();
for (Project.NameKey name : projectCache.all()) {
ProjectState c = projectCache.get(name);
- if (c != null) {
+ if (c != null && c.statePermitsRead()) {
projects.put(c.getNameKey(), c.getProject());
}
}
diff --git a/java/com/google/gerrit/server/project/RefUtil.java b/java/com/google/gerrit/server/project/RefUtil.java
index 62e48be..e42a7df 100644
--- a/java/com/google/gerrit/server/project/RefUtil.java
+++ b/java/com/google/gerrit/server/project/RefUtil.java
@@ -39,6 +39,8 @@
public class RefUtil {
private static final Logger log = LoggerFactory.getLogger(RefUtil.class);
+ private RefUtil() {}
+
public static ObjectId parseBaseRevision(
Repository repo, Project.NameKey projectName, String baseRevision)
throws InvalidRevisionException {
diff --git a/java/com/google/gerrit/server/project/SuggestParentCandidates.java b/java/com/google/gerrit/server/project/SuggestParentCandidates.java
index 014029b..99833af 100644
--- a/java/com/google/gerrit/server/project/SuggestParentCandidates.java
+++ b/java/com/google/gerrit/server/project/SuggestParentCandidates.java
@@ -44,17 +44,17 @@
public List<Project.NameKey> getNameKeys() throws PermissionBackendException {
return permissionBackend
.currentUser()
- .filter(ProjectPermission.ACCESS, parents())
+ .filter(ProjectPermission.ACCESS, readableParents())
.stream()
.sorted()
.collect(toList());
}
- private Set<Project.NameKey> parents() {
+ private Set<Project.NameKey> readableParents() {
Set<Project.NameKey> parents = new HashSet<>();
for (Project.NameKey p : projectCache.all()) {
ProjectState ps = projectCache.get(p);
- if (ps != null) {
+ if (ps != null && ps.statePermitsRead()) {
Project.NameKey parent = ps.getProject().getParent();
if (parent != null) {
parents.add(parent);
diff --git a/java/com/google/gerrit/server/query/account/AccountPredicates.java b/java/com/google/gerrit/server/query/account/AccountPredicates.java
index acb963c..57a0dcc 100644
--- a/java/com/google/gerrit/server/query/account/AccountPredicates.java
+++ b/java/com/google/gerrit/server/query/account/AccountPredicates.java
@@ -126,8 +126,7 @@
public static Predicate<AccountState> cansee(
AccountQueryBuilder.Arguments args, ChangeNotes changeNotes) {
- return new CanSeeChangePredicate(
- args.db, args.permissionBackend, args.userFactory, changeNotes);
+ return new CanSeeChangePredicate(args.db, args.permissionBackend, changeNotes);
}
static class AccountPredicate extends IndexPredicate<AccountState>
diff --git a/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java b/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java
index 8b6e1e4..f627ec8 100644
--- a/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java
@@ -63,7 +63,6 @@
public static class Arguments {
final Provider<ReviewDb> db;
final ChangeFinder changeFinder;
- final IdentifiedUser.GenericFactory userFactory;
final PermissionBackend permissionBackend;
private final Provider<CurrentUser> self;
@@ -75,13 +74,11 @@
AccountIndexCollection indexes,
Provider<ReviewDb> db,
ChangeFinder changeFinder,
- IdentifiedUser.GenericFactory userFactory,
PermissionBackend permissionBackend) {
this.self = self;
this.indexes = indexes;
this.db = db;
this.changeFinder = changeFinder;
- this.userFactory = userFactory;
this.permissionBackend = permissionBackend;
}
diff --git a/java/com/google/gerrit/server/query/account/CanSeeChangePredicate.java b/java/com/google/gerrit/server/query/account/CanSeeChangePredicate.java
index c436a45..b5e7b90 100644
--- a/java/com/google/gerrit/server/query/account/CanSeeChangePredicate.java
+++ b/java/com/google/gerrit/server/query/account/CanSeeChangePredicate.java
@@ -17,7 +17,6 @@
import com.google.gerrit.index.query.PostFilterPredicate;
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.ChangePermission;
@@ -31,17 +30,12 @@
public class CanSeeChangePredicate extends PostFilterPredicate<AccountState> {
private final Provider<ReviewDb> db;
private final PermissionBackend permissionBackend;
- private final IdentifiedUser.GenericFactory userFactory;
private final ChangeNotes changeNotes;
CanSeeChangePredicate(
- Provider<ReviewDb> db,
- PermissionBackend permissionBackend,
- IdentifiedUser.GenericFactory userFactory,
- ChangeNotes changeNotes) {
+ Provider<ReviewDb> db, PermissionBackend permissionBackend, ChangeNotes changeNotes) {
this.db = db;
this.permissionBackend = permissionBackend;
- this.userFactory = userFactory;
this.changeNotes = changeNotes;
}
@@ -49,7 +43,7 @@
public boolean match(AccountState accountState) throws OrmException {
try {
return permissionBackend
- .user(userFactory.create(accountState.getAccount().getId()))
+ .absentUser(accountState.getAccount().getId())
.database(db)
.change(changeNotes)
.test(ChangePermission.READ);
@@ -65,7 +59,7 @@
@Override
public Predicate<AccountState> copy(Collection<? extends Predicate<AccountState>> children) {
- return new CanSeeChangePredicate(db, permissionBackend, userFactory, changeNotes);
+ return new CanSeeChangePredicate(db, permissionBackend, changeNotes);
}
@Override
diff --git a/java/com/google/gerrit/server/query/account/InternalAccountQuery.java b/java/com/google/gerrit/server/query/account/InternalAccountQuery.java
index f1be580..02386ae 100644
--- a/java/com/google/gerrit/server/query/account/InternalAccountQuery.java
+++ b/java/com/google/gerrit/server/query/account/InternalAccountQuery.java
@@ -17,11 +17,9 @@
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toSet;
-import com.google.common.base.Joiner;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
-import com.google.common.collect.Lists;
import com.google.common.collect.Multimap;
import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.IndexConfig;
@@ -37,8 +35,6 @@
import java.util.Arrays;
import java.util.List;
import java.util.Set;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/**
* Query wrapper for the account index.
@@ -47,8 +43,6 @@
* holding on to a single instance.
*/
public class InternalAccountQuery extends InternalQuery<AccountState> {
- private static final Logger log = LoggerFactory.getLogger(InternalAccountQuery.class);
-
@Inject
InternalAccountQuery(
AccountQueryProcessor queryProcessor,
@@ -94,28 +88,6 @@
return query(AccountPredicates.externalIdIncludingSecondaryEmails(externalId.toString()));
}
- public AccountState oneByExternalId(String externalId) throws OrmException {
- return oneByExternalId(ExternalId.Key.parse(externalId));
- }
-
- public AccountState oneByExternalId(String scheme, String id) throws OrmException {
- return oneByExternalId(ExternalId.Key.create(scheme, id));
- }
-
- public AccountState oneByExternalId(ExternalId.Key externalId) throws OrmException {
- List<AccountState> accountStates = byExternalId(externalId);
- if (accountStates.size() == 1) {
- return accountStates.get(0);
- } else if (accountStates.size() > 0) {
- StringBuilder msg = new StringBuilder();
- msg.append("Ambiguous external ID ").append(externalId).append(" for accounts: ");
- Joiner.on(", ")
- .appendTo(msg, Lists.transform(accountStates, AccountState.ACCOUNT_ID_FUNCTION));
- log.warn(msg.toString());
- }
- return null;
- }
-
public List<AccountState> byFullName(String fullName) throws OrmException {
return query(AccountPredicates.fullName(fullName));
}
diff --git a/java/com/google/gerrit/server/restapi/change/DeleteChange.java b/java/com/google/gerrit/server/restapi/change/DeleteChange.java
index e33b4a4..4bd10ed 100644
--- a/java/com/google/gerrit/server/restapi/change/DeleteChange.java
+++ b/java/com/google/gerrit/server/restapi/change/DeleteChange.java
@@ -56,7 +56,7 @@
protected Response<?> applyImpl(
BatchUpdate.Factory updateFactory, ChangeResource rsrc, Input input)
throws RestApiException, UpdateException, PermissionBackendException {
- if (rsrc.getChange().getStatus() == Change.Status.MERGED) {
+ if (!isChangeDeletable(rsrc.getChange().getStatus())) {
throw new MethodNotAllowedException("delete not permitted");
}
rsrc.permissions().database(db).check(ChangePermission.DELETE);
@@ -78,20 +78,15 @@
return new UiAction.Description()
.setLabel("Delete")
.setTitle("Delete change " + rsrc.getId())
- .setVisible(and(couldDeleteWhenIn(status), perm.testCond(ChangePermission.DELETE)));
+ .setVisible(and(isChangeDeletable(status), perm.testCond(ChangePermission.DELETE)));
}
- private boolean couldDeleteWhenIn(Change.Status status) {
- switch (status) {
- case NEW:
- case ABANDONED:
- // New or abandoned changes can be deleted with the right permissions.
- return true;
-
- case MERGED:
- // Merged changes should never be deleted.
- return false;
+ private static boolean isChangeDeletable(Change.Status status) {
+ if (status == Change.Status.MERGED) {
+ // Merged changes should never be deleted.
+ return false;
}
- return false;
+ // New or abandoned changes can be deleted with the right permissions.
+ return true;
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java b/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java
index 5f6b088..78687cd 100644
--- a/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java
+++ b/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java
@@ -20,6 +20,7 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.index.query.Predicate;
@@ -100,7 +101,7 @@
}
public List<Account.Id> suggestReviewers(
- ChangeNotes changeNotes,
+ @Nullable ChangeNotes changeNotes,
SuggestReviewers suggestReviewers,
ProjectState projectState,
List<Account.Id> candidateList)
diff --git a/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java b/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
index ac88b65..95557b5 100644
--- a/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
+++ b/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
@@ -21,6 +21,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.extensions.common.GroupBaseInfo;
import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
@@ -161,17 +162,25 @@
}
public List<SuggestedReviewerInfo> suggestReviewers(
- ChangeNotes changeNotes,
+ @Nullable ChangeNotes changeNotes,
SuggestReviewers suggestReviewers,
ProjectState projectState,
VisibilityControl visibilityControl,
boolean excludeGroups)
throws IOException, OrmException, ConfigInvalidException, PermissionBackendException {
CurrentUser currentUser = self.get();
- log.debug(
- "Suggesting reviewers for change {} to user {}.",
- changeNotes.getChangeId().get(),
- currentUser.getLoggableName());
+ if (changeNotes != null) {
+ log.debug(
+ "Suggesting reviewers for change {} to user {}.",
+ changeNotes.getChangeId().get(),
+ currentUser.getLoggableName());
+ } else {
+ log.debug(
+ "Suggesting default reviewers for project {} to user {}.",
+ projectState.getName(),
+ currentUser.getLoggableName());
+ }
+
String query = suggestReviewers.getQuery();
log.debug("Query: {}", query);
int limit = suggestReviewers.getLimit();
@@ -272,7 +281,7 @@
}
private List<Account.Id> recommendAccounts(
- ChangeNotes changeNotes,
+ @Nullable ChangeNotes changeNotes,
SuggestReviewers suggestReviewers,
ProjectState projectState,
List<Account.Id> candidateList)
diff --git a/java/com/google/gerrit/server/restapi/config/ListTasks.java b/java/com/google/gerrit/server/restapi/config/ListTasks.java
index 71ee5ad..fb2819c 100644
--- a/java/com/google/gerrit/server/restapi/config/ListTasks.java
+++ b/java/com/google/gerrit/server/restapi/config/ListTasks.java
@@ -28,6 +28,8 @@
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.util.IdGenerator;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -46,13 +48,18 @@
private final PermissionBackend permissionBackend;
private final WorkQueue workQueue;
private final Provider<CurrentUser> self;
+ private final ProjectCache projectCache;
@Inject
public ListTasks(
- PermissionBackend permissionBackend, WorkQueue workQueue, Provider<CurrentUser> self) {
+ PermissionBackend permissionBackend,
+ WorkQueue workQueue,
+ Provider<CurrentUser> self,
+ ProjectCache projectCache) {
this.permissionBackend = permissionBackend;
this.workQueue = workQueue;
this.self = self;
+ this.projectCache = projectCache;
}
@Override
@@ -77,14 +84,17 @@
if (task.projectName != null) {
Boolean visible = visibilityCache.get(task.projectName);
if (visible == null) {
- try {
- permissionBackend
- .user(user)
- .project(new Project.NameKey(task.projectName))
- .check(ProjectPermission.ACCESS);
- visible = true;
- } catch (AuthException e) {
+ Project.NameKey nameKey = new Project.NameKey(task.projectName);
+ ProjectState state = projectCache.get(nameKey);
+ if (state == null || !state.statePermitsRead()) {
visible = false;
+ } else {
+ try {
+ permissionBackend.user(user).project(nameKey).check(ProjectPermission.ACCESS);
+ visible = true;
+ } catch (AuthException e) {
+ visible = false;
+ }
}
visibilityCache.put(task.projectName, visible);
}
diff --git a/java/com/google/gerrit/server/restapi/config/TasksCollection.java b/java/com/google/gerrit/server/restapi/config/TasksCollection.java
index f5b6e56..dda54a0 100644
--- a/java/com/google/gerrit/server/restapi/config/TasksCollection.java
+++ b/java/com/google/gerrit/server/restapi/config/TasksCollection.java
@@ -18,8 +18,10 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestView;
+import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.config.ConfigResource;
import com.google.gerrit.server.config.TaskResource;
@@ -30,6 +32,8 @@
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -41,6 +45,7 @@
private final WorkQueue workQueue;
private final Provider<CurrentUser> self;
private final PermissionBackend permissionBackend;
+ private final ProjectCache projectCache;
@Inject
TasksCollection(
@@ -48,12 +53,14 @@
ListTasks list,
WorkQueue workQueue,
Provider<CurrentUser> self,
- PermissionBackend permissionBackend) {
+ PermissionBackend permissionBackend,
+ ProjectCache projectCache) {
this.views = views;
this.list = list;
this.workQueue = workQueue;
this.self = self;
this.permissionBackend = permissionBackend;
+ this.projectCache = projectCache;
}
@Override
@@ -63,7 +70,8 @@
@Override
public TaskResource parse(ConfigResource parent, IdString id)
- throws ResourceNotFoundException, AuthException, PermissionBackendException {
+ throws ResourceNotFoundException, AuthException, PermissionBackendException,
+ ResourceConflictException {
CurrentUser user = self.get();
if (!user.isIdentifiedUser()) {
throw new AuthException("Authentication required");
@@ -78,11 +86,16 @@
Task<?> task = workQueue.getTask(taskId);
if (task instanceof ProjectTask) {
+ Project.NameKey nameKey = ((ProjectTask<?>) task).getProjectNameKey();
+ ProjectState state = projectCache.get(nameKey);
+ if (state == null) {
+ throw new ResourceNotFoundException(String.format("project %s not found", nameKey));
+ }
+
+ state.checkStatePermitsRead();
+
try {
- permissionBackend
- .user(user)
- .project(((ProjectTask<?>) task).getProjectNameKey())
- .check(ProjectPermission.ACCESS);
+ permissionBackend.user(user).project(nameKey).check(ProjectPermission.ACCESS);
return new TaskResource(task);
} catch (AuthException e) {
// Fall through and try view queue permission.
diff --git a/java/com/google/gerrit/server/restapi/project/ListChildProjects.java b/java/com/google/gerrit/server/restapi/project/ListChildProjects.java
index 7db4f84..3067c89 100644
--- a/java/com/google/gerrit/server/restapi/project/ListChildProjects.java
+++ b/java/com/google/gerrit/server/restapi/project/ListChildProjects.java
@@ -80,7 +80,9 @@
Map<Project.NameKey, Project> children = new HashMap<>();
for (Project.NameKey name : projectCache.all()) {
ProjectState c = projectCache.get(name);
- if (c != null && parent.equals(c.getProject().getParent(allProjects))) {
+ if (c != null
+ && parent.equals(c.getProject().getParent(allProjects))
+ && c.statePermitsRead()) {
children.put(c.getNameKey(), c.getProject());
}
}
diff --git a/java/com/google/gerrit/server/restapi/project/ListDashboards.java b/java/com/google/gerrit/server/restapi/project/ListDashboards.java
index 08b4069..882e922 100644
--- a/java/com/google/gerrit/server/restapi/project/ListDashboards.java
+++ b/java/com/google/gerrit/server/restapi/project/ListDashboards.java
@@ -88,8 +88,11 @@
private Collection<ProjectState> tree(ProjectResource rsrc) throws PermissionBackendException {
Map<Project.NameKey, ProjectState> tree = new LinkedHashMap<>();
for (ProjectState ps : rsrc.getProjectState().tree()) {
- tree.put(ps.getNameKey(), ps);
+ if (ps.statePermitsRead()) {
+ tree.put(ps.getNameKey(), ps);
+ }
}
+
tree.keySet()
.retainAll(permissionBackend.currentUser().filter(ProjectPermission.ACCESS, tree.keySet()));
return tree.values();
diff --git a/java/com/google/gerrit/server/restapi/project/ListProjects.java b/java/com/google/gerrit/server/restapi/project/ListProjects.java
index a1572c6..9a8232e 100644
--- a/java/com/google/gerrit/server/restapi/project/ListProjects.java
+++ b/java/com/google/gerrit/server/restapi/project/ListProjects.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.restapi.project;
+import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.gerrit.extensions.client.ProjectState.HIDDEN;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.toList;
@@ -521,11 +522,28 @@
if (type == FilterType.PARENT_CANDIDATES) {
matches = parentsOf(matches);
}
- // TODO(dborowitz): Streamified PermissionBackend#filter.
- return perm.filter(ProjectPermission.ACCESS, matches.collect(toList()))
- .stream()
- .sorted()
- .collect(toList());
+
+ List<Project.NameKey> results = new ArrayList<>();
+ List<Project.NameKey> projectNameKeys = matches.sorted().collect(toList());
+ for (Project.NameKey nameKey : projectNameKeys) {
+ ProjectState state = projectCache.get(nameKey);
+ checkNotNull(state, "Failed to load project %s", nameKey);
+
+ // Hidden projects(permitsRead = false) should only be accessible by the project owners.
+ // READ_CONFIG is checked here because it's only allowed to project owners(ACCESS may also
+ // be allowed for other users). Allowing project owners to access here will help them to view
+ // and update the config of hidden projects easily.
+ ProjectPermission permissionToCheck =
+ state.statePermitsRead() ? ProjectPermission.ACCESS : ProjectPermission.READ_CONFIG;
+ try {
+ perm.project(nameKey).check(permissionToCheck);
+ results.add(nameKey);
+ } catch (AuthException e) {
+ // Not added to results.
+ }
+ }
+
+ return results;
}
private Stream<Project.NameKey> parentsOf(Stream<Project.NameKey> matches) {
@@ -551,13 +569,19 @@
}
private boolean isParentAccessible(
- Map<Project.NameKey, Boolean> checked, PermissionBackend.WithUser perm, ProjectState p)
+ Map<Project.NameKey, Boolean> checked, PermissionBackend.WithUser perm, ProjectState state)
throws PermissionBackendException {
- Project.NameKey name = p.getNameKey();
+ Project.NameKey name = state.getNameKey();
Boolean b = checked.get(name);
if (b == null) {
try {
- perm.project(name).check(ProjectPermission.ACCESS);
+ // Hidden projects(permitsRead = false) should only be accessible by the project owners.
+ // READ_CONFIG is checked here because it's only allowed to project owners(ACCESS may also
+ // be allowed for other users). Allowing project owners to access here will help them to view
+ // and update the config of hidden projects easily.
+ ProjectPermission permissionToCheck =
+ state.statePermitsRead() ? ProjectPermission.ACCESS : ProjectPermission.READ_CONFIG;
+ perm.project(name).check(permissionToCheck);
b = true;
} catch (AuthException denied) {
b = false;
diff --git a/java/com/google/gerrit/server/restapi/project/ProjectsCollection.java b/java/com/google/gerrit/server/restapi/project/ProjectsCollection.java
index c5fb41e..3af8424 100644
--- a/java/com/google/gerrit/server/restapi/project/ProjectsCollection.java
+++ b/java/com/google/gerrit/server/restapi/project/ProjectsCollection.java
@@ -150,8 +150,14 @@
}
if (checkAccess) {
+ // Hidden projects(permitsRead = false) should only be accessible by the project owners.
+ // READ_CONFIG is checked here because it's only allowed to project owners(ACCESS may also
+ // be allowed for other users). Allowing project owners to access here will help them to view
+ // and update the config of hidden projects easily.
+ ProjectPermission permissionToCheck =
+ state.statePermitsRead() ? ProjectPermission.ACCESS : ProjectPermission.READ_CONFIG;
try {
- permissionBackend.currentUser().project(nameKey).check(ProjectPermission.ACCESS);
+ permissionBackend.currentUser().project(nameKey).check(permissionToCheck);
} catch (AuthException e) {
return null; // Pretend like not found on access denied.
}
diff --git a/java/com/google/gerrit/sshd/commands/KillCommand.java b/java/com/google/gerrit/sshd/commands/KillCommand.java
index a7e751a..ef12f5f 100644
--- a/java/com/google/gerrit/sshd/commands/KillCommand.java
+++ b/java/com/google/gerrit/sshd/commands/KillCommand.java
@@ -20,6 +20,7 @@
import com.google.gerrit.extensions.annotations.RequiresAnyCapability;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.server.config.ConfigResource;
import com.google.gerrit.server.config.TaskResource;
@@ -51,7 +52,10 @@
try {
TaskResource taskRsrc = tasksCollection.parse(cfgRsrc, IdString.fromDecoded(id));
deleteTask.apply(taskRsrc, null);
- } catch (AuthException | ResourceNotFoundException | PermissionBackendException e) {
+ } catch (AuthException
+ | ResourceNotFoundException
+ | ResourceConflictException
+ | PermissionBackendException e) {
stderr.print("kill: " + id + ": No such task\n");
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java b/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
index 7d60b8d..95b64e0 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
@@ -15,6 +15,7 @@
package com.google.gerrit.acceptance.rest.account;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.acceptance.GitUtil.fetch;
import static com.google.gerrit.acceptance.GitUtil.pushHead;
import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_MAILTO;
@@ -31,6 +32,7 @@
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo;
@@ -74,6 +76,7 @@
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.notes.NoteMap;
+import org.eclipse.jgit.revwalk.FooterLine;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.PushResult;
@@ -779,6 +782,130 @@
assertThat(externalIds.byAccount(admin.id)).containsExactlyElementsIn(expectedExternalIds);
}
+ @Test
+ public void unsetEmail() throws Exception {
+ ExternalId extId = ExternalId.createWithEmail("x", "1", user.id, "x@example.com");
+ insertExtId(extId);
+
+ ExternalId extIdWithoutEmail = ExternalId.create("x", "1", user.id);
+ try (Repository allUsersRepo = repoManager.openRepository(allUsers);
+ MetaDataUpdate md = metaDataUpdateFactory.create(allUsers)) {
+ ExternalIdNotes extIdNotes = externalIdNotesFactory.load(allUsersRepo);
+ extIdNotes.upsert(extIdWithoutEmail);
+ extIdNotes.commit(md);
+
+ assertThat(extIdNotes.get(extId.key())).hasValue(extIdWithoutEmail);
+ }
+ }
+
+ @Test
+ public void unsetHttpPassword() throws Exception {
+ ExternalId extId =
+ ExternalId.createWithPassword(ExternalId.Key.create("y", "1"), user.id, null, "secret");
+ insertExtId(extId);
+
+ ExternalId extIdWithoutPassword = ExternalId.create("y", "1", user.id);
+ try (Repository allUsersRepo = repoManager.openRepository(allUsers);
+ MetaDataUpdate md = metaDataUpdateFactory.create(allUsers)) {
+ ExternalIdNotes extIdNotes = externalIdNotesFactory.load(allUsersRepo);
+ extIdNotes.upsert(extIdWithoutPassword);
+ extIdNotes.commit(md);
+
+ assertThat(extIdNotes.get(extId.key())).hasValue(extIdWithoutPassword);
+ }
+ }
+
+ @Test
+ public void footers() throws Exception {
+ // Insert external ID for different accounts
+ TestAccount user1 = accountCreator.create("user1");
+ TestAccount user2 = accountCreator.create("user2");
+ ExternalId extId1 = ExternalId.create("foo", "1", user1.id);
+ ExternalId extId2 = ExternalId.create("foo", "2", user1.id);
+ ExternalId extId3 = ExternalId.create("foo", "3", user2.id);
+ try (Repository allUsersRepo = repoManager.openRepository(allUsers);
+ MetaDataUpdate md = metaDataUpdateFactory.create(allUsers)) {
+ ExternalIdNotes extIdNotes = externalIdNotesFactory.load(allUsersRepo);
+ extIdNotes.insert(ImmutableSet.of(extId1, extId2, extId3));
+ RevCommit c = extIdNotes.commit(md);
+ assertThat(getFooters(c))
+ .containsExactly("Account: " + user1.getId(), "Account: " + user2.getId())
+ .inOrder();
+ }
+
+ // Insert external ID with different emails
+ ExternalId extId4 = ExternalId.createWithEmail("foo", "4", user1.id, "foo4@example.com");
+ ExternalId extId5 = ExternalId.createWithEmail("foo", "5", user2.id, "foo5@example.com");
+ try (Repository allUsersRepo = repoManager.openRepository(allUsers);
+ MetaDataUpdate md = metaDataUpdateFactory.create(allUsers)) {
+ ExternalIdNotes extIdNotes = externalIdNotesFactory.load(allUsersRepo);
+ extIdNotes.insert(ImmutableSet.of(extId4, extId5));
+ RevCommit c = extIdNotes.commit(md);
+ assertThat(getFooters(c))
+ .containsExactly(
+ "Account: " + user1.getId(),
+ "Account: " + user2.getId(),
+ "Email: foo4@example.com",
+ "Email: foo5@example.com")
+ .inOrder();
+ }
+
+ // Update external ID - Add Email
+ ExternalId extId1a = ExternalId.createWithEmail("foo", "1", user1.id, "foo1@example.com");
+ try (Repository allUsersRepo = repoManager.openRepository(allUsers);
+ MetaDataUpdate md = metaDataUpdateFactory.create(allUsers)) {
+ ExternalIdNotes extIdNotes = externalIdNotesFactory.load(allUsersRepo);
+ extIdNotes.upsert(extId1a);
+ RevCommit c = extIdNotes.commit(md);
+ assertThat(getFooters(c))
+ .containsExactly("Account: " + user1.getId(), "Email: foo1@example.com")
+ .inOrder();
+ }
+
+ // Update external ID - Remove Email
+ try (Repository allUsersRepo = repoManager.openRepository(allUsers);
+ MetaDataUpdate md = metaDataUpdateFactory.create(allUsers)) {
+ ExternalIdNotes extIdNotes = externalIdNotesFactory.load(allUsersRepo);
+ extIdNotes.upsert(extId1);
+ RevCommit c = extIdNotes.commit(md);
+ assertThat(getFooters(c))
+ .containsExactly("Account: " + user1.getId(), "Email: foo1@example.com")
+ .inOrder();
+ }
+
+ // Delete external IDs
+ try (Repository allUsersRepo = repoManager.openRepository(allUsers);
+ MetaDataUpdate md = metaDataUpdateFactory.create(allUsers)) {
+ ExternalIdNotes extIdNotes = externalIdNotesFactory.load(allUsersRepo);
+ extIdNotes.delete(ImmutableSet.of(extId1, extId5));
+ RevCommit c = extIdNotes.commit(md);
+ assertThat(getFooters(c))
+ .containsExactly(
+ "Account: " + user1.getId(), "Account: " + user2.getId(), "Email: foo5@example.com")
+ .inOrder();
+ }
+
+ // Delete external ID by key without email
+ try (Repository allUsersRepo = repoManager.openRepository(allUsers);
+ MetaDataUpdate md = metaDataUpdateFactory.create(allUsers)) {
+ ExternalIdNotes extIdNotes = externalIdNotesFactory.load(allUsersRepo);
+ extIdNotes.delete(extId2.accountId(), extId2.key());
+ RevCommit c = extIdNotes.commit(md);
+ assertThat(getFooters(c)).containsExactly("Account: " + user1.getId()).inOrder();
+ }
+
+ // Delete external ID by key with email
+ try (Repository allUsersRepo = repoManager.openRepository(allUsers);
+ MetaDataUpdate md = metaDataUpdateFactory.create(allUsers)) {
+ ExternalIdNotes extIdNotes = externalIdNotesFactory.load(allUsersRepo);
+ extIdNotes.delete(extId4.accountId(), extId4.key());
+ RevCommit c = extIdNotes.commit(md);
+ assertThat(getFooters(c))
+ .containsExactly("Account: " + user1.getId(), "Email: foo4@example.com")
+ .inOrder();
+ }
+ }
+
private void insertExtId(ExternalId extId) throws Exception {
accountsUpdateProvider
.get()
@@ -823,6 +950,10 @@
}
}
+ private List<String> getFooters(RevCommit c) {
+ return c.getFooterLines().stream().map(FooterLine::toString).collect(toList());
+ }
+
private List<AccountExternalIdInfo> toExternalIdInfos(Collection<ExternalId> extIds) {
return extIds.stream().map(this::toExternalIdInfo).collect(toList());
}
diff --git a/plugins/replication b/plugins/replication
index dc1e7ff..9b08f32 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit dc1e7ff434fd7fce90dd6f6c4e312280bd884bd0
+Subproject commit 9b08f324462e62872ac071175e16e7553ba76e2b
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.html b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.html
index 8e179cc..5d69499 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.html
@@ -105,6 +105,7 @@
<gr-autocomplete
id="parentInput"
query="[[_query]]"
+ no-debounce
text="{{_inputText}}"
on-tap="_handleEnterChangeNumberTap"
on-commit="_handleBaseSelected"
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_test.html b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_test.html
index 24815db..b92fe1d 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_test.html
@@ -166,7 +166,8 @@
test('input text change triggers function', () => {
sandbox.spy(element, '_getRecentChanges');
- element._inputText = '1';
+ element.$.parentInput.noDebounce = true;
+ element.$.parentInput.text = '1';
assert.isTrue(element._getRecentChanges.calledOnce);
element._inputText = '12';
assert.isTrue(element._getRecentChanges.calledTwice);
diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.html b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.html
index 169fd85..fdd730d 100644
--- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.html
+++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.html
@@ -48,7 +48,6 @@
id="searchInput"
text="{{_inputVal}}"
query="[[query]]"
- debounce-wait="200"
on-commit="_handleInputCommit"
allow-non-suggested-values
multi
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.html b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.html
index 9d85d37..c67a2af 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.html
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.html
@@ -61,12 +61,14 @@
suite('edit button CUJ', () => {
let navStubs;
+ let openAutoCcmplete;
setup(() => {
navStubs = [
sandbox.stub(Gerrit.Nav, 'getEditUrlForDiff'),
sandbox.stub(Gerrit.Nav, 'navigateToRelativeUrl'),
];
+ openAutoCcmplete = element.$.openDialog.querySelector('gr-autocomplete');
});
test('_isValidPath', () => {
@@ -84,8 +86,8 @@
assert.isTrue(element._hideAllDialogs.called);
assert.isTrue(element.$.openDialog.disabled);
assert.isFalse(queryStub.called);
- element.$.openDialog.querySelector('gr-autocomplete').text =
- 'src/test.cpp';
+ openAutoCcmplete.noDebounce = true;
+ openAutoCcmplete.text = 'src/test.cpp';
assert.isTrue(queryStub.called);
assert.isFalse(element.$.openDialog.disabled);
MockInteractions.tap(element.$.openDialog.$$('gr-button[primary]'));
@@ -100,8 +102,8 @@
MockInteractions.tap(element.$$('#open'));
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element.$.openDialog.disabled);
- element.$.openDialog.querySelector('gr-autocomplete').text =
- 'src/test.cpp';
+ openAutoCcmplete.noDebounce = true;
+ openAutoCcmplete.text = 'src/test.cpp';
assert.isFalse(element.$.openDialog.disabled);
MockInteractions.tap(element.$.openDialog.$$('gr-button'));
for (const stub of navStubs) { assert.isFalse(stub.called); }
@@ -114,10 +116,13 @@
suite('delete button CUJ', () => {
let navStub;
let deleteStub;
+ let deleteAutocomplete;
setup(() => {
navStub = sandbox.stub(Gerrit.Nav, 'navigateToChange');
deleteStub = sandbox.stub(element.$.restAPI, 'deleteFileInChangeEdit');
+ deleteAutocomplete =
+ element.$.deleteDialog.querySelector('gr-autocomplete');
});
test('delete', () => {
@@ -126,8 +131,8 @@
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element.$.deleteDialog.disabled);
assert.isFalse(queryStub.called);
- element.$.deleteDialog.querySelector('gr-autocomplete').text =
- 'src/test.cpp';
+ deleteAutocomplete.noDebounce = true;
+ deleteAutocomplete.text = 'src/test.cpp';
assert.isTrue(queryStub.called);
assert.isFalse(element.$.deleteDialog.disabled);
MockInteractions.tap(element.$.deleteDialog.$$('gr-button[primary]'));
@@ -149,8 +154,8 @@
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element.$.deleteDialog.disabled);
assert.isFalse(queryStub.called);
- element.$.deleteDialog.querySelector('gr-autocomplete').text =
- 'src/test.cpp';
+ deleteAutocomplete.noDebounce = true;
+ deleteAutocomplete.text = 'src/test.cpp';
assert.isTrue(queryStub.called);
assert.isFalse(element.$.deleteDialog.disabled);
MockInteractions.tap(element.$.deleteDialog.$$('gr-button[primary]'));
@@ -183,10 +188,13 @@
suite('rename button CUJ', () => {
let navStub;
let renameStub;
+ let renameAutocomplete;
setup(() => {
navStub = sandbox.stub(Gerrit.Nav, 'navigateToChange');
renameStub = sandbox.stub(element.$.restAPI, 'renameFileInChangeEdit');
+ renameAutocomplete =
+ element.$.renameDialog.querySelector('gr-autocomplete');
});
test('rename', () => {
@@ -195,8 +203,8 @@
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element.$.renameDialog.disabled);
assert.isFalse(queryStub.called);
- element.$.renameDialog.querySelector('gr-autocomplete').text =
- 'src/test.cpp';
+ renameAutocomplete.noDebounce = true;
+ renameAutocomplete.text = 'src/test.cpp';
assert.isTrue(queryStub.called);
assert.isTrue(element.$.renameDialog.disabled);
@@ -223,8 +231,8 @@
return showDialogSpy.lastCall.returnValue.then(() => {
assert.isTrue(element.$.renameDialog.disabled);
assert.isFalse(queryStub.called);
- element.$.renameDialog.querySelector('gr-autocomplete').text =
- 'src/test.cpp';
+ renameAutocomplete.noDebounce = true;
+ renameAutocomplete.text = 'src/test.cpp';
assert.isTrue(queryStub.called);
assert.isTrue(element.$.renameDialog.disabled);
diff --git a/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.html b/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.html
index 0b8c331..558140f 100644
--- a/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.html
+++ b/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.html
@@ -96,7 +96,6 @@
<th>
<gr-autocomplete
id="newProject"
- debounce-wait="200"
query="[[_query]]"
threshold="1"
placeholder="Project"></gr-autocomplete>
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
index 5cfd1c0..1eaad4e 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
@@ -18,6 +18,7 @@
'use strict';
const TOKENIZE_REGEX = /(?:[^\s"]+|"[^"]*")+/g;
+ const DEBOUNCE_WAIT_MS = 200;
Polymer({
is: 'gr-autocomplete',
@@ -132,12 +133,11 @@
},
/**
- * The number of milliseconds to use as the debounce wait time. If null,
- * no debouncing is used.
+ * When true, querying for suggestions is not debounced w/r/t keypresses
*/
- debounceWait: {
- type: Number,
- value: null,
+ noDebounce: {
+ type: Boolean,
+ value: false,
},
/** @type {?} */
@@ -167,7 +167,7 @@
observers: [
'_maybeOpenDropdown(_suggestions, _focused)',
- '_updateSuggestions(text, threshold, debounceWait)',
+ '_updateSuggestions(text, threshold, noDebounce)',
],
attached() {
@@ -176,6 +176,7 @@
detached() {
this.unlisten(document.body, 'tap', '_handleBodyTap');
+ this.cancelDebouncer('update-suggestions');
},
get focusStart() {
@@ -219,7 +220,7 @@
_onInputFocus() {
this._focused = true;
- this._updateSuggestions(this.text, this.threshold, this.debounceWait);
+ this._updateSuggestions(this.text, this.threshold, this.noDebounce);
this.$.input.classList.remove('warnUncommitted');
// Needed so that --paper-input-container-input updated style is applied.
this.updateStyles();
@@ -232,7 +233,7 @@
this.updateStyles();
},
- _updateSuggestions(text, threshold, debounceWait) {
+ _updateSuggestions(text, threshold, noDebounce) {
if (this._disableSuggestions) { return; }
if (text === undefined || text.length < threshold) {
this._suggestions = [];
@@ -257,10 +258,10 @@
});
};
- if (debounceWait) {
- this.debounce('update-suggestions', update, debounceWait);
- } else {
+ if (noDebounce) {
update();
+ } else {
+ this.debounce('update-suggestions', update, DEBOUNCE_WAIT_MS);
}
},
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
index 7b1eed2..872f79f 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
@@ -28,7 +28,7 @@
<test-fixture id="basic">
<template>
- <gr-autocomplete></gr-autocomplete>
+ <gr-autocomplete no-debounce></gr-autocomplete>
</template>
</test-fixture>
@@ -236,7 +236,7 @@
assert.isTrue(queryStub.called);
});
- test('debounceWait debounces the query', () => {
+ test('noDebounce=false debounces the query', () => {
const queryStub = sandbox.spy(() => {
return Promise.resolve([]);
});
@@ -244,11 +244,11 @@
const debounceStub = sandbox.stub(element, 'debounce',
(name, cb) => { callback = cb; });
element.query = queryStub;
- element.debounceWait = 100;
+ element.noDebounce = false;
element.text = 'a';
assert.isFalse(queryStub.called);
assert.isTrue(debounceStub.called);
- assert.equal(debounceStub.lastCall.args[2], 100);
+ assert.equal(debounceStub.lastCall.args[2], 200);
assert.isFunction(callback);
callback();
assert.isTrue(queryStub.called);
