anonymous user should not have permission to change password or autocomplete

This commit is contained in:
Eduard Heimbuch
2019-10-17 11:08:55 +02:00
parent a33acf5326
commit 1fd6337f64
6 changed files with 48 additions and 6 deletions

View File

@@ -6,6 +6,7 @@ import org.apache.shiro.SecurityUtils;
import org.apache.shiro.subject.PrincipalCollection; import org.apache.shiro.subject.PrincipalCollection;
import org.apache.shiro.subject.Subject; import org.apache.shiro.subject.Subject;
import sonia.scm.group.GroupCollector; import sonia.scm.group.GroupCollector;
import sonia.scm.security.Authentications;
import sonia.scm.user.User; import sonia.scm.user.User;
import sonia.scm.user.UserManager; import sonia.scm.user.UserManager;
import sonia.scm.user.UserPermissions; import sonia.scm.user.UserPermissions;
@@ -63,7 +64,7 @@ public class MeDtoFactory extends HalAppenderMapper {
if (UserPermissions.modify(user).isPermitted()) { if (UserPermissions.modify(user).isPermitted()) {
linksBuilder.single(link("update", resourceLinks.me().update(user.getName()))); 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())); linksBuilder.single(link("password", resourceLinks.me().passwordChange()));
} }

View File

@@ -254,9 +254,11 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector
collectGlobalPermissions(builder, user, groups); collectGlobalPermissions(builder, user, groups);
collectRepositoryPermissions(builder, user, groups); collectRepositoryPermissions(builder, user, groups);
builder.add(canReadOwnUser(user)); builder.add(canReadOwnUser(user));
if (!Authentications.isSubjectAnonymous(user.getName())) {
builder.add(getUserAutocompletePermission()); builder.add(getUserAutocompletePermission());
builder.add(getGroupAutocompletePermission()); builder.add(getGroupAutocompletePermission());
builder.add(getChangeOwnPasswordPermission(user)); builder.add(getChangeOwnPasswordPermission(user));
}
SimpleAuthorizationInfo info = new SimpleAuthorizationInfo(ImmutableSet.of(Role.USER)); SimpleAuthorizationInfo info = new SimpleAuthorizationInfo(ImmutableSet.of(Role.USER));
info.addStringPermissions(builder.build()); info.addStringPermissions(builder.build());

View File

@@ -48,6 +48,7 @@ import sonia.scm.SCMContextProvider;
import sonia.scm.TransformFilter; import sonia.scm.TransformFilter;
import sonia.scm.search.SearchRequest; import sonia.scm.search.SearchRequest;
import sonia.scm.search.SearchUtil; import sonia.scm.search.SearchUtil;
import sonia.scm.security.Authentications;
import sonia.scm.util.CollectionAppender; import sonia.scm.util.CollectionAppender;
import sonia.scm.util.Util; import sonia.scm.util.Util;
@@ -378,7 +379,7 @@ public class DefaultUserManager extends AbstractUserManager
public void changePasswordForLoggedInUser(String oldPassword, String newPassword) { public void changePasswordForLoggedInUser(String oldPassword, String newPassword) {
User user = get((String) SecurityUtils.getSubject().getPrincipals().getPrimaryPrincipal()); 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())); throw new InvalidPasswordException(ContextEntry.ContextBuilder.entity("PasswordChange", "-").in(User.class, user.getName()));
} }
@@ -397,13 +398,17 @@ public class DefaultUserManager extends AbstractUserManager
if (user == null) { if (user == null) {
throw new NotFoundException(User.class, userId); 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()); throw new ChangePasswordNotAllowedException(ContextEntry.ContextBuilder.entity("PasswordChange", "-").in(User.class, user.getName()), user.getType());
} }
user.setPassword(newPassword); user.setPassword(newPassword);
this.modify(user); this.modify(user);
} }
private boolean isAnonymousUser(User user) {
return Authentications.isSubjectAnonymous(user.getName());
}
//~--- fields --------------------------------------------------------------- //~--- fields ---------------------------------------------------------------
private final UserDAO userDAO; private final UserDAO userDAO;

View File

@@ -12,14 +12,17 @@ import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoExtension;
import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.junit.jupiter.MockitoSettings;
import org.mockito.quality.Strictness; import org.mockito.quality.Strictness;
import sonia.scm.SCMContext;
import sonia.scm.group.GroupCollector; import sonia.scm.group.GroupCollector;
import sonia.scm.user.User; import sonia.scm.user.User;
import sonia.scm.user.UserManager; import sonia.scm.user.UserManager;
import sonia.scm.user.UserPermissions;
import sonia.scm.user.UserTestData; import sonia.scm.user.UserTestData;
import java.net.URI; import java.net.URI;
import static org.assertj.core.api.Assertions.assertThat; 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.mock;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
@@ -159,6 +162,18 @@ class MeDtoFactoryTest {
assertThat(dto.getLinks().getLinkBy("password")).isNotPresent(); 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 @Test
void shouldAppendLinks() { void shouldAppendLinks() {
prepareSubject(UserTestData.createTrillian()); prepareSubject(UserTestData.createTrillian());

View File

@@ -48,6 +48,7 @@ import org.junit.runner.RunWith;
import org.mockito.Mock; import org.mockito.Mock;
import org.mockito.Mockito; import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner; import org.mockito.junit.MockitoJUnitRunner;
import sonia.scm.SCMContext;
import sonia.scm.cache.Cache; import sonia.scm.cache.Cache;
import sonia.scm.cache.CacheManager; import sonia.scm.cache.CacheManager;
import sonia.scm.group.GroupCollector; import sonia.scm.group.GroupCollector;
@@ -172,6 +173,23 @@ public class DefaultAuthorizationCollectorTest {
assertThat(authInfo.getObjectPermissions(), nullValue()); 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. * Tests {@link AuthorizationCollector#collect(PrincipalCollection)} ()} with repository permissions.
*/ */

View File

@@ -1,6 +1,7 @@
[users] [users]
trillian = secret, user trillian = secret, user
dent = secret, admin dent = secret, admin
_anonymous = secret, user
[roles] [roles]
admin = * admin = *