Skip to content

Store serverName on Cedar Authorizer#4861

Open
jhrozek wants to merge 1 commit intomainfrom
jhrozek/store-server-name
Open

Store serverName on Cedar Authorizer#4861
jhrozek wants to merge 1 commit intomainfrom
jhrozek/store-server-name

Conversation

@jhrozek
Copy link
Copy Markdown
Contributor

@jhrozek jhrozek commented Apr 15, 2026

Summary

The AuthorizerFactory interface already passes serverName to CreateAuthorizer, but the Cedar implementation discarded it. This stores it on the Authorizer struct so downstream enterprise features (#4769, #4770) can scope Cedar policies to specific MCP servers. When empty, behavior is identical to today.

The serverName becomes the MCP parent on resource entities (added by #4769), enabling Cedar's in operator to evaluate server-scoped policies like resource in MCP::"<server>". Per the authorization RFC, this prevents a deny rule compiled from one server's policy from silently affecting same-named tools on other servers when the enterprise controller merges policies into a single set.

Closes #4764

Type of change

  • New feature

Test plan

  • Unit tests (task test)
  • Linting (task lint-fix)
  • Manual testing (describe below)

E2E tested in a Kind cluster with real Okta tokens. The key test deploys two MCPServers that share a single Cedar ConfigMap -- the same setup the enterprise controller will produce when it compiles policies from multiple CRDs into one set.

Shared Cedar policy set:
  permit(principal in THVGroup::"engineering",
         action, resource in MCP::"<server-a>");
  permit(principal in THVGroup::"engineering",
         action, resource in MCP::"<server-b>");
  forbid(principal, action == Action::"call_tool",
         resource == Tool::"echo")
    when { resource in MCP::"<server-b>" };

Okta JWT: { "groups": ["Everyone", "engineering"],
            "sub": "jakub@stacklok.com" }

The same user calling the same tool ("echo") gets allowed on server-a and 403'd on server-b. The only difference is the serverName stored on each authorizer, which determines the MCP parent on the resource entity. Cedar's diagnostic log confirms the forbid fires only where scoped and does not bleed across servers.

Special notes for reviewers

This is the second of a stacked PR series for #376. Depends on #4847 (merged). The next PR (#4765, variadic parents on entity factory) depends on this one.

Generated with Claude Code

The AuthorizerFactory interface already passes serverName to
CreateAuthorizer, but the Cedar implementation discarded it. Store it
on the Authorizer struct so downstream enterprise features (#4769,
behavior is identical to today.

The serverName becomes the MCP parent on resource entities (added by
policies like "resource in MCP::<server>". Per the authorization RFC,
this prevents a deny rule compiled from one server's policy from
silently affecting same-named tools on other servers when the
enterprise controller merges policies into a single set.

E2E tested in a Kind cluster with real Okta tokens. The key test
deploys two MCPServers that share a single Cedar ConfigMap -- the
same setup the enterprise controller will produce when it compiles
policies from multiple CRDs into one set.

  Shared Cedar policy set:
    permit(principal in THVGroup::"engineering",
           action, resource in MCP::"<server-a>");
    permit(principal in THVGroup::"engineering",
           action, resource in MCP::"<server-b>");
    forbid(principal, action == Action::"call_tool",
           resource == Tool::"echo")
      when { resource in MCP::"<server-b>" };

  Okta JWT: { "groups": ["Everyone", "engineering"],
              "sub": "jakub@stacklok.com" }

The same user calling the same tool ("echo") gets allowed on server-a
and 403'd on server-b. The only difference is the serverName stored
on each authorizer, which determines the MCP parent on the resource
entity. Cedar's diagnostic log confirms the forbid fires only where
scoped and does not bleed across servers.

Closes #4764

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added the size/XS Extra small PR: < 100 lines changed label Apr 15, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.06%. Comparing base (9d5982d) to head (c56bc5d).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4861      +/-   ##
==========================================
- Coverage   69.11%   69.06%   -0.06%     
==========================================
  Files         530      531       +1     
  Lines       55128    55173      +45     
==========================================
+ Hits        38102    38104       +2     
- Misses      14102    14140      +38     
- Partials     2924     2929       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — straightforward preparatory change that stores the previously-discarded serverName on the Cedar authorizer struct with no behavioral impact when empty.

Extended reasoning...

Overview

The PR modifies pkg/authz/authorizers/cedar/core.go and associated test files. The sole logic change is renaming the ignored _ string parameter in CreateAuthorizer and NewCedarAuthorizer to serverName and storing it on the Authorizer struct. The remaining diff is mechanical: every NewCedarAuthorizer(...) call in tests gains a trailing "" argument.

Security risks

None. The field is stored but not used in any authorization decision in this PR. When serverName is empty (all existing callers), behavior is identical to before.

Level of scrutiny

Low. This is an infrastructure/plumbing change with zero behavioral impact today. It follows the existing pattern for other config fields (groupClaimName, roleClaimName) that are stored on the struct and used later. The code comment even documents the forward-compatibility intent ("If a second runtime-injected value is needed, bundle both into a RuntimeContext struct").

Other factors

  • No CODEOWNER-sensitive paths touched beyond the Cedar authorizer package itself.
  • Test coverage is adequate: a dedicated Stores server name test case and an assertion in TestFactoryCreateAuthorizer verify the field round-trips correctly.
  • The PR is part of a documented stacked series; its dependency (#4847) is already merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Store serverName on Authorizer and update NewCedarAuthorizer

1 participant