auth: bind GitHub OIDC token exchange to a per-deployment audience (#1229)
## What this does The publisher CLI now derives the OIDC audience to request from the `--registry` URL instead of using a fixed constant, and the server validates the token's `aud` claim against a new per-deployment env var (`MCP_REGISTRY_GITHUB_OIDC_AUDIENCE`). Production sets this env var to `https://registry.modelcontextprotocol.io` (and per-environment URL elsewhere), so the deployment only accepts tokens that were requested for it. ## Compatibility This is a breaking change for older `mcp-publisher` binaries that still request the legacy hardcoded audience. After this rolls to a deployment, users on old `mcp-publisher` will see a clear `invalid audience: expected …, got […]` error and need to upgrade. Users following the documented `releases/latest` install pattern get the new binary automatically. ## Test plan - [ ] CI green - [ ] Run a synthetic publish from a real GitHub Actions workflow against staging after deploy - [ ] Confirm an older `mcp-publisher` against the same staging deploy returns the expected `invalid audience` error 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
88bbb851ad
commit
3f89fc2b1f
6 changed files with 166 additions and 8 deletions
|
|
@ -7,7 +7,9 @@ import (
|
|||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"os"
|
||||
"strings"
|
||||
)
|
||||
|
||||
type GitHubOIDCProvider struct {
|
||||
|
|
@ -23,8 +25,13 @@ func NewGitHubOIDCProvider(registryURL string) Provider {
|
|||
|
||||
// GetToken retrieves the registry JWT token using GitHub Actions OIDC token
|
||||
func (o *GitHubOIDCProvider) GetToken(ctx context.Context) (string, error) {
|
||||
audience, err := audienceFromRegistryURL(o.registryURL)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("invalid --registry URL: %w", err)
|
||||
}
|
||||
|
||||
// Get OIDC token from GitHub Actions endpoint
|
||||
oidcToken, err := o.getOIDCTokenFromGitHub(ctx)
|
||||
oidcToken, err := o.getOIDCTokenFromGitHub(ctx, audience)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("failed to get OIDC token from GitHub: %w", err)
|
||||
}
|
||||
|
|
@ -99,8 +106,8 @@ func (o *GitHubOIDCProvider) exchangeOIDCTokenForRegistry(ctx context.Context, o
|
|||
return tokenResp.RegistryToken, nil
|
||||
}
|
||||
|
||||
// getOIDCTokenFromGitHub fetches the OIDC token from GitHub Actions endpoint
|
||||
func (o *GitHubOIDCProvider) getOIDCTokenFromGitHub(ctx context.Context) (string, error) {
|
||||
// getOIDCTokenFromGitHub fetches the OIDC token from GitHub Actions endpoint.
|
||||
func (o *GitHubOIDCProvider) getOIDCTokenFromGitHub(ctx context.Context, audience string) (string, error) {
|
||||
// Check for required environment variables
|
||||
requestToken := os.Getenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN")
|
||||
if requestToken == "" {
|
||||
|
|
@ -113,7 +120,7 @@ func (o *GitHubOIDCProvider) getOIDCTokenFromGitHub(ctx context.Context) (string
|
|||
}
|
||||
|
||||
// Build the full URL with audience parameter
|
||||
fullURL := requestURL + "&audience=mcp-registry"
|
||||
fullURL := requestURL + "&audience=" + url.QueryEscape(audience)
|
||||
|
||||
// Create the request
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, fullURL, nil) //nolint:gosec // G704: URL is from GitHub Actions ACTIONS_ID_TOKEN_REQUEST_URL env var
|
||||
|
|
@ -158,3 +165,19 @@ func (o *GitHubOIDCProvider) getOIDCTokenFromGitHub(ctx context.Context) (string
|
|||
|
||||
return tokenResp.Value, nil
|
||||
}
|
||||
|
||||
// audienceFromRegistryURL returns scheme + lowercased host for the given URL.
|
||||
func audienceFromRegistryURL(registryURL string) (string, error) {
|
||||
registryURL = strings.TrimSpace(registryURL)
|
||||
if registryURL == "" {
|
||||
return "", fmt.Errorf("registry URL is empty")
|
||||
}
|
||||
u, err := url.Parse(registryURL)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("parse %q: %w", registryURL, err)
|
||||
}
|
||||
if u.Scheme == "" || u.Host == "" {
|
||||
return "", fmt.Errorf("registry URL must include scheme and host: %q", registryURL)
|
||||
}
|
||||
return u.Scheme + "://" + strings.ToLower(u.Host), nil
|
||||
}
|
||||
|
|
|
|||
48
cmd/publisher/auth/github-oidc_internal_test.go
Normal file
48
cmd/publisher/auth/github-oidc_internal_test.go
Normal file
|
|
@ -0,0 +1,48 @@
|
|||
package auth
|
||||
|
||||
import "testing"
|
||||
|
||||
func TestAudienceFromRegistryURL(t *testing.T) {
|
||||
tests := []struct {
|
||||
input string
|
||||
want string
|
||||
wantErr bool
|
||||
}{
|
||||
// Canonical forms
|
||||
{"https://registry.modelcontextprotocol.io", "https://registry.modelcontextprotocol.io", false},
|
||||
{"https://staging.registry.modelcontextprotocol.io", "https://staging.registry.modelcontextprotocol.io", false},
|
||||
|
||||
// Trailing slash and path are stripped
|
||||
{"https://registry.modelcontextprotocol.io/", "https://registry.modelcontextprotocol.io", false},
|
||||
{"https://registry.modelcontextprotocol.io/api/v0", "https://registry.modelcontextprotocol.io", false},
|
||||
|
||||
// Host is lowercased
|
||||
{"https://Registry.Example.COM", "https://registry.example.com", false},
|
||||
|
||||
// Whitespace is tolerated
|
||||
{" https://registry.example ", "https://registry.example", false},
|
||||
|
||||
// Invalid inputs
|
||||
{"", "", true},
|
||||
{"registry.example", "", true}, // missing scheme
|
||||
{"https://", "", true}, // missing host
|
||||
{"://nothing", "", true}, // missing scheme
|
||||
}
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.input, func(t *testing.T) {
|
||||
got, err := audienceFromRegistryURL(tc.input)
|
||||
if tc.wantErr {
|
||||
if err == nil {
|
||||
t.Fatalf("expected error for %q, got %q", tc.input, got)
|
||||
}
|
||||
return
|
||||
}
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error for %q: %v", tc.input, err)
|
||||
}
|
||||
if got != tc.want {
|
||||
t.Errorf("audienceFromRegistryURL(%q) = %q, want %q", tc.input, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
@ -185,6 +185,15 @@ func DeployMCPRegistry(ctx *pulumi.Context, cluster *providers.ProviderInfo, env
|
|||
Name: pulumi.String("MCP_REGISTRY_OIDC_PUBLISH_PERMISSIONS"),
|
||||
Value: pulumi.String("*"),
|
||||
},
|
||||
&corev1.EnvVarArgs{
|
||||
Name: pulumi.String("MCP_REGISTRY_GITHUB_OIDC_AUDIENCE"),
|
||||
Value: pulumi.String(func() string {
|
||||
if environment == "prod" {
|
||||
return "https://registry.modelcontextprotocol.io"
|
||||
}
|
||||
return "https://" + environment + ".registry.modelcontextprotocol.io"
|
||||
}()),
|
||||
},
|
||||
},
|
||||
// StartupProbe protects the DB-retry budget in cmd/registry/main.go:
|
||||
// 30 × 5s = 150s allowed before liveness/readiness take over, which
|
||||
|
|
|
|||
|
|
@ -252,8 +252,10 @@ func RegisterGitHubOIDCEndpoint(api huma.API, pathPrefix string, cfg *config.Con
|
|||
|
||||
// ExchangeToken exchanges a GitHub OIDC token for a Registry JWT token
|
||||
func (h *GitHubOIDCHandler) ExchangeToken(ctx context.Context, oidcToken string) (*auth.TokenResponse, error) {
|
||||
// Validate OIDC token with audience "mcp-registry"
|
||||
claims, err := h.validator.ValidateToken(ctx, oidcToken, "mcp-registry")
|
||||
if h.config.GitHubOIDCAudience == "" {
|
||||
return nil, fmt.Errorf("GitHub OIDC exchange disabled: this deployment has no audience configured")
|
||||
}
|
||||
claims, err := h.validator.ValidateToken(ctx, oidcToken, h.config.GitHubOIDCAudience)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to validate OIDC token: %w", err)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -26,7 +26,8 @@ func (m *MockOIDCValidator) ValidateToken(ctx context.Context, token string, aud
|
|||
|
||||
func TestGitHubOIDCHandler_ExchangeToken(t *testing.T) {
|
||||
cfg := &config.Config{
|
||||
JWTPrivateKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", // 32 bytes hex
|
||||
JWTPrivateKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", // 32 bytes hex
|
||||
GitHubOIDCAudience: "https://registry.test",
|
||||
}
|
||||
|
||||
handler := auth.NewGitHubOIDCHandler(cfg)
|
||||
|
|
@ -118,9 +119,82 @@ func TestGitHubOIDCHandler_ExchangeToken(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestGitHubOIDCHandler_AudienceBinding(t *testing.T) {
|
||||
t.Run("empty audience config fails closed", func(t *testing.T) {
|
||||
cfg := &config.Config{
|
||||
JWTPrivateKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
|
||||
GitHubOIDCAudience: "",
|
||||
}
|
||||
handler := auth.NewGitHubOIDCHandler(cfg)
|
||||
// Validator should never be called — the audience precondition rejects first.
|
||||
handler.SetValidator(&MockOIDCValidator{
|
||||
validateFunc: func(_ context.Context, _ string, _ string) (*auth.GitHubOIDCClaims, error) {
|
||||
t.Fatal("validator must not be called when audience is unconfigured")
|
||||
return nil, fmt.Errorf("unreachable")
|
||||
},
|
||||
})
|
||||
|
||||
_, err := handler.ExchangeToken(context.Background(), "any-token")
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "no audience configured")
|
||||
})
|
||||
|
||||
t.Run("configured audience is what the validator sees", func(t *testing.T) {
|
||||
cfg := &config.Config{
|
||||
JWTPrivateKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
|
||||
GitHubOIDCAudience: "https://registry.example.com",
|
||||
}
|
||||
handler := auth.NewGitHubOIDCHandler(cfg)
|
||||
|
||||
var seenAudience string
|
||||
handler.SetValidator(&MockOIDCValidator{
|
||||
validateFunc: func(_ context.Context, _ string, audience string) (*auth.GitHubOIDCClaims, error) {
|
||||
seenAudience = audience
|
||||
return &auth.GitHubOIDCClaims{
|
||||
RegisteredClaims: jwt.RegisteredClaims{
|
||||
Subject: "repo:octo-org/octo-repo:environment:prod",
|
||||
ExpiresAt: jwt.NewNumericDate(time.Now().Add(1 * time.Hour)),
|
||||
Audience: jwt.ClaimStrings{audience},
|
||||
},
|
||||
RepositoryOwner: "octo-org",
|
||||
}, nil
|
||||
},
|
||||
})
|
||||
|
||||
_, err := handler.ExchangeToken(context.Background(), "any-token")
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "https://registry.example.com", seenAudience)
|
||||
})
|
||||
|
||||
t.Run("token issued for another deployment is rejected", func(t *testing.T) {
|
||||
cfg := &config.Config{
|
||||
JWTPrivateKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
|
||||
GitHubOIDCAudience: "https://registry.modelcontextprotocol.io",
|
||||
}
|
||||
handler := auth.NewGitHubOIDCHandler(cfg)
|
||||
|
||||
// The mock validator does what the real one does: reject if the
|
||||
// expected audience is not present in the token's aud claim.
|
||||
handler.SetValidator(&MockOIDCValidator{
|
||||
validateFunc: func(_ context.Context, _ string, expected string) (*auth.GitHubOIDCClaims, error) {
|
||||
tokenAudience := "https://attacker-registry.example" //nolint:gosec // G101 false positive: URL string, not a credential
|
||||
if expected != tokenAudience {
|
||||
return nil, fmt.Errorf("invalid audience: expected %s, got [%s]", expected, tokenAudience)
|
||||
}
|
||||
return &auth.GitHubOIDCClaims{RepositoryOwner: "octo-org"}, nil
|
||||
},
|
||||
})
|
||||
|
||||
_, err := handler.ExchangeToken(context.Background(), "captured-token")
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "invalid audience")
|
||||
})
|
||||
}
|
||||
|
||||
func TestBuildPermissionsFromOIDC(t *testing.T) {
|
||||
cfg := &config.Config{
|
||||
JWTPrivateKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
|
||||
JWTPrivateKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
|
||||
GitHubOIDCAudience: "https://registry.test",
|
||||
}
|
||||
handler := auth.NewGitHubOIDCHandler(cfg)
|
||||
|
||||
|
|
|
|||
|
|
@ -17,6 +17,8 @@ type Config struct {
|
|||
EnableAnonymousAuth bool `env:"ENABLE_ANONYMOUS_AUTH" envDefault:"false"`
|
||||
EnableRegistryValidation bool `env:"ENABLE_REGISTRY_VALIDATION" envDefault:"true"`
|
||||
|
||||
GitHubOIDCAudience string `env:"GITHUB_OIDC_AUDIENCE" envDefault:""`
|
||||
|
||||
// OIDC Configuration
|
||||
OIDCEnabled bool `env:"OIDC_ENABLED" envDefault:"false"`
|
||||
OIDCIssuer string `env:"OIDC_ISSUER" envDefault:""`
|
||||
|
|
|
|||
Loading…
Reference in a new issue