Handle users without username
Currently the plugin saves the username of the service user creator in
its database. Some users may not have a username and then the creator
is unknown and trying to lookup the service user creator during the
permission checks fails.
Store the account ID of the service user creator in addition to the
username and do all lookups of the service user creator by using the
account ID instead of the username.
Change ServiceUserInfo so that it now contains an AccountInfo entity
for the creator.
Change-Id: I2aef3bcfb51d2642af32410fe05fcaa62e88022f
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/CreateServiceUser.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/CreateServiceUser.java
index 0365fd4..585fbad 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/CreateServiceUser.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/CreateServiceUser.java
@@ -19,6 +19,7 @@
import com.google.common.collect.Lists;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.annotations.RequiresCapability;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
@@ -76,6 +77,7 @@
public static final String USER = "user";
public static final String KEY_CREATED_BY = "createdBy";
public static final String KEY_CREATED_AT = "createdAt";
+ public static final String KEY_CREATOR_ID = "creatorId";
public static final String KEY_OWNER = "owner";
static class Input {
@@ -102,6 +104,7 @@
private final ProjectLevelConfig storage;
private final DateFormat rfc2822DateFormatter;
private final Provider<GetConfig> getConfig;
+ private final AccountInfo.Loader.Factory accountLoader;
@Inject
CreateServiceUser(PluginConfigFactory cfgFactory,
@@ -116,7 +119,8 @@
MetaDataUpdate.User metaDataUpdateFactory,
ProjectCache projectCache,
@Assisted String username,
- Provider<GetConfig> getConfig) {
+ Provider<GetConfig> getConfig,
+ AccountInfo.Loader.Factory accountLoader) {
this.cfg = cfgFactory.getFromGerritConfig(pluginName);
this.createAccountFactory = createAccountFactory;
this.groupCache = groupCache;
@@ -141,12 +145,18 @@
this.rfc2822DateFormatter.setCalendar(Calendar.getInstance(
gerritIdent.getTimeZone(), Locale.US));
this.getConfig = getConfig;
+ this.accountLoader = accountLoader;
}
@Override
public Response<ServiceUserInfo> apply(ConfigResource resource, Input input)
- throws BadRequestException, ResourceConflictException,
+ throws AuthException, BadRequestException, ResourceConflictException,
UnprocessableEntityException, OrmException, IOException {
+ CurrentUser user = userProvider.get();
+ if (user == null || !user.isIdentifiedUser()) {
+ throw new AuthException("authentication required");
+ }
+
if (input == null) {
input = new Input();
}
@@ -176,11 +186,15 @@
createAccountFactory.create(username).apply(TopLevelResource.INSTANCE, in);
addToGroups(response.value()._id, cfg.getStringList("group"));
- String creator = userProvider.get().getUserName();
+ String creator = user.getUserName();
+ Account.Id creatorId = ((IdentifiedUser)user).getAccountId();
String creationDate = rfc2822DateFormatter.format(new Date());
Config db = storage.get();
- db.setString(USER, username, KEY_CREATED_BY, creator);
+ db.setInt(USER, username, KEY_CREATOR_ID, creatorId.get());
+ if (creator != null) {
+ db.setString(USER, username, KEY_CREATED_BY, creator);
+ }
db.setString(USER, username, KEY_CREATED_AT, creationDate);
MetaDataUpdate md = metaDataUpdateFactory.create(allProjects);
@@ -188,7 +202,9 @@
storage.commit(md);
ServiceUserInfo info = new ServiceUserInfo(response.value());
- info.createdBy = creator;
+ AccountInfo.Loader al = accountLoader.create(true);
+ info.createdBy = al.get(creatorId);
+ al.fill();
info.createdAt = creationDate;
return Response.created(info);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/CreateServiceUserNotes.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/CreateServiceUserNotes.java
index 14ca5b1..600a9b1 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/CreateServiceUserNotes.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/CreateServiceUserNotes.java
@@ -220,7 +220,7 @@
fmt.appendDate();
fmt.append("Project", project.get());
fmt.append("Branch", branch);
- fmt.append(KEY_CREATED_BY, serviceUser.createdBy);
+ fmt.appendUser(KEY_CREATED_BY, serviceUser.createdBy);
for (AccountInfo owner : listOwners(serviceUser)) {
fmt.appendUser(KEY_OWNER, owner);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/GetServiceUser.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/GetServiceUser.java
index c5979f9..da5d4cf 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/GetServiceUser.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/GetServiceUser.java
@@ -15,7 +15,7 @@
package com.googlesource.gerrit.plugins.serviceuser;
import static com.googlesource.gerrit.plugins.serviceuser.CreateServiceUser.KEY_CREATED_AT;
-import static com.googlesource.gerrit.plugins.serviceuser.CreateServiceUser.KEY_CREATED_BY;
+import static com.googlesource.gerrit.plugins.serviceuser.CreateServiceUser.KEY_CREATOR_ID;
import static com.googlesource.gerrit.plugins.serviceuser.CreateServiceUser.USER;
import static javax.servlet.http.HttpServletResponse.SC_OK;
@@ -23,6 +23,7 @@
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.account.AccountInfo;
import com.google.gerrit.server.account.GetAccount;
import com.google.gerrit.server.git.ProjectLevelConfig;
@@ -38,14 +39,16 @@
private final Provider<GetAccount> getAccount;
private final ProjectLevelConfig storage;
private final GetOwner getOwner;
+ private final AccountInfo.Loader.Factory accountLoader;
@Inject
GetServiceUser(Provider<GetAccount> getAccount,
@PluginName String pluginName, ProjectCache projectCache,
- GetOwner getOwner) {
+ GetOwner getOwner, AccountInfo.Loader.Factory accountLoader) {
this.getAccount = getAccount;
this.storage = projectCache.getAllProjects().getConfig(pluginName + ".db");
this.getOwner = getOwner;
+ this.accountLoader = accountLoader;
}
@Override
@@ -58,7 +61,10 @@
}
ServiceUserInfo info = new ServiceUserInfo(getAccount.get().apply(rsrc));
- info.createdBy = db.getString(USER, username, KEY_CREATED_BY);
+ AccountInfo.Loader al = accountLoader.create(true);
+ info.createdBy =
+ al.get(new Account.Id(db.getInt(USER, username, KEY_CREATOR_ID, -1)));
+ al.fill();
info.createdAt = db.getString(USER, username, KEY_CREATED_AT);
info.inactive = !rsrc.getUser().getAccount().isActive() ? true : null;
@@ -71,7 +77,7 @@
}
public static class ServiceUserInfo extends AccountInfo {
- public String createdBy;
+ public AccountInfo createdBy;
public String createdAt;
public Boolean inactive;
public GroupInfo owner;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ListServiceUsers.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ListServiceUsers.java
index 6339dfa..0980804 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ListServiceUsers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ListServiceUsers.java
@@ -14,7 +14,7 @@
package com.googlesource.gerrit.plugins.serviceuser;
-import static com.googlesource.gerrit.plugins.serviceuser.CreateServiceUser.KEY_CREATED_BY;
+import static com.googlesource.gerrit.plugins.serviceuser.CreateServiceUser.KEY_CREATOR_ID;
import static com.googlesource.gerrit.plugins.serviceuser.CreateServiceUser.USER;
import com.google.common.collect.Maps;
@@ -22,7 +22,7 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.server.AnonymousUser;
+import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountCache;
@@ -63,17 +63,17 @@
public Map<String, ServiceUserInfo> apply(ConfigResource rscr)
throws OrmException, AuthException {
CurrentUser user = userProvider.get();
- if (user instanceof AnonymousUser) {
+ if (user == null || !user.isIdentifiedUser()) {
throw new AuthException("Authentication required");
}
Map<String, ServiceUserInfo> accounts = Maps.newTreeMap();
Config db = storage.get();
boolean isAdmin = user.getCapabilities().canAdministrateServer();
- String currentUser = user.getUserName();
for (String username : db.getSubsections(USER)) {
- String createdBy = db.getString(USER, username, KEY_CREATED_BY);
- if (isAdmin || currentUser.equals(createdBy)) {
+ Account.Id createdBy =
+ new Account.Id(db.getInt(USER, username, KEY_CREATOR_ID, -1));
+ if (isAdmin || ((IdentifiedUser)user).getAccountId().equals(createdBy)) {
AccountState account = accountCache.getByUsername(username);
if (account != null) {
ServiceUserInfo info;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ServiceUserCollection.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ServiceUserCollection.java
index ce24645..e8546cd 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ServiceUserCollection.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ServiceUserCollection.java
@@ -14,7 +14,7 @@
package com.googlesource.gerrit.plugins.serviceuser;
-import static com.googlesource.gerrit.plugins.serviceuser.CreateServiceUser.KEY_CREATED_BY;
+import static com.googlesource.gerrit.plugins.serviceuser.CreateServiceUser.KEY_CREATOR_ID;
import static com.googlesource.gerrit.plugins.serviceuser.CreateServiceUser.KEY_OWNER;
import static com.googlesource.gerrit.plugins.serviceuser.CreateServiceUser.USER;
@@ -29,8 +29,9 @@
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
-import com.google.gerrit.server.AnonymousUser;
+import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountsCollection;
import com.google.gerrit.server.config.ConfigResource;
import com.google.gerrit.server.git.ProjectLevelConfig;
@@ -74,7 +75,7 @@
throw new ResourceNotFoundException(id);
}
CurrentUser user = userProvider.get();
- if (user instanceof AnonymousUser) {
+ if (user == null || !user.isIdentifiedUser()) {
throw new AuthException("Authentication required");
}
if (!user.getCapabilities().canAdministrateServer()) {
@@ -88,8 +89,8 @@
} catch (UnprocessableEntityException e) {
throw new ResourceNotFoundException(id);
}
- } else if (!user.getUserName().equals(
- storage.get().getString(USER, id.get(), KEY_CREATED_BY))) {
+ } else if (!((IdentifiedUser)user).getAccountId().equals(
+ new Account.Id(storage.get().getInt(USER, id.get(), KEY_CREATOR_ID, -1)))) {
throw new ResourceNotFoundException(id);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/client/AccountInfo.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/client/AccountInfo.java
new file mode 100644
index 0000000..962b2e9
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/client/AccountInfo.java
@@ -0,0 +1,27 @@
+// Copyright (C) 2014 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.serviceuser.client;
+
+import com.google.gwt.core.client.JavaScriptObject;
+
+public class AccountInfo extends JavaScriptObject {
+ public final native int _account_id() /*-{ return this._account_id || 0; }-*/;
+ public final native String name() /*-{ return this.name; }-*/;
+ public final native String username() /*-{ return this.username; }-*/;
+ public final native String email() /*-{ return this.email; }-*/;
+
+ protected AccountInfo() {
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/client/ServiceUserInfo.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/client/ServiceUserInfo.java
index abad7c9..b412e95 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/client/ServiceUserInfo.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/client/ServiceUserInfo.java
@@ -17,11 +17,23 @@
import com.google.gwt.core.client.JavaScriptObject;
public class ServiceUserInfo extends JavaScriptObject {
+ public final String getDisplayName() {
+ if (created_by().username() != null) {
+ return created_by().username();
+ } else {
+ if (created_by()._account_id() != -1) {
+ return Integer.toString(created_by()._account_id());
+ } else {
+ return "N/A";
+ }
+ }
+ }
+
public final native int _account_id() /*-{ return this._account_id || 0; }-*/;
public final native String name() /*-{ return this.name; }-*/;
public final native String username() /*-{ return this.username; }-*/;
public final native String email() /*-{ return this.email; }-*/;
- public final native String created_by() /*-{ return this.created_by; }-*/;
+ public final native AccountInfo created_by() /*-{ return this.created_by; }-*/;
public final native String created_at() /*-{ return this.created_at; }-*/;
public final native boolean active() /*-{ return this.inactive ? false : true; }-*/;
public final native GroupInfo owner() /*-{ return this.owner; }-*/;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/client/ServiceUserListScreen.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/client/ServiceUserListScreen.java
index 95d3af0..0f767cc 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/client/ServiceUserListScreen.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/client/ServiceUserListScreen.java
@@ -93,7 +93,7 @@
}
}
- t.setText(row, 4, a.created_by());
+ t.setText(row, 4, a.getDisplayName());
t.setText(row, 5, a.created_at());
t.setText(row, 6, !a.active() ? "Inactive" : "");
row++;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/client/ServiceUserScreen.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/client/ServiceUserScreen.java
index 39a1367..cbc77b3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/client/ServiceUserScreen.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/client/ServiceUserScreen.java
@@ -112,7 +112,7 @@
t.addRow("Email Address", info.email());
}
t.addRow("Owner Group", createOwnerWidget(info));
- t.addRow("Created By", info.created_by());
+ t.addRow("Created By", info.getDisplayName());
t.addRow("Created At", info.created_at());
add(t);
diff --git a/src/main/resources/Documentation/rest-api-config.md b/src/main/resources/Documentation/rest-api-config.md
index 3b99618..b9a4d34 100644
--- a/src/main/resources/Documentation/rest-api-config.md
+++ b/src/main/resources/Documentation/rest-api-config.md
@@ -672,8 +672,9 @@
[AccountInfo](../../../Documentation/rest-api-accounts.html#account-info)
and in addition the following fields:
-* _created\_by_: The username of the user that created this service
- user.
+* _created\_by_: The user that created this service user as a detailed
+ [AccountInfo](../../../Documentation/rest-api-accounts.html#account-info)
+ entity.
* _created\_at_: The date when the service user was created in the
format 'EEE, dd MMM yyyy HH:mm:ss Z'.
* _inactive_: Whether the account state of the service user is
