Liron Aravot has uploaded a new change for review. Change subject: core: find connections with same password ......................................................................
core: find connections with same password When saving connections to the db the password is being encrypted (introduced in change d0ed215) - the problem with that is that if we attempt to check if a connection with specific details was already saved - we'll not be able to fetch it (as the encrypted password is different each time). This issue was handled in I997b by removing the password out of the query but further fix is required, for example if connection with wrong password already exists in the db, we'll use it instead of using the other password. In this change i changed the way that we check if similar connection exists, the conenctions with the 'same' details (besides the password) are being loaded and then it's being checked if the password is the same as the entered one - if it is, it's the same connection and otherwise it's a different one. Change-Id: I0fc70dd4e7988f4ea4597f016c9de8419c17adfa Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1176402 Signed-off-by: Liron Aravot <lara...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageServerConnectionCommandBase.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java 5 files changed, 42 insertions(+), 21 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/04/36404/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java index b02a69d..87b8d2e 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java @@ -16,6 +16,7 @@ import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.network.VdsNetworkInterface; import org.ovirt.engine.core.common.errors.VdcFault; +import org.ovirt.engine.core.common.utils.ObjectUtils; import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.common.vdscommands.StorageServerConnectionManagementVDSParameters; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; @@ -153,13 +154,31 @@ return (List<StorageServerConnections>) CollectionUtils.subtract(connections, toRemove); } + public static StorageServerConnections findConnectionWithSameDetails(StorageServerConnections connection) { + // As we encrypt the password when saving the connection to the DB and each encryption generates different + // result, + // we can't query the connections to check if connection with the exact + // same details was already added - so we query the connections with the same (currently relevant) details and + // then compare the password after it was already + // decrypted. + List<StorageServerConnections> connections = + DbFacade.getInstance().getStorageServerConnectionDao().getAllForConnection(connection); + for (StorageServerConnections dbConnection : connections) { + if (ObjectUtils.objectsEqual(dbConnection.getpassword(), connection.getpassword())) { + return dbConnection; + } + } + + return null; + } + private void fillConnectionDetailsIfNeeded(StorageServerConnections connection) { // in case that the connection id is null (in case it wasn't loaded from the db before) - we can attempt to load // it from the db by its details. if (connection.getid() == null) { - List<StorageServerConnections> dbConnections = DbFacade.getInstance().getStorageServerConnectionDao().getAllForConnection(connection); - if (!dbConnections.isEmpty()) { - connection.setid(dbConnections.get(0).getid()); + StorageServerConnections dbConnection = findConnectionWithSameDetails(connection); + if (dbConnection != null) { + connection.setid(dbConnection.getid()); } } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java index 146e916..4516920 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java @@ -304,15 +304,14 @@ } for (StorageServerConnections connection : lun.getLunConnections()) { - List<StorageServerConnections> connections = DbFacade.getInstance() - .getStorageServerConnectionDao().getAllForConnection(connection); - if (connections.isEmpty()) { + StorageServerConnections dbConnection = ISCSIStorageHelper.findConnectionWithSameDetails(connection); + if (dbConnection == null) { connection.setid(Guid.newGuid().toString()); connection.setstorage_type(storageType); DbFacade.getInstance().getStorageServerConnectionDao().save(connection); } else { - connection.setid(connections.get(0).getid()); + connection.setid(dbConnection.getid()); } if (DbFacade.getInstance() .getStorageServerConnectionLunMapDao() diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageServerConnectionCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageServerConnectionCommandBase.java index b63e92d..782ba70 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageServerConnectionCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageServerConnectionCommandBase.java @@ -1,5 +1,6 @@ package org.ovirt.engine.core.bll.storage; +import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -84,8 +85,11 @@ String connectionField = connection.getconnection(); connections = getStorageConnDao().getAllForStorage(connectionField); } - else { - connections = getStorageConnDao().getAllForConnection(connection); + else if (connection.getstorage_type() == StorageType.ISCSI) { + StorageServerConnections sameConnection = findConnectionWithSameDetails(connection); + connections = + sameConnection != null ? Arrays.asList(sameConnection) + : Collections.<StorageServerConnections> emptyList(); } boolean isDuplicateConnExists = (connections.size() > 1 @@ -93,6 +97,10 @@ return isDuplicateConnExists; } + protected StorageServerConnections findConnectionWithSameDetails(StorageServerConnections connection) { + return ISCSIStorageHelper.findConnectionWithSameDetails(connection); + } + protected boolean checkIsConnectionFieldEmpty(StorageServerConnections connection) { if (StringUtils.isEmpty(connection.getconnection())) { String fieldName = getFieldName(connection); diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java index a188fb0..92bb823 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java @@ -2,12 +2,12 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; -import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -49,6 +49,7 @@ command = spy(new AddStorageServerConnectionCommand<StorageServerConnectionParametersBase>(parameters, null)); doReturn(storageConnDao).when(command).getStorageConnDao(); doReturn(storageDomainDao).when(command).getStorageDomainDao(); + doReturn(null).when(command).findConnectionWithSameDetails(any(StorageServerConnections.class)); } @Test @@ -194,10 +195,7 @@ StorageServerConnections existingConn = createISCSIConnection("1.2.3.4", StorageType.ISCSI, "iqn.2013-04.myhat.com:aaa-target1", "3650", "user1", "mypassword123"); existingConn.setid(Guid.newGuid().toString()); - List<StorageServerConnections> connections = new ArrayList<>(); - connections.add(existingConn); - - when(storageConnDao.getAllForConnection(newISCSIConnection)).thenReturn(connections); + when(command.findConnectionWithSameDetails(newISCSIConnection)).thenReturn(existingConn); boolean isExists = command.isConnWithSameDetailsExists(newISCSIConnection, null); assertTrue(isExists); } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java index 03a4985..24b8f3d 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.never; @@ -105,6 +106,7 @@ doReturn(storageConnDao).when(command).getStorageConnDao(); doReturn(storageDomainDynamicDao).when((UpdateStorageServerConnectionCommand) command).getStorageDomainDynamicDao(); doReturn(storagePoolIsoMapDAO).when((UpdateStorageServerConnectionCommand) command).getStoragePoolIsoMapDao(); + doReturn(null).when(command).findConnectionWithSameDetails(any(StorageServerConnections.class)); doReturn(lunDAO).when(command).getLunDao(); doReturn(vmDAO).when(command).getVmDAO(); doReturn(storageDomainDAO).when(command).getStorageDomainDao(); @@ -689,11 +691,9 @@ public void isConnWithSameDetailsExistBlockDomains() { StorageServerConnections newISCSIConnection = createISCSIConnection("1.2.3.4", StorageType.ISCSI, "iqn.2013-04.myhat.com:aaa-target1", "3260", "user1", "mypassword123"); - List<StorageServerConnections> connections = new ArrayList<>(); StorageServerConnections connection1 = createISCSIConnection("1.2.3.4", StorageType.ISCSI, "iqn.2013-04.myhat.com:aaa-target1", "3260", "user1", "mypassword123"); - connections.add(connection1); - when(storageConnDao.getAllForConnection(newISCSIConnection)).thenReturn(connections); + when(command.findConnectionWithSameDetails(newISCSIConnection)).thenReturn(connection1); boolean isExists = command.isConnWithSameDetailsExists(newISCSIConnection, null); assertTrue(isExists); } @@ -702,10 +702,7 @@ public void isConnWithSameDetailsExistCheckSameConn() { StorageServerConnections newISCSIConnection = createISCSIConnection("1.2.3.4", StorageType.ISCSI, "iqn.2013-04.myhat.com:aaa-target1", "3260", "user1", "mypassword123"); - List<StorageServerConnections> connections = new ArrayList<>(); - connections.add(newISCSIConnection); - - when(storageConnDao.getAllForConnection(newISCSIConnection)).thenReturn(connections); + when(command.findConnectionWithSameDetails(newISCSIConnection)).thenReturn(newISCSIConnection); boolean isExists = command.isConnWithSameDetailsExists(newISCSIConnection, null); assertFalse(isExists); } -- To view, visit http://gerrit.ovirt.org/36404 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0fc70dd4e7988f4ea4597f016c9de8419c17adfa Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.5 Gerrit-Owner: Liron Aravot <lara...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
