Skip to content

feat: list spaces for a connection that authenticates with OIDC - #265

Merged
matt-richardson merged 53 commits into
mainfrom
mr/space-picker-oidc
Oct 16, 2026
Merged

matt-richardson merged 53 commits into
mainfrom
mr/space-picker-oidc

Conversation

@matt-richardson

@matt-richardson matt-richardson commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

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/v1 for 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

  • TeamCity: the connection-credentials API exists from TeamCity 2023.05, and this plugin still loads on older servers. OidcIdTokenRequester is 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-core is provided and non-transitive. It's pinned to 2025.11 because 2026.x is compiled for Java 21.
  • OIDC plugin: an OIDC plugin that predates DPT-3614 rejects the request. The picker then asks for the plugin to be updated, and typing the name still works.

Notes

  • Listing spaces through an OIDC connection needs RUN_BUILD on the project as well as EDIT_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.
  • Neither the ID token nor the access token is echoed back. The token-exchange POST doesn't follow redirects, so the ID token can't be replayed elsewhere.
  • The e2e stack now serves TeamCity over HTTPS through a Caddy proxy whose CA Octopus trusts, and sets TeamCity's root URL to it. The OIDC plugin only issues tokens for an HTTPS issuer, and Octopus fetches the issuer's signing keys from it. Every full-stack test runs on this stack, so I ran the whole e2e suite locally against the real containers: 17 classes, all green, with the OIDC picker case run rather than skipped.

🤖 Generated with Claude Code

matt-richardson and others added 5 commits October 14, 2026 10:09
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>
@github-actions github-actions Bot added the feat label Oct 13, 2026
matt-richardson and others added 24 commits October 14, 2026 11:38
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
matt-richardson and others added 13 commits October 15, 2026 15:28
…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>
Base automatically changed from mr/space-picker-parameter-key to main October 15, 2026 06:46
matt-richardson and others added 3 commits October 15, 2026 17:48
# 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>
Comment on lines -174 to -288
/**
* 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;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to SpacesLookupCredentials.

Comment on lines -318 to -335

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);
}
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to Credentials and CredentialsUnavailableException.

Comment on lines -195 to -227
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;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to SpacesLookupCredentialsTest.

Comment on lines -237 to -319
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");

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to SpacesLookupCredentialsTest, apart from explainsThatAnOidcConnectionCannotListSpaces, which the OIDC support in this PR replaces.

matt-richardson and others added 2 commits October 15, 2026 18:55
…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());

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@APErebus APErebus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice work.

@matt-richardson
matt-richardson merged commit 0632c0a into main Oct 16, 2026
7 checks passed
@matt-richardson
matt-richardson deleted the mr/space-picker-oidc branch October 16, 2026 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants