opendev/gerrit
Code Issues Proposed changes

Add project config boolean to require signed push on a project

Browse Source 
This is controlled by receive.requireSignedPush in the project.config,
which is a separate bit from receive.enableSignedPush. (Adding an
inheritable tri-state enum would have been complex to implement and
have hard-to-define semantics.)

requireSignedPush is only inspected if enableSignedPush is true; this
allows project owners to temporarily disable signed push entirely e.g.
due to a bug, without having to flip both bits.

Change-Id: I07999b6fa185d470b30509941473e3158f9dfa2c
This commit is contained in:
Dave Borowitz 2015-10-20 10:35:26 -04:00
parent ff473bb345
commit 0543c735eb
14 changed files with 117 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
Documentation/config-project-config.txt
View File

@ -164,6 +164,17 @@ configuration] for details.
Default is `INHERIT`, which means that this property is inherited from
the parent project.
[[receive.requireSignedPush]]receive.requireSignedPush::
+
Controls whether server-side signed push validation is required on the
project. Only has an effect if signed push validation is enabled on the
server, and link:#receive.enableSignedPush is set on the project. See
the link:config-gerrit.html#receive.enableSignedPush[global
configuration] for details.
+
Default is `INHERIT`, which means that this property is inherited from
the parent project.
[[submit-section]]
=== Submit section

13
Documentation/rest-api-projects.txt
View File

@ -732,6 +732,7 @@ link:#config-input[ConfigInput] entity.
"use_signed_off_by": "INHERIT",
"create_new_change_for_all_not_in_target": "INHERIT",
"enable_signed_push": "INHERIT",
"require_signed_push": "INHERIT",
"require_change_id": "TRUE",
"max_object_size_limit": "10m",
"submit_type": "REBASE_IF_NECESSARY",
@ -780,6 +781,11 @@ ConfigInfo] entity.
"configured_value": "INHERIT",
"inherited_value": false
},
"require_signed_push": {
"value": false,
"configured_value": "INHERIT",
"inherited_value": false
},
"max_object_size_limit": {
"value": "10m",
"configured_value": "10m",
@ -1982,9 +1988,14 @@ link:#inherited-boolean-info[InheritedBooleanInfo] that tells whether a
valid link:user-changeid.html[Change-Id] footer in any commit uploaded
for review is required. This does not apply to commits pushed directly
to a branch or tag.
|`enable_signed_push` |optional|
|`enable_signed_push`|
optional, not set if signed push is disabled|
link:#inherited-boolean-info[InheritedBooleanInfo] that tells whether
signed push validation is enabled on the project.
|`require_signed_push`|
optional, not set if signed push is disabled
link:#inherited-boolean-info[InheritedBooleanInfo] that tells whether
signed push validation is required on the project.
|`max_object_size_limit` ||
The link:config-gerrit.html#receive.maxObjectSizeLimit[max object size
limit] of this project as a link:#max-object-size-limit-info[

19
gerrit-gpg/src/main/java/com/google/gerrit/gpg/SignedPushModule.java
View File

@ -15,7 +15,6 @@
package com.google.gerrit.gpg;
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.EnableSignedPush;
@ -33,6 +32,7 @@ import com.google.inject.Singleton;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.transport.PreReceiveHook;
import org.eclipse.jgit.transport.PreReceiveHookChain;
import org.eclipse.jgit.transport.ReceivePack;
import org.eclipse.jgit.transport.SignedPushConfig;
@ -42,6 +42,8 @@ import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.List;
import java.util.Random;
class SignedPushModule extends AbstractModule {
@ -92,15 +94,22 @@ class SignedPushModule extends AbstractModule {
if (!ps.isEnableSignedPush()) {
rp.setSignedPushConfig(null);
return;
}
if (signedPushConfig == null) {
} else if (signedPushConfig == null) {
log.error("receive.enableSignedPush is true for project {} but"
+ " false in gerrit.config, so signed push verification is"
+ " disabled", project.get());
rp.setSignedPushConfig(null);
return;
}
rp.setSignedPushConfig(signedPushConfig);
rp.setPreReceiveHook(PreReceiveHookChain.newChain(Lists.newArrayList(
hook, rp.getPreReceiveHook())));
List<PreReceiveHook> hooks = new ArrayList<>(3);
if (ps.isRequireSignedPush()) {
hooks.add(SignedPushPreReceiveHook.Required.INSTANCE);
}
hooks.add(hook);
hooks.add(rp.getPreReceiveHook());
rp.setPreReceiveHook(PreReceiveHookChain.newChain(hooks));
}
}

