diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddUserProfilePropertyCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddUserProfilePropertyCommand.java index 2fb13d093ce..011356bb331 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddUserProfilePropertyCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddUserProfilePropertyCommand.java @@ -46,7 +46,8 @@ protected boolean validate() { userProfileDao.getProfile(newProp.getUserId()), newProp, getCurrentUser(), - this::validate); + this::validate, + isSystemSuperUser()); } public static String buildUserName(DbUserDao userDao, Guid userId) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUserProfilePropertiesByUserIdQuery.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUserProfilePropertiesByUserIdQuery.java index f4756a248fb..16242c86f2b 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUserProfilePropertiesByUserIdQuery.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUserProfilePropertiesByUserIdQuery.java @@ -20,7 +20,7 @@ public GetUserProfilePropertiesByUserIdQuery(P parameters, EngineContext engineC } private boolean validate() { - return validate(validator.authorized(getUser(), getParameters().getId())); + return validate(validator.authorized(getUser(), getParameters().getId(), getUser() != null && getUser().isAdmin())); } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUserProfilePropertyByNameAndUserIdQuery.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUserProfilePropertyByNameAndUserIdQuery.java index eb814ee3256..a80bc899b62 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUserProfilePropertyByNameAndUserIdQuery.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUserProfilePropertyByNameAndUserIdQuery.java @@ -18,7 +18,7 @@ public GetUserProfilePropertyByNameAndUserIdQuery(P parameters, EngineContext en } private boolean validate() { - return validate(validator.authorized(getUser(), getParameters().getId())); + return validate(validator.authorized(getUser(), getParameters().getId(), getUser() != null && getUser().isAdmin())); } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUserProfilePropertyQuery.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUserProfilePropertyQuery.java index 7c8322f2b2d..8247a17527e 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUserProfilePropertyQuery.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUserProfilePropertyQuery.java @@ -19,7 +19,7 @@ public GetUserProfilePropertyQuery(P parameters, EngineContext engineContext) { } private boolean validate(UserProfileProperty prop) { - return validate(validator.authorized(getUser(), prop.getUserId())); + return validate(validator.authorized(getUser(), prop.getUserId(), getUser() != null && getUser().isAdmin())); } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveUserProfilePropertyCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveUserProfilePropertyCommand.java index b5218463bb2..0d27eecaa2a 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveUserProfilePropertyCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveUserProfilePropertyCommand.java @@ -29,7 +29,6 @@ public class RemoveUserProfilePropertyCommand extends Co @Inject protected DbUserDao userDao; - private final UserProfileValidator validator = new UserProfileValidator(); public RemoveUserProfilePropertyCommand(T parameters, CommandContext commandContext) { @@ -64,8 +63,10 @@ protected boolean validate() { .map(UserProfileProperty::getName) .orElse(getParameters().getId().toString())); return validate(validator.propertyProvided(currentProp)) && - validate(validator.authorized(getCurrentUser(), - Optional.ofNullable(currentProp).map(UserProfileProperty::getUserId).orElse(null))); + validate(validator.authorized( + getCurrentUser(), + Optional.ofNullable(currentProp).map(UserProfileProperty::getUserId).orElse(null), + isSystemSuperUser())); } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateUserProfilePropertyCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateUserProfilePropertyCommand.java index e5824677483..861822bc65f 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateUserProfilePropertyCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateUserProfilePropertyCommand.java @@ -64,7 +64,8 @@ protected boolean validate() { userProfileDao.get(propertyToUpdate.getPropertyId()), getCurrentUser(), this::validate, - propertyToUpdate + propertyToUpdate, + isSystemSuperUser() ); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/UserProfileValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/UserProfileValidator.java index 9f8e502d128..2eea35a4ce5 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/UserProfileValidator.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/UserProfileValidator.java @@ -40,12 +40,13 @@ public ValidationResult propertyProvided(UserProfileProperty property) { .when(property == null); } - public ValidationResult authorized(DbUser currentUser, Guid targetUserId) { + public ValidationResult authorized(DbUser currentUser, Guid targetUserId, boolean sufficientAdminRights) { return ValidationResult .failWith(EngineMessage.USER_NOT_AUTHORIZED_TO_PERFORM_ACTION) .when(currentUser == null || currentUser.getId() == null - || !Objects.equals(currentUser.getId(), targetUserId) && !currentUser.isAdmin()); + || targetUserId == null + || !Objects.equals(currentUser.getId(), targetUserId) && !sufficientAdminRights); } public ValidationResult validPublicSshKey(UserProfileProperty property) { @@ -88,8 +89,9 @@ public ValidationResult firstPublicSshKey(UserProfile profile, UserProfileProper public boolean validateAdd(UserProfile existingProfile, UserProfileProperty newProp, DbUser currentUser, - Function validate) { - return validate.apply(authorized(currentUser, newProp.getUserId())) && + Function validate, + boolean isSuperUser) { + return validate.apply(authorized(currentUser, newProp.getUserId(), isSuperUser)) && validate.apply(sameOwner(existingProfile.getUserId(), newProp.getUserId())) && validate.apply(validPublicSshKey(newProp)) && // /sshpublickeys endpoint is not aware of names so first check by SSH type @@ -101,10 +103,11 @@ public boolean validateAdd(UserProfile existingProfile, public boolean validateUpdate(UserProfileProperty currentProp, DbUser currentUser, Function validate, - UserProfileProperty update) { + UserProfileProperty update, + boolean isSuperUser) { return validate.apply(propertyProvided(currentProp)) && validate.apply(sameOwner(currentProp.getUserId(), update.getUserId())) && - validate.apply(authorized(currentUser, update.getUserId())) && + validate.apply(authorized(currentUser, update.getUserId(), isSuperUser)) && validate.apply(validPublicSshKey(update)) && validate.apply(sameName(currentProp.getName(), update.getName(), currentProp.getPropertyId())) && validate.apply(sameType(currentProp.getType(), update.getType(), currentProp.getPropertyId())); diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddUserProfilePropertyCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddUserProfilePropertyCommandTest.java index 4f5bb215c81..227c984eb07 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddUserProfilePropertyCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddUserProfilePropertyCommandTest.java @@ -12,6 +12,8 @@ import static org.ovirt.engine.core.bll.UserProfileTestHelper.checkAssertsForSshProp; import static org.ovirt.engine.core.bll.UserProfileTestHelper.createAdmin; +import java.util.function.Predicate; + import org.junit.jupiter.api.Test; import org.mockito.InjectMocks; import org.mockito.Mock; @@ -31,6 +33,9 @@ class AddUserProfilePropertyCommandTest extends BaseCommandTest { @Mock protected DbUserDao userDaoMock; + @Mock + private Predicate isSystemSuperUserPredicate; + private UserProfilePropertyParameters parameters = mock(UserProfilePropertyParameters.class); @InjectMocks @@ -54,6 +59,7 @@ void addSinglSshPropToEmptyProfileForAnotherUser() { when(userDaoMock.get(any())).thenReturn(mock(DbUser.class)); when(parameters.getUserProfileProperty()).thenReturn(inputProp); + when(isSystemSuperUserPredicate.test(any())).thenReturn(true); addCommand.setCurrentUser(createAdmin(Guid.newGuid())); assertTrue(addCommand.validate(), buildValidationMessage(addCommand.getReturnValue())); diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveUserProfilePropertyCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveUserProfilePropertyCommandTest.java index 0054b581d02..7b014e0904b 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveUserProfilePropertyCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveUserProfilePropertyCommandTest.java @@ -11,12 +11,15 @@ import static org.ovirt.engine.core.bll.UserProfileTestHelper.buildValidationMessage; import static org.ovirt.engine.core.bll.UserProfileTestHelper.createWithId; +import java.util.function.Predicate; + import org.junit.jupiter.api.Test; import org.mockito.InjectMocks; import org.mockito.Mock; import org.ovirt.engine.core.bll.context.CommandContext; import org.ovirt.engine.core.common.action.IdParameters; import org.ovirt.engine.core.common.businessentities.UserProfileProperty; +import org.ovirt.engine.core.common.businessentities.aaa.DbUser; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dao.DbUserDao; import org.ovirt.engine.core.dao.UserProfileDao; @@ -28,6 +31,9 @@ class RemoveUserProfilePropertyCommandTest extends BaseCommandTest { @Mock private DbUserDao userDaoMock; + @Mock + private Predicate isSystemSuperUserPredicate; + private IdParameters parameters = mock(IdParameters.class); @InjectMocks diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateUserProfilePropertyCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateUserProfilePropertyCommandTest.java index d2e940755f1..4a1d34749bd 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateUserProfilePropertyCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateUserProfilePropertyCommandTest.java @@ -14,6 +14,8 @@ import static org.ovirt.engine.core.bll.UserProfileTestHelper.checkAssertsForSshProp; import static org.ovirt.engine.core.bll.UserProfileTestHelper.createWithId; +import java.util.function.Predicate; + import org.junit.jupiter.api.Test; import org.mockito.InjectMocks; import org.mockito.Mock; @@ -32,6 +34,9 @@ class UpdateUserProfilePropertyCommandTest extends BaseCommandTest { @Mock private DbUserDao userDaoMock; + @Mock + private Predicate isSystemSuperUserPredicate; + private UserProfilePropertyParameters parameters = mock(UserProfilePropertyParameters.class); @InjectMocks diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/UserProfileValidatorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/UserProfileValidatorTest.java index 1af93ebc95c..d6d0cbe536d 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/UserProfileValidatorTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/UserProfileValidatorTest.java @@ -1,6 +1,5 @@ package org.ovirt.engine.core.bll.validator; -import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; @@ -45,41 +44,41 @@ void noPropertyWithGivenName() { @Test void notAuthorizedDueToNullCurrentUser() { - assertFalse(validator.authorized(null, Guid.Empty).isValid()); + assertFalse(validator.authorized(null, Guid.Empty, true).isValid()); } @Test void notAuthorizedDueToNullCurrentUserId() { - assertFalse(validator.authorized(mock(DbUser.class), Guid.Empty).isValid()); + assertFalse(validator.authorized(mock(DbUser.class), Guid.Empty, true).isValid()); } @Test void notAuthorizedDueToNullTargetId() { - assertFalse(validator.authorized(emptyUser(), null).isValid()); + assertFalse(validator.authorized(emptyUser(), null, true).isValid()); } @Test void notAuthorizedDueToDifferentTargetId() { - assertFalse(validator.authorized(emptyUser(), Guid.newGuid()).isValid()); + assertFalse(validator.authorized(emptyUser(), Guid.newGuid(), false).isValid()); } @Test void authorized() { - assertTrue(validator.authorized(emptyUser(), Guid.Empty).isValid()); + assertTrue(validator.authorized(emptyUser(), Guid.Empty, false).isValid()); } @Test void authorizedAsAdminForOtherUser() { DbUser user = emptyUser(); user.setAdmin(true); - assertTrue(validator.authorized(user, Guid.newGuid()).isValid()); + assertTrue(validator.authorized(user, Guid.newGuid(), true).isValid()); } @Test void authorizedAsAdminForHimself() { DbUser user = emptyUser(); user.setAdmin(true); - assertTrue(validator.authorized(user, Guid.Empty).isValid()); + assertTrue(validator.authorized(user, Guid.Empty, true).isValid()); } @Test @@ -131,7 +130,7 @@ void secondSshKey() { @Test void noSshKeyYet() { UserProfileProperty sshProp = UserProfileProperty.builder().withDefaultSshProp().build(); - assertThat(validator.firstPublicSshKey(new UserProfile(), sshProp).isValid()); + assertTrue(validator.firstPublicSshKey(new UserProfile(), sshProp).isValid()); } @Test