Restrict user profile write acces to owner and super user

oVirt · Sep 29, 2022 · 271b83b · 271b83b
1 parent c25c3f3
commit 271b83b
Show file tree

Hide file tree

Showing 7 changed files with 38 additions and 11 deletions.
diff --git a/...er/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddUserProfilePropertyCommand.java b/...er/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/...modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveUserProfilePropertyCommand.java b/...modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveUserProfilePropertyCommand.java
@@ -29,7 +29,6 @@ public class RemoveUserProfilePropertyCommand<T extends IdParameters> 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.authorizedToModify(
+                        getCurrentUser(),
+                        Optional.ofNullable(currentProp).map(UserProfileProperty::getUserId).orElse(null),
+                        isSystemSuperUser()));
     }
 
     @Override

diff --git a/...modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateUserProfilePropertyCommand.java b/...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/...r/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/UserProfileValidator.java b/...r/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/UserProfileValidator.java
@@ -40,14 +40,19 @@ public ValidationResult propertyProvided(UserProfileProperty property) {
                 .when(property == null);
     }
 
-    public ValidationResult authorized(DbUser currentUser, Guid targetUserId) {
+    public ValidationResult authorizedToModify(DbUser currentUser, Guid targetUserId, boolean isSuperUser) {
         return ValidationResult
                 .failWith(EngineMessage.USER_NOT_AUTHORIZED_TO_PERFORM_ACTION)
                 .when(currentUser == null
                         || currentUser.getId() == null
-                        || !Objects.equals(currentUser.getId(), targetUserId) && !currentUser.isAdmin());
+                        || !Objects.equals(currentUser.getId(), targetUserId) && !isSuperUser);
+    }
+
+    public ValidationResult authorized(DbUser currentUser, Guid targetUserId) {
+        return authorizedToModify(currentUser, targetUserId, currentUser != null && currentUser.isAdmin());
     }
 
+
     public ValidationResult validPublicSshKey(UserProfileProperty property) {
         return ValidationResult
                 .failWith(EngineMessage.ACTION_TYPE_FAILED_INVALID_PUBLIC_SSH_KEY)
@@ -88,8 +93,9 @@ public ValidationResult firstPublicSshKey(UserProfile profile, UserProfileProper
     public boolean validateAdd(UserProfile existingProfile,
             UserProfileProperty newProp,
             DbUser currentUser,
-            Function<ValidationResult, Boolean> validate) {
-        return validate.apply(authorized(currentUser, newProp.getUserId())) &&
+            Function<ValidationResult, Boolean> validate,
+            boolean isSuperUser) {
+        return validate.apply(authorizedToModify(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 +107,11 @@ public boolean validateAdd(UserProfile existingProfile,
     public boolean validateUpdate(UserProfileProperty currentProp,
             DbUser currentUser,
             Function<ValidationResult, Boolean> 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(authorizedToModify(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/...odules/bll/src/test/java/org/ovirt/engine/core/bll/AddUserProfilePropertyCommandTest.java b/...odules/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<DbUser> 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/...les/bll/src/test/java/org/ovirt/engine/core/bll/RemoveUserProfilePropertyCommandTest.java b/...les/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<DbUser> isSystemSuperUserPredicate;
+
     private IdParameters parameters = mock(IdParameters.class);
 
     @InjectMocks

diff --git a/...les/bll/src/test/java/org/ovirt/engine/core/bll/UpdateUserProfilePropertyCommandTest.java b/...les/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<DbUser> isSystemSuperUserPredicate;
+
     private UserProfilePropertyParameters parameters = mock(UserProfilePropertyParameters.class);
 
     @InjectMocks