fix: Remove stored inline creds on save when a connection is selected

octopus-server/src/main/java/octopus/teamcity/server/OctopusBuildInformationRunType.java

@@ -28,6 +28,7 @@
 import jetbrains.buildServer.util.StringUtil;
 import jetbrains.buildServer.web.openapi.PluginDescriptor;
 import octopus.teamcity.common.OctopusConstants;
+import octopus.teamcity.server.connection.ConnectionInlineFieldCleaner;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -85,6 +86,10 @@ public Collection<InvalidProperty> process(@Nullable final Map<String, String> p
         checkNotEmpty(p, c.getPackageIdKey(), "Package ID must be specified", result);
         checkNotEmpty(p, c.getPackageVersionKey(), "Package version be specified", result);
 
+        if (result.isEmpty()) {
+          ConnectionInlineFieldCleaner.stripInlineFieldsIfUsingConnection(p);
+        }
+
         return result;
       }
     };

octopus-server/src/main/java/octopus/teamcity/server/OctopusCreateReleaseRunType.java

@@ -12,6 +12,7 @@
 import jetbrains.buildServer.util.StringUtil;
 import jetbrains.buildServer.web.openapi.PluginDescriptor;
 import octopus.teamcity.common.OctopusConstants;
+import octopus.teamcity.server.connection.ConnectionInlineFieldCleaner;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -68,6 +69,10 @@ public Collection<InvalidProperty> process(@Nullable final Map<String, String> p
         }
         checkNotEmpty(p, c.getProjectNameKey(), "Project name must be specified", result);
 
+        if (result.isEmpty()) {
+          ConnectionInlineFieldCleaner.stripInlineFieldsIfUsingConnection(p);
+        }
+
         return result;
       }
     };

octopus-server/src/main/java/octopus/teamcity/server/OctopusDeployReleaseRunType.java

@@ -28,6 +28,7 @@
 import jetbrains.buildServer.util.StringUtil;
 import jetbrains.buildServer.web.openapi.PluginDescriptor;
 import octopus.teamcity.common.OctopusConstants;
+import octopus.teamcity.server.connection.ConnectionInlineFieldCleaner;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -86,6 +87,10 @@ public Collection<InvalidProperty> process(@Nullable final Map<String, String> p
         checkNotEmpty(p, c.getReleaseNumberKey(), "Release number must be specified", result);
         checkNotEmpty(p, c.getDeployToKey(), "Deploy to must be specified", result);
 
+        if (result.isEmpty()) {
+          ConnectionInlineFieldCleaner.stripInlineFieldsIfUsingConnection(p);
+        }
+
         return result;
       }
     };

octopus-server/src/main/java/octopus/teamcity/server/OctopusPromoteReleaseRunType.java

@@ -28,6 +28,7 @@
 import jetbrains.buildServer.util.StringUtil;
 import jetbrains.buildServer.web.openapi.PluginDescriptor;
 import octopus.teamcity.common.OctopusConstants;
+import octopus.teamcity.server.connection.ConnectionInlineFieldCleaner;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -87,6 +88,10 @@ public Collection<InvalidProperty> process(@Nullable final Map<String, String> p
             p, c.getPromoteFromKey(), "Environment to promote from must be specified", result);
         checkNotEmpty(p, c.getDeployToKey(), "Deploy to must be specified", result);
 
+        if (result.isEmpty()) {
+          ConnectionInlineFieldCleaner.stripInlineFieldsIfUsingConnection(p);
+        }
+
         return result;
       }
     };

octopus-server/src/main/java/octopus/teamcity/server/OctopusPushPackageRunType.java

@@ -28,6 +28,7 @@
 import jetbrains.buildServer.util.StringUtil;
 import jetbrains.buildServer.web.openapi.PluginDescriptor;
 import octopus.teamcity.common.OctopusConstants;
+import octopus.teamcity.server.connection.ConnectionInlineFieldCleaner;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -84,6 +85,10 @@ public Collection<InvalidProperty> process(@Nullable final Map<String, String> p
         }
         checkNotEmpty(p, c.getPackagePathsKey(), "Package paths must be specified", result);
 
+        if (result.isEmpty()) {
+          ConnectionInlineFieldCleaner.stripInlineFieldsIfUsingConnection(p);
+        }
+
         return result;
       }
     };

octopus-server/src/main/java/octopus/teamcity/server/connection/ConnectionInlineFieldCleaner.java

@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) Octopus Deploy and contributors. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use
+ *  these files except in compliance with the License. You may obtain a copy of the
+ * License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed
+ * under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
+ * CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ */
+
+package octopus.teamcity.server.connection;
+
+import java.util.Map;
+
+import jetbrains.buildServer.util.StringUtil;
+import octopus.teamcity.common.OctopusConstants;
+
+/**
+ * Removes a build step's inline Octopus credential parameters when the step references a reusable
+ * connection. The connection supplies the server URL, API key, and version; keeping these old
+ * inline values leaves stale credentials behind.
+ */
+public final class ConnectionInlineFieldCleaner {
+  private static final OctopusConstants CONSTANTS = new OctopusConstants();
+
+  private ConnectionInlineFieldCleaner() {}
+
+  public static void stripInlineFieldsIfUsingConnection(final Map<String, String> properties) {
+    if (properties == null) {
+      return;
+    }
+    final boolean usingConnection =
+        !StringUtil.isEmptyOrSpaces(properties.get(CONSTANTS.getConnectionIdKey()));
+    if (!usingConnection) {
+      return;
+    }
+    properties.remove(CONSTANTS.getServerKey());
+    properties.remove(CONSTANTS.getApiKey());
+    properties.remove(CONSTANTS.getOctopusVersion());
+  }
+}

