diff --git a/scm-core/src/main/java/sonia/scm/security/AccessToken.java b/scm-core/src/main/java/sonia/scm/security/AccessToken.java index ac7700b030..3341500199 100644 --- a/scm-core/src/main/java/sonia/scm/security/AccessToken.java +++ b/scm-core/src/main/java/sonia/scm/security/AccessToken.java @@ -33,6 +33,7 @@ package sonia.scm.security; import java.util.Date; import java.util.Map; import java.util.Optional; +import java.util.Set; /** * An access token can be used to access scm-manager without providing username and password. An {@link AccessToken} can @@ -103,6 +104,13 @@ public interface AccessToken { */ Scope getScope(); + /** + * Returns name of groups, in which the user should be a member. + * + * @return name of groups + */ + Set getGroups(); + /** * Returns an optional value of a custom token field. * diff --git a/scm-core/src/main/java/sonia/scm/security/AccessTokenBuilder.java b/scm-core/src/main/java/sonia/scm/security/AccessTokenBuilder.java index 5e36ba468f..0924716bd8 100644 --- a/scm-core/src/main/java/sonia/scm/security/AccessTokenBuilder.java +++ b/scm-core/src/main/java/sonia/scm/security/AccessTokenBuilder.java @@ -89,16 +89,25 @@ public interface AccessTokenBuilder { * @return {@code this} */ AccessTokenBuilder refreshableFor(long count, TimeUnit unit); - + /** * Reduces the permissions of the token by providing a scope. - * + * * @param scope scope of token - * + * * @return {@code this} */ AccessTokenBuilder scope(Scope scope); - + + /** + * Define the logged in user as member of the given groups. + * + * @param groups group names + * + * @return {@code this} + */ + AccessTokenBuilder groups(String... groups); + /** * Creates a new {@link AccessToken} with the provided settings. * diff --git a/scm-core/src/main/java/sonia/scm/security/DAORealmHelper.java b/scm-core/src/main/java/sonia/scm/security/DAORealmHelper.java index 3fcab8762c..115bb082c9 100644 --- a/scm-core/src/main/java/sonia/scm/security/DAORealmHelper.java +++ b/scm-core/src/main/java/sonia/scm/security/DAORealmHelper.java @@ -37,7 +37,6 @@ import com.google.common.base.MoreObjects; import com.google.common.base.Strings; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet.Builder; -import org.apache.shiro.authc.AuthenticationException; import org.apache.shiro.authc.AuthenticationInfo; import org.apache.shiro.authc.AuthenticationToken; import org.apache.shiro.authc.DisabledAccountException; @@ -54,6 +53,8 @@ import sonia.scm.group.GroupNames; import sonia.scm.user.User; import sonia.scm.user.UserDAO; +import java.util.Collections; + import static com.google.common.base.Preconditions.checkArgument; /** @@ -63,8 +64,7 @@ import static com.google.common.base.Preconditions.checkArgument; * @author Sebastian Sdorra * @since 2.0.0 */ -public final class DAORealmHelper -{ +public final class DAORealmHelper { /** * the logger for DAORealmHelper @@ -109,37 +109,37 @@ public final class DAORealmHelper public CredentialsMatcher wrapCredentialsMatcher(CredentialsMatcher credentialsMatcher) { return new RetryLimitPasswordMatcher(loginAttemptHandler, credentialsMatcher); } - + /** - * Method description + * Creates {@link AuthenticationInfo} from a {@link UsernamePasswordToken}. The method accepts + * {@link AuthenticationInfo} as argument, so that the caller does not need to cast. * + * @param token authentication token, it must be {@link UsernamePasswordToken} * - * @param token - * - * @return - * - * @throws AuthenticationException + * @return authentication info */ - public AuthenticationInfo getAuthenticationInfo(AuthenticationToken token) throws AuthenticationException { + public AuthenticationInfo getAuthenticationInfo(AuthenticationToken token) { checkArgument(token instanceof UsernamePasswordToken, "%s is required", UsernamePasswordToken.class); UsernamePasswordToken upt = (UsernamePasswordToken) token; String principal = upt.getUsername(); - return getAuthenticationInfo(principal, null, null); + return getAuthenticationInfo(principal, null, null, Collections.emptySet()); } /** - * Method description + * Returns a builder for {@link AuthenticationInfo}. * + * @param principal name of principal (username) * - * @param principal - * @param credentials - * @param scope - * - * @return + * @return authentication info builder */ - public AuthenticationInfo getAuthenticationInfo(String principal, String credentials, Scope scope) { + public AuthenticationInfoBuilder authenticationInfoBuilder(String principal) { + return new AuthenticationInfoBuilder(principal); + } + + + private AuthenticationInfo getAuthenticationInfo(String principal, String credentials, Scope scope, Iterable groups) { checkArgument(!Strings.isNullOrEmpty(principal), "username is required"); LOG.debug("try to authenticate {}", principal); @@ -157,7 +157,7 @@ public final class DAORealmHelper collection.add(principal, realm); collection.add(user, realm); - collection.add(collectGroups(principal), realm); + collection.add(collectGroups(principal, groups), realm); collection.add(MoreObjects.firstNonNull(scope, Scope.empty()), realm); String creds = credentials; @@ -171,11 +171,15 @@ public final class DAORealmHelper //~--- methods -------------------------------------------------------------- - private GroupNames collectGroups(String principal) { + private GroupNames collectGroups(String principal, Iterable groupNames) { Builder builder = ImmutableSet.builder(); builder.add(GroupNames.AUTHENTICATED); + for (String group : groupNames) { + builder.add(group); + } + for (Group group : groupDAO.getAll()) { if (group.isMember(principal)) { builder.add(group.getName()); @@ -187,6 +191,69 @@ public final class DAORealmHelper return groups; } + /** + * Builder class for {@link AuthenticationInfo}. + */ + public class AuthenticationInfoBuilder { + + private final String principal; + + private String credentials; + private Scope scope; + private Iterable groups = Collections.emptySet(); + + private AuthenticationInfoBuilder(String principal) { + this.principal = principal; + } + + /** + * With credentials uses the given credentials for the {@link AuthenticationInfo}, this is particularly important + * for caching purposes. + * + * @param credentials credentials such as password + * + * @return {@code this} + */ + public AuthenticationInfoBuilder withCredentials(String credentials) { + this.credentials = credentials; + return this; + } + + /** + * With the scope object it is possible to limit the access permissions to scm-manager. + * + * @param scope scope object + * + * @return {@code this} + */ + public AuthenticationInfoBuilder withScope(Scope scope) { + this.scope = scope; + return this; + } + + /** + * With groups adds extra groups, besides those which come from the {@link GroupDAO}, to the authentication info. + * + * @param groups extra groups + * + * @return {@code this} + */ + public AuthenticationInfoBuilder withGroups(Iterable groups) { + this.groups = groups; + return this; + } + + /** + * Build creates the authentication info from the given information. + * + * @return authentication info + */ + public AuthenticationInfo build() { + return getAuthenticationInfo(principal, credentials, scope, groups); + } + + } + private static class RetryLimitPasswordMatcher implements CredentialsMatcher { private final LoginAttemptHandler loginAttemptHandler; diff --git a/scm-core/src/test/java/sonia/scm/security/DAORealmHelperTest.java b/scm-core/src/test/java/sonia/scm/security/DAORealmHelperTest.java new file mode 100644 index 0000000000..af4bf37915 --- /dev/null +++ b/scm-core/src/test/java/sonia/scm/security/DAORealmHelperTest.java @@ -0,0 +1,155 @@ +package sonia.scm.security; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import org.apache.shiro.authc.AuthenticationInfo; +import org.apache.shiro.authc.DisabledAccountException; +import org.apache.shiro.authc.UnknownAccountException; +import org.apache.shiro.authc.UsernamePasswordToken; +import org.apache.shiro.subject.PrincipalCollection; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import sonia.scm.group.Group; +import sonia.scm.group.GroupDAO; +import sonia.scm.group.GroupNames; +import sonia.scm.user.User; +import sonia.scm.user.UserDAO; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class DAORealmHelperTest { + + @Mock + private LoginAttemptHandler loginAttemptHandler; + + @Mock + private UserDAO userDAO; + + @Mock + private GroupDAO groupDAO; + + private DAORealmHelper helper; + + @BeforeEach + void setUpObjectUnderTest() { + helper = new DAORealmHelper(loginAttemptHandler, userDAO, groupDAO, "hitchhiker"); + } + + @Test + void shouldThrowExceptionWithoutUsername() { + assertThrows(IllegalArgumentException.class, () -> helper.authenticationInfoBuilder(null).build()); + } + + @Test + void shouldThrowExceptionWithEmptyUsername() { + assertThrows(IllegalArgumentException.class, () -> helper.authenticationInfoBuilder("").build()); + } + + @Test + void shouldThrowExceptionWithUnknownUser() { + assertThrows(UnknownAccountException.class, () -> helper.authenticationInfoBuilder("trillian").build()); + } + + @Test + void shouldThrowExceptionOnDisabledAccount() { + User user = new User("trillian"); + user.setActive(false); + when(userDAO.get("trillian")).thenReturn(user); + + assertThrows(DisabledAccountException.class, () -> helper.authenticationInfoBuilder("trillian").build()); + } + + @Test + void shouldReturnAuthenticationInfo() { + User user = new User("trillian"); + when(userDAO.get("trillian")).thenReturn(user); + + AuthenticationInfo authenticationInfo = helper.authenticationInfoBuilder("trillian").build(); + PrincipalCollection principals = authenticationInfo.getPrincipals(); + assertThat(principals.oneByType(User.class)).isSameAs(user); + assertThat(principals.oneByType(GroupNames.class)).containsOnly("_authenticated"); + assertThat(principals.oneByType(Scope.class)).isEmpty(); + } + + @Test + void shouldReturnAuthenticationInfoWithGroups() { + User user = new User("trillian"); + when(userDAO.get("trillian")).thenReturn(user); + + Group one = new Group("xml", "one", "trillian"); + Group two = new Group("xml", "two", "trillian"); + Group six = new Group("xml", "six", "dent"); + when(groupDAO.getAll()).thenReturn(ImmutableList.of(one, two, six)); + + AuthenticationInfo authenticationInfo = helper.authenticationInfoBuilder("trillian") + .withGroups(ImmutableList.of("three")) + .build(); + + PrincipalCollection principals = authenticationInfo.getPrincipals(); + assertThat(principals.oneByType(GroupNames.class)).containsOnly("_authenticated", "one", "two", "three"); + } + + @Test + void shouldReturnAuthenticationInfoWithScope() { + User user = new User("trillian"); + when(userDAO.get("trillian")).thenReturn(user); + + Scope scope = Scope.valueOf("user:*", "group:*"); + + AuthenticationInfo authenticationInfo = helper.authenticationInfoBuilder("trillian") + .withScope(scope) + .build(); + + PrincipalCollection principals = authenticationInfo.getPrincipals(); + assertThat(principals.oneByType(Scope.class)).isSameAs(scope); + } + + @Test + void shouldReturnAuthenticationInfoWithCredentials() { + User user = new User("trillian"); + when(userDAO.get("trillian")).thenReturn(user); + + AuthenticationInfo authenticationInfo = helper.authenticationInfoBuilder("trillian") + .withCredentials("secret") + .build(); + + assertThat(authenticationInfo.getCredentials()).isEqualTo("secret"); + } + + @Test + void shouldReturnAuthenticationInfoWithCredentialsFromUser() { + User user = new User("trillian"); + user.setPassword("secret"); + when(userDAO.get("trillian")).thenReturn(user); + + AuthenticationInfo authenticationInfo = helper.authenticationInfoBuilder("trillian").build(); + + assertThat(authenticationInfo.getCredentials()).isEqualTo("secret"); + } + + @Test + void shouldThrowExceptionWithWrongTypeOfToken() { + assertThrows(IllegalArgumentException.class, () -> helper.getAuthenticationInfo(BearerToken.valueOf("__bearer__"))); + } + + @Test + void shouldGetAuthenticationInfo() { + User user = new User("trillian"); + when(userDAO.get("trillian")).thenReturn(user); + + AuthenticationInfo authenticationInfo = helper.getAuthenticationInfo(new UsernamePasswordToken("trillian", "secret")); + + PrincipalCollection principals = authenticationInfo.getPrincipals(); + assertThat(principals.oneByType(User.class)).isSameAs(user); + assertThat(principals.oneByType(GroupNames.class)).containsOnly("_authenticated"); + assertThat(principals.oneByType(Scope.class)).isEmpty(); + + assertThat(authenticationInfo.getCredentials()).isNull(); + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/security/BearerRealm.java b/scm-webapp/src/main/java/sonia/scm/security/BearerRealm.java index 6847fd324c..b237a0a5ff 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/BearerRealm.java +++ b/scm-webapp/src/main/java/sonia/scm/security/BearerRealm.java @@ -101,11 +101,11 @@ public class BearerRealm extends AuthenticatingRealm BearerToken bt = (BearerToken) token; AccessToken accessToken = tokenResolver.resolve(bt); - return helper.getAuthenticationInfo( - accessToken.getSubject(), - bt.getCredentials(), - Scopes.fromClaims(accessToken.getClaims()) - ); + return helper.authenticationInfoBuilder(accessToken.getSubject()) + .withCredentials(bt.getCredentials()) + .withScope(Scopes.fromClaims(accessToken.getClaims())) + .withGroups(accessToken.getGroups()) + .build(); } } diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java index 4418cb40a8..64e26405a1 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessToken.java @@ -30,12 +30,15 @@ */ package sonia.scm.security; +import com.google.common.collect.ImmutableSet; import io.jsonwebtoken.Claims; import java.util.Collections; import java.util.Date; +import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import static java.util.Optional.ofNullable; @@ -49,6 +52,8 @@ public final class JwtAccessToken implements AccessToken { public static final String REFRESHABLE_UNTIL_CLAIM_KEY = "scm-manager.refreshExpiration"; public static final String PARENT_TOKEN_ID_CLAIM_KEY = "scm-manager.parentTokenId"; + public static final String GROUPS_CLAIM_KEY = "scm-manager.groups"; + private final Claims claims; private final String compact; @@ -103,6 +108,16 @@ public final class JwtAccessToken implements AccessToken { return Optional.ofNullable(claims.get(key)); } + @Override + @SuppressWarnings("unchecked") + public Set getGroups() { + Iterable groups = claims.get(GROUPS_CLAIM_KEY, Iterable.class); + if (groups != null) { + return ImmutableSet.copyOf(groups); + } + return ImmutableSet.of(); + } + @Override public String compact() { return compact; 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 66db720125..a2f4369cd7 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java @@ -39,9 +39,12 @@ import io.jsonwebtoken.SignatureAlgorithm; import java.time.Clock; import java.time.Instant; +import java.util.Collections; import java.util.Date; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; +import java.util.Set; import java.util.concurrent.TimeUnit; import org.apache.shiro.SecurityUtils; import org.apache.shiro.subject.Subject; @@ -74,6 +77,7 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { private Instant refreshExpiration; private String parentKeyId; private Scope scope = Scope.empty(); + private Set groups = new HashSet<>(); private final Map custom = Maps.newHashMap(); @@ -134,6 +138,12 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { return this; } + @Override + public JwtAccessTokenBuilder groups(String... groups) { + Collections.addAll(this.groups, groups); + return this; + } + JwtAccessTokenBuilder refreshExpiration(Instant refreshExpiration) { this.refreshExpiration = refreshExpiration; this.refreshableFor = 0; @@ -195,6 +205,10 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { claims.setIssuer(issuer); } + if (!groups.isEmpty()) { + claims.put(JwtAccessToken.GROUPS_CLAIM_KEY, groups); + } + // sign token and create compact version String compact = Jwts.builder() .setClaims(claims) diff --git a/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java b/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java index e66462cd01..5c7aa08f37 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java @@ -31,11 +31,15 @@ package sonia.scm.security; +import com.google.common.collect.ImmutableSet; import org.apache.shiro.authc.AuthenticationInfo; +import org.apache.shiro.authc.SimpleAuthenticationInfo; import org.apache.shiro.authc.UsernamePasswordToken; +import org.junit.Ignore; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Answers; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.invocation.InvocationOnMock; @@ -43,6 +47,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.stubbing.Answer; import java.util.HashMap; +import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -65,6 +70,9 @@ class BearerRealmTest { @Mock private DAORealmHelper realmHelper; + @Mock + private DAORealmHelper.AuthenticationInfoBuilder builder; + @Mock private AccessTokenResolver accessTokenResolver; @@ -84,15 +92,19 @@ class BearerRealmTest { void shouldDoGetAuthentication() { BearerToken bearerToken = BearerToken.valueOf("__bearer__"); AccessToken accessToken = mock(AccessToken.class); - when(accessToken.getSubject()).thenReturn("trillian"); - when(accessToken.getClaims()).thenReturn(new HashMap<>()); + Set groups = ImmutableSet.of("HeartOfGold", "Puzzle42"); + + when(accessToken.getSubject()).thenReturn("trillian"); + when(accessToken.getGroups()).thenReturn(groups); + when(accessToken.getClaims()).thenReturn(new HashMap<>()); when(accessTokenResolver.resolve(bearerToken)).thenReturn(accessToken); - // we have to use answer, because we could not mock the result of Scopes - when(realmHelper.getAuthenticationInfo( - anyString(), anyString(), any(Scope.class) - )).thenAnswer(createAnswer("trillian", "__bearer__", true)); + when(realmHelper.authenticationInfoBuilder("trillian")).thenReturn(builder); + when(builder.withGroups(groups)).thenReturn(builder); + when(builder.withCredentials("__bearer__")).thenReturn(builder); + when(builder.withScope(any(Scope.class))).thenReturn(builder); + when(builder.build()).thenReturn(authenticationInfo); AuthenticationInfo result = realm.doGetAuthenticationInfo(bearerToken); assertThat(result).isSameAs(authenticationInfo); @@ -102,25 +114,4 @@ class BearerRealmTest { void shouldThrowIllegalArgumentExceptionForWrongTypeOfToken() { assertThrows(IllegalArgumentException.class, () -> realm.doGetAuthenticationInfo(new UsernamePasswordToken())); } - - private Answer createAnswer(String expectedSubject, String expectedCredentials, boolean scopeEmpty) { - return (iom) -> { - String subject = iom.getArgument(0); - assertThat(subject).isEqualTo(expectedSubject); - String credentials = iom.getArgument(1); - assertThat(credentials).isEqualTo(expectedCredentials); - Scope scope = iom.getArgument(2); - assertThat(scope.isEmpty()).isEqualTo(scopeEmpty); - - return authenticationInfo; - }; - } - - private class MyAnswer implements Answer { - - @Override - public AuthenticationInfo answer(InvocationOnMock invocationOnMock) throws Throwable { - return null; - } - } } 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 c005e7d381..f7a2c02d01 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java @@ -47,8 +47,7 @@ import org.mockito.junit.MockitoJUnitRunner; import java.util.Set; import java.util.concurrent.TimeUnit; -import static org.hamcrest.Matchers.isEmptyOrNullString; -import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.*; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; @@ -135,6 +134,7 @@ public class JwtAccessTokenBuilderTest { .issuer("https://www.scm-manager.org") .expiresIn(5, TimeUnit.SECONDS) .custom("a", "b") + .groups("one", "two", "three") .scope(Scope.valueOf("repo:*")) .build(); @@ -161,5 +161,6 @@ public class JwtAccessTokenBuilderTest { assertEquals(token.getIssuer().get(), "https://www.scm-manager.org"); assertEquals("b", token.getCustom("a").get()); assertEquals("[\"repo:*\"]", token.getScope().toString()); + assertThat(token.getGroups(), containsInAnyOrder("one", "two", "three")); } }