CurrentUser: Add isImpersonating method

It's possible for an IdentifiedUser to be impersonating itself by
constructing it with another copy of the same IdentifiedUser, which
fails the equality check in DefaultPermissionBackend. One likely culprit
for a codepath that causes this is in CacheBasedWebSession:

  @Override
  public void setUserAccountId(Account.Id id) {
    key = new Key("id:" + id);
    val = new Val(id, 0, false, null, 0, null, null);
    user = identified.runAs(id, user);
  }

Nothing stops a downstream caller from passing an id equal to
user.getAccountId(), which would then cause the admin check in
DefaultPermissionBackend to fail. This appears to be what was happening
in issue 7385, although I wasn't able to reproduce in my own test
server.

Bug: Issue 7385
Change-Id: If7603c32ceaa4745338cca2830397e19eda8b2fb
This commit is contained in:
Dave Borowitz 2017-10-27 16:41:07 -04:00
parent 8e06a77ef3
commit ef7267a966
3 changed files with 19 additions and 1 deletions

View File

@ -63,6 +63,10 @@ public abstract class CurrentUser {
return this;
}
public boolean isImpersonating() {
return false;
}
/**
* If the {@link #getRealUser()} has an account ID associated with it, call the given setter with
* that ID.

View File

@ -260,6 +260,20 @@ public class IdentifiedUser extends CurrentUser {
return realUser;
}
@Override
public boolean isImpersonating() {
if (realUser == this) {
return false;
}
if (realUser.isIdentifiedUser()) {
if (realUser.getAccountId().equals(getAccountId())) {
// Impersonating another copy of this user is allowed.
return false;
}
}
return true;
}
public AccountState state() {
if (state == null) {
state = accountCache.get(getAccountId());

View File

@ -154,7 +154,7 @@ public class DefaultPermissionBackend extends PermissionBackend {
private Boolean computeAdmin() {
Boolean r = user.get(IS_ADMIN);
if (r == null) {
if (user.getRealUser() != user) {
if (user.isImpersonating()) {
r = false;
} else if (user instanceof PeerDaemonUser) {
r = true;