octopus-server/src/test/java/octopus/teamcity/server/OctopusDeployReleaseRunTypeValidationTest.java

@@ -55,4 +55,38 @@ void neitherConnectionNorManualIsInvalid() {
     final Map<String, String> properties = withMandatoryNonCredentialFields(new HashMap<>());
     assertThat(validate(properties)).contains(CONSTANTS.getServerKey(), CONSTANTS.getApiKey());
   }
+
+  @Test
+  void stripsInlineCredentialFieldsWhenConnectionSelectedAndValidationPasses() {
+    final Map<String, String> properties = withMandatoryNonCredentialFields(new HashMap<>());
+    properties.put(CONSTANTS.getConnectionIdKey(), "PROJECT_EXT_1");
+    properties.put(CONSTANTS.getServerKey(), "https://octo");
+    properties.put(CONSTANTS.getApiKey(), "API-KEY");
+    properties.put(CONSTANTS.getOctopusVersion(), "3.0+");
+    properties.put(CONSTANTS.getSpaceName(), "Default");
+
+    assertThat(validate(properties)).isEmpty();
+
+    assertThat(properties)
+        .doesNotContainKeys(
+            CONSTANTS.getServerKey(), CONSTANTS.getApiKey(), CONSTANTS.getOctopusVersion());
+    assertThat(properties).containsEntry(CONSTANTS.getSpaceName(), "Default");
+  }
+
+  @Test
+  void retainsInlineCredentialFieldsWhenValidationFails() {
+    // Missing mandatory project name. Validation fails, so nothing should be stripped.
+    final Map<String, String> properties = new HashMap<>();
+    properties.put(CONSTANTS.getConnectionIdKey(), "PROJECT_EXT_1");
+    properties.put(CONSTANTS.getServerKey(), "https://octo");
+    properties.put(CONSTANTS.getApiKey(), "API-KEY");
+    properties.put(CONSTANTS.getOctopusVersion(), "3.0+");
+
+    final Collection<String> errors = validate(properties);
+
+    assertThat(errors).contains(CONSTANTS.getProjectNameKey());
+    assertThat(properties)
+        .containsKeys(
+            CONSTANTS.getServerKey(), CONSTANTS.getApiKey(), CONSTANTS.getOctopusVersion());
+  }
 }

octopus-server/src/test/java/octopus/teamcity/server/connection/ConnectionInlineFieldCleanerTest.java

@@ -0,0 +1,70 @@
+package octopus.teamcity.server.connection;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import octopus.teamcity.common.OctopusConstants;
+import org.junit.jupiter.api.Test;
+
+class ConnectionInlineFieldCleanerTest {
+  private static final OctopusConstants CONSTANTS = new OctopusConstants();
+
+  @Test
+  void removesInlineCredentialFieldsWhenConnectionSelected() {
+    final Map<String, String> properties = new HashMap<>();
+    properties.put(CONSTANTS.getConnectionIdKey(), "PROJECT_EXT_1");
+    properties.put(CONSTANTS.getServerKey(), "https://octo");
+    properties.put(CONSTANTS.getApiKey(), "API-KEY");
+    properties.put(CONSTANTS.getOctopusVersion(), "3.0+");
+    properties.put(CONSTANTS.getSpaceName(), "Default");
+
+    ConnectionInlineFieldCleaner.stripInlineFieldsIfUsingConnection(properties);
+
+    assertThat(properties)
+        .doesNotContainKeys(
+            CONSTANTS.getServerKey(), CONSTANTS.getApiKey(), CONSTANTS.getOctopusVersion());
+    assertThat(properties).containsEntry(CONSTANTS.getSpaceName(), "Default");
+    assertThat(properties).containsEntry(CONSTANTS.getConnectionIdKey(), "PROJECT_EXT_1");
+  }
+
+  @Test
+  void leavesInlineFieldsWhenNoConnectionSelected() {
+    final Map<String, String> properties = new HashMap<>();
+    properties.put(CONSTANTS.getServerKey(), "https://octo");
+    properties.put(CONSTANTS.getApiKey(), "API-KEY");
+    properties.put(CONSTANTS.getOctopusVersion(), "3.0+");
+
+    ConnectionInlineFieldCleaner.stripInlineFieldsIfUsingConnection(properties);
+
+    assertThat(properties)
+        .containsKeys(
+            CONSTANTS.getServerKey(), CONSTANTS.getApiKey(), CONSTANTS.getOctopusVersion());
+  }
+
+  @Test
+  void leavesInlineFieldsWhenConnectionIdIsBlank() {
+    final Map<String, String> properties = new HashMap<>();
+    properties.put(CONSTANTS.getConnectionIdKey(), "   ");
+    properties.put(CONSTANTS.getServerKey(), "https://octo");
+    properties.put(CONSTANTS.getApiKey(), "API-KEY");
+    properties.put(CONSTANTS.getOctopusVersion(), "3.0+");
+
+    ConnectionInlineFieldCleaner.stripInlineFieldsIfUsingConnection(properties);
+
+    assertThat(properties)
+        .containsKeys(
+            CONSTANTS.getServerKey(), CONSTANTS.getApiKey(), CONSTANTS.getOctopusVersion());
+  }
+
+  @Test
+  void toleratesConnectionSelectedWithNoInlineFields() {
+    final Map<String, String> properties = new HashMap<>();
+    properties.put(CONSTANTS.getConnectionIdKey(), "PROJECT_EXT_1");
+
+    ConnectionInlineFieldCleaner.stripInlineFieldsIfUsingConnection(properties);
+
+    assertThat(properties).containsOnlyKeys(CONSTANTS.getConnectionIdKey());
+  }
+}