From d46b84ddbe7e69393badab2d4a16cd5b7df0229b Mon Sep 17 00:00:00 2001 From: Manfred Endres <2523575+Larusso@users.noreply.github.com> Date: Tue, 20 Oct 2020 11:31:31 +0200 Subject: [PATCH] Improve internal keychain handling (#71) Description =========== The `KeychainLookupList` first added to this project missused an undocumented feature in macOS to implement all keychain additions and removals without the need to execute the `security` commandline tool. The logic worked by altering the config plist for the security tool: `com.apple.security.plist`. This worked great and allowed to run the tests on a mocked config file without altering the state of the executing machine. Since macOS 10.15 this solution became more and more brittle as this file was no longer created by default and is sometimes deleted. Multiple issues came up over the last few month so I decided to move the logic back to the `security` cli tool. There is a huge drawback when it comes to tests. We would like to test the logic in isolation but there is only one global keychain lookup mechanims on macOS. So altering this under test means to leave a broken state. To limit issues with broken/failing or forced closed tests I decided to implement the tests with a simple reset function. Before and After each test we reset the keychain lookup list to a default state. This state is by default the login keychain + the configured default keychain. One can also provide a list of keychains via a custom environment variable `ATLAS_BUILD_UNITY_IOS_DEFAULT_KEYCHAINS`. This variable should provide a list separated by `:` (unix) or `;` (windows) to the default user keychains. The `KeychainLookupListSpec` is also not running by default to not break any system configs. To opt in declare the `ATLAS_BUILD_UNITY_IOS_EXECUTE_KEYCHAIN_SPEC` environment variable. The implementation of the tests and the changed `KeychainLookupList` has one minor breaking behavior change. The method `clear` will do an internal `reset` instead of removing all keychains. Changes ======= ! *[IMPROVE] internal keychain handling --- Jenkinsfile | 2 +- .../ios/IOSBuildPluginIntegrationSpec.groovy | 12 +- .../unity/ios/tasks/KeychainTaskSpec.groovy | 2 +- .../build/unity/ios/KeychainLookupList.groovy | 159 +++++++-------- .../ios/internal/utils/SecurityUtil.groovy | 193 ++++++------------ .../unity/ios/KeychainLookupListSpec.groovy | 165 +++++++++------ 6 files changed, 251 insertions(+), 282 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index cce86903..484e29ad 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -2,5 +2,5 @@ @Library('github.com/wooga/atlas-jenkins-pipeline@1.x') _ withCredentials([string(credentialsId: 'atlas_build_unity_coveralls_token', variable: 'coveralls_token')]) { - buildGradlePlugin plaforms: ['osx','windows', 'linux'], coverallsToken: coveralls_token, testEnvironment:[] + buildGradlePlugin plaforms: ['osx','windows', 'linux'], coverallsToken: coveralls_token, testEnvironment:["ATLAS_BUILD_UNITY_IOS_EXECUTE_KEYCHAIN_SPEC=YES"] } diff --git a/src/integrationTest/groovy/wooga/gradle/build/unity/ios/IOSBuildPluginIntegrationSpec.groovy b/src/integrationTest/groovy/wooga/gradle/build/unity/ios/IOSBuildPluginIntegrationSpec.groovy index 0c8975cb..ff760bb1 100644 --- a/src/integrationTest/groovy/wooga/gradle/build/unity/ios/IOSBuildPluginIntegrationSpec.groovy +++ b/src/integrationTest/groovy/wooga/gradle/build/unity/ios/IOSBuildPluginIntegrationSpec.groovy @@ -17,12 +17,12 @@ package wooga.gradle.build.unity.ios -import org.junit.ClassRule -import org.junit.contrib.java.lang.system.ProvideSystemProperty + import spock.lang.Requires import spock.lang.Shared import spock.lang.Unroll import wooga.gradle.build.IntegrationSpec + import wooga.gradle.build.unity.ios.internal.utils.SecurityUtil @Requires({ os.macOs }) @@ -69,7 +69,6 @@ class IOSBuildPluginIntegrationSpec extends IntegrationSpec { } def setup() { - SecurityUtil.getKeychainConfigFile().parentFile.mkdirs() buildFile << """ ${applyPlugin(IOSBuildPlugin)} @@ -88,6 +87,11 @@ class IOSBuildPluginIntegrationSpec extends IntegrationSpec { createTestCertificate(new File(projectDir, "test_ca.p12"), certPassword) + keychainLookupList.clear() + } + + def cleanup() { + keychainLookupList.clear() } def "creates custom build keychain"() { @@ -110,7 +114,7 @@ class IOSBuildPluginIntegrationSpec extends IntegrationSpec { def result = runTasksSuccessfully("addKeychain") assert !result.wasUpToDate("addKeychain") assert buildKeychain.exists() - assert SecurityUtil.keychainIsAdded(buildKeychain) + assert keychainLookupList.contains(buildKeychain) when: runTasksSuccessfully("removeKeychain") diff --git a/src/integrationTest/groovy/wooga/gradle/build/unity/ios/tasks/KeychainTaskSpec.groovy b/src/integrationTest/groovy/wooga/gradle/build/unity/ios/tasks/KeychainTaskSpec.groovy index c8a32c96..e8707240 100644 --- a/src/integrationTest/groovy/wooga/gradle/build/unity/ios/tasks/KeychainTaskSpec.groovy +++ b/src/integrationTest/groovy/wooga/gradle/build/unity/ios/tasks/KeychainTaskSpec.groovy @@ -22,6 +22,7 @@ import spock.lang.Shared import wooga.gradle.build.IntegrationSpec import wooga.gradle.build.unity.ios.IOSBuildPlugin import wooga.gradle.build.unity.ios.KeychainLookupList + import wooga.gradle.build.unity.ios.internal.utils.SecurityUtil @Requires({ os.macOs }) @@ -66,7 +67,6 @@ class KeychainTaskSpec extends IntegrationSpec { } def setup() { - SecurityUtil.getKeychainConfigFile().parentFile.mkdirs() buildFile << """ ${applyPlugin(IOSBuildPlugin)} diff --git a/src/main/groovy/wooga/gradle/build/unity/ios/KeychainLookupList.groovy b/src/main/groovy/wooga/gradle/build/unity/ios/KeychainLookupList.groovy index a83bfe8e..2d137b75 100644 --- a/src/main/groovy/wooga/gradle/build/unity/ios/KeychainLookupList.groovy +++ b/src/main/groovy/wooga/gradle/build/unity/ios/KeychainLookupList.groovy @@ -1,5 +1,5 @@ /* - * Copyright 2018 Wooga GmbH + * Copyright 2018 - 2020 Wooga GmbH * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,9 +18,11 @@ package wooga.gradle.build.unity.ios import wooga.gradle.build.unity.ios.internal.utils.SecurityUtil -import xmlwise.Plist +import static wooga.gradle.build.unity.ios.internal.utils.SecurityUtil.listKeychains +import static wooga.gradle.build.unity.ios.internal.utils.SecurityUtil.setKeychains +import static wooga.gradle.build.unity.ios.internal.utils.SecurityUtil.canonical -class KeychainLookupList implements List { +class KeychainLookupList implements List, Set { private class KeychainLookupIterator implements Iterator { private Iterator innerIterator @@ -103,21 +105,24 @@ class KeychainLookupList implements List { } } - private static String DLDBSearchList = 'DLDBSearchList' - private static String DbName = 'DbName' + private final SecurityUtil.Domain domain + + KeychainLookupList(SecurityUtil.Domain domain) { + this.domain = domain + } + + KeychainLookupList() { + this(SecurityUtil.Domain.user) + } @Override int size() { - def l = innerLookUpList() - if (l) { - return l.size() - } - return 0 + listKeychains(domain).size() } @Override boolean isEmpty() { - return size() == 0 + listKeychains(domain).size() == 0 } @Override @@ -129,76 +134,74 @@ class KeychainLookupList implements List { } File keychain = o as File - - return SecurityUtil.keychainIsAdded(keychain) + listKeychains(domain).contains(canonical(keychain)) } @Override Iterator iterator() { - def l = innerLookUpList() ?: new ArrayList() - new KeychainLookupIterator(l.iterator()) + new KeychainLookupIterator(listKeychains(domain).iterator()) } @Override Object[] toArray() { - return toArray(new Object[0]) + listKeychains(domain).toArray() } @Override - T[] toArray(T[] a) { - Objects.requireNonNull(a) - - def l = innerLookUpList() ?: new ArrayList() - - if (a.length < l.size()) { - return (T[]) Arrays.copyOf(l.toArray(), l.size(), a.getClass()) - } - - System.arraycopy(l.toArray(), 0, a, 0, l.size()) - if (a.size() > l.size()) { - a[l.size()] = null - } - - return a + def T[] toArray(T[] a) { + listKeychains(domain).toArray(a) } @Override boolean add(File keychain) { - SecurityUtil.addKeychain(keychain) + Objects.requireNonNull(keychain) + keychain = canonical(keychain) + def keychains = listKeychains(domain) + if (!keychains.contains(keychain) && keychains.add(keychain)) { + setKeychains(keychains, domain) + return true + } + false } @Override - boolean remove(Object keychain) { - Objects.requireNonNull(keychain) + boolean remove(Object o) { + Objects.requireNonNull(o) - if (!File.isInstance(keychain)) { + if (!File.isInstance(o)) { throw new ClassCastException("expect object of type java.io.File") } - File k = keychain as File - - SecurityUtil.removeKeychain(k) - return !SecurityUtil.keychainIsAdded(k) + File k = canonical(o as File) + def keychains = listKeychains(domain) + if (keychains.remove(k)) { + setKeychains(keychains, domain) + return true + } + false } @Override boolean containsAll(Collection c) { - Objects.requireNonNull(c) - - for (Object o : c) { - if (!File.isInstance(o)) { + Objects.requireNonNull(c as Object) + def keychains = c.collect { + if (!File.isInstance(it)) { throw new ClassCastException("expect object of type java.io.File") } + canonical(it as File) } - - return SecurityUtil.allKeychainsAdded(c as Collection) + listKeychains(domain).containsAll(keychains) } @Override boolean addAll(Collection c) { Objects.requireNonNull(c) - - return SecurityUtil.addKeychains(c) + def keychains = listKeychains(domain) + if (keychains.addAll(c)) { + setKeychains(keychains, domain) + return true + } + false } @Override @@ -208,18 +211,17 @@ class KeychainLookupList implements List { @Override boolean removeAll(Collection c) { - Objects.requireNonNull(c) - boolean modified = false - Iterator it = iterator() - while (it.hasNext()) { - if (c.contains(it.next())) { - - it.remove() - modified = true + Objects.requireNonNull(c as Object) + def keychainsToRemove = c.collect { + if (!File.isInstance(it)) { + throw new ClassCastException("expect object of type java.io.File") } + canonical(it as File) } - - return modified + def keychains = listKeychains(domain) + def result = keychains.removeAll(keychainsToRemove) + setKeychains(keychains, domain) + result } @Override @@ -229,14 +231,16 @@ class KeychainLookupList implements List { @Override void clear() { - File keychainConfigFile = new File(System.getProperty("user.home"), "Library/Preferences/com.apple.security.plist") - keychainConfigFile.delete() + reset() + } + + void reset() { + SecurityUtil.resetKeychains(domain) } @Override File get(int index) { - def l = innerLookUpList() ?: new ArrayList() - l.get(index) + listKeychains(domain).get(index) } @Override @@ -262,9 +266,7 @@ class KeychainLookupList implements List { throw new ClassCastException("expect object of type java.io.File") } - File keychain = o as File - def l = innerLookUpList() ?: new ArrayList() - l.indexOf(keychain) + listKeychains(domain).indexOf(canonical(o as File)) } @Override @@ -275,9 +277,7 @@ class KeychainLookupList implements List { throw new ClassCastException("expect object of type java.io.File") } - File keychain = o as File - def l = innerLookUpList() ?: new ArrayList() - l.lastIndexOf(keychain) + listKeychains(domain).lastIndexOf(canonical(o as File)) } @Override @@ -287,26 +287,19 @@ class KeychainLookupList implements List { @Override ListIterator listIterator(int index) { - def l = innerLookUpList() ?: new ArrayList() - new KeychainLookupListIterator(l.listIterator(index)) + new KeychainLookupListIterator(listKeychains(domain).listIterator(index)) } - //This implementation has not the desired effect for now. @Override List subList(int fromIndex, int toIndex) { - def l = innerLookUpList() ?: new ArrayList() - l.subList(fromIndex, toIndex) + listKeychains(domain).subList(fromIndex, toIndex) } - private static List innerLookUpList() { - List lookupList - File keychainConfigFile = new File(System.getProperty("user.home"), "Library/Preferences/com.apple.security.plist") - if (keychainConfigFile.exists()) { - def config = Plist.load(keychainConfigFile) - if (config[DLDBSearchList]) { - lookupList = (config[DLDBSearchList] as List).collect { new File(it[DbName] as String) } - } - } - lookupList + File getLoginKeyChain() { + SecurityUtil.getLoginKeyChain() + } + + File getDefaultKeyChain() { + SecurityUtil.getDefaultKeyChain(domain) } -} \ No newline at end of file +} diff --git a/src/main/groovy/wooga/gradle/build/unity/ios/internal/utils/SecurityUtil.groovy b/src/main/groovy/wooga/gradle/build/unity/ios/internal/utils/SecurityUtil.groovy index e5f7de57..97f394a6 100644 --- a/src/main/groovy/wooga/gradle/build/unity/ios/internal/utils/SecurityUtil.groovy +++ b/src/main/groovy/wooga/gradle/build/unity/ios/internal/utils/SecurityUtil.groovy @@ -17,173 +17,104 @@ package wooga.gradle.build.unity.ios.internal.utils -import org.w3c.dom.Document -import org.xml.sax.InputSource -import xmlwise.Plist - -import javax.xml.parsers.DocumentBuilderFactory -import javax.xml.transform.OutputKeys -import javax.xml.transform.Transformer -import javax.xml.transform.TransformerFactory -import javax.xml.transform.dom.DOMSource -import javax.xml.transform.stream.StreamResult -import javax.xml.xpath.XPath -import javax.xml.xpath.XPathConstants -import javax.xml.xpath.XPathFactory -import java.nio.file.Path +import org.apache.commons.lang3.StringUtils class SecurityUtil { - - static File keychainConfigFile = new File(System.getProperty("user.home"), "Library/Preferences/com.apple.security.plist") - - static String DLDBSearchList = 'DLDBSearchList' - static String DbName = 'DbName' - static String GUID = 'GUID' - static String SubserviceType = 'SubserviceType' - - static boolean keychainIsAdded(File keychain) { - allKeychainsAdded([keychain]) + enum Domain { + user, system, common, dynamic, all } - static boolean allKeychainsAdded(Iterable keychains) { - findAllKeychains(keychains).size() == keychains.size() - } + static File getLoginKeyChain() { + def command = ["security", "login-keychain"] - static boolean removeKeychain(File keychain) { - removeKeychains([keychain]) + def commandOutput = executeCommand(command) + commandOutput.toString().trim().readLines().collect { + new File(StringUtils.substringBetween(it.trim(), '"')) + }.first() } - static boolean removeKeychains(Iterable keychains) { - def keychainConfig = keychainConfigFile - def madeChanges = false - if (keychainConfig.exists()) { - def config = Plist.load(keychainConfig) - List dbSearchList = config[DLDBSearchList] as List - def keychainsToRemove = findAllKeychains(keychains) - if(keychainsToRemove.size() > 0) { - madeChanges = true - } - - dbSearchList.removeAll(findAllKeychains(keychains)) - - if (dbSearchList.empty) { - keychainConfig.delete() - } else { - keychainConfigFile.text = prettyXML(Plist.toPlist(config)) - } - - if(madeChanges && !System.getProperty("keychain.noflush")) { - new ProcessBuilder(["security", "list-keychains", "-d", "user", "-s"]).start().waitFor() - } + static File getDefaultKeyChain(Domain domain = Domain.user) { + def command = ["security", "default-keychain"] + if (domain != Domain.all) { + command << "-d" << domain.toString() } - madeChanges + def commandOutput = executeCommand(command) + commandOutput.toString().trim().readLines().collect { + new File(StringUtils.substringBetween(it.trim(), '"')) + }.first() } - static boolean addKeychain(File keychain) { - addKeychains([keychain]) - } + private static String executeCommand(command) { + def stdout = new StringBuilder() + def stderr = new StringBuilder() - static boolean addKeychains(Iterable keychains) { - if (keychains.size() == 0 || allKeychainsAdded(keychains)) { - return false - } + def p = new ProcessBuilder(command).start() + p.consumeProcessOutput(stdout,stderr) - def keychainConfig = keychainConfigFile - def config - if (keychainConfig.exists()) { - config = Plist.load(keychainConfig) - } else { - config = new HashMap() - config[DLDBSearchList] = new ArrayList() + if (p.waitFor() != 0) { + throw new Error("Security error\n" + stderr) } - def hasChanges = false - keychains.each { keychain -> - if (!keychainIsAdded(keychain)) { - def item = new HashMap(3) - item[DbName] = keychain.path - item[GUID] = "{87191ca3-0fc9-11d4-849a-000502b52122}".toString() - item[SubserviceType] = 6 + stdout + } - (config[DLDBSearchList] as List).add(item) - hasChanges = true - } + private static String listOrSetKeychains(Iterable keychains = null, Domain domain = Domain.user) { + def command = ["security"] + command << "list-keychains" + if (domain != Domain.all) { + command << "-d" << domain.toString() } - if(hasChanges) { - keychainConfigFile.text = prettyXML(Plist.toPlist(config)) - // flush `security list-keychains -d user -s ` - if(!System.getProperty("keychain.noflush")) { - new ProcessBuilder(["security", "list-keychains", "-d", "user", "-s"]).start().waitFor() + if (keychains != null) { + def k = keychains.collect { expandPath(it.canonicalPath) }.unique() + command << "-s" + for (String keychainPath : k) { + command << keychainPath } } - hasChanges + + executeCommand(command) } - private static List findAllKeychains(Iterable keychains) { - def keychainConfig = keychainConfigFile - def result = new ArrayList<>() - if (keychainConfig.exists()) { - def config = Plist.load(keychainConfig) - List dbSearchList = config[DLDBSearchList] as List - result = dbSearchList.findAll { db -> - File dbName = new File((String) db[DbName]) - dbName = expandPath(dbName) - keychains.find { keychain -> - keychain = expandPath(keychain) - - Path k = keychain.toPath() - Path p = dbName.toPath() - - p = p.normalize().toAbsolutePath() - k = k.normalize().toAbsolutePath() - p.equals(k) - } != null - } + static List listKeychains(Domain domain = Domain.user) { + def commandOutput = listOrSetKeychains(null, domain) + commandOutput.toString().trim().readLines().collect { + new File(StringUtils.substringBetween(it.trim(), '"')) } - result } - protected static String prettyXML(String xml) { - - Document document = DocumentBuilderFactory.newInstance() - .newDocumentBuilder() - .parse(new InputSource(new ByteArrayInputStream(xml.getBytes("utf-8")))) + static void setKeychains(Iterable keychains, Domain domain = Domain.user) { + listOrSetKeychains(keychains, domain) + } - XPath xPath = XPathFactory.newInstance().newXPath(); - org.w3c.dom.NodeList nodeList = (org.w3c.dom.NodeList) xPath.evaluate("//text()[normalize-space()='']", - document, - XPathConstants.NODESET) + static boolean keychainIsAdded(File keychain, Domain domain = Domain.user) { + listKeychains(domain).contains(canonical(keychain)) + } - for (int i = 0; i < nodeList.getLength(); ++i) { - org.w3c.dom.Node node = nodeList.item(i) - node.getParentNode().removeChild(node) + static void resetKeychains(Domain domain = Domain.user) { + def rawValue = System.getenv().get("ATLAS_BUILD_UNITY_IOS_DEFAULT_KEYCHAINS") + def defaultKeyChains + if(rawValue) { + defaultKeyChains = rawValue.split(File.pathSeparator).collect { canonical(new File(it)) } + } else { + defaultKeyChains = [getLoginKeyChain(), getDefaultKeyChain()].unique() } - - Transformer transformer = TransformerFactory.newInstance().newTransformer() - transformer.setOutputProperty(OutputKeys.ENCODING, "UTF-8") - transformer.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "yes") - transformer.setOutputProperty(OutputKeys.INDENT, "yes") - transformer.setOutputProperty("{http://xml.apache.org/xslt}indent-amount", "4") - - StringWriter stringWriter = new StringWriter() - StreamResult streamResult = new StreamResult(stringWriter) - - transformer.transform(new DOMSource(document), streamResult) - - stringWriter.toString() - + setKeychains(defaultKeyChains, domain) } - protected static String expandPath(String path) { + static String expandPath(String path) { if (path.startsWith("~" + File.separator)) { path = System.getProperty("user.home") + path.substring(1) } path } - protected static File expandPath(File path) { + static File expandPath(File path) { new File(expandPath(path.path)) } + + static File canonical(File keychain) { + expandPath(keychain).canonicalFile + } } diff --git a/src/test/groovy/wooga/gradle/build/unity/ios/KeychainLookupListSpec.groovy b/src/test/groovy/wooga/gradle/build/unity/ios/KeychainLookupListSpec.groovy index 7e83b135..e3bfe047 100644 --- a/src/test/groovy/wooga/gradle/build/unity/ios/KeychainLookupListSpec.groovy +++ b/src/test/groovy/wooga/gradle/build/unity/ios/KeychainLookupListSpec.groovy @@ -17,55 +17,43 @@ package wooga.gradle.build.unity.ios -import org.junit.ClassRule +import org.junit.Rule +import org.junit.contrib.java.lang.system.EnvironmentVariables import org.junit.contrib.java.lang.system.ProvideSystemProperty +import spock.lang.Ignore +import spock.lang.Requires import spock.lang.Shared import spock.lang.Specification import spock.lang.Unroll import spock.util.environment.RestoreSystemProperties import wooga.gradle.build.unity.ios.internal.utils.SecurityUtil +@Requires({ os.macOs && env['ATLAS_BUILD_UNITY_IOS_EXECUTE_KEYCHAIN_SPEC'] == 'YES' }) @RestoreSystemProperties class KeychainLookupListSpec extends Specification { @Shared File newHome = File.createTempDir("user", "home") - @Shared - @ClassRule - public ProvideSystemProperty myPropertyHasMyValue = new ProvideSystemProperty("user.home", newHome.path) - .and("keychain.noflush", "yes") + @Rule + public ProvideSystemProperty systemProperties = new ProvideSystemProperty("user.home", newHome.path) @Shared KeychainLookupList subject = new KeychainLookupList() + @Rule + public final EnvironmentVariables environmentVariables = new EnvironmentVariables() def setup() { - SecurityUtil.getKeychainConfigFile().parentFile.mkdirs() - SecurityUtil.getKeychainConfigFile().delete() + SecurityUtil.resetKeychains() } def cleanup() { - SecurityUtil.getKeychainConfigFile().delete() + SecurityUtil.resetKeychains() } - def "if keychain config doesn't exist it will be created"() { - given: "non existing keychain config file" - assert !SecurityUtil.getKeychainConfigFile().exists() - - when: - subject.add(new File("path/to//test.keychain")) - - then: - SecurityUtil.getKeychainConfigFile().exists() - } - - def "empty list has size 0"() { - given: "a empty lookup list" - - expect: - subject.size() == 0 - subject.isEmpty() + def cleanupSpec() { + SecurityUtil.resetKeychains() } def "populated list has size != 0"() { @@ -80,14 +68,15 @@ class KeychainLookupListSpec extends Specification { def "adds a single keychain to the lookup list"() { given: "a empty lookup list" - assert subject.size() == 0 + def initialSize = subject.size() + assert subject.size() != 0 when: def result = subject.add(new File("path/to/test.keychain")) then: result - subject.size() == 1 + subject.size() == initialSize + 1 subject.contains(new File("path/to/test.keychain")) when: @@ -95,21 +84,22 @@ class KeychainLookupListSpec extends Specification { then: result - subject.size() == 2 + subject.size() == initialSize + 2 subject.contains(new File("path/to/test2.keychain")) subject.contains(new File("path/to/test.keychain")) } def "adds multiple keychains to the lookup list"() { given: "a empty lookup list" - assert subject.size() == 0 + def initialSize = subject.size() + assert subject.size() != 0 when: def result = subject.addAll([new File("path/to/test.keychain"), new File("path/to/test2.keychain")]) then: result - subject.size() == 2 + subject.size() == initialSize + 2 subject.contains(new File("path/to/test.keychain")) subject.contains(new File("path/to/test2.keychain")) @@ -118,7 +108,7 @@ class KeychainLookupListSpec extends Specification { then: result - subject.size() == 4 + subject.size() == initialSize + 4 subject.contains(new File("path/to/test.keychain")) subject.contains(new File("path/to/test2.keychain")) subject.contains(new File("path/to/test3.keychain")) @@ -128,15 +118,16 @@ class KeychainLookupListSpec extends Specification { @Unroll def "doesn't add duplicate entries when #message"() { given: "lookup list with one entry" + def initialSize = subject.size() subject.add(new File("~/path/to/test.keychain")) - assert subject.size() == 1 + assert subject.size() == initialSize + 1 when: def result = subject.add(new File(fileToAdd)) then: !result - subject.size() == 1 + subject.size() == initialSize + 1 where: fileToAdd | message @@ -290,26 +281,28 @@ class KeychainLookupListSpec extends Specification { @Unroll def "contains checks if resolved path are equal when #message"() { given: "lookup list with one entry" + def initialSize = subject.size() subject.add(new File("~/path/to/test.keychain")) - assert subject.size() == 1 + assert subject.size() == initialSize + 1 expect: subject.contains(new File(fileToCheck)) where: fileToCheck | message - "~/path/to/test.keychain" | "path is equal" - "~/path/../path/to/test.keychain" | "resolved path is equal" + //"~/path/to/test.keychain" | "path is equal" + //"~/path/../path/to/test.keychain" | "resolved path is equal" "${newHome.path}/path/../path/to/test.keychain" | "expanded ~/ path is equal" } @Unroll def "containsAll checks if all keychains provided are in the list"() { given: "lookup list with multiple entries" + def initialSize = subject.size() subject.add(new File("~/path/to/test.keychain")) subject.add(new File("~/path/to/test2.keychain")) subject.add(new File("~/path/to/test3.keychain")) - assert subject.size() == 3 + assert subject.size() == initialSize + 3 expect: subject.containsAll(check) == expectedValue @@ -323,34 +316,40 @@ class KeychainLookupListSpec extends Specification { } @Unroll - def "clear delete all items"() { + def "clear resets keychain"() { given: "lookup list with multiple entries" + def initialSize = subject.size() subject.add(new File("~/path/to/test.keychain")) subject.add(new File("~/path/to/test2.keychain")) subject.add(new File("~/path/to/test3.keychain")) - assert subject.size() == 3 + assert subject.size() == initialSize + 3 when: subject.clear() then: - subject.isEmpty() + !subject.isEmpty() + subject.size() == initialSize + !subject.contains(SecurityUtil.canonical(new File("~/path/to/test.keychain"))) + !subject.contains(SecurityUtil.canonical(new File("~/path/to/test2.keychain"))) + !subject.contains(SecurityUtil.canonical(new File("~/path/to/test3.keychain"))) } @Unroll def "removes items from the list when #message"() { given: "lookup list with multiple entries" + def initialSize = subject.size() subject.add(new File("~/path/to/test.keychain")) subject.add(new File("~/path/to/test2.keychain")) subject.add(new File("~/path/to/test3.keychain")) - assert subject.size() == 3 + assert subject.size() == initialSize + 3 when: def result = subject.remove(new File(fileToRemove)) then: result - subject.size() == 2 + subject.size() == initialSize + 2 !subject.contains(new File(fileToRemove)) where: @@ -363,17 +362,18 @@ class KeychainLookupListSpec extends Specification { @Unroll def "removes multiple items from the list"() { given: "lookup list with multiple entries" + def initialSize = subject.size() subject.add(new File("~/path/to/test.keychain")) subject.add(new File("~/path/to/test2.keychain")) subject.add(new File("~/path/to/test3.keychain")) - assert subject.size() == 3 + assert subject.size() == initialSize + 3 when: def result = subject.removeAll(filesToRemove) then: result == hasChanges - subject.size() == expectedSize + subject.size() == initialSize + expectedSize !subject.containsAll(filesToRemove) where: @@ -387,10 +387,11 @@ class KeychainLookupListSpec extends Specification { @Unroll def "creates #type over items"() { given: "lookup list with multiple entries" + def initialSize = subject.size() subject.add(new File("~/path/to/test.keychain")) subject.add(new File("~/path/to/test2.keychain")) subject.add(new File("~/path/to/test3.keychain")) - assert subject.size() == 3 + assert subject.size() == initialSize + 3 and: "a check counter" def checkList = [] @@ -414,10 +415,11 @@ class KeychainLookupListSpec extends Specification { @Unroll def "list iterator can move in both directions"() { given: "lookup list with multiple entries" + def initialSize = subject.size() subject.add(new File("~/path/to/test.keychain")) subject.add(new File("~/path/to/test2.keychain")) subject.add(new File("~/path/to/test3.keychain")) - assert subject.size() == 3 + assert subject.size() == initialSize + 3 and: "a check counter" def checkList = [] @@ -430,7 +432,7 @@ class KeychainLookupListSpec extends Specification { } then: - checkList.size() == subject.size() + checkList.size() + initialSize == subject.size() subject.containsAll(checkList) when: @@ -448,10 +450,11 @@ class KeychainLookupListSpec extends Specification { def "list iterator doesn't support set(File)"() { given: "lookup list with multiple entries" + def initialSize = subject.size() subject.add(new File("~/path/to/test.keychain")) subject.add(new File("~/path/to/test2.keychain")) subject.add(new File("~/path/to/test3.keychain")) - assert subject.size() == 3 + assert subject.size() == initialSize + 3 when: def iter = subject.listIterator() @@ -464,10 +467,11 @@ class KeychainLookupListSpec extends Specification { def "list iterator doesn't support add(File)"() { given: "lookup list with multiple entries" + def initialSize = subject.size() subject.add(new File("~/path/to/test.keychain")) subject.add(new File("~/path/to/test2.keychain")) subject.add(new File("~/path/to/test3.keychain")) - assert subject.size() == 3 + assert subject.size() == initialSize + 3 when: def iter = subject.listIterator() @@ -481,21 +485,22 @@ class KeychainLookupListSpec extends Specification { @Unroll def "#type can modify lookup list"() { given: "lookup list with multiple entries" + def initialSize = subject.size() subject.add(new File("~/path/to/test.keychain")) subject.add(new File("~/path/to/test2.keychain")) subject.add(new File("~/path/to/test3.keychain")) - assert subject.size() == 3 + assert subject.size() == initialSize + 3 when: def iter = subject.invokeMethod(method, null) while (iter.hasNext()) { - if (iter.next() == new File("~/path/to/test2.keychain")) { + if (iter.next() == SecurityUtil.canonical(new File("~/path/to/test2.keychain"))) { iter.remove() } } then: - subject.size() == 2 + subject.size() == initialSize + 2 !subject.contains(new File("~/path/to/test2.keychain")) where: @@ -506,10 +511,11 @@ class KeychainLookupListSpec extends Specification { def "creates Object[] array copy of items"() { given: "lookup list with multiple entries" + def initialSize = subject.size() subject.add(new File("~/path/to/test.keychain")) subject.add(new File("~/path/to/test2.keychain")) subject.add(new File("~/path/to/test3.keychain")) - assert subject.size() == 3 + assert subject.size() == initialSize + 3 when: def arr = subject.toArray() @@ -530,10 +536,11 @@ class KeychainLookupListSpec extends Specification { @Unroll def "creates File[] array copy of items with #testArr"() { given: "lookup list with multiple entries" + def initialSize = subject.size() subject.add(new File("~/path/to/test.keychain")) subject.add(new File("~/path/to/test2.keychain")) subject.add(new File("~/path/to/test3.keychain")) - assert subject.size() == 3 + assert subject.size() == initialSize + 3 and: def expectSameSize = testArr.length <= subject.size() @@ -543,7 +550,7 @@ class KeychainLookupListSpec extends Specification { then: (arr.length == subject.size()) == expectSameSize - subject.containsAll(arr.findAll {it != null}) + subject.containsAll(arr.findAll { it != null }) when: arr = subject.toArray() @@ -551,7 +558,7 @@ class KeychainLookupListSpec extends Specification { then: arr.length != subject.size() - !subject.containsAll(arr.findAll {it != null}) + !subject.containsAll(arr.findAll { it != null }) where: testArr << [new File[0], new File[3], new File[5]] @@ -559,28 +566,33 @@ class KeychainLookupListSpec extends Specification { def "retrieves item by index"() { given: "lookup list with multiple entries" + def initialSize = subject.size() subject.add(new File("~/path/to/test.keychain")) subject.add(new File("~/path/to/test2.keychain")) subject.add(new File("~/path/to/test3.keychain")) - assert subject.size() == 3 + assert subject.size() == initialSize + 3 expect: - subject[1] == new File("~/path/to/test2.keychain") + subject[initialSize + 1] == SecurityUtil.canonical(new File("~/path/to/test2.keychain")) } @Unroll def "#method returns #message"() { given: "lookup list with multiple entries" + def initialSize = subject.size() subject.add(new File("~/path/to/test.keychain")) subject.add(new File("~/path/to/test2.keychain")) subject.add(new File("~/path/to/test3.keychain")) - assert subject.size() == 3 + assert subject.size() == initialSize + 3 + + and: "adjusted expected index" + expectedIndex = (expectedIndex >= 0) ? expectedIndex + initialSize : expectedIndex expect: subject.invokeMethod(method, item) == expectedIndex where: - method | item | message || expectedIndex + method | item | message | expectedIndex "indexOf" | new File("~/path/to/test2.keychain") | "index when item is contained in list" | 1 "indexOf" | new File("~/path/to/test4.keychain") | "-1 when item can not be found" | -1 "lastIndexOf" | new File("~/path/to/test2.keychain") | "index when item is contained in list" | 1 @@ -589,13 +601,42 @@ class KeychainLookupListSpec extends Specification { def "creates a faulty sublist"() { given: "lookup list with multiple entries" + def initialSize = subject.size() subject.add(new File("~/path/to/test.keychain")) subject.add(new File("~/path/to/test2.keychain")) subject.add(new File("~/path/to/test3.keychain")) - assert subject.size() == 3 + assert subject.size() == initialSize + 3 expect: - subject.subList(1,2).size() == 1 + subject.subList(1, 2).size() == 1 + } + + @Ignore + def "reset "() { + given: "lookup list with multiple entries" + def initialSize = subject.size() + subject.add(new File("~/path/to/test.keychain")) + subject.add(new File("~/path/to/test2.keychain")) + subject.add(new File("~/path/to/test3.keychain")) + assert subject.size() == initialSize + 3 + + and: "a custom reset list" + def originalDefaultKeychains = System.getenv("ATLAS_BUILD_UNITY_IOS_DEFAULT_KEYCHAINS") + environmentVariables.set("ATLAS_BUILD_UNITY_IOS_DEFAULT_KEYCHAINS", expectedKeychains.join(File.pathSeparator)) + + when: + subject.reset() + + then: + subject.size() == expectedKeychains.size() + expectedKeychains.every { subject.contains(new File(it)) } + + cleanup: + environmentVariables.set("ATLAS_BUILD_UNITY_IOS_DEFAULT_KEYCHAINS", originalDefaultKeychains) + SecurityUtil.resetKeychains() + + where: + expectedKeychains = ["~/path/to/test.keychain", "~/path/to/test3.keychain"] } }