opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.16' into stable-3.0

Browse Source 
* stable-2.16:
  PolyGerrit: Fix typos in comments
  gr-create-project-dialog: Fix typo in element id name
  AccountIT: Fix typo in variable name
  Set version to 2.16.9-SNAPSHOT
  AccountIT: Add tests for email notification on adding SSH/GPG keys
  AccountApi: Add methods to generate and set the HTTP password

Change-Id: I4f27b310c3bc100fb043c070012afe980905c9ec
This commit is contained in:
David Pursehouse
2019-05-02 09:36:50 +09:00
parent b8ec7e243f 3255182532
commit 92fb7f7dbe
8 changed files with 151 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
java/com/google/gerrit/extensions/api/accounts/AccountApi.java
View File

@@ -114,6 +114,23 @@ public interface AccountApi {
void setName(String name) throws RestApiException;
/**
* Generate a new HTTP password.
*
* @return the generated password.
*/
String generateHttpPassword() throws RestApiException;
/**
* Set a new HTTP password.
*
* <p>May only be invoked by administrators.
*
* @param httpPassword the new password, {@code null} to remove the password.
* @return the new password, {@code null} if the password was removed.
*/
String setHttpPassword(String httpPassword) throws RestApiException;
/**
* A default implementation which allows source compatibility when adding new methods to the
* interface.
@@ -317,5 +334,15 @@ public interface AccountApi {
public void setName(String name) throws RestApiException {
throw new NotImplementedException();
}
@Override
public String generateHttpPassword() throws RestApiException {
throw new NotImplementedException();
}
@Override
public String setHttpPassword(String httpPassword) throws RestApiException {
throw new NotImplementedException();
}
}
}

32
java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
View File

@@ -40,6 +40,7 @@ import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.EmailInfo;
import com.google.gerrit.extensions.common.GpgKeyInfo;
import com.google.gerrit.extensions.common.GroupInfo;
import com.google.gerrit.extensions.common.HttpPasswordInput;
import com.google.gerrit.extensions.common.Input;
import com.google.gerrit.extensions.common.NameInput;
import com.google.gerrit.extensions.common.SshKeyInfo;
@@ -75,6 +76,7 @@ import com.google.gerrit.server.restapi.account.Index;
import com.google.gerrit.server.restapi.account.PostWatchedProjects;
import com.google.gerrit.server.restapi.account.PutActive;
import com.google.gerrit.server.restapi.account.PutAgreement;
import com.google.gerrit.server.restapi.account.PutHttpPassword;
import com.google.gerrit.server.restapi.account.PutName;
import com.google.gerrit.server.restapi.account.PutStatus;
import com.google.gerrit.server.restapi.account.SetDiffPreferences;
@@ -135,6 +137,7 @@ public class AccountApiImpl implements AccountApi {
private final GetGroups getGroups;
private final EmailApiImpl.Factory emailApi;
private final PutName putName;
private final PutHttpPassword putHttpPassword;
@Inject
AccountApiImpl(
@@ -177,6 +180,7 @@ public class AccountApiImpl implements AccountApi {
GetGroups getGroups,
EmailApiImpl.Factory emailApi,
PutName putName,
PutHttpPassword putPassword,
@Assisted AccountResource account) {
this.account = account;
this.accountLoaderFactory = ailf;
@@ -218,6 +222,7 @@ public class AccountApiImpl implements AccountApi {
this.getGroups = getGroups;
this.emailApi = emailApi;
this.putName = putName;
this.putHttpPassword = putPassword;
}
@Override
@@ -593,4 +598,31 @@ public class AccountApiImpl implements AccountApi {
throw asRestApiException("Cannot set account name", e);
}
}
@Override
public String generateHttpPassword() throws RestApiException {
HttpPasswordInput input = new HttpPasswordInput();
input.generate = true;
try {
// Response should never be 'none' for a generated password, but
// let's make sure.
Response<String> result = putHttpPassword.apply(account, input);
return result.isNone() ? null : result.value();
} catch (Exception e) {
throw asRestApiException("Cannot generate HTTP password", e);
}
}
@Override
public String setHttpPassword(String password) throws RestApiException {
HttpPasswordInput input = new HttpPasswordInput();
input.generate = false;
input.httpPassword = password;
try {
Response<String> result = putHttpPassword.apply(account, input);
return result.isNone() ? null : result.value();
} catch (Exception e) {
throw asRestApiException("Cannot generate HTTP password", e);
}
}
}

87
javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
View File

@@ -1891,8 +1891,11 @@ public class AccountIT extends AbstractDaemonTest {
String id = key.getKeyIdString();
addExternalIdEmail(admin, "test1@example.com");
sender.clear();
assertKeyMapContains(key, addGpgKey(key.getPublicKeyArmored()));
assertKeys(key);
assertThat(sender.getMessages()).hasSize(1);
assertThat(sender.getMessages().get(0).body()).contains("new GPG keys have been added");
requestScopeOperations.setApiUser(user.id());
exception.expect(ResourceNotFoundException.class);
@@ -1907,14 +1910,21 @@ public class AccountIT extends AbstractDaemonTest {
String id = key.getKeyIdString();
PGPPublicKey pk = key.getPublicKey();
sender.clear();
GpgKeyInfo info = addGpgKey(armor(pk)).get(id);
assertThat(info.userIds).hasSize(2);
assertIteratorSize(2, getOnlyKeyFromStore(key).getUserIDs());
assertThat(sender.getMessages()).hasSize(1);
assertThat(sender.getMessages().get(0).body()).contains("new GPG keys have been added");
pk = PGPPublicKey.removeCertification(pk, "foo:myId");
sender.clear();
info = addGpgKeyNoReindex(armor(pk)).get(id);
assertThat(info.userIds).hasSize(1);
assertIteratorSize(1, getOnlyKeyFromStore(key).getUserIDs());
// TODO: Issue 10769: Adding an already existing key should not result in a notification email
assertThat(sender.getMessages()).hasSize(1);
assertThat(sender.getMessages().get(0).body()).contains("new GPG keys have been added");
}
@Test
@@ -2024,32 +2034,42 @@ public class AccountIT extends AbstractDaemonTest {
assertSequenceNumbers(info);
SshKeyInfo key = info.get(0);
KeyPair keyPair = sshKeys.getKeyPair(admin);
String inital = TestSshKeys.publicKey(keyPair, admin.email());
assertThat(key.sshPublicKey).isEqualTo(inital);
String initial = TestSshKeys.publicKey(keyPair, admin.email());
assertThat(key.sshPublicKey).isEqualTo(initial);
accountIndexedCounter.assertNoReindex();
// Add a new key
sender.clear();
String newKey = TestSshKeys.publicKey(TestSshKeys.genSshKey(), admin.email());
gApi.accounts().self().addSshKey(newKey);
info = gApi.accounts().self().listSshKeys();
assertThat(info).hasSize(2);
assertSequenceNumbers(info);
accountIndexedCounter.assertReindexOf(admin);
assertThat(sender.getMessages()).hasSize(1);
assertThat(sender.getMessages().get(0).body()).contains("new SSH keys have been added");
// Add an existing key (the request succeeds, but the key isn't added again)
gApi.accounts().self().addSshKey(inital);
sender.clear();
gApi.accounts().self().addSshKey(initial);
info = gApi.accounts().self().listSshKeys();
assertThat(info).hasSize(2);
assertSequenceNumbers(info);
accountIndexedCounter.assertNoReindex();
// TODO: Issue 10769: Adding an already existing key should not result in a notification email
assertThat(sender.getMessages()).hasSize(1);
assertThat(sender.getMessages().get(0).body()).contains("new SSH keys have been added");
// Add another new key
sender.clear();
String newKey2 = TestSshKeys.publicKey(TestSshKeys.genSshKey(), admin.email());
gApi.accounts().self().addSshKey(newKey2);
info = gApi.accounts().self().listSshKeys();
assertThat(info).hasSize(3);
assertSequenceNumbers(info);
accountIndexedCounter.assertReindexOf(admin);
assertThat(sender.getMessages()).hasSize(1);
assertThat(sender.getMessages().get(0).body()).contains("new SSH keys have been added");
// Delete second key
gApi.accounts().self().deleteSshKey(2);
@@ -2719,6 +2739,67 @@ public class AccountIT extends AbstractDaemonTest {
}
}
@Test
public void userCanGenerateNewHttpPassword() throws Exception {
String newPassword = gApi.accounts().self().generateHttpPassword();
assertThat(newPassword).isNotNull();
}
@Test
public void adminCanGenerateNewHttpPasswordForUser() throws Exception {
requestScopeOperations.setApiUser(admin.id());
String newPassword = gApi.accounts().id(user.username()).generateHttpPassword();
assertThat(newPassword).isNotNull();
}
@Test
public void userCannotGenerateNewHttpPasswordForOtherUser() throws Exception {
requestScopeOperations.setApiUser(user.id());
exception.expect(AuthException.class);
gApi.accounts().id(admin.username()).generateHttpPassword();
}
@Test
public void userCannotExplicitlySetHttpPassword() throws Exception {
requestScopeOperations.setApiUser(user.id());
exception.expect(AuthException.class);
gApi.accounts().self().setHttpPassword("my-new-password");
}
@Test
public void userCannotExplicitlySetHttpPasswordForOtherUser() throws Exception {
requestScopeOperations.setApiUser(user.id());
exception.expect(AuthException.class);
gApi.accounts().id(admin.username()).setHttpPassword("my-new-password");
}
@Test
public void userCanRemoveHttpPassword() throws Exception {
requestScopeOperations.setApiUser(user.id());
assertThat(gApi.accounts().self().setHttpPassword(null)).isNull();
}
@Test
public void userCannotRemoveHttpPasswordForOtherUser() throws Exception {
requestScopeOperations.setApiUser(user.id());
exception.expect(AuthException.class);
gApi.accounts().id(admin.username()).setHttpPassword(null);
}
@Test
public void adminCanExplicitlySetHttpPasswordForUser() throws Exception {
requestScopeOperations.setApiUser(admin.id());
String httpPassword = "new-password-for-user";
assertThat(gApi.accounts().id(user.username()).setHttpPassword(httpPassword))
.isEqualTo(httpPassword);
}
@Test
public void adminCanRemoveHttpPasswordForUser() throws Exception {
requestScopeOperations.setApiUser(admin.id());
assertThat(gApi.accounts().id(user.username()).setHttpPassword(null)).isNull();
}
private void createDraft(PushOneCommit.Result r, String path, String message) throws Exception {
DraftInput in = new DraftInput();
in.path = path;

2
polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.html
View File

@@ -85,7 +85,7 @@ limitations under the License.
<span class="title">Create initial empty commit</span>
<span class="value">
<gr-select
id="initalCommit"
id="initialCommit"
bind-value="{{_repoConfig.create_empty_commit}}">
<select>
<option value="false">False</option>

4
polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_test.html
View File

@@ -50,7 +50,7 @@ limitations under the License.
});
test('default values are populated', () => {
assert.isTrue(element.$.initalCommit.bindValue);
assert.isTrue(element.$.initialCommit.bindValue);
assert.isFalse(element.$.parentRepo.bindValue);
});
@@ -83,7 +83,7 @@ limitations under the License.
element.$.repoNameInput.bindValue = configInputObj.name;
element.$.rightsInheritFromInput.bindValue = configInputObj.parent;
element.$.ownerInput.text = configInputObj.owners[0];
element.$.initalCommit.bindValue =
element.$.initialCommit.bindValue =
configInputObj.create_empty_commit;
element.$.parentRepo.bindValue =
configInputObj.permissions_only;

4
polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
View File

@@ -98,7 +98,7 @@ limitations under the License.
loadCommentSpy = sandbox.spy(commentApiWrapper.$.commentAPI, 'loadAll');
// Stub methods on the changeComments object after changeComments has
// been initalized.
// been initialized.
commentApiWrapper.loadComments().then(() => {
sandbox.stub(element.changeComments, 'getPaths').returns({});
sandbox.stub(element.changeComments, 'getCommentsBySideForPath')
@@ -1451,7 +1451,7 @@ limitations under the License.
sandbox.stub(element, '_reviewFile');
// Stub methods on the changeComments object after changeComments has
// been initalized.
// been initialized.
commentApiWrapper.loadComments().then(() => {
sandbox.stub(element.changeComments, 'getPaths').returns({});
sandbox.stub(element.changeComments, 'getCommentsBySideForPath')

4
polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
View File

@@ -149,7 +149,7 @@ limitations under the License.
element.messages = messages;
// Stub methods on the changeComments object after changeComments has
// been initalized.
// been initialized.
return commentApiWrapper.loadComments();
});
@@ -466,7 +466,7 @@ limitations under the License.
element.messages = messages;
// Stub methods on the changeComments object after changeComments has
// been initalized.
// been initialized.
return commentApiWrapper.loadComments();
});

2
polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.html
View File

@@ -76,7 +76,7 @@ limitations under the License.
element = commentApiWrapper.$.patchRange;
// Stub methods on the changeComments object after changeComments has
// been initalized.
// been initialized.
return commentApiWrapper.loadComments();
});