diff --git a/scm-core/src/main/java/sonia/scm/group/GroupNames.java b/scm-core/src/main/java/sonia/scm/group/GroupNames.java index 24d32972b6..faeec7218b 100644 --- a/scm-core/src/main/java/sonia/scm/group/GroupNames.java +++ b/scm-core/src/main/java/sonia/scm/group/GroupNames.java @@ -70,21 +70,9 @@ public final class GroupNames implements Serializable, Iterable * Constructs ... * */ - @SuppressWarnings("unchecked") public GroupNames() { - this.collection = Collections.EMPTY_LIST; - } - - /** - * Constructs ... - * - * - * @param collection - */ - public GroupNames(Collection collection) - { - this.collection = Collections.unmodifiableCollection(collection); + this(Collections.emptyList()); } /** @@ -96,7 +84,30 @@ public final class GroupNames implements Serializable, Iterable */ public GroupNames(String groupName, String... groupNames) { - this.collection = Lists.asList(groupName, groupNames); + this(Lists.asList(groupName, groupNames)); + } + + /** + * Constructs ... + * + * + * @param collection + */ + public GroupNames(Collection collection) + { + this(collection, false); + } + + /** + * Constructs ... + * + * + * @param collection + */ + public GroupNames(Collection collection, boolean external) + { + this.collection = Collections.unmodifiableCollection(collection); + this.external = external; } //~--- methods -------------------------------------------------------------- @@ -165,7 +176,7 @@ public final class GroupNames implements Serializable, Iterable @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 return collection; } - //~--- fields --------------------------------------------------------------- + public boolean isExternal() { + return external; + } + //~--- fields --------------------------------------------------------------- /** Field description */ private final Collection collection; + + private final boolean external; } diff --git a/scm-core/src/main/java/sonia/scm/security/SyncingRealmHelper.java b/scm-core/src/main/java/sonia/scm/security/SyncingRealmHelper.java index 0e3c06e32d..da11400140 100644 --- a/scm-core/src/main/java/sonia/scm/security/SyncingRealmHelper.java +++ b/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 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 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 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 groups) { + private AuthenticationInfo createAuthenticationInfo(String realm, User user, + Collection 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 { } } - }); - } + }); } +} diff --git a/scm-core/src/test/java/sonia/scm/security/SyncingRealmHelperTest.java b/scm-core/src/test/java/sonia/scm/security/SyncingRealmHelperTest.java index 50bde53ae1..b8d6ae909e 100644 --- a/scm-core/src/test/java/sonia/scm/security/SyncingRealmHelperTest.java +++ b/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)); + } } diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java index a2f4369cd7..61c7631119 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java +++ b/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 diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java index f7a2c02d01..2928a6c7e6 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java +++ b/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() );