diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt index 771e323190..995b1551ce 100644 --- a/Documentation/access-control.txt +++ b/Documentation/access-control.txt @@ -220,11 +220,16 @@ shortest possible pattern expansion must be a valid ref name: thus `^refs/heads/.*/name` will fail because `refs/heads//name` is not a valid reference, but `^refs/heads/.+/name` will work. -References can have the current user name automatically included, -creating dynamic access controls that change to match the currently -logged in user. For example to provide a personal sandbox space -to all developers, `+refs/heads/sandbox/${username}/*+` allowing -the user 'joe' to use 'refs/heads/sandbox/joe/foo'. +References can have the user name or the sharded account ID of the +current user automatically included, creating dynamic access controls +that change to match the currently logged in user. For example to +provide a personal sandbox space to all developers, +`+refs/heads/sandbox/${username}/*+` allows the user 'joe' to use +'refs/heads/sandbox/joe/foo'. The sharded account ID can be used to +give users access to their user branch in the `All-Users` repository, +for example `+refs/users/${shardeduserid}+` is resolved to +'refs/users/23/1011123' if the account ID of the current user is +`1011123`. When evaluating a reference-level access right, Gerrit will use the full set of access rights to determine if the user diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java index adbc9ceee2..b2d6a901f2 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java @@ -61,6 +61,7 @@ import com.google.gerrit.reviewdb.client.AccountExternalId; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.git.ProjectConfig; +import com.google.gerrit.server.project.RefPattern; import com.google.gerrit.server.util.MagicBranch; import com.google.gerrit.testutil.ConfigSuite; import com.google.gerrit.testutil.FakeEmailSender.Message; @@ -393,7 +394,8 @@ public class AccountIT extends AbstractDaemonTest { } // allow each user to read its own user branch - grant(Permission.READ, allUsers, RefNames.REFS_USERS + "*", false, + grant(Permission.READ, allUsers, + RefNames.REFS_USERS + "${" + RefPattern.USERID_SHARDED + "}", false, REGISTERED_USERS); // fetch user branch using refs/users/YY/XXXXXXX diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RefNames.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RefNames.java index ec4ddb2e71..1af82da876 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RefNames.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RefNames.java @@ -84,14 +84,7 @@ public class RefNames { public static String changeMetaRef(Change.Id id) { StringBuilder r = new StringBuilder(); r.append(REFS_CHANGES); - int n = id.get(); - int m = n % 100; - if (m < 10) { - r.append('0'); - } - r.append(m); - r.append('/'); - r.append(n); + r.append(shard(id.get())); r.append(META_SUFFIX); return r.toString(); } @@ -99,14 +92,7 @@ public class RefNames { public static String refsUsers(Account.Id accountId) { StringBuilder r = new StringBuilder(); r.append(REFS_USERS); - int account = accountId.get(); - int m = account % 100; - if (m < 10) { - r.append('0'); - } - r.append(m); - r.append('/'); - r.append(account); + r.append(shard(accountId.get())); return r.toString(); } @@ -135,6 +121,16 @@ public class RefNames { private static StringBuilder buildRefsPrefix(String prefix, int id) { StringBuilder r = new StringBuilder(); r.append(prefix); + r.append(shard(id)); + r.append('/'); + return r; + } + + public static String shard(int id) { + if (id < 0) { + return null; + } + StringBuilder r = new StringBuilder(); int n = id % 100; if (n < 10) { r.append('0'); @@ -142,8 +138,7 @@ public class RefNames { r.append(n); r.append('/'); r.append(id); - r.append('/'); - return r; + return r.toString(); } /** diff --git a/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/RefNamesTest.java b/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/RefNamesTest.java index 4e3b6593bd..af1c0740de 100644 --- a/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/RefNamesTest.java +++ b/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/RefNamesTest.java @@ -141,4 +141,14 @@ public class RefNamesTest { assertThat(parseRefSuffix("a4")).isNull(); assertThat(parseRefSuffix("4a")).isNull(); } + + @Test + public void shard() throws Exception { + assertThat(RefNames.shard(1011123)).isEqualTo("23/1011123"); + assertThat(RefNames.shard(537)).isEqualTo("37/537"); + assertThat(RefNames.shard(12)).isEqualTo("12/12"); + assertThat(RefNames.shard(0)).isEqualTo("00/0"); + assertThat(RefNames.shard(1)).isEqualTo("01/1"); + assertThat(RefNames.shard(-1)).isNull(); + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefPattern.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefPattern.java index 2b03951a7b..52663d09a8 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefPattern.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefPattern.java @@ -26,6 +26,7 @@ import java.util.regex.Pattern; import java.util.regex.PatternSyntaxException; public class RefPattern { + public static final String USERID_SHARDED = "shardeduserid"; public static final String USERNAME = "username"; public static String shortestExample(String refPattern) { @@ -77,7 +78,9 @@ public class RefPattern { public static void validateRegExp(String refPattern) throws InvalidNameException { try { - Pattern.compile(refPattern.replace("${" + USERNAME + "}/", "")); + refPattern = refPattern.replace("${" + USERID_SHARDED + "}", ""); + refPattern = refPattern.replace("${" + USERNAME + "}", ""); + Pattern.compile(refPattern); } catch (PatternSyntaxException e) { throw new InvalidNameException(refPattern + " " + e.getMessage()); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefPatternMatcher.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefPatternMatcher.java index 97f70b43dd..d8f4054538 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefPatternMatcher.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefPatternMatcher.java @@ -16,14 +16,18 @@ package com.google.gerrit.server.project; import static com.google.gerrit.server.project.RefPattern.isRE; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.gerrit.common.data.ParameterizedString; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.server.CurrentUser; import dk.brics.automaton.Automaton; -import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import java.util.Set; import java.util.regex.Pattern; @@ -89,15 +93,17 @@ public abstract class RefPatternMatcher { template = new ParameterizedString(pattern); if (isRE(pattern)) { - // Replace ${username} with ":USERNAME:" as : is not legal - // in a reference and the string :USERNAME: is not likely to - // be a valid part of the regex. This later allows the pattern - // prefix to be clipped, saving time on evaluation. - String replacement = ":" + RefPattern.USERNAME.toUpperCase() + ":"; + // Replace ${username} and ${shardeduserid} with ":PLACEHOLDER:" + // as : is not legal in a reference and the string :PLACEHOLDER: + // is not likely to be a valid part of the regex. This later + // allows the pattern prefix to be clipped, saving time on + // evaluation. + String replacement = ":PLACEHOLDER:"; + Map params = ImmutableMap.of( + RefPattern.USERID_SHARDED, replacement, + RefPattern.USERNAME, replacement); Automaton am = - RefPattern.toRegExp( - template.replace(Collections.singletonMap(RefPattern.USERNAME, - replacement))).toAutomaton(); + RefPattern.toRegExp(template.replace(params)).toAutomaton(); String rePrefix = am.getCommonPrefix(); prefix = rePrefix.substring(0, rePrefix.indexOf(replacement)); } else { @@ -119,8 +125,11 @@ public abstract class RefPatternMatcher { u = username; } - RefPatternMatcher next = getMatcher(expand(template, u)); - if (next != null && next.match(expand(ref, u), user)) { + Account.Id accountId = user.isIdentifiedUser() + ? user.getAccountId() + : null; + RefPatternMatcher next = getMatcher(expand(template, u, accountId)); + if (next != null && next.match(expand(ref, u, accountId), user)) { return true; } } @@ -147,16 +156,21 @@ public abstract class RefPatternMatcher { return ref.startsWith(prefix); } - private String expand(String parameterizedRef, String userName) { + private String expand(String parameterizedRef, String userName, Account.Id accountId) { if (parameterizedRef.contains("${")) { - return expand(new ParameterizedString(parameterizedRef), userName); + return expand(new ParameterizedString(parameterizedRef), userName, accountId); } return parameterizedRef; } - private String expand(ParameterizedString parameterizedRef, String userName) { - return parameterizedRef - .replace(Collections.singletonMap(RefPattern.USERNAME, userName)); + private String expand(ParameterizedString parameterizedRef, String userName, + Account.Id accountId) { + Map params = new HashMap<>(); + params.put(RefPattern.USERNAME, userName); + if (accountId != null) { + params.put(RefPattern.USERID_SHARDED, RefNames.shard(accountId.get())); + } + return parameterizedRef.replace(params); } } }