Skip to content

feat: add test connection button to JWT build feature#24

Merged
matt-richardson merged 4 commits intomainfrom
mattr/test-connection
Apr 16, 2026
Merged

feat: add test connection button to JWT build feature#24
matt-richardson merged 4 commits intomainfrom
mattr/test-connection

Conversation

@matt-richardson
Copy link
Copy Markdown
Contributor

@matt-richardson matt-richardson commented Apr 14, 2026

Summary

  • Adds a Test button to the JWT build feature configuration page
  • Issues a dry-run JWT using the current feature settings and verifies the OIDC discovery/JWKS endpoints are reachable
  • Optionally exchanges the JWT for an access token at a given service URL (HTTPS required; sends Content-Type: application/x-www-form-urlencoded per RFC 8693)

Test plan

  • Configure a JWT build feature and click "Test" — should show step results inline
  • Try with an invalid cron/service URL — should return a clear error
  • Verify CSRF token is validated (reject requests without it)
  • Verify non-admin users receive 403

🤖 Generated with Claude Code

Adds a "Test" button to the build feature configuration page that:
- issues a dry-run JWT using the current feature settings
- verifies the OIDC discovery and JWKS endpoints are reachable
- optionally exchanges the JWT for an access token at a given service URL
  (requires HTTPS; Content-Type: application/json for Octopus compatibility)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@matt-richardson matt-richardson requested a review from a team as a code owner April 14, 2026 04:02
Copy link
Copy Markdown

@mattburnell mattburnell left a comment

Choose a reason for hiding this comment

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

Feature and tests look really good. Claude was bothered by what it sees as a moderate SSRS attack (by allowing non-https traffic for localhost URLs, anyone with MANAGE_SERVER_INSTALLATION can use the service to probe local HTTP endpoints). While the test logic looks thorough, I feel the readability could be substantially improved by encapsulating some of the setup/teardown logic into higher-order functions.

Comment thread oidc-plugin-server/src/main/java/com/octopus/teamcity/oidc/JwtTestController.java Outdated
Comment thread oidc-plugin-server/src/main/java/com/octopus/teamcity/oidc/JwtTestController.java Outdated
@matt-richardson
Copy link
Copy Markdown
Contributor Author

Claude was bothered by what it sees as a moderate SSRS attack (by allowing non-https traffic for localhost URLs, anyone with MANAGE_SERVER_INSTALLATION can use the service to probe local HTTP endpoints).

I removed the support for isLocalHost, and switched the tests to use mocking instead.

While the test logic looks thorough, I feel the readability could be substantially improved by encapsulating some of the setup/teardown logic into higher-order functions.

Done. Removed ~200 lines from the PR 😄

@matt-richardson matt-richardson enabled auto-merge (squash) April 16, 2026 05:05
@matt-richardson matt-richardson merged commit 7e9ec0d into main Apr 16, 2026
3 checks passed
@matt-richardson matt-richardson deleted the mattr/test-connection branch April 16, 2026 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants