opendev
/
gerrit
Code Issues Proposed changes

Fix permissions checks on Gerrit API on current user 

When the current user is using GerritApi, a new user object
with the same user-id of CurrentUser gets created on-the-fly.

Even though the user accountId is the same, the user instances
are different and will fail all the REST API permissions checks.

Turn instance checks with accountId checks to allow user to
execute GerritApi on themselves without the requirement of being
Gerrit administrators.

NOTE: GerritApi are mostly used in plugins, so this change allows
other plugins to function properly.

Change-Id: Iaeb204dda3791eb2757d89fe6bce6994c6305e04
This commit is contained in:
Luca Milanesio 2018-05-10 17:17:12 +01:00
parent 8f708b8cc0
commit 5872780a5a
32 changed files with 77 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/GpgKeys.java
View File

@ -219,7 +219,7 @@ public class GpgKeys implements ChildCollection<AccountResource, GpgKey> {
if (!BouncyCastleUtil.havePGP()) {
throw new ResourceNotFoundException("GPG not enabled");
}
if (self.get() != rsrc.getUser()) {
if (!self.get().hasSameAccountId(rsrc.getUser())) {
throw new ResourceNotFoundException();
}
}

13
gerrit-server/src/main/java/com/google/gerrit/server/CurrentUser.java
View File

@ -158,4 +158,17 @@ public abstract class CurrentUser {
public ExternalId.Key getLastLoginExternalIdKey() {
return get(lastLoginExternalIdPropertyKey);
}
/**
* Checks if the current user has the same account id of another.
*
* <p>Provide a generic interface for allowing subclasses to define whether two accounts represent
* the same account id.
*
* @param other user to compare
* @return true if the two users have the same account id
*/
public boolean hasSameAccountId(CurrentUser other) {
return false;
}
}

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

@ -498,6 +498,11 @@ public class IdentifiedUser extends CurrentUser {
realUser);
}
@Override
public boolean hasSameAccountId(CurrentUser other) {
return getAccountId().get() == other.getAccountId().get();
}
private String guessHost() {
String host = null;
SocketAddress remotePeer = null;

3
gerrit-server/src/main/java/com/google/gerrit/server/account/AddSshKey.java
View File

@ -69,7 +69,8 @@ public class AddSshKey implements RestModifyView<AccountResource, Input> {
@Override
public Response<SshKeyInfo> apply(AccountResource rsrc, Input input)
throws AuthException, BadRequestException, OrmException, IOException, ConfigInvalidException {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canAdministrateServer()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("not allowed to add SSH keys");
}
return apply(rsrc.getUser(), input);

3
gerrit-server/src/main/java/com/google/gerrit/server/account/Capabilities.java
View File

@ -51,7 +51,8 @@ class Capabilities implements ChildCollection<AccountResource, AccountResource.C
@Override
public Capability parse(AccountResource parent, IdString id)
throws ResourceNotFoundException, AuthException {
if (self.get() != parent.getUser() && !self.get().getCapabilities().canAdministrateServer()) {
if (!self.get().hasSameAccountId(parent.getUser())
&& !self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("restricted to administrator");
}

3
gerrit-server/src/main/java/com/google/gerrit/server/account/CreateEmail.java
View File

@ -79,7 +79,8 @@ public class CreateEmail implements RestModifyView<AccountResource, EmailInput>
throws AuthException, BadRequestException, ResourceConflictException,
ResourceNotFoundException, OrmException, EmailException, MethodNotAllowedException,
IOException, ConfigInvalidException {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canModifyAccount()) {
throw new AuthException("not allowed to add email address");
}

3
gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteEmail.java
View File

@ -60,7 +60,8 @@ public class DeleteEmail implements RestModifyView<AccountResource.Email, Input>
public Response<?> apply(AccountResource.Email rsrc, Input input)
throws AuthException, ResourceNotFoundException, ResourceConflictException,
MethodNotAllowedException, OrmException, IOException, ConfigInvalidException {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canModifyAccount()) {
throw new AuthException("not allowed to delete email address");
}
return apply(rsrc.getUser(), rsrc.getEmail());

2
gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteExternalIds.java
View File

@ -60,7 +60,7 @@ public class DeleteExternalIds implements RestModifyView<AccountResource, List<S
@Override
public Response<?> apply(AccountResource resource, List<String> externalIds)
throws RestApiException, IOException, OrmException, ConfigInvalidException {
if (self.get() != resource.getUser()) {
if (!self.get().hasSameAccountId(resource.getUser())) {
throw new AuthException("not allowed to delete external IDs");
}

3
gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteSshKey.java
View File

@ -50,7 +50,8 @@ public class DeleteSshKey implements RestModifyView<AccountResource.SshKey, Inpu
public Response<?> apply(AccountResource.SshKey rsrc, Input input)
throws AuthException, OrmException, RepositoryNotFoundException, IOException,
ConfigInvalidException {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canAdministrateServer()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("not allowed to delete SSH keys");
}

3
gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteWatchedProjects.java
View File

@ -52,7 +52,8 @@ public class DeleteWatchedProjects
public Response<?> apply(AccountResource rsrc, List<ProjectWatchInfo> input)
throws AuthException, UnprocessableEntityException, OrmException, IOException,
ConfigInvalidException {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canAdministrateServer()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("It is not allowed to edit project watches of other users");
}
if (input == null) {

3
gerrit-server/src/main/java/com/google/gerrit/server/account/GetCapabilities.java
View File

@ -78,7 +78,8 @@ class GetCapabilities implements RestReadView<AccountResource> {
@Override
public Object apply(AccountResource resource) throws AuthException {
if (self.get() != resource.getUser() && !self.get().getCapabilities().canAdministrateServer()) {
if (!self.get().hasSameAccountId(resource.getUser())
&& !self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("restricted to administrator");
}

3
gerrit-server/src/main/java/com/google/gerrit/server/account/GetDiffPreferences.java
View File

@ -57,7 +57,8 @@ public class GetDiffPreferences implements RestReadView<AccountResource> {
@Override
public DiffPreferencesInfo apply(AccountResource rsrc)
throws AuthException, ConfigInvalidException, IOException {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canAdministrateServer()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("restricted to administrator");
}

3
gerrit-server/src/main/java/com/google/gerrit/server/account/GetEditPreferences.java
View File

@ -49,7 +49,8 @@ public class GetEditPreferences implements RestReadView<AccountResource> {
@Override
public EditPreferencesInfo apply(AccountResource rsrc)
throws AuthException, IOException, ConfigInvalidException {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canModifyAccount()) {
throw new AuthException("requires Modify Account capability");
}

2
gerrit-server/src/main/java/com/google/gerrit/server/account/GetExternalIds.java
View File

@ -49,7 +49,7 @@ public class GetExternalIds implements RestReadView<AccountResource> {
@Override
public List<AccountExternalIdInfo> apply(AccountResource resource)
throws RestApiException, OrmException {
if (self.get() != resource.getUser()) {
if (!self.get().hasSameAccountId(resource.getUser())) {
throw new AuthException("not allowed to get external IDs");
}

2
gerrit-server/src/main/java/com/google/gerrit/server/account/GetOAuthToken.java
View File

@ -50,7 +50,7 @@ class GetOAuthToken implements RestReadView<AccountResource> {
@Override
public OAuthTokenInfo apply(AccountResource rsrc)
throws AuthException, ResourceNotFoundException {
if (self.get() != rsrc.getUser()) {
if (!self.get().hasSameAccountId(rsrc.getUser())) {
throw new AuthException("not allowed to get access token");
}
Account a = rsrc.getUser().getAccount();

3
gerrit-server/src/main/java/com/google/gerrit/server/account/GetPreferences.java
View File

@ -36,7 +36,8 @@ public class GetPreferences implements RestReadView<AccountResource> {
@Override
public GeneralPreferencesInfo apply(AccountResource rsrc) throws AuthException {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canModifyAccount()) {
throw new AuthException("requires Modify Account capability");
}

3
gerrit-server/src/main/java/com/google/gerrit/server/account/GetSshKeys.java
View File

@ -47,7 +47,8 @@ public class GetSshKeys implements RestReadView<AccountResource> {
public List<SshKeyInfo> apply(AccountResource rsrc)
throws AuthException, OrmException, RepositoryNotFoundException, IOException,
ConfigInvalidException {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canModifyAccount()) {
throw new AuthException("not allowed to get SSH keys");
}
return apply(rsrc.getUser());

3
gerrit-server/src/main/java/com/google/gerrit/server/account/GetWatchedProjects.java
View File

@ -51,7 +51,8 @@ public class GetWatchedProjects implements RestReadView<AccountResource> {
@Override
public List<ProjectWatchInfo> apply(AccountResource rsrc)
throws OrmException, AuthException, IOException, ConfigInvalidException {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canAdministrateServer()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("It is not allowed to list project watches of other users");
}
Account.Id accountId = rsrc.getUser().getAccountId();

3
gerrit-server/src/main/java/com/google/gerrit/server/account/Index.java
View File

@ -39,7 +39,8 @@ public class Index implements RestModifyView<AccountResource, Input> {
@Override
public Response<?> apply(AccountResource rsrc, Input input) throws IOException, AuthException {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canModifyAccount()) {
throw new AuthException("not allowed to index account");
}

3
gerrit-server/src/main/java/com/google/gerrit/server/account/PostWatchedProjects.java
View File

@ -63,7 +63,8 @@ public class PostWatchedProjects
@Override
public List<ProjectWatchInfo> apply(AccountResource rsrc, List<ProjectWatchInfo> input)
throws OrmException, RestApiException, IOException, ConfigInvalidException {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canAdministrateServer()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("not allowed to edit project watches");
}
Account.Id accountId = rsrc.getUser().getAccountId();

2
gerrit-server/src/main/java/com/google/gerrit/server/account/PutAgreement.java
View File

@ -72,7 +72,7 @@ public class PutAgreement implements RestModifyView<AccountResource, AgreementIn
throw new MethodNotAllowedException("contributor agreements disabled");
}
if (self.get() != resource.getUser()) {
if (!self.get().hasSameAccountId(resource.getUser())) {
throw new AuthException("not allowed to enter contributor agreement");
}

6
gerrit-server/src/main/java/com/google/gerrit/server/account/PutHttpPassword.java
View File

@ -77,13 +77,15 @@ public class PutHttpPassword implements RestModifyView<AccountResource, Input> {
String newPassword;
if (input.generate) {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canAdministrateServer()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("not allowed to generate HTTP password");
}
newPassword = generate();
} else if (input.httpPassword == null) {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canAdministrateServer()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("not allowed to clear HTTP password");
}
newPassword = null;

3
gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java
View File

@ -61,7 +61,8 @@ public class PutName implements RestModifyView<AccountResource, Input> {
public Response<String> apply(AccountResource rsrc, Input input)
throws AuthException, MethodNotAllowedException, ResourceNotFoundException, OrmException,
IOException {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canModifyAccount()) {
throw new AuthException("not allowed to change name");
}
return apply(rsrc.getUser(), input);

3
gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java
View File

@ -50,7 +50,8 @@ public class PutPreferred implements RestModifyView<AccountResource.Email, Input
@Override
public Response<String> apply(AccountResource.Email rsrc, Input input)
throws AuthException, ResourceNotFoundException, OrmException, IOException {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canModifyAccount()) {
throw new AuthException("not allowed to set preferred email address");
}
return apply(rsrc.getUser(), rsrc.getEmail());

3
gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java
View File

@ -58,7 +58,8 @@ public class PutStatus implements RestModifyView<AccountResource, Input> {
@Override
public Response<String> apply(AccountResource rsrc, Input input)
throws AuthException, ResourceNotFoundException, OrmException, IOException {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canModifyAccount()) {
throw new AuthException("not allowed to set status");
}
return apply(rsrc.getUser(), input);

3
gerrit-server/src/main/java/com/google/gerrit/server/account/PutUsername.java
View File

@ -59,7 +59,8 @@ public class PutUsername implements RestModifyView<AccountResource, Input> {
public String apply(AccountResource rsrc, Input input)
throws AuthException, MethodNotAllowedException, UnprocessableEntityException,
ResourceConflictException, OrmException, IOException, ConfigInvalidException {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canAdministrateServer()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("not allowed to set username");
}

3
gerrit-server/src/main/java/com/google/gerrit/server/account/SetDiffPreferences.java
View File

@ -59,7 +59,8 @@ public class SetDiffPreferences implements RestModifyView<AccountResource, DiffP
public DiffPreferencesInfo apply(AccountResource rsrc, DiffPreferencesInfo in)
throws AuthException, BadRequestException, ConfigInvalidException,
RepositoryNotFoundException, IOException {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canModifyAccount()) {
throw new AuthException("requires Modify Account capability");
}

3
gerrit-server/src/main/java/com/google/gerrit/server/account/SetEditPreferences.java
View File

@ -59,7 +59,8 @@ public class SetEditPreferences implements RestModifyView<AccountResource, EditP
public EditPreferencesInfo apply(AccountResource rsrc, EditPreferencesInfo in)
throws AuthException, BadRequestException, RepositoryNotFoundException, IOException,
ConfigInvalidException {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canModifyAccount()) {
throw new AuthException("requires Modify Account capability");
}

3
gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java
View File

@ -75,7 +75,8 @@ public class SetPreferences implements RestModifyView<AccountResource, GeneralPr
@Override
public GeneralPreferencesInfo apply(AccountResource rsrc, GeneralPreferencesInfo i)
throws AuthException, BadRequestException, IOException, ConfigInvalidException {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canModifyAccount()) {
throw new AuthException("requires Modify Account capability");
}

3
gerrit-server/src/main/java/com/google/gerrit/server/account/SshKeys.java
View File

@ -56,7 +56,8 @@ public class SshKeys implements ChildCollection<AccountResource, AccountResource
@Override
public AccountResource.SshKey parse(AccountResource rsrc, IdString id)
throws ResourceNotFoundException, OrmException, IOException, ConfigInvalidException {
if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canModifyAccount()) {
if (!self.get().hasSameAccountId(rsrc.getUser())
&& !self.get().getCapabilities().canModifyAccount()) {
throw new ResourceNotFoundException();
}
return parse(rsrc.getUser(), id);

6
gerrit-server/src/main/java/com/google/gerrit/server/account/StarredChanges.java
View File

@ -130,7 +130,7 @@ public class StarredChanges
@Override
public Response<?> apply(AccountResource rsrc, EmptyInput in)
throws AuthException, OrmException, IOException {
if (self.get() != rsrc.getUser()) {
if (!self.get().hasSameAccountId(rsrc.getUser())) {
throw new AuthException("not allowed to add starred change");
}
try {
@ -159,7 +159,7 @@ public class StarredChanges
@Override
public Response<?> apply(AccountResource.StarredChange rsrc, EmptyInput in)
throws AuthException {
if (self.get() != rsrc.getUser()) {
if (!self.get().hasSameAccountId(rsrc.getUser())) {
throw new AuthException("not allowed update starred changes");
}
return Response.none();
@ -180,7 +180,7 @@ public class StarredChanges
@Override
public Response<?> apply(AccountResource.StarredChange rsrc, EmptyInput in)
throws AuthException, OrmException, IOException {
if (self.get() != rsrc.getUser()) {
if (!self.get().hasSameAccountId(rsrc.getUser())) {
throw new AuthException("not allowed remove starred change");
}
starredChangesUtil.star(

6
gerrit-server/src/main/java/com/google/gerrit/server/account/Stars.java
View File

@ -97,7 +97,7 @@ public class Stars implements ChildCollection<AccountResource, AccountResource.S
@SuppressWarnings("unchecked")
public List<ChangeInfo> apply(AccountResource rsrc)
throws BadRequestException, AuthException, OrmException {
if (self.get() != rsrc.getUser()) {
if (!self.get().hasSameAccountId(rsrc.getUser())) {
throw new AuthException("not allowed to list stars of another account");
}
QueryChanges query = changes.list();
@ -119,7 +119,7 @@ public class Stars implements ChildCollection<AccountResource, AccountResource.S
@Override
public SortedSet<String> apply(AccountResource.Star rsrc) throws AuthException, OrmException {
if (self.get() != rsrc.getUser()) {
if (!self.get().hasSameAccountId(rsrc.getUser())) {
throw new AuthException("not allowed to get stars of another account");
}
return starredChangesUtil.getLabels(self.get().getAccountId(), rsrc.getChange().getId());
@ -140,7 +140,7 @@ public class Stars implements ChildCollection<AccountResource, AccountResource.S
@Override
public Collection<String> apply(AccountResource.Star rsrc, StarsInput in)
throws AuthException, BadRequestException, OrmException {
if (self.get() != rsrc.getUser()) {
if (!self.get().hasSameAccountId(rsrc.getUser())) {
throw new AuthException("not allowed to update stars of another account");
}
try {