Repository navigation
feat: list spaces for a connection that authenticates with OIDC - #265
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…API key Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…chosen id Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ackage Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnectionUiData.java # octopus-server/src/main/resources/buildServerResources/spacePicker.jsp
…eter-key # Conflicts: # e2e/src/test/java/octopus/teamcity/e2e/test/OctopusSpacePickerUiTest.java # octopus-server/src/main/java/octopus/teamcity/server/connection/OctopusConnectionUiData.java # octopus-server/src/main/resources/buildServerResources/js/spacePicker.js # octopus-server/src/main/resources/buildServerResources/spacePicker.jsp
…there Resolving the parameter sends its value, which can be a password the user cannot read, to the connection's URL. Requiring RUN_BUILD keeps the picker within what a build step could already do, and each lookup is logged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesController.java
…project A build finds its connection from its own project, so a build configuration from any other project would pair a URL and key no build would use. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…roject Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nding whitespace Validation and the space picker both trim the reference, so builds now use the same value. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # e2e/src/test/java/octopus/teamcity/e2e/test/OctopusSpacePickerUiTest.java # octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesController.java # octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesControllerTest.java
…ed e2e helper The connection form requests a project-scoped token, so the message no longer refers to a build configuration. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ontroller The controller now handles only the request, permissions and JSON. SpacesLookupCredentials decides how to authenticate for each API key source, and OidcAccessTokens gets an access token for an OIDC connection. An OIDC lookup now also requires RUN_BUILD, as an API key parameter lookup does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| /** | ||
| * Credentials for the lookup, always read from a saved connection. | ||
| * | ||
| * <p>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. | ||
| * | ||
| * <p>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 that authenticates with OIDC. | ||
| */ | ||
| private Credentials resolveCredentials( | ||
| final HttpServletRequest request, final SProject project, final SUser user) | ||
| 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<OAuthConnectionDescriptor> connection = | ||
| connectionsManager.resolve(project, connectionId.trim()); | ||
| if (!connection.isPresent()) { | ||
| throw new CredentialsUnavailableException("That Octopus connection could not be resolved."); | ||
| } | ||
|
|
||
| final Map<String, String> 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_PARAMETER.equals(source)) { | ||
| final SBuildType buildType = projectBuildType(request, project); | ||
| if (!user.isPermissionGrantedForProject(project.getProjectId(), Permission.RUN_BUILD)) { | ||
| throw new CredentialsUnavailableException( | ||
| "Spaces can only be listed with an API key parameter by someone who can run builds" | ||
| + " here. Enter the space name instead."); | ||
| } | ||
| final String reference = apiKeyParameterReference(params); | ||
| final String apiKey = | ||
| resolveApiKeyParameter(reference, buildType == null ? project : buildType); | ||
| Loggers.SERVER.info( | ||
| "User " | ||
| + user.getUsername() | ||
| + " resolved the API key parameter " | ||
| + reference | ||
| + " of Octopus connection " | ||
| + connectionId.trim() | ||
| + " in " | ||
| + (buildType == null | ||
| ? "project " + project.getExternalId() | ||
| : "build configuration " + buildType.getExternalId()) | ||
| + " to list the spaces on " | ||
| + serverUrl); | ||
| return new Credentials(serverUrl, OctopusAuth.apiKey(apiKey)); | ||
| } | ||
| if (!ConnectionPropertyNames.API_KEY_SOURCE_KEY.equals(source)) { | ||
| throw new CredentialsUnavailableException( | ||
| "Spaces cannot be listed for a connection that authenticates with OIDC. Enter the space name instead." | ||
| + " name instead."); | ||
| } | ||
| return new Credentials( | ||
| serverUrl, OctopusAuth.apiKey(params.get(ConnectionPropertyNames.API_KEY))); | ||
| } | ||
|
|
||
| /** | ||
| * The step form's build configuration, or {@code null} on a form that has none. A missing one and | ||
| * one in another project get the same message, so the request cannot be used to find out which | ||
| * build configurations exist. | ||
| */ | ||
| @Nullable | ||
| private SBuildType projectBuildType(final HttpServletRequest request, final SProject project) | ||
| throws CredentialsUnavailableException { | ||
| final String buildTypeId = request.getParameter("buildTypeId"); | ||
| if (buildTypeId == null || buildTypeId.trim().isEmpty()) { | ||
| return null; | ||
| } | ||
| final SBuildType buildType = projectManager.findBuildTypeByExternalId(buildTypeId.trim()); | ||
| if (buildType == null || !project.getProjectId().equals(buildType.getProjectId())) { | ||
| throw new CredentialsUnavailableException( | ||
| "Build configuration '" + buildTypeId.trim() + "' was not found in this project."); | ||
| } | ||
| return buildType; | ||
| } | ||
|
|
||
| /** The connection's {@code %parameter%} reference, trimmed as a build injects it. */ | ||
| private static String apiKeyParameterReference(final Map<String, String> params) | ||
| throws CredentialsUnavailableException { | ||
| final String reference = params.get(ConnectionPropertyNames.API_KEY_PARAMETER); | ||
| if (reference == null || reference.trim().isEmpty()) { | ||
| throw new CredentialsUnavailableException( | ||
| "This connection does not use an API key parameter."); | ||
| } | ||
| return reference.trim(); | ||
| } | ||
|
|
||
| /** Resolves {@code reference} as a build of {@code scope} would. */ | ||
| private static String resolveApiKeyParameter( | ||
| final String reference, final ParametersSupport scope) | ||
| throws CredentialsUnavailableException { | ||
| final ProcessingResult resolved = scope.getValueResolver().resolve(reference); | ||
| final String apiKey = resolved.getResult(); | ||
| if (!resolved.isFullyResolved() || apiKey == null || apiKey.trim().isEmpty()) { | ||
| throw new CredentialsUnavailableException( | ||
| "The API key parameter " | ||
| + reference | ||
| + " can't be resolved here - it may only be set when a build runs. Enter the space" | ||
| + " name instead."); | ||
| } | ||
| return apiKey; | ||
| } | ||
|
|
There was a problem hiding this comment.
Moved to SpacesLookupCredentials.
|
|
||
| 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); | ||
| } | ||
| } |
There was a problem hiding this comment.
Moved to Credentials and CredentialsUnavailableException.
| private Map<String, String> parameterSourcedConnection() { | ||
| final Map<String, String> params = new HashMap<>(); | ||
| params.put(ConnectionPropertyNames.SERVER_URL, "https://stored.example.com"); | ||
| params.put( | ||
| ConnectionPropertyNames.API_KEY_SOURCE, ConnectionPropertyNames.API_KEY_SOURCE_PARAMETER); | ||
| params.put(ConnectionPropertyNames.API_KEY_PARAMETER, "%octopus.apikey%"); | ||
| 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(user.isPermissionGrantedForProject(PROJECT_INTERNAL_ID, Permission.RUN_BUILD)) | ||
| .thenReturn(true); | ||
| return params; | ||
| } | ||
|
|
||
| private static ValueResolver resolverReturning(final String value, final boolean resolved) { | ||
| final ProcessingResult result = mock(ProcessingResult.class); | ||
| when(result.getResult()).thenReturn(value); | ||
| when(result.isFullyResolved()).thenReturn(resolved); | ||
| final ValueResolver resolver = mock(ValueResolver.class); | ||
| when(resolver.resolve("%octopus.apikey%")).thenReturn(result); | ||
| return resolver; | ||
| } | ||
|
|
||
| private SBuildType buildTypeInProject(final String projectInternalId) { | ||
| final SBuildType buildType = mock(SBuildType.class); | ||
| when(buildType.getProjectId()).thenReturn(projectInternalId); | ||
| when(buildType.getExternalId()).thenReturn("MyProject_Deploy"); | ||
| when(projectManager.findBuildTypeByExternalId("MyProject_Deploy")).thenReturn(buildType); | ||
| when(request.getParameter("buildTypeId")).thenReturn("MyProject_Deploy"); | ||
| return buildType; | ||
| } | ||
|
|
There was a problem hiding this comment.
Moved to SpacesLookupCredentialsTest.
| assertThat(body).contains("Spaces-123"); | ||
| assertThat(fetchedKeys).containsExactly("API-FROM-PARAMETER"); | ||
| assertThat(body).doesNotContain("API-FROM-PARAMETER"); | ||
| } | ||
|
|
||
| @Test | ||
| void resolvesAParameterSourcedKeyAgainstTheProjectWithoutABuildConfiguration() throws Exception { | ||
| parameterSourcedConnection(); | ||
| final ValueResolver resolver = resolverReturning("API-FROM-PROJECT", true); | ||
| when(project.getValueResolver()).thenReturn(resolver); | ||
|
|
||
| handle(); | ||
|
|
||
| assertThat(fetchedKeys).containsExactly("API-FROM-PROJECT"); | ||
| } | ||
|
|
||
| @Test | ||
| void explainsAParameterThatOnlyResolvesDuringABuild() throws Exception { | ||
| parameterSourcedConnection(); | ||
| final ValueResolver resolver = resolverReturning("%octopus.apikey%", false); | ||
| when(project.getValueResolver()).thenReturn(resolver); | ||
|
|
||
| final String body = handle(); | ||
|
|
||
| assertThat(body).contains("%octopus.apikey%").contains("space name instead"); | ||
| assertThat(fetchedUrls).isEmpty(); | ||
| } | ||
|
|
||
| @Test | ||
| void refusesABuildConfigurationFromAnotherProject() throws Exception { | ||
| parameterSourcedConnection(); | ||
| final SBuildType buildType = buildTypeInProject("project999"); | ||
| when(user.isPermissionGrantedForProject("project999", Permission.EDIT_PROJECT)) | ||
| .thenReturn(true); | ||
| when(user.isPermissionGrantedForProject("project999", Permission.RUN_BUILD)).thenReturn(true); | ||
| final ValueResolver resolver = resolverReturning("API-FROM-PARAMETER", true); | ||
| when(buildType.getValueResolver()).thenReturn(resolver); | ||
|
|
||
| assertThat(handle()).contains("MyProject_Deploy").contains("was not found in this project"); | ||
| assertThat(fetchedUrls).isEmpty(); | ||
| } | ||
|
|
||
| @Test | ||
| void refusesAParameterSourcedKeyToAUserWhoCannotRunBuilds() throws Exception { | ||
| parameterSourcedConnection(); | ||
| when(user.isPermissionGrantedForProject(PROJECT_INTERNAL_ID, Permission.RUN_BUILD)) | ||
| .thenReturn(false); | ||
| final ValueResolver resolver = resolverReturning("API-FROM-PROJECT", true); | ||
| when(project.getValueResolver()).thenReturn(resolver); | ||
|
|
||
| assertThat(handle()).contains("can run builds").contains("space name instead"); | ||
| assertThat(fetchedUrls).isEmpty(); | ||
| } | ||
|
|
||
| @Test | ||
| void refusesAnUnknownBuildConfiguration() throws Exception { | ||
| parameterSourcedConnection(); | ||
| when(request.getParameter("buildTypeId")).thenReturn("Nope"); | ||
|
|
||
| assertThat(handle()).contains("Nope").contains("was not found"); | ||
| assertThat(fetchedUrls).isEmpty(); | ||
| } | ||
|
|
||
| @Test | ||
| void explainsThatAnOidcConnectionCannotListSpaces() throws Exception { | ||
| final Map<String, String> 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("authenticates with OIDC").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"); |
There was a problem hiding this comment.
Moved to SpacesLookupCredentialsTest, apart from explainsThatAnOidcConnectionCannotListSpaces, which the OIDC support in this PR replaces.
…tials API Checking for the API from OidcIdTokenRequester loaded the API's classes, and the check failed with NoClassDefFoundError on TeamCity before 2023.05. The check now lives in a class that does not reference the API, and a LinkageError turns OIDC lookups off instead of failing the controller. A 404 from Octopus's token-exchange endpoint, an unknown host and an empty failure message each get their own explanation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ence for builds The spaces lookup accepted any connection id in the project, including connections of other types. Builds now trim the OIDC connector's audience, as the spaces lookup does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # e2e/src/test/java/octopus/teamcity/e2e/dsl/OctopusTeamCityStack.java # octopus-server/src/main/java/octopus/teamcity/server/client/OctopusTokenExchange.java # octopus-server/src/main/java/octopus/teamcity/server/spaces/OctopusSpacesController.java # octopus-server/src/main/java/octopus/teamcity/server/spaces/SpacesLookupCredentials.java # octopus-server/src/test/java/octopus/teamcity/server/spaces/OctopusSpacesControllerTest.java # octopus-server/src/test/java/octopus/teamcity/server/spaces/SpacesLookupCredentialsTest.java
| runner.addRunnerParameter( | ||
| CONSTANTS.getApiKeySourceKey(), ConnectionPropertyNames.API_KEY_SOURCE_OIDC); | ||
| runner.addRunnerParameter(CONSTANTS.getOidcServiceAccountIdKey(), audience); | ||
| runner.addRunnerParameter(CONSTANTS.getOidcServiceAccountIdKey(), audience.trim()); |
There was a problem hiding this comment.
Claude here: this trim is outside the spaces lookup. It changes the build-time OIDC path, so the service account id injected into every OIDC build has surrounding whitespace removed. The spaces lookup already trims the audience; this keeps builds consistent with it. Covered by oidcSourceInjectsTheAudienceWithoutSurroundingWhitespace.
Part of DPT-3630 (Renaming an Octopus space breaks TeamCity configurations that name it), and depends on DPT-3614 (Other TeamCity plugins can't use an OIDC connection outside a running build). Builds on #267 (refactor: move the spaces lookup's credential resolution out of the controller), #268 (feat: exchange an OIDC ID token for an Octopus access token) and #269 (test: serve TeamCity over HTTPS in the full e2e stack).
Why
An OIDC connection has no API key at all. Its token is minted per build by the OIDC Identity Token plugin, so until now the picker could only ask the admin to type the space name. This lists spaces for those connections too.
How
The server asks the OIDC plugin for an ID token outside a build, using TeamCity's connection-credentials API (
ProjectConnectionCredentialsManager). It then exchanges that token at Octopus's/token/v1for an access token, and lists spaces with the access token. On a step form the ID token is issued with the subject a build of that build configuration would get, so Octopus trusts it exactly when it would trust a build. On the connection form it is project-scoped.Compatibility
OidcIdTokenRequesteris the only class that references the API, and it is constructed only after a class check confirms the API is present. Otherwise the picker says the server can't issue tokens outside a build.connections-api-coreisprovidedand non-transitive. It's pinned to 2025.11 because 2026.x is compiled for Java 21.Notes
RUN_BUILDon the project as well asEDIT_PROJECT, the same as an API key parameter. The server gets a token and sends it to the connection's URL, and someone who can edit and run a build could already do that from a build step.🤖 Generated with Claude Code