Skip to content

Commit 9ceca8f

Browse files
committed
Default resource indicator based on server URL
1 parent 27bd9b9 commit 9ceca8f

File tree

9 files changed

+357
-14
lines changed

9 files changed

+357
-14
lines changed

cmd/thv/app/run.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/stacklok/toolhive/pkg/networking"
2323
"github.com/stacklok/toolhive/pkg/process"
2424
"github.com/stacklok/toolhive/pkg/runner"
25+
"github.com/stacklok/toolhive/pkg/validation"
2526
"github.com/stacklok/toolhive/pkg/workloads"
2627
)
2728

@@ -461,6 +462,16 @@ func validateRunFlags(cmd *cobra.Command, args []string) error {
461462
return err
462463
}
463464

465+
// Validate --remote-auth-resource flag (RFC 8707)
466+
if resourceFlag := cmd.Flags().Lookup("remote-auth-resource"); resourceFlag != nil && resourceFlag.Changed {
467+
resource := resourceFlag.Value.String()
468+
if resource != "" {
469+
if err := validation.ValidateResourceURI(resource); err != nil {
470+
return fmt.Errorf("invalid --remote-auth-resource: %w", err)
471+
}
472+
}
473+
}
474+
464475
// Validate --from-config flag usage
465476
fromConfigFlag := cmd.Flags().Lookup("from-config")
466477
if fromConfigFlag != nil && fromConfigFlag.Value.String() != "" {

cmd/thv/app/run_flags.go

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -681,7 +681,13 @@ func getRemoteAuthFromRemoteServerMetadata(
681681
authCfg.Issuer = firstNonEmpty(f.RemoteAuthIssuer, oc.Issuer)
682682
authCfg.AuthorizeURL = firstNonEmpty(f.RemoteAuthAuthorizeURL, oc.AuthorizeURL)
683683
authCfg.TokenURL = firstNonEmpty(f.RemoteAuthTokenURL, oc.TokenURL)
684-
authCfg.Resource = firstNonEmpty(f.RemoteAuthResource, oc.Resource)
684+
685+
resourceIndicator := firstNonEmpty(f.RemoteAuthResource, oc.Resource)
686+
if resourceIndicator != "" {
687+
authCfg.Resource = resourceIndicator
688+
} else {
689+
authCfg.Resource = remote.DefaultResourceIndicator(remoteServerMetadata.URL)
690+
}
685691

686692
// OAuthParams: REPLACE metadata when CLI provides any key/value.
687693
if len(runFlags.OAuthParams) > 0 {
@@ -716,6 +722,12 @@ func getRemoteAuthFromRunFlags(runFlags *RunFlags) (*remote.Config, error) {
716722
}
717723
}
718724

725+
// Derive the resource parameter (RFC 8707)
726+
resource := runFlags.RemoteAuthFlags.RemoteAuthResource
727+
if resource == "" && runFlags.ResourceURL != "" {
728+
resource = remote.DefaultResourceIndicator(runFlags.RemoteURL)
729+
}
730+
719731
return &remote.Config{
720732
ClientID: runFlags.RemoteAuthFlags.RemoteAuthClientID,
721733
ClientSecret: clientSecret,
@@ -726,7 +738,7 @@ func getRemoteAuthFromRunFlags(runFlags *RunFlags) (*remote.Config, error) {
726738
Issuer: runFlags.RemoteAuthFlags.RemoteAuthIssuer,
727739
AuthorizeURL: runFlags.RemoteAuthFlags.RemoteAuthAuthorizeURL,
728740
TokenURL: runFlags.RemoteAuthFlags.RemoteAuthTokenURL,
729-
Resource: runFlags.RemoteAuthFlags.RemoteAuthResource,
741+
Resource: resource,
730742
OAuthParams: runFlags.OAuthParams,
731743
}, nil
732744
}

docs/remote-mcp-authentication.md

Lines changed: 60 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,60 @@ When no client credentials are provided ([`pkg/auth/oauth/dynamic_registration.g
150150
4. **Store Credentials**: Use returned client_id (and client_secret if provided)
151151
5. **Proceed with OAuth Flow**: Using registered credentials
152152

153+
## Resource Parameter (RFC 8707) Implementation
154+
155+
ToolHive implements the OAuth 2.0 Resource Indicators (RFC 8707) as required by the MCP specification:
156+
157+
**Location**: [`pkg/auth/remote/handler.go:52-69`](../pkg/auth/remote/handler.go#L52)
158+
159+
### Automatic Defaulting
160+
When no explicit `--remote-auth-resource` flag is provided, ToolHive automatically:
161+
1. Defaults the resource parameter to the remote server URL (the canonical URI of the MCP server)
162+
2. Validates the URI format according to MCP specification requirements
163+
3. Normalizes the URI (lowercase scheme/host, strips fragments, preserves trailing slashes)
164+
4. If the resource parameter cannot be derived, then it will not be sent
165+
166+
### Validation Rules
167+
The resource parameter must conform to MCP canonical URI requirements:
168+
- **Must** include a scheme (http/https)
169+
- **Must** include a host
170+
- **Must not** contain fragments (#)
171+
172+
When the resource parameter is **defaulted** from the remote URL:
173+
- Scheme and host are normalized to lowercase
174+
- Fragments are stripped (not allowed in resource indicators per spec)
175+
- Trailing slashes are preserved (we cannot determine semantic significance)
176+
177+
When the resource parameter is **explicitly provided** by the user:
178+
- Value is validated but **not modified**
179+
- Returns an error if the value is invalid
180+
- User must provide a properly formatted canonical URI
181+
182+
### Examples
183+
```bash
184+
# Automatic resource parameter (defaults and normalizes to remote URL)
185+
thv run https://MCP.Example.COM/api#section
186+
# Resource defaults to: https://mcp.example.com/api (normalized, fragment stripped)
187+
188+
# Explicit resource parameter (not modified, must be valid)
189+
thv run https://mcp.example.com/api \
190+
--remote-auth-resource https://mcp.example.com
191+
192+
# Invalid explicit resource parameter with fragment (returns error)
193+
thv run https://mcp.example.com/api \
194+
--remote-auth-resource https://mcp.example.com#fragment
195+
# Error: invalid resource parameter: resource URI must not contain fragments
196+
197+
# Invalid explicit resource parameter without scheme (returns error)
198+
thv run https://mcp.example.com/api \
199+
--remote-auth-resource mcp.example.com
200+
# Error: invalid resource parameter: resource URI must include a scheme
201+
```
202+
203+
The validated and normalized resource parameter is sent in both:
204+
- Authorization requests (as `resource` query parameter)
205+
- Token exchange requests (as `resource` parameter)
206+
153207
## Security Features
154208

155209
### HTTPS Enforcement
@@ -277,17 +331,18 @@ The `oauth_config` section supports:
277331
| OAuth 2.1 PKCE | ✅ Compliant | Enabled by default |
278332
| WWW-Authenticate Parsing | ✅ Compliant | Supports Bearer with realm/resource_metadata |
279333
| Multiple Auth Servers | ✅ Compliant | Iterates and validates all servers |
280-
| Resource Parameter (RFC 8707) | ⚠️ Partial | Infrastructure ready, not yet sent in requests |
334+
| Resource Parameter (RFC 8707) | ✅ Compliant | Automatically defaults to remote server URL, validated and normalized |
281335
| Token Audience Validation | ⚠️ Partial | Server-side validation support ready |
282336

337+
338+
283339
## Future Enhancements
284340

285341
While ToolHive is highly compliant with the current MCP specification, potential improvements include:
286342

287-
1. **Resource Parameter**: Add explicit `resource` parameter to OAuth requests (infrastructure exists)
288-
2. **Token Audience Validation**: Enhanced client-side validation of token audience claims
289-
3. **Refresh Token Rotation**: Implement automatic refresh token rotation for long-lived sessions
290-
4. **Client Credential Caching**: Persist dynamically registered clients across sessions
343+
1. **Token Audience Validation**: Enhanced client-side validation of token audience claims
344+
2. **Refresh Token Rotation**: Implement automatic refresh token rotation for long-lived sessions
345+
3. **Client Credential Caching**: Persist dynamically registered clients across sessions
291346

292347
## Conclusion
293348

pkg/api/v1/workload_service.go

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,13 @@ func (s *WorkloadService) BuildFullRunConfig(ctx context.Context, req *createReq
102102
}
103103
}
104104

105+
// Validate user-provided resource indicator (RFC 8707)
106+
if req.OAuthConfig.Resource != "" {
107+
if err := validation.ValidateResourceURI(req.OAuthConfig.Resource); err != nil {
108+
return nil, fmt.Errorf("%w: invalid resource parameter: %v", retriever.ErrInvalidRunConfig, err)
109+
}
110+
}
111+
105112
// Default group if not specified
106113
groupName := req.Group
107114
if groupName == "" {
@@ -152,6 +159,15 @@ func (s *WorkloadService) BuildFullRunConfig(ctx context.Context, req *createReq
152159

153160
if remoteServerMetadata, ok := serverMetadata.(*registry.RemoteServerMetadata); ok {
154161
if remoteServerMetadata.OAuthConfig != nil {
162+
// Default resource: user-provided > registry metadata > derived from remote URL
163+
resource := req.OAuthConfig.Resource
164+
if resource == "" {
165+
resource = remoteServerMetadata.OAuthConfig.Resource
166+
}
167+
if resource == "" && remoteServerMetadata.URL != "" {
168+
resource = remote.DefaultResourceIndicator(remoteServerMetadata.URL)
169+
}
170+
155171
remoteAuthConfig = &remote.Config{
156172
ClientID: req.OAuthConfig.ClientID,
157173
Scopes: remoteServerMetadata.OAuthConfig.Scopes,
@@ -160,7 +176,7 @@ func (s *WorkloadService) BuildFullRunConfig(ctx context.Context, req *createReq
160176
AuthorizeURL: remoteServerMetadata.OAuthConfig.AuthorizeURL,
161177
TokenURL: remoteServerMetadata.OAuthConfig.TokenURL,
162178
UsePKCE: remoteServerMetadata.OAuthConfig.UsePKCE,
163-
Resource: remoteServerMetadata.OAuthConfig.Resource,
179+
Resource: resource,
164180
OAuthParams: remoteServerMetadata.OAuthConfig.OAuthParams,
165181
Headers: remoteServerMetadata.Headers,
166182
EnvVars: remoteServerMetadata.EnvVars,
@@ -255,6 +271,12 @@ func createRequestToRemoteAuthConfig(
255271
req *createRequest,
256272
) *remote.Config {
257273

274+
// Default resource: user-provided > derived from remote URL
275+
resource := req.OAuthConfig.Resource
276+
if resource == "" && req.URL != "" {
277+
resource = remote.DefaultResourceIndicator(req.URL)
278+
}
279+
258280
// Create RemoteAuthConfig
259281
remoteAuthConfig := &remote.Config{
260282
ClientID: req.OAuthConfig.ClientID,
@@ -263,6 +285,7 @@ func createRequestToRemoteAuthConfig(
263285
AuthorizeURL: req.OAuthConfig.AuthorizeURL,
264286
TokenURL: req.OAuthConfig.TokenURL,
265287
UsePKCE: req.OAuthConfig.UsePKCE,
288+
Resource: resource,
266289
OAuthParams: req.OAuthConfig.OAuthParams,
267290
CallbackPort: req.OAuthConfig.CallbackPort,
268291
SkipBrowser: req.OAuthConfig.SkipBrowser,

pkg/api/v1/workloads_types_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ func TestRunConfigToCreateRequest(t *testing.T) {
161161
ClientSecret: "oauth-client-secret,target=oauth_secret",
162162
Scopes: []string{"read", "write"},
163163
UsePKCE: true,
164+
Resource: "https://mcp.example.com",
164165
OAuthParams: map[string]string{"custom": "param"},
165166
CallbackPort: 8081,
166167
},
@@ -176,6 +177,7 @@ func TestRunConfigToCreateRequest(t *testing.T) {
176177
assert.Equal(t, "test-client", result.OAuthConfig.ClientID)
177178
assert.Equal(t, []string{"read", "write"}, result.OAuthConfig.Scopes)
178179
assert.True(t, result.OAuthConfig.UsePKCE)
180+
assert.Equal(t, "https://mcp.example.com", result.OAuthConfig.Resource)
179181
assert.Equal(t, map[string]string{"custom": "param"}, result.OAuthConfig.OAuthParams)
180182
assert.Equal(t, 8081, result.OAuthConfig.CallbackPort)
181183

pkg/auth/remote/config.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,13 @@ package remote
33
import (
44
"encoding/json"
55
"fmt"
6+
"net/url"
7+
"strings"
68
"time"
79

10+
"github.com/stacklok/toolhive/pkg/logger"
811
"github.com/stacklok/toolhive/pkg/registry"
12+
"github.com/stacklok/toolhive/pkg/validation"
913
)
1014

1115
// Config holds authentication configuration for remote MCP servers.
@@ -98,3 +102,50 @@ func (r *Config) UnmarshalJSON(data []byte) error {
98102

99103
// DefaultCallbackPort is the default port for the OAuth callback server
100104
const DefaultCallbackPort = 8666
105+
106+
// DefaultResourceIndicator derives the resource indicator (RFC 8707) from the remote server URL.
107+
// This function should only be called when the user has not explicitly provided a resource indicator.
108+
// If the resource indicator cannot be derived, it returns an empty string.
109+
func DefaultResourceIndicator(remoteServerURL string) string {
110+
// Normalize the remote server URL
111+
normalized, err := normalizeResourceURI(remoteServerURL)
112+
if err != nil {
113+
// Normalization failed - log warning and leave resource empty
114+
logger.Warnf("Failed to normalize resource indicator from remote server URL %s: %v", remoteServerURL, err)
115+
return ""
116+
}
117+
118+
// Validate the normalized result
119+
if err := validation.ValidateResourceURI(normalized); err != nil {
120+
// Validation failed - log warning and leave resource empty
121+
logger.Warnf("Normalized resource indicator is invalid %s: %v", normalized, err)
122+
return ""
123+
}
124+
125+
return normalized
126+
}
127+
128+
// normalizeResourceURI normalizes a resource URI to conform to MCP specification requirements.
129+
// This function performs the following normalizations:
130+
// - Lowercase scheme and host
131+
// - Strip fragments
132+
func normalizeResourceURI(resourceURI string) (string, error) {
133+
if resourceURI == "" {
134+
return "", fmt.Errorf("resource URI cannot be empty")
135+
}
136+
137+
// Parse the URI
138+
parsed, err := url.Parse(resourceURI)
139+
if err != nil {
140+
return "", fmt.Errorf("invalid resource URI: %w", err)
141+
}
142+
143+
// Normalize: lowercase scheme and host
144+
parsed.Scheme = strings.ToLower(parsed.Scheme)
145+
parsed.Host = strings.ToLower(parsed.Host)
146+
147+
// Strip fragment if present (fragments are not allowed in resource indicators)
148+
parsed.Fragment = ""
149+
150+
return parsed.String(), nil
151+
}

pkg/auth/remote/config_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package remote
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
8+
"github.com/stacklok/toolhive/pkg/logger"
9+
)
10+
11+
func TestDeriveResourceIndicator(t *testing.T) {
12+
t.Parallel()
13+
logger.Initialize()
14+
15+
tests := []struct {
16+
name string
17+
remoteServerURL string
18+
expectedResource string
19+
}{
20+
{
21+
name: "valid remote URL - derive and normalize",
22+
remoteServerURL: "https://MCP.Example.COM/api#fragment",
23+
expectedResource: "https://mcp.example.com/api",
24+
},
25+
{
26+
name: "remote URL with trailing slash - preserve it",
27+
remoteServerURL: "https://mcp.example.com/api/",
28+
expectedResource: "https://mcp.example.com/api/",
29+
},
30+
{
31+
name: "remote URL with port - preserve port",
32+
remoteServerURL: "https://mcp.example.com:8443/api",
33+
expectedResource: "https://mcp.example.com:8443/api",
34+
},
35+
{
36+
name: "empty remote URL - return empty",
37+
remoteServerURL: "",
38+
expectedResource: "",
39+
},
40+
{
41+
name: "invalid remote URL - return empty",
42+
remoteServerURL: "ht!tp://invalid",
43+
expectedResource: "",
44+
},
45+
{
46+
name: "derived resource with query params - preserve them",
47+
remoteServerURL: "https://mcp.example.com/api?token=abc123",
48+
expectedResource: "https://mcp.example.com/api?token=abc123",
49+
},
50+
}
51+
52+
for _, tt := range tests {
53+
t.Run(tt.name, func(t *testing.T) {
54+
t.Parallel()
55+
56+
got := DefaultResourceIndicator(tt.remoteServerURL)
57+
assert.Equal(t, tt.expectedResource, got)
58+
})
59+
}
60+
}

pkg/validation/validation.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package validation
33

44
import (
55
"fmt"
6+
"net/url"
67
"regexp"
78
"strings"
89

@@ -86,3 +87,40 @@ func ValidateHTTPHeaderValue(value string) error {
8687

8788
return nil
8889
}
90+
91+
// ValidateResourceURI validates that a resource URI conforms to MCP specification requirements
92+
// for canonical URIs (RFC 8707).
93+
// This is used for user-provided values that should not be normalized.
94+
//
95+
// According to MCP spec, a valid canonical URI must:
96+
// - Include a scheme (http/https)
97+
// - Include a host
98+
// - Not contain fragments
99+
func ValidateResourceURI(resourceURI string) error {
100+
if resourceURI == "" {
101+
return fmt.Errorf("resource URI cannot be empty")
102+
}
103+
104+
// Parse the URI
105+
parsed, err := url.Parse(resourceURI)
106+
if err != nil {
107+
return fmt.Errorf("invalid resource URI: %w", err)
108+
}
109+
110+
// Must have a scheme
111+
if parsed.Scheme == "" {
112+
return fmt.Errorf("resource URI must include a scheme (e.g., https://): %s", resourceURI)
113+
}
114+
115+
// Must have a host
116+
if parsed.Host == "" {
117+
return fmt.Errorf("resource URI must include a host: %s", resourceURI)
118+
}
119+
120+
// Must not contain fragments
121+
if parsed.Fragment != "" {
122+
return fmt.Errorf("resource URI must not contain fragments (#): %s", resourceURI)
123+
}
124+
125+
return nil
126+
}

0 commit comments

Comments
 (0)