Skip to content

Commit 1d62b05

Browse files
cmsparkscmsparks
authored
fix mcp server hosts (#517)
1 parent feadae0 commit 1d62b05

14 files changed

Lines changed: 87 additions & 30 deletions

‎docs/api/authentication.mdx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ A denied or expired flow returns `400` with `"access_denied"` or `"expired_token
115115

116116
## Connecting MCP clients
117117

118-
The Omnara API is also exposed as an MCP server at `https://app.omnara.com/api/mcp`. MCP clients that support the [MCP authorization spec](https://modelcontextprotocol.io/specification/draft/basic/authorization) discover the OAuth endpoints themselves and sign you in through the browser, so no token needs to be pasted into the client.
118+
The Omnara API is also exposed as an MCP server at `https://app.omnara.com/mcp`. MCP clients that support the [MCP authorization spec](https://modelcontextprotocol.io/specification/draft/basic/authorization) discover the OAuth endpoints themselves and sign you in through the browser, so no token needs to be pasted into the client.
119119

120120
Add the server to your client's `mcp.json`:
121121

@@ -124,7 +124,7 @@ Add the server to your client's `mcp.json`:
124124
"mcpServers": {
125125
"omnara": {
126126
"type": "http",
127-
"url": "https://app.omnara.com/api/mcp"
127+
"url": "https://app.omnara.com/mcp"
128128
}
129129
}
130130
}
@@ -139,7 +139,7 @@ Clients without OAuth support can pass a personal access token or org API key in
139139
"mcpServers": {
140140
"omnara": {
141141
"type": "http",
142-
"url": "https://app.omnara.com/api/mcp",
142+
"url": "https://app.omnara.com/mcp",
143143
"headers": { "Authorization": "Bearer omnara_pat_v1_…" }
144144
}
145145
}

‎frontend/apps/web/src/routes/OAuthAuthorize.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ function oauthAuthorizeApi({
2525
redirect_uri: 'http://127.0.0.1:3000/callback',
2626
redirect_host: '127.0.0.1:3000',
2727
loopback: true,
28-
resource: 'https://omnara.test/api/mcp',
28+
resource: 'https://omnara.test/mcp',
2929
}),
3030
approve = () => jsonResponse({ redirect_url: 'http://127.0.0.1:3000/callback?code=abc' }),
3131
deny = neverResponds,

‎internal/httpapi/apimcp/server.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
)
2424

