From 75472f3bf84826182d590067fd40d3780276dc3b Mon Sep 17 00:00:00 2001 From: Pavol Mederly Date: Wed, 15 Jun 2016 12:19:58 +0200 Subject: [PATCH 1/2] Temporary fix for MID-3111: Remove user password button over GUI is not provisioned to target system when strong outbound mapping is set --- .../icf/dummy/connector/DummyConnector.java | 36 ++++++++------- .../lens/projector/CredentialsProcessor.java | 31 +++++++++---- .../midpoint/model/intest/TestPassword.java | 46 +++++++++++++++++-- .../ucf/impl/ConnectorInstanceIcfImpl.java | 18 +++----- 4 files changed, 89 insertions(+), 42 deletions(-) diff --git a/icf-connectors/dummy-connector/src/main/java/com/evolveum/icf/dummy/connector/DummyConnector.java b/icf-connectors/dummy-connector/src/main/java/com/evolveum/icf/dummy/connector/DummyConnector.java index b365fffd372..df71ec842b5 100644 --- a/icf-connectors/dummy-connector/src/main/java/com/evolveum/icf/dummy/connector/DummyConnector.java +++ b/icf-connectors/dummy-connector/src/main/java/com/evolveum/icf/dummy/connector/DummyConnector.java @@ -179,7 +179,7 @@ public void access(char[] chars) { } /** - * Disposes of the {@link CSVFileConnector}'s resources. + * Disposes of the connector's resources. * * @see Connector#dispose() */ @@ -1808,23 +1808,25 @@ private Date getDate(Attribute attr) { private void changePassword(final DummyAccount account, Attribute attr) throws ConnectException, FileNotFoundException, SchemaViolationException { - if (attr.getValue() == null || attr.getValue().isEmpty()) { - throw new IllegalArgumentException("Empty password was provided"); - } - Object passwdObject = attr.getValue().get(0); - if (!(passwdObject instanceof GuardedString)) { - throw new IllegalArgumentException("Password was provided as "+passwdObject.getClass().getName()+" while expecting GuardedString"); - } - final String[] passwdArray = { "" }; - ((GuardedString)passwdObject).access(new Accessor() { - @Override - public void access(char[] passwdChars) { - if (configuration.getMinPasswordLength() != null && passwdChars.length < configuration.getMinPasswordLength()) { - throw new InvalidAttributeValueException("Password too short"); - } - passwdArray[0] = new String(passwdChars); + final String[] passwdArray = { null }; + if (attr.getValue() != null && !attr.getValue().isEmpty()) { + Object passwdObject = attr.getValue().get(0); + if (!(passwdObject instanceof GuardedString)) { + throw new IllegalArgumentException( + "Password was provided as " + passwdObject.getClass().getName() + " while expecting GuardedString"); } - }); + ((GuardedString)passwdObject).access(new Accessor() { + @Override + public void access(char[] passwdChars) { + if (configuration.getMinPasswordLength() != null && passwdChars.length < configuration.getMinPasswordLength()) { + throw new InvalidAttributeValueException("Password too short"); + } + passwdArray[0] = new String(passwdChars); + } + }); + } else { + // empty password => null + } account.setPassword(passwdArray[0]); } diff --git a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/projector/CredentialsProcessor.java b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/projector/CredentialsProcessor.java index 097bcfb8d07..a94567313fe 100644 --- a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/projector/CredentialsProcessor.java +++ b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/projector/CredentialsProcessor.java @@ -16,7 +16,7 @@ package com.evolveum.midpoint.model.impl.lens.projector; import java.util.Collection; -import java.util.List; +import java.util.Collections; import javax.xml.datatype.XMLGregorianCalendar; import javax.xml.namespace.QName; @@ -227,14 +227,27 @@ public StringPolicyType resolve() { passwordMapping.setStringPolicyResolver(stringPolicyResolver); mappingEvaluator.evaluateMapping(passwordMapping, context, task, result); - - PrismProperty accountPasswordNew = (PrismProperty) passwordMapping.getOutput(); - if (accountPasswordNew == null || accountPasswordNew.isEmpty()) { - LOGGER.trace("Credentials 'password' expression resulted in null, skipping credentials processing for {}", rat); - return; - } - PropertyDelta accountPasswordDeltaNew = new PropertyDelta(SchemaConstants.PATH_PASSWORD_VALUE, accountPasswordPropertyDefinition, prismContext); - accountPasswordDeltaNew.setValuesToReplace(accountPasswordNew.getClonedValues()); + + // TODO review all this code !! MID-3156 + // Originally here was "do nothing if output is null or empty". + // But because of MID-3111 we have to be a bit more cautious + PrismProperty accountPasswordNew = (PrismProperty) passwordMapping.getOutput(); + if (accountPasswordNew == null || accountPasswordNew.isEmpty()) { + if (passwordMapping.getOutputTriple() == null) { + LOGGER.trace("Credentials 'password' expression resulted in null output triple, skipping credentials processing for {}", rat); + return; + } + if (passwordMapping.getStrength() != MappingStrengthType.STRONG) { + LOGGER.trace("Credentials 'password' expression resulted in 'no value' via non-strong mapping, skipping credentials processing for {}", rat); + return; + } + } + PropertyDelta accountPasswordDeltaNew = new PropertyDelta<>(SchemaConstants.PATH_PASSWORD_VALUE, accountPasswordPropertyDefinition, prismContext); + if (accountPasswordNew != null) { + accountPasswordDeltaNew.setValuesToReplace(accountPasswordNew.getClonedValues()); + } else { + accountPasswordDeltaNew.setValuesToReplace(Collections.>emptyList()); + } LOGGER.trace("Adding new password delta for account {}", rat); accCtx.swallowToSecondaryDelta(accountPasswordDeltaNew); diff --git a/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/TestPassword.java b/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/TestPassword.java index ffbf0726e30..d58b4d9b550 100644 --- a/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/TestPassword.java +++ b/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/TestPassword.java @@ -16,10 +16,7 @@ package com.evolveum.midpoint.model.intest; import static com.evolveum.midpoint.test.IntegrationTestTools.display; -import static org.testng.AssertJUnit.assertEquals; -import static org.testng.AssertJUnit.assertNotNull; -import static org.testng.AssertJUnit.assertTrue; -import static org.testng.AssertJUnit.assertFalse; +import static org.testng.AssertJUnit.*; import java.io.File; import java.util.ArrayList; @@ -750,6 +747,47 @@ public void test300TwoParentOrgRefs() throws Exception { assertDummyPassword(RESOURCE_DUMMY_RED_NAME, ACCOUNT_JACK_DUMMY_USERNAME, USER_PASSWORD_VALID); } + /* + * Remove password. It should be removed from red resource as well. (MID-3111) + * Also unassign yellow resource (requires non-empty password), all orgs, and remove default password policy. + */ + @Test + public void test310RemovePassword() throws Exception { + final String TEST_NAME = "test310RemovePassword"; + TestUtil.displayTestTile(this, TEST_NAME); + + // GIVEN + Task task = taskManager.createTaskInstance(TestPassword.class.getName() + "." + TEST_NAME); + OperationResult result = task.getResult(); + assumeAssignmentPolicy(AssignmentPolicyEnforcementType.FULL); + + unassignAccount(USER_JACK_OID, RESOURCE_DUMMY_YELLOW_OID, null, task, result); + unassignOrg(USER_JACK_OID, ORG_GOVERNOR_OFFICE_OID, null, task, result); + unassignOrg(USER_JACK_OID, ORG_GOVERNOR_OFFICE_OID, SchemaConstants.ORG_MANAGER, task, result); + modifyObjectReplaceReference(SystemConfigurationType.class, SystemObjectsType.SYSTEM_CONFIGURATION.value(), + SystemConfigurationType.F_GLOBAL_PASSWORD_POLICY_REF, task, result); + + // WHEN + modifyUserReplace(USER_JACK_OID, PASSWORD_VALUE_PATH, task, result); + + // THEN + result.computeStatus(); + TestUtil.assertSuccess(result); + + PrismObject userJack = getUser(USER_JACK_OID); + display("User after change execution", userJack); + assertUserJack(userJack, "Jack Sparrow"); + + assertNull("User password is not null", userJack.asObjectable().getCredentials().getPassword().getValue()); + assertDummyPassword(ACCOUNT_JACK_DUMMY_USERNAME, USER_PASSWORD_VALID); // password mapping is weak here - so no change is expected + + assertNoDummyAccount(RESOURCE_DUMMY_YELLOW_NAME, ACCOUNT_JACK_DUMMY_USERNAME); + + assertDummyAccount(RESOURCE_DUMMY_RED_NAME, ACCOUNT_JACK_DUMMY_USERNAME, ACCOUNT_JACK_DUMMY_FULLNAME, true); + assertDummyPassword(RESOURCE_DUMMY_RED_NAME, ACCOUNT_JACK_DUMMY_USERNAME, null); // password mapping is strong here + } + + private void assertDummyPassword(String userId, String expectedClearPassword) throws SchemaViolationException { assertDummyPassword(null, userId, expectedClearPassword); } diff --git a/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/ucf/impl/ConnectorInstanceIcfImpl.java b/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/ucf/impl/ConnectorInstanceIcfImpl.java index 0fe91f09e29..19d794329a6 100644 --- a/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/ucf/impl/ConnectorInstanceIcfImpl.java +++ b/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/ucf/impl/ConnectorInstanceIcfImpl.java @@ -19,13 +19,7 @@ import java.io.File; import java.lang.reflect.Array; -import java.util.ArrayList; -import java.util.Collection; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; import javax.xml.datatype.XMLGregorianCalendar; import javax.xml.namespace.QName; @@ -2728,12 +2722,12 @@ private void convertFromPassword(Set attributes, PropertyDelta newPassword = passwordDelta.getPropertyNewMatchingPath(); if (newPassword == null || newPassword.isEmpty()) { - LOGGER.trace("Skipping processing password delta. Password delta does not contain new value."); - return; + LOGGER.debug("Setting null password."); + attributes.add(AttributeBuilder.build(OperationalAttributes.PASSWORD_NAME, Collections.EMPTY_LIST)); + } else { + GuardedString guardedPassword = IcfUtil.toGuardedString(newPassword.getValue().getValue(), "new password", protector); + attributes.add(AttributeBuilder.build(OperationalAttributes.PASSWORD_NAME, guardedPassword)); } - GuardedString guardedPassword = IcfUtil.toGuardedString(newPassword.getValue().getValue(), "new password", protector); - attributes.add(AttributeBuilder.build(OperationalAttributes.PASSWORD_NAME, guardedPassword)); - } private void addConvertedValues(Collection> pvals, From fabf5503a881075008abc3661d6d734b538e64a8 Mon Sep 17 00:00:00 2001 From: Radovan Semancik Date: Wed, 15 Jun 2016 13:16:27 +0200 Subject: [PATCH 2/2] Fixed empty ProtectedStringType in delta (MID-3150) --- .../gui/api/component/password/PasswordPanel.java | 14 +++++++------- .../web/component/prism/PropertyWrapper.java | 4 +--- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/api/component/password/PasswordPanel.java b/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/api/component/password/PasswordPanel.java index d19a834b6a6..35ac67aaa9e 100644 --- a/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/api/component/password/PasswordPanel.java +++ b/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/api/component/password/PasswordPanel.java @@ -177,10 +177,6 @@ private void onRemovePassword(IModel model, AjaxRequestTarg get(ID_LINK_CONTAINER).get(ID_PASSWORD_REMOVE).setVisible(true); passwordInputVisble = false; target.add(this); - - ProtectedStringType newValue = new ProtectedStringType(); - byte[] temp = new byte[0]; - newValue.setClearBytes(temp); model.setObject(null); } @@ -263,10 +259,14 @@ public String getObject() { @Override public void setObject(String object) { - if (psModel.getObject() == null) { - psModel.setObject(new ProtectedStringType()); + if (object == null) { + psModel.setObject(null); + } else { + if (psModel.getObject() == null) { + psModel.setObject(new ProtectedStringType()); + } + psModel.getObject().setClearValue(object); } - psModel.getObject().setClearValue(object); } } diff --git a/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/prism/PropertyWrapper.java b/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/prism/PropertyWrapper.java index 620050b90c2..6e45d8d4f33 100644 --- a/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/prism/PropertyWrapper.java +++ b/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/prism/PropertyWrapper.java @@ -44,9 +44,7 @@ public class PropertyWrapper, ID extend public PropertyWrapper(@Nullable ContainerWrapper container, I property, boolean readonly, ValueStatus status) { super(container, property, readonly, status); - ItemPath passwordPath = new ItemPath(SchemaConstantsGenerated.C_CREDENTIALS, - CredentialsType.F_PASSWORD); - if (container != null && passwordPath.equivalent(container.getPath()) + if (container != null && SchemaConstants.PATH_PASSWORD.equivalent(container.getPath()) && PasswordType.F_VALUE.equals(property.getElementName())) { super.setDisplayName("prismPropertyPanel.name.credentials.password"); }