[i,l,-r] auto-narrow: cap auto-picked teammates at 5 closest (SECOP-952)

sources/pom.xml

@@ -24,7 +24,7 @@
   <modelVersion>4.0.0</modelVersion>
   <groupId>com.google.solutions</groupId>
   <artifactId>jitaccess</artifactId>
-  <version>2.3.0-wavemm.9</version>
+  <version>2.3.0-wavemm.10</version>
   <properties>
     <surefire-plugin.version>3.5.3</surefire-plugin.version>
     <surefire-plugin.version>3.5.3</surefire-plugin.version>

sources/src/main/java/com/google/solutions/jitaccess/web/rest/GroupsResource.java

@@ -407,19 +407,33 @@ private static boolean parseNotifyReviewers(@Nullable List<String> raw) {
    * and — with no teammate to narrow to — refuse to broadcast and
    * require an explicit pick instead.
    *
-   * <p>Also caps how many teammates the auto-narrowed filter itself
-   * carries. Deliberately distinct from {@link
-   * ReviewerCandidates#SUGGESTED_BADGE_TOP_N}, which is a UI
-   * presentation cap: tying the DM audience to the badge would let a
-   * cosmetic tweak silently change production notification fan-out.
-   *
    * <p>Sized to a generous single team: Wave teams are typically &lt;15
    * (see {@link ReviewerCandidates#SUGGESTION_GROUP_MAX_SIZE}), so a set
    * larger than this almost always means a broad group like
-   * {@code engineering@} crept into the ACL.
+   * {@code engineering@} crept into the ACL. Note this tolerance applies
+   * to approvers the policy names explicitly (or a no-teammate-signal
+   * fallback); the AUTO-PICKED teammate set has its own, much smaller
+   * cap — {@link #AUTO_NARROW_TEAMMATE_CAP}.
    */
   static final int AUTO_NARROW_BROADCAST_LIMIT = 15;
 
+  /**
+   * Maximum number of teammates the auto-narrow picks on the
+   * requester's behalf (SECOP-952 follow-up). Candidates are ranked by
+   * team affinity, so this is "your {@value} closest colleagues" — the
+   * first production broad-role request auto-picked 15 and that was
+   * still judged too loud; when nobody is picked explicitly, a handful
+   * of close teammates is the intent, not a mid-sized broadcast.
+   *
+   * <p>Deliberately distinct from {@link
+   * ReviewerCandidates#SUGGESTED_BADGE_TOP_N} (a UI presentation cap:
+   * tying the DM audience to the badge would let a cosmetic tweak
+   * silently change production notification fan-out) and from
+   * {@link #AUTO_NARROW_BROADCAST_LIMIT} (the tolerance for DMing an
+   * explicitly-named small approver set in full).
+   */
+  static final int AUTO_NARROW_TEAMMATE_CAP = 5;
+
   /**
    * Compute the reviewer filter for an empty picker selection on a
    * join that requires approval (SECOP-952). The requester didn't pick
@@ -442,7 +456,7 @@ private static boolean parseNotifyReviewers(@Nullable List<String> raw) {
    *   <li>a non-empty subset — the requester's teammates
    *       ({@link ReviewerCandidates.Candidate#teammate()}, i.e.
    *       sharing at least one small group), highest-affinity first,
-   *       capped at {@link #AUTO_NARROW_BROADCAST_LIMIT}. Used when
+   *       capped at {@link #AUTO_NARROW_TEAMMATE_CAP}. Used when
    *       the ACL contains a group (or many direct approvers) so we
    *       avoid broadcasting to the whole group.
    * </ul>
@@ -550,16 +564,16 @@ private static boolean parseNotifyReviewers(@Nullable List<String> raw) {
     }
 
     //
-    // Teammates = every candidate sharing at least one small group
-    // with the requester, in affinity order (compute() ranks them
-    // score-first). NOT the `suggested` badge: that is capped at
-    // ReviewerCandidates.SUGGESTED_BADGE_TOP_N for presentation, and
-    // an audience of 3 is fragile — a couple of OOO or offboarded
-    // teammates would strand the request.
+    // Teammates = candidates sharing at least one small group with the
+    // requester, in affinity order (compute() ranks them score-first),
+    // capped at the requester's AUTO_NARROW_TEAMMATE_CAP closest. NOT
+    // the `suggested` badge (a presentation cap), and deliberately a
+    // few rather than one — a couple of OOO or offboarded teammates
+    // must not strand the request.
     //
     var teammates = candidates.stream()
       .filter(ReviewerCandidates.Candidate::teammate)
-      .limit(AUTO_NARROW_BROADCAST_LIMIT)
+      .limit(AUTO_NARROW_TEAMMATE_CAP)
       .map(c -> new EndUserId(c.email()))
       .collect(Collectors.toSet());
 

sources/src/test/java/com/google/solutions/jitaccess/web/rest/TestGroupsResource.java

@@ -733,6 +733,89 @@ public void post_whenNoReviewersPickedAndTeammatesExist_narrowsToTeammates()
       "empty selection on a broad set must narrow to teammates");
   }
 
+  /**
+   * SECOP-952 follow-up: the auto-picked teammate set is capped at
+   * AUTO_NARROW_TEAMMATE_CAP (5) even when more teammates exist — the
+   * first production broad-role request auto-picked 15 and that was
+   * still too loud. Higher-affinity teammates (more shared small
+   * groups) win the cut.
+   */
+  @Test
+  public void post_whenManyTeammates_capsAutoPickAtFiveClosest()
+    throws Exception {
+    GroupsResource.clearReviewerRateLimiters();
+
+    // 8 teammates; three of them share TWO small groups with the
+    // requester (higher affinity), five share one. A broad group-free
+    // ACL pushes the request onto the auto-narrow path.
+    var acl = new AccessControlList.Builder()
+      .allow(SAMPLE_USER, PolicyPermission.JOIN.toMask());
+    for (int i = 0; i < 8; i++) {
+      acl.allow(new EndUserId("teammate-" + i + "@example.com"),
+        PolicyPermission.APPROVE_OTHERS.toMask());
+    }
+    for (int i = 0; i < 20; i++) {
+      acl.allow(new EndUserId("approver-" + i + "@example.com"),
+        PolicyPermission.APPROVE_OTHERS.toMask());
+    }
+    var group = Policies.createJitGroupPolicy(
+      "g-1",
+      acl.build(),
+      Map.of(Policy.ConstraintClass.JOIN, List.of(new ExpiryConstraint(Duration.ofMinutes(1)))));
+
+    var resource = new GroupsResource();
+    resource.options = new GroupsResource.Options(false);
+    resource.logger = Mockito.mock(Logger.class);
+    resource.auditTrail = Mockito.mock(OperationAuditTrail.class);
+    resource.catalog = createCatalog(group);
+    resource.subject = Subjects.create(SAMPLE_USER);
+    resource.executor = Runnable::run;
+    resource.groupsClient = Mockito.mock(CloudIdentityGroupsClient.class);
+    resource.proposalHandler = Mockito.mock(ProposalHandler.class);
+    when(resource.proposalHandler.propose(any(), any(), any()))
+      .thenReturn(new ProposalHandler.ProposalToken(
+        "token", Set.of(SAMPLE_APPROVING_USER), Instant.MAX));
+
+    var teamA = new GroupId("team-a@example.com");   // all 8 teammates
+    var teamB = new GroupId("team-b@example.com");   // teammates 5..7 only
+    when(resource.groupsClient.listMembershipsByUser(eq(SAMPLE_USER)))
+      .thenReturn(List.of(
+        new MembershipRelation().setGroupKey(new EntityKey().setId(teamA.email)),
+        new MembershipRelation().setGroupKey(new EntityKey().setId(teamB.email))));
+    var teamAMembers = new java.util.ArrayList<Membership>();
+    for (int i = 0; i < 8; i++) {
+      teamAMembers.add(new Membership().setType("user")
+        .setPreferredMemberKey(new EntityKey().setId("teammate-" + i + "@example.com")));
+    }
+    when(resource.groupsClient.listMemberships(eq(teamA))).thenReturn(teamAMembers);
+    when(resource.groupsClient.listMemberships(eq(teamB)))
+      .thenReturn(List.of(
+        new Membership().setType("user")
+          .setPreferredMemberKey(new EntityKey().setId("teammate-5@example.com")),
+        new Membership().setType("user")
+          .setPreferredMemberKey(new EntityKey().setId("teammate-6@example.com")),
+        new Membership().setType("user")
+          .setPreferredMemberKey(new EntityKey().setId("teammate-7@example.com"))));
+
+    resource.post(
+      group.id().environment(),
+      group.id().system(),
+      group.id().name(),
+      new MultivaluedHashMap<>());
+
+    var captor = org.mockito.ArgumentCaptor.forClass(
+      ProposalHandler.ProposeOptions.class);
+    verify(resource.proposalHandler).propose(any(), any(), captor.capture());
+    var picked = captor.getValue().reviewerFilter();
+    assertEquals(GroupsResource.AUTO_NARROW_TEAMMATE_CAP, picked.size(),
+      "auto-pick must stop at the teammate cap");
+    // The three double-affinity teammates must all make the cut; the
+    // remaining two slots go to single-affinity teammates.
+    assertTrue(picked.contains(new EndUserId("teammate-5@example.com")));
+    assertTrue(picked.contains(new EndUserId("teammate-6@example.com")));
+    assertTrue(picked.contains(new EndUserId("teammate-7@example.com")));
+  }
+
   /**
    * SECOP-952 safety net: on an empty selection, when the approver set
    * is large AND the requester shares no team with anyone, we refuse to