Merged in feature/external_groups (pull request #191)

Support groups from external authentication resources

scm-core/src/main/java/sonia/scm/group/GroupNames.java

@@ -70,21 +70,9 @@ public final class GroupNames implements Serializable, Iterable<String>
    * Constructs ...
    *
    */
-  @SuppressWarnings("unchecked")
   public GroupNames()
   {
-    this.collection = Collections.EMPTY_LIST;
-  }
-
-  /**
-   * Constructs ...
-   *
-   *
-   * @param collection
-   */
-  public GroupNames(Collection<String> collection)
-  {
-    this.collection = Collections.unmodifiableCollection(collection);
+    this(Collections.emptyList());
   }
 
   /**
@@ -96,7 +84,30 @@ public final class GroupNames implements Serializable, Iterable<String>
    */
   public GroupNames(String groupName, String... groupNames)
   {
-    this.collection = Lists.asList(groupName, groupNames);
+    this(Lists.asList(groupName, groupNames));
+  }
+
+  /**
+   * Constructs ...
+   *
+   *
+   * @param collection
+   */
+  public GroupNames(Collection<String> collection)
+  {
+    this(collection, false);
+  }
+
+  /**
+   * Constructs ...
+   *
+   *
+   * @param collection
+   */
+  public GroupNames(Collection<String> collection, boolean external)
+  {
+    this.collection = Collections.unmodifiableCollection(collection);
+    this.external = external;
   }
 
   //~--- methods --------------------------------------------------------------
@@ -165,7 +176,7 @@ public final class GroupNames implements Serializable, Iterable<String>
   @Override
   public String toString()
   {
-    return Joiner.on(", ").join(collection);
+    return Joiner.on(", ").join(collection) + "(" + (external? "external": "internal") + ")";
   }
 
   //~--- get methods ----------------------------------------------------------
@@ -181,8 +192,13 @@ public final class GroupNames implements Serializable, Iterable<String>
     return collection;
   }
 
-  //~--- fields ---------------------------------------------------------------
+  public boolean isExternal() {
+    return external;
+  }
 
+  //~--- fields ---------------------------------------------------------------
   /** Field description */
   private final Collection<String> collection;
+
+  private final boolean external;
 }

scm-core/src/main/java/sonia/scm/security/SyncingRealmHelper.java

@@ -45,7 +45,12 @@ import sonia.scm.user.User;
 import sonia.scm.user.UserManager;
 import sonia.scm.web.security.AdministrationContext;
 
+import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
+
+import static java.util.Arrays.asList;
+import static java.util.Collections.emptyList;
 
 /**
  * Helper class for syncing realms. The class should simplify the creation of realms, which are syncing authenticated
@@ -80,22 +85,107 @@ public final class SyncingRealmHelper {
     this.groupManager = groupManager;
   }
 
-  //~--- methods --------------------------------------------------------------
   /**
    * Create {@link AuthenticationInfo} from user and groups.
-   *
-   *
-   * @param realm name of the realm
-   * @param user authenticated user
-   * @param groups groups of the authenticated user
-   *
-   * @return authentication info
    */
