From 91c66c78c1468bebb1b8c70308a30241946387fa Mon Sep 17 00:00:00 2001 From: Vladimir Krivosheev Date: Fri, 30 Nov 2018 09:44:38 +0100 Subject: [PATCH] sensitive information check: ignore use-password --- .../xml/ForbidSensitiveInformationTest.kt | 112 ++++++++++++++++++ .../testSrc/xml/XmlSerializerTest.kt | 92 +------------- .../configurationStore/BaseXmlOutputter.kt | 11 +- .../configurationStore/JbXmlOutputter.kt | 2 +- 4 files changed, 127 insertions(+), 90 deletions(-) create mode 100644 platform/configuration-store-impl/testSrc/xml/ForbidSensitiveInformationTest.kt diff --git a/platform/configuration-store-impl/testSrc/xml/ForbidSensitiveInformationTest.kt b/platform/configuration-store-impl/testSrc/xml/ForbidSensitiveInformationTest.kt new file mode 100644 index 000000000000..08d6ac879e8d --- /dev/null +++ b/platform/configuration-store-impl/testSrc/xml/ForbidSensitiveInformationTest.kt @@ -0,0 +1,112 @@ +// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package com.intellij.configurationStore.xml + +import com.intellij.configurationStore.JbXmlOutputter +import com.intellij.openapi.util.io.FileUtilRt +import com.intellij.testFramework.assertions.Assertions.assertThat +import com.intellij.util.SystemProperties +import com.intellij.util.xmlb.annotations.Attribute +import com.intellij.util.xmlb.annotations.OptionTag +import com.intellij.util.xmlb.annotations.Tag +import org.assertj.core.api.Assertions.assertThatThrownBy +import org.junit.Test +import java.io.StringWriter + +internal class ForbidSensitiveInformationTest { + @Test + fun `do not store password as attribute`() { + @Tag("bean") + class Bean { + @Attribute + var password: String? = null + + @Attribute + var foo: String? = null + } + + val bean = Bean() + bean.foo = "module" + bean.password = "ab" + // it is not part of XML bindings to ensure that even if you will use JDOM directly, you cannot output sensitive data + // so, testSerializer must not throw error + val element = assertSerializer(bean, "") + + assertThatThrownBy { + val xmlWriter = JbXmlOutputter() + xmlWriter.output(element, StringWriter()) + }.hasMessage("Attribute \"password\" probably contains sensitive information") + } + + @Test + fun `do not store password as element`() { + @Tag("bean") + class Bean { + var password: String? = null + + @Attribute + var foo: String? = null + } + + val bean = Bean() + bean.foo = "module" + bean.password = "ab" + // it is not part of XML bindings to ensure that even if you will use JDOM directly, you cannot output sensitive data + // so, testSerializer must not throw error + val element = assertSerializer(bean, """ + + + """.trimIndent()) + + assertThatThrownBy { + val xmlWriter = JbXmlOutputter( + storageFilePathForDebugPurposes = "${FileUtilRt.toSystemIndependentName(SystemProperties.getUserHome())}/foo/bar.xml") + xmlWriter.output(element, StringWriter()) + }.hasMessage("Element \"password\" probably contains sensitive information (file: ~/foo/bar.xml)") + } + + @Test + fun `configuration name with password word`() { + @Tag("bean") + class Bean { + @OptionTag(tag = "configuration", valueAttribute = "bar") + var password: String? = null + + // check that use or save password fields are ignored + var usePassword = false + var savePassword = false + var rememberPassword = false + @Attribute("keep-password") + var keepPassword = false + } + + val bean = Bean() + bean.password = "ab" + bean.usePassword = true + bean.keepPassword = true + bean.rememberPassword = true + bean.savePassword = true + // it is not part of XML bindings to ensure that even if you will use JDOM directly, you cannot output sensitive data + // so, testSerializer must not throw error + val element = assertSerializer(bean, """ + + + """.trimIndent()) + + val xmlWriter = JbXmlOutputter() + val stringWriter = StringWriter() + xmlWriter.output(element, stringWriter) + assertThat(stringWriter.toString()).isEqualTo(""" + + + """.trimIndent()) + } +} \ No newline at end of file diff --git a/platform/configuration-store-impl/testSrc/xml/XmlSerializerTest.kt b/platform/configuration-store-impl/testSrc/xml/XmlSerializerTest.kt index e983850c2590..8a44826adfe3 100644 --- a/platform/configuration-store-impl/testSrc/xml/XmlSerializerTest.kt +++ b/platform/configuration-store-impl/testSrc/xml/XmlSerializerTest.kt @@ -3,25 +3,23 @@ package com.intellij.configurationStore.xml -import com.intellij.configurationStore.* -import com.intellij.openapi.util.io.FileUtilRt +import com.intellij.configurationStore.StoredPropertyStateTest +import com.intellij.configurationStore.clearBindingCache +import com.intellij.configurationStore.deserialize +import com.intellij.configurationStore.serialize import com.intellij.openapi.util.text.StringUtil import com.intellij.testFramework.UsefulTestCase import com.intellij.testFramework.assertConcurrent import com.intellij.testFramework.assertions.Assertions.assertThat -import com.intellij.util.SystemProperties import com.intellij.util.loadElement import com.intellij.util.xmlb.* import com.intellij.util.xmlb.annotations.* -import com.intellij.util.xmlb.annotations.Property import junit.framework.TestCase -import org.assertj.core.api.Assertions.assertThatThrownBy import org.intellij.lang.annotations.Language import org.jdom.Element import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.Suite -import java.io.StringWriter import java.util.* @RunWith(Suite::class) @@ -34,7 +32,8 @@ import java.util.* KotlinXmlSerializerTest::class, XmlSerializerConversionTest::class, XmlSerializerListTest::class, - XmlSerializerSetTest::class + XmlSerializerSetTest::class, + ForbidSensitiveInformationTest::class ) class XmlSerializerTestSuite @@ -650,85 +649,6 @@ internal class XmlSerializerTest { testSerializer("", bean, SkipDefaultsSerializationFilter()) } - @Test - fun `do not store password as attribute`() { - @Tag("bean") - class Bean { - @Attribute - var password: String? = null - - @Attribute - var foo: String? = null - } - - val bean = Bean() - bean.foo = "module" - bean.password = "ab" - // it is not part of XML bindings to ensure that even if you will use JDOM directly, you cannot output sensitive data - // so, testSerializer must not throw error - val element = assertSerializer(bean, "") - - assertThatThrownBy { - val xmlWriter = JbXmlOutputter() - xmlWriter.output(element, StringWriter()) - }.hasMessage("Attribute \"password\" probably contains sensitive information") - } - - @Test - fun `do not store password as element`() { - @Tag("bean") - class Bean { - var password: String? = null - - @Attribute - var foo: String? = null - } - - val bean = Bean() - bean.foo = "module" - bean.password = "ab" - // it is not part of XML bindings to ensure that even if you will use JDOM directly, you cannot output sensitive data - // so, testSerializer must not throw error - val element = assertSerializer(bean, """ - - - """.trimIndent()) - - assertThatThrownBy { - val xmlWriter = JbXmlOutputter(storageFilePathForDebugPurposes = "${FileUtilRt.toSystemIndependentName(SystemProperties.getUserHome())}/foo/bar.xml") - xmlWriter.output(element, StringWriter()) - }.hasMessage("Element \"password\" probably contains sensitive information (file: ~/foo/bar.xml)") - } - - @Test - fun `configuration name with password word`() { - @Tag("bean") - class Bean { - @OptionTag(tag ="configuration", valueAttribute = "bar") - var password: String? = null - } - - val bean = Bean() - bean.password = "ab" - // it is not part of XML bindings to ensure that even if you will use JDOM directly, you cannot output sensitive data - // so, testSerializer must not throw error - val element = assertSerializer(bean, """ - - - - """.trimIndent()) - - val xmlWriter = JbXmlOutputter() - val stringWriter = StringWriter() - xmlWriter.output(element, stringWriter) - assertThat(stringWriter.toString()).isEqualTo(""" - - - - """.trimIndent()) - } - @Test fun cdataAfterNewLine() { @Tag("bean") diff --git a/platform/projectModel-impl/src/com/intellij/configurationStore/BaseXmlOutputter.kt b/platform/projectModel-impl/src/com/intellij/configurationStore/BaseXmlOutputter.kt index 20f2b3179d74..87c006cf7c61 100644 --- a/platform/projectModel-impl/src/com/intellij/configurationStore/BaseXmlOutputter.kt +++ b/platform/projectModel-impl/src/com/intellij/configurationStore/BaseXmlOutputter.kt @@ -9,9 +9,14 @@ import java.io.Writer abstract class BaseXmlOutputter(protected val lineSeparator: String) { companion object { fun isNameIndicatesSensitiveInformation(name: String): Boolean { - return name.contains("password") && !(name.contains("remember", ignoreCase = true) || - name.contains("keep", ignoreCase = true) || - name.contains("save", ignoreCase = true)) + if (name.contains("password")) { + val isRemember = name.contains("remember", ignoreCase = true) || + name.contains("keep", ignoreCase = true) || + name.contains("use", ignoreCase = true) || + name.contains("save", ignoreCase = true) + return !isRemember + } + return false } } diff --git a/platform/projectModel-impl/src/com/intellij/configurationStore/JbXmlOutputter.kt b/platform/projectModel-impl/src/com/intellij/configurationStore/JbXmlOutputter.kt index 43b68af893e8..645ef925b8d1 100644 --- a/platform/projectModel-impl/src/com/intellij/configurationStore/JbXmlOutputter.kt +++ b/platform/projectModel-impl/src/com/intellij/configurationStore/JbXmlOutputter.kt @@ -508,7 +508,7 @@ open class JbXmlOutputter @JvmOverloads constructor(lineSeparator: String = "\n" var name: String? = element.name @Suppress("SpellCheckingInspection") - if (BaseXmlOutputter.isNameIndicatesSensitiveInformation(name!!)) { + if (isNameIndicatesSensitiveInformation(name!!)) { logSensitiveInformationError(name, "Element") }