From ac3cd3eb217485666596b6f5f4180dbb5d4be099 Mon Sep 17 00:00:00 2001 From: Wadeck Follonier Date: Sat, 28 Oct 2017 23:01:50 +0200 Subject: [PATCH] [JENKINS-47426] getPropertyKey is not consistent over time (#3080) * JENKINS-47426: getPropertyKey is not consistent over time - add passing test for the case the rootUrl is set - add failing test for the case the rootUrl is not set yet, the propertyKey is not consistent - add a warning message when the flow passes in the fallback mode * - fixing the getEncryptedValue using a static memory * - add issue annotations * - modifications proposed by Jesse: use legacyId instead of randomId, stable over restart * - small correction for a test case --- .../hudson/cli/ClientAuthenticationCache.java | 9 ++++++--- .../cli/ClientAuthenticationCacheTest.java | 20 ++++++++++++++++++- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/hudson/cli/ClientAuthenticationCache.java b/core/src/main/java/hudson/cli/ClientAuthenticationCache.java index 05e80bc515..0441cc74ed 100644 --- a/core/src/main/java/hudson/cli/ClientAuthenticationCache.java +++ b/core/src/main/java/hudson/cli/ClientAuthenticationCache.java @@ -38,7 +38,7 @@ public class ClientAuthenticationCache implements Serializable { private static final HMACConfidentialKey MAC = new HMACConfidentialKey(ClientAuthenticationCache.class, "MAC"); private static final Logger LOGGER = Logger.getLogger(ClientAuthenticationCache.class.getName()); - + /** * Where the store should be placed. */ @@ -110,9 +110,12 @@ public class ClientAuthenticationCache implements Serializable { */ @VisibleForTesting String getPropertyKey() { - String url = Jenkins.getActiveInstance().getRootUrl(); + Jenkins j = Jenkins.getActiveInstance(); + String url = j.getRootUrl(); if (url!=null) return url; - return Secret.fromString("key").getEncryptedValue(); + + LOGGER.log(Level.WARNING, "The instance is not configured using a rootUrl, the key that represents your instance will not be stable"); + return j.getLegacyInstanceId(); } /** diff --git a/test/src/test/java/hudson/cli/ClientAuthenticationCacheTest.java b/test/src/test/java/hudson/cli/ClientAuthenticationCacheTest.java index 7f347c3974..6194485ab6 100644 --- a/test/src/test/java/hudson/cli/ClientAuthenticationCacheTest.java +++ b/test/src/test/java/hudson/cli/ClientAuthenticationCacheTest.java @@ -112,7 +112,25 @@ public class ClientAuthenticationCacheTest { assertEquals(r.getURL().toString(), cache.getPropertyKey()); JenkinsLocationConfiguration.get().setUrl(null); String key = cache.getPropertyKey(); - assertTrue(key, Secret.decrypt(key) != null); + assertEquals(r.jenkins.getLegacyInstanceId(), key); + } + + @Test + @Issue("JENKINS-47426") + public void getPropertyKey_mustBeEquivalentOverTime() throws Exception { + ClientAuthenticationCache cache = new ClientAuthenticationCache(null); + + String key1 = cache.getPropertyKey(); + String key2 = cache.getPropertyKey(); + + assertEquals("Two calls to the getPropertyKey() must be equivalent over time, with rootUrl", key1, key2); + + JenkinsLocationConfiguration.get().setUrl(null); + + key1 = cache.getPropertyKey(); + key2 = cache.getPropertyKey(); + + assertEquals("Two calls to the getPropertyKey() must be equivalent over time, without rootUrl", key1, key2); } private void assertCLI(int code, @CheckForNull String output, File jar, String... args) throws Exception { -- GitLab