From 1fd6337f64036c68c19eb0f3120b73d018ece978 Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Thu, 17 Oct 2019 11:08:55 +0200 Subject: [PATCH] anonymous user should not have permission to change password or autocomplete --- .../scm/api/v2/resources/MeDtoFactory.java | 3 ++- .../DefaultAuthorizationCollector.java | 8 +++++--- .../sonia/scm/user/DefaultUserManager.java | 9 +++++++-- .../scm/api/v2/resources/MeDtoFactoryTest.java | 15 +++++++++++++++ .../DefaultAuthorizationCollectorTest.java | 18 ++++++++++++++++++ .../src/test/resources/sonia/scm/shiro-001.ini | 1 + 6 files changed, 48 insertions(+), 6 deletions(-) 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 index c2bebd389a..34bd035c6f 100644 --- 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 @@ -6,6 +6,7 @@ import org.apache.shiro.SecurityUtils; import org.apache.shiro.subject.PrincipalCollection; import org.apache.shiro.subject.Subject; import sonia.scm.group.GroupCollector; +import sonia.scm.security.Authentications; import sonia.scm.user.User; import sonia.scm.user.UserManager; import sonia.scm.user.UserPermissions; @@ -63,7 +64,7 @@ public class MeDtoFactory extends HalAppenderMapper { if (UserPermissions.modify(user).isPermitted()) { linksBuilder.single(link("update", resourceLinks.me().update(user.getName()))); } - if (userManager.isTypeDefault(user) && UserPermissions.changePassword(user).isPermitted()) { + if (userManager.isTypeDefault(user) && UserPermissions.changePassword(user).isPermitted() && !Authentications.isSubjectAnonymous(user.getName())) { linksBuilder.single(link("password", resourceLinks.me().passwordChange())); } 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 28f61df34f..57866ebe3c 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java +++ b/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java @@ -254,9 +254,11 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector collectGlobalPermissions(builder, user, groups); collectRepositoryPermissions(builder, user, groups); builder.add(canReadOwnUser(user)); - builder.add(getUserAutocompletePermission()); - builder.add(getGroupAutocompletePermission()); - builder.add(getChangeOwnPasswordPermission(user)); + if (!Authentications.isSubjectAnonymous(user.getName())) { + builder.add(getUserAutocompletePermission()); + builder.add(getGroupAutocompletePermission()); + builder.add(getChangeOwnPasswordPermission(user)); + } SimpleAuthorizationInfo info = new SimpleAuthorizationInfo(ImmutableSet.of(Role.USER)); info.addStringPermissions(builder.build()); diff --git a/scm-webapp/src/main/java/sonia/scm/user/DefaultUserManager.java b/scm-webapp/src/main/java/sonia/scm/user/DefaultUserManager.java index 212b067dbc..d297d51b45 100644 --- a/scm-webapp/src/main/java/sonia/scm/user/DefaultUserManager.java +++ b/scm-webapp/src/main/java/sonia/scm/user/DefaultUserManager.java @@ -48,6 +48,7 @@ import sonia.scm.SCMContextProvider; import sonia.scm.TransformFilter; import sonia.scm.search.SearchRequest; import sonia.scm.search.SearchUtil; +import sonia.scm.security.Authentications; import sonia.scm.util.CollectionAppender; import sonia.scm.util.Util; @@ -378,7 +379,7 @@ public class DefaultUserManager extends AbstractUserManager public void changePasswordForLoggedInUser(String oldPassword, String newPassword) { User user = get((String) SecurityUtils.getSubject().getPrincipals().getPrimaryPrincipal()); - if (!user.getPassword().equals(oldPassword)) { + if (!isAnonymousUser(user) && !user.getPassword().equals(oldPassword)) { throw new InvalidPasswordException(ContextEntry.ContextBuilder.entity("PasswordChange", "-").in(User.class, user.getName())); } @@ -397,13 +398,17 @@ public class DefaultUserManager extends AbstractUserManager if (user == null) { throw new NotFoundException(User.class, userId); } - if (!isTypeDefault(user)) { + if (!isTypeDefault(user) || isAnonymousUser(user)) { throw new ChangePasswordNotAllowedException(ContextEntry.ContextBuilder.entity("PasswordChange", "-").in(User.class, user.getName()), user.getType()); } user.setPassword(newPassword); this.modify(user); } + private boolean isAnonymousUser(User user) { + return Authentications.isSubjectAnonymous(user.getName()); + } + //~--- fields --------------------------------------------------------------- private final UserDAO userDAO; 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 index d9572dc04c..42d78b99f1 100644 --- 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 @@ -12,14 +12,17 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.quality.Strictness; +import sonia.scm.SCMContext; import sonia.scm.group.GroupCollector; 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; @@ -159,6 +162,18 @@ class MeDtoFactoryTest { assertThat(dto.getLinks().getLinkBy("password")).isNotPresent(); } + @Test + void shouldNotGetPasswordLinkForAnonymousUser() { + User user = SCMContext.ANONYMOUS; + prepareSubject(user); + + when(userManager.isTypeDefault(any())).thenReturn(true); + when(UserPermissions.changePassword(user).isPermitted()).thenReturn(true); + + MeDto dto = meDtoFactory.create(); + assertThat(dto.getLinks().getLinkBy("password")).isNotPresent(); + } + @Test void shouldAppendLinks() { prepareSubject(UserTestData.createTrillian()); 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 930a06d249..08216e9fd2 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/DefaultAuthorizationCollectorTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/DefaultAuthorizationCollectorTest.java @@ -48,6 +48,7 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import sonia.scm.SCMContext; import sonia.scm.cache.Cache; import sonia.scm.cache.CacheManager; import sonia.scm.group.GroupCollector; @@ -172,6 +173,23 @@ public class DefaultAuthorizationCollectorTest { assertThat(authInfo.getObjectPermissions(), nullValue()); } + /** + * Tests {@link AuthorizationCollector#collect(PrincipalCollection)} ()} without permissions. + */ + @Test + @SubjectAware( + configuration = "classpath:sonia/scm/shiro-001.ini" + ) + public void testCollectWithoutPermissionsForAnonymousUser() { + User anonymous = SCMContext.ANONYMOUS; + authenticate(anonymous, "anon"); + + AuthorizationInfo authInfo = collector.collect(); + assertThat(authInfo.getStringPermissions(), hasSize(1)); + assertThat(authInfo.getStringPermissions(), containsInAnyOrder("user:read:_anonymous")); + assertThat(authInfo.getObjectPermissions(), nullValue()); + } + /** * Tests {@link AuthorizationCollector#collect(PrincipalCollection)} ()} with repository permissions. */ diff --git a/scm-webapp/src/test/resources/sonia/scm/shiro-001.ini b/scm-webapp/src/test/resources/sonia/scm/shiro-001.ini index 54741bcf4d..df0fd4940c 100644 --- a/scm-webapp/src/test/resources/sonia/scm/shiro-001.ini +++ b/scm-webapp/src/test/resources/sonia/scm/shiro-001.ini @@ -1,6 +1,7 @@ [users] trillian = secret, user dent = secret, admin +_anonymous = secret, user [roles] admin = *