diff --git a/CHANGELOG.md b/CHANGELOG.md index 0569eda2..e3b0ba25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,6 +70,7 @@ Targets the 2.4.0 release. Significant additions include enterprise SSO (SAML Id ### Deprecated +- **`--mcp-authorizer-url`** — **has no effect as of 2.4.0**; pass `--url` with the same value instead. The two fed different mechanisms: `--url` sets the trusted URL, consulted *before* any header, while this flag only stamped an `x-authorizer-url` header. `authorizer mcp` now honours `--url` (see Fixed), which supplies the value outright — so the flag is read by nothing and a stale or wrong value can no longer affect issuer validation. It is still **parsed**, so a 2.3.x invocation keeps starting rather than dying on `unknown flag`; it warns and is ignored. Removed in 2.5.0 with the subcommand. Setting `--mcp-bearer` without `--url` is now refused at startup with an explanatory message, rather than failing later as a bare `Unauthenticated` on every tool call. - **`authorizer mcp` (stdio transport)** — superseded by `--mcp-enabled`, removed in 2.5.0. The stdio subcommand ran a second copy of every provider (storage, memory store, embedded FGA engine) alongside the real server, and its identity was a single process-wide `--mcp-bearer`, so one process could only ever serve one user. Both are gone with the HTTP transport: the MCP surface shares the running server's providers, and every request carries its own token. The subcommand keeps working and now prints a deprecation notice. ### Security @@ -94,6 +95,7 @@ Targets the 2.4.0 release. Significant additions include enterprise SSO (SAML Id ### Fixed +- **`authorizer mcp` silently ignored `--url`**: the subcommand inherits the root flag set, so `--url` was always *accepted* there, but `parsers.SetTrustedURL` was only called from the server's own startup path. Passing `--url` to the stdio MCP subcommand therefore looked configured and did nothing, leaving issuer validation on header derivation and making `--mcp-authorizer-url` the only mechanism that worked. `runMCP` now pins the trusted URL exactly as the server does. A flag that is accepted and ignored is worse than one that is rejected. - **Public client_id now exposed in Client API type**: `Client` GraphQL type and proto now include `client_id` (distinct from surrogate `id`). Dashboard Clients page displays correct "Client ID" (the configured client_id, not the internal id). Seeded interactive client has immutable `client_id` from `--client-id` flag ([#664](https://github.com/authorizerdev/authorizer/pull/664)). - **Nil-pointer panics in claim/header type assertions**: two unguarded type assertions on untrusted map values (email-verify redirect-uri and webhook-event headers) could panic and crash the process; now guarded with safe type coercion ([#701](https://github.com/authorizerdev/authorizer/pull/701)). - **Dashboard and login UI crashes**: CSV file import error handling in dashboard, non-null assertion guards in InputField component, logout button event handling, and WCAG label association for home realm discovery email input ([#702](https://github.com/authorizerdev/authorizer/pull/702)). diff --git a/cmd/mcp.go b/cmd/mcp.go index 85241421..3b73d44f 100644 --- a/cmd/mcp.go +++ b/cmd/mcp.go @@ -4,6 +4,7 @@ import ( "context" "os" "os/signal" + "strings" "syscall" "github.com/rs/zerolog" @@ -18,6 +19,7 @@ import ( "github.com/authorizerdev/authorizer/internal/grpcsrv" "github.com/authorizerdev/authorizer/internal/mcp" "github.com/authorizerdev/authorizer/internal/memory_store" + "github.com/authorizerdev/authorizer/internal/parsers" "github.com/authorizerdev/authorizer/internal/service" "github.com/authorizerdev/authorizer/internal/sms" "github.com/authorizerdev/authorizer/internal/storage" @@ -34,9 +36,11 @@ var mcpArgs struct { // (`profile`, `check_permissions`, `list_permissions`) won't have a // caller to attribute to. bearer string - // authorizerURL is the public URL of the Authorizer instance that - // minted the bearer token; stamped as `x-authorizer-url` so JWT issuer - // validation passes for identity-bearing tools. + // authorizerURL is DEPRECATED and has NO EFFECT as of 2.4.0. It is still + // parsed so that an existing 2.3.x invocation keeps starting instead of + // dying on `unknown flag`, but nothing reads it — the value is supplied by + // --url, which runMCP now pins as the trusted URL. Kept as a named field + // rather than discarded so the flag registration below stays readable. authorizerURL string } @@ -85,11 +89,29 @@ func init() { "user identity for tools like Profile / Permissions / Session). "+ "When unset the MCP server runs anonymously; public tools (Meta) "+ "still work but identity-bearing tools will fail authn.") + + // DEPRECATED and INERT as of 2.4.0. Superseded by --url. + // + // The two fed different mechanisms: --url sets the trusted URL, which + // GetHostFromRequest consults before it reads any header, while this flag + // only stamped an `x-authorizer-url` header. Until runMCP started calling + // SetTrustedURL, --url was accepted here and silently did nothing, so this + // was the only mechanism that worked in the stdio path. + // + // Now that --url is honoured here it supplies the value outright, and this + // flag is read by nothing. Still PARSED rather than deleted so a 2.3.x + // invocation keeps starting instead of dying on `unknown flag` — it warns + // and is ignored. Goes with the subcommand in 2.5.0. mcpCmd.Flags().StringVar(&mcpArgs.authorizerURL, "mcp-authorizer-url", "", - "Public URL of the Authorizer instance that issued --mcp-bearer "+ - "(e.g. https://auth.example.com). Required with --mcp-bearer: "+ - "JWT issuer validation compares the token's iss claim against "+ - "this value.") + "DEPRECATED and ignored — use --url instead.") + if err := mcpCmd.Flags().MarkDeprecated("mcp-authorizer-url", + "it has NO EFFECT as of 2.4.0 — pass --url with the same value instead. "+ + "--url is required for the server and is honoured by this subcommand, "+ + "and is what the token's iss claim is validated against."); err != nil { + // Only fails when the flag name does not exist, which is a + // programming error in the line directly above. + panic(err) + } RootCmd.AddCommand(mcpCmd) } @@ -103,6 +125,31 @@ func runMCP(_ *cobra.Command, _ []string) { log.Warn().Msg("`authorizer mcp` (stdio) is deprecated and will be removed in 2.5.0 — " + "run the server with --mcp-enabled and connect to POST /mcp instead") + // Honour --url here as the server does. + // + // This subcommand inherits the root flag set, so --url was always ACCEPTED + // here — but SetTrustedURL was only ever called from runRoot, so it silently + // did nothing, and --mcp-authorizer-url (which stamped an `x-authorizer-url` + // header) was the only mechanism that worked. A flag that is accepted and + // ignored is worse than one that is rejected: + // `authorizer mcp --url=https://auth.example.com` looked configured and left + // issuer validation on header derivation. Wiring it here is what lets + // --mcp-authorizer-url become inert without breaking the stdio path. + parsers.SetTrustedURL(rootArgs.config.AuthorizerURL) + parsers.SetLogger(&log) + + // Identity-bearing tools validate the bearer's `iss` against this server's + // own URL. With no --url there is nothing to compare against — and no HTTP + // request to derive a host from either, since tool calls arrive as gRPC + // metadata — so every such call fails with a bare `Unauthenticated` that + // looks like a bad token rather than missing config. Say so up front. + if mcpArgs.bearer != "" && strings.TrimSpace(rootArgs.config.AuthorizerURL) == "" { + log.Fatal().Msg("--url is required with --mcp-bearer: identity-bearing tools " + + "validate the token's iss claim against this server's own URL, and without " + + "it every tool call fails as Unauthenticated. Pass --url=, e.g. https://auth.example.com") + } + // Wire all subsystems an MCP-exposed tool might need. As more ops // migrate into internal/service, this list stays the same — the // service-provider dependencies don't change per op, only the methods @@ -206,7 +253,7 @@ func runMCP(_ *cobra.Command, _ []string) { Name: "authorizer", Version: constants.VERSION, Bearer: mcpArgs.bearer, - AuthorizerURL: mcpArgs.authorizerURL, + AuthorizerURL: rootArgs.config.AuthorizerURL, }) if err != nil { log.Fatal().Err(err).Msg("failed to create mcp server") diff --git a/internal/e2e/smoke_test.go b/internal/e2e/smoke_test.go index 2e0d3d7a..1f64bf05 100644 --- a/internal/e2e/smoke_test.go +++ b/internal/e2e/smoke_test.go @@ -493,8 +493,10 @@ func TestReleaseSmoke(t *testing.T) { "--jwt-type=HS256", "--jwt-secret=" + smokeJWTSecret, "--admin-secret=" + smokeAdminSecret, "--client-id=" + smokeClientID, "--client-secret=" + smokeClientSecret, + // Replaces --mcp-authorizer-url, inert as of 2.4.0. This is what the + // bearer's iss claim is validated against. + "--url=" + baseURL, "--mcp-bearer=" + token, - "--mcp-authorizer-url=" + baseURL, } mcp := startMCP(t, bin, mcpArgs)