2525
const (
26-
Path = "/api/mcp"
26+
Path = "/mcp"
2727

2828
serverName = "omnara"
2929
bodyArgument = "body"

‎internal/httpapi/auth/handler.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ type Handler struct {
8888
signupEnabled bool
8989
resetEnabled bool
9090
publicURL string
91+
mcpResourceURLs []string
9192
trustedProxyNets []*net.IPNet
9293
principalFromContext PrincipalFunc
9394
httpClient *http.Client
@@ -104,6 +105,7 @@ type Config struct {
104105
SignupEnabled bool
105106
ResetEnabled bool
106107
PublicURL string
108+
MCPResourceURLs []string
107109
TrustedProxyNets []*net.IPNet
108110
PrincipalFromContext PrincipalFunc
109111
HTTPClient *http.Client
@@ -170,6 +172,7 @@ func New(config Config) *Handler {
170172
signupEnabled: config.SignupEnabled,
171173
resetEnabled: config.ResetEnabled,
172174
publicURL: config.PublicURL,
175+
mcpResourceURLs: config.MCPResourceURLs,
173176
trustedProxyNets: config.TrustedProxyNets,
174177
principalFromContext: config.PrincipalFromContext,
175178
httpClient: httpClientWithoutRedirects(config.HTTPClient),

‎internal/httpapi/auth/oauth_authorize.go‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,15 @@ func (r oauthAuthorizeRequest) redirectError(code, description string) *oauthAut
7272
return &oauthAuthorizeError{code: code, description: description, redirectURI: r.RedirectURI, state: r.State}
7373
}
7474

75-
func (h *Handler) mcpResourceURL(r *http.Request) string {
75+
func (h *Handler) allowedMCPResourceURLs(r *http.Request) []string {
76+
if len(h.mcpResourceURLs) > 0 {
77+
return h.mcpResourceURLs
78+
}
7679
issuer := h.issuerURL(r)
7780
if issuer == "" {
78-
return ""
81+
return nil
7982
}
80-
return issuer + apimcp.Path
83+
return []string{issuer + apimcp.Path}
8184
}
8285

8386
func parseOAuthAuthorizeRequest(values url.Values) (oauthAuthorizeRequest, *oauthAuthorizeError) {
@@ -105,7 +108,7 @@ func parseOAuthAuthorizeRequest(values url.Values) (oauthAuthorizeRequest, *oaut
105108
return request, nil
106109
}
107110

108-
func (r oauthAuthorizeRequest) validateGrantParams(values url.Values, resource string) *oauthAuthorizeError {
111+
func (r oauthAuthorizeRequest) validateGrantParams(values url.Values, resources []string) *oauthAuthorizeError {
109112
if values.Get("response_type") != "code" {
110113
return r.redirectError("unsupported_response_type", "response_type must be code")
111114
}
@@ -125,8 +128,8 @@ func (r oauthAuthorizeRequest) validateGrantParams(values url.Values, resource s
125128
if r.Resource == "" {
126129
return r.redirectError("invalid_target", "resource is required")
127130
}
128-
if r.Resource != resource {
129-
return r.redirectError("invalid_target", "resource must be "+resource)
131+
if !slices.Contains(resources, r.Resource) {
132+
return r.redirectError("invalid_target", "resource must be one of "+strings.Join(resources, ", "))
130133
}
131134
return nil
132135
}
@@ -249,8 +252,8 @@ func (h *Handler) resolveOAuthAuthorizeRequest(
249252
r *http.Request,
250253
values url.Values,
251254
) (oauthAuthorizeRequest, oauthClientMetadata, *oauthAuthorizeError) {
252-
resource := h.mcpResourceURL(r)
253-
if resource == "" {
255+
resources := h.allowedMCPResourceURLs(r)
256+
if len(resources) == 0 {
254257
return oauthAuthorizeRequest{}, oauthClientMetadata{}, clientError(
255258
"temporarily_unavailable", "issuer is not configured",
256259
)
@@ -268,7 +271,7 @@ func (h *Handler) resolveOAuthAuthorizeRequest(
268271
"invalid_request", "redirect_uri is not registered for this client",
269272
)
270273
}
271-
if authErr := request.validateGrantParams(values, resource); authErr != nil {
274+
if authErr := request.validateGrantParams(values, resources); authErr != nil {
272275
return oauthAuthorizeRequest{}, oauthClientMetadata{}, authErr
273276
}
274277
return request, metadata, nil

‎internal/httpapi/auth/oauth_authorize_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010
"testing"
1111
)
1212

13-
const testResource = "https://omnara.test/api/mcp"
13+
const testResource = "https://omnara.test/mcp"
1414

1515
func validAuthorizeValues() url.Values {
1616
return url.Values{
@@ -29,7 +29,7 @@ func resolveAuthorizeValues(values url.Values) (oauthAuthorizeRequest, *oauthAut
2929
if authErr != nil {
3030
return oauthAuthorizeRequest{}, authErr
3131
}
32-
if authErr := request.validateGrantParams(values, testResource); authErr != nil {
32+
if authErr := request.validateGrantParams(values, []string{testResource}); authErr != nil {
3333
return oauthAuthorizeRequest{}, authErr
3434
}
3535
return request, nil

‎internal/httpapi/host_guard.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
type configuredOrigin struct {
1414
scheme string
1515
host string
16+
url string
1617
}
1718

1819
func parseConfiguredOrigin(raw string) (configuredOrigin, error) {
@@ -25,7 +26,11 @@ func parseConfiguredOrigin(raw string) (configuredOrigin, error) {
2526
return configuredOrigin{}, fmt.Errorf("invalid public URL %q", raw)
2627
}
2728
scheme := strings.ToLower(parsed.Scheme)
28-
return configuredOrigin{scheme: scheme, host: httporigin.CanonicalHost(scheme, parsed.Host)}, nil
29+
return configuredOrigin{
30+
scheme: scheme,
31+
host: httporigin.CanonicalHost(scheme, parsed.Host),
32+
url: scheme + "://" + parsed.Host,
33+
}, nil
2934
}
3035

3136
func (o configuredOrigin) matchesHost(rawHost string) bool {

‎internal/httpapi/mcp_resource_metadata.go‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,27 +22,45 @@ func (s *Server) issuerURL(r *http.Request) string {
2222
return scheme + "://" + r.Host
2323
}
2424

25+
func (s *Server) mcpResourceOrigin(r *http.Request) string {
26+
for _, origin := range s.publicOrigins {
27+
if origin.matchesHost(r.Host) {
28+
return origin.url
29+
}
30+
}
31+
return s.issuerURL(r)
32+
}
33+
2534
func (s *Server) mcpResourceURL(r *http.Request) string {
26-
issuer := s.issuerURL(r)
27-
if issuer == "" {
35+
origin := s.mcpResourceOrigin(r)
36+
if origin == "" {
2837
return ""
2938
}
30-
return issuer + apimcp.Path
39+
return origin + apimcp.Path
40+
}
41+
42+
func (s *Server) mcpResourceURLs() []string {
43+
resources := make([]string, 0, len(s.publicOrigins))
44+
for _, origin := range s.publicOrigins {
45+
resources = append(resources, origin.url+apimcp.Path)
46+
}
47+
return resources
3148
}
3249

3350
func (s *Server) mcpProtectedResourceMetadataURL(r *http.Request) string {
34-
return s.issuerURL(r) + mcpProtectedResourceMetadataPath
51+
return s.mcpResourceOrigin(r) + mcpProtectedResourceMetadataPath
3552
}
3653

3754
func (s *Server) mcpProtectedResourceMetadataRoute(w http.ResponseWriter, r *http.Request) {
3855
issuer := s.issuerURL(r)
39-
if issuer == "" {
56+
resource := s.mcpResourceURL(r)
57+
if issuer == "" || resource == "" {
4058
s.notFound(w, r)
4159
return
4260
}
4361
w.Header().Set("Cache-Control", "public, max-age=3600")
4462
writeJSON(w, http.StatusOK, map[string]any{
45-
"resource": issuer + apimcp.Path,
63+
"resource": resource,
4664
"authorization_servers": []string{issuer},
4765
"bearer_methods_supported": []string{"header"},
4866
"resource_name": "Omnara",

‎internal/httpapi/mcp_server_oauth_integration_test.go‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ import (
2121
"github.com/omnara-ai/omnara/internal/testutil/storagetest"
2222
)
2323

24-
const mcpOAuthTestPublicURL = "https://omnara.test"
24+
const (
25+
mcpOAuthTestPublicURL = "https://omnara.test"
26+
mcpOAuthTestPublicAPIURL = "https://api.omnara.test"
27+
)
2528

2629
type mcpOAuthTestClient struct {
2730
server *httptest.Server
@@ -134,6 +137,7 @@ func TestMCPServerOAuthAuthorizationCodeFlow(t *testing.T) {
134137
handler := newIntegrationServer(
135138
pool,
136139
WithPublicURL(mcpOAuthTestPublicURL),
140+
WithPublicAPIURL(mcpOAuthTestPublicAPIURL+"/v1"),
137141
WithOAuthClientMetadataHTTPClient(client.server.Client()),
138142
WithAuthRateLimiter(allowAllAuthLimiter{}),
139143
)
@@ -154,13 +158,13 @@ func TestMCPServerOAuthAuthorizationCodeFlow(t *testing.T) {
154158
if rec.Code != http.StatusUnauthorized {
155159
t.Fatalf("unauthenticated mcp status=%d body=%s", rec.Code, rec.Body.String())
156160
}
157-
wantChallenge := `Bearer resource_metadata="` + mcpOAuthTestPublicURL + `/.well-known/oauth-protected-resource/api/mcp"`
161+
wantChallenge := `Bearer resource_metadata="` + mcpOAuthTestPublicURL + `/.well-known/oauth-protected-resource/mcp"`
158162
if got := rec.Header().Get("WWW-Authenticate"); got != wantChallenge {
159163
t.Fatalf("WWW-Authenticate = %q, want %q", got, wantChallenge)
160164
}
161165

162166
rec = performRequest(handler, httptest.NewRequest(
163-
http.MethodGet, mcpOAuthTestPublicURL+"/.well-known/oauth-protected-resource/api/mcp", nil,
167+
http.MethodGet, mcpOAuthTestPublicURL+"/.well-known/oauth-protected-resource/mcp", nil,
164168
))
165169
if rec.Code != http.StatusOK {
166170
t.Fatalf("protected resource metadata status=%d body=%s", rec.Code, rec.Body.String())
@@ -170,6 +174,27 @@ func TestMCPServerOAuthAuthorizationCodeFlow(t *testing.T) {
170174
t.Fatalf("protected resource metadata = %v", prm)
171175
}
172176

177+
apiHostResource := mcpOAuthTestPublicAPIURL + apimcp.Path
178+
apiHostToolCall := mcpToolCallRequest("", "whoami")
179+
apiHostToolCall.URL.Host = "api.omnara.test"
180+
apiHostToolCall.Host = "api.omnara.test"
181+
rec = performRequest(handler, apiHostToolCall)
182+
wantAPIHostChallenge := `Bearer resource_metadata="` + mcpOAuthTestPublicAPIURL + `/.well-known/oauth-protected-resource/mcp"`
183+
if rec.Code != http.StatusUnauthorized || rec.Header().Get("WWW-Authenticate") != wantAPIHostChallenge {
184+
t.Fatalf("api host unauthenticated mcp status=%d WWW-Authenticate=%q", rec.Code, rec.Header().Get("WWW-Authenticate"))
185+
}
186+
rec = performRequest(handler, httptest.NewRequest(
187+
http.MethodGet, mcpOAuthTestPublicAPIURL+"/.well-known/oauth-protected-resource/mcp", nil,
188+
))
189+
if rec.Code != http.StatusOK {
190+
t.Fatalf("api host protected resource metadata status=%d body=%s", rec.Code, rec.Body.String())
191+
}
192+
apiHostPRM := decodeJSONBody(t, rec)
193+
if apiHostPRM["resource"] != apiHostResource ||
194+
jsonFirstElement(t, apiHostPRM, "authorization_servers") != mcpOAuthTestPublicURL {
195+
t.Fatalf("api host protected resource metadata = %v", apiHostPRM)
196+
}
197+
173198
rec = performRequest(handler, httptest.NewRequest(
174199
http.MethodGet, mcpOAuthTestPublicURL+"/.well-known/oauth-authorization-server", nil,
175200
))

‎internal/httpapi/middleware_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ func TestRequiresAuthCoversAllAPIRoutes(t *testing.T) {
1818
{path: "/api/v1/daemon/runtimes/abc/socket", want: true},
1919
{path: "/api/v1/orgs/abc/skills/skl_abc/archive", want: true},
2020
{path: "/api/v1/orgs", want: true},
21-
{path: "/api/mcp", want: true},
21+
{path: "/mcp", want: true},
2222
{path: "/healthz", want: false},
2323
{path: "/api/auth/logout", want: true},
2424
}

0 commit comments

Comments
 (0)