15
gerrit-gpg/src/main/java/com/google/gerrit/gpg/SignedPushPreReceiveHook.java
View File

@ -36,6 +36,21 @@ import java.util.Collection;
*/
@Singleton
public class SignedPushPreReceiveHook implements PreReceiveHook {
public static class Required implements PreReceiveHook {
public static final Required INSTANCE = new Required();
@Override
public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
if (rp.getPushCertificate() == null) {
rp.sendMessage("ERROR: Signed push is required");
reject(commands, "push cert error");
}
}
private Required() {
}
}
private final Provider<IdentifiedUser> user;
private final GerritPushCertificateChecker.Factory checkerFactory;

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.java
View File

@ -43,6 +43,7 @@ public interface AdminConstants extends Constants {
String useSignedOffBy();
String createNewChangeForAllNotInTarget();
String enableSignedPush();
String requireSignedPush();
String requireChangeID();
String headingMaxObjectSizeLimit();
String headingGroupOptions();

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties
View File

@ -25,6 +25,7 @@ useContributorAgreements = Require a valid contributor agreement to upload
useSignedOffBy = Require <code>Signed-off-by</code> in commit message
createNewChangeForAllNotInTarget = Create a new change for every commit not in the target branch
enableSignedPush = Enable signed push
requireSignedPush = Require signed push
requireChangeID = Require <code>Change-Id</code> in commit message
headingMaxObjectSizeLimit = Maximum Git object size limit
headingGroupOptions = Group Options

16
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectInfoScreen.java
View File

@ -84,6 +84,7 @@ public class ProjectInfoScreen extends ProjectScreen {
private ListBox contentMerge;
private ListBox newChangeForAllNotInTarget;
private ListBox enableSignedPush;
private ListBox requireSignedPush;
private NpTextBox maxObjectSizeLimit;
private Label effectiveMaxObjectSizeLimit;
private Map<String, Map<String, HasEnabled>> pluginConfigWidgets;
@ -247,6 +248,9 @@ public class ProjectInfoScreen extends ProjectScreen {
enableSignedPush = newInheritedBooleanBox();
saveEnabler.listenTo(enableSignedPush);
grid.add(Util.C.enableSignedPush(), enableSignedPush);
requireSignedPush = newInheritedBooleanBox();
saveEnabler.listenTo(requireSignedPush);
grid.add(Util.C.requireSignedPush(), requireSignedPush);
}
maxObjectSizeLimit = new NpTextBox();
@ -326,6 +330,9 @@ public class ProjectInfoScreen extends ProjectScreen {
}
private void setBool(ListBox box, InheritedBooleanInfo inheritedBoolean) {
if (box == null) {
return;
}
int inheritedIndex = -1;
for (int i = 0; i < box.getItemCount(); i++) {
if (box.getValue(i).startsWith(InheritableBoolean.INHERIT.name())) {
@ -372,8 +379,9 @@ public class ProjectInfoScreen extends ProjectScreen {
setBool(contentMerge, result.useContentMerge());
setBool(newChangeForAllNotInTarget, result.createNewChangeForAllNotInTarget());
setBool(requireChangeID, result.requireChangeId());
if (enableSignedPush != null) {
if (Gerrit.info().receive().enableSignedPush()) {
setBool(enableSignedPush, result.enableSignedPush());
setBool(requireSignedPush, result.requireSignedPush());
}
setSubmitType(result.submitType());
setState(result.state());
@ -644,12 +652,14 @@ public class ProjectInfoScreen extends ProjectScreen {
private void doSave() {
enableForm(false);
saveProject.setEnabled(false);
InheritableBoolean sp = enableSignedPush != null
InheritableBoolean esp = enableSignedPush != null
? getBool(enableSignedPush) : null;
InheritableBoolean rsp = requireSignedPush != null
? getBool(requireSignedPush) : null;
ProjectApi.setConfig(getProjectKey(), descTxt.getText().trim(),
getBool(contributorAgreements), getBool(contentMerge),
getBool(signedOffBy), getBool(newChangeForAllNotInTarget), getBool(requireChangeID),
sp,
esp, rsp,
maxObjectSizeLimit.getText().trim(),
SubmitType.valueOf(submitType.getValue(submitType.getSelectedIndex())),
ProjectState.valueOf(state.getValue(state.getSelectedIndex())),

3
gerrit-gwtui/src/main/java/com/google/gerrit/client/projects/ConfigInfo.java
View File

@ -53,6 +53,9 @@ public class ConfigInfo extends JavaScriptObject {
public final native InheritedBooleanInfo enableSignedPush()
/*-{ return this.enable_signed_push; }-*/;
public final native InheritedBooleanInfo requireSignedPush()
/*-{ return this.require_signed_push; }-*/;
public final SubmitType submitType() {
return SubmitType.valueOf(submitTypeRaw());
}

10
gerrit-gwtui/src/main/java/com/google/gerrit/client/projects/ProjectApi.java
View File

@ -117,6 +117,7 @@ public class ProjectApi {
InheritableBoolean createNewChangeForAllNotInTarget,
InheritableBoolean requireChangeId,
InheritableBoolean enableSignedPush,
InheritableBoolean requireSignedPush,
String maxObjectSizeLimit,
SubmitType submitType, ProjectState state,
Map<String, Map<String, ConfigParameterValue>> pluginConfigValues,
@ -131,6 +132,9 @@ public class ProjectApi {
if (enableSignedPush != null) {
in.setEnableSignedPush(enableSignedPush);
}
if (requireSignedPush != null) {
in.setRequireSignedPush(requireSignedPush);
}
in.setMaxObjectSizeLimit(maxObjectSizeLimit);
in.setSubmitType(submitType);
in.setState(state);
@ -257,6 +261,12 @@ public class ProjectApi {
private final native void setEnableSignedPushRaw(String v)
/*-{ if(v)this.enable_signed_push=v; }-*/;
final void setRequireSignedPush(InheritableBoolean v) {
setRequireSignedPushRaw(v.name());
}
private final native void setRequireSignedPushRaw(String v)
/*-{ if(v)this.require_signed_push=v; }-*/;
final native void setMaxObjectSizeLimit(String l)
/*-{ if(l)this.max_object_size_limit=l; }-*/;

10
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Project.java
View File

@ -97,6 +97,7 @@ public final class Project {
protected InheritableBoolean createNewChangeForAllNotInTarget;
protected InheritableBoolean enableSignedPush;
protected InheritableBoolean requireSignedPush;
protected Project() {
}
@ -111,6 +112,7 @@ public final class Project {
useContentMerge = InheritableBoolean.INHERIT;
createNewChangeForAllNotInTarget = InheritableBoolean.INHERIT;
enableSignedPush = InheritableBoolean.INHERIT;
requireSignedPush = InheritableBoolean.INHERIT;
}
public Project.NameKey getNameKey() {
@ -182,6 +184,14 @@ public final class Project {
enableSignedPush = enable;
}
public InheritableBoolean getRequireSignedPush() {
return requireSignedPush;
}
public void setRequireSignedPush(InheritableBoolean require) {
requireSignedPush = require;
}
public void setMaxObjectSizeLimit(final String limit) {
maxObjectSizeLimit = limit;
}

5
gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
View File

@ -118,6 +118,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
"requireContributorAgreement";
private static final String KEY_CHECK_RECEIVED_OBJECTS = "checkReceivedObjects";
private static final String KEY_ENABLE_SIGNED_PUSH = "enableSignedPush";
private static final String KEY_REQUIRE_SIGNED_PUSH = "requireSignedPush";
private static final String SUBMIT = "submit";
private static final String KEY_ACTION = "action";
@ -420,6 +421,8 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
p.setCreateNewChangeForAllNotInTarget(getEnum(rc, RECEIVE, null, KEY_USE_ALL_NOT_IN_TARGET, InheritableBoolean.INHERIT));
p.setEnableSignedPush(getEnum(rc, RECEIVE, null,
KEY_ENABLE_SIGNED_PUSH, InheritableBoolean.INHERIT));
p.setRequireSignedPush(getEnum(rc, RECEIVE, null,
KEY_REQUIRE_SIGNED_PUSH, InheritableBoolean.INHERIT));
p.setMaxObjectSizeLimit(rc.getString(RECEIVE, null, KEY_MAX_OBJECT_SIZE_LIMIT));
p.setSubmitType(getEnum(rc, SUBMIT, null, KEY_ACTION, defaultSubmitAction));
@ -828,6 +831,8 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
set(rc, RECEIVE, null, KEY_MAX_OBJECT_SIZE_LIMIT, validMaxObjectSizeLimit(p.getMaxObjectSizeLimit()));
set(rc, RECEIVE, null, KEY_ENABLE_SIGNED_PUSH,
p.getEnableSignedPush(), InheritableBoolean.INHERIT);
set(rc, RECEIVE, null, KEY_REQUIRE_SIGNED_PUSH,
p.getRequireSignedPush(), InheritableBoolean.INHERIT);
set(rc, SUBMIT, null, KEY_ACTION, p.getSubmitType(), defaultSubmitAction);
set(rc, SUBMIT, null, KEY_MERGE_CONTENT, p.getUseContentMerge(), InheritableBoolean.INHERIT);

5
gerrit-server/src/main/java/com/google/gerrit/server/project/ConfigInfo.java
View File

@ -46,6 +46,7 @@ public class ConfigInfo {
public InheritedBooleanInfo createNewChangeForAllNotInTarget;
public InheritedBooleanInfo requireChangeId;
public InheritedBooleanInfo enableSignedPush;
public InheritedBooleanInfo requireSignedPush;
public MaxObjectSizeLimitInfo maxObjectSizeLimit;
public SubmitType submitType;
public com.google.gerrit.extensions.client.ProjectState state;
@ -74,6 +75,7 @@ public class ConfigInfo {
InheritedBooleanInfo createNewChangeForAllNotInTarget =
new InheritedBooleanInfo();
InheritedBooleanInfo enableSignedPush = new InheritedBooleanInfo();
InheritedBooleanInfo requireSignedPush = new InheritedBooleanInfo();
useContributorAgreements.value = projectState.isUseContributorAgreements();
useSignedOffBy.value = projectState.isUseSignedOffBy();
@ -90,6 +92,7 @@ public class ConfigInfo {
createNewChangeForAllNotInTarget.configuredValue =
p.getCreateNewChangeForAllNotInTarget();
enableSignedPush.configuredValue = p.getEnableSignedPush();
requireSignedPush.configuredValue = p.getRequireSignedPush();
ProjectState parentState = Iterables.getFirst(projectState
.parents(), null);
@ -102,6 +105,7 @@ public class ConfigInfo {
createNewChangeForAllNotInTarget.inheritedValue =
parentState.isCreateNewChangeForAllNotInTarget();
enableSignedPush.inheritedValue = projectState.isEnableSignedPush();
requireSignedPush.inheritedValue = projectState.isRequireSignedPush();
}
this.useContributorAgreements = useContributorAgreements;
@ -111,6 +115,7 @@ public class ConfigInfo {
this.createNewChangeForAllNotInTarget = createNewChangeForAllNotInTarget;
if (serverEnableSignedPush) {
this.enableSignedPush = enableSignedPush;
this.requireSignedPush = requireSignedPush;
}
MaxObjectSizeLimitInfo maxObjectSizeLimit = new MaxObjectSizeLimitInfo();

9
gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java
View File

@ -413,6 +413,15 @@ public class ProjectState {
});
}
public boolean isRequireSignedPush() {
return getInheritableBoolean(new Function<Project, InheritableBoolean>() {
@Override
public InheritableBoolean apply(Project input) {
return input.getRequireSignedPush();
}
});
}
public LabelTypes getLabelTypes() {
Map<String, LabelType> types = Maps.newLinkedHashMap();
for (ProjectState s : treeInOrder()) {

10
gerrit-server/src/main/java/com/google/gerrit/server/project/PutConfig.java
View File

@ -70,6 +70,7 @@ public class PutConfig implements RestModifyView<ProjectResource, Input> {
public InheritableBoolean createNewChangeForAllNotInTarget;
public InheritableBoolean requireChangeId;
public InheritableBoolean enableSignedPush;
public InheritableBoolean requireSignedPush;
public String maxObjectSizeLimit;
public SubmitType submitType;
public com.google.gerrit.extensions.client.ProjectState state;
@ -166,8 +167,13 @@ public class PutConfig implements RestModifyView<ProjectResource, Input> {
p.setRequireChangeID(input.requireChangeId);
}
if (input.enableSignedPush != null) {
p.setEnableSignedPush(input.enableSignedPush);
if (serverEnableSignedPush) {
if (input.enableSignedPush != null) {
p.setEnableSignedPush(input.enableSignedPush);
}
if (input.requireSignedPush != null) {
p.setRequireSignedPush(input.requireSignedPush);
}
}
if (input.maxObjectSizeLimit != null) {