-  public AuthenticationInfo createAuthenticationInfo(String realm, User user,
-    String... groups) {
-    return createAuthenticationInfo(realm, user, ImmutableList.copyOf(groups));
+  public AuthenticationInfoBuilder.ForRealm authenticationInfo() {
+    return new AuthenticationInfoBuilder().new ForRealm();
   }
 
+  public class AuthenticationInfoBuilder {
+    private String realm;
+    private User user;
+    private Collection<String> groups;
+    private boolean external;
+
+    private AuthenticationInfo build() {
+      return SyncingRealmHelper.this.createAuthenticationInfo(realm, user, groups, external);
+    }
+
+    public class ForRealm {
+      private ForRealm() {
+      }
+
+      /**
+       * Sets the realm.
+       * @param realm name of the realm
+       */
+      public ForUser forRealm(String realm) {
+        AuthenticationInfoBuilder.this.realm = realm;
+        return AuthenticationInfoBuilder.this.new ForUser();
+      }
+    }
+
+    public class ForUser {
+      private ForUser() {
+      }
+
+      /**
+       * Sets the user.
+       * @param user authenticated user
+       */
+      public AuthenticationInfoBuilder.WithGroups andUser(User user) {
+        AuthenticationInfoBuilder.this.user = user;
+        return AuthenticationInfoBuilder.this.new WithGroups();
+      }
+    }
+
+    public class WithGroups {
+      private WithGroups() {
+      }
+
+      /**
+       * Build the authentication info without groups.
+       * @return The complete {@link AuthenticationInfo}
+       */
+      public AuthenticationInfo withoutGroups() {
+        return withGroups(emptyList());
+      }
+
+      /**
+       * Set the internal groups for the user.
+       * @param groups groups of the authenticated user
+       * @return The complete {@link AuthenticationInfo}
+       */
+      public AuthenticationInfo withGroups(String... groups) {
+        return withGroups(asList(groups));
+      }
+
+      /**
+       * Set the internal groups for the user.
+       * @param groups groups of the authenticated user
+       * @return The complete {@link AuthenticationInfo}
+       */
+      public AuthenticationInfo withGroups(Collection<String> groups) {
+        AuthenticationInfoBuilder.this.groups = groups;
+        AuthenticationInfoBuilder.this.external = false;
+        return build();
+      }
+
+      /**
+       * Set the external groups for the user.
+       * @param groups external groups of the authenticated user
+       * @return The complete {@link AuthenticationInfo}
+       */
+      public AuthenticationInfo withExternalGroups(String... groups) {
+        return withExternalGroups(asList(groups));
+      }
+
+      /**
+       * Set the external groups for the user.
+       * @param groups external groups of the authenticated user
+       * @return The complete {@link AuthenticationInfo}
+       */
+      public AuthenticationInfo withExternalGroups(Collection<String> groups) {
+        AuthenticationInfoBuilder.this.groups = groups;
+        AuthenticationInfoBuilder.this.external = true;
+        return build();
+      }
+    }
+  }
+
+  //~--- methods --------------------------------------------------------------
+
   /**
    * Create {@link AuthenticationInfo} from user and groups.
    *
@@ -106,13 +196,13 @@ public final class SyncingRealmHelper {
    *
    * @return authentication info
    */
-  public AuthenticationInfo createAuthenticationInfo(String realm, User user,
-    Collection<String> groups) {
+  private AuthenticationInfo createAuthenticationInfo(String realm, User user,
+    Collection<String> groups, boolean externalGroups) {
     SimplePrincipalCollection collection = new SimplePrincipalCollection();
 
     collection.add(user.getId(), realm);
     collection.add(user, realm);
-    collection.add(new GroupNames(groups), realm);
+    collection.add(new GroupNames(groups, externalGroups), realm);
 
     return new SimpleAuthenticationInfo(collection, user.getPassword());
   }
@@ -161,6 +251,6 @@ public final class SyncingRealmHelper {
 
         }
       }
-      });
-    }
+    });
   }
+}

scm-core/src/test/java/sonia/scm/security/SyncingRealmHelperTest.java

@@ -37,6 +37,7 @@ package sonia.scm.security;
 
 import com.google.common.base.Throwables;
 import org.apache.shiro.authc.AuthenticationInfo;
+import org.assertj.core.api.Assertions;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -53,10 +54,14 @@ import sonia.scm.web.security.PrivilegedAction;
 
 import java.io.IOException;
 
+import static java.util.Collections.singletonList;
+import static org.assertj.core.util.Arrays.asList;
 import static org.hamcrest.Matchers.hasItem;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -106,25 +111,6 @@ public class SyncingRealmHelperTest {
     helper = new SyncingRealmHelper(ctx, userManager, groupManager);
   }
 
