Support adding configured external Ids during remote sync
This change allows admins to configure a list of external Ids that will
always be added during a remote sync. This helps prevent login failures
caused by the deletion of external Ids from the remote.
Change-Id: I3a047b19e79d181c66511286b604d7b152e49b3a
diff --git a/README.md b/README.md
index f61d6c1..99158b2 100644
--- a/README.md
+++ b/README.md
@@ -178,3 +178,14 @@
[cache "accounts"]
refreshAfterWrite = 23h
```
+
+# How to set up external IDs that should be persistently saved
+
+External Ids that should always be retained after a remote sync can be
+configured in the `${GERRIT_SITE}/etc/remote-gerrit-account-cache_externalIds`
+file. This file expects the account id, external id and email with a tab delimiter.
+
+Example:
+```
+1000001 externalId=mailto:user@example.com email=user@example.com
+```
\ No newline at end of file
diff --git a/src/main/java/com/googlesource/gerrit/plugins/remotegerritaccountcache/AccountCacheImpl.java b/src/main/java/com/googlesource/gerrit/plugins/remotegerritaccountcache/AccountCacheImpl.java
index 1d6589f..42de925 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/remotegerritaccountcache/AccountCacheImpl.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/remotegerritaccountcache/AccountCacheImpl.java
@@ -39,13 +39,15 @@
import com.google.gerrit.server.account.CachedAccountDetails;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.account.externalids.ExternalIdKeyFactory;
-import com.google.gerrit.server.account.externalids.ExternalIds;
+import com.google.gerrit.server.account.externalids.storage.notedb.ExternalIdsNoteDbImpl;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.CachedPreferences;
import com.google.gerrit.server.config.DefaultPreferencesCache;
import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.meta.TabFile;
import com.google.gerrit.server.index.IndexExecutor;
import com.google.gerrit.server.index.account.AccountIndexer;
import com.google.gerrit.server.util.time.TimeUtil;
@@ -64,11 +66,15 @@
import java.net.http.HttpClient;
import java.net.http.HttpRequest;
import java.net.http.HttpResponse;
+import java.nio.file.Files;
+import java.nio.file.Path;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ExecutionException;
@@ -132,7 +138,7 @@
}
public static final String BYID_AND_REV_NAME = "accounts";
- private final ExternalIds externalIds;
+ private final ExternalIdsNoteDbImpl externalIds;
private final LoadingCache<CachedAccountDetails.Key, CachedAccountDetails> accountDetailsCache;
private final GitRepositoryManager repoManager;
private final AllUsersName allUsersName;
@@ -141,7 +147,7 @@
@Inject
AccountCacheImpl(
- ExternalIds externalIds,
+ ExternalIdsNoteDbImpl externalIds,
@Named(BYID_AND_REV_NAME)
LoadingCache<CachedAccountDetails.Key, CachedAccountDetails> accountDetailsCache,
GitRepositoryManager repoManager,
@@ -243,13 +249,14 @@
private final AccountsUpdate.AccountsUpdateLoader accountsUpdateFactory;
private final Provider<AccountIndexer> accountIndexerProvider;
private final ListeningExecutorService executor;
- private final ExternalIds externalIds;
+ private final ExternalIdsNoteDbImpl externalIds;
private final Config config;
private final ExternalIdKeyFactory externalIdKeyFactory;
private final GitRepositoryManager repoManager;
private final AllUsersName allUsersName;
private final HttpClient client;
private final APIRateLimiter rateLimiter;
+ private final Map<Account.Id, List<ExternalId>> extIdsByAccount;
@Inject
Loader(
@@ -257,12 +264,13 @@
AccountsUpdate.AccountsUpdateLoader accountsUpdateFactory,
Provider<AccountIndexer> accountIndexerProvider,
@IndexExecutor(BATCH) ListeningExecutorService executor,
- ExternalIds externalIds,
+ ExternalIdsNoteDbImpl externalIds,
Config config,
ExternalIdKeyFactory externalIdKeyFactory,
GitRepositoryManager repoManager,
AllUsersName allUsersName,
- APIRateLimiter rateLimiter) {
+ APIRateLimiter rateLimiter,
+ ConfiguredExternalIds configuredExternalIds) {
this.accountsUpdateFactory = accountsUpdateFactory;
this.accountIndexerProvider = accountIndexerProvider;
this.executor = executor;
@@ -273,6 +281,7 @@
this.allUsersName = allUsersName;
this.rateLimiter = rateLimiter;
this.client = getHttpClient();
+ this.extIdsByAccount = configuredExternalIds.get();
}
@SuppressWarnings("unused")
@@ -285,7 +294,7 @@
}
}
Account.Id accountId = Account.id(accountInfo._accountId);
- Collection<ExternalId> extIds = new ArrayList<>();
+ Collection<ExternalId> extIds = extIdsByAccount.getOrDefault(accountId, new ArrayList<>());
Collection<ExternalId> oldExtIds = new ArrayList<>();
try {
extIds.addAll(getExternalIdsFromRemoteSite(accountId));
@@ -300,7 +309,7 @@
.setPreferredEmail(accountInfo.email)
.setStatus(accountInfo.status)
.deleteExternalIds(oldExtIds)
- .addExternalIds(extIds);
+ .updateExternalIds(extIds);
try (Repository repo = repoManager.openRepository(allUsersName)) {
Ref userRef = repo.exactRef(RefNames.refsUsers(key.accountId()));
@@ -404,8 +413,77 @@
}
}
+ protected static class ConfiguredExternalIds extends TabFile {
+ private static final String FILENAME = "remote-gerrit-account-cache_externalIds";
+ private static final String KEY_EXTERNAL_ID = "externalId";
+ private static final String KEY_EMAIL = "email";
+
+ private final ExternalIdKeyFactory externalIdKeyFactory;
+ private final Path configuredExternalIdsFile;
+
+ @Inject
+ ConfiguredExternalIds(ExternalIdKeyFactory externalIdKeyFactory, SitePaths site) {
+ this.externalIdKeyFactory = externalIdKeyFactory;
+ this.configuredExternalIdsFile = site.etc_dir.resolve(FILENAME);
+ }
+
+ Map<Account.Id, List<ExternalId>> get() {
+ List<TabFile.Row> rows;
+ try {
+ rows =
+ parse(
+ getFileContent(),
+ FILENAME,
+ TRIM,
+ TRIM,
+ error ->
+ logger.atSevere().log(
+ "Error parsing file %s: %s",
+ configuredExternalIdsFile, error.getMessage()));
+ } catch (IOException e) {
+ logger.atInfo().withCause(e).log("Unable to read %s", configuredExternalIdsFile);
+ return Collections.emptyMap();
+ }
+
+ Map<Account.Id, List<ExternalId>> extIdsByAccount = new HashMap<>();
+ for (TabFile.Row row : rows) {
+ try {
+ ExternalId externalId = parseRow(row);
+ extIdsByAccount
+ .computeIfAbsent(externalId.accountId(), k -> new ArrayList<>())
+ .add(externalId);
+ } catch (AccountNotFoundException e) {
+ logger.atSevere().withCause(e).log("Failed to process %s", row);
+ }
+ }
+ return extIdsByAccount;
+ }
+
+ protected String getFileContent() throws IOException {
+ return Files.readString(configuredExternalIdsFile);
+ }
+
+ protected ExternalId parseRow(TabFile.Row row) throws AccountNotFoundException {
+ Account.Id accountId =
+ Account.Id.tryParse(row.left)
+ .orElseThrow(() -> new AccountNotFoundException(row.left + " not found"));
+ Map<String, String> properties = new HashMap<>();
+ String[] args = row.right.split("\t");
+ for (String arg : args) {
+ String[] keyValue = arg.split("=", 2);
+ properties.put(keyValue[0], keyValue[1]);
+ }
+ return ExternalId.create(
+ externalIdKeyFactory.parse(properties.get(KEY_EXTERNAL_ID)),
+ accountId,
+ properties.get(KEY_EMAIL),
+ null,
+ null);
+ }
+ }
+
/** Signals that the account was not found in the primary storage. */
- private static class AccountNotFoundException extends Exception {
+ public static class AccountNotFoundException extends Exception {
private static final long serialVersionUID = 1L;
public AccountNotFoundException(String message) {
@@ -414,7 +492,7 @@
}
/** Signals that the account was not found in the remote Gerrit. */
- private static class AccountNotFoundInRemoteGerritException extends Exception {
+ public static class AccountNotFoundInRemoteGerritException extends Exception {
private static final long serialVersionUID = 1L;
public AccountNotFoundInRemoteGerritException(String message) {
diff --git a/test/docker/gerrit/etc/remote-gerrit-account-cache_externalIds b/test/docker/gerrit/etc/remote-gerrit-account-cache_externalIds
new file mode 100644
index 0000000..344c41a
--- /dev/null
+++ b/test/docker/gerrit/etc/remote-gerrit-account-cache_externalIds
@@ -0,0 +1 @@
+1000001 externalId=mailto:user1@secondary.com email=user1@secondary.com
diff --git a/test/test_remote_gerrit_account_cache.sh b/test/test_remote_gerrit_account_cache.sh
index 7aa5550..3b08a41 100755
--- a/test/test_remote_gerrit_account_cache.sh
+++ b/test/test_remote_gerrit_account_cache.sh
@@ -31,4 +31,22 @@
)
result_out_json "Email added on remote gerrit site is available on the internal site" \
"$EXPECTED" "$ACTUAL"
+
+# The test case assumes that `user1` does not have `user1@secondary.com` email
+# address configured on remote gerrit site and internal site has the external
+# id configured in $GERRIT_SITE/etc/remote-gerrit-account-cache_externalIds
+ACTUAL=$(gcurl --request GET "$GERRIT_BASE_URL/a/accounts/user1/external.ids" | \
+ jq '.[] | select(.identity == "mailto:user1@secondary.com")')
+EXPECTED=$(cat <<EOF
+{
+ "can_delete": true,
+ "email_address": "user1@secondary.com",
+ "identity": "mailto:user1@secondary.com",
+ "trusted": true
+}
+EOF
+)
+result_out_json "Configured external id is available on the internal site" \
+ "$EXPECTED" "$ACTUAL"
+
exit "$RESULT"
\ No newline at end of file
