Password hashing fixes. More tests. Work in progress.

infra/prism/src/main/java/com/evolveum/midpoint/prism/crypto/BaseProtector.java

@@ -42,7 +42,7 @@ public <T> void decrypt(ProtectedData<T> protectedData) throws EncryptionExcepti
     protected abstract <T> byte[] decryptBytes(ProtectedData<T> protectedData) throws SchemaException, EncryptionException;
 
     @Override
-    public String decryptString(ProtectedStringType protectedString) throws EncryptionException {
+    public String decryptString(ProtectedData<String> protectedString) throws EncryptionException {
         try {
             if (!protectedString.isEncrypted()) {
                 return protectedString.getClearValue();

infra/prism/src/main/java/com/evolveum/midpoint/prism/crypto/Protector.java

@@ -50,7 +50,7 @@ public interface Protector {
 	 *             if protectedString argument is null or EncryptedData in
 	 *             protectedString argument is null
 	 */
-	String decryptString(ProtectedStringType protectedString) throws EncryptionException;
+	String decryptString(ProtectedData<String> protectedString) throws EncryptionException;
 
 	/**
 	 *

infra/prism/src/main/java/com/evolveum/midpoint/prism/crypto/ProtectorImpl.java

@@ -500,11 +500,12 @@ public <T> void hash(ProtectedData<T> protectedData) throws EncryptionException,
 
 		protectedData.setHashedData(hashedDataType);
 		protectedData.destroyCleartext();
+		protectedData.setEncryptedData(null);
     }
 
     private HashedDataType hashPbkd(ProtectedData<String> protectedData, String algorithmUri, String algorithmName) throws EncryptionException {	
 
-    	char[] clearChars = protectedData.getClearValue().toCharArray();
+    	char[] clearChars = getClearChars(protectedData);
     	byte[] salt = generatePbkdSalt();
     	int iterations = getPbkdIterations();
 
@@ -536,6 +537,14 @@ private HashedDataType hashPbkd(ProtectedData<String> protectedData, String algo
 		return hashedDataType;
 	}
 
+	private char[] getClearChars(ProtectedData<String> protectedData) throws EncryptionException {
+		if (protectedData.isEncrypted()) {
+			return decryptString(protectedData).toCharArray();
+		} else {
+			return protectedData.getClearValue().toCharArray();
+		}
+	}
+
 	private byte[] generatePbkdSalt() {
 		byte[] salt = new byte[getPbkdSaltLength()/8];
 		randomNumberGenerator.nextBytes(salt);
@@ -567,6 +576,9 @@ public boolean compare(ProtectedStringType a, ProtectedStringType b) throws Encr
 				hashedPs = b;
 				clear = decryptString(a);
 			}
+			if (clear == null) {
+				return false;
+			}
 			return compareHashed(hashedPs, clear.toCharArray());
 
 		} else {

infra/prism/src/test/java/com/evolveum/midpoint/prism/crypto/TestProtector.java

@@ -201,6 +201,43 @@ public void testProtectorHashRoundTrip() throws Exception {
 
 		// THEN
 		assertFalse("compare10 unexpected success", compare10);
+
+		ProtectedStringType pstEncHash = new ProtectedStringType();
+		pstEncHash.setClearValue(value);
+		assertFalse(pstEncHash.isEmpty());
+		protector256.encrypt(pstEncHash);
+
+		// WHEN
+		protector256.hash(pstEncHash);
+
+		// THEN
+		assertFalse(pstEncHash.isEmpty());
+		assertTrue(pstEncHash.isHashed());
+		assertFalse(pstEncHash.isEncrypted());
+		assertNull(pstEncHash.getClearValue());
+
+		// WHEN
+		boolean compare1e = protector256.compare(checkPstClear, pstEncHash);
+
+		// THEN
+		assertTrue("compare1e failed", compare1e);
+
+		// WHEN
+		boolean compare2e = protector256.compare(pstEncHash, checkPstClear);
+
+		// THEN
+		assertTrue("compare2e failed", compare2e);
+
+		// WHEN
+		boolean compare3e = protector256.compare(pstEncHash, checkPstEnc);
 
+		// THEN
+		assertTrue("compare3e failed", compare3e);
+
+		// WHEN
+		boolean compare4e = protector256.compare(checkPstEnc, pstEncHash);
+
+		// THEN
+		assertTrue("compare4e failed", compare4e);
 	}
 }

infra/schema/src/main/java/com/evolveum/midpoint/schema/util/SchemaDebugUtil.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2016 Evolveum
+ * Copyright (c) 2010-2017 Evolveum
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -365,6 +365,10 @@ public static String prettyPrint(ProtectedStringType protectedStringType) {
 		if (protectedStringType.getEncryptedDataType() != null) {
 			sb.append("[encrypted data]");
 		}
+
+		if (protectedStringType.getHashedDataType() != null) {
+			sb.append("[hashed data]");
+		}
 
 		if (protectedStringType.getClearValue() != null) {
 			sb.append("\"");

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/projector/CredentialsProcessor.java

@@ -462,7 +462,11 @@ private <O extends ObjectType> void transformFocusExectionDeltaCredential(LensCo
 			if (delta.isAdd()) {
 				delta.getObjectToAdd().removeProperty(valuePropertyPath);
 			} else {
-				delta.removePropertyModification(valuePropertyPath);
+				PropertyDelta<ProtectedStringType> propDelta = delta.findPropertyDelta(valuePropertyPath);
+				if (propDelta != null) {
+					// Replace with nothing. We need this to clear any existing value that there might be.
+					propDelta.setValueToReplace();
+				}
 			}
 		} else {
 			throw new SchemaException("Unkwnon storage type "+storageType);

model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/AbstractConfiguredModelIntegrationTest.java

@@ -285,7 +285,10 @@ public class AbstractConfiguredModelIntegrationTest extends AbstractModelIntegra
 	protected static final File USER_HERMAN_FILE = new File(COMMON_DIR, "user-herman.xml");
 	protected static final String USER_HERMAN_OID = "c0c010c0-d34d-b33f-f00d-111111111122";
 	protected static final String USER_HERMAN_USERNAME = "herman";
+	protected static final String USER_HERMAN_GIVEN_NAME = "Herman";
+	protected static final String USER_HERMAN_FAMILY_NAME = "Toothrot";
 	protected static final String USER_HERMAN_FULL_NAME = "Herman Toothrot";
+	protected static final String USER_HERMAN_PASSWORD = "m0nk3y";
 	protected static final Date USER_HERMAN_VALID_FROM_DATE = MiscUtil.asDate(1700, 5, 30, 11, 00, 00);
 	protected static final Date USER_HERMAN_VALID_TO_DATE = MiscUtil.asDate(2233, 3, 23, 18, 30, 00);
 

model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/password/AbstractPasswordTest.java

@@ -165,7 +165,8 @@ public void test050CheckJackPassword() throws Exception {
 		display("User after change execution", userJack);
 		assertUserJack(userJack, "Jack Sparrow");
 
-		assertEncryptedUserPassword(userJack, USER_JACK_PASSWORD);
+		// Password still encrypted. We haven't changed it yet.
+		assertUserPassword(userJack, USER_JACK_PASSWORD, CredentialsStorageTypeType.ENCRYPTION);
 	}
 
 
@@ -196,7 +197,7 @@ public void test051ModifyUserJackPassword() throws Exception {
 		display("User after change execution", userJack);
 		assertUserJack(userJack, "Jack Sparrow");
 
-		assertEncryptedUserPassword(userJack, USER_PASSWORD_1_CLEAR);
+		assertUserPassword(userJack, USER_PASSWORD_1_CLEAR);
 		assertPasswordMetadata(userJack, false, startCal, endCal);
 		// Password policy is not active yet. No history should be kept.
 		assertPasswordHistoryEntries(userJack);
@@ -207,11 +208,14 @@ public void test060CheckJackPasswordModelInteraction() throws Exception {
 		final String TEST_NAME = "test060CheckJackPasswordModelInteraction";
         TestUtil.displayTestTile(this, TEST_NAME);
 
+        if (getPasswordStorageType() == CredentialsStorageTypeType.NONE) {
+        	// Nothing to check in this case
+        	return;
+        }
+
         // GIVEN
         Task task = createTask(AbstractPasswordTest.class.getName() + "." + TEST_NAME);
         OperationResult result = task.getResult();
-        assumeAssignmentPolicy(AssignmentPolicyEnforcementType.FULL);
-
 
 		// WHEN, THEN
         ProtectedStringType userPasswordPsGood = new ProtectedStringType();
@@ -233,6 +237,38 @@ public void test060CheckJackPasswordModelInteraction() throws Exception {
 
 	}
 
+	@Test
+    public void test070AddUserHerman() throws Exception {
+		final String TEST_NAME = "test070AddUserHerman";
+        TestUtil.displayTestTile(this, TEST_NAME);
+
+        // GIVEN
+        Task task = createTask(AbstractPasswordTest.class.getName() + "." + TEST_NAME);
+        OperationResult result = task.getResult();
+
+        XMLGregorianCalendar startCal = clock.currentTimeXMLGregorianCalendar();
+
+		// WHEN
+        TestUtil.displayWhen(TEST_NAME);
+        addObject(USER_HERMAN_FILE, task, result);
+
+		// THEN
+        TestUtil.displayThen(TEST_NAME);
+		result.computeStatus();
+        TestUtil.assertSuccess("executeChanges result", result);
+
+        XMLGregorianCalendar endCal = clock.currentTimeXMLGregorianCalendar();
+
+        PrismObject<UserType> userAfter = getUser(USER_HERMAN_OID);
+		display("User after", userAfter);
+		assertUser(userAfter, USER_HERMAN_OID, USER_HERMAN_USERNAME, 
+				USER_HERMAN_FULL_NAME, USER_HERMAN_GIVEN_NAME, USER_HERMAN_FAMILY_NAME);
+
+		assertUserPassword(userAfter, USER_HERMAN_PASSWORD);
+		assertPasswordMetadata(userAfter, true, startCal, endCal);
+		// Password policy is not active yet. No history should be kept.
+		assertPasswordHistoryEntries(userAfter);
+	}
 
 	@Test
     public void test100ModifyUserJackAssignAccount() throws Exception {
@@ -298,7 +334,7 @@ public void test110ModifyUserJackPassword() throws Exception {
 		display("User after change execution", userJack);
 		assertUserJack(userJack, "Jack Sparrow");
 
-		assertEncryptedUserPassword(userJack, USER_PASSWORD_2_CLEAR);
+		assertUserPassword(userJack, USER_PASSWORD_2_CLEAR);
 		assertDummyPassword(ACCOUNT_JACK_DUMMY_USERNAME, USER_PASSWORD_2_CLEAR);
 		assertPasswordMetadata(userJack, false, lastPasswordChangeStart, lastPasswordChangeEnd);
 	}
@@ -328,7 +364,7 @@ public void test111ModifyAccountJackPassword() throws Exception {
 		assertUserJack(userJack, "Jack Sparrow");
 
 		// User should still have old password
-		assertEncryptedUserPassword(userJack, USER_PASSWORD_2_CLEAR);
+		assertUserPassword(userJack, USER_PASSWORD_2_CLEAR);
 		// Account has new password
 		assertDummyPassword(ACCOUNT_JACK_DUMMY_USERNAME, USER_PASSWORD_3_CLEAR);
 
@@ -376,7 +412,7 @@ public void test112ModifyJackPasswordUserAndAccount() throws Exception {
 		assertUserJack(userJack, "Jack Sparrow");
 
 		// User should still have old password
-		assertEncryptedUserPassword(userJack, USER_PASSWORD_4_CLEAR);
+		assertUserPassword(userJack, USER_PASSWORD_4_CLEAR);
 		// Account has new password
 		assertDummyPassword(ACCOUNT_JACK_DUMMY_USERNAME, USER_PASSWORD_5_CLEAR);
 
@@ -419,7 +455,7 @@ public void test120ModifyUserJackAssignAccountDummyRedAndUgly() throws Exception
 		assertDummyPassword(RESOURCE_DUMMY_UGLY_NAME, ACCOUNT_JACK_DUMMY_USERNAME, USER_JACK_EMPLOYEE_NUMBER);
 
 		// User and default dummy account should have unchanged passwords
-        assertEncryptedUserPassword(userJack, USER_PASSWORD_4_CLEAR);
+        assertUserPassword(userJack, USER_PASSWORD_4_CLEAR);
      	assertDummyPassword("jack", USER_PASSWORD_5_CLEAR);
 
      	assertPasswordMetadata(userJack, false, lastPasswordChangeStart, lastPasswordChangeEnd);
@@ -466,7 +502,7 @@ public void test121ModifyJackPasswordUserAndAccountRed() throws Exception {
 		assertUserJack(userJack, USER_JACK_FULL_NAME);
 
 		// User should still have old password
-		assertEncryptedUserPassword(userJack, USER_PASSWORD_1_CLEAR);
+		assertUserPassword(userJack, USER_PASSWORD_1_CLEAR);
 		// Red account has the same account as user
 		assertDummyPassword(RESOURCE_DUMMY_RED_NAME, ACCOUNT_JACK_DUMMY_USERNAME, USER_PASSWORD_1_CLEAR);
 		// ... and default account has also the same password as user now. There was no other change on default dummy instance 
@@ -511,7 +547,7 @@ public void test125ModifyJackEmployeeNumberBad() throws Exception {
         PrismObject<UserType> userJack = getUser(USER_JACK_OID);
 		display("User after", userJack);
 
-		assertEncryptedUserPassword(userJack, USER_PASSWORD_1_CLEAR);
+		assertUserPassword(userJack, USER_PASSWORD_1_CLEAR);
 		assertDummyPassword(RESOURCE_DUMMY_RED_NAME, ACCOUNT_JACK_DUMMY_USERNAME, USER_PASSWORD_1_CLEAR);
 		assertDummyPassword(ACCOUNT_JACK_DUMMY_USERNAME, USER_PASSWORD_1_CLEAR);
 		// ugly password should be changed
@@ -547,7 +583,7 @@ public void test128ModifyJackEmployeeNumberGood() throws Exception {
         PrismObject<UserType> userJack = getUser(USER_JACK_OID);
 		display("User after", userJack);
 
-		assertEncryptedUserPassword(userJack, USER_PASSWORD_1_CLEAR);
+		assertUserPassword(userJack, USER_PASSWORD_1_CLEAR);
 		assertDummyPassword(RESOURCE_DUMMY_RED_NAME, ACCOUNT_JACK_DUMMY_USERNAME, USER_PASSWORD_1_CLEAR);
 		assertDummyPassword(ACCOUNT_JACK_DUMMY_USERNAME, USER_PASSWORD_1_CLEAR);
 		// ugly password should be changed
@@ -592,7 +628,7 @@ public void test130ModifyUserJackAssignAccountDummyYellow() throws Exception {
         assertDummyPassword(RESOURCE_DUMMY_RED_NAME, ACCOUNT_JACK_DUMMY_USERNAME, USER_PASSWORD_1_CLEAR);
 
         // User and default dummy account should have unchanged passwords
-        assertEncryptedUserPassword(userJack, USER_PASSWORD_1_CLEAR);
+        assertUserPassword(userJack, USER_PASSWORD_1_CLEAR);
      	assertDummyPassword(ACCOUNT_JACK_DUMMY_USERNAME, USER_PASSWORD_1_CLEAR);
 
 		// this one is not changed
@@ -642,7 +678,7 @@ public void test132ModifyUserJackPasswordA() throws Exception {
         assertDummyPassword(RESOURCE_DUMMY_RED_NAME, ACCOUNT_JACK_DUMMY_USERNAME, USER_PASSWORD_A_CLEAR);
 
         // User and default dummy account should have unchanged passwords
-        assertEncryptedUserPassword(userJack, USER_PASSWORD_A_CLEAR);
+        assertUserPassword(userJack, USER_PASSWORD_A_CLEAR);
      	assertDummyPassword(ACCOUNT_JACK_DUMMY_USERNAME, USER_PASSWORD_A_CLEAR);
 
 		// this one is not changed
@@ -708,7 +744,7 @@ public void test202ReconcileUserJack() throws Exception {
         assertDummyPassword(RESOURCE_DUMMY_RED_NAME, ACCOUNT_JACK_DUMMY_USERNAME, USER_PASSWORD_A_CLEAR);
 
         // User and default dummy account should have unchanged passwords
-        assertEncryptedUserPassword(userJack, USER_PASSWORD_A_CLEAR);
+        assertUserPassword(userJack, USER_PASSWORD_A_CLEAR);
      	assertDummyPassword(ACCOUNT_JACK_DUMMY_USERNAME, USER_PASSWORD_A_CLEAR);
 
 		// this one is not changed
@@ -969,7 +1005,7 @@ private void assertJackPasswordsWithHistory(String expectedCurrentPassword, Stri
 		assertLinks(userJack, 4);
         accountYellowOid = getLinkRefOid(userJack, RESOURCE_DUMMY_YELLOW_OID);
 
-        assertEncryptedUserPassword(userJack, expectedCurrentPassword);
+        assertUserPassword(userJack, expectedCurrentPassword);
         assertPasswordMetadata(userJack, false, lastPasswordChangeStart, lastPasswordChangeEnd);
 
         assertDummyPassword(ACCOUNT_JACK_DUMMY_USERNAME, expectedCurrentPassword);
@@ -1039,7 +1075,7 @@ public void test300TwoParentOrgRefs() throws Exception {
 
 		// Make sure that the password is unchanged
 
-		assertEncryptedUserPassword(userJack, USER_PASSWORD_VALID_1);
+		assertUserPassword(userJack, USER_PASSWORD_VALID_1);
 		assertPasswordMetadata(userJack, false, lastPasswordChangeStart, lastPasswordChangeEnd);
 
 		assertDummyPassword(ACCOUNT_JACK_DUMMY_USERNAME, USER_PASSWORD_VALID_1);

model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/password/TestPasswordDefaultHashing.java

@@ -24,6 +24,7 @@
 
 import com.evolveum.midpoint.schema.result.OperationResult;
 import com.evolveum.midpoint.task.api.Task;
+import com.evolveum.midpoint.xml.ns._public.common.common_3.CredentialsStorageTypeType;
 
 /**
  * Password test with HASHING storage for all credential types.
@@ -45,4 +46,9 @@ protected String getSecurityPolicyOid() {
 		return SECURITY_POLICY_DEFAULT_STORAGE_HASHING_OID;
 	}
 
+	@Override
+	protected CredentialsStorageTypeType getPasswordStorageType() {
+		return CredentialsStorageTypeType.HASHING;
+	}
+
 }

model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/password/TestPasswordNone.java

@@ -0,0 +1,54 @@
+/*
+ * Copyright (c) 2010-2017 Evolveum
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.evolveum.midpoint.model.intest.password;
+
+import static com.evolveum.midpoint.test.IntegrationTestTools.display;
+import static org.testng.AssertJUnit.*;
+
+import org.springframework.test.annotation.DirtiesContext;
+import org.springframework.test.annotation.DirtiesContext.ClassMode;
+import org.springframework.test.context.ContextConfiguration;
+
+import com.evolveum.midpoint.schema.result.OperationResult;
+import com.evolveum.midpoint.task.api.Task;
+import com.evolveum.midpoint.xml.ns._public.common.common_3.CredentialsStorageTypeType;
+
+/**
+ * Password test with NONE password storage (default storage for other types)
+ * 
+ * @author semancik
+ *
+ */
+@ContextConfiguration(locations = {"classpath:ctx-model-intest-test-main.xml"})
+@DirtiesContext(classMode = ClassMode.AFTER_CLASS)
+public class TestPasswordNone extends AbstractPasswordTest {
+
+	@Override
+	public void initSystem(Task initTask, OperationResult initResult) throws Exception {
+		super.initSystem(initTask, initResult);
+	}
+
+	@Override
+	protected String getSecurityPolicyOid() {
+		return SECURITY_POLICY_PASSWORD_STORAGE_NONE_OID;
+	}
+
+	@Override
+	protected CredentialsStorageTypeType getPasswordStorageType() {
+		return CredentialsStorageTypeType.NONE;
+	}
+
+}