registry/internal/api/handlers/v0/auth/github_at.go
Tadas Antanavicius 2bc37b1999
fix(auth): grant org namespace only to org Owners, not all members (#1383)
## The bug (scoped to `github-at`)

When you exchange a GitHub OAuth/PAT token for a registry JWT, the
handler granted `io.github.<org>/*` publish permission for **every**
organization you belong to. It derived that list from [`GET
/users/{username}/orgs`](https://docs.github.com/en/rest/orgs/orgs#list-organizations-for-a-user),
which:

1. returns only **public** org memberships, and
2. carries **no role** — an org Owner and a brand-new member look
identical.

So membership was silently treated as publishing authority. Anyone who
is (or whose public membership lists them as) a member of `acme` could
publish — and, because publish is a pure namespace-glob with no
per-server ownership check, **overwrite** — any server under
`io.github.acme/*`.

## The fix

Switch to [`GET
/user/memberships/orgs`](https://docs.github.com/en/rest/orgs/members#list-organization-memberships-for-the-authenticated-user),
which returns the **authenticated caller's role** per org (and includes
private memberships), and grant the org namespace only when the role is
`admin` (org Owner).

- **Personal namespaces are unchanged** — you always get
`io.github.<your-username>/*`.
- **Org namespaces now require Owner.** GitHub has no org-level
"maintainer" role; org membership is either `admin` (Owner) or `member`.
We grant only on `admin`.
- **`read:org` is the only scope needed.** A minimal token without it
still authenticates and still publishes to your personal namespace; a
genuine missing-scope `403` from the memberships call is treated as "no
admin orgs" rather than a hard failure, so we don't push anyone to
over-scope a token. A `403`/`429` that is actually a rate limit
(`X-RateLimit-Remaining: 0` / `Retry-After`) fails closed instead, so a
throttle can't silently strip a real Owner's org grant. Critically, the
token needs **no repository scopes** — the registry never reads or
writes code, so an org Owner can hand CI a near-powerless `read:org`
token.

The authorization decision stays stateless and delegated to GitHub — no
new storage, no new endpoints, no schema changes — consistent with
[design principle #2](docs/design/design-principles.md) (minimal
operational burden).

### Containing the CI token

The registry-side change makes the org Owner the only one who can mint
org-namespace authority. But if that Owner drops the resulting PAT into
a plain repo secret, **every repository writer** can reach it (via
branch/tag pushes that run repo-controlled build/test code in the
privileged job) — quietly re-widening publish access beyond Owners.
External fork contributors still cannot reach it by default (no secrets
on fork PRs, no branch push), barring misconfigurations like
`pull_request_target`-with-checkout or self-hosted runners on public
repos.

`docs/modelcontextprotocol-io/github-actions.mdx` documents how to
contain that blast radius:

- store the token as a **GitHub Actions Environment secret** (the PAT
example uses `environment:`),
- restrict that environment to the default branch / release tags and
require PR reviews on it, so only reviewed, maintainer-approved code can
reach the token,
- optionally add a **required environment reviewer** for explicit
per-publish approval,
- and avoid the two footguns (`pull_request_target` checkout,
self-hosted runners on public repos).

This shrinks *who holds* the org credential; it does not change the fact
that an authorized publish can still overwrite a different server in the
same namespace until per-server publisher ownership exists (the
follow-up noted below).

## Why this shape, given the alternatives

[Pree's
writeup](https://github.com/modelcontextprotocol/registry/issues/982#issuecomment-3982959559)
([Notion analysis
doc](https://www.notion.so/MCP-Registry-Authorization-Model-Analysis-Issue-982-31736f79f74980bc90c6e53de1c564c5))
is the clearest framing of the problem and the option space, laying out
three broad solutions:

- **Approach A — GitHub App as the authority layer.** Strong model, but
its natural unit of authority is a *repo*, so it needs a repo→server
mapping to authorize a namespace. The safe, flexible version of that
mapping is registry-managed state — which collapses into Approach B.
It's also GitHub-only.
- **Approach B — a registry-managed grants table** (OIDC bootstrap, or
device-flow admin check). The most capable and provider-agnostic option,
and probably where v1 wants to go. But it introduces exactly the
"registry manages an authorization database" layer we wanted to avoid
taking on right now: revocation, sync, audit, migration.
- **Approach C — scoped bearer tokens (npm-style).** Good ergonomics,
but again implies issuance/storage the registry would own.

The two-property test (does the registry control the decision? is it
granted through an act requiring real authority?) is the right lens.
This change satisfies it **without** new state: the registry makes the
decision (it filters on role), and the authority is real (live GitHub
Owner role, re-checked on every short-lived token exchange).

**What this deliberately gives up**, so reviewers can weigh it honestly:

- **Owners are always in the loop.** Only org Owners can publish org
servers; there's no delegated "this team can publish this server" — that
needs Approach B's state. Reduced flexibility in exchange for zero new
infrastructure.
- **Blast radius within an org is unchanged.** An Owner (or Owner-scoped
CI token) can still overwrite *any* server under the org namespace,
because there's still no per-server publisher ownership. That
hijack-an-existing-server problem is real but **orthogonal** to this fix
and belongs in its own change (record publisher identity per server;
enforce it on update) — a follow-up to take alongside the OIDC hardening
below.
- **OIDC is untouched and remains coarser.** The `github-oidc` path
still grants the whole `io.github.<owner>/*` namespace to any workflow
with `id-token: write`, i.e. effectively anyone with write access to any
repo in the org, and it excludes non-Actions CI (Jenkins/GitLab can't
mint GitHub Actions OIDC). The admin-PAT path here is the
higher-assurance option for org publishing; OIDC stays as a
lower-assurance Actions convenience to be tightened separately.

## Revocation

The registry JWT is **5-minute, no-refresh** (per #264/#273), so loss of
Owner status propagates within minutes on the next token exchange, with
no revocation list to maintain.

## Tests

`go test ./internal/api/handlers/v0/auth/...` passes, including:

- member-role org is **not** granted; only admin orgs are
- a missing-scope `403` (no `read:org`) still yields the personal
namespace and no error
- an admin org on a **later page** of memberships is still granted
(pagination correctness)
- a `5xx` from the memberships call **fails closed** (no token issued)
rather than degrading to personal-only
- a rate-limit `403` (`X-RateLimit-Remaining: 0` / `Retry-After`)
**fails closed** — not mistaken for a missing-scope `403`
- existing org/name-validation tests updated to the new endpoint + role
shape

`gofmt`/`go vet`/`golangci-lint` (CI-pinned v2.11.4) clean; CI green on
all checks.

## Live verification

Confirmed end-to-end against real `api.github.com` (not just mocks) by
exercising this branch's `ExchangeToken` and decoding the `permissions`
claim of the returned JWT.

A dedicated org, **Tadasant Test Org** (login `Tadasant-Test-Org`), is
Owned by one account and has a second account joined as a plain
**member**. Both memberships are `active`, verified via the same
endpoint the fix uses (`GET /user/memberships/orgs/{org}`):

| Token (role) | `io.github.Tadasant-Test-Org/*` granted? | Other
patterns granted |
|---|---|---|
| Owner (`admin`) | ✅ yes | personal `io.github.<owner>/*` |
| member | ❌ no | personal `io.github.<member>/*` + the orgs that
account actually Owns |

The member case is the meaningful one: that membership is **active and
returned** by `GET /user/memberships/orgs`, and is excluded purely
because its role is `member`, not `admin`. Under the previous `GET
/users/{username}/orgs` behavior (membership = authority, no role), the
same active membership **would have granted** the org namespace. The
role filter is also selective rather than blanket-deny — the member
account still receives the namespaces of the orgs where it genuinely
holds Owner.

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Radoslav Dimitrov <radoslav@stacklok.com>
2026-07-10 17:05:47 -04:00

314 lines
12 KiB
Go

package auth
import (
"context"
"encoding/json"
"fmt"
"log"
"net/http"
"regexp"
"strings"
"github.com/danielgtaylor/huma/v2"
v0 "github.com/modelcontextprotocol/registry/internal/api/handlers/v0"
"github.com/modelcontextprotocol/registry/internal/auth"
"github.com/modelcontextprotocol/registry/internal/config"
)
// GitHubTokenExchangeInput represents the input for GitHub token exchange
type GitHubTokenExchangeInput struct {
Body struct {
GitHubToken string `json:"github_token" doc:"GitHub OAuth token" required:"true"`
}
}
// GitHubHandler handles GitHub authentication
type GitHubHandler struct {
config *config.Config
jwtManager *auth.JWTManager
baseURL string // Configurable for testing
}
// NewGitHubHandler creates a new GitHub handler
func NewGitHubHandler(cfg *config.Config) *GitHubHandler {
return &GitHubHandler{
config: cfg,
jwtManager: auth.NewJWTManager(cfg),
baseURL: "https://api.github.com",
}
}
// SetBaseURL sets the base URL for GitHub API (used for testing)
func (h *GitHubHandler) SetBaseURL(url string) {
h.baseURL = url
}
// RegisterGitHubATEndpoint registers the GitHub access token authentication endpoint with a custom path prefix
func RegisterGitHubATEndpoint(api huma.API, pathPrefix string, cfg *config.Config) {
handler := NewGitHubHandler(cfg)
// GitHub token exchange endpoint
huma.Register(api, huma.Operation{
OperationID: "exchange-github-token" + strings.ReplaceAll(pathPrefix, "/", "-"),
Method: http.MethodPost,
Path: pathPrefix + "/auth/github-at",
Summary: "Exchange GitHub OAuth access token for Registry JWT",
Description: "Exchange a GitHub OAuth access token for a short-lived Registry JWT token",
Tags: []string{"auth"},
}, func(ctx context.Context, input *GitHubTokenExchangeInput) (*v0.Response[auth.TokenResponse], error) {
response, err := handler.ExchangeToken(ctx, input.Body.GitHubToken)
if err != nil {
return nil, tokenExchangeError(err)
}
return &v0.Response[auth.TokenResponse]{
Body: *response,
}, nil
})
}
// tokenExchangeError maps an internal ExchangeToken failure to a client-facing
// 401 without leaking internal detail. huma serializes extra error args into the
// response body, and the wrapped err here can include the raw upstream GitHub
// response body captured by readErrorBody — so passing it to huma would echo that
// detail back to the caller (CWE-209). We log it server-side (like the sibling
// handlers in servers.go) and return only a generic message.
func tokenExchangeError(err error) error {
log.Printf("github-at token exchange failed: %v", err)
return huma.Error401Unauthorized("Token exchange failed")
}
// ExchangeToken exchanges a GitHub OAuth token for a Registry JWT token
func (h *GitHubHandler) ExchangeToken(ctx context.Context, githubToken string) (*auth.TokenResponse, error) {
// Get GitHub user information
user, err := h.getGitHubUser(ctx, githubToken)
if err != nil {
return nil, fmt.Errorf("failed to get GitHub user: %w", err)
}
// Get the organizations the user administers. Org namespaces are only granted
// to org Owners (membership role "admin"), not to ordinary members.
orgs, err := h.getGitHubAdminOrgs(ctx, githubToken)
if err != nil {
return nil, fmt.Errorf("failed to get GitHub organizations: %w", err)
}
// Build permissions based on user and organizations
permissions := h.buildPermissions(user.Login, orgs)
// Create JWT claims with GitHub user info
claims := auth.JWTClaims{
AuthMethod: auth.MethodGitHubAT,
AuthMethodSubject: user.Login,
Permissions: permissions,
}
// Generate Registry JWT token
tokenResponse, err := h.jwtManager.GenerateTokenResponse(ctx, claims)
if err != nil {
return nil, fmt.Errorf("failed to generate JWT token: %w", err)
}
return tokenResponse, nil
}
type GitHubUserOrOrg struct {
Login string `json:"login"`
ID int `json:"id"`
}
// getGitHubUser gets the authenticated user's information
func (h *GitHubHandler) getGitHubUser(ctx context.Context, token string) (*GitHubUserOrOrg, error) {
req, err := http.NewRequestWithContext(ctx, http.MethodGet, h.baseURL+"/user", nil)
if err != nil {
return nil, fmt.Errorf("failed to create request: %w", err)
}
req.Header.Set("Authorization", "Bearer "+token)
req.Header.Set("Accept", "application/vnd.github.v3+json")
req.Header.Set("X-GitHub-Api-Version", "2022-11-28")
resp, err := http.DefaultClient.Do(req)
if err != nil {
return nil, fmt.Errorf("failed to get user info: %w", err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
return nil, fmt.Errorf("GitHub API error (status %d): %s", resp.StatusCode, readErrorBody(resp.Body))
}
var user GitHubUserOrOrg
if err := json.NewDecoder(resp.Body).Decode(&user); err != nil {
return nil, fmt.Errorf("failed to decode user response: %w", err)
}
return &user, nil
}
// githubOrgRoleAdmin is the membership role GitHub returns for an organization
// Owner. It is the only role we treat as carrying publish authority for the org
// namespace. GET /user/memberships/orgs reports the caller's role as either
// "admin" (Owner) or "member" (GitHub has no org-level "maintainer" role); only
// "admin" carries publish authority, so a "member" is intentionally not granted.
const githubOrgRoleAdmin = "admin"
// githubMembershipStateActive is the membership state for an accepted (as opposed
// to merely invited/pending) org membership. We only honor active memberships so a
// not-yet-accepted owner invitation never grants the org namespace.
const githubMembershipStateActive = "active"
// orgMembershipsPageSize is the page size we request when listing the user's org
// memberships. 100 is GitHub's maximum.
const orgMembershipsPageSize = 100
// maxOrgMembershipPages bounds the pagination loop. At orgMembershipsPageSize=100
// this covers 10,000 org memberships — far beyond any real user — and exists only
// as a backstop so a misbehaving or redirected upstream cannot drive an unbounded
// number of requests.
const maxOrgMembershipPages = 100
// githubOrgMembership is one entry from GET /user/memberships/orgs: the
// authenticated user's membership in a single organization, including their role.
type githubOrgMembership struct {
State string `json:"state"`
Role string `json:"role"`
Organization GitHubUserOrOrg `json:"organization"`
}
// getGitHubAdminOrgs returns the organizations in which the authenticated user is
// an Owner (membership role "admin").
//
// This deliberately uses GET /user/memberships/orgs rather than
// GET /users/{username}/orgs. The latter only returns *public* memberships and
// carries no role, so it cannot distinguish an Owner from an ordinary member —
// historically every org a user belonged to was granted publish access to its
// namespace regardless of the user's role. The memberships endpoint returns the
// caller's role per org (and includes private memberships), letting us grant the
// org namespace only to people who actually administer the org.
//
// The endpoint requires the read:org scope. A minimal token used only for
// personal-namespace publishing won't have it and will get a 403 here; that is
// treated as "no admin orgs" (see fetchOrgMembershipsPage) so personal publishing
// keeps working without asking users to over-scope their token.
func (h *GitHubHandler) getGitHubAdminOrgs(ctx context.Context, token string) ([]GitHubUserOrOrg, error) {
var adminOrgs []GitHubUserOrOrg
for page := 1; page <= maxOrgMembershipPages; page++ {
memberships, err := h.fetchOrgMembershipsPage(ctx, token, page)
if err != nil {
return nil, err
}
for _, m := range memberships {
// state=active is already requested in the query string, but re-check it
// here as defense in depth: if that param is ever dropped or changed, this
// keeps a pending (not-yet-accepted) owner invitation from being granted
// the org namespace.
if m.State == githubMembershipStateActive && m.Role == githubOrgRoleAdmin {
adminOrgs = append(adminOrgs, m.Organization)
}
}
// A short page is the last page: return the complete result.
if len(memberships) < orgMembershipsPageSize {
return adminOrgs, nil
}
}
// Every page up to the cap was full. A real user never has this many org
// memberships, so rather than return a possibly-truncated set (which would
// silently strip a legitimate Owner's grant), fail closed.
return nil, fmt.Errorf("org memberships exceeded %d pages; refusing to issue a possibly-truncated grant", maxOrgMembershipPages)
}
// fetchOrgMembershipsPage fetches a single page of the authenticated user's
// active organization memberships.
func (h *GitHubHandler) fetchOrgMembershipsPage(ctx context.Context, token string, page int) ([]githubOrgMembership, error) {
url := fmt.Sprintf("%s/user/memberships/orgs?state=active&per_page=%d&page=%d", h.baseURL, orgMembershipsPageSize, page)
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
if err != nil {
return nil, fmt.Errorf("failed to create request: %w", err)
}
req.Header.Set("Authorization", "Bearer "+token)
req.Header.Set("Accept", "application/vnd.github.v3+json")
req.Header.Set("X-GitHub-Api-Version", "2022-11-28")
resp, err := http.DefaultClient.Do(req)
if err != nil {
return nil, fmt.Errorf("failed to get user organization memberships: %w", err)
}
defer resp.Body.Close()
// GitHub returns 403 here for two very different reasons, which we must not
// conflate:
// 1. The token lacks the read:org scope. This is the common, benign case for a
// minimal personal-publishing token: GET /user still works, but org
// memberships are forbidden. We degrade gracefully to "no admin orgs" so
// personal-namespace publishing keeps working without over-scoping.
// 2. Rate limiting. GitHub signals primary/secondary rate limits with 403 (or
// 429), setting X-RateLimit-Remaining: 0 and/or Retry-After. Degrading here
// would silently strip a legitimate Owner's org grant, so we fail closed
// (return an error) rather than mistake a throttle for "not an admin".
if resp.StatusCode == http.StatusForbidden {
if resp.Header.Get("X-RateLimit-Remaining") == "0" || resp.Header.Get("Retry-After") != "" {
return nil, fmt.Errorf("GitHub API rate limit exceeded while listing org memberships (status 403): %s", readErrorBody(resp.Body))
}
// A SAML/SSO-enforced org returns 403 with an X-GitHub-SSO header when the
// token has not been SSO-authorized. That is an Owner being blocked, not a
// missing scope, so fail closed rather than silently dropping the org grant
// (which would degrade a legitimate Owner to personal-only with no signal).
if resp.Header.Get("X-GitHub-SSO") != "" {
return nil, fmt.Errorf("GitHub org memberships require SSO authorization for this token (status 403): %s", readErrorBody(resp.Body))
}
return nil, nil
}
if resp.StatusCode != http.StatusOK {
return nil, fmt.Errorf("GitHub API error (status %d): %s", resp.StatusCode, readErrorBody(resp.Body))
}
var memberships []githubOrgMembership
if err := json.NewDecoder(resp.Body).Decode(&memberships); err != nil {
return nil, fmt.Errorf("failed to decode organization memberships response: %w", err)
}
return memberships, nil
}
// buildPermissions builds permissions based on GitHub user and their organizations
func (h *GitHubHandler) buildPermissions(username string, orgs []GitHubUserOrOrg) []auth.Permission {
// Assert the username matches the expected regex, to harden against people doing
// weird things in names. The username is the caller's own identity, so if it is
// invalid we grant nothing.
if !isValidGitHubName(username) {
return nil
}
// Add permission for user's own namespace
permissions := []auth.Permission{{
Action: auth.PermissionActionPublish,
ResourcePattern: fmt.Sprintf("io.github.%s/*", username),
}}
// Add permissions for each organization the user administers (Owner role). Skip
// any org whose name fails validation rather than rejecting the whole set, so one
// unexpected org name can't strip the caller's personal namespace or their other
// (valid) org grants.
for _, org := range orgs {
if !isValidGitHubName(org.Login) {
continue
}
permissions = append(permissions, auth.Permission{
Action: auth.PermissionActionPublish,
ResourcePattern: fmt.Sprintf("io.github.%s/*", org.Login),
})
}
return permissions
}
func isValidGitHubName(name string) bool {
return regexp.MustCompile(`^[a-zA-Z0-9-]+$`).MatchString(name)
}