From 8d75084f0657e6cc3b48d81f7b3570ff374295db Mon Sep 17 00:00:00 2001 From: nicolatimeus Date: Thu, 12 Oct 2023 16:51:05 +0200 Subject: [PATCH] fix: Return placeholder instead of encrypted passwords in REST APIs (#4893) fix: Return placeholder instead of encrypted passwords in web ui Signed-off-by: Nicola Timeus --- .../META-INF/MANIFEST.MF | 2 +- .../ConfigurationRestService.java | 8 ++- .../api/ComponentConfigurationDTO.java | 42 ++++++++++++- .../api/ComponentConfigurationList.java | 13 +++- .../META-INF/MANIFEST.MF | 2 +- .../internal/rest/wire/WireRestService.java | 4 +- .../test/ConfigurationRestServiceTest.java | 62 ++++++++++++++----- 7 files changed, 109 insertions(+), 24 deletions(-) diff --git a/kura/org.eclipse.kura.rest.configuration.provider/META-INF/MANIFEST.MF b/kura/org.eclipse.kura.rest.configuration.provider/META-INF/MANIFEST.MF index 5b2ec2e7e5f..5ffbf7e9f39 100644 --- a/kura/org.eclipse.kura.rest.configuration.provider/META-INF/MANIFEST.MF +++ b/kura/org.eclipse.kura.rest.configuration.provider/META-INF/MANIFEST.MF @@ -25,4 +25,4 @@ Import-Package: javax.annotation.security;version="1.2.0", org.osgi.service.component;version="1.3.0", org.osgi.service.useradmin;version="1.1.0";resolution:=optional, org.slf4j;version="1.7.25" -Export-Package: org.eclipse.kura.rest.configuration.api;version="1.0.0" +Export-Package: org.eclipse.kura.rest.configuration.api;version="1.1.0" diff --git a/kura/org.eclipse.kura.rest.configuration.provider/src/main/java/org/eclipse/kura/internal/rest/configuration/ConfigurationRestService.java b/kura/org.eclipse.kura.rest.configuration.provider/src/main/java/org/eclipse/kura/internal/rest/configuration/ConfigurationRestService.java index 20ed9b9120a..64b966932ba 100644 --- a/kura/org.eclipse.kura.rest.configuration.provider/src/main/java/org/eclipse/kura/internal/rest/configuration/ConfigurationRestService.java +++ b/kura/org.eclipse.kura.rest.configuration.provider/src/main/java/org/eclipse/kura/internal/rest/configuration/ConfigurationRestService.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2021, 2022 Eurotech and/or its affiliates and others + * Copyright (c) 2021, 2023 Eurotech and/or its affiliates and others * * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 @@ -361,7 +361,7 @@ public ComponentConfigurationList listComponentConfigurations() { throw DefaultExceptionHandler.toWebApplicationException(e); } - return DTOUtil.toComponentConfigurationList(ccs, this.cryptoService, false); + return DTOUtil.toComponentConfigurationList(ccs, this.cryptoService, false).replacePasswordsWithPlaceholder(); } @@ -398,7 +398,8 @@ public ComponentConfigurationList listComponentConfigurations(final PidSet pids) } }); - return DTOUtil.toComponentConfigurationList(configs, this.cryptoService, false); + return DTOUtil.toComponentConfigurationList(configs, this.cryptoService, false) + .replacePasswordsWithPlaceholder(); } /** @@ -562,4 +563,5 @@ public Response rollbackSnapshot(final SnapshotId id) { return Response.ok().build(); } + } diff --git a/kura/org.eclipse.kura.rest.configuration.provider/src/main/java/org/eclipse/kura/rest/configuration/api/ComponentConfigurationDTO.java b/kura/org.eclipse.kura.rest.configuration.provider/src/main/java/org/eclipse/kura/rest/configuration/api/ComponentConfigurationDTO.java index 6863ded1455..ddf2f37c869 100644 --- a/kura/org.eclipse.kura.rest.configuration.provider/src/main/java/org/eclipse/kura/rest/configuration/api/ComponentConfigurationDTO.java +++ b/kura/org.eclipse.kura.rest.configuration.provider/src/main/java/org/eclipse/kura/rest/configuration/api/ComponentConfigurationDTO.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2021, 2022 Eurotech and/or its affiliates and others + * Copyright (c) 2021, 2023 Eurotech and/or its affiliates and others * * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 @@ -12,12 +12,15 @@ *******************************************************************************/ package org.eclipse.kura.rest.configuration.api; +import java.util.HashMap; import java.util.Map; +import java.util.Map.Entry; import java.util.Optional; import javax.ws.rs.core.Response.Status; import org.eclipse.kura.configuration.metatype.OCD; +import org.eclipse.kura.configuration.metatype.Scalar; import org.eclipse.kura.request.handler.jaxrs.DefaultExceptionHandler; public class ComponentConfigurationDTO implements Validable { @@ -45,6 +48,43 @@ public Map getProperties() { return this.properties; } + public ComponentConfigurationDTO replacePasswordsWithPlaceholder() { + + if (properties == null) { + return this; + } + + final Map result = new HashMap<>(this.properties); + + for (final Entry e : result.entrySet()) { + e.setValue(replacePasswordsWithPlaceholder(e.getValue())); + } + + return new ComponentConfigurationDTO(pid, definition, result); + } + + public PropertyDTO replacePasswordsWithPlaceholder(final PropertyDTO property) { + + if (property == null || property.getType() != Scalar.PASSWORD) { + return property; + } + + if (property.getValue() instanceof String[]) { + final String[] asStringArray = (String[]) property.getValue(); + final String[] result = new String[asStringArray.length]; + + for (int i = 0; i < asStringArray.length; i++) { + if (asStringArray[i] != null) { + result[i] = "placeholder"; + } + } + + return new PropertyDTO(result, Scalar.PASSWORD); + } else { + return new PropertyDTO("placeholder", Scalar.PASSWORD); + } + } + @Override public void validate() { FailureHandler.requireParameter(this.pid, "pid"); diff --git a/kura/org.eclipse.kura.rest.configuration.provider/src/main/java/org/eclipse/kura/rest/configuration/api/ComponentConfigurationList.java b/kura/org.eclipse.kura.rest.configuration.provider/src/main/java/org/eclipse/kura/rest/configuration/api/ComponentConfigurationList.java index bc92aed3e39..c20b40f715f 100644 --- a/kura/org.eclipse.kura.rest.configuration.provider/src/main/java/org/eclipse/kura/rest/configuration/api/ComponentConfigurationList.java +++ b/kura/org.eclipse.kura.rest.configuration.provider/src/main/java/org/eclipse/kura/rest/configuration/api/ComponentConfigurationList.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2021, 2022 Eurotech and/or its affiliates and others + * Copyright (c) 2021, 2023 Eurotech and/or its affiliates and others * * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 @@ -13,6 +13,7 @@ package org.eclipse.kura.rest.configuration.api; import java.util.List; +import java.util.stream.Collectors; public class ComponentConfigurationList implements Validable { @@ -30,4 +31,14 @@ public List getConfigs() { public void validate() { FailureHandler.requireParameter(this.configs, "configs"); } + + public ComponentConfigurationList replacePasswordsWithPlaceholder() { + if (configs == null) { + return this; + } + + return new ComponentConfigurationList( + configs.stream().map(ComponentConfigurationDTO::replacePasswordsWithPlaceholder) + .collect(Collectors.toList())); + } } diff --git a/kura/org.eclipse.kura.rest.wire.provider/META-INF/MANIFEST.MF b/kura/org.eclipse.kura.rest.wire.provider/META-INF/MANIFEST.MF index 9828d8607b5..1c459eacd09 100644 --- a/kura/org.eclipse.kura.rest.wire.provider/META-INF/MANIFEST.MF +++ b/kura/org.eclipse.kura.rest.wire.provider/META-INF/MANIFEST.MF @@ -24,7 +24,7 @@ Import-Package: com.google.gson;version="2.7.0", org.eclipse.kura.internal.wire.asset;version="[1.0,2.0)", org.eclipse.kura.marshalling;version="[1.0,2.0)", org.eclipse.kura.request.handler.jaxrs;version="[1.0,2.0)", - org.eclipse.kura.rest.configuration.api;version="[1.0,2.0)", + org.eclipse.kura.rest.configuration.api;version="[1.1,2.0)", org.eclipse.kura.util.service;version="[1.0,2.0)", org.eclipse.kura.wire;version="[2.0,3.0)", org.eclipse.kura.wire.graph;version="[1.0,2.0)", diff --git a/kura/org.eclipse.kura.rest.wire.provider/src/main/java/org/eclipse/kura/internal/rest/wire/WireRestService.java b/kura/org.eclipse.kura.rest.wire.provider/src/main/java/org/eclipse/kura/internal/rest/wire/WireRestService.java index bcb127ad286..674d883a00e 100644 --- a/kura/org.eclipse.kura.rest.wire.provider/src/main/java/org/eclipse/kura/internal/rest/wire/WireRestService.java +++ b/kura/org.eclipse.kura.rest.wire.provider/src/main/java/org/eclipse/kura/internal/rest/wire/WireRestService.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2022 Eurotech and/or its affiliates and others + * Copyright (c) 2023 Eurotech and/or its affiliates and others * * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 @@ -362,7 +362,7 @@ public ComponentConfigurationList getWireConfigsByPid(final PidSet pidSet) { } } - return DTOUtil.toComponentConfigurationList(result, cryptoService, true); + return DTOUtil.toComponentConfigurationList(result, cryptoService, false).replacePasswordsWithPlaceholder(); } catch (final Exception e) { throw DefaultExceptionHandler.toWebApplicationException(e); diff --git a/kura/test/org.eclipse.kura.rest.configuration.provider.test/src/main/java/org/eclipse/kura/rest/configuration/provider/test/ConfigurationRestServiceTest.java b/kura/test/org.eclipse.kura.rest.configuration.provider.test/src/main/java/org/eclipse/kura/rest/configuration/provider/test/ConfigurationRestServiceTest.java index 4df3d30f39b..958d5f6c572 100644 --- a/kura/test/org.eclipse.kura.rest.configuration.provider.test/src/main/java/org/eclipse/kura/rest/configuration/provider/test/ConfigurationRestServiceTest.java +++ b/kura/test/org.eclipse.kura.rest.configuration.provider.test/src/main/java/org/eclipse/kura/rest/configuration/provider/test/ConfigurationRestServiceTest.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2021, 2022 Eurotech and/or its affiliates and others + * Copyright (c) 2021, 2023 Eurotech and/or its affiliates and others * * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 @@ -80,7 +80,7 @@ @RunWith(Parameterized.class) public class ConfigurationRestServiceTest extends AbstractRequestHandlerTest { - private char[] EncryptedPassword; + private char[] encryptedPassword; @Test public void shouldSupportGetSnapshots() throws KuraException { @@ -270,16 +270,45 @@ public void testGetLongProperty() throws KuraException { } @Test - public void testGetPasswordProperty() throws KuraException { - this.EncryptedPassword = this.cryptoService.encryptAes("foobar".toCharArray()); + public void testReturnPlaceholderInsteadOfEncryptedPassword() throws KuraException { + givenEncryptedPassword("foobar"); givenATestConfigurationPropertyWithAdTypeAndValue(Scalar.PASSWORD, - new Password(this.EncryptedPassword)); + new Password(this.encryptedPassword)); whenRequestIsPerformed(new MethodSpec("GET"), "/configurableComponents/configurations"); thenRequestSucceeds(); thenTestPropertyTypeIs(Json.value("PASSWORD")); - thenTestPropertyValueIs(Json.value(new String(this.EncryptedPassword))); + thenTestPropertyValueIs(Json.value("placeholder")); + } + + @Test + public void testReturnNoValueForMissingPasswordProperty() throws KuraException { + givenEncryptedPassword("foobar"); + givenATestConfigurationPropertyWithAdTypeAndValue(Scalar.PASSWORD, + null); + + whenRequestIsPerformed(new MethodSpec("GET"), "/configurableComponents/configurations"); + + thenRequestSucceeds(); + thenTestPropertyIsMissing(); + } + + @Test + public void testGetSnapshotReturnsUnencryptedPassword() throws KuraException { + givenATestConfigurationPropertyWithAdTypeAndValue(Scalar.PASSWORD, + new Password("foobar".toCharArray())); + + whenRequestIsPerformed(new MethodSpec("POST"), "/snapshots/byId", + "{\"id\":1}"); + + thenRequestSucceeds(); + thenTestPropertyTypeIs(Json.value("PASSWORD")); + thenTestPropertyValueIs(Json.value("foobar")); + } + + private void givenEncryptedPassword(final String password) throws KuraException { + this.encryptedPassword = this.cryptoService.encryptAes(password.toCharArray()); } @Test @@ -394,16 +423,16 @@ public void testGetLongArrayProperty() throws KuraException { @Test public void testGetPasswordArrayProperty() throws KuraException { - this.EncryptedPassword = this.cryptoService.encryptAes("foobar".toCharArray()); + givenEncryptedPassword("foobar"); givenATestConfigurationPropertyWithAdTypeAndValue(Scalar.PASSWORD, - new Password[] { new Password(this.EncryptedPassword) }); + new Password[] { new Password(this.encryptedPassword) }); whenRequestIsPerformed(new MethodSpec("GET"), "/configurableComponents/configurations"); thenRequestSucceeds(); thenTestPropertyTypeIs(Json.value("PASSWORD")); - thenTestPropertyValueIs(Json.array(new String(this.EncryptedPassword))); + thenTestPropertyValueIs(Json.array("placeholder")); } @Test @@ -1036,7 +1065,7 @@ public void testADOption() throws KuraException { @Test public void testGetConfigurationByPid() throws KuraException { - this.EncryptedPassword = this.cryptoService.encryptAes("foobar".toCharArray()); + givenEncryptedPassword("foobar"); givenConfigurations(configurationBuilder("foo") // .withDefinition( // ocdBuilder("foo") // @@ -1046,7 +1075,7 @@ public void testGetConfigurationByPid() throws KuraException { .build()) // .build()) // .withConfigurationProperties( - singletonMap("testProp", new Password[] { new Password(this.EncryptedPassword) })) + singletonMap("testProp", new Password[] { new Password(this.encryptedPassword) })) .build()); whenRequestIsPerformed(new MethodSpec("POST"), "/configurableComponents/configurations/byPid", @@ -1058,14 +1087,14 @@ public void testGetConfigurationByPid() throws KuraException { + "{\"label\":\"pass\",\"value\":\"baz\"}],\"id\":\"fooAdName\",\"type\":\"PASSWORD\"," + "\"cardinality\":0,\"isRequired\":false}],\"id\":\"foo\"}," + "\"properties\":{\"testProp\":{\"value\":[\"" - + new String(this.EncryptedPassword) + + "placeholder" + "\"],\"type\":\"PASSWORD\"}}}"), self().field("configs").arrayItem(0)); } @Test public void testGetConfigurationByPidDefault() throws KuraException { - this.EncryptedPassword = this.cryptoService.encryptAes("foobar".toCharArray()); + givenEncryptedPassword("foobar"); givenConfigurations(configurationBuilder("foo") // .withDefinition( // ocdBuilder("foo") // @@ -1076,7 +1105,7 @@ public void testGetConfigurationByPidDefault() throws KuraException { .build()) // .build()) // .withConfigurationProperties( - singletonMap("testProp", new Password[] { new Password(this.EncryptedPassword) })) + singletonMap("testProp", new Password[] { new Password(this.encryptedPassword) })) .build()); whenRequestIsPerformed(new MethodSpec("POST"), "/configurableComponents/configurations/byPid/_default", @@ -1088,7 +1117,7 @@ public void testGetConfigurationByPidDefault() throws KuraException { + "{\"label\":\"pass\",\"value\":\"baz\"}],\"id\":\"fooAdName\",\"type\":\"STRING\"," + "\"cardinality\":0,\"defaultValue\":\"default\",\"isRequired\":false}]" + ",\"id\":\"foo\"},\"properties\":{\"testProp\":{\"value\":[\"" - + new String(this.EncryptedPassword) + + new String(this.encryptedPassword) + "\"],\"type\":\"PASSWORD\"}}}"), self().field("configs").arrayItem(0)); } @@ -1405,6 +1434,9 @@ private void givenConfigurations(final ComponentConfiguration... configurations) final String pid = i.getArgument(0, String.class); return byPid.get(pid); }); + + Mockito.when(configurationService.getSnapshot(Mockito.anyLong())) + .thenReturn(byPid.values().stream().collect(Collectors.toList())); } private void givenATestConfigurationPropertyWithAdTypeAndValue(final Scalar type, final Object value)