-  /**
-   * Tests {@link SyncingRealmHelper#createAuthenticationInfo(String, User, String...)}.
-   */
-  @Test
-  public void testCreateAuthenticationInfo() {
-    User user = new User("tricia");
-    AuthenticationInfo authInfo = helper.createAuthenticationInfo("unit-test",
-      user, "heartOfGold");
-
-    assertNotNull(authInfo);
-    assertEquals("tricia", authInfo.getPrincipals().getPrimaryPrincipal());
-    assertThat(authInfo.getPrincipals().getRealmNames(), hasItem("unit-test"));
-    assertEquals(user, authInfo.getPrincipals().oneByType(User.class));
-
-    GroupNames groups = authInfo.getPrincipals().oneByType(GroupNames.class);
-
-    assertThat(groups, hasItem("heartOfGold"));
-  }
-
   /**
    * Tests {@link SyncingRealmHelper#store(Group)}.
    *
@@ -198,4 +184,45 @@ public class SyncingRealmHelperTest {
     helper.store(user);
     verify(userManager, times(1)).modify(user);
   }
+
+  @Test
+  public void builderShouldSetInternalGroups() {
+    AuthenticationInfo authenticationInfo = helper
+      .authenticationInfo()
+      .forRealm("unit-test")
+      .andUser(new User("ziltoid"))
+      .withGroups("internal");
+
+    GroupNames groupNames = authenticationInfo.getPrincipals().oneByType(GroupNames.class);
+    Assertions.assertThat(groupNames.getCollection()).containsOnly("internal");
+    Assertions.assertThat(groupNames.isExternal()).isFalse();
+  }
+
+  @Test
+  public void builderShouldSetExternalGroups() {
+    AuthenticationInfo authenticationInfo = helper
+      .authenticationInfo()
+      .forRealm("unit-test")
+      .andUser(new User("ziltoid"))
+      .withExternalGroups("external");
+
+    GroupNames groupNames = authenticationInfo.getPrincipals().oneByType(GroupNames.class);
+    Assertions.assertThat(groupNames.getCollection()).containsOnly("external");
+    Assertions.assertThat(groupNames.isExternal()).isTrue();
+  }
+
+  @Test
+  public void builderShouldSetValues() {
+    User user = new User("ziltoid");
+    AuthenticationInfo authInfo = helper
+      .authenticationInfo()
+      .forRealm("unit-test")
+      .andUser(user)
+      .withoutGroups();
+
+    assertNotNull(authInfo);
+    assertEquals("ziltoid", authInfo.getPrincipals().getPrimaryPrincipal());
+    assertThat(authInfo.getPrincipals().getRealmNames(), hasItem("unit-test"));
+    assertEquals(user, authInfo.getPrincipals().oneByType(User.class));
+  }
 }

scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java

@@ -50,6 +50,7 @@ import org.apache.shiro.SecurityUtils;
 import org.apache.shiro.subject.Subject;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import sonia.scm.group.GroupNames;
 
 /**
  * Jwt implementation of {@link AccessTokenBuilder}.
@@ -207,6 +208,12 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder {
 
     if (!groups.isEmpty()) {
       claims.put(JwtAccessToken.GROUPS_CLAIM_KEY, groups);
+    } else {
+      Subject currentSubject = SecurityUtils.getSubject();
+      GroupNames groupNames = currentSubject.getPrincipals().oneByType(GroupNames.class);
+      if (groupNames != null && groupNames.isExternal()) {
+        claims.put(JwtAccessToken.GROUPS_CLAIM_KEY, groupNames.getCollection().toArray(new String[]{}));
+      }
     }
 
     // sign token and create compact version

scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java

@@ -36,23 +36,32 @@ import com.github.sdorra.shiro.SubjectAware;
 import com.google.common.collect.Sets;
 import io.jsonwebtoken.Claims;
 import io.jsonwebtoken.Jwts;
+import org.apache.shiro.SecurityUtils;
+import org.apache.shiro.subject.PrincipalCollection;
+import org.apache.shiro.subject.Subject;
 import org.apache.shiro.util.ThreadContext;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
+import org.mockito.invocation.InvocationOnMock;
 import org.mockito.junit.MockitoJUnitRunner;
+import sonia.scm.group.GroupNames;
 
+import java.util.Arrays;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.anyString;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
 import static sonia.scm.security.SecureKeyTestUtil.createSecureKey;
 
@@ -62,6 +71,11 @@ import static sonia.scm.security.SecureKeyTestUtil.createSecureKey;
  * @author Sebastian Sdorra
  */
 @RunWith(MockitoJUnitRunner.class)
+@SubjectAware(
+  configuration = "classpath:sonia/scm/shiro-001.ini",
+  username = "trillian",
+  password = "secret"
+)
 public class JwtAccessTokenBuilderTest {
   
   {
@@ -96,11 +110,6 @@ public class JwtAccessTokenBuilderTest {
    * Tests {@link JwtAccessTokenBuilder#build()} with subject from shiro context.
    */
   @Test
-  @SubjectAware(
-    configuration = "classpath:sonia/scm/shiro-001.ini",
-    username = "trillian",
-    password = "secret"
-  )
   public void testBuildWithoutSubject() {
     JwtAccessToken token = factory.create().build();
     assertEquals("trillian", token.getSubject());
@@ -150,7 +159,33 @@ public class JwtAccessTokenBuilderTest {
       .getBody();
     assertClaims(new JwtAccessToken(claims, compact));
   }
-  
+
+  @Test
+  public void testWithExternalGroups() {
+    applyExternalGroupsToSubject(true, "external");
+    JwtAccessToken token = factory.create().subject("dent").build();
+    assertArrayEquals(new String[]{"external"}, token.getCustom(JwtAccessToken.GROUPS_CLAIM_KEY).map(x -> (String[]) x).get());
+  }
+
+  @Test
+  public void testWithInternalGroups() {
+    applyExternalGroupsToSubject(false, "external");
+    JwtAccessToken token = factory.create().subject("dent").build();
+    assertFalse(token.getCustom(JwtAccessToken.GROUPS_CLAIM_KEY).isPresent());
+  }
+
+  private void applyExternalGroupsToSubject(boolean external, String... groups) {
+    Subject subject = spy(SecurityUtils.getSubject());
+    when(subject.getPrincipals()).thenAnswer(invocation -> enrichWithGroups(invocation, groups, external));
+    shiro.setSubject(subject);
+  }
+
+  private Object enrichWithGroups(InvocationOnMock invocation, String[] groups, boolean external) throws Throwable {
+    PrincipalCollection principals = (PrincipalCollection) spy(invocation.callRealMethod());
+    when(principals.oneByType(GroupNames.class)).thenReturn(new GroupNames(Arrays.asList(groups), external));
+    return principals;
+  }
+
   private void assertClaims(JwtAccessToken token){
     assertThat(token.getId(), not(isEmptyOrNullString()));
     assertNotNull( token.getIssuedAt() );