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>
This commit is contained in:
parent
224e23dc2c
commit
2bc37b1999
1 changed files with 573 additions and 37 deletions
|
|
@ -22,9 +22,82 @@ import (
|
|||
|
||||
const (
|
||||
githubUserEndpoint = "/user"
|
||||
githubOrgsEndpoint = "/users/testuser/orgs"
|
||||
// githubOrgsEndpoint is the endpoint used to list the authenticated user's org
|
||||
// memberships (with role). It replaces the old public-only /users/{user}/orgs.
|
||||
githubOrgsEndpoint = "/user/memberships/orgs"
|
||||
)
|
||||
|
||||
// orgMembership mirrors a single entry of GET /user/memberships/orgs for use in
|
||||
// mock GitHub API responses. Role is "admin" (Owner) or "member".
|
||||
type orgMembership struct {
|
||||
State string `json:"state"`
|
||||
Role string `json:"role"`
|
||||
Organization v0auth.GitHubUserOrOrg `json:"organization"`
|
||||
}
|
||||
|
||||
// adminMemberships wraps orgs as active "admin"-role memberships, the shape the
|
||||
// memberships endpoint returns for an org Owner.
|
||||
func adminMemberships(orgs []v0auth.GitHubUserOrOrg) []orgMembership {
|
||||
memberships := make([]orgMembership, 0, len(orgs))
|
||||
for _, org := range orgs {
|
||||
memberships = append(memberships, orgMembership{State: "active", Role: "admin", Organization: org})
|
||||
}
|
||||
return memberships
|
||||
}
|
||||
|
||||
// newMockGitHubServer returns a mock GitHub API server that serves a fixed
|
||||
// "testuser" on /user and delegates the org-memberships endpoint to orgsHandler.
|
||||
// It keeps individual test cases small (only the memberships behavior varies),
|
||||
// which also keeps their cyclomatic complexity down.
|
||||
func newMockGitHubServer(orgsHandler http.HandlerFunc) *httptest.Server {
|
||||
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
switch r.URL.Path {
|
||||
case githubUserEndpoint:
|
||||
user := v0auth.GitHubUserOrOrg{Login: "testuser", ID: 12345}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(user) //nolint:errcheck
|
||||
case githubOrgsEndpoint:
|
||||
orgsHandler(w, r)
|
||||
default:
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}
|
||||
}))
|
||||
}
|
||||
|
||||
// grantedPatterns exchanges the token and returns the resource patterns granted
|
||||
// in the resulting JWT.
|
||||
func grantedPatterns(t *testing.T, cfg *config.Config, server *httptest.Server) []string {
|
||||
t.Helper()
|
||||
handler := v0auth.NewGitHubHandler(cfg)
|
||||
handler.SetBaseURL(server.URL)
|
||||
|
||||
ctx := context.Background()
|
||||
response, err := handler.ExchangeToken(ctx, "valid-github-token")
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, response)
|
||||
|
||||
claims, err := auth.NewJWTManager(cfg).ValidateToken(ctx, response.RegistryToken)
|
||||
require.NoError(t, err)
|
||||
|
||||
patterns := make([]string, 0, len(claims.Permissions))
|
||||
for _, perm := range claims.Permissions {
|
||||
patterns = append(patterns, perm.ResourcePattern)
|
||||
}
|
||||
return patterns
|
||||
}
|
||||
|
||||
// assertExchangeFailsClosed asserts that a token exchange against server returns
|
||||
// an error and no token (used for the fail-closed 403 variants).
|
||||
func assertExchangeFailsClosed(t *testing.T, cfg *config.Config, server *httptest.Server) {
|
||||
t.Helper()
|
||||
handler := v0auth.NewGitHubHandler(cfg)
|
||||
handler.SetBaseURL(server.URL)
|
||||
|
||||
response, err := handler.ExchangeToken(context.Background(), "valid-github-token")
|
||||
require.Error(t, err)
|
||||
assert.Nil(t, response)
|
||||
}
|
||||
|
||||
func TestGitHubHandler_ExchangeToken(t *testing.T) {
|
||||
// Create test handler with mock config
|
||||
testSeed := make([]byte, ed25519.SeedSize)
|
||||
|
|
@ -96,12 +169,12 @@ func TestGitHubHandler_ExchangeToken(t *testing.T) {
|
|||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(user) //nolint:errcheck
|
||||
case githubOrgsEndpoint:
|
||||
orgs := []v0auth.GitHubUserOrOrg{
|
||||
memberships := adminMemberships([]v0auth.GitHubUserOrOrg{
|
||||
{Login: "test-org-1", ID: 1},
|
||||
{Login: "test-org-2", ID: 2},
|
||||
}
|
||||
})
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(orgs) //nolint:errcheck
|
||||
json.NewEncoder(w).Encode(memberships) //nolint:errcheck
|
||||
default:
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}
|
||||
|
|
@ -225,10 +298,9 @@ func TestGitHubHandler_ExchangeToken(t *testing.T) {
|
|||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(user) //nolint:errcheck
|
||||
case "/users/user with spaces/orgs":
|
||||
orgs := []v0auth.GitHubUserOrOrg{}
|
||||
case githubOrgsEndpoint:
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(orgs) //nolint:errcheck
|
||||
json.NewEncoder(w).Encode([]orgMembership{}) //nolint:errcheck
|
||||
}
|
||||
}))
|
||||
defer mockServer.Close()
|
||||
|
|
@ -264,12 +336,12 @@ func TestGitHubHandler_ExchangeToken(t *testing.T) {
|
|||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(user) //nolint:errcheck
|
||||
case githubOrgsEndpoint:
|
||||
orgs := []v0auth.GitHubUserOrOrg{
|
||||
memberships := adminMemberships([]v0auth.GitHubUserOrOrg{
|
||||
{Login: "valid-org", ID: 1},
|
||||
{Login: "org with spaces", ID: 2}, // Invalid name
|
||||
}
|
||||
})
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(orgs) //nolint:errcheck
|
||||
json.NewEncoder(w).Encode(memberships) //nolint:errcheck
|
||||
}
|
||||
}))
|
||||
defer mockServer.Close()
|
||||
|
|
@ -290,7 +362,15 @@ func TestGitHubHandler_ExchangeToken(t *testing.T) {
|
|||
claims, err := jwtManager.ValidateToken(ctx, response.RegistryToken)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "testuser", claims.AuthMethodSubject)
|
||||
assert.Empty(t, claims.Permissions) // No permissions because one org has invalid name
|
||||
|
||||
// The invalid org is skipped, but the personal namespace and the valid org
|
||||
// are still granted — one weird org name must not strip everything else.
|
||||
patterns := make([]string, 0, len(claims.Permissions))
|
||||
for _, perm := range claims.Permissions {
|
||||
patterns = append(patterns, perm.ResourcePattern)
|
||||
}
|
||||
assert.ElementsMatch(t, []string{"io.github.testuser/*", "io.github.valid-org/*"}, patterns)
|
||||
assert.NotContains(t, patterns, "io.github.org with spaces/*")
|
||||
})
|
||||
|
||||
t.Run("malformed JSON response", func(t *testing.T) {
|
||||
|
|
@ -317,6 +397,274 @@ func TestGitHubHandler_ExchangeToken(t *testing.T) {
|
|||
})
|
||||
}
|
||||
|
||||
func TestGitHubHandler_ExchangeToken_OrgRoles(t *testing.T) {
|
||||
testSeed := make([]byte, ed25519.SeedSize)
|
||||
_, err := rand.Read(testSeed)
|
||||
require.NoError(t, err)
|
||||
|
||||
cfg := &config.Config{
|
||||
JWTPrivateKey: hex.EncodeToString(testSeed),
|
||||
}
|
||||
|
||||
t.Run("member-role org is not granted, only admin orgs", func(t *testing.T) {
|
||||
// The user is an Owner (admin) of one org and an ordinary member of another.
|
||||
// Only the org they administer should yield a publish permission.
|
||||
mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
switch r.URL.Path {
|
||||
case githubUserEndpoint:
|
||||
user := v0auth.GitHubUserOrOrg{Login: "testuser", ID: 12345}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(user) //nolint:errcheck
|
||||
case githubOrgsEndpoint:
|
||||
memberships := []orgMembership{
|
||||
{State: "active", Role: "admin", Organization: v0auth.GitHubUserOrOrg{Login: "admin-org", ID: 1}},
|
||||
{State: "active", Role: "member", Organization: v0auth.GitHubUserOrOrg{Login: "member-org", ID: 2}},
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(memberships) //nolint:errcheck
|
||||
default:
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}
|
||||
}))
|
||||
defer mockServer.Close()
|
||||
|
||||
handler := v0auth.NewGitHubHandler(cfg)
|
||||
handler.SetBaseURL(mockServer.URL)
|
||||
|
||||
ctx := context.Background()
|
||||
response, err := handler.ExchangeToken(ctx, "valid-github-token")
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, response)
|
||||
|
||||
jwtManager := auth.NewJWTManager(cfg)
|
||||
claims, err := jwtManager.ValidateToken(ctx, response.RegistryToken)
|
||||
require.NoError(t, err)
|
||||
|
||||
patterns := make([]string, 0, len(claims.Permissions))
|
||||
for _, perm := range claims.Permissions {
|
||||
patterns = append(patterns, perm.ResourcePattern)
|
||||
}
|
||||
assert.ElementsMatch(t, []string{"io.github.testuser/*", "io.github.admin-org/*"}, patterns)
|
||||
assert.NotContains(t, patterns, "io.github.member-org/*")
|
||||
})
|
||||
|
||||
t.Run("missing read:org scope (403) still grants personal namespace", func(t *testing.T) {
|
||||
// A minimal token without read:org authenticates fine but is forbidden from
|
||||
// reading org memberships. Personal-namespace publishing must still work.
|
||||
mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
switch r.URL.Path {
|
||||
case githubUserEndpoint:
|
||||
user := v0auth.GitHubUserOrOrg{Login: "testuser", ID: 12345}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(user) //nolint:errcheck
|
||||
case githubOrgsEndpoint:
|
||||
w.WriteHeader(http.StatusForbidden)
|
||||
w.Write([]byte(`{"message": "Token does not have the required scope"}`)) //nolint:errcheck
|
||||
default:
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}
|
||||
}))
|
||||
defer mockServer.Close()
|
||||
|
||||
handler := v0auth.NewGitHubHandler(cfg)
|
||||
handler.SetBaseURL(mockServer.URL)
|
||||
|
||||
ctx := context.Background()
|
||||
response, err := handler.ExchangeToken(ctx, "valid-github-token")
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, response)
|
||||
|
||||
jwtManager := auth.NewJWTManager(cfg)
|
||||
claims, err := jwtManager.ValidateToken(ctx, response.RegistryToken)
|
||||
require.NoError(t, err)
|
||||
assert.Len(t, claims.Permissions, 1)
|
||||
assert.Equal(t, "io.github.testuser/*", claims.Permissions[0].ResourcePattern)
|
||||
})
|
||||
|
||||
t.Run("admin org on a later page is still granted (pagination)", func(t *testing.T) {
|
||||
// Page 1 is a full page (100) of member-role orgs; the only admin org is on
|
||||
// page 2. The pagination loop must not stop at the first page, or the Owner's
|
||||
// org grant would be silently dropped.
|
||||
mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
switch r.URL.Path {
|
||||
case githubUserEndpoint:
|
||||
user := v0auth.GitHubUserOrOrg{Login: "testuser", ID: 12345}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(user) //nolint:errcheck
|
||||
case githubOrgsEndpoint:
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
if r.URL.Query().Get("page") == "1" {
|
||||
page1 := make([]orgMembership, 0, 100)
|
||||
for i := 0; i < 100; i++ {
|
||||
page1 = append(page1, orgMembership{
|
||||
State: "active", Role: "member",
|
||||
Organization: v0auth.GitHubUserOrOrg{Login: fmt.Sprintf("member-org-%d", i), ID: 1000 + i},
|
||||
})
|
||||
}
|
||||
json.NewEncoder(w).Encode(page1) //nolint:errcheck
|
||||
return
|
||||
}
|
||||
// page 2: a single admin org, signalling the last (short) page.
|
||||
page2 := []orgMembership{
|
||||
{State: "active", Role: "admin", Organization: v0auth.GitHubUserOrOrg{Login: "late-admin-org", ID: 2}},
|
||||
}
|
||||
json.NewEncoder(w).Encode(page2) //nolint:errcheck
|
||||
default:
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}
|
||||
}))
|
||||
defer mockServer.Close()
|
||||
|
||||
handler := v0auth.NewGitHubHandler(cfg)
|
||||
handler.SetBaseURL(mockServer.URL)
|
||||
|
||||
ctx := context.Background()
|
||||
response, err := handler.ExchangeToken(ctx, "valid-github-token")
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, response)
|
||||
|
||||
jwtManager := auth.NewJWTManager(cfg)
|
||||
claims, err := jwtManager.ValidateToken(ctx, response.RegistryToken)
|
||||
require.NoError(t, err)
|
||||
|
||||
patterns := make([]string, 0, len(claims.Permissions))
|
||||
for _, perm := range claims.Permissions {
|
||||
patterns = append(patterns, perm.ResourcePattern)
|
||||
}
|
||||
assert.ElementsMatch(t, []string{"io.github.testuser/*", "io.github.late-admin-org/*"}, patterns)
|
||||
})
|
||||
|
||||
t.Run("memberships server error fails closed (no token issued)", func(t *testing.T) {
|
||||
// A 5xx (or any non-200, non-scope-403) from the memberships endpoint must
|
||||
// abort the exchange rather than silently degrade to personal-only perms.
|
||||
mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
switch r.URL.Path {
|
||||
case githubUserEndpoint:
|
||||
user := v0auth.GitHubUserOrOrg{Login: "testuser", ID: 12345}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(user) //nolint:errcheck
|
||||
case githubOrgsEndpoint:
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
default:
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}
|
||||
}))
|
||||
defer mockServer.Close()
|
||||
|
||||
handler := v0auth.NewGitHubHandler(cfg)
|
||||
handler.SetBaseURL(mockServer.URL)
|
||||
|
||||
ctx := context.Background()
|
||||
response, err := handler.ExchangeToken(ctx, "valid-github-token")
|
||||
require.Error(t, err)
|
||||
assert.Nil(t, response)
|
||||
})
|
||||
|
||||
t.Run("rate-limit 403 fails closed (not treated as missing scope)", func(t *testing.T) {
|
||||
// A 403 with X-RateLimit-Remaining: 0 is a throttle, not a missing-scope
|
||||
// signal. Degrading would strip a real Owner's org grant, so it must error.
|
||||
mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
switch r.URL.Path {
|
||||
case githubUserEndpoint:
|
||||
user := v0auth.GitHubUserOrOrg{Login: "testuser", ID: 12345}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(user) //nolint:errcheck
|
||||
case githubOrgsEndpoint:
|
||||
w.Header().Set("X-RateLimit-Remaining", "0")
|
||||
w.WriteHeader(http.StatusForbidden)
|
||||
w.Write([]byte(`{"message": "API rate limit exceeded"}`)) //nolint:errcheck
|
||||
default:
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}
|
||||
}))
|
||||
defer mockServer.Close()
|
||||
|
||||
handler := v0auth.NewGitHubHandler(cfg)
|
||||
handler.SetBaseURL(mockServer.URL)
|
||||
|
||||
ctx := context.Background()
|
||||
response, err := handler.ExchangeToken(ctx, "valid-github-token")
|
||||
require.Error(t, err)
|
||||
assert.Nil(t, response)
|
||||
})
|
||||
}
|
||||
|
||||
func TestGitHubHandler_ExchangeToken_MembershipFiltering(t *testing.T) {
|
||||
testSeed := make([]byte, ed25519.SeedSize)
|
||||
_, err := rand.Read(testSeed)
|
||||
require.NoError(t, err)
|
||||
|
||||
cfg := &config.Config{JWTPrivateKey: hex.EncodeToString(testSeed)}
|
||||
|
||||
t.Run("pending admin membership is not granted (state filter)", func(t *testing.T) {
|
||||
// A user invited as an Owner but who has not accepted has role "admin" with
|
||||
// state "pending". They are not an Owner yet, so the org must not be granted
|
||||
// even if the state=active query filter is somehow bypassed.
|
||||
server := newMockGitHubServer(func(w http.ResponseWriter, _ *http.Request) {
|
||||
memberships := []orgMembership{
|
||||
{State: "pending", Role: "admin", Organization: v0auth.GitHubUserOrOrg{Login: "pending-org", ID: 1}},
|
||||
{State: "active", Role: "admin", Organization: v0auth.GitHubUserOrOrg{Login: "active-org", ID: 2}},
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(memberships) //nolint:errcheck
|
||||
})
|
||||
defer server.Close()
|
||||
|
||||
patterns := grantedPatterns(t, cfg, server)
|
||||
assert.ElementsMatch(t, []string{"io.github.testuser/*", "io.github.active-org/*"}, patterns)
|
||||
assert.NotContains(t, patterns, "io.github.pending-org/*")
|
||||
})
|
||||
|
||||
t.Run("billing_manager role is not granted", func(t *testing.T) {
|
||||
server := newMockGitHubServer(func(w http.ResponseWriter, _ *http.Request) {
|
||||
memberships := []orgMembership{
|
||||
{State: "active", Role: "billing_manager", Organization: v0auth.GitHubUserOrOrg{Login: "billing-org", ID: 1}},
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(memberships) //nolint:errcheck
|
||||
})
|
||||
defer server.Close()
|
||||
|
||||
patterns := grantedPatterns(t, cfg, server)
|
||||
assert.Equal(t, []string{"io.github.testuser/*"}, patterns)
|
||||
})
|
||||
}
|
||||
|
||||
func TestGitHubHandler_ExchangeToken_403FailClosed(t *testing.T) {
|
||||
testSeed := make([]byte, ed25519.SeedSize)
|
||||
_, err := rand.Read(testSeed)
|
||||
require.NoError(t, err)
|
||||
|
||||
cfg := &config.Config{JWTPrivateKey: hex.EncodeToString(testSeed)}
|
||||
|
||||
t.Run("Retry-After 403 fails closed (secondary rate limit)", func(t *testing.T) {
|
||||
// A 403 carrying only Retry-After (no X-RateLimit-Remaining: 0) is a secondary
|
||||
// rate limit, not a missing scope. Degrading would strip a real Owner's grant.
|
||||
server := newMockGitHubServer(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.Header().Set("Retry-After", "60")
|
||||
w.WriteHeader(http.StatusForbidden)
|
||||
w.Write([]byte(`{"message": "You have exceeded a secondary rate limit"}`)) //nolint:errcheck
|
||||
})
|
||||
defer server.Close()
|
||||
|
||||
assertExchangeFailsClosed(t, cfg, server)
|
||||
})
|
||||
|
||||
t.Run("SSO-enforced 403 fails closed (not treated as missing scope)", func(t *testing.T) {
|
||||
// A SAML/SSO-enforced org returns 403 with X-GitHub-SSO when the token is not
|
||||
// SSO-authorized. That is an Owner being blocked, so it must fail closed rather
|
||||
// than silently degrade to personal-only.
|
||||
server := newMockGitHubServer(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.Header().Set("X-GitHub-SSO", "required; two_factor_authentication=false")
|
||||
w.WriteHeader(http.StatusForbidden)
|
||||
w.Write([]byte(`{"message": "Resource protected by organization SAML enforcement."}`)) //nolint:errcheck
|
||||
})
|
||||
defer server.Close()
|
||||
|
||||
assertExchangeFailsClosed(t, cfg, server)
|
||||
})
|
||||
}
|
||||
|
||||
func TestJWTTokenValidation(t *testing.T) {
|
||||
testSeed := make([]byte, ed25519.SeedSize)
|
||||
_, err := rand.Read(testSeed)
|
||||
|
|
@ -521,7 +869,16 @@ func TestValidGitHubNames(t *testing.T) {
|
|||
orgs: []v0auth.GitHubUserOrOrg{
|
||||
{Login: "invalid org", ID: 1},
|
||||
},
|
||||
wantPerms: 0, // Should return nil if any name is invalid
|
||||
wantPerms: 1, // Personal namespace kept; the invalid org is skipped
|
||||
},
|
||||
{
|
||||
name: "valid username with one valid and one invalid org",
|
||||
username: "valid-user",
|
||||
orgs: []v0auth.GitHubUserOrOrg{
|
||||
{Login: "valid-org", ID: 1},
|
||||
{Login: "invalid org", ID: 2},
|
||||
},
|
||||
wantPerms: 2, // Personal + valid org; the invalid org is skipped
|
||||
},
|
||||
}
|
||||
|
||||
|
|
@ -537,9 +894,9 @@ func TestValidGitHubNames(t *testing.T) {
|
|||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(user) //nolint:errcheck
|
||||
case "/users/" + tc.username + "/orgs":
|
||||
case githubOrgsEndpoint:
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(tc.orgs) //nolint:errcheck
|
||||
json.NewEncoder(w).Encode(adminMemberships(tc.orgs)) //nolint:errcheck
|
||||
}
|
||||
}))
|
||||
defer mockServer.Close()
|
||||
|
|
|
|||
Loading…
Reference in a new issue