microsoft / vsts-cordova-tasks

Streamline CI setup for your Apache Cordova, PhoneGap, Ionic, or Cordova CLI compatible app using a set of useful pre-defined build steps for VS Team Services or TFS
http://go.microsoft.com/fwlink/?LinkID=691188
Other
25 stars 27 forks source link

_tasktmp.keychain not properly removed from keychain search path if task fails to find a valid identity #35

Closed Chuxel closed 8 years ago

Chuxel commented 8 years ago

This was exposed by https://github.com/Microsoft/vsts-cordova-tasks/issues/34

In this case, a P12 file was valid, but the contents are expired. The code currently updates the keychain search path before looking inside the P12 to find a valid identity. Unfortunately the code is then failing to clean up the search path and delete the _temp keychain if no valid identity is found. This logic will need to be modified to handle invalid cert scenarios like this one.

You can tell this is happening by looking at the long list of keychains when this error occurs.

Ex:

46:33.340Z:     "/Users/vso112256/vso-agent/_work/2/s/tfs-vnext-test/_tasktmp.keychain"

56 
2016-02-16T19:46:33.340Z:     "/Users/vso112256/vso-agent/_work/2/s/tfs-vnext-test/_tasktmp.keychain"

57 
2016-02-16T19:46:33.340Z:     "/Users/vso112256/vso-agent/_work/9/s/tfs-vnext-test/_tasktmp.keychain"

58 
2016-02-16T19:46:33.340Z:     "/Users/vso112256/vso-agent/_work/9/s/tfs-vnext-test/_tasktmp.keychain"

59 
2016-02-16T19:46:33.340Z:     "/Users/vso112256/vso-agent/_work/9/s/tfs-vnext-test/_tasktmp.keychain"

60 
2016-02-16T19:46:33.340Z:     "/Users/vso112256/vso-agent/_work/9/s/tfs-vnext-test/_tasktmp.keychain"

61 
2016-02-16T19:46:33.341Z:     "/Users/vso112256/vso-agent/_work/9/s/tfs-vnext-test/_tasktmp.keychain"

62 
2016-02-16T19:46:33.341Z:     "/Users/vso112256/vso-agent/_work/9/s/tfs-vnext-test/_tasktmp.keychain"

63 
2016-02-16T19:46:33.341Z:     "/Users/vso112256/vso-agent/_work/9/s/tfs-vnext-test/_tasktmp.keychain"

64 
2016-02-16T19:46:33.341Z:     "/Users/vso112256/vso-agent/_work/9/s/tfs-vnext-test/_tasktmp.keychain"

65 
2016-02-16T19:46:33.341Z:     "/Users/vso112256/vso-agent/_work/8/s/tfs-vnext-test/_tasktmp.keychain"

66 
2016-02-16T19:46:33.341Z:     "/Users/vso112256/vso-agent/_work/6/s/_tasktmp.keychain"

67 
2016-02-16T19:46:33.341Z:     "/Users/vso112256/vso-agent/_work/9/s/tfs-vnext-test/_tasktmp.keychain"

68 
2016-02-16T19:46:33.341Z:     "/Users/vso112256/vso-agent/_work/9/s/tfs-vnext-test/_tasktmp.keychain"

69 
2016-02-16T19:46:33.341Z:     "/Users/vso112256/vso-agent/_work/9/s/tfs-vnext-test/_tasktmp.keychain"

70 
2016-02-16T19:46:33.342Z:     "/Users/vso112256/vso-agent/_work/9/s/tfs-vnext-test/_tasktmp.keychain"

71 
2016-02-16T19:46:33.342Z:     "/Users/vso112256/vso-agent/_work/9/s/tfs-vnext-test/_tasktmp.keychain"

72 
2016-02-16T19:46:33.342Z:     "/Users/vso112256/vso-agent/_work/8/s/tfs-vnext-test/_tasktmp.keychain"

73 
2016-02-16T19:46:33.342Z:     "/Users/vso112256/vso-agent/_work/6/s/_tasktmp.keychain"

74 
2016-02-16T19:46:33.342Z:     "/Users/vso112256/vso-agent/_work/8/s/tfs-vnext-test/_tasktmp.keychain"

75 
2016-02-16T19:46:33.342Z:     "/Users/vso112256/vso-agent/_work/9/s/tfs-vnext-test/_tasktmp.keychain"

76 
2016-02-16T19:46:33.342Z:     "/Users/vso112256/vso-agent/_work/2/s/tfs-vnext-test/_tasktmp.keychain"

77 
2016-02-16T19:46:33.342Z:     "/Users/vso112256/vso-agent/_work/3/s/_tasktmp.keychain"

78 
2016-02-16T19:46:33.342Z:     "/Users/vso112256/vso-agent/_work/3/s/_tasktmp.keychain"

79 
2016-02-16T19:46:33.342Z:     "/Users/vso112256/vso-agent/_work/build/73a624315c745cc6fa2abffd59813a3580d136a51860bb81ca943e36daf0d95b/repo/tfs-vnext-test/_tasktmp.keychain"

80 
2016-02-16T19:46:33.343Z:     "/Users/vso112256/vso-agent/_work/build/a29ba8b965d7cc0210d0c34e3bbd55a1b70754593d626db5cfe30793d43efa87/repo/tfs-vnext-test/_tasktmp.keychain"

81 
2016-02-16T19:46:33.343Z:     "/Users/vso112256/vso-agent/_work/build/a29ba8b965d7cc0210d0c34e3bbd55a1b70754593d626db5cfe30793d43efa87/repo/tfs-vnext-test/_tasktmp.keychain"
Chuxel commented 8 years ago

We should also improve error messaging in this case to be clear that the P12 cert did not contain a valid certificate and that is why it is failing.

ryuyu commented 8 years ago

Addressed with 1.3.0