From 7d282d8930d6aff0f4c0d05598cea20496635ef4 Mon Sep 17 00:00:00 2001 From: Matt Richardson Date: Wed, 14 Oct 2026 10:09:17 +1100 Subject: [PATCH 1/9] feat: store a space id and prefer it over the space name Co-Authored-By: Claude Opus 5.5 --- .../teamcity/common/OctopusConstants.java | 9 +++ .../teamcity/common/SpaceSelection.java | 30 +++++-- .../connection/ConnectionPropertyNames.java | 6 ++ .../teamcity/common/SpaceSelectionTest.java | 54 +++++++++---- .../server/connection/OctopusConnection.java | 14 +++- .../OctopusConnectionBuildStartProcessor.java | 13 +++- .../connection/OctopusConnectionUiData.java | 1 + .../connectionSelector.jsp | 6 +- .../buildServerResources/spaceDisplay.jsp | 4 + ...opusConnectionBuildStartProcessorTest.java | 78 +++++++++++++++++++ .../connection/OctopusConnectionTest.java | 23 ++++++ 11 files changed, 214 insertions(+), 24 deletions(-) diff --git a/octopus-common/src/main/java/octopus/teamcity/common/OctopusConstants.java b/octopus-common/src/main/java/octopus/teamcity/common/OctopusConstants.java index 3a4df880..093d11bd 100644 --- a/octopus-common/src/main/java/octopus/teamcity/common/OctopusConstants.java +++ b/octopus-common/src/main/java/octopus/teamcity/common/OctopusConstants.java @@ -53,6 +53,15 @@ public String getSpaceName() { return "octopus_space_name"; } + /** + * The id of the selected space, e.g. {@code Spaces-1795}. Preferred over {@link #getSpaceName()} + * because it survives a space being renamed. Both CLIs resolve a space by name or id, so either + * value works on the wire; see {@link SpaceSelection}. + */ + public String getSpaceId() { + return "octopus_space_id"; + } + public String getProjectNameKey() { return "octopus_project_name"; } diff --git a/octopus-common/src/main/java/octopus/teamcity/common/SpaceSelection.java b/octopus-common/src/main/java/octopus/teamcity/common/SpaceSelection.java index d97d199c..de566f3d 100644 --- a/octopus-common/src/main/java/octopus/teamcity/common/SpaceSelection.java +++ b/octopus-common/src/main/java/octopus/teamcity/common/SpaceSelection.java @@ -20,9 +20,16 @@ /** * Works out which space a step should act on. * - *

The two CLI paths differ in how they treat "no space set": {@link #resolve} returns an empty - * string so the legacy path can omit {@code --space} altogether, while {@link #resolveOrDefault} - * substitutes {@link #DEFAULT_SPACE}, as the Go CLI path has always done. + *

A step (or the connection it inherits from) can identify a space either by id - {@code + * Spaces-1795} - or by name. The id is preferred because it survives the space being renamed, which + * a name does not. Both CLIs the agent drives accept either: the legacy {@code octo} CLI resolves + * {@code --space} with {@code Spaces.FindByNameOrIdOrFail}, and the Go CLI prefers a name match + * then falls back to an id. So whichever value we pick here can be passed through unchanged. + * + *

The two CLI paths differ in how they treat "no space set", so this class deliberately offers + * both behaviours rather than imposing one: {@link #resolve} returns an empty string so the legacy + * path can omit {@code --space} altogether, while {@link #resolveOrDefault} substitutes {@link + * #DEFAULT_SPACE} as the Go CLI path has always done. */ public final class SpaceSelection { @@ -31,9 +38,17 @@ public final class SpaceSelection { private SpaceSelection() {} - /** The value to pass to {@code --space}, or an empty string when no space is set. */ + /** + * The value to pass to {@code --space}, preferring the space id over the space name, or an empty + * string when neither is set. + */ public static String resolve(final Map params) { - return trimToEmpty(params.get(OctopusConstants.Instance.getSpaceName())); + final OctopusConstants constants = OctopusConstants.Instance; + final String spaceId = trimToEmpty(params.get(constants.getSpaceId())); + if (!spaceId.isEmpty()) { + return spaceId; + } + return trimToEmpty(params.get(constants.getSpaceName())); } /** As {@link #resolve}, but falls back to {@link #DEFAULT_SPACE} when no space is set. */ @@ -42,6 +57,11 @@ public static String resolveOrDefault(final Map params) { return space.isEmpty() ? DEFAULT_SPACE : space; } + /** Whether a space was identified at all, by either id or name. */ + public static boolean isSet(final Map params) { + return !resolve(params).isEmpty(); + } + private static String trimToEmpty(final String value) { return value == null ? "" : value.trim(); } diff --git a/octopus-common/src/main/java/octopus/teamcity/common/connection/ConnectionPropertyNames.java b/octopus-common/src/main/java/octopus/teamcity/common/connection/ConnectionPropertyNames.java index e078addb..a6e66aaa 100644 --- a/octopus-common/src/main/java/octopus/teamcity/common/connection/ConnectionPropertyNames.java +++ b/octopus-common/src/main/java/octopus/teamcity/common/connection/ConnectionPropertyNames.java @@ -25,6 +25,8 @@ public class ConnectionPropertyNames { // No longer configurable; retained because existing connections may still store it. public static final String VERSION = "octopus_version"; public static final String SPACE_NAME = "octopus_space_name"; + // The id of the selected space. Preferred over SPACE_NAME as it survives a rename. + public static final String SPACE_ID = "octopus_space_id"; // API key source — selects how the connection supplies credentials. public static final String API_KEY_SOURCE = "octopus_apikey_source"; @@ -67,6 +69,10 @@ public String getSpaceNamePropertyName() { return SPACE_NAME; } + public String getSpaceIdPropertyName() { + return SPACE_ID; + } + public String getApiKeySourcePropertyName() { return API_KEY_SOURCE; } diff --git a/octopus-common/src/test/java/octopus/teamcity/common/SpaceSelectionTest.java b/octopus-common/src/test/java/octopus/teamcity/common/SpaceSelectionTest.java index c20d538a..4878ebd6 100644 --- a/octopus-common/src/test/java/octopus/teamcity/common/SpaceSelectionTest.java +++ b/octopus-common/src/test/java/octopus/teamcity/common/SpaceSelectionTest.java @@ -24,41 +24,69 @@ class SpaceSelectionTest { - private Map params(final String spaceName) { + private static final OctopusConstants CONSTANTS = OctopusConstants.Instance; + + private Map params(final String spaceId, final String spaceName) { final Map params = new HashMap<>(); + if (spaceId != null) { + params.put(CONSTANTS.getSpaceId(), spaceId); + } if (spaceName != null) { - params.put(OctopusConstants.Instance.getSpaceName(), spaceName); + params.put(CONSTANTS.getSpaceName(), spaceName); } return params; } @Test - void resolvesTheSpaceName() { - assertThat(SpaceSelection.resolve(params("Modern Deployments"))) + void prefersTheSpaceIdOverTheSpaceName() { + // The whole point: the id keeps working after the space is renamed in Octopus, so a stale + // stored name must not win over it. + assertThat(SpaceSelection.resolve(params("Spaces-1795", "Modern Deployments"))) + .isEqualTo("Spaces-1795"); + } + + @Test + void fallsBackToTheSpaceNameWhenNoIdIsStored() { + assertThat(SpaceSelection.resolve(params(null, "Modern Deployments"))) + .isEqualTo("Modern Deployments"); + } + + @Test + void treatsABlankIdAsAbsent() { + assertThat(SpaceSelection.resolve(params(" ", "Modern Deployments"))) .isEqualTo("Modern Deployments"); } @Test void trimsSurroundingWhitespace() { - assertThat(SpaceSelection.resolve(params(" Modern Deployments "))) + assertThat(SpaceSelection.resolve(params(null, " Modern Deployments "))) .isEqualTo("Modern Deployments"); } @Test - void resolvesToEmptyWhenNoSpaceIsSet() { - assertThat(SpaceSelection.resolve(params(null))).isEmpty(); - assertThat(SpaceSelection.resolve(params(" "))).isEmpty(); + void resolvesToEmptyWhenNeitherIsSet() { + assertThat(SpaceSelection.resolve(params(null, null))).isEmpty(); + assertThat(SpaceSelection.resolve(params("", " "))).isEmpty(); } @Test void resolveOrDefaultSubstitutesTheDefaultSpace() { - assertThat(SpaceSelection.resolveOrDefault(params(null))).isEqualTo("Default"); - assertThat(SpaceSelection.resolveOrDefault(params(" "))).isEqualTo("Default"); + // The Go CLI path has always sent "Default" rather than omitting --space. + assertThat(SpaceSelection.resolveOrDefault(params(null, null))).isEqualTo("Default"); + assertThat(SpaceSelection.DEFAULT_SPACE).isEqualTo("Default"); } @Test - void resolveOrDefaultKeepsASpaceThatIsSet() { - assertThat(SpaceSelection.resolveOrDefault(params("Modern Deployments"))) - .isEqualTo("Modern Deployments"); + void resolveOrDefaultStillPrefersAnIdWhenPresent() { + assertThat(SpaceSelection.resolveOrDefault(params("Spaces-2", "Whatever"))) + .isEqualTo("Spaces-2"); + } + + @Test + void isSetReportsWhetherASpaceWasIdentified() { + assertThat(SpaceSelection.isSet(params("Spaces-3", null))).isTrue(); + assertThat(SpaceSelection.isSet(params(null, "Named"))).isTrue(); + assertThat(SpaceSelection.isSet(params(null, null))).isFalse(); + assertThat(SpaceSelection.isSet(params(" ", " "))).isFalse(); } } diff --git a/octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnection.java b/octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnection.java index 2ee427b6..450e7c8a 100644 --- a/octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnection.java +++ b/octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnection.java @@ -66,11 +66,19 @@ public Map getDefaultProperties() { public String describeConnection(@NotNull final OAuthConnectionDescriptor connection) { final Map params = connection.getParameters(); final String url = params.getOrDefault(ConnectionPropertyNames.SERVER_URL, "(no URL)"); - final String space = params.getOrDefault(ConnectionPropertyNames.SPACE_NAME, ""); + final String spaceName = params.getOrDefault(ConnectionPropertyNames.SPACE_NAME, ""); + final String spaceId = params.getOrDefault(ConnectionPropertyNames.SPACE_ID, ""); // TeamCity renders newlines in a connection description as line breaks on the Connections tab. final StringBuilder description = new StringBuilder("Octopus URL: ").append(url); - if (!space.isEmpty()) { - description.append("\nSpace Name: ").append(space); + if (!spaceName.isEmpty()) { + description.append("\nSpace Name: ").append(spaceName); + // The id is what is actually sent, so show it too - it is how a stale name becomes visible + // after the space has been renamed in Octopus. + if (!spaceId.isEmpty()) { + description.append(" (").append(spaceId).append(")"); + } + } else if (!spaceId.isEmpty()) { + description.append("\nSpace Id: ").append(spaceId); } return description.toString(); } diff --git a/octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnectionBuildStartProcessor.java b/octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnectionBuildStartProcessor.java index 1177e50a..61975047 100644 --- a/octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnectionBuildStartProcessor.java +++ b/octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnectionBuildStartProcessor.java @@ -115,8 +115,17 @@ public void updateParameters(final BuildStartContext buildStartContext) { runner, CONSTANTS.getApiKey(), connParams.get(CONNECTION_KEYS.getApiKeyPropertyName())); } - // Space precedence: keep the step's value if it set one; otherwise use the connection's. - if (StringUtil.isEmptyOrSpaces(stepParams.get(CONSTANTS.getSpaceName()))) { + // Space precedence: a step that identified a space - by id or by name - keeps its own + // selection. The id and name move together as one unit, because inheriting the connection's + // id alongside a name the step set deliberately would silently retarget the step. + final boolean stepChoseSpace = + !StringUtil.isEmptyOrSpaces(stepParams.get(CONSTANTS.getSpaceId())) + || !StringUtil.isEmptyOrSpaces(stepParams.get(CONSTANTS.getSpaceName())); + if (!stepChoseSpace) { + setIfPresent( + runner, + CONSTANTS.getSpaceId(), + connParams.get(CONNECTION_KEYS.getSpaceIdPropertyName())); setIfPresent( runner, CONSTANTS.getSpaceName(), diff --git a/octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnectionUiData.java b/octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnectionUiData.java index 6fafb1a9..7d156bcd 100644 --- a/octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnectionUiData.java +++ b/octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnectionUiData.java @@ -88,6 +88,7 @@ static List> availableConnections( view.put("displayName", descriptor.getConnectionDisplayName()); view.put("url", params.getOrDefault(CONNECTION_KEYS.getServerUrlPropertyName(), "")); view.put("space", params.getOrDefault(CONNECTION_KEYS.getSpaceNamePropertyName(), "")); + view.put("spaceId", params.getOrDefault(CONNECTION_KEYS.getSpaceIdPropertyName(), "")); final String apiKeySource = params.getOrDefault(CONNECTION_KEYS.getApiKeySourcePropertyName(), ""); diff --git a/octopus-server/src/main/resources/buildServerResources/connectionSelector.jsp b/octopus-server/src/main/resources/buildServerResources/connectionSelector.jsp index 7bf96996..ffc74ab5 100644 --- a/octopus-server/src/main/resources/buildServerResources/connectionSelector.jsp +++ b/octopus-server/src/main/resources/buildServerResources/connectionSelector.jsp @@ -28,6 +28,7 @@ @@ -84,10 +85,13 @@ const spaceField = document.getElementById("${keys.spaceName}"); const spaceRow = spaceField ? spaceField.closest("tr") : null; if (spaceRow) { + // A connection may identify its space by id, by name, or both. let connSpace = ""; if (usingConnection) { const meta = getOctopusConnectionMetadataFor(select.value); - connSpace = meta ? meta.getAttribute("data-conn-space") : ""; + connSpace = meta + ? (meta.getAttribute("data-conn-space") || meta.getAttribute("data-conn-space-id")) + : ""; } spaceRow.style.display = connSpace ? "none" : "table-row"; } diff --git a/octopus-server/src/main/resources/buildServerResources/spaceDisplay.jsp b/octopus-server/src/main/resources/buildServerResources/spaceDisplay.jsp index 59869fe5..9fb1e0e1 100644 --- a/octopus-server/src/main/resources/buildServerResources/spaceDisplay.jsp +++ b/octopus-server/src/main/resources/buildServerResources/spaceDisplay.jsp @@ -1,5 +1,6 @@ <%@ include file="/include-internal.jsp" %> <%@ taglib prefix="props" tagdir="/WEB-INF/tags/props" %> +<%@ taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core" %> <%-- ~ Copyright (c) Octopus Deploy and contributors. All rights reserved. @@ -23,4 +24,7 @@

Space name: + + id: +
diff --git a/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionBuildStartProcessorTest.java b/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionBuildStartProcessorTest.java index f995ddf8..fd36b55d 100644 --- a/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionBuildStartProcessorTest.java +++ b/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionBuildStartProcessorTest.java @@ -73,6 +73,84 @@ private OAuthConnectionDescriptor connectionWith( return descriptor; } + /** As {@link #connectionWith} but also giving the connection a space id. */ + private OAuthConnectionDescriptor connectionWithSpace( + final String spaceId, final String spaceName) { + final OAuthConnectionDescriptor descriptor = mock(OAuthConnectionDescriptor.class); + final Map params = new HashMap<>(); + params.put(ConnectionPropertyNames.SERVER_URL, "https://octo"); + params.put(ConnectionPropertyNames.API_KEY, "API-KEY"); + if (spaceId != null) { + params.put(ConnectionPropertyNames.SPACE_ID, spaceId); + } + if (spaceName != null) { + params.put(ConnectionPropertyNames.SPACE_NAME, spaceName); + } + when(descriptor.getParameters()).thenReturn(params); + return descriptor; + } + + @Test + void inheritsTheConnectionsSpaceIdAndNameTogether() { + final Map properties = new HashMap<>(); + properties.put(CONSTANTS.getConnectionIdKey(), "PROJECT_EXT_1"); + when(runnerContext.getParameters()).thenReturn(properties); + + final OAuthConnectionDescriptor descriptor = connectionWithSpace("Spaces-1795", "Swordfish"); + when(connectionsManager.resolve(project, "PROJECT_EXT_1")).thenReturn(Optional.of(descriptor)); + + processor.updateParameters(buildStartContext); + + verify(runnerContext).addRunnerParameter(CONSTANTS.getSpaceId(), "Spaces-1795"); + verify(runnerContext).addRunnerParameter(CONSTANTS.getSpaceName(), "Swordfish"); + } + + @Test + void aStepSpaceNameBlocksInheritingTheConnectionsSpaceId() { + // The important case: were the id inherited alongside the step's own name, the id would win in + // SpaceSelection and silently retarget the step at the connection's space. + final Map properties = new HashMap<>(); + properties.put(CONSTANTS.getConnectionIdKey(), "PROJECT_EXT_1"); + properties.put(CONSTANTS.getSpaceName(), "StepSpace"); + when(runnerContext.getParameters()).thenReturn(properties); + + final OAuthConnectionDescriptor descriptor = connectionWithSpace("Spaces-1795", "Swordfish"); + when(connectionsManager.resolve(project, "PROJECT_EXT_1")).thenReturn(Optional.of(descriptor)); + + processor.updateParameters(buildStartContext); + + verify(runnerContext, never()) + .addRunnerParameter( + org.mockito.ArgumentMatchers.eq(CONSTANTS.getSpaceId()), + org.mockito.ArgumentMatchers.anyString()); + verify(runnerContext, never()) + .addRunnerParameter( + org.mockito.ArgumentMatchers.eq(CONSTANTS.getSpaceName()), + org.mockito.ArgumentMatchers.anyString()); + } + + @Test + void aStepSpaceIdBlocksInheritingTheConnectionsSpace() { + final Map properties = new HashMap<>(); + properties.put(CONSTANTS.getConnectionIdKey(), "PROJECT_EXT_1"); + properties.put(CONSTANTS.getSpaceId(), "Spaces-42"); + when(runnerContext.getParameters()).thenReturn(properties); + + final OAuthConnectionDescriptor descriptor = connectionWithSpace("Spaces-1795", "Swordfish"); + when(connectionsManager.resolve(project, "PROJECT_EXT_1")).thenReturn(Optional.of(descriptor)); + + processor.updateParameters(buildStartContext); + + verify(runnerContext, never()) + .addRunnerParameter( + org.mockito.ArgumentMatchers.eq(CONSTANTS.getSpaceId()), + org.mockito.ArgumentMatchers.anyString()); + verify(runnerContext, never()) + .addRunnerParameter( + org.mockito.ArgumentMatchers.eq(CONSTANTS.getSpaceName()), + org.mockito.ArgumentMatchers.anyString()); + } + @Test void injectsConnectionValuesWhenConnectionSelected() { final Map properties = new HashMap<>(); diff --git a/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionTest.java b/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionTest.java index 4ba10f7f..e465114f 100644 --- a/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionTest.java +++ b/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionTest.java @@ -57,6 +57,29 @@ void describeConnectionIncludesSpaceOnItsOwnLineWhenSet() { .isEqualTo("Octopus URL: https://octopus.example.com\nSpace Name: My Space"); } + @Test + void describeConnectionShowsTheSpaceIdBesideTheName() { + // The id is what is actually sent, so surfacing it is how a stale name becomes visible after + // the space has been renamed in Octopus. + final Map params = new HashMap<>(); + params.put(ConnectionPropertyNames.SERVER_URL, "https://octopus.example.com"); + params.put(ConnectionPropertyNames.SPACE_NAME, "Swordfish"); + params.put(ConnectionPropertyNames.SPACE_ID, "Spaces-1795"); + + assertThat(provider.describeConnection(descriptorWith(params))) + .isEqualTo("Octopus URL: https://octopus.example.com\nSpace Name: Swordfish (Spaces-1795)"); + } + + @Test + void describeConnectionShowsTheSpaceIdAloneWhenNoNameIsStored() { + final Map params = new HashMap<>(); + params.put(ConnectionPropertyNames.SERVER_URL, "https://octopus.example.com"); + params.put(ConnectionPropertyNames.SPACE_ID, "Spaces-1795"); + + assertThat(provider.describeConnection(descriptorWith(params))) + .isEqualTo("Octopus URL: https://octopus.example.com\nSpace Id: Spaces-1795"); + } + @Test void describeConnectionFallsBackWhenUrlMissing() { assertThat(provider.describeConnection(descriptorWith(new HashMap<>()))) From acbdceea84eff134d7ff796e46cba1f7d88c0880 Mon Sep 17 00:00:00 2001 From: Matt Richardson Date: Wed, 14 Oct 2026 10:11:18 +1100 Subject: [PATCH 2/9] feat: list an Octopus server's spaces for a connection with a stored API key Co-Authored-By: Claude Opus 5.5 --- octopus-server/build.gradle | 3 + .../teamcity/server/spaces/OctopusAuth.java | 61 +++++ .../teamcity/server/spaces/OctopusSpace.java | 35 +++ .../spaces/OctopusSpacesController.java | 246 ++++++++++++++++++ .../server/spaces/OctopusSpacesFetcher.java | 201 ++++++++++++++ .../build-server-plugin-Octopus.TeamCity.xml | 1 + .../spaces/OctopusSpacesControllerTest.java | 246 ++++++++++++++++++ .../spaces/OctopusSpacesFetcherTest.java | 225 ++++++++++++++++ 8 files changed, 1018 insertions(+) create mode 100644 octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusAuth.java create mode 100644 octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpace.java create mode 100644 octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesController.java create mode 100644 octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesFetcher.java create mode 100644 octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesControllerTest.java create mode 100644 octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesFetcherTest.java diff --git a/octopus-server/build.gradle b/octopus-server/build.gradle index ddf7bdea..d4dfb374 100644 --- a/octopus-server/build.gradle +++ b/octopus-server/build.gradle @@ -17,6 +17,8 @@ dependencies { provided 'org.jetbrains.teamcity:oauth' implementation project(':octopus-common') implementation 'com.google.guava:guava' + implementation 'com.google.code.gson:gson' + implementation 'commons-io:commons-io' testImplementation 'org.jetbrains.teamcity:server-api' testImplementation 'org.jetbrains.teamcity:oauth' @@ -26,6 +28,7 @@ dependencies { testImplementation 'org.assertj:assertj-core' testImplementation 'org.mockito:mockito-core' testImplementation 'org.mockito:mockito-junit-jupiter' + testImplementation 'com.google.code.gson:gson' } distributions { diff --git a/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusAuth.java b/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusAuth.java new file mode 100644 index 00000000..1371a45c --- /dev/null +++ b/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusAuth.java @@ -0,0 +1,61 @@ +/* + * 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.spaces; + +/** The header a request to Octopus authenticates with: an API key, or an OIDC access token. */ +public final class OctopusAuth { + + private final String header; + private final String value; + private final String description; + + private OctopusAuth(final String header, final String value, final String description) { + this.header = header; + this.value = value; + this.description = description; + } + + public static OctopusAuth apiKey(final String apiKey) { + return new OctopusAuth("X-Octopus-ApiKey", trimmed(apiKey), "API key"); + } + + public static OctopusAuth accessToken(final String accessToken) { + final String token = trimmed(accessToken); + return new OctopusAuth( + "Authorization", token.isEmpty() ? "" : "Bearer " + token, "access token"); + } + + public String header() { + return header; + } + + public String value() { + return value; + } + + /** What the credential is, for error messages: "API key" or "access token". */ + public String description() { + return description; + } + + boolean isEmpty() { + return value.isEmpty(); + } + + private static String trimmed(final String value) { + return value == null ? "" : value.trim(); + } +} diff --git a/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpace.java b/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpace.java new file mode 100644 index 00000000..8362772c --- /dev/null +++ b/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpace.java @@ -0,0 +1,35 @@ +/* + * 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.spaces; + +/** A space on an Octopus server, as offered to the space picker. */ +public class OctopusSpace { + private final String id; + private final String name; + + public OctopusSpace(final String id, final String name) { + this.id = id; + this.name = name; + } + + public String getId() { + return id; + } + + public String getName() { + return name; + } +} diff --git a/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesController.java b/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesController.java new file mode 100644 index 00000000..d21697d9 --- /dev/null +++ b/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesController.java @@ -0,0 +1,246 @@ +/* + * 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.spaces; + +import java.io.IOException; +import java.io.PrintWriter; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; +import jetbrains.buildServer.controllers.BaseController; +import jetbrains.buildServer.serverSide.ProjectManager; +import jetbrains.buildServer.serverSide.SProject; +import jetbrains.buildServer.serverSide.auth.Permission; +import jetbrains.buildServer.serverSide.oauth.OAuthConnectionDescriptor; +import jetbrains.buildServer.users.SUser; +import jetbrains.buildServer.web.openapi.WebControllerManager; +import jetbrains.buildServer.web.util.SessionUser; +import octopus.teamcity.common.connection.ConnectionPropertyNames; +import octopus.teamcity.server.connection.OctopusConnectionsManager; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.springframework.web.servlet.ModelAndView; + +/** + * Lists an Octopus server's spaces for the space picker in the connection and step forms. + * + *

This endpoint makes an outbound request to a URL the caller influences, so it is deliberately + * narrow: + * + *

    + *
  • the caller must be logged in and hold {@link Permission#EDIT_PROJECT} on the project whose + * form they are editing - the same permission needed to configure a connection in the first + * place, so it grants no reach a user did not already have; + *
  • when a saved connection is named, its credentials are read server-side and the request's + * own {@code apiKey} is ignored, because TeamCity renders a stored secret as a placeholder + * and the browser therefore does not hold the real key; + *
  • the API key is never echoed back, in an error message or otherwise. + *
+ */ +public class OctopusSpacesController extends BaseController { + + public static final String PATH = "/octopus/listSpaces.html"; + + private static final Gson GSON = new GsonBuilder().create(); + + /** How the current user is found. A seam so the permission checks are testable. */ + interface UserResolver { + SUser resolve(HttpServletRequest request); + } + + private final OctopusConnectionsManager connectionsManager; + private final ProjectManager projectManager; + private final OctopusSpacesFetcher fetcher; + private final UserResolver userResolver; + + public OctopusSpacesController( + final WebControllerManager webControllerManager, + final OctopusConnectionsManager connectionsManager, + final ProjectManager projectManager) { + this( + webControllerManager, + connectionsManager, + projectManager, + new OctopusSpacesFetcher(), + SessionUser::getUser); + } + + OctopusSpacesController( + final WebControllerManager webControllerManager, + final OctopusConnectionsManager connectionsManager, + final ProjectManager projectManager, + final OctopusSpacesFetcher fetcher, + final UserResolver userResolver) { + this.connectionsManager = connectionsManager; + this.projectManager = projectManager; + this.fetcher = fetcher; + this.userResolver = userResolver; + if (webControllerManager != null) { + webControllerManager.registerController(PATH, this); + } + } + + @Nullable + @Override + protected ModelAndView doHandle( + @NotNull final HttpServletRequest request, @NotNull final HttpServletResponse response) + throws Exception { + response.setContentType("application/json; charset=UTF-8"); + // This is a lookup for a form; a stale list must never be served from a cache. + response.setHeader("Cache-Control", "no-store"); + + final SUser user = userResolver.resolve(request); + if (user == null) { + writeError(response, HttpServletResponse.SC_UNAUTHORIZED, "You are not logged in."); + return null; + } + + final SProject project = findProject(request.getParameter("projectId")); + if (project == null) { + writeError(response, HttpServletResponse.SC_BAD_REQUEST, "Unknown project."); + return null; + } + if (!user.isPermissionGrantedForProject(project.getProjectId(), Permission.EDIT_PROJECT)) { + writeError( + response, + HttpServletResponse.SC_FORBIDDEN, + "You do not have permission to edit this project."); + return null; + } + + final Credentials credentials; + try { + credentials = resolveCredentials(request, project); + } catch (final CredentialsUnavailableException e) { + writeError(response, HttpServletResponse.SC_OK, e.getMessage()); + return null; + } + + try { + final List spaces = fetcher.fetch(credentials.serverUrl, credentials.auth); + writeSpaces(response, spaces); + } catch (final IOException e) { + // The fetcher's messages describe the failure without quoting the credential back. + writeError(response, HttpServletResponse.SC_OK, e.getMessage()); + } + return null; + } + + @Nullable + private SProject findProject(@Nullable final String projectExternalId) { + if (projectExternalId == null || projectExternalId.trim().isEmpty()) { + return null; + } + return projectManager.findProjectByExternalId(projectExternalId.trim()); + } + + /** + * Credentials for the lookup, always read from a saved connection. + * + *

The request names a connection; it never supplies a URL or key of its own. That is + * deliberate: it keeps the server from being asked to fetch an arbitrary caller-supplied address, + * so the only URLs it will ever call are ones an admin has already persisted into project + * configuration. It also sidesteps TeamCity rendering a saved secret as a placeholder, which + * means the browser could not supply the real key anyway. + * + *

The cost is that a brand-new connection has to be saved once before its spaces can be + * listed, and a step using inline credentials cannot list spaces at all. Both fall back to typing + * the space name, as does a connection whose API key is not stored on it. + */ + private Credentials resolveCredentials(final HttpServletRequest request, final SProject project) + throws CredentialsUnavailableException { + final String connectionId = request.getParameter("connectionId"); + if (connectionId == null || connectionId.trim().isEmpty()) { + throw new CredentialsUnavailableException( + "Spaces can only be listed for a saved Octopus connection. Save this connection (or" + + " select one) and try again, or enter the space name instead."); + } + + final Optional connection = + connectionsManager.resolve(project, connectionId.trim()); + if (!connection.isPresent()) { + throw new CredentialsUnavailableException("That Octopus connection could not be resolved."); + } + + final Map params = connection.get().getParameters(); + final String serverUrl = params.get(ConnectionPropertyNames.SERVER_URL); + final String source = + params.getOrDefault( + ConnectionPropertyNames.API_KEY_SOURCE, ConnectionPropertyNames.API_KEY_SOURCE_KEY); + if (!ConnectionPropertyNames.API_KEY_SOURCE_KEY.equals(source)) { + throw new CredentialsUnavailableException( + "Spaces can only be listed for a connection that stores its API key. Enter the space" + + " name instead."); + } + return new Credentials( + serverUrl, OctopusAuth.apiKey(params.get(ConnectionPropertyNames.API_KEY))); + } + + private static void writeSpaces( + final HttpServletResponse response, final List spaces) throws IOException { + final List> items = new ArrayList<>(); + for (final OctopusSpace space : spaces) { + final Map item = new LinkedHashMap<>(); + item.put("id", space.getId()); + item.put("name", space.getName()); + items.add(item); + } + final Map body = new LinkedHashMap<>(); + body.put("spaces", items); + writeJson(response, body); + } + + private static void writeError( + final HttpServletResponse response, final int status, final String message) + throws IOException { + response.setStatus(status); + final Map body = new LinkedHashMap<>(); + body.put("error", message); + writeJson(response, body); + } + + private static void writeJson(final HttpServletResponse response, final Map body) + throws IOException { + try (PrintWriter writer = response.getWriter()) { + writer.write(GSON.toJson(body)); + } + } + + private static final class Credentials { + private final String serverUrl; + private final OctopusAuth auth; + + private Credentials(final String serverUrl, final OctopusAuth auth) { + this.serverUrl = serverUrl; + this.auth = auth; + } + } + + private static final class CredentialsUnavailableException extends Exception { + private static final long serialVersionUID = 1L; + + private CredentialsUnavailableException(final String message) { + super(message); + } + } +} diff --git a/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesFetcher.java b/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesFetcher.java new file mode 100644 index 00000000..4e3607e2 --- /dev/null +++ b/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesFetcher.java @@ -0,0 +1,201 @@ +/* + * 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.spaces; + +import java.io.IOException; +import java.io.InputStream; +import java.net.HttpURLConnection; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; + +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; +import com.google.gson.JsonSyntaxException; +import org.apache.commons.io.IOUtils; + +/** + * Lists the spaces on an Octopus server, so the UI can offer them as a dropdown rather than asking + * an admin to type a space name by hand. + * + *

Follows {@code CommitHistoryFetcher}'s shape: the HTTP call sits behind {@link Requester} so + * the parsing and URL construction can be tested without a server. + */ +public class OctopusSpacesFetcher { + + /** How many spaces we ask for. Far above any realistic instance. */ + static final int TAKE = 1000; + + private static final int CONNECT_TIMEOUT_MILLIS = 10_000; + private static final int READ_TIMEOUT_MILLIS = 20_000; + private static final Gson GSON = new GsonBuilder().create(); + + /** Performs the GET. Separated out so tests need no server. */ + public interface Requester { + String get(String url, OctopusAuth auth) throws IOException; + } + + private final Requester requester; + + public OctopusSpacesFetcher() { + this(OctopusSpacesFetcher::httpGet); + } + + OctopusSpacesFetcher(final Requester requester) { + this.requester = requester; + } + + /** The spaces visible to the supplied API key, ordered by name. */ + public List fetch(final String serverUrl, final String apiKey) throws IOException { + return fetch(serverUrl, OctopusAuth.apiKey(apiKey)); + } + + /** + * The spaces visible to the supplied credential, ordered by name. + * + * @throws IOException if the server cannot be reached, rejects the credential, or returns + * something that is not a spaces collection. + */ + public List fetch(final String serverUrl, final OctopusAuth auth) + throws IOException { + if (serverUrl == null || serverUrl.trim().isEmpty()) { + throw new IOException("An Octopus URL is required to list spaces."); + } + if (auth.isEmpty()) { + throw new IOException("An Octopus " + auth.description() + " is required to list spaces."); + } + + final String body = requester.get(spacesUrl(serverUrl), auth); + + final SpacesCollection collection; + try { + collection = GSON.fromJson(body, SpacesCollection.class); + } catch (final JsonSyntaxException e) { + throw new IOException( + "The Octopus server did not return a spaces list. Check the URL points at an Octopus" + + " server.", + e); + } + if (collection == null || collection.Items == null) { + throw new IOException( + "The Octopus server did not return a spaces list. Check the URL points at an Octopus" + + " server."); + } + + final List spaces = new ArrayList<>(); + for (final SpaceItem item : collection.Items) { + if (item != null && item.Id != null && !item.Id.isEmpty()) { + spaces.add(new OctopusSpace(item.Id, item.Name == null ? item.Id : item.Name)); + } + } + Collections.sort(spaces, Comparator.comparing(space -> space.getName().toLowerCase())); + return spaces; + } + + /** + * Builds the spaces URL, rejecting anything that is not a plain http(s) address. + * + *

The caller supplies this URL when configuring a connection, so the scheme is restricted to + * http and https - without that, a value such as {@code file:} or {@code jar:} would make {@link + * URL#openConnection()} read from somewhere other than a web server. A host is required so a + * scheme-only value cannot slip through. + */ + static String spacesUrl(final String serverUrl) throws IOException { + return baseUrl(serverUrl) + "/api/spaces?skip=0&take=" + TAKE; + } + + /** The server URL without trailing slashes, after the checks {@link #spacesUrl} describes. */ + static String baseUrl(final String serverUrl) throws IOException { + String base = serverUrl.trim(); + while (base.endsWith("/")) { + base = base.substring(0, base.length() - 1); + } + + final URI parsed; + try { + parsed = new URI(base); + } catch (final URISyntaxException e) { + throw new IOException("'" + base + "' is not a valid URL.", e); + } + final String scheme = parsed.getScheme() == null ? "" : parsed.getScheme().toLowerCase(); + if (!"http".equals(scheme) && !"https".equals(scheme)) { + throw new IOException("The Octopus URL must start with http:// or https://."); + } + if (parsed.getHost() == null || parsed.getHost().isEmpty()) { + throw new IOException("The Octopus URL is missing a host name."); + } + + return base; + } + + private static String httpGet(final String url, final OctopusAuth auth) throws IOException { + final HttpURLConnection connection = (HttpURLConnection) new URL(url).openConnection(); + try { + connection.setRequestMethod("GET"); + // A redirect would take the request somewhere other than the address just validated, and + // would also replay the credential there. + connection.setInstanceFollowRedirects(false); + connection.setConnectTimeout(CONNECT_TIMEOUT_MILLIS); + connection.setReadTimeout(READ_TIMEOUT_MILLIS); + connection.setRequestProperty(auth.header(), auth.value()); + connection.setRequestProperty("Accept", "application/json"); + + final int status = connection.getResponseCode(); + if (status == HttpURLConnection.HTTP_UNAUTHORIZED + || status == HttpURLConnection.HTTP_FORBIDDEN) { + throw new IOException( + "Octopus rejected the " + + auth.description() + + " (HTTP " + + status + + "). Check its permissions."); + } + if (status >= 300 && status <= 399) { + throw new IOException( + "The Octopus URL redirected (HTTP " + + status + + "). Point it directly at the Octopus server."); + } + if (status < 200 || status > 299) { + throw new IOException("Octopus returned HTTP " + status + " when listing spaces."); + } + try (InputStream in = connection.getInputStream()) { + return IOUtils.toString(in, StandardCharsets.UTF_8); + } + } finally { + connection.disconnect(); + } + } + + /** Shape of the subset of {@code /api/spaces} we read. Field names match the JSON. */ + private static final class SpacesCollection { + @SuppressWarnings("MemberName") + private List Items; + } + + private static final class SpaceItem { + @SuppressWarnings("MemberName") + private String Id; + + @SuppressWarnings("MemberName") + private String Name; + } +} diff --git a/octopus-server/src/main/resources/META-INF/build-server-plugin-Octopus.TeamCity.xml b/octopus-server/src/main/resources/META-INF/build-server-plugin-Octopus.TeamCity.xml index f6539ce0..8c47fe8e 100644 --- a/octopus-server/src/main/resources/META-INF/build-server-plugin-Octopus.TeamCity.xml +++ b/octopus-server/src/main/resources/META-INF/build-server-plugin-Octopus.TeamCity.xml @@ -21,4 +21,5 @@ + diff --git a/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesControllerTest.java b/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesControllerTest.java new file mode 100644 index 00000000..5bc8a8f6 --- /dev/null +++ b/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesControllerTest.java @@ -0,0 +1,246 @@ +/* + * 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.spaces; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import jetbrains.buildServer.serverSide.ProjectManager; +import jetbrains.buildServer.serverSide.SProject; +import jetbrains.buildServer.serverSide.auth.Permission; +import jetbrains.buildServer.serverSide.oauth.OAuthConnectionDescriptor; +import jetbrains.buildServer.users.SUser; +import octopus.teamcity.common.connection.ConnectionPropertyNames; +import octopus.teamcity.server.connection.OctopusConnectionsManager; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class OctopusSpacesControllerTest { + + private static final String PROJECT_EXTERNAL_ID = "MyProject"; + private static final String PROJECT_INTERNAL_ID = "project123"; + + private final OctopusConnectionsManager connectionsManager = + mock(OctopusConnectionsManager.class); + private final ProjectManager projectManager = mock(ProjectManager.class); + private final HttpServletRequest request = mock(HttpServletRequest.class); + private final HttpServletResponse response = mock(HttpServletResponse.class); + private final SUser user = mock(SUser.class); + private final SProject project = mock(SProject.class); + private final StringWriter written = new StringWriter(); + + private final List fetchedUrls = new java.util.ArrayList<>(); + private final List fetchedKeys = new java.util.ArrayList<>(); + + private SUser currentUser = null; + private IOException fetchFailure = null; + + @BeforeEach + void setUp() throws Exception { + when(response.getWriter()).thenReturn(new PrintWriter(written)); + when(project.getProjectId()).thenReturn(PROJECT_INTERNAL_ID); + when(projectManager.findProjectByExternalId(PROJECT_EXTERNAL_ID)).thenReturn(project); + when(request.getParameter("projectId")).thenReturn(PROJECT_EXTERNAL_ID); + currentUser = user; + grantEditProject(true); + } + + private void grantEditProject(final boolean granted) { + when(user.isPermissionGrantedForProject(PROJECT_INTERNAL_ID, Permission.EDIT_PROJECT)) + .thenReturn(granted); + } + + private OctopusSpacesController controller() { + final OctopusSpacesFetcher fetcher = + new OctopusSpacesFetcher( + (url, auth) -> { + fetchedUrls.add(url); + fetchedKeys.add(auth.value()); + if (fetchFailure != null) { + throw fetchFailure; + } + return "{\"Items\":[{\"Id\":\"Spaces-1795\",\"Name\":\"Swordfish\"}]}"; + }); + return new OctopusSpacesController( + null, connectionsManager, projectManager, fetcher, req -> currentUser); + } + + private String handle() throws Exception { + controller().doHandle(request, response); + return written.toString(); + } + + private OAuthConnectionDescriptor connectionWith(final Map params) { + final OAuthConnectionDescriptor descriptor = mock(OAuthConnectionDescriptor.class); + when(descriptor.getParameters()).thenReturn(params); + return descriptor; + } + + @Test + void returnsSpacesForASavedConnection() throws Exception { + final Map params = new HashMap<>(); + params.put(ConnectionPropertyNames.SERVER_URL, "https://stored.example.com"); + params.put(ConnectionPropertyNames.API_KEY, "API-STORED"); + final OAuthConnectionDescriptor connection = connectionWith(params); + when(connectionsManager.resolve(eq(project), eq("octopus-conn-1"))) + .thenReturn(Optional.of(connection)); + when(request.getParameter("connectionId")).thenReturn("octopus-conn-1"); + + final String body = handle(); + + assertThat(body).contains("Spaces-1795").contains("Swordfish"); + assertThat(fetchedKeys).containsExactly("API-STORED"); + } + + @Test + void refusesToFetchWithoutASavedConnection() throws Exception { + // The request must never supply the address to fetch: the only URLs this endpoint will call are + // ones an admin already persisted into project configuration. + when(request.getParameter("serverUrl")).thenReturn("https://attacker.example.com"); + when(request.getParameter("apiKey")).thenReturn("API-TYPED"); + + final String body = handle(); + + assertThat(body).contains("saved Octopus connection"); + assertThat(fetchedUrls).isEmpty(); + } + + @Test + void rejectsAnAnonymousCaller() throws Exception { + currentUser = null; + + final String body = handle(); + + verify(response).setStatus(HttpServletResponse.SC_UNAUTHORIZED); + assertThat(body).contains("not logged in"); + assertThat(fetchedUrls).isEmpty(); + } + + @Test + void rejectsAUserWithoutEditProjectPermission() throws Exception { + // Without this the endpoint would let any logged-in user aim the server at a URL of their + // choosing and test credentials through it. + grantEditProject(false); + + final String body = handle(); + + verify(response).setStatus(HttpServletResponse.SC_FORBIDDEN); + assertThat(body).contains("permission"); + assertThat(fetchedUrls).isEmpty(); + } + + @Test + void rejectsAnUnknownProject() throws Exception { + when(request.getParameter("projectId")).thenReturn("nope"); + + final String body = handle(); + + verify(response).setStatus(HttpServletResponse.SC_BAD_REQUEST); + assertThat(body).contains("Unknown project"); + assertThat(fetchedUrls).isEmpty(); + } + + @Test + void ignoresPostedCredentialsEntirely() throws Exception { + // Posted values are not consulted at all, so they can neither redirect the fetch nor replace + // the stored key. + final Map params = new HashMap<>(); + params.put(ConnectionPropertyNames.SERVER_URL, "https://stored.example.com"); + params.put(ConnectionPropertyNames.API_KEY, "API-STORED"); + final OAuthConnectionDescriptor connection = connectionWith(params); + when(connectionsManager.resolve(eq(project), eq("octopus-conn-1"))) + .thenReturn(Optional.of(connection)); + when(request.getParameter("connectionId")).thenReturn("octopus-conn-1"); + when(request.getParameter("apiKey")).thenReturn("API-ATTACKER-SUPPLIED"); + when(request.getParameter("serverUrl")).thenReturn("https://attacker.example.com"); + + handle(); + + assertThat(fetchedKeys).containsExactly("API-STORED"); + assertThat(fetchedUrls) + .containsExactly("https://stored.example.com/api/spaces?skip=0&take=1000"); + } + + @Test + void explainsThatOnlyAStoredApiKeyCanListSpaces() throws Exception { + final Map params = new HashMap<>(); + params.put(ConnectionPropertyNames.SERVER_URL, "https://stored.example.com"); + params.put(ConnectionPropertyNames.API_KEY_SOURCE, ConnectionPropertyNames.API_KEY_SOURCE_OIDC); + final OAuthConnectionDescriptor connection = connectionWith(params); + when(connectionsManager.resolve(eq(project), eq("octopus-conn-1"))) + .thenReturn(Optional.of(connection)); + when(request.getParameter("connectionId")).thenReturn("octopus-conn-1"); + + assertThat(handle()).contains("stores its API key").contains("space name instead"); + assertThat(fetchedUrls).isEmpty(); + } + + @Test + void reportsAnUnresolvableConnection() throws Exception { + when(connectionsManager.resolve(eq(project), any())).thenReturn(Optional.empty()); + when(request.getParameter("connectionId")).thenReturn("gone"); + + assertThat(handle()).contains("could not be resolved"); + assertThat(fetchedUrls).isEmpty(); + } + + @Test + void surfacesALookupFailureAsAnErrorMessage() throws Exception { + final Map params = new HashMap<>(); + params.put(ConnectionPropertyNames.SERVER_URL, "https://stored.example.com"); + params.put(ConnectionPropertyNames.API_KEY, "API-STORED"); + final OAuthConnectionDescriptor connection = connectionWith(params); + when(connectionsManager.resolve(eq(project), any())).thenReturn(Optional.of(connection)); + when(request.getParameter("connectionId")).thenReturn("octopus-conn-1"); + fetchFailure = new IOException("Octopus rejected the API key (HTTP 401)."); + + final String body = handle(); + + assertThat(body).contains("Octopus rejected the API key"); + // The key itself must never be echoed back to the browser. + assertThat(body).doesNotContain("API-STORED"); + } + + @Test + void neverCachesTheLookup() throws Exception { + final Map params = new HashMap<>(); + params.put(ConnectionPropertyNames.SERVER_URL, "https://stored.example.com"); + params.put(ConnectionPropertyNames.API_KEY, "API-STORED"); + final OAuthConnectionDescriptor connection = connectionWith(params); + when(connectionsManager.resolve(eq(project), any())).thenReturn(Optional.of(connection)); + when(request.getParameter("connectionId")).thenReturn("octopus-conn-1"); + + handle(); + + verify(response).setHeader("Cache-Control", "no-store"); + verify(response, never()).setStatus(HttpServletResponse.SC_FORBIDDEN); + } +} diff --git a/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesFetcherTest.java b/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesFetcherTest.java new file mode 100644 index 00000000..975a0834 --- /dev/null +++ b/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesFetcherTest.java @@ -0,0 +1,225 @@ +/* + * 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.spaces; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +import org.junit.jupiter.api.Test; + +class OctopusSpacesFetcherTest { + + private final List requestedUrls = new ArrayList<>(); + private final List sentApiKeys = new ArrayList<>(); + private final List sentHeaders = new ArrayList<>(); + + private OctopusSpacesFetcher fetcherReturning(final String body) { + return new OctopusSpacesFetcher( + (url, auth) -> { + requestedUrls.add(url); + sentHeaders.add(auth.header()); + sentApiKeys.add(auth.value()); + return body; + }); + } + + @Test + void readsIdAndNameFromTheSpacesCollection() throws Exception { + final OctopusSpacesFetcher fetcher = + fetcherReturning( + "{\"TotalResults\":2,\"Items\":[" + + "{\"Id\":\"Spaces-1\",\"Name\":\"Default\",\"Slug\":\"default\"}," + + "{\"Id\":\"Spaces-1795\",\"Name\":\"Swordfish\"}]}"); + + final List spaces = fetcher.fetch("https://octopus.example.com", "API-KEY"); + + assertThat(spaces).extracting(OctopusSpace::getId).containsExactly("Spaces-1", "Spaces-1795"); + assertThat(spaces).extracting(OctopusSpace::getName).containsExactly("Default", "Swordfish"); + } + + @Test + void ordersSpacesByNameCaseInsensitively() throws Exception { + final OctopusSpacesFetcher fetcher = + fetcherReturning( + "{\"Items\":[{\"Id\":\"Spaces-3\",\"Name\":\"zebra\"}," + + "{\"Id\":\"Spaces-1\",\"Name\":\"Apple\"}," + + "{\"Id\":\"Spaces-2\",\"Name\":\"banana\"}]}"); + + assertThat(fetcher.fetch("https://octopus.example.com", "API-KEY")) + .extracting(OctopusSpace::getName) + .containsExactly("Apple", "banana", "zebra"); + } + + @Test + void skipsItemsWithNoIdAndFallsBackToTheIdWhenUnnamed() throws Exception { + final OctopusSpacesFetcher fetcher = + fetcherReturning("{\"Items\":[{\"Name\":\"no id at all\"},{\"Id\":\"Spaces-9\"}]}"); + + final List spaces = fetcher.fetch("https://octopus.example.com", "API-KEY"); + + assertThat(spaces).extracting(OctopusSpace::getId).containsExactly("Spaces-9"); + assertThat(spaces).extracting(OctopusSpace::getName).containsExactly("Spaces-9"); + } + + @Test + void toleratesAnEmptyCollection() throws Exception { + assertThat(fetcherReturning("{\"TotalResults\":0,\"Items\":[]}").fetch("https://o", "API-KEY")) + .isEmpty(); + } + + @Test + void buildsTheSpacesUrlAndTrimsTrailingSlashes() throws Exception { + final OctopusSpacesFetcher fetcher = fetcherReturning("{\"Items\":[]}"); + + fetcher.fetch("https://octopus.example.com/// ", "API-KEY"); + + assertThat(requestedUrls) + .containsExactly("https://octopus.example.com/api/spaces?skip=0&take=1000"); + } + + @Test + void trimsTheApiKeyBeforeSendingIt() throws Exception { + fetcherReturning("{\"Items\":[]}").fetch("https://octopus.example.com", " API-KEY "); + + assertThat(sentApiKeys).containsExactly("API-KEY"); + } + + @Test + void sendsAnApiKeyInTheOctopusApiKeyHeader() throws Exception { + fetcherReturning("{\"Items\":[]}").fetch("https://octopus.example.com", "API-KEY"); + + assertThat(sentHeaders).containsExactly("X-Octopus-ApiKey"); + assertThat(sentApiKeys).containsExactly("API-KEY"); + } + + @Test + void sendsAnAccessTokenAsABearerToken() throws Exception { + fetcherReturning("{\"Items\":[]}") + .fetch("https://octopus.example.com", OctopusAuth.accessToken(" access-token ")); + + assertThat(sentHeaders).containsExactly("Authorization"); + assertThat(sentApiKeys).containsExactly("Bearer access-token"); + } + + @Test + void rejectsAMissingAccessTokenWithoutCallingOut() { + assertThatThrownBy( + () -> fetcherReturning("{}").fetch("https://o", OctopusAuth.accessToken(" "))) + .isInstanceOf(IOException.class) + .hasMessageContaining("access token"); + assertThat(requestedUrls).isEmpty(); + } + + @Test + void rejectsAMissingUrlOrKeyWithoutCallingOut() { + final OctopusSpacesFetcher fetcher = fetcherReturning("{\"Items\":[]}"); + + assertThatThrownBy(() -> fetcher.fetch(" ", "API-KEY")) + .isInstanceOf(IOException.class) + .hasMessageContaining("Octopus URL is required"); + assertThatThrownBy(() -> fetcher.fetch("https://octopus.example.com", " ")) + .isInstanceOf(IOException.class) + .hasMessageContaining("API key is required"); + assertThat(requestedUrls).isEmpty(); + } + + @Test + void rejectsSchemesOtherThanHttpAndHttps() { + // Without this, URL.openConnection() would happily read a file: or jar: URL. + final OctopusSpacesFetcher fetcher = fetcherReturning("{\"Items\":[]}"); + + for (final String url : + new String[] { + "file:///etc/passwd", "jar:file:/tmp/x.jar!/y", "ftp://octopus.example.com", "gopher://x" + }) { + assertThatThrownBy(() -> fetcher.fetch(url, "API-KEY")) + .isInstanceOf(IOException.class) + .hasMessageContaining("must start with http:// or https://"); + } + assertThat(requestedUrls).isEmpty(); + } + + @Test + void rejectsAUrlWithNoHost() { + // These parse as a URI with the right scheme but no authority, e.g. "http:example.com". + final OctopusSpacesFetcher fetcher = fetcherReturning("{\"Items\":[]}"); + + for (final String url : new String[] {"http:example.com", "https:/onlypath"}) { + assertThatThrownBy(() -> fetcher.fetch(url, "API-KEY")) + .isInstanceOf(IOException.class) + .hasMessageContaining("missing a host name"); + } + assertThat(requestedUrls).isEmpty(); + } + + @Test + void rejectsAMalformedUrl() { + // "https://" and a URL containing a space are not parseable at all. + final OctopusSpacesFetcher fetcher = fetcherReturning("{\"Items\":[]}"); + + for (final String url : new String[] {"https://", "http://exa mple.com"}) { + assertThatThrownBy(() -> fetcher.fetch(url, "API-KEY")) + .isInstanceOf(IOException.class) + .hasMessageContaining("is not a valid URL"); + } + assertThat(requestedUrls).isEmpty(); + } + + @Test + void acceptsPlainHttpAndHttps() throws Exception { + // On-prem Octopus installs are commonly plain http on an internal host, so http must work. + fetcherReturning("{\"Items\":[]}").fetch("http://octopus.internal:8065", "API-KEY"); + fetcherReturning("{\"Items\":[]}").fetch("https://octopus.example.com", "API-KEY"); + + assertThat(requestedUrls) + .containsExactly( + "http://octopus.internal:8065/api/spaces?skip=0&take=1000", + "https://octopus.example.com/api/spaces?skip=0&take=1000"); + } + + @Test + void failsClearlyWhenTheResponseIsNotASpacesCollection() { + // e.g. the URL points at something that is not an Octopus server and returns HTML. + assertThatThrownBy( + () -> fetcherReturning("not octopus").fetch("https://x", "API-KEY")) + .isInstanceOf(IOException.class) + .hasMessageContaining("did not return a spaces list"); + } + + @Test + void failsClearlyWhenTheResponseHasNoItems() { + assertThatThrownBy(() -> fetcherReturning("{\"Links\":{}}").fetch("https://x", "API-KEY")) + .isInstanceOf(IOException.class) + .hasMessageContaining("did not return a spaces list"); + } + + @Test + void propagatesTransportFailures() { + final OctopusSpacesFetcher fetcher = + new OctopusSpacesFetcher( + (url, auth) -> { + throw new IOException("connection refused"); + }); + + assertThatThrownBy(() -> fetcher.fetch("https://octopus.example.com", "API-KEY")) + .isInstanceOf(IOException.class) + .hasMessageContaining("connection refused"); + } +} From d80397ea37e2db5dfa9ebdac26e7d123fae24990 Mon Sep 17 00:00:00 2001 From: Matt Richardson Date: Wed, 14 Oct 2026 10:14:11 +1100 Subject: [PATCH 3/9] feat: suggest a connection's spaces in the space field and store the chosen id Co-Authored-By: Claude Opus 5.5 --- .../teamcity/e2e/dsl/TeamCityRest.java | 14 + .../e2e/test/OctopusSpacePickerUiTest.java | 263 ++++++++++++ .../connection/OctopusConnectionUiData.java | 52 +++ .../buildServerResources/spacePicker.jsp | 381 +++++++++++++++++- .../OctopusConnectionUiDataTest.java | 81 ++++ 5 files changed, 786 insertions(+), 5 deletions(-) create mode 100644 e2e/src/test/java/octopus/teamcity/e2e/test/OctopusSpacePickerUiTest.java diff --git a/e2e/src/test/java/octopus/teamcity/e2e/dsl/TeamCityRest.java b/e2e/src/test/java/octopus/teamcity/e2e/dsl/TeamCityRest.java index d904297d..82488ab0 100644 --- a/e2e/src/test/java/octopus/teamcity/e2e/dsl/TeamCityRest.java +++ b/e2e/src/test/java/octopus/teamcity/e2e/dsl/TeamCityRest.java @@ -287,6 +287,20 @@ public void addBuildInfoStepUsingConnection( json); } + /** Sets one property on an existing build step, as saving the step's form would. */ + public void setStepProperty( + final String buildTypeId, final String stepId, final String name, final String value) + throws Exception { + final String path = + "/httpAuth/app/rest/buildTypes/" + buildTypeId + "/steps/" + stepId + "/parameters/" + name; + // This endpoint answers in plain text, so it refuses the JSON Accept header send() uses. + final Http.Response resp = request("PUT", baseUrl + path, "text/plain", "text/plain", value); + if (resp.statusCode() < 200 || resp.statusCode() >= 300) { + throw new IllegalStateException( + "PUT " + path + " -> " + resp.statusCode() + ": " + resp.body()); + } + } + /** * Adds an empty Create release (octopus.create.release) step; enough for the edit page to render. * Returns the generated runner id (e.g. {@code RUNNER_1}). diff --git a/e2e/src/test/java/octopus/teamcity/e2e/test/OctopusSpacePickerUiTest.java b/e2e/src/test/java/octopus/teamcity/e2e/test/OctopusSpacePickerUiTest.java new file mode 100644 index 00000000..5e0fbae0 --- /dev/null +++ b/e2e/src/test/java/octopus/teamcity/e2e/test/OctopusSpacePickerUiTest.java @@ -0,0 +1,263 @@ +package octopus.teamcity.e2e.test; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.List; + +import com.microsoft.playwright.Locator; +import com.microsoft.playwright.Page; +import com.microsoft.playwright.options.LoadState; +import com.microsoft.playwright.options.WaitForSelectorState; +import com.microsoft.playwright.options.WaitUntilState; +import octopus.teamcity.e2e.dsl.OctopusTeamCityStack; +import octopus.teamcity.e2e.dsl.PlaywrightUi; +import octopus.teamcity.e2e.dsl.SharedStack; +import octopus.teamcity.e2e.dsl.TeamCityRest; +import org.junit.jupiter.api.Test; + +/** + * UI test for the space picker: the space name field suggests the connection's spaces, loaded from + * the Octopus container on their own, and choosing one stores the space's id rather than + * only its name. Anything else typed - such as a parameter reference - is kept as the name. + * + *

Storing the id is the whole point of the picker - a name breaks as soon as the space is + * renamed in Octopus, whereas the id does not. + */ +class OctopusSpacePickerUiTest { + + @Test + void suggestsTheConnectionsSpacesAndStoresTheChosenId() throws Exception { + try (final OctopusTeamCityStack stack = SharedStack.full()) { + final TeamCityRest tc = stack.rest(); + tc.createProject("SpacePickerIT", "Space picker IT"); + // A connection with a real URL and key, so the lookup can actually succeed. The TeamCity + // server makes the call, so it needs the container-network URL. + final String connectionId = + tc.createOctopusConnection( + "SpacePickerIT", + "Real Octopus", + stack.octopusUrlForContainers(), + stack.octopusApiKey(), + ""); + tc.createBuildType("SpacePickerIT_Release", "Create release", "SpacePickerIT"); + final String runnerId = tc.addCreateReleaseStep("SpacePickerIT_Release"); + + PlaywrightUi.withLoggedInPage( + stack, + page -> { + openStep(page, stack, "SpacePickerIT_Release", runnerId); + // No connection yet, so there is nothing to suggest. + assertThat(status(page).innerText()).contains("Select a connection"); + assertNoSpacesListed(page); + + // Choosing the connection loads its spaces without any further click. + page.selectOption("#octopusConnectionId", new String[] {connectionId}); + waitForSpaces(page); + + // A fresh Octopus has the Default space, suggested as " ()". + assertThat(suggestions(page)).contains("Default (Spaces-1)"); + page.locator("ul.ui-autocomplete li", new Page.LocatorOptions().setHasText("Default")) + .first() + .click(); + + assertThat(spaceId(page)).isEqualTo("Spaces-1"); + assertThat(spaceName(page).inputValue()).isEqualTo("Default"); + }); + } + } + + @Test + void keepsATypedNameAndStoresTheIdOnlyForAnExactMatch() throws Exception { + try (final OctopusTeamCityStack stack = SharedStack.full()) { + final TeamCityRest tc = stack.rest(); + tc.createProject("SpacePickerTypedIT", "Space picker typed name IT"); + final String connectionId = + tc.createOctopusConnection( + "SpacePickerTypedIT", + "Real Octopus", + stack.octopusUrlForContainers(), + stack.octopusApiKey(), + ""); + tc.createBuildType("SpacePickerTypedIT_Release", "Create release", "SpacePickerTypedIT"); + final String runnerId = tc.addCreateReleaseStep("SpacePickerTypedIT_Release"); + tc.setStepProperty( + "SpacePickerTypedIT_Release", runnerId, "octopus_connection_id", connectionId); + + PlaywrightUi.withLoggedInPage( + stack, + page -> { + openStep(page, stack, "SpacePickerTypedIT_Release", runnerId); + waitForSpaces(page); + + // Typing narrows the suggestions to spaces whose name contains the text. + spaceName(page).fill(""); + spaceName(page).pressSequentially("defa"); + assertThat(suggestions(page)).containsExactly("Default (Spaces-1)"); + + // An exact name, in any case, picks the space. + spaceName(page).fill("default"); + assertThat(spaceId(page)).isEqualTo("Spaces-1"); + + // After a %, TeamCity's parameters are suggested instead, and choosing one completes + // the reference. + spaceName(page).fill(""); + spaceName(page).pressSequentially("%build.num"); + page.locator( + "ul.ui-autocomplete:visible li", + new Page.LocatorOptions().setHasText("build.number")) + .first() + .click(); + assertThat(spaceName(page).inputValue()).isEqualTo("%build.number%"); + assertThat(spaceId(page)).isEmpty(); + + // A parameter reference is resolved when the build runs, so it is kept as the name and + // not checked against Octopus. + spaceName(page).fill("%octopus.space%"); + spaceName(page).blur(); + assertThat(spaceId(page)).isEmpty(); + assertThat(missingSpace(page).isVisible()).isFalse(); + + // Any other name is kept as typed, and flagged. + spaceName(page).fill("Nowhere"); + spaceName(page).blur(); + assertThat(spaceId(page)).isEmpty(); + assertThat(missingSpace(page).innerText()) + .isEqualTo("No space named 'Nowhere' was found in Octopus."); + }); + } + } + + /** + * The parameter completion comes from TeamCity internals the picker calls into. If those change, + * the field must still suggest spaces and accept whatever is typed. + */ + @Test + void stillSuggestsSpacesWhenTeamCitysParameterCompletionFails() throws Exception { + try (final OctopusTeamCityStack stack = SharedStack.full()) { + final TeamCityRest tc = stack.rest(); + tc.createProject("SpacePickerBrokenIT", "Space picker broken completion IT"); + final String connectionId = + tc.createOctopusConnection( + "SpacePickerBrokenIT", + "Real Octopus", + stack.octopusUrlForContainers(), + stack.octopusApiKey(), + ""); + tc.createBuildType("SpacePickerBrokenIT_Release", "Create release", "SpacePickerBrokenIT"); + final String runnerId = tc.addCreateReleaseStep("SpacePickerBrokenIT_Release"); + tc.setStepProperty( + "SpacePickerBrokenIT_Release", runnerId, "octopus_connection_id", connectionId); + + PlaywrightUi.withLoggedInPage( + stack, + page -> { + final List pageErrors = new java.util.ArrayList<>(); + page.onPageError(pageErrors::add); + openStep(page, stack, "SpacePickerBrokenIT_Release", runnerId); + waitForSpaces(page); + page.evaluate( + "() => { BS.AvailableParams.sliceForCompletion = function () {" + + " throw new Error('changed in this TeamCity version'); };" + + " BS.AvailableParams.sourceForElement = undefined; }"); + + spaceName(page).fill(""); + spaceName(page).pressSequentially("%octo"); + spaceName(page).fill(""); + spaceName(page).pressSequentially("Def"); + assertThat(suggestions(page)).containsExactly("Default (Spaces-1)"); + spaceName(page).fill("%octopus.space%"); + assertThat(spaceName(page).inputValue()).isEqualTo("%octopus.space%"); + assertThat(pageErrors).isEmpty(); + }); + } + } + + @Test + void flagsASavedSpaceThatOctopusDoesNotHave() throws Exception { + try (final OctopusTeamCityStack stack = SharedStack.full()) { + final TeamCityRest tc = stack.rest(); + tc.createProject("SpacePickerMissingIT", "Space picker missing space IT"); + final String connectionId = + tc.createOctopusConnection( + "SpacePickerMissingIT", + "Real Octopus", + stack.octopusUrlForContainers(), + stack.octopusApiKey(), + ""); + tc.createBuildType("SpacePickerMissingIT_Release", "Create release", "SpacePickerMissingIT"); + final String runnerId = tc.addCreateReleaseStep("SpacePickerMissingIT_Release"); + tc.setStepProperty( + "SpacePickerMissingIT_Release", runnerId, "octopus_connection_id", connectionId); + tc.setStepProperty( + "SpacePickerMissingIT_Release", runnerId, "octopus_space_id", "Spaces-999"); + tc.setStepProperty("SpacePickerMissingIT_Release", runnerId, "octopus_space_name", "Gone"); + + PlaywrightUi.withLoggedInPage( + stack, + page -> { + openStep(page, stack, "SpacePickerMissingIT_Release", runnerId); + waitForSpaces(page); + + assertThat(missingSpace(page).innerText()) + .isEqualTo( + "The saved space 'Gone' (Spaces-999) was not found. Please enter the space" + + " name."); + // The saved setting is left alone until someone changes it. + assertThat(spaceId(page)).isEqualTo("Spaces-999"); + assertThat(spaceName(page).inputValue()).isEqualTo("Gone"); + }); + } + } + + private static void openStep( + final Page page, + final OctopusTeamCityStack stack, + final String buildTypeId, + final String runnerId) { + page.navigate( + stack.tcBaseUrl() + + "/admin/editRunType.html?id=buildType:" + + buildTypeId + + "&runnerId=" + + runnerId, + new Page.NavigateOptions().setWaitUntil(WaitUntilState.DOMCONTENTLOADED)); + page.waitForLoadState(LoadState.NETWORKIDLE); + } + + private static Locator status(final Page page) { + return page.locator("span.octopusSpaceStatus").first(); + } + + private static Locator spaceName(final Page page) { + return page.locator("#octopus_space_name").first(); + } + + private static String spaceId(final Page page) { + return page.locator("input[name='prop:octopus_space_id']").first().inputValue(); + } + + private static Locator missingSpace(final Page page) { + return page.locator("span.octopusSpaceMissing").first(); + } + + /** The by-id note is shown exactly when the connection's spaces have been listed. */ + private static void waitForSpaces(final Page page) { + page.locator("span.octopusSpaceByIdNote") + .first() + .waitFor( + new Locator.WaitForOptions().setState(WaitForSelectorState.VISIBLE).setTimeout(60000)); + } + + private static void assertNoSpacesListed(final Page page) { + assertThat(spaceName(page).isVisible()).isTrue(); + assertThat(page.locator("span.octopusSpaceByIdNote").first().isVisible()).isFalse(); + } + + /** What the space name field suggests for what it currently holds. */ + private static List suggestions(final Page page) { + spaceName(page).focus(); + final Locator items = page.locator("ul.ui-autocomplete:visible li"); + items.first().waitFor(new Locator.WaitForOptions().setTimeout(10000)); + return items.allInnerTexts(); + } +} diff --git a/octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnectionUiData.java b/octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnectionUiData.java index 7d156bcd..9d997f45 100644 --- a/octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnectionUiData.java +++ b/octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnectionUiData.java @@ -20,6 +20,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import javax.servlet.http.HttpServletRequest; import jetbrains.buildServer.serverSide.ProjectManager; @@ -170,6 +171,57 @@ public static String buildFeaturesUrl(final HttpServletRequest request) { + buildType.getExternalId(); } + /** + * The project whose form is being edited, for the space picker's permission check. + * + *

A connection page carries {@code projectId} directly; a step page instead carries {@code + * id=buildType:...}, so the project is derived from the build type. + */ + @NotNull + public static String spacePickerProjectId(final HttpServletRequest request) { + final String projectId = request.getParameter("projectId"); + if (projectId != null && !projectId.trim().isEmpty()) { + return projectId.trim(); + } + final String fromBuildType = currentProjectExternalId(request); + return fromBuildType == null ? "" : fromBuildType; + } + + /** + * The Octopus connection this page is editing, or {@code ""} when there is not one yet. + * + *

The space picker needs a saved connection to look spaces up through. On a step form + * the connection selector supplies that; on the connection form itself it has to come from the + * request. TeamCity's own parameter name for it is not contractual, so each plausible name is + * tried and only accepted when it actually resolves to an Octopus connection in this project. An + * unknown name simply yields {@code ""}, and the picker then asks the admin to save first rather + * than failing. + */ + @NotNull + public static String spacePickerConnectionId(final HttpServletRequest request) { + if (connectionsManager == null || projectManager == null) { + return ""; + } + final SProject project = projectManager.findProjectByExternalId(spacePickerProjectId(request)); + if (project == null) { + return ""; + } + for (final String parameterName : + new String[] {"connectionId", "oauthConnectionId", "featureId", "id"}) { + final String candidate = request.getParameter(parameterName); + if (candidate == null || candidate.trim().isEmpty()) { + continue; + } + final Optional resolved = + connectionsManager.resolve(project, candidate.trim()); + if (resolved.isPresent() + && OctopusConnection.TYPE.equals(resolved.get().getParameters().get("providerType"))) { + return candidate.trim(); + } + } + return ""; + } + private static String currentProjectExternalId(final HttpServletRequest request) { final SBuildType buildType = currentBuildType(request); return buildType == null ? null : buildType.getProject().getExternalId(); diff --git a/octopus-server/src/main/resources/buildServerResources/spacePicker.jsp b/octopus-server/src/main/resources/buildServerResources/spacePicker.jsp index 7a7a18ad..d08c6c19 100644 --- a/octopus-server/src/main/resources/buildServerResources/spacePicker.jsp +++ b/octopus-server/src/main/resources/buildServerResources/spacePicker.jsp @@ -1,6 +1,9 @@ <%@ include file="/include-internal.jsp" %> <%@ taglib prefix="props" tagdir="/WEB-INF/tags/props" %> +<%@ taglib prefix="l" tagdir="/WEB-INF/tags/layout" %> +<%@ taglib prefix="bs" tagdir="/WEB-INF/tags" %> <%@ taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core" %> +<%@ page import="octopus.teamcity.server.connection.OctopusConnectionUiData" %> <%-- ~ Copyright (c) Octopus Deploy and contributors. All rights reserved. @@ -17,15 +20,383 @@ ~ specific language governing permissions and limitations under the License. --%> -<%-- Space field shared by the connection form and every step form; both store the space under the - same property name. The including page passes its help text as the "note" param. --%> +<%-- + Space picker, shared by the connection form and every step form. + + A text field that suggests the connection's spaces as you type. Choosing a space, or typing its + exact name, stores the space's *id* alongside the name; the id survives the space being renamed + in Octopus. Anything else typed - including a parameter reference such as %octopus.space%, which + TeamCity resolves when the build runs - is stored as the name alone. + + The property names are the same on a connection and on a step, so OctopusConstants supplies both + regardless of which page is including this. The including page passes its own help text as the + "note" param. +--%> + +<% + pageContext.setAttribute( + "spacePickerProjectId", OctopusConnectionUiData.spacePickerProjectId(request)); + pageContext.setAttribute( + "spacePickerConnectionId", OctopusConnectionUiData.spacePickerConnectionId(request)); +%> - - Space name: + + Space: - + + + + + + + <%-- Why the spaces could not be listed, shown in a tooltip; the reason is in data-reason. --%> + + + + <%-- Config passed as HTML-escaped data attributes rather than interpolated into JS string + literals, so nothing server-side can break out into script. --%> + + + + + diff --git a/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionUiDataTest.java b/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionUiDataTest.java index 01797905..f52fe00d 100644 --- a/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionUiDataTest.java +++ b/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionUiDataTest.java @@ -1,6 +1,7 @@ package octopus.teamcity.server.connection; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -9,6 +10,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import javax.servlet.http.HttpServletRequest; import jetbrains.buildServer.serverSide.ProjectManager; @@ -247,4 +249,83 @@ void buildFeaturesUrlEmptyWhenUnresolved() { assertThat(OctopusConnectionUiData.buildFeaturesUrl(request)).isEqualTo(""); } + + private static HttpServletRequest requestWith(final Map parameters) { + final HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getParameter(anyString())) + .thenAnswer(invocation -> parameters.get(invocation.getArgument(0))); + return request; + } + + private SProject projectNamed(final String externalId) { + final SProject project = mock(SProject.class); + when(projectManager.findProjectByExternalId(externalId)).thenReturn(project); + return project; + } + + private static OAuthConnectionDescriptor connectionOfType(final String providerType) { + final OAuthConnectionDescriptor descriptor = mock(OAuthConnectionDescriptor.class); + when(descriptor.getParameters()) + .thenReturn(Collections.singletonMap("providerType", providerType)); + return descriptor; + } + + @Test + void spacePickerProjectIdIsTheConnectionFormsProject() { + final Map parameters = new HashMap<>(); + parameters.put("projectId", " Rtest "); + + assertThat(OctopusConnectionUiData.spacePickerProjectId(requestWith(parameters))) + .isEqualTo("Rtest"); + } + + @Test + void spacePickerProjectIdIsTheStepFormsBuildConfigurationsProject() { + final SBuildType buildType = mock(SBuildType.class); + final SProject project = mock(SProject.class); + when(projectManager.findBuildTypeByExternalId("Rtest_Build")).thenReturn(buildType); + when(buildType.getProject()).thenReturn(project); + when(project.getExternalId()).thenReturn("Rtest"); + final Map parameters = new HashMap<>(); + parameters.put("id", "buildType:Rtest_Build"); + + assertThat(OctopusConnectionUiData.spacePickerProjectId(requestWith(parameters))) + .isEqualTo("Rtest"); + } + + @Test + void spacePickerConnectionIdIsTheOctopusConnectionBeingEdited() { + final SProject project = projectNamed("Rtest"); + final OAuthConnectionDescriptor octopus = connectionOfType(OctopusConnection.TYPE); + when(connectionsManager.resolve(project, "PROJECT_EXT_7")).thenReturn(Optional.of(octopus)); + final Map parameters = new HashMap<>(); + parameters.put("projectId", "Rtest"); + parameters.put("featureId", "PROJECT_EXT_7"); + + assertThat(OctopusConnectionUiData.spacePickerConnectionId(requestWith(parameters))) + .isEqualTo("PROJECT_EXT_7"); + } + + @Test + void spacePickerConnectionIdIgnoresAConnectionOfAnotherType() { + final SProject project = projectNamed("Rtest"); + final OAuthConnectionDescriptor github = connectionOfType("GitHubApp"); + when(connectionsManager.resolve(project, "PROJECT_EXT_8")).thenReturn(Optional.of(github)); + final Map parameters = new HashMap<>(); + parameters.put("projectId", "Rtest"); + parameters.put("connectionId", "PROJECT_EXT_8"); + + assertThat(OctopusConnectionUiData.spacePickerConnectionId(requestWith(parameters))).isEmpty(); + } + + @Test + void spacePickerConnectionIdIsEmptyForAnUnsavedConnection() { + final SProject project = projectNamed("Rtest"); + when(connectionsManager.resolve(project, "unsaved")).thenReturn(Optional.empty()); + final Map parameters = new HashMap<>(); + parameters.put("projectId", "Rtest"); + parameters.put("connectionId", "unsaved"); + + assertThat(OctopusConnectionUiData.spacePickerConnectionId(requestWith(parameters))).isEmpty(); + } } From 6373389b8e1d8c8e13b0f091447df17015ace360 Mon Sep 17 00:00:00 2001 From: Matt Richardson Date: Wed, 14 Oct 2026 11:38:15 +1100 Subject: [PATCH 4/9] test: use placeholder space ids and names Co-Authored-By: Claude Opus 5.5 --- .../octopus/teamcity/common/OctopusConstants.java | 2 +- .../octopus/teamcity/common/SpaceSelection.java | 2 +- .../teamcity/common/SpaceSelectionTest.java | 14 ++++++-------- .../OctopusConnectionBuildStartProcessorTest.java | 10 +++++----- .../server/connection/OctopusConnectionTest.java | 11 ++++++----- 5 files changed, 19 insertions(+), 20 deletions(-) diff --git a/octopus-common/src/main/java/octopus/teamcity/common/OctopusConstants.java b/octopus-common/src/main/java/octopus/teamcity/common/OctopusConstants.java index 093d11bd..7f4c29ca 100644 --- a/octopus-common/src/main/java/octopus/teamcity/common/OctopusConstants.java +++ b/octopus-common/src/main/java/octopus/teamcity/common/OctopusConstants.java @@ -54,7 +54,7 @@ public String getSpaceName() { } /** - * The id of the selected space, e.g. {@code Spaces-1795}. Preferred over {@link #getSpaceName()} + * The id of the selected space, e.g. {@code Spaces-123}. Preferred over {@link #getSpaceName()} * because it survives a space being renamed. Both CLIs resolve a space by name or id, so either * value works on the wire; see {@link SpaceSelection}. */ diff --git a/octopus-common/src/main/java/octopus/teamcity/common/SpaceSelection.java b/octopus-common/src/main/java/octopus/teamcity/common/SpaceSelection.java index de566f3d..fdc8431f 100644 --- a/octopus-common/src/main/java/octopus/teamcity/common/SpaceSelection.java +++ b/octopus-common/src/main/java/octopus/teamcity/common/SpaceSelection.java @@ -21,7 +21,7 @@ * Works out which space a step should act on. * *

A step (or the connection it inherits from) can identify a space either by id - {@code - * Spaces-1795} - or by name. The id is preferred because it survives the space being renamed, which + * Spaces-123} - or by name. The id is preferred because it survives the space being renamed, which * a name does not. Both CLIs the agent drives accept either: the legacy {@code octo} CLI resolves * {@code --space} with {@code Spaces.FindByNameOrIdOrFail}, and the Go CLI prefers a name match * then falls back to an id. So whichever value we pick here can be passed through unchanged. diff --git a/octopus-common/src/test/java/octopus/teamcity/common/SpaceSelectionTest.java b/octopus-common/src/test/java/octopus/teamcity/common/SpaceSelectionTest.java index 4878ebd6..7ab53553 100644 --- a/octopus-common/src/test/java/octopus/teamcity/common/SpaceSelectionTest.java +++ b/octopus-common/src/test/java/octopus/teamcity/common/SpaceSelectionTest.java @@ -41,26 +41,24 @@ private Map params(final String spaceId, final String spaceName) void prefersTheSpaceIdOverTheSpaceName() { // The whole point: the id keeps working after the space is renamed in Octopus, so a stale // stored name must not win over it. - assertThat(SpaceSelection.resolve(params("Spaces-1795", "Modern Deployments"))) - .isEqualTo("Spaces-1795"); + assertThat(SpaceSelection.resolve(params("Spaces-123", "Example Space"))) + .isEqualTo("Spaces-123"); } @Test void fallsBackToTheSpaceNameWhenNoIdIsStored() { - assertThat(SpaceSelection.resolve(params(null, "Modern Deployments"))) - .isEqualTo("Modern Deployments"); + assertThat(SpaceSelection.resolve(params(null, "Example Space"))).isEqualTo("Example Space"); } @Test void treatsABlankIdAsAbsent() { - assertThat(SpaceSelection.resolve(params(" ", "Modern Deployments"))) - .isEqualTo("Modern Deployments"); + assertThat(SpaceSelection.resolve(params(" ", "Example Space"))).isEqualTo("Example Space"); } @Test void trimsSurroundingWhitespace() { - assertThat(SpaceSelection.resolve(params(null, " Modern Deployments "))) - .isEqualTo("Modern Deployments"); + assertThat(SpaceSelection.resolve(params(null, " Example Space "))) + .isEqualTo("Example Space"); } @Test diff --git a/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionBuildStartProcessorTest.java b/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionBuildStartProcessorTest.java index fd36b55d..c485699f 100644 --- a/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionBuildStartProcessorTest.java +++ b/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionBuildStartProcessorTest.java @@ -96,13 +96,13 @@ void inheritsTheConnectionsSpaceIdAndNameTogether() { properties.put(CONSTANTS.getConnectionIdKey(), "PROJECT_EXT_1"); when(runnerContext.getParameters()).thenReturn(properties); - final OAuthConnectionDescriptor descriptor = connectionWithSpace("Spaces-1795", "Swordfish"); + final OAuthConnectionDescriptor descriptor = connectionWithSpace("Spaces-123", "Example Space"); when(connectionsManager.resolve(project, "PROJECT_EXT_1")).thenReturn(Optional.of(descriptor)); processor.updateParameters(buildStartContext); - verify(runnerContext).addRunnerParameter(CONSTANTS.getSpaceId(), "Spaces-1795"); - verify(runnerContext).addRunnerParameter(CONSTANTS.getSpaceName(), "Swordfish"); + verify(runnerContext).addRunnerParameter(CONSTANTS.getSpaceId(), "Spaces-123"); + verify(runnerContext).addRunnerParameter(CONSTANTS.getSpaceName(), "Example Space"); } @Test @@ -114,7 +114,7 @@ void aStepSpaceNameBlocksInheritingTheConnectionsSpaceId() { properties.put(CONSTANTS.getSpaceName(), "StepSpace"); when(runnerContext.getParameters()).thenReturn(properties); - final OAuthConnectionDescriptor descriptor = connectionWithSpace("Spaces-1795", "Swordfish"); + final OAuthConnectionDescriptor descriptor = connectionWithSpace("Spaces-123", "Example Space"); when(connectionsManager.resolve(project, "PROJECT_EXT_1")).thenReturn(Optional.of(descriptor)); processor.updateParameters(buildStartContext); @@ -136,7 +136,7 @@ void aStepSpaceIdBlocksInheritingTheConnectionsSpace() { properties.put(CONSTANTS.getSpaceId(), "Spaces-42"); when(runnerContext.getParameters()).thenReturn(properties); - final OAuthConnectionDescriptor descriptor = connectionWithSpace("Spaces-1795", "Swordfish"); + final OAuthConnectionDescriptor descriptor = connectionWithSpace("Spaces-123", "Example Space"); when(connectionsManager.resolve(project, "PROJECT_EXT_1")).thenReturn(Optional.of(descriptor)); processor.updateParameters(buildStartContext); diff --git a/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionTest.java b/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionTest.java index e465114f..bc0be217 100644 --- a/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionTest.java +++ b/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionTest.java @@ -63,21 +63,22 @@ void describeConnectionShowsTheSpaceIdBesideTheName() { // the space has been renamed in Octopus. final Map params = new HashMap<>(); params.put(ConnectionPropertyNames.SERVER_URL, "https://octopus.example.com"); - params.put(ConnectionPropertyNames.SPACE_NAME, "Swordfish"); - params.put(ConnectionPropertyNames.SPACE_ID, "Spaces-1795"); + params.put(ConnectionPropertyNames.SPACE_NAME, "Example Space"); + params.put(ConnectionPropertyNames.SPACE_ID, "Spaces-123"); assertThat(provider.describeConnection(descriptorWith(params))) - .isEqualTo("Octopus URL: https://octopus.example.com\nSpace Name: Swordfish (Spaces-1795)"); + .isEqualTo( + "Octopus URL: https://octopus.example.com\nSpace Name: Example Space (Spaces-123)"); } @Test void describeConnectionShowsTheSpaceIdAloneWhenNoNameIsStored() { final Map params = new HashMap<>(); params.put(ConnectionPropertyNames.SERVER_URL, "https://octopus.example.com"); - params.put(ConnectionPropertyNames.SPACE_ID, "Spaces-1795"); + params.put(ConnectionPropertyNames.SPACE_ID, "Spaces-123"); assertThat(provider.describeConnection(descriptorWith(params))) - .isEqualTo("Octopus URL: https://octopus.example.com\nSpace Id: Spaces-1795"); + .isEqualTo("Octopus URL: https://octopus.example.com\nSpace Id: Spaces-123"); } @Test From d86f110cf3ac736ebb9a7d9e9da2778aabf1ef31 Mon Sep 17 00:00:00 2001 From: Matt Richardson Date: Wed, 14 Oct 2026 11:38:16 +1100 Subject: [PATCH 5/9] test: use placeholder space ids and names Co-Authored-By: Claude Opus 5.5 --- .../server/spaces/OctopusSpacesControllerTest.java | 4 ++-- .../teamcity/server/spaces/OctopusSpacesFetcherTest.java | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesControllerTest.java b/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesControllerTest.java index 5bc8a8f6..06afa074 100644 --- a/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesControllerTest.java +++ b/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesControllerTest.java @@ -87,7 +87,7 @@ private OctopusSpacesController controller() { if (fetchFailure != null) { throw fetchFailure; } - return "{\"Items\":[{\"Id\":\"Spaces-1795\",\"Name\":\"Swordfish\"}]}"; + return "{\"Items\":[{\"Id\":\"Spaces-123\",\"Name\":\"Example Space\"}]}"; }); return new OctopusSpacesController( null, connectionsManager, projectManager, fetcher, req -> currentUser); @@ -116,7 +116,7 @@ void returnsSpacesForASavedConnection() throws Exception { final String body = handle(); - assertThat(body).contains("Spaces-1795").contains("Swordfish"); + assertThat(body).contains("Spaces-123").contains("Example Space"); assertThat(fetchedKeys).containsExactly("API-STORED"); } diff --git a/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesFetcherTest.java b/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesFetcherTest.java index 975a0834..db0ddf2d 100644 --- a/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesFetcherTest.java +++ b/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesFetcherTest.java @@ -46,12 +46,14 @@ void readsIdAndNameFromTheSpacesCollection() throws Exception { fetcherReturning( "{\"TotalResults\":2,\"Items\":[" + "{\"Id\":\"Spaces-1\",\"Name\":\"Default\",\"Slug\":\"default\"}," - + "{\"Id\":\"Spaces-1795\",\"Name\":\"Swordfish\"}]}"); + + "{\"Id\":\"Spaces-123\",\"Name\":\"Example Space\"}]}"); final List spaces = fetcher.fetch("https://octopus.example.com", "API-KEY"); - assertThat(spaces).extracting(OctopusSpace::getId).containsExactly("Spaces-1", "Spaces-1795"); - assertThat(spaces).extracting(OctopusSpace::getName).containsExactly("Default", "Swordfish"); + assertThat(spaces).extracting(OctopusSpace::getId).containsExactly("Spaces-1", "Spaces-123"); + assertThat(spaces) + .extracting(OctopusSpace::getName) + .containsExactly("Default", "Example Space"); } @Test From 91103bae3766de0a21c375f3b7bb7930b23dff63 Mon Sep 17 00:00:00 2001 From: Matt Richardson Date: Wed, 14 Oct 2026 11:59:34 +1100 Subject: [PATCH 6/9] refactor: move Octopus HTTP calls and credentials out of the spaces package Co-Authored-By: Claude Opus 5.5 --- .../{spaces => client}/OctopusAuth.java | 4 +- .../server/client/OctopusHttpClient.java | 124 ++++++++++++++++++ .../spaces/OctopusSpacesController.java | 1 + .../server/spaces/OctopusSpacesFetcher.java | 97 +------------- .../server/client/OctopusAuthTest.java | 53 ++++++++ .../server/client/OctopusHttpClientTest.java | 83 ++++++++++++ .../spaces/OctopusSpacesFetcherTest.java | 76 ++--------- 7 files changed, 280 insertions(+), 158 deletions(-) rename octopus-server/src/main/java/octopus/teamcity/server/{spaces => client}/OctopusAuth.java (96%) create mode 100644 octopus-server/src/main/java/octopus/teamcity/server/client/OctopusHttpClient.java create mode 100644 octopus-server/src/test/java/octopus/teamcity/server/client/OctopusAuthTest.java create mode 100644 octopus-server/src/test/java/octopus/teamcity/server/client/OctopusHttpClientTest.java diff --git a/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusAuth.java b/octopus-server/src/main/java/octopus/teamcity/server/client/OctopusAuth.java similarity index 96% rename from octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusAuth.java rename to octopus-server/src/main/java/octopus/teamcity/server/client/OctopusAuth.java index 1371a45c..1de608d2 100644 --- a/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusAuth.java +++ b/octopus-server/src/main/java/octopus/teamcity/server/client/OctopusAuth.java @@ -13,7 +13,7 @@ * specific language governing permissions and limitations under the License. */ -package octopus.teamcity.server.spaces; +package octopus.teamcity.server.client; /** The header a request to Octopus authenticates with: an API key, or an OIDC access token. */ public final class OctopusAuth { @@ -51,7 +51,7 @@ public String description() { return description; } - boolean isEmpty() { + public boolean isEmpty() { return value.isEmpty(); } diff --git a/octopus-server/src/main/java/octopus/teamcity/server/client/OctopusHttpClient.java b/octopus-server/src/main/java/octopus/teamcity/server/client/OctopusHttpClient.java new file mode 100644 index 00000000..195fcc50 --- /dev/null +++ b/octopus-server/src/main/java/octopus/teamcity/server/client/OctopusHttpClient.java @@ -0,0 +1,124 @@ +/* + * 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.client; + +import java.io.IOException; +import java.io.InputStream; +import java.net.HttpURLConnection; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.URL; +import java.nio.charset.StandardCharsets; + +import org.apache.commons.io.IOUtils; + +/** Makes requests from the TeamCity server to an Octopus server. */ +public class OctopusHttpClient { + + private static final int CONNECT_TIMEOUT_MILLIS = 10_000; + private static final int READ_TIMEOUT_MILLIS = 20_000; + + /** + * The Octopus server URL without trailing slashes, rejecting anything that is not a plain http(s) + * address. + * + *

The URL comes from a connection's settings, so the scheme is restricted to http and https - + * without that, a value such as {@code file:} or {@code jar:} would make {@link + * URL#openConnection()} read from somewhere other than a web server. A host is required so a + * scheme-only value cannot slip through. + */ + public static String baseUrl(final String serverUrl) throws IOException { + if (serverUrl == null || serverUrl.trim().isEmpty()) { + throw new IOException("An Octopus URL is required."); + } + String base = serverUrl.trim(); + while (base.endsWith("/")) { + base = base.substring(0, base.length() - 1); + } + + final URI parsed; + try { + parsed = new URI(base); + } catch (final URISyntaxException e) { + throw new IOException("'" + base + "' is not a valid URL.", e); + } + final String scheme = parsed.getScheme() == null ? "" : parsed.getScheme().toLowerCase(); + if (!"http".equals(scheme) && !"https".equals(scheme)) { + throw new IOException("The Octopus URL must start with http:// or https://."); + } + if (parsed.getHost() == null || parsed.getHost().isEmpty()) { + throw new IOException("The Octopus URL is missing a host name."); + } + + return base; + } + + /** + * The body of a successful GET. + * + * @throws IOException if the server cannot be reached, rejects the credential, redirects, or + * returns a non-2xx status. + */ + public String get(final String url, final OctopusAuth auth) throws IOException { + final HttpURLConnection connection = open(url); + try { + connection.setRequestMethod("GET"); + connection.setRequestProperty(auth.header(), auth.value()); + + final int status = connection.getResponseCode(); + if (status == HttpURLConnection.HTTP_UNAUTHORIZED + || status == HttpURLConnection.HTTP_FORBIDDEN) { + throw new IOException( + "Octopus rejected the " + + auth.description() + + " (HTTP " + + status + + "). Check its permissions."); + } + return readSuccessfulBody(connection, status); + } finally { + connection.disconnect(); + } + } + + private static HttpURLConnection open(final String url) throws IOException { + final HttpURLConnection connection = (HttpURLConnection) new URL(url).openConnection(); + // A redirect would take the request somewhere other than the validated address, and would + // also replay the credential there. + connection.setInstanceFollowRedirects(false); + connection.setConnectTimeout(CONNECT_TIMEOUT_MILLIS); + connection.setReadTimeout(READ_TIMEOUT_MILLIS); + connection.setRequestProperty("Accept", "application/json"); + return connection; + } + + private static String readSuccessfulBody(final HttpURLConnection connection, final int status) + throws IOException { + if (status >= 300 && status <= 399) { + throw new IOException( + "The Octopus URL redirected (HTTP " + + status + + "). Point it directly at the Octopus server."); + } + if (status < 200 || status > 299) { + throw new IOException( + "Octopus returned HTTP " + status + " for " + connection.getURL().getPath() + "."); + } + try (InputStream in = connection.getInputStream()) { + return IOUtils.toString(in, StandardCharsets.UTF_8); + } + } +} diff --git a/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesController.java b/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesController.java index d21697d9..fa763794 100644 --- a/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesController.java +++ b/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesController.java @@ -36,6 +36,7 @@ import jetbrains.buildServer.web.openapi.WebControllerManager; import jetbrains.buildServer.web.util.SessionUser; import octopus.teamcity.common.connection.ConnectionPropertyNames; +import octopus.teamcity.server.client.OctopusAuth; import octopus.teamcity.server.connection.OctopusConnectionsManager; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; diff --git a/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesFetcher.java b/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesFetcher.java index 4e3607e2..11fb4bcc 100644 --- a/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesFetcher.java +++ b/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesFetcher.java @@ -16,12 +16,6 @@ package octopus.teamcity.server.spaces; import java.io.IOException; -import java.io.InputStream; -import java.net.HttpURLConnection; -import java.net.URI; -import java.net.URISyntaxException; -import java.net.URL; -import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; @@ -30,25 +24,21 @@ import com.google.gson.Gson; import com.google.gson.GsonBuilder; import com.google.gson.JsonSyntaxException; -import org.apache.commons.io.IOUtils; +import octopus.teamcity.server.client.OctopusAuth; +import octopus.teamcity.server.client.OctopusHttpClient; /** * Lists the spaces on an Octopus server, so the UI can offer them as a dropdown rather than asking * an admin to type a space name by hand. - * - *

Follows {@code CommitHistoryFetcher}'s shape: the HTTP call sits behind {@link Requester} so - * the parsing and URL construction can be tested without a server. */ public class OctopusSpacesFetcher { /** How many spaces we ask for. Far above any realistic instance. */ static final int TAKE = 1000; - private static final int CONNECT_TIMEOUT_MILLIS = 10_000; - private static final int READ_TIMEOUT_MILLIS = 20_000; private static final Gson GSON = new GsonBuilder().create(); - /** Performs the GET. Separated out so tests need no server. */ + /** Performs the GET, so tests need no server. */ public interface Requester { String get(String url, OctopusAuth auth) throws IOException; } @@ -56,7 +46,7 @@ public interface Requester { private final Requester requester; public OctopusSpacesFetcher() { - this(OctopusSpacesFetcher::httpGet); + this(new OctopusHttpClient()::get); } OctopusSpacesFetcher(final Requester requester) { @@ -76,14 +66,12 @@ public List fetch(final String serverUrl, final String apiKey) thr */ public List fetch(final String serverUrl, final OctopusAuth auth) throws IOException { - if (serverUrl == null || serverUrl.trim().isEmpty()) { - throw new IOException("An Octopus URL is required to list spaces."); - } + final String url = spacesUrl(serverUrl); if (auth.isEmpty()) { throw new IOException("An Octopus " + auth.description() + " is required to list spaces."); } - final String body = requester.get(spacesUrl(serverUrl), auth); + final String body = requester.get(url, auth); final SpacesCollection collection; try { @@ -110,79 +98,8 @@ public List fetch(final String serverUrl, final OctopusAuth auth) return spaces; } - /** - * Builds the spaces URL, rejecting anything that is not a plain http(s) address. - * - *

The caller supplies this URL when configuring a connection, so the scheme is restricted to - * http and https - without that, a value such as {@code file:} or {@code jar:} would make {@link - * URL#openConnection()} read from somewhere other than a web server. A host is required so a - * scheme-only value cannot slip through. - */ static String spacesUrl(final String serverUrl) throws IOException { - return baseUrl(serverUrl) + "/api/spaces?skip=0&take=" + TAKE; - } - - /** The server URL without trailing slashes, after the checks {@link #spacesUrl} describes. */ - static String baseUrl(final String serverUrl) throws IOException { - String base = serverUrl.trim(); - while (base.endsWith("/")) { - base = base.substring(0, base.length() - 1); - } - - final URI parsed; - try { - parsed = new URI(base); - } catch (final URISyntaxException e) { - throw new IOException("'" + base + "' is not a valid URL.", e); - } - final String scheme = parsed.getScheme() == null ? "" : parsed.getScheme().toLowerCase(); - if (!"http".equals(scheme) && !"https".equals(scheme)) { - throw new IOException("The Octopus URL must start with http:// or https://."); - } - if (parsed.getHost() == null || parsed.getHost().isEmpty()) { - throw new IOException("The Octopus URL is missing a host name."); - } - - return base; - } - - private static String httpGet(final String url, final OctopusAuth auth) throws IOException { - final HttpURLConnection connection = (HttpURLConnection) new URL(url).openConnection(); - try { - connection.setRequestMethod("GET"); - // A redirect would take the request somewhere other than the address just validated, and - // would also replay the credential there. - connection.setInstanceFollowRedirects(false); - connection.setConnectTimeout(CONNECT_TIMEOUT_MILLIS); - connection.setReadTimeout(READ_TIMEOUT_MILLIS); - connection.setRequestProperty(auth.header(), auth.value()); - connection.setRequestProperty("Accept", "application/json"); - - final int status = connection.getResponseCode(); - if (status == HttpURLConnection.HTTP_UNAUTHORIZED - || status == HttpURLConnection.HTTP_FORBIDDEN) { - throw new IOException( - "Octopus rejected the " - + auth.description() - + " (HTTP " - + status - + "). Check its permissions."); - } - if (status >= 300 && status <= 399) { - throw new IOException( - "The Octopus URL redirected (HTTP " - + status - + "). Point it directly at the Octopus server."); - } - if (status < 200 || status > 299) { - throw new IOException("Octopus returned HTTP " + status + " when listing spaces."); - } - try (InputStream in = connection.getInputStream()) { - return IOUtils.toString(in, StandardCharsets.UTF_8); - } - } finally { - connection.disconnect(); - } + return OctopusHttpClient.baseUrl(serverUrl) + "/api/spaces?skip=0&take=" + TAKE; } /** Shape of the subset of {@code /api/spaces} we read. Field names match the JSON. */ diff --git a/octopus-server/src/test/java/octopus/teamcity/server/client/OctopusAuthTest.java b/octopus-server/src/test/java/octopus/teamcity/server/client/OctopusAuthTest.java new file mode 100644 index 00000000..4999af47 --- /dev/null +++ b/octopus-server/src/test/java/octopus/teamcity/server/client/OctopusAuthTest.java @@ -0,0 +1,53 @@ +/* + * 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.client; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; + +class OctopusAuthTest { + + @Test + void sendsAnApiKeyInTheOctopusApiKeyHeader() { + final OctopusAuth auth = OctopusAuth.apiKey("API-KEY"); + + assertThat(auth.header()).isEqualTo("X-Octopus-ApiKey"); + assertThat(auth.value()).isEqualTo("API-KEY"); + } + + @Test + void sendsAnAccessTokenAsABearerToken() { + final OctopusAuth auth = OctopusAuth.accessToken("access-token"); + + assertThat(auth.header()).isEqualTo("Authorization"); + assertThat(auth.value()).isEqualTo("Bearer access-token"); + } + + @Test + void trimsTheCredential() { + assertThat(OctopusAuth.apiKey(" API-KEY ").value()).isEqualTo("API-KEY"); + assertThat(OctopusAuth.accessToken(" access-token ").value()) + .isEqualTo("Bearer access-token"); + } + + @Test + void treatsABlankOrMissingCredentialAsEmpty() { + assertThat(OctopusAuth.apiKey(" ").isEmpty()).isTrue(); + assertThat(OctopusAuth.apiKey(null).isEmpty()).isTrue(); + assertThat(OctopusAuth.accessToken(" ").isEmpty()).isTrue(); + } +} diff --git a/octopus-server/src/test/java/octopus/teamcity/server/client/OctopusHttpClientTest.java b/octopus-server/src/test/java/octopus/teamcity/server/client/OctopusHttpClientTest.java new file mode 100644 index 00000000..d6baa189 --- /dev/null +++ b/octopus-server/src/test/java/octopus/teamcity/server/client/OctopusHttpClientTest.java @@ -0,0 +1,83 @@ +/* + * 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.client; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.io.IOException; + +import org.junit.jupiter.api.Test; + +class OctopusHttpClientTest { + + @Test + void trimsWhitespaceAndTrailingSlashes() throws Exception { + assertThat(OctopusHttpClient.baseUrl(" https://octopus.example.com/// ")) + .isEqualTo("https://octopus.example.com"); + } + + @Test + void acceptsPlainHttpAndHttps() throws Exception { + // On-prem Octopus installs are commonly plain http on an internal host, so http must work. + assertThat(OctopusHttpClient.baseUrl("http://octopus.internal:8065")) + .isEqualTo("http://octopus.internal:8065"); + assertThat(OctopusHttpClient.baseUrl("https://octopus.example.com")) + .isEqualTo("https://octopus.example.com"); + } + + @Test + void rejectsAMissingUrl() { + for (final String url : new String[] {null, "", " "}) { + assertThatThrownBy(() -> OctopusHttpClient.baseUrl(url)) + .isInstanceOf(IOException.class) + .hasMessageContaining("Octopus URL is required"); + } + } + + @Test + void rejectsSchemesOtherThanHttpAndHttps() { + // Without this, URL.openConnection() would happily read a file: or jar: URL. + for (final String url : + new String[] { + "file:///etc/passwd", "jar:file:/tmp/x.jar!/y", "ftp://octopus.example.com", "gopher://x" + }) { + assertThatThrownBy(() -> OctopusHttpClient.baseUrl(url)) + .isInstanceOf(IOException.class) + .hasMessageContaining("must start with http:// or https://"); + } + } + + @Test + void rejectsAUrlWithNoHost() { + // These parse as a URI with the right scheme but no authority, e.g. "http:example.com". + for (final String url : new String[] {"http:example.com", "https:/onlypath"}) { + assertThatThrownBy(() -> OctopusHttpClient.baseUrl(url)) + .isInstanceOf(IOException.class) + .hasMessageContaining("missing a host name"); + } + } + + @Test + void rejectsAMalformedUrl() { + // "https://" and a URL containing a space are not parseable at all. + for (final String url : new String[] {"https://", "http://exa mple.com"}) { + assertThatThrownBy(() -> OctopusHttpClient.baseUrl(url)) + .isInstanceOf(IOException.class) + .hasMessageContaining("is not a valid URL"); + } + } +} diff --git a/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesFetcherTest.java b/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesFetcherTest.java index db0ddf2d..a6f4d9e8 100644 --- a/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesFetcherTest.java +++ b/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesFetcherTest.java @@ -22,20 +22,19 @@ import java.util.ArrayList; import java.util.List; +import octopus.teamcity.server.client.OctopusAuth; import org.junit.jupiter.api.Test; class OctopusSpacesFetcherTest { private final List requestedUrls = new ArrayList<>(); - private final List sentApiKeys = new ArrayList<>(); - private final List sentHeaders = new ArrayList<>(); + private final List sentAuth = new ArrayList<>(); private OctopusSpacesFetcher fetcherReturning(final String body) { return new OctopusSpacesFetcher( (url, auth) -> { requestedUrls.add(url); - sentHeaders.add(auth.header()); - sentApiKeys.add(auth.value()); + sentAuth.add(auth); return body; }); } @@ -97,27 +96,11 @@ void buildsTheSpacesUrlAndTrimsTrailingSlashes() throws Exception { } @Test - void trimsTheApiKeyBeforeSendingIt() throws Exception { - fetcherReturning("{\"Items\":[]}").fetch("https://octopus.example.com", " API-KEY "); - - assertThat(sentApiKeys).containsExactly("API-KEY"); - } - - @Test - void sendsAnApiKeyInTheOctopusApiKeyHeader() throws Exception { + void sendsTheApiKey() throws Exception { fetcherReturning("{\"Items\":[]}").fetch("https://octopus.example.com", "API-KEY"); - assertThat(sentHeaders).containsExactly("X-Octopus-ApiKey"); - assertThat(sentApiKeys).containsExactly("API-KEY"); - } - - @Test - void sendsAnAccessTokenAsABearerToken() throws Exception { - fetcherReturning("{\"Items\":[]}") - .fetch("https://octopus.example.com", OctopusAuth.accessToken(" access-token ")); - - assertThat(sentHeaders).containsExactly("Authorization"); - assertThat(sentApiKeys).containsExactly("Bearer access-token"); + assertThat(sentAuth).extracting(OctopusAuth::header).containsExactly("X-Octopus-ApiKey"); + assertThat(sentAuth).extracting(OctopusAuth::value).containsExactly("API-KEY"); } @Test @@ -130,60 +113,21 @@ void rejectsAMissingAccessTokenWithoutCallingOut() { } @Test - void rejectsAMissingUrlOrKeyWithoutCallingOut() { + void rejectsAnInvalidUrlOrMissingKeyWithoutCallingOut() { final OctopusSpacesFetcher fetcher = fetcherReturning("{\"Items\":[]}"); assertThatThrownBy(() -> fetcher.fetch(" ", "API-KEY")) .isInstanceOf(IOException.class) .hasMessageContaining("Octopus URL is required"); + assertThatThrownBy(() -> fetcher.fetch("file:///etc/passwd", "API-KEY")) + .isInstanceOf(IOException.class) + .hasMessageContaining("must start with http:// or https://"); assertThatThrownBy(() -> fetcher.fetch("https://octopus.example.com", " ")) .isInstanceOf(IOException.class) .hasMessageContaining("API key is required"); assertThat(requestedUrls).isEmpty(); } - @Test - void rejectsSchemesOtherThanHttpAndHttps() { - // Without this, URL.openConnection() would happily read a file: or jar: URL. - final OctopusSpacesFetcher fetcher = fetcherReturning("{\"Items\":[]}"); - - for (final String url : - new String[] { - "file:///etc/passwd", "jar:file:/tmp/x.jar!/y", "ftp://octopus.example.com", "gopher://x" - }) { - assertThatThrownBy(() -> fetcher.fetch(url, "API-KEY")) - .isInstanceOf(IOException.class) - .hasMessageContaining("must start with http:// or https://"); - } - assertThat(requestedUrls).isEmpty(); - } - - @Test - void rejectsAUrlWithNoHost() { - // These parse as a URI with the right scheme but no authority, e.g. "http:example.com". - final OctopusSpacesFetcher fetcher = fetcherReturning("{\"Items\":[]}"); - - for (final String url : new String[] {"http:example.com", "https:/onlypath"}) { - assertThatThrownBy(() -> fetcher.fetch(url, "API-KEY")) - .isInstanceOf(IOException.class) - .hasMessageContaining("missing a host name"); - } - assertThat(requestedUrls).isEmpty(); - } - - @Test - void rejectsAMalformedUrl() { - // "https://" and a URL containing a space are not parseable at all. - final OctopusSpacesFetcher fetcher = fetcherReturning("{\"Items\":[]}"); - - for (final String url : new String[] {"https://", "http://exa mple.com"}) { - assertThatThrownBy(() -> fetcher.fetch(url, "API-KEY")) - .isInstanceOf(IOException.class) - .hasMessageContaining("is not a valid URL"); - } - assertThat(requestedUrls).isEmpty(); - } - @Test void acceptsPlainHttpAndHttps() throws Exception { // On-prem Octopus installs are commonly plain http on an internal host, so http must work. From 6f8758f35f4de29f7edd6c61ccc5b2027232e0ac Mon Sep 17 00:00:00 2001 From: Matt Richardson Date: Wed, 14 Oct 2026 15:52:55 +1100 Subject: [PATCH 7/9] fix: list every space, and log why a spaces response could not be read Co-Authored-By: Claude Opus 5.5 --- .../server/spaces/OctopusSpacesFetcher.java | 30 +++++++++++++------ .../spaces/OctopusSpacesControllerTest.java | 2 +- .../spaces/OctopusSpacesFetcherTest.java | 10 +++---- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesFetcher.java b/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesFetcher.java index 11fb4bcc..8f149dfc 100644 --- a/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesFetcher.java +++ b/octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesFetcher.java @@ -24,6 +24,7 @@ import com.google.gson.Gson; import com.google.gson.GsonBuilder; import com.google.gson.JsonSyntaxException; +import jetbrains.buildServer.log.Loggers; import octopus.teamcity.server.client.OctopusAuth; import octopus.teamcity.server.client.OctopusHttpClient; @@ -33,9 +34,10 @@ */ public class OctopusSpacesFetcher { - /** How many spaces we ask for. Far above any realistic instance. */ - static final int TAKE = 1000; + /** Asks for every space in one page. */ + static final int TAKE = Integer.MAX_VALUE; + private static final int LOGGED_BODY_LENGTH = 1000; private static final Gson GSON = new GsonBuilder().create(); /** Performs the GET, so tests need no server. */ @@ -77,15 +79,10 @@ public List fetch(final String serverUrl, final OctopusAuth auth) try { collection = GSON.fromJson(body, SpacesCollection.class); } catch (final JsonSyntaxException e) { - throw new IOException( - "The Octopus server did not return a spaces list. Check the URL points at an Octopus" - + " server.", - e); + throw notASpacesList(url, body, e); } if (collection == null || collection.Items == null) { - throw new IOException( - "The Octopus server did not return a spaces list. Check the URL points at an Octopus" - + " server."); + throw notASpacesList(url, body, null); } final List spaces = new ArrayList<>(); @@ -98,6 +95,21 @@ public List fetch(final String serverUrl, final OctopusAuth auth) return spaces; } + private static IOException notASpacesList( + final String url, final String body, final JsonSyntaxException cause) { + Loggers.SERVER.warn( + "The response from " + + url + + " was not an Octopus spaces list. Response body (first " + + LOGGED_BODY_LENGTH + + " characters): " + + (body == null || body.length() <= LOGGED_BODY_LENGTH + ? body + : body.substring(0, LOGGED_BODY_LENGTH)), + cause); + return new IOException("Unable to retrieve spaces from the Octopus Server."); + } + static String spacesUrl(final String serverUrl) throws IOException { return OctopusHttpClient.baseUrl(serverUrl) + "/api/spaces?skip=0&take=" + TAKE; } diff --git a/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesControllerTest.java b/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesControllerTest.java index 06afa074..c4127e22 100644 --- a/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesControllerTest.java +++ b/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesControllerTest.java @@ -186,7 +186,7 @@ void ignoresPostedCredentialsEntirely() throws Exception { assertThat(fetchedKeys).containsExactly("API-STORED"); assertThat(fetchedUrls) - .containsExactly("https://stored.example.com/api/spaces?skip=0&take=1000"); + .containsExactly("https://stored.example.com/api/spaces?skip=0&take=" + Integer.MAX_VALUE); } @Test diff --git a/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesFetcherTest.java b/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesFetcherTest.java index a6f4d9e8..629b3a98 100644 --- a/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesFetcherTest.java +++ b/octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesFetcherTest.java @@ -92,7 +92,7 @@ void buildsTheSpacesUrlAndTrimsTrailingSlashes() throws Exception { fetcher.fetch("https://octopus.example.com/// ", "API-KEY"); assertThat(requestedUrls) - .containsExactly("https://octopus.example.com/api/spaces?skip=0&take=1000"); + .containsExactly("https://octopus.example.com/api/spaces?skip=0&take=" + Integer.MAX_VALUE); } @Test @@ -136,8 +136,8 @@ void acceptsPlainHttpAndHttps() throws Exception { assertThat(requestedUrls) .containsExactly( - "http://octopus.internal:8065/api/spaces?skip=0&take=1000", - "https://octopus.example.com/api/spaces?skip=0&take=1000"); + "http://octopus.internal:8065/api/spaces?skip=0&take=" + Integer.MAX_VALUE, + "https://octopus.example.com/api/spaces?skip=0&take=" + Integer.MAX_VALUE); } @Test @@ -146,14 +146,14 @@ void failsClearlyWhenTheResponseIsNotASpacesCollection() { assertThatThrownBy( () -> fetcherReturning("not octopus").fetch("https://x", "API-KEY")) .isInstanceOf(IOException.class) - .hasMessageContaining("did not return a spaces list"); + .hasMessageContaining("Unable to retrieve spaces from the Octopus Server."); } @Test void failsClearlyWhenTheResponseHasNoItems() { assertThatThrownBy(() -> fetcherReturning("{\"Links\":{}}").fetch("https://x", "API-KEY")) .isInstanceOf(IOException.class) - .hasMessageContaining("did not return a spaces list"); + .hasMessageContaining("Unable to retrieve spaces from the Octopus Server."); } @Test From 70ec3f20e6e6a23ff89792215434b456dfc4f664 Mon Sep 17 00:00:00 2001 From: Matt Richardson Date: Wed, 14 Oct 2026 19:56:16 +1100 Subject: [PATCH 8/9] refactor: move the space picker's script out of the JSP Co-Authored-By: Claude Opus 5.5 --- .../connection/OctopusConnectionUiData.java | 22 +- .../buildServerResources/js/spacePicker.js | 412 ++++++++++++++++++ .../buildServerResources/spacePicker.jsp | 344 +-------------- .../OctopusConnectionUiDataTest.java | 16 +- 4 files changed, 468 insertions(+), 326 deletions(-) create mode 100644 octopus-server/src/main/resources/buildServerResources/js/spacePicker.js diff --git a/octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnectionUiData.java b/octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnectionUiData.java index 9d997f45..aa2baba5 100644 --- a/octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnectionUiData.java +++ b/octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnectionUiData.java @@ -29,6 +29,7 @@ import jetbrains.buildServer.serverSide.SProject; import jetbrains.buildServer.serverSide.oauth.OAuthConnectionDescriptor; import jetbrains.buildServer.users.SUser; +import jetbrains.buildServer.web.openapi.PluginDescriptor; import jetbrains.buildServer.web.util.SessionUser; import octopus.teamcity.common.connection.ConnectionPropertyNames; import org.jetbrains.annotations.NotNull; @@ -47,13 +48,17 @@ public class OctopusConnectionUiData { private static volatile OctopusConnectionsManager connectionsManager; private static volatile ProjectManager projectManager; + private static volatile PluginDescriptor pluginDescriptor; @SuppressWarnings( "StaticAssignmentInConstructor") // JSPs have no Spring context; capture statically. public OctopusConnectionUiData( - final OctopusConnectionsManager connectionsManager, final ProjectManager projectManager) { + final OctopusConnectionsManager connectionsManager, + final ProjectManager projectManager, + final PluginDescriptor pluginDescriptor) { OctopusConnectionUiData.connectionsManager = connectionsManager; OctopusConnectionUiData.projectManager = projectManager; + OctopusConnectionUiData.pluginDescriptor = pluginDescriptor; } /** @@ -187,6 +192,21 @@ public static String spacePickerProjectId(final HttpServletRequest request) { return fromBuildType == null ? "" : fromBuildType; } + /** + * Context-relative URL of the space picker's script. The plugin version is appended so a browser + * does not keep running a cached copy after the plugin is upgraded. + */ + @NotNull + public static String spacePickerScriptUrl(final HttpServletRequest request) { + if (pluginDescriptor == null) { + return ""; + } + final String version = pluginDescriptor.getPluginVersion(); + return request.getContextPath() + + pluginDescriptor.getPluginResourcesPath("js/spacePicker.js") + + (version == null ? "" : "?v=" + version); + } + /** * The Octopus connection this page is editing, or {@code ""} when there is not one yet. * diff --git a/octopus-server/src/main/resources/buildServerResources/js/spacePicker.js b/octopus-server/src/main/resources/buildServerResources/js/spacePicker.js new file mode 100644 index 00000000..63e7bf29 --- /dev/null +++ b/octopus-server/src/main/resources/buildServerResources/js/spacePicker.js @@ -0,0 +1,412 @@ +/* + * 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. + */ + +/* + * Behaviour for the space picker that spacePicker.jsp renders. The JSP loads this file once per + * page and calls OctopusSpacePicker.setupAll(), which is safe to call again for markup added later. + */ +window.OctopusSpacePicker = window.OctopusSpacePicker || (function () { + + function escapeHtml(text) { + const holder = document.createElement("span"); + holder.textContent = text; + return holder.innerHTML; + } + + // TeamCity resolves a parameter reference when the build runs, so it cannot be checked here. + function isParameterReference(text) { + return text.indexOf("%") !== -1; + } + + function describe(name, id) { + if (name && id) return "'" + name + "' (" + id + ")"; + return "'" + (name || id) + "'"; + } + + function csrfToken() { + const tokenMeta = document.querySelector("meta[name='tc-csrf-token']"); + return tokenMeta ? tokenMeta.getAttribute("content") || "" : ""; + } + + /** The connection's spaces, or no spaces while they are not known. */ + function spaceList(spaces) { + return { + known: !!spaces, + + withId: function (id) { + for (let i = 0; spaces && i < spaces.length; i++) { + if (spaces[i].id === id) return spaces[i]; + } + return null; + }, + + named: function (name) { + const wanted = name.trim().toLowerCase(); + for (let i = 0; spaces && wanted && i < spaces.length; i++) { + if (spaces[i].name.toLowerCase() === wanted) return spaces[i]; + } + return null; + }, + + suggestionsFor: function (term) { + if (!spaces || isParameterReference(term)) return []; + const wanted = term.trim().toLowerCase(); + const matches = []; + for (let i = 0; i < spaces.length; i++) { + const space = spaces[i]; + if (space.name.toLowerCase().indexOf(wanted) === -1) continue; + matches.push({ + // TeamCity's autocomplete inserts the label as HTML, and a space name is admin-supplied. + label: escapeHtml(space.name + " (" + space.id + ")"), + value: space.name, + id: space.id, + selectable: true + }); + } + return matches; + } + }; + } + + /** The status line, its details tooltip, the missing-space warning and the by-id note. */ + function pickerView(cell) { + const status = cell.querySelector(".octopusSpaceStatus"); + const statusText = cell.querySelector(".octopusSpaceStatusText"); + const details = cell.querySelector(".octopusSpaceDetails"); + const missing = cell.querySelector(".octopusSpaceMissing"); + const byIdNote = cell.querySelector(".octopusSpaceByIdNote"); + + function showDetails(reason) { + details.setAttribute("data-reason", reason); + details.setAttribute("aria-label", reason); + details.style.display = reason ? "" : "none"; + } + + function showDetailsTooltip() { + const reason = details.getAttribute("data-reason"); + if (!reason || !(window.BS && BS.Tooltip)) return; + BS.Tooltip.hidePopup(); + // The tooltip takes HTML, and the reason can quote admin-entered values. + BS.Tooltip.showMessage(details, { shift: { x: 10, y: 20 }, delay: 0 }, escapeHtml(reason)); + } + + function hideDetailsTooltip() { + if (window.BS && BS.Tooltip) BS.Tooltip.hidePopup(); + } + + details.addEventListener("mouseover", showDetailsTooltip); + details.addEventListener("click", showDetailsTooltip); + details.addEventListener("focus", showDetailsTooltip); + details.addEventListener("mouseout", hideDetailsTooltip); + details.addEventListener("blur", hideDetailsTooltip); + + const view = { + setStatus: function (message, isError) { + statusText.textContent = message || ""; + status.className = isError ? "smallNote error octopusSpaceStatus" : "smallNote octopusSpaceStatus"; + showDetails(""); + }, + + showMissing: function (message) { + missing.textContent = message || ""; + missing.style.display = message ? "" : "none"; + }, + + showByIdNote: function (shown) { + byIdNote.style.display = shown ? "" : "none"; + }, + + loading: function () { + view.showByIdNote(false); + view.showMissing(""); + view.setStatus("Loading spaces…", false); + }, + + listed: function () { + view.setStatus("", false); + view.showByIdNote(true); + }, + + cannotList: function (reason, isError) { + view.showByIdNote(false); + view.showMissing(""); + if (isError === false) { + view.setStatus(reason, false); + return; + } + view.setStatus("Unable to retrieve spaces, please enter space name manually", true); + showDetails(reason); + } + }; + return view; + } + + // The lookup names a saved connection and nothing else - it never sends a URL or key. The + // server reads those from the stored connection, so it is never asked to fetch an address + // supplied by this page. + function requestSpaces(meta, connectionId, onSpaces, onFailure) { + $j.ajax({ + url: meta.getAttribute("data-lookup-url"), + type: "POST", + data: { + projectId: meta.getAttribute("data-project-id"), + connectionId: connectionId + }, + dataType: "json", + // TeamCity's own scripts add this header to jQuery requests only once the page has + // finished initialising, and this lookup runs before that. + headers: { "X-TC-CSRF-Token": csrfToken() } + }).done(function (data) { + if (data && data.error) { + onFailure(data.error); + return; + } + const listed = (data && data.spaces) || []; + if (!listed.length) { + onFailure("Octopus returned no spaces for this connection."); + return; + } + onSpaces(listed); + }).fail(function (xhr) { + let message = "Could not list spaces."; + try { + const parsed = JSON.parse(xhr.responseText); + if (parsed && parsed.error) message = parsed.error; + } catch (ignored) { /* fall back to the generic message */ } + onFailure(message); + }); + } + + /** + * One autocomplete on the name field offering both TeamCity's parameters and the connection's + * spaces. A step form's text fields have TeamCity's own autocomplete, which completes parameter + * references after a %, and a field can only have one. So this one offers TeamCity's parameters + * while a reference is being typed, and spaces otherwise. + */ + function attachAutocomplete(nameField, callbacks) { + if (!$j.fn.autocomplete) return; + const field = $j(nameField); + // TeamCity's parameter completion, if this page has the parts of it used here. They are + // TeamCity internals, so a field without them - or where they fail - offers spaces only. + const parameters = window.BS && BS.AvailableParams; + const hasParameterCompletion = !!parameters + && ["buildSourceUrl", "sliceForCompletion", "sourceForElement", "moreForElement", + "selectForElement"].every(function (name) { + return typeof parameters[name] === "function"; + }); + + // TeamCity sets its autocomplete up on first focus unless the field is marked as done. + nameField._autocompleteInited = true; + + function parameterSourceUrl() { + return parameters.buildSourceUrl(field.data("editableObjId")); + } + + function completingParameter() { + if (!hasParameterCompletion) return false; + try { + return !!parameterSourceUrl() + && (parameters.sliceForCompletion(field) !== null || !!field.data("ctrlSpace")); + } catch (ignored) { + return false; + } + } + + // Entering the field offers every space, whatever it already holds. Registered before the + // autocomplete, whose own focus handler runs the search. + let offerEverySpace = false; + nameField.addEventListener("focus", function () { + offerEverySpace = true; + }); + + field.autocomplete({ + minLength: 0, + delay: 0, + showOnFocus: true, + showEmpty: false, + insertItemWithFocus: false, + completeOnCtrlSpace: true, + source: function (request, respond) { + const everySpace = offerEverySpace; + offerEverySpace = false; + if (completingParameter()) { + try { + parameters.sourceForElement(field, parameterSourceUrl(), true)(request, respond); + return; + } catch (ignored) { /* offer spaces instead */ } + } + field.data("sliced_text", null); + respond(callbacks.suggestionsFor(everySpace ? "" : request.term)); + }, + more: function (from, respond) { + try { + if (field.data("sliced_text")) { + parameters.moreForElement(field, parameterSourceUrl(), true)(from, respond); + return; + } + } catch (ignored) { /* nothing more to offer */ } + respond(null); + }, + select: function (event, ui) { + if (field.data("sliced_text")) { + try { + parameters.selectForElement(field).call(this, event, ui); + } catch (ignored) { /* the typed text is left as it is */ } + callbacks.parameterChosen(); + return false; + } + callbacks.spaceChosen(ui.item.value, ui.item.id); + return false; + } + }); + } + + function setup(cell) { + const meta = cell.querySelector(".octopusSpacePickerMeta"); + const nameInput = cell.querySelector(".octopusSpaceNameInput"); + if (!meta || !nameInput || !cell.querySelector(".octopusSpaceStatusText") + || !cell.querySelector(".octopusSpaceDetails") || cell.dataset.octopusBound === "true") { + return; + } + cell.dataset.octopusBound = "true"; + + // The props taglib renders these fields with name="prop:" and id="". + // Look the field up both ways so this does not hinge on that prefix. + const idFieldName = meta.getAttribute("data-space-id-field"); + const idField = + cell.querySelector("input[name='prop:" + idFieldName + "']") + || cell.querySelector("input[name='" + idFieldName + "']") + || document.getElementById(idFieldName); + const nameField = nameInput.querySelector("input[type='text']"); + if (!nameField) return; + + // A step form has a connection selector; the connection form does not. + const connectionSelect = document.getElementById("octopusConnectionId"); + const view = pickerView(cell); + let spaces = spaceList(null); + let lookupSequence = 0; + + // Typing a space's exact name picks that space; anything else abandons the stored id, which + // would otherwise silently win over the name. + function storeIdForTypedName() { + const space = spaces.named(nameField.value); + if (idField) idField.value = space ? space.id : ""; + } + + function checkTypedName() { + const name = nameField.value.trim(); + if (spaces.known && name && !isParameterReference(name) && !spaces.named(name)) { + view.showMissing("No space named '" + name + "' was found in Octopus."); + } else { + view.showMissing(""); + } + } + + function checkSavedSpace() { + const storedId = idField ? idField.value : ""; + const storedName = nameField.value.trim(); + if (storedId) { + const space = spaces.withId(storedId); + if (space) { + // Shows the space's current name, which differs from the saved one after a rename. + nameField.value = space.name; + view.showMissing(""); + } else { + view.showMissing("The saved space " + describe(storedName, storedId) + + " was not found. Please enter the space name."); + } + return; + } + // A configuration that only stored a name gains the id when it is next saved. + storeIdForTypedName(); + checkTypedName(); + } + + // On the connection form, the connection being edited is identified by the page itself once + // it has been saved. + function savedConnectionId() { + if (connectionSelect) return connectionSelect.value || ""; + return meta.getAttribute("data-connection-id") || ""; + } + + function cannotList(reason, isError) { + spaces = spaceList(null); + view.cannotList(reason, isError); + } + + function load() { + const sequence = ++lookupSequence; + const connectionId = savedConnectionId(); + if (!connectionId) { + cannotList(connectionSelect + ? "Select a connection to choose from its spaces." + : "Save this connection to choose from its spaces.", false); + return; + } + spaces = spaceList(null); + view.loading(); + requestSpaces( + meta, + connectionId, + function (listed) { + // A slower answer for an earlier connection must not replace the current one. + if (sequence !== lookupSequence) return; + spaces = spaceList(listed); + view.listed(); + checkSavedSpace(); + }, + function (reason) { + if (sequence !== lookupSequence) return; + cannotList(reason); + }); + } + + attachAutocomplete(nameField, { + suggestionsFor: function (term) { + return spaces.suggestionsFor(term); + }, + parameterChosen: function () { + storeIdForTypedName(); + view.showMissing(""); + }, + spaceChosen: function (name, id) { + nameField.value = name; + if (idField) idField.value = id; + view.showMissing(""); + } + }); + + nameField.addEventListener("input", function () { + storeIdForTypedName(); + view.showMissing(""); + }); + nameField.addEventListener("change", function () { + storeIdForTypedName(); + checkTypedName(); + }); + + if (connectionSelect) connectionSelect.addEventListener("change", load); + load(); + } + + return { + setupAll: function () { + const cells = document.querySelectorAll("tr.octopusSpacePickerRow td"); + for (let index = 0; index < cells.length; index++) { + setup(cells[index]); + } + } + }; +})(); diff --git a/octopus-server/src/main/resources/buildServerResources/spacePicker.jsp b/octopus-server/src/main/resources/buildServerResources/spacePicker.jsp index d08c6c19..84ea3649 100644 --- a/octopus-server/src/main/resources/buildServerResources/spacePicker.jsp +++ b/octopus-server/src/main/resources/buildServerResources/spacePicker.jsp @@ -39,6 +39,8 @@ "spacePickerProjectId", OctopusConnectionUiData.spacePickerProjectId(request)); pageContext.setAttribute( "spacePickerConnectionId", OctopusConnectionUiData.spacePickerConnectionId(request)); + pageContext.setAttribute( + "spacePickerScriptUrl", OctopusConnectionUiData.spacePickerScriptUrl(request)); %> @@ -61,7 +63,8 @@ data-project-id="" data-lookup-url="/octopus/listSpaces.html" data-space-id-field="" - data-connection-id=""> + data-connection-id="" + data-script-url="">

The space picker needs a saved connection to look spaces up through. On a step form - * the connection selector supplies that; on the connection form itself it has to come from the - * request. TeamCity's own parameter name for it is not contractual, so each plausible name is - * tried and only accepted when it actually resolves to an Octopus connection in this project. An - * unknown name simply yields {@code ""}, and the picker then asks the admin to save first rather - * than failing. - */ - @NotNull - public static String spacePickerConnectionId(final HttpServletRequest request) { - if (connectionsManager == null || projectManager == null) { - return ""; - } - final SProject project = projectManager.findProjectByExternalId(spacePickerProjectId(request)); - if (project == null) { - return ""; - } - for (final String parameterName : - new String[] {"connectionId", "oauthConnectionId", "featureId", "id"}) { - final String candidate = request.getParameter(parameterName); - if (candidate == null || candidate.trim().isEmpty()) { - continue; - } - final Optional resolved = - connectionsManager.resolve(project, candidate.trim()); - if (resolved.isPresent() - && OctopusConnection.TYPE.equals(resolved.get().getParameters().get("providerType"))) { - return candidate.trim(); - } - } - return ""; - } - private static String currentProjectExternalId(final HttpServletRequest request) { final SBuildType buildType = currentBuildType(request); return buildType == null ? null : buildType.getProject().getExternalId(); diff --git a/octopus-server/src/main/resources/buildServerResources/editOctopusConnection.jsp b/octopus-server/src/main/resources/buildServerResources/editOctopusConnection.jsp index 27da8efe..4ad0ca06 100644 --- a/octopus-server/src/main/resources/buildServerResources/editOctopusConnection.jsp +++ b/octopus-server/src/main/resources/buildServerResources/editOctopusConnection.jsp @@ -93,6 +93,7 @@ + diff --git a/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionUiDataTest.java b/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionUiDataTest.java index 40914da1..42e5d63c 100644 --- a/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionUiDataTest.java +++ b/octopus-server/src/test/java/octopus/teamcity/server/connection/OctopusConnectionUiDataTest.java @@ -10,7 +10,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Optional; import javax.servlet.http.HttpServletRequest; import jetbrains.buildServer.serverSide.ProjectManager; @@ -259,19 +258,6 @@ private static HttpServletRequest requestWith(final Map paramete return request; } - private SProject projectNamed(final String externalId) { - final SProject project = mock(SProject.class); - when(projectManager.findProjectByExternalId(externalId)).thenReturn(project); - return project; - } - - private static OAuthConnectionDescriptor connectionOfType(final String providerType) { - final OAuthConnectionDescriptor descriptor = mock(OAuthConnectionDescriptor.class); - when(descriptor.getParameters()) - .thenReturn(Collections.singletonMap("providerType", providerType)); - return descriptor; - } - @Test void spacePickerScriptUrlIsContextRelativeAndVersioned() { final HttpServletRequest request = mock(HttpServletRequest.class); @@ -306,40 +292,4 @@ void spacePickerProjectIdIsTheStepFormsBuildConfigurationsProject() { assertThat(OctopusConnectionUiData.spacePickerProjectId(requestWith(parameters))) .isEqualTo("Rtest"); } - - @Test - void spacePickerConnectionIdIsTheOctopusConnectionBeingEdited() { - final SProject project = projectNamed("Rtest"); - final OAuthConnectionDescriptor octopus = connectionOfType(OctopusConnection.TYPE); - when(connectionsManager.resolve(project, "PROJECT_EXT_7")).thenReturn(Optional.of(octopus)); - final Map parameters = new HashMap<>(); - parameters.put("projectId", "Rtest"); - parameters.put("featureId", "PROJECT_EXT_7"); - - assertThat(OctopusConnectionUiData.spacePickerConnectionId(requestWith(parameters))) - .isEqualTo("PROJECT_EXT_7"); - } - - @Test - void spacePickerConnectionIdIgnoresAConnectionOfAnotherType() { - final SProject project = projectNamed("Rtest"); - final OAuthConnectionDescriptor github = connectionOfType("GitHubApp"); - when(connectionsManager.resolve(project, "PROJECT_EXT_8")).thenReturn(Optional.of(github)); - final Map parameters = new HashMap<>(); - parameters.put("projectId", "Rtest"); - parameters.put("connectionId", "PROJECT_EXT_8"); - - assertThat(OctopusConnectionUiData.spacePickerConnectionId(requestWith(parameters))).isEmpty(); - } - - @Test - void spacePickerConnectionIdIsEmptyForAnUnsavedConnection() { - final SProject project = projectNamed("Rtest"); - when(connectionsManager.resolve(project, "unsaved")).thenReturn(Optional.empty()); - final Map parameters = new HashMap<>(); - parameters.put("projectId", "Rtest"); - parameters.put("connectionId", "unsaved"); - - assertThat(OctopusConnectionUiData.spacePickerConnectionId(requestWith(parameters))).isEmpty(); - } }