-
Notifications
You must be signed in to change notification settings - Fork 126
Open
Labels
authenticationenhancementNew feature or requestNew feature or requestgoPull requests that update go codePull requests that update go codegood first issueGood for newcomersGood for newcomers
Description
Parent issue: #2041
Background
PR #2143 added OAuth 2.0 Token Exchange (RFC 8693) support to both thv proxy
and thv run
commands. While the core token exchange functionality has extensive test coverage (1,900+ lines of tests in pkg/auth/tokenexchange/
), the integration layer in the runner package lacks explicit test coverage.
Current State
Codecov reports low patch coverage for:
pkg/runner/config_builder.go
: Missing coverage for token exchange config integrationpkg/runner/middleware.go
: Missing coverage for middleware factory registration
Recommended Tests
Priority 1: Core Integration (High Value)
Add tests to pkg/runner/config_builder_test.go
:
-
Token exchange middleware NOT added when config is nil
- Verify middleware count and types when
tokenExchangeConfig
is nil
- Verify middleware count and types when
-
Token exchange middleware IS added when config is provided
- Verify middleware is added with valid config
- Check middleware type is correct
-
Middleware ordering is correct
- Ensure token exchange comes after auth, before MCP parser
- Validate middleware chain integrity
Priority 2: Integration Scenarios (Medium Value)
- Token exchange works alongside other middlewares
- Test with OIDC, telemetry, authz enabled simultaneously
- Verify no conflicts or ordering issues
Priority 3: Edge Cases (Low Value)
- Nil safety
- Passing nil tokenExchangeConfig doesn't panic
- Error handling for malformed configs
Implementation Guidance
- Follow existing test patterns in
pkg/runner/config_builder_test.go
- Use table-driven tests with testify/assert
- Estimated effort: ~30 minutes for Priority 1 tests
Related
- Parent: Implement token-exchange and downstream authenticator support in ToolHive proxy #2041
- PR: Integrate token exchange middleware #2143
- Review comment from Claude's automated review
Metadata
Metadata
Assignees
Labels
authenticationenhancementNew feature or requestNew feature or requestgoPull requests that update go codePull requests that update go codegood first issueGood for newcomersGood for newcomers