Use MODIFY_ACCOUNT instead of ACCESS_DATABASE for external id endpoints
With this change, we use MODIFY_ACCOUNT instead of ACCESS_DATABASE to
allow users to get/delete someone else's external IDs. The rational is
that this is the more narrow, better fitting permission.
We also document this possibility.
Change-Id: I90b5547326590ea9d3e5bcf5afcc9c803066598d
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt
index 28ecc97..10a2ff3 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -1728,6 +1728,10 @@
Retrieves the external ids of a user account.
+Only external ids belonging to the caller may be requested. Users that have
+link:access-control.html#capability_modifyAccount[Modify Account] can request
+external ids that belong to other accounts.
+
.Request
----
GET /a/accounts/self/external.ids HTTP/1.0
@@ -1761,7 +1765,9 @@
Delete a list of external ids for a user account. The target external ids must
be provided as a list in the request body.
-Only external ids belonging to the caller may be deleted.
+Only external ids belonging to the caller may be deleted. Users that have
+link:access-control.html#capability_modifyAccount[Modify Account] can delete
+external ids that belong to other accounts.
.Request
----
diff --git a/java/com/google/gerrit/server/restapi/account/DeleteExternalIds.java b/java/com/google/gerrit/server/restapi/account/DeleteExternalIds.java
index 80f0bb1..445a5d6 100644
--- a/java/com/google/gerrit/server/restapi/account/DeleteExternalIds.java
+++ b/java/com/google/gerrit/server/restapi/account/DeleteExternalIds.java
@@ -73,7 +73,7 @@
public Response<?> apply(AccountResource resource, List<String> extIds)
throws RestApiException, IOException, ConfigInvalidException, PermissionBackendException {
if (!self.get().hasSameAccountId(resource.getUser())) {
- permissionBackend.currentUser().check(GlobalPermission.ACCESS_DATABASE);
+ permissionBackend.currentUser().check(GlobalPermission.MODIFY_ACCOUNT);
}
if (extIds == null || extIds.isEmpty()) {
diff --git a/java/com/google/gerrit/server/restapi/account/GetExternalIds.java b/java/com/google/gerrit/server/restapi/account/GetExternalIds.java
index c5b4454..a3c48b9 100644
--- a/java/com/google/gerrit/server/restapi/account/GetExternalIds.java
+++ b/java/com/google/gerrit/server/restapi/account/GetExternalIds.java
@@ -67,7 +67,7 @@
public Response<List<AccountExternalIdInfo>> apply(AccountResource resource)
throws RestApiException, IOException, PermissionBackendException {
if (!self.get().hasSameAccountId(resource.getUser())) {
- permissionBackend.currentUser().check(GlobalPermission.ACCESS_DATABASE);
+ permissionBackend.currentUser().check(GlobalPermission.MODIFY_ACCOUNT);
}
Collection<ExternalId> ids = externalIds.byAccount(resource.getUser().getAccountId());
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java b/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
index 6e16435..53e871f 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
@@ -132,14 +132,14 @@
AuthException thrown =
assertThrows(
AuthException.class, () -> gApi.accounts().id(admin.id().get()).getExternalIds());
- assertThat(thrown).hasMessageThat().contains("access database not permitted");
+ assertThat(thrown).hasMessageThat().contains("modify account not permitted");
}
@Test
- public void getExternalIdsOfOtherUserWithAccessDatabase() throws Exception {
+ public void getExternalIdsOfOtherUserWithModifyAccount() throws Exception {
projectOperations
.allProjectsForUpdate()
- .add(allowCapability(GlobalCapability.ACCESS_DATABASE).group(REGISTERED_USERS))
+ .add(allowCapability(GlobalCapability.MODIFY_ACCOUNT).group(REGISTERED_USERS))
.update();
Collection<ExternalId> expectedIds = getAccountState(admin.id()).externalIds();
@@ -193,7 +193,7 @@
gApi.accounts()
.id(admin.id().get())
.deleteExternalIds(extIds.stream().map(e -> e.identity).collect(toList())));
- assertThat(thrown).hasMessageThat().contains("access database not permitted");
+ assertThat(thrown).hasMessageThat().contains("modify account not permitted");
}
@Test
@@ -213,10 +213,10 @@
}
@Test
- public void deleteExternalIdsOfOtherUserWithAccessDatabase() throws Exception {
+ public void deleteExternalIdsOfOtherUserWithModifyAccount() throws Exception {
projectOperations
.allProjectsForUpdate()
- .add(allowCapability(GlobalCapability.ACCESS_DATABASE).group(REGISTERED_USERS))
+ .add(allowCapability(GlobalCapability.MODIFY_ACCOUNT).group(REGISTERED_USERS))
.update();
List<AccountExternalIdInfo> externalIds = gApi.accounts().self().getExternalIds();
@@ -464,7 +464,7 @@
public void readExternalIdsWhenInvalidExternalIdsExist() throws Exception {
projectOperations
.allProjectsForUpdate()
- .add(allowCapability(GlobalCapability.ACCESS_DATABASE).group(REGISTERED_USERS))
+ .add(allowCapability(GlobalCapability.MODIFY_ACCOUNT).group(REGISTERED_USERS))
.update();
requestScopeOperations.resetCurrentApiUser();
