From b2e1dcf0e9addbd12ee2e6c94a17c8af2e25be03 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 16 Jan 2019 14:40:34 +0100 Subject: [PATCH 1/9] archive artifacts if the build runs on the main branch --- Jenkinsfile | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Jenkinsfile b/Jenkinsfile index f69069d72c..693c7efb00 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -50,6 +50,11 @@ node('docker') { def dockerImageTag = "2.0.0-dev-${commitHash.substring(0,7)}-${BUILD_NUMBER}" if (isMainBranch()) { + stage('Archive') { + archiveArtifacts 'scm-webapp/target/scm-webapp.war' + archiveArtifacts 'scm-server/target/scm-server-app.*' + } + stage('Docker') { def image = docker.build('cloudogu/scm-manager') docker.withRegistry('', 'hub.docker.com-cesmarvin') { From 66357ca196b2994f0a928e255c09ebea449df0bd Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 17 Jan 2019 15:40:11 +0100 Subject: [PATCH 2/9] display group membership on the profile page (/me) --- scm-ui-components/packages/ui-types/src/Me.js | 1 + scm-ui/public/locales/en/commons.json | 1 + scm-ui/src/containers/ProfileInfo.js | 116 ++++++----- scm-ui/src/modules/auth.js | 9 - .../scm/api/v2/resources/MapperModule.java | 2 +- .../sonia/scm/api/v2/resources/MeDto.java | 26 +++ .../scm/api/v2/resources/MeDtoFactory.java | 81 ++++++++ .../scm/api/v2/resources/MeResource.java | 30 ++- .../api/v2/resources/MeToUserDtoMapper.java | 45 ----- .../api/v2/resources/MeDtoFactoryTest.java | 186 ++++++++++++++++++ .../scm/api/v2/resources/MeResourceTest.java | 36 ++-- .../v2/resources/MeToUserDtoMapperTest.java | 151 -------------- 12 files changed, 392 insertions(+), 292 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDto.java create mode 100644 scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDtoFactory.java delete mode 100644 scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeToUserDtoMapper.java create mode 100644 scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeDtoFactoryTest.java delete mode 100644 scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeToUserDtoMapperTest.java diff --git a/scm-ui-components/packages/ui-types/src/Me.js b/scm-ui-components/packages/ui-types/src/Me.js index 12516ade1b..f6cb9c2036 100644 --- a/scm-ui-components/packages/ui-types/src/Me.js +++ b/scm-ui-components/packages/ui-types/src/Me.js @@ -6,5 +6,6 @@ export type Me = { name: string, displayName: string, mail: string, + groups: [], _links: Links }; diff --git a/scm-ui/public/locales/en/commons.json b/scm-ui/public/locales/en/commons.json index 3196f3a328..e3e1dbf032 100644 --- a/scm-ui/public/locales/en/commons.json +++ b/scm-ui/public/locales/en/commons.json @@ -48,6 +48,7 @@ "username": "Username", "displayName": "Display Name", "mail": "E-Mail", + "groups": "Groups", "information": "Information", "change-password": "Change password", "error-title": "Error", diff --git a/scm-ui/src/containers/ProfileInfo.js b/scm-ui/src/containers/ProfileInfo.js index 9c4a5a9323..4c333174d1 100644 --- a/scm-ui/src/containers/ProfileInfo.js +++ b/scm-ui/src/containers/ProfileInfo.js @@ -1,53 +1,63 @@ -// @flow -import React from "react"; -import type { Me } from "@scm-manager/ui-types"; -import { MailLink, AvatarWrapper, AvatarImage } from "@scm-manager/ui-components"; -import { compose } from "redux"; -import { translate } from "react-i18next"; - -type Props = { - me: Me, - - // Context props - t: string => string -}; -type State = {}; - -class ProfileInfo extends React.Component { - render() { - const { me, t } = this.props; - return ( -
- -
-

- -

-
- -
- - - - - - - - - - - - - - - -
{t("profile.username")}{me.name}
{t("profile.displayName")}{me.displayName}
{t("profile.mail")} - -
-
-
- ); - } -} - -export default compose(translate("commons"))(ProfileInfo); +// @flow +import React from "react"; +import type { Me } from "@scm-manager/ui-types"; +import { MailLink, AvatarWrapper, AvatarImage } from "@scm-manager/ui-components"; +import { compose } from "redux"; +import { translate } from "react-i18next"; + +type Props = { + me: Me, + + // Context props + t: string => string +}; +type State = {}; + +class ProfileInfo extends React.Component { + render() { + const { me, t } = this.props; + return ( +
+ +
+

+ +

+
+ +
+ + + + + + + + + + + + + + + + + + + +
{t("profile.username")}{me.name}
{t("profile.displayName")}{me.displayName}
{t("profile.mail")} + +
{t("profile.groups")} +
    + {me.groups.map((group) => { + return
  • {group}
    • ; + })} +
+
+
+
+ ); + } +} + +export default compose(translate("commons"))(ProfileInfo); diff --git a/scm-ui/src/modules/auth.js b/scm-ui/src/modules/auth.js index e9bccb8fbc..5d15107406 100644 --- a/scm-ui/src/modules/auth.js +++ b/scm-ui/src/modules/auth.js @@ -134,15 +134,6 @@ const callFetchMe = (link: string): Promise => { .get(link) .then(response => { return response.json(); - }) - .then(json => { - const { name, displayName, mail, _links } = json; - return { - name, - displayName, - mail, - _links - }; }); }; diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MapperModule.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MapperModule.java index 66eadaad7d..669a10143a 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MapperModule.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MapperModule.java @@ -8,7 +8,6 @@ public class MapperModule extends AbstractModule { @Override protected void configure() { bind(UserDtoToUserMapper.class).to(Mappers.getMapper(UserDtoToUserMapper.class).getClass()); - bind(MeToUserDtoMapper.class).to(Mappers.getMapper(MeToUserDtoMapper.class).getClass()); bind(UserToUserDtoMapper.class).to(Mappers.getMapper(UserToUserDtoMapper.class).getClass()); bind(UserCollectionToDtoMapper.class); @@ -46,6 +45,7 @@ public class MapperModule extends AbstractModule { bind(MergeResultToDtoMapper.class).to(Mappers.getMapper(MergeResultToDtoMapper.class).getClass()); // no mapstruct required + bind(MeDtoFactory.class); bind(UIPluginDtoMapper.class); bind(UIPluginDtoCollectionMapper.class); diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDto.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDto.java new file mode 100644 index 0000000000..5488faca28 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDto.java @@ -0,0 +1,26 @@ +package sonia.scm.api.v2.resources; + +import de.otto.edison.hal.HalRepresentation; +import de.otto.edison.hal.Links; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; + +import java.util.List; + +@Getter +@Setter +@NoArgsConstructor +public class MeDto extends HalRepresentation { + + private String name; + private String displayName; + private String mail; + private List groups; + + @Override + @SuppressWarnings("squid:S1185") // We want to have this method available in this package + protected HalRepresentation add(Links links) { + return super.add(links); + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDtoFactory.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDtoFactory.java new file mode 100644 index 0000000000..082db7fd94 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDtoFactory.java @@ -0,0 +1,81 @@ +package sonia.scm.api.v2.resources; + +import com.google.common.collect.ImmutableList; +import de.otto.edison.hal.Links; +import org.apache.shiro.SecurityUtils; +import org.apache.shiro.subject.PrincipalCollection; +import org.apache.shiro.subject.Subject; +import sonia.scm.group.GroupNames; +import sonia.scm.user.User; +import sonia.scm.user.UserManager; +import sonia.scm.user.UserPermissions; + +import javax.inject.Inject; +import java.util.Collections; + +import static de.otto.edison.hal.Link.link; +import static de.otto.edison.hal.Links.linkingTo; + +public class MeDtoFactory extends LinkAppenderMapper { + + private final ResourceLinks resourceLinks; + private final UserManager userManager; + + @Inject + public MeDtoFactory(ResourceLinks resourceLinks, UserManager userManager) { + this.resourceLinks = resourceLinks; + this.userManager = userManager; + } + + public MeDto create() { + PrincipalCollection principals = getPrincipalCollection(); + + MeDto dto = new MeDto(); + + User user = principals.oneByType(User.class); + + mapUserProperties(user, dto); + mapGroups(principals, dto); + + appendLinks(user, dto); + return dto; + } + + private void mapGroups(PrincipalCollection principals, MeDto dto) { + Iterable groups = principals.oneByType(GroupNames.class); + if (groups == null) { + groups = Collections.emptySet(); + } + dto.setGroups(ImmutableList.copyOf(groups)); + } + + private void mapUserProperties(User user, MeDto dto) { + dto.setName(user.getName()); + dto.setDisplayName(user.getDisplayName()); + dto.setMail(user.getMail()); + } + + private PrincipalCollection getPrincipalCollection() { + Subject subject = SecurityUtils.getSubject(); + return subject.getPrincipals(); + } + + + private void appendLinks(User user, MeDto target) { + Links.Builder linksBuilder = linkingTo().self(resourceLinks.me().self()); + if (UserPermissions.delete(user).isPermitted()) { + linksBuilder.single(link("delete", resourceLinks.me().delete(target.getName()))); + } + if (UserPermissions.modify(user).isPermitted()) { + linksBuilder.single(link("update", resourceLinks.me().update(target.getName()))); + } + if (userManager.isTypeDefault(user) && UserPermissions.changePassword(user).isPermitted()) { + linksBuilder.single(link("password", resourceLinks.me().passwordChange())); + } + + appendLinks(new EdisonLinkAppender(linksBuilder), new Me(), user); + + target.add(linksBuilder.build()); + } + +} diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeResource.java index 20fc35923c..2c2e208893 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeResource.java @@ -3,14 +3,11 @@ package sonia.scm.api.v2.resources; import com.webcohesion.enunciate.metadata.rs.ResponseCode; import com.webcohesion.enunciate.metadata.rs.StatusCodes; import com.webcohesion.enunciate.metadata.rs.TypeHint; -import org.apache.shiro.SecurityUtils; import org.apache.shiro.authc.credential.PasswordService; -import sonia.scm.user.User; import sonia.scm.user.UserManager; import sonia.scm.web.VndMediaType; import javax.inject.Inject; -import javax.inject.Named; import javax.validation.Valid; import javax.ws.rs.Consumes; import javax.ws.rs.GET; @@ -28,20 +25,18 @@ import javax.ws.rs.core.UriInfo; */ @Path(MeResource.ME_PATH_V2) public class MeResource { - public static final String ME_PATH_V2 = "v2/me/"; - private final MeToUserDtoMapper meToUserDtoMapper; + static final String ME_PATH_V2 = "v2/me/"; - private final IdResourceManagerAdapter adapter; - private final PasswordService passwordService; + private final MeDtoFactory meDtoFactory; private final UserManager userManager; + private final PasswordService passwordService; @Inject - public MeResource(MeToUserDtoMapper meToUserDtoMapper, UserManager manager, PasswordService passwordService) { - this.meToUserDtoMapper = meToUserDtoMapper; - this.adapter = new IdResourceManagerAdapter<>(manager, User.class); + public MeResource(MeDtoFactory meDtoFactory, UserManager userManager, PasswordService passwordService) { + this.meDtoFactory = meDtoFactory; + this.userManager = userManager; this.passwordService = passwordService; - this.userManager = manager; } /** @@ -49,17 +44,15 @@ public class MeResource { */ @GET @Path("") - @Produces(VndMediaType.USER) - @TypeHint(UserDto.class) + @Produces(VndMediaType.ME) + @TypeHint(MeDto.class) @StatusCodes({ @ResponseCode(code = 200, condition = "success"), @ResponseCode(code = 401, condition = "not authenticated / invalid credentials"), @ResponseCode(code = 500, condition = "internal server error") }) public Response get(@Context Request request, @Context UriInfo uriInfo) { - - String id = (String) SecurityUtils.getSubject().getPrincipals().getPrimaryPrincipal(); - return adapter.get(id, meToUserDtoMapper::map); + return Response.ok(meDtoFactory.create()).build(); } /** @@ -75,7 +68,10 @@ public class MeResource { @TypeHint(TypeHint.NO_CONTENT.class) @Consumes(VndMediaType.PASSWORD_CHANGE) public Response changePassword(@Valid PasswordChangeDto passwordChange) { - userManager.changePasswordForLoggedInUser(passwordService.encryptPassword(passwordChange.getOldPassword()), passwordService.encryptPassword(passwordChange.getNewPassword())); + userManager.changePasswordForLoggedInUser( + passwordService.encryptPassword(passwordChange.getOldPassword()), + passwordService.encryptPassword(passwordChange.getNewPassword()) + ); return Response.noContent().build(); } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeToUserDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeToUserDtoMapper.java deleted file mode 100644 index c6d98a826e..0000000000 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeToUserDtoMapper.java +++ /dev/null @@ -1,45 +0,0 @@ -package sonia.scm.api.v2.resources; - -import de.otto.edison.hal.Links; -import org.mapstruct.AfterMapping; -import org.mapstruct.Mapper; -import org.mapstruct.MappingTarget; -import sonia.scm.user.User; -import sonia.scm.user.UserManager; -import sonia.scm.user.UserPermissions; - -import javax.inject.Inject; - -import static de.otto.edison.hal.Link.link; -import static de.otto.edison.hal.Links.linkingTo; - -@Mapper -public abstract class MeToUserDtoMapper extends UserToUserDtoMapper { - - @Inject - private UserManager userManager; - - @Inject - private ResourceLinks resourceLinks; - - - @Override - @AfterMapping - protected void appendLinks(User user, @MappingTarget UserDto target) { - Links.Builder linksBuilder = linkingTo().self(resourceLinks.me().self()); - if (UserPermissions.delete(user).isPermitted()) { - linksBuilder.single(link("delete", resourceLinks.me().delete(target.getName()))); - } - if (UserPermissions.modify(user).isPermitted()) { - linksBuilder.single(link("update", resourceLinks.me().update(target.getName()))); - } - if (userManager.isTypeDefault(user)) { - linksBuilder.single(link("password", resourceLinks.me().passwordChange())); - } - - appendLinks(new EdisonLinkAppender(linksBuilder), new Me(), user); - - target.add(linksBuilder.build()); - } - -} diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeDtoFactoryTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeDtoFactoryTest.java new file mode 100644 index 0000000000..138387938b --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeDtoFactoryTest.java @@ -0,0 +1,186 @@ +package sonia.scm.api.v2.resources; + +import org.apache.shiro.subject.PrincipalCollection; +import org.apache.shiro.subject.Subject; +import org.apache.shiro.util.ThreadContext; +import org.assertj.core.util.Lists; +import org.junit.jupiter.api.AfterEach; +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 org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; +import sonia.scm.group.GroupNames; +import sonia.scm.user.User; +import sonia.scm.user.UserManager; +import sonia.scm.user.UserPermissions; +import sonia.scm.user.UserTestData; + +import java.net.URI; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) +class MeDtoFactoryTest { + + private final URI baseUri = URI.create("https://scm.hitchhiker.com/scm/"); + + @Mock + private UserManager userManager; + + @Mock + private Subject subject; + + private MeDtoFactory meDtoFactory; + + @BeforeEach + void setUpContext() { + ThreadContext.bind(subject); + ResourceLinks resourceLinks = ResourceLinksMock.createMock(baseUri); + meDtoFactory = new MeDtoFactory(resourceLinks, userManager); + } + + @AfterEach + void unbindSubject() { + ThreadContext.unbindSubject(); + } + + @Test + void shouldCreateMeDtoFromUser() { + prepareSubject(UserTestData.createTrillian()); + + MeDto dto = meDtoFactory.create(); + assertThat(dto.getName()).isEqualTo("trillian"); + assertThat(dto.getDisplayName()).isEqualTo("Tricia McMillan"); + assertThat(dto.getMail()).isEqualTo("tricia.mcmillan@hitchhiker.com"); + } + + @Test + void shouldCreateMeDtoWithEmptyGroups() { + prepareSubject(UserTestData.createTrillian()); + MeDto dto = meDtoFactory.create(); + assertThat(dto.getGroups()).isEmpty(); + } + + @Test + void shouldCreateMeDtoWithGroups() { + prepareSubject(UserTestData.createTrillian(), "HeartOfGold", "Puzzle42"); + MeDto dto = meDtoFactory.create(); + assertThat(dto.getGroups()).containsOnly("HeartOfGold", "Puzzle42"); + } + + private void prepareSubject(User user, String... groups) { + PrincipalCollection collection = mock(PrincipalCollection.class); + when(subject.getPrincipals()).thenReturn(collection); + when(collection.oneByType(any(Class.class))).then(ic -> { + Class type = ic.getArgument(0); + if (type.isAssignableFrom(User.class)) { + return user; + } else if (type.isAssignableFrom(GroupNames.class)) { + return new GroupNames(Lists.newArrayList(groups)); + } else { + return null; + } + }); + } + + @Test + void shouldAppendSelfLink() { + prepareSubject(UserTestData.createTrillian()); + + MeDto dto = meDtoFactory.create(); + assertThat(dto.getLinks().getLinkBy("self").get().getHref()).isEqualTo("https://scm.hitchhiker.com/scm/v2/me/"); + } + + @Test + void shouldAppendDeleteLink() { + prepareSubject(UserTestData.createTrillian()); + when(subject.isPermitted("user:delete:trillian")).thenReturn(true); + + MeDto dto = meDtoFactory.create(); + assertThat(dto.getLinks().getLinkBy("delete").get().getHref()).isEqualTo("https://scm.hitchhiker.com/scm/v2/users/trillian"); + } + + @Test + void shouldNotAppendDeleteLink() { + prepareSubject(UserTestData.createTrillian()); + + MeDto dto = meDtoFactory.create(); + assertThat(dto.getLinks().getLinkBy("delete")).isNotPresent(); + } + + @Test + void shouldAppendUpdateLink() { + prepareSubject(UserTestData.createTrillian()); + when(subject.isPermitted("user:modify:trillian")).thenReturn(true); + + MeDto dto = meDtoFactory.create(); + assertThat(dto.getLinks().getLinkBy("update").get().getHref()).isEqualTo("https://scm.hitchhiker.com/scm/v2/users/trillian"); + } + + @Test + void shouldNotAppendUpdateLink() { + prepareSubject(UserTestData.createTrillian()); + + MeDto dto = meDtoFactory.create(); + assertThat(dto.getLinks().getLinkBy("update")).isNotPresent(); + } + + @Test + void shouldGetPasswordLinkOnlyForDefaultUserType() { + User user = UserTestData.createTrillian(); + prepareSubject(user); + + when(subject.isPermitted("user:changePassword:trillian")).thenReturn(true); + when(userManager.isTypeDefault(user)).thenReturn(true); + + MeDto dto = meDtoFactory.create(); + assertThat(dto.getLinks().getLinkBy("password").get().getHref()).isEqualTo("https://scm.hitchhiker.com/scm/v2/me/password"); + } + + @Test + void shouldNotGetPasswordLinkWithoutPermision() { + User user = UserTestData.createTrillian(); + prepareSubject(user); + + when(userManager.isTypeDefault(user)).thenReturn(true); + + MeDto dto = meDtoFactory.create(); + assertThat(dto.getLinks().getLinkBy("password")).isNotPresent(); + } + + @Test + void shouldNotGetPasswordLinkForNonDefaultUsers() { + User user = UserTestData.createTrillian(); + prepareSubject(user); + + when(subject.isPermitted("user:changePassword:trillian")).thenReturn(true); + + MeDto dto = meDtoFactory.create(); + assertThat(dto.getLinks().getLinkBy("password")).isNotPresent(); + } + + @Test + void shouldAppendLinks() { + prepareSubject(UserTestData.createTrillian()); + + LinkEnricherRegistry registry = new LinkEnricherRegistry(); + meDtoFactory.setRegistry(registry); + + registry.register(Me.class, (ctx, appender) -> { + User user = ctx.oneRequireByType(User.class); + appender.appendOne("profile", "http://hitchhiker.com/users/" + user.getName()); + }); + + MeDto dto = meDtoFactory.create(); + assertThat(dto.getLinks().getLinkBy("profile").get().getHref()).isEqualTo("http://hitchhiker.com/users/trillian"); + } + + +} diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeResourceTest.java index 454e24aa92..052a059959 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeResourceTest.java @@ -2,12 +2,14 @@ package sonia.scm.api.v2.resources; import com.github.sdorra.shiro.ShiroRule; import com.github.sdorra.shiro.SubjectAware; +import org.apache.shiro.SecurityUtils; import org.apache.shiro.authc.credential.PasswordService; +import org.apache.shiro.subject.PrincipalCollection; +import org.apache.shiro.subject.Subject; import org.jboss.resteasy.core.Dispatcher; import org.jboss.resteasy.mock.MockHttpRequest; import org.jboss.resteasy.mock.MockHttpResponse; import org.junit.Before; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.mockito.ArgumentCaptor; @@ -19,7 +21,6 @@ import sonia.scm.user.User; import sonia.scm.user.UserManager; import sonia.scm.web.VndMediaType; -import javax.lang.model.util.Types; import javax.servlet.http.HttpServletResponse; import java.net.URI; import java.net.URISyntaxException; @@ -27,11 +28,7 @@ import java.net.URISyntaxException; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.doNothing; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; import static org.mockito.MockitoAnnotations.initMocks; import static sonia.scm.api.v2.resources.DispatcherMock.createDispatcher; @@ -57,7 +54,7 @@ public class MeResourceTest { private UserManager userManager; @InjectMocks - private MeToUserDtoMapperImpl userToDtoMapper; + private MeDtoFactory meDtoFactory; private ArgumentCaptor userCaptor = ArgumentCaptor.forClass(User.class); @@ -66,7 +63,7 @@ public class MeResourceTest { private User originalUser; @Before - public void prepareEnvironment() throws Exception { + public void prepareEnvironment() { initMocks(this); originalUser = createDummyUser("trillian"); when(userManager.create(userCaptor.capture())).thenAnswer(invocation -> invocation.getArguments()[0]); @@ -74,17 +71,18 @@ public class MeResourceTest { doNothing().when(userManager).delete(userCaptor.capture()); when(userManager.isTypeDefault(userCaptor.capture())).thenCallRealMethod(); when(userManager.getDefaultType()).thenReturn("xml"); - MeResource meResource = new MeResource(userToDtoMapper, userManager, passwordService); + MeResource meResource = new MeResource(meDtoFactory, userManager, passwordService); when(uriInfo.getApiRestUri()).thenReturn(URI.create("/")); when(scmPathInfoStore.get()).thenReturn(uriInfo); dispatcher = createDispatcher(meResource); } @Test - @SubjectAware(username = "trillian", password = "secret") public void shouldReturnCurrentlyAuthenticatedUser() throws URISyntaxException { + applyUserToSubject(originalUser); + MockHttpRequest request = MockHttpRequest.get("/" + MeResource.ME_PATH_V2); - request.accept(VndMediaType.USER); + request.accept(VndMediaType.ME); MockHttpResponse response = new MockHttpResponse(); dispatcher.invoke(request, response); @@ -95,8 +93,17 @@ public class MeResourceTest { assertTrue(response.getContentAsString().contains("\"delete\":{\"href\":\"/v2/users/trillian\"}")); } + private void applyUserToSubject(User user) { + // use spy here to keep applied permissions from ShiroRule + Subject subject = spy(SecurityUtils.getSubject()); + PrincipalCollection collection = mock(PrincipalCollection.class); + when(collection.getPrimaryPrincipal()).thenReturn(user.getName()); + when(subject.getPrincipals()).thenReturn(collection); + when(collection.oneByType(User.class)).thenReturn(user); + shiro.setSubject(subject); + } + @Test - @SubjectAware(username = "trillian", password = "secret") public void shouldEncryptPasswordBeforeChanging() throws Exception { String newPassword = "pwd123"; String encryptedNewPassword = "encrypted123"; @@ -124,7 +131,6 @@ public class MeResourceTest { } @Test - @SubjectAware(username = "trillian", password = "secret") public void shouldGet400OnMissingOldPassword() throws Exception { originalUser.setType("not an xml type"); String newPassword = "pwd123"; @@ -141,7 +147,6 @@ public class MeResourceTest { } @Test - @SubjectAware(username = "trillian", password = "secret") public void shouldGet400OnMissingEmptyPassword() throws Exception { String newPassword = "pwd123"; String oldPassword = ""; @@ -158,7 +163,6 @@ public class MeResourceTest { } @Test - @SubjectAware(username = "trillian", password = "secret") public void shouldMapExceptionFromManager() throws Exception { String newPassword = "pwd123"; String oldPassword = "secret"; diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeToUserDtoMapperTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeToUserDtoMapperTest.java deleted file mode 100644 index 5aa940304e..0000000000 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeToUserDtoMapperTest.java +++ /dev/null @@ -1,151 +0,0 @@ -package sonia.scm.api.v2.resources; - -import org.apache.shiro.subject.Subject; -import org.apache.shiro.subject.support.SubjectThreadState; -import org.apache.shiro.util.ThreadContext; -import org.apache.shiro.util.ThreadState; -import org.junit.After; -import org.junit.Before; -import org.junit.Test; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import sonia.scm.user.User; -import sonia.scm.user.UserManager; -import sonia.scm.user.UserTestData; - -import java.net.URI; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; -import static org.mockito.MockitoAnnotations.initMocks; - -public class MeToUserDtoMapperTest { - - private final URI baseUri = URI.create("http://example.com/base/"); - @SuppressWarnings("unused") // Is injected - private final ResourceLinks resourceLinks = ResourceLinksMock.createMock(baseUri); - - @Mock - private UserManager userManager; - - @InjectMocks - private MeToUserDtoMapperImpl mapper; - - private final Subject subject = mock(Subject.class); - private final ThreadState subjectThreadState = new SubjectThreadState(subject); - - private URI expectedBaseUri; - private URI expectedUserBaseUri; - - @Before - public void init() { - initMocks(this); - when(userManager.getDefaultType()).thenReturn("xml"); - expectedBaseUri = baseUri.resolve(MeResource.ME_PATH_V2 + "/"); - expectedUserBaseUri = baseUri.resolve(UserRootResource.USERS_PATH_V2 + "/"); - subjectThreadState.bind(); - ThreadContext.bind(subject); - } - - @After - public void unbindSubject() { - ThreadContext.unbindSubject(); - } - - @Test - public void shouldMapTheUpdateLink() { - User user = createDefaultUser(); - when(subject.isPermitted("user:modify:abc")).thenReturn(true); - - UserDto userDto = mapper.map(user); - assertEquals("expected update link", expectedUserBaseUri.resolve("abc").toString(), userDto.getLinks().getLinkBy("update").get().getHref()); - - when(subject.isPermitted("user:modify:abc")).thenReturn(false); - userDto = mapper.map(user); - assertFalse("expected no update link", userDto.getLinks().getLinkBy("update").isPresent()); - } - - @Test - public void shouldMapTheSelfLink() { - User user = createDefaultUser(); - when(subject.isPermitted("user:modify:abc")).thenReturn(true); - - UserDto userDto = mapper.map(user); - assertEquals("expected self link", expectedBaseUri.toString(), userDto.getLinks().getLinkBy("self").get().getHref()); - - } - - @Test - public void shouldMapTheDeleteLink() { - User user = createDefaultUser(); - when(subject.isPermitted("user:delete:abc")).thenReturn(true); - - UserDto userDto = mapper.map(user); - assertEquals("expected update link", expectedUserBaseUri.resolve("abc").toString(), userDto.getLinks().getLinkBy("delete").get().getHref()); - - when(subject.isPermitted("user:delete:abc")).thenReturn(false); - userDto = mapper.map(user); - assertFalse("expected no delete link", userDto.getLinks().getLinkBy("delete").isPresent()); - } - - @Test - public void shouldGetPasswordLinkOnlyForDefaultUserType() { - User user = createDefaultUser(); - when(subject.isPermitted("user:modify:abc")).thenReturn(true); - when(userManager.isTypeDefault(eq(user))).thenReturn(true); - - UserDto userDto = mapper.map(user); - - assertEquals("expected password link with modify permission", expectedBaseUri.resolve("password").toString(), userDto.getLinks().getLinkBy("password").get().getHref()); - - when(subject.isPermitted("user:modify:abc")).thenReturn(false); - userDto = mapper.map(user); - assertEquals("expected password link on mission modify permission", expectedBaseUri.resolve("password").toString(), userDto.getLinks().getLinkBy("password").get().getHref()); - - when(userManager.isTypeDefault(eq(user))).thenReturn(false); - - userDto = mapper.map(user); - - assertFalse("expected no password link", userDto.getLinks().getLinkBy("password").isPresent()); - } - - - @Test - public void shouldGetEmptyPasswordProperty() { - User user = createDefaultUser(); - user.setPassword("myHighSecurePassword"); - when(subject.isPermitted("user:modify:abc")).thenReturn(true); - - UserDto userDto = mapper.map(user); - - assertThat(userDto.getPassword()).as("hide password for the me resource").isBlank(); - } - - @Test - public void shouldAppendLinks() { - LinkEnricherRegistry registry = new LinkEnricherRegistry(); - registry.register(Me.class, (ctx, appender) -> { - User user = ctx.oneRequireByType(User.class); - appender.appendOne("profile", "http://hitchhiker.com/users/" + user.getName()); - }); - mapper.setRegistry(registry); - - User trillian = UserTestData.createTrillian(); - UserDto dto = mapper.map(trillian); - - assertEquals("http://hitchhiker.com/users/trillian", dto.getLinks().getLinkBy("profile").get().getHref()); - } - - private User createDefaultUser() { - User user = new User(); - user.setName("abc"); - user.setCreationDate(1L); - return user; - } - - -} From 36ea444e6967615ef8a58486dea1d3cc7c10ddc0 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Fri, 18 Jan 2019 08:35:34 +0100 Subject: [PATCH 3/9] fix assignment of administrator privileges by configuration --- .../DefaultAuthorizationCollector.java | 45 ++++++++++++++++--- .../DefaultAuthorizationCollectorTest.java | 44 +++++++++++++++--- 2 files changed, 77 insertions(+), 12 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java b/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java index 8a79293642..eed03c2cd4 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java +++ b/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java @@ -51,6 +51,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.cache.Cache; import sonia.scm.cache.CacheManager; +import sonia.scm.config.ScmConfiguration; import sonia.scm.group.GroupNames; import sonia.scm.group.GroupPermissions; import sonia.scm.plugin.Extension; @@ -76,9 +77,6 @@ import java.util.Set; public class DefaultAuthorizationCollector implements AuthorizationCollector { - // TODO move to util class - private static final String SEPARATOR = System.getProperty("line.separator", "\n"); - /** Field description */ private static final String ADMIN_PERMISSION = "*"; @@ -98,14 +96,16 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector * * * + * @param configuration * @param cacheManager * @param repositoryDAO * @param securitySystem */ @Inject - public DefaultAuthorizationCollector(CacheManager cacheManager, - RepositoryDAO repositoryDAO, SecuritySystem securitySystem) + public DefaultAuthorizationCollector(ScmConfiguration configuration, CacheManager cacheManager, + RepositoryDAO repositoryDAO, SecuritySystem securitySystem) { + this.configuration = configuration; this.cache = cacheManager.getCache(CACHE_NAME); this.repositoryDAO = repositoryDAO; this.securitySystem = securitySystem; @@ -239,7 +239,7 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector Set roles; Set permissions; - if (user.isAdmin()) + if (isAdmin(user, groups)) { if (logger.isDebugEnabled()) { @@ -270,6 +270,37 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector return info; } + private boolean isAdmin(User user, GroupNames groups) { + boolean admin = user.isAdmin(); + if (admin) { + logger.debug("user {} is marked as admin, because of the user flag", user.getName()); + return true; + } + if (isUserAdminInConfiguration(user)) { + logger.debug("user {} is marked as admin, because of the admin user configuration", user.getName()); + return true; + } + return isUserAdminInGroupConfiguration(user, groups); + } + + private boolean isUserAdminInGroupConfiguration(User user, GroupNames groups) { + Set adminGroups = configuration.getAdminGroups(); + if (adminGroups != null && groups != null) { + for (String group : groups) { + if (adminGroups.contains(group)) { + logger.debug("user {} is marked as admin, because of the admin group configuration for group {}", user.getName(), group); + return true; + } + } + } + return false; + } + + private boolean isUserAdminInConfiguration(User user) { + Set adminUsers = configuration.getAdminUsers(); + return adminUsers != null && adminUsers.contains(user.getName()); + } + private String getGroupAutocompletePermission() { return GroupPermissions.autocomplete().asShiroString(); } @@ -373,6 +404,8 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector //~--- fields --------------------------------------------------------------- + private final ScmConfiguration configuration; + /** authorization cache */ private final Cache cache; diff --git a/scm-webapp/src/test/java/sonia/scm/security/DefaultAuthorizationCollectorTest.java b/scm-webapp/src/test/java/sonia/scm/security/DefaultAuthorizationCollectorTest.java index 2f455469dd..cc39965299 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/DefaultAuthorizationCollectorTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/DefaultAuthorizationCollectorTest.java @@ -34,6 +34,7 @@ package sonia.scm.security; import com.github.sdorra.shiro.ShiroRule; import com.github.sdorra.shiro.SubjectAware; import com.google.common.base.Predicate; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; import org.apache.shiro.authz.AuthorizationInfo; import org.apache.shiro.authz.SimpleAuthorizationInfo; @@ -49,6 +50,7 @@ import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; import sonia.scm.cache.Cache; import sonia.scm.cache.CacheManager; +import sonia.scm.config.ScmConfiguration; import sonia.scm.group.GroupNames; import sonia.scm.repository.PermissionType; import sonia.scm.repository.Repository; @@ -76,6 +78,8 @@ import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) public class DefaultAuthorizationCollectorTest { + private ScmConfiguration configuration; + @Mock private Cache cache; @@ -99,8 +103,38 @@ public class DefaultAuthorizationCollectorTest { @Before public void setUp(){ when(cacheManager.getCache(Mockito.any(String.class))).thenReturn(cache); + configuration = new ScmConfiguration(); + collector = new DefaultAuthorizationCollector(configuration, cacheManager, repositoryDAO, securitySystem); + } - collector = new DefaultAuthorizationCollector(cacheManager, repositoryDAO, securitySystem); + @Test + @SubjectAware( + configuration = "classpath:sonia/scm/shiro-001.ini" + ) + public void shouldGetAdminPrivilegedByConfiguration() { + configuration.setAdminUsers(ImmutableSet.of("trillian")); + authenticate(UserTestData.createTrillian(), "main"); + + AuthorizationInfo authInfo = collector.collect(); + assertIsAdmin(authInfo); + } + + private void assertIsAdmin(AuthorizationInfo authInfo) { + assertThat(authInfo.getRoles(), Matchers.containsInAnyOrder(Role.USER, Role.ADMIN)); + assertThat(authInfo.getObjectPermissions(), nullValue()); + assertThat(authInfo.getStringPermissions(), Matchers.contains("*")); + } + + @Test + @SubjectAware( + configuration = "classpath:sonia/scm/shiro-001.ini" + ) + public void shouldGetAdminPrivilegedByGroupConfiguration() { + configuration.setAdminGroups(ImmutableSet.of("heartOfGold")); + authenticate(UserTestData.createTrillian(), "heartOfGold"); + + AuthorizationInfo authInfo = collector.collect(); + assertIsAdmin(authInfo); } /** @@ -142,7 +176,7 @@ public class DefaultAuthorizationCollectorTest { public void testCollectWithCache() { authenticate(UserTestData.createTrillian(), "main"); - AuthorizationInfo authInfo = collector.collect(); + collector.collect(); verify(cache).put(any(), any()); } @@ -176,9 +210,7 @@ public class DefaultAuthorizationCollectorTest { authenticate(trillian, "main"); AuthorizationInfo authInfo = collector.collect(); - assertThat(authInfo.getRoles(), Matchers.containsInAnyOrder(Role.USER, Role.ADMIN)); - assertThat(authInfo.getObjectPermissions(), nullValue()); - assertThat(authInfo.getStringPermissions(), Matchers.contains("*")); + assertIsAdmin(authInfo); } /** @@ -238,7 +270,7 @@ public class DefaultAuthorizationCollectorTest { } /** - * Tests {@link AuthorizationCollector#invalidateCache(sonia.scm.security.AuthorizationChangedEvent)}. + * Tests {@link DefaultAuthorizationCollector#invalidateCache(sonia.scm.security.AuthorizationChangedEvent)}. */ @Test public void testInvalidateCache() { From 736f883c8671a98cb5eee8e3d5c20ecba0416a34 Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Fri, 18 Jan 2019 09:12:34 +0000 Subject: [PATCH 4/9] Close branch bugfix/administrator_permission_from_configuration From c26cc0fe5606621bb8ebbc22e82316fc4976503e Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Fri, 18 Jan 2019 11:15:38 +0100 Subject: [PATCH 5/9] fixed integration tests --- .../src/test/java/sonia/scm/it/MeITCase.java | 13 ---------- .../java/sonia/scm/it/utils/ScmRequests.java | 25 +++++++++++-------- 2 files changed, 15 insertions(+), 23 deletions(-) diff --git a/scm-it/src/test/java/sonia/scm/it/MeITCase.java b/scm-it/src/test/java/sonia/scm/it/MeITCase.java index ce6593ef11..89c6eeb7b8 100644 --- a/scm-it/src/test/java/sonia/scm/it/MeITCase.java +++ b/scm-it/src/test/java/sonia/scm/it/MeITCase.java @@ -1,13 +1,10 @@ package sonia.scm.it; -import org.junit.Assert; import org.junit.Before; import org.junit.Test; import sonia.scm.it.utils.ScmRequests; import sonia.scm.it.utils.TestData; -import static org.assertj.core.api.Assertions.assertThat; - public class MeITCase { @Before @@ -23,9 +20,6 @@ public class MeITCase { .requestIndexResource(TestData.USER_SCM_ADMIN, TestData.USER_SCM_ADMIN) .requestMe() .assertStatusCode(200) - .assertAdmin(aBoolean -> assertThat(aBoolean).isEqualTo(Boolean.TRUE)) - .assertPassword(Assert::assertNull) - .assertType(s -> assertThat(s).isEqualTo("xml")) .requestChangePassword(TestData.USER_SCM_ADMIN, newPassword) .assertStatusCode(204); // assert password is changed -> login with the new Password than undo changes @@ -33,7 +27,6 @@ public class MeITCase { .requestIndexResource(TestData.USER_SCM_ADMIN, newPassword) .requestMe() .assertStatusCode(200) - .assertAdmin(aBoolean -> assertThat(aBoolean).isEqualTo(Boolean.TRUE))// still admin .requestChangePassword(newPassword, TestData.USER_SCM_ADMIN) .assertStatusCode(204); } @@ -49,9 +42,6 @@ public class MeITCase { .requestIndexResource(username, password) .requestMe() .assertStatusCode(200) - .assertAdmin(aBoolean -> assertThat(aBoolean).isEqualTo(Boolean.FALSE)) - .assertPassword(Assert::assertNull) - .assertType(s -> assertThat(s).isEqualTo("xml")) .requestChangePassword(password, newPassword) .assertStatusCode(204); // assert password is changed -> login with the new Password than undo changes @@ -72,9 +62,6 @@ public class MeITCase { .requestIndexResource(newUser, password) .requestMe() .assertStatusCode(200) - .assertAdmin(aBoolean -> assertThat(aBoolean).isEqualTo(Boolean.TRUE)) - .assertPassword(Assert::assertNull) - .assertType(s -> assertThat(s).isEqualTo(type)) .assertPasswordLinkDoesNotExists(); } } diff --git a/scm-it/src/test/java/sonia/scm/it/utils/ScmRequests.java b/scm-it/src/test/java/sonia/scm/it/utils/ScmRequests.java index bde3892773..9386f1d9c5 100644 --- a/scm-it/src/test/java/sonia/scm/it/utils/ScmRequests.java +++ b/scm-it/src/test/java/sonia/scm/it/utils/ScmRequests.java @@ -48,7 +48,7 @@ public class ScmRequests { return new IndexResponse(applyGETRequest(RestUtil.REST_BASE_URL.toString())); } - public , T extends ModelResponse> UserResponse requestUser(String username, String password, String pathParam) { + public UserResponse requestUser(String username, String password, String pathParam) { setUsername(username); setPassword(password); return new UserResponse<>(applyGETRequest(RestUtil.REST_BASE_URL.resolve("users/"+pathParam).toString()), null); @@ -195,7 +195,7 @@ public class ScmRequests { return new MeResponse<>(applyGETRequestFromLink(response, LINK_ME), this); } - public UserResponse requestUser(String username) { + public UserResponse requestUser(String username) { return new UserResponse<>(applyGETRequestFromLinkWithParams(response, LINK_USERS, username), this); } @@ -307,19 +307,24 @@ public class ScmRequests { } - public class MeResponse extends UserResponse, PREV> { + public class MeResponse extends ModelResponse, PREV> { + public static final String LINKS_PASSWORD_HREF = "_links.password.href"; public MeResponse(Response response, PREV previousResponse) { super(response, previousResponse); } - public ChangePasswordResponse requestChangePassword(String oldPassword, String newPassword) { + public MeResponse assertPasswordLinkDoesNotExists() { + return assertPropertyPathDoesNotExists(LINKS_PASSWORD_HREF); + } + + public ChangePasswordResponse requestChangePassword(String oldPassword, String newPassword) { return new ChangePasswordResponse<>(applyPUTRequestFromLink(super.response, LINKS_PASSWORD_HREF, VndMediaType.PASSWORD_CHANGE, createPasswordChangeJson(oldPassword, newPassword)), this); } } - public class UserResponse, PREV extends ModelResponse> extends ModelResponse { + public class UserResponse extends ModelResponse, PREV> { public static final String LINKS_PASSWORD_HREF = "_links.password.href"; @@ -327,23 +332,23 @@ public class ScmRequests { super(response, previousResponse); } - public SELF assertPassword(Consumer assertPassword) { + public UserResponse assertPassword(Consumer assertPassword) { return super.assertSingleProperty(assertPassword, "password"); } - public SELF assertType(Consumer assertType) { + public UserResponse assertType(Consumer assertType) { return assertSingleProperty(assertType, "type"); } - public SELF assertAdmin(Consumer assertAdmin) { + public UserResponse assertAdmin(Consumer assertAdmin) { return assertSingleProperty(assertAdmin, "admin"); } - public SELF assertPasswordLinkDoesNotExists() { + public UserResponse assertPasswordLinkDoesNotExists() { return assertPropertyPathDoesNotExists(LINKS_PASSWORD_HREF); } - public SELF assertPasswordLinkExists() { + public UserResponse assertPasswordLinkExists() { return assertPropertyPathExists(LINKS_PASSWORD_HREF); } From 7e8ba2d835e8015f138f05c3687d51af2f4f0b9a Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Fri, 18 Jan 2019 12:02:19 +0000 Subject: [PATCH 6/9] Close branch feature/assigned_groups_on_profile From 067da58e9655af5d209ed5d5d6d2841a7f3cdfd4 Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Mon, 21 Jan 2019 14:11:36 +0100 Subject: [PATCH 7/9] add group extension point --- scm-ui/src/groups/containers/SingleGroup.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/scm-ui/src/groups/containers/SingleGroup.js b/scm-ui/src/groups/containers/SingleGroup.js index 1dd4aa569f..fb38636a3b 100644 --- a/scm-ui/src/groups/containers/SingleGroup.js +++ b/scm-ui/src/groups/containers/SingleGroup.js @@ -27,6 +27,7 @@ import { import { translate } from "react-i18next"; import EditGroup from "./EditGroup"; import { getGroupsLink } from "../../modules/indexResource"; +import {ExtensionPoint} from "@scm-manager/ui-extensions"; type Props = { name: string, @@ -88,6 +89,11 @@ class SingleGroup extends React.Component { const url = this.matchedUrl(); + const extensionProps = { + group, + url + }; + return (
@@ -102,6 +108,11 @@ class SingleGroup extends React.Component { exact component={() => } /> +
@@ -118,6 +129,11 @@ class SingleGroup extends React.Component { /> +
From 3bd1cbf53d0c769c734f9bc603dbfae331f7fe39 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Mon, 21 Jan 2019 14:27:14 +0100 Subject: [PATCH 8/9] added option to define extra groups for AccessToken --- .../java/sonia/scm/security/AccessToken.java | 8 + .../scm/security/AccessTokenBuilder.java | 17 +- .../sonia/scm/security/DAORealmHelper.java | 109 +++++++++--- .../scm/security/DAORealmHelperTest.java | 155 ++++++++++++++++++ .../java/sonia/scm/security/BearerRealm.java | 10 +- .../sonia/scm/security/JwtAccessToken.java | 15 ++ .../scm/security/JwtAccessTokenBuilder.java | 14 ++ .../sonia/scm/security/BearerRealmTest.java | 45 ++--- .../security/JwtAccessTokenBuilderTest.java | 5 +- 9 files changed, 319 insertions(+), 59 deletions(-) create mode 100644 scm-core/src/test/java/sonia/scm/security/DAORealmHelperTest.java 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")); } } From d077ceaf3844c06dd19145dc3e33731248e59a54 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Mon, 21 Jan 2019 14:49:37 +0100 Subject: [PATCH 9/9] close branch feature/jwt_groups