From 81c5647f2a451212a9fe8fdac0910778506055b2 Mon Sep 17 00:00:00 2001 From: Ossama Lafhel Date: Sun, 14 Sep 2025 11:41:06 +0200 Subject: [PATCH] fix: use authenticated user endpoint for fetching organizations (#398) Changed GitHub API endpoint from /users/{username}/orgs to /user/orgs to retrieve all organizations for the authenticated user. ## Problem The /users/{username}/orgs endpoint only returns organizations where the user's membership is public, causing 403 errors for users with private organization membership when trying to publish. ## Solution Using /user/orgs returns all organizations (public and private) for the authenticated user, allowing them to publish to any organization they belong to. ## Impact - Users can publish to all their organizations without changing visibility - Maintains existing security model (same auth token required) - Resolves 403 permission errors for private org members - Added test to ensure correct endpoint usage --- internal/api/handlers/v0/auth/github_at.go | 4 +- .../handlers/v0/auth/github_at_org_test.go | 114 ++++++++++++++++++ .../api/handlers/v0/auth/github_at_test.go | 6 +- 3 files changed, 119 insertions(+), 5 deletions(-) create mode 100644 internal/api/handlers/v0/auth/github_at_org_test.go diff --git a/internal/api/handlers/v0/auth/github_at.go b/internal/api/handlers/v0/auth/github_at.go index c1d0e644..9c5ddb69 100644 --- a/internal/api/handlers/v0/auth/github_at.go +++ b/internal/api/handlers/v0/auth/github_at.go @@ -134,8 +134,8 @@ func (h *GitHubHandler) getGitHubUser(ctx context.Context, token string) (*GitHu return &user, nil } -func (h *GitHubHandler) getGitHubUserOrgs(ctx context.Context, username string, token string) ([]GitHubUserOrOrg, error) { - req, err := http.NewRequestWithContext(ctx, http.MethodGet, h.baseURL+"/users/"+username+"/orgs", nil) +func (h *GitHubHandler) getGitHubUserOrgs(ctx context.Context, _ string, token string) ([]GitHubUserOrOrg, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, h.baseURL+"/user/orgs", nil) if err != nil { return nil, fmt.Errorf("failed to create request: %w", err) } diff --git a/internal/api/handlers/v0/auth/github_at_org_test.go b/internal/api/handlers/v0/auth/github_at_org_test.go new file mode 100644 index 00000000..e14ae04c --- /dev/null +++ b/internal/api/handlers/v0/auth/github_at_org_test.go @@ -0,0 +1,114 @@ +package auth_test + +import ( + "context" + "crypto/ed25519" + "crypto/rand" + "encoding/hex" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + v0auth "github.com/modelcontextprotocol/registry/internal/api/handlers/v0/auth" + "github.com/modelcontextprotocol/registry/internal/auth" + "github.com/modelcontextprotocol/registry/internal/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + userEndpoint = "/user" + userOrgsEndpoint = "/user/orgs" +) + +func TestGitHubHandler_UsesUserOrgsEndpoint(t *testing.T) { + // This test verifies that we use /user/orgs instead of /users/{username}/orgs + // to ensure we get ALL organizations (including private ones) + + testSeed := make([]byte, ed25519.SeedSize) + _, err := rand.Read(testSeed) + require.NoError(t, err) + + cfg := &config.Config{ + JWTPrivateKey: hex.EncodeToString(testSeed), + } + + // Track which endpoints were called + var calledEndpoints []string + + // Create mock GitHub API server + mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calledEndpoints = append(calledEndpoints, r.URL.Path) + + switch r.URL.Path { + case userEndpoint: + user := v0auth.GitHubUserOrOrg{ + Login: "testuser", + ID: 12345, + } + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(user); err != nil { + t.Logf("Failed to encode user response: %v", err) + } + case userOrgsEndpoint: + // NEW endpoint returns ALL orgs (public + private) + orgs := []v0auth.GitHubUserOrOrg{ + {Login: "public-org", ID: 1}, + {Login: "private-org", ID: 2}, // This would NOT be returned by /users/{username}/orgs + } + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(orgs); err != nil { + t.Logf("Failed to encode orgs response: %v", err) + } + case "/users/testuser/orgs": + // OLD endpoint would only return public orgs + t.Error("Should not call /users/{username}/orgs endpoint") + orgs := []v0auth.GitHubUserOrOrg{ + {Login: "public-org", ID: 1}, + // private-org would NOT be included here + } + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(orgs); err != nil { + t.Logf("Failed to encode orgs response: %v", err) + } + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer mockServer.Close() + + // Create handler and set mock server URL + handler := v0auth.NewGitHubHandler(cfg) + handler.SetBaseURL(mockServer.URL) + + // Test token exchange + ctx := context.Background() + response, err := handler.ExchangeToken(ctx, "test-token") + require.NoError(t, err) + require.NotNil(t, response) + + // Verify the correct endpoints were called + assert.Contains(t, calledEndpoints, userEndpoint, "Should call /user endpoint") + assert.Contains(t, calledEndpoints, userOrgsEndpoint, "Should call /user/orgs endpoint") + assert.NotContains(t, calledEndpoints, "/users/testuser/orgs", "Should NOT call /users/{username}/orgs") + + // Validate the JWT token includes permissions for BOTH orgs + jwtManager := auth.NewJWTManager(cfg) + claims, err := jwtManager.ValidateToken(ctx, response.RegistryToken) + require.NoError(t, err) + + // Should have 3 permissions: user + 2 orgs (including private) + assert.Len(t, claims.Permissions, 3, "Should have permissions for user and both orgs") + + expectedPatterns := []string{ + "io.github.testuser/*", + "io.github.public-org/*", + "io.github.private-org/*", // This is the key - private org is included! + } + + for i, perm := range claims.Permissions { + assert.Equal(t, auth.PermissionActionPublish, perm.Action) + assert.Equal(t, expectedPatterns[i], perm.ResourcePattern) + } +} diff --git a/internal/api/handlers/v0/auth/github_at_test.go b/internal/api/handlers/v0/auth/github_at_test.go index c0fa4ac9..ba971f6d 100644 --- a/internal/api/handlers/v0/auth/github_at_test.go +++ b/internal/api/handlers/v0/auth/github_at_test.go @@ -22,7 +22,7 @@ import ( const ( githubUserEndpoint = "/user" - githubOrgsEndpoint = "/users/testuser/orgs" + githubOrgsEndpoint = "/user/orgs" ) func TestGitHubHandler_ExchangeToken(t *testing.T) { @@ -225,7 +225,7 @@ 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": + case githubOrgsEndpoint: orgs := []v0auth.GitHubUserOrOrg{} w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(orgs) //nolint:errcheck @@ -537,7 +537,7 @@ 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 }