fix: store tokens in ~/.config/mcp-publisher/ instead of cwd (#1166)
## Summary Fixes #663 - **Security fix**: `mcp-publisher login` no longer writes `.mcpregistry_github_token` and `.mcpregistry_registry_token` to the current working directory. GitHub tokens are kept in memory during the login→exchange flow and cleared immediately after. - **Token location**: Unified token moved from `~/.mcp_publisher_token` to `~/.config/mcp-publisher/token.json` (dir 0700, file 0600), following the convention used by `gh`, `docker`, and `kubectl`. - **Dead code removed**: `NeedsLogin()` was on the `Provider` interface but never called — removed from interface and all 4 implementations. `forceLogin` field also removed. - **Auth tests added**: 11 new tests for `GitHubATProvider` (package previously had 0 tests). - **Test isolation**: `SetupTestToken` now uses `t.TempDir()` + `t.Setenv("HOME", ...)` instead of writing to real `$HOME`. ### Breaking change Users must re-login after upgrading (`mcp-publisher login <method>`). Existing tokens at `~/.mcp_publisher_token` are not auto-migrated. `logout` cleans up both old and new locations. ## Test plan - [x] `make check` passes (lint, unit tests, integration tests) - [x] `mcp-publisher login none` stores token at `~/.config/mcp-publisher/token.json` - [x] No `.mcpregistry_*` files created in cwd after login (with `--token`, `MCP_GITHUB_TOKEN`, or device flow) - [x] `mcp-publisher publish` reads token from new location - [x] `mcp-publisher validate` reads registry URL from new location - [x] `mcp-publisher logout` cleans up new location, legacy `~/.mcp_publisher_token`, and cwd `.mcpregistry_*` files - [x] `mcp-publisher logout` when not logged in shows "Not logged in" - [x] File permissions: dir 0700, file 0600 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
77bbd41fd3
commit
90baa0528f
6 changed files with 556 additions and 203 deletions
|
|
@ -171,11 +171,6 @@ func parseRawPrivateKey(curve elliptic.Curve, privateKeyBytes []byte) (*ecdsa.Pr
|
|||
}, nil
|
||||
}
|
||||
|
||||
// NeedsLogin always returns false for cryptographic auth since no interactive login is needed
|
||||
func (c *CryptoProvider) NeedsLogin() bool {
|
||||
return false
|
||||
}
|
||||
|
||||
// Login is not needed for cryptographic auth since authentication is cryptographic
|
||||
func (c *CryptoProvider) Login(_ context.Context) error {
|
||||
return nil
|
||||
|
|
|
|||
|
|
@ -12,8 +12,6 @@ import (
|
|||
)
|
||||
|
||||
const (
|
||||
gitHubTokenFilePath = ".mcpregistry_github_token" // #nosec:G101
|
||||
registryTokenFilePath = ".mcpregistry_registry_token" // #nosec:G101
|
||||
// GitHub OAuth URLs
|
||||
GitHubDeviceCodeURL = "https://github.com/login/device/code" // #nosec:G101
|
||||
GitHubAccessTokenURL = "https://github.com/login/oauth/access_token" // #nosec:G101
|
||||
|
|
@ -42,18 +40,12 @@ type RegistryTokenResponse struct {
|
|||
ExpiresAt int64 `json:"expires_at"`
|
||||
}
|
||||
|
||||
// StoredRegistryToken represents the registry token with expiration stored locally
|
||||
type StoredRegistryToken struct {
|
||||
Token string `json:"token"`
|
||||
ExpiresAt int64 `json:"expires_at"`
|
||||
}
|
||||
|
||||
// GitHubATProvider implements the Provider interface using GitHub's device flow
|
||||
type GitHubATProvider struct {
|
||||
clientID string
|
||||
forceLogin bool
|
||||
registryURL string
|
||||
providedToken string // Token provided via --token flag or MCP_GITHUB_TOKEN env var
|
||||
githubToken string // In-memory GitHub token set by Login()
|
||||
}
|
||||
|
||||
// ServerHealthResponse represents the response from the health endpoint
|
||||
|
|
@ -63,14 +55,13 @@ type ServerHealthResponse struct {
|
|||
}
|
||||
|
||||
// NewGitHubATProvider creates a new GitHub OAuth provider
|
||||
func NewGitHubATProvider(forceLogin bool, registryURL, token string) Provider {
|
||||
func NewGitHubATProvider(registryURL, token string) Provider {
|
||||
// Check for token from flag or environment variable
|
||||
if token == "" {
|
||||
token = os.Getenv("MCP_GITHUB_TOKEN")
|
||||
}
|
||||
|
||||
return &GitHubATProvider{
|
||||
forceLogin: forceLogin,
|
||||
registryURL: registryURL,
|
||||
providedToken: token,
|
||||
}
|
||||
|
|
@ -78,69 +69,26 @@ func NewGitHubATProvider(forceLogin bool, registryURL, token string) Provider {
|
|||
|
||||
// GetToken retrieves the registry JWT token (exchanges GitHub token if needed)
|
||||
func (g *GitHubATProvider) GetToken(ctx context.Context) (string, error) {
|
||||
// Check if we have a valid registry token
|
||||
registryToken, err := readRegistryToken()
|
||||
if err == nil && registryToken != "" {
|
||||
return registryToken, nil
|
||||
}
|
||||
|
||||
// If no valid registry token, exchange GitHub token for registry token
|
||||
githubToken, err := readToken()
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("failed to read GitHub token: %w", err)
|
||||
if g.githubToken == "" {
|
||||
return "", fmt.Errorf("no GitHub token available; run Login() first")
|
||||
}
|
||||
|
||||
// Exchange GitHub token for registry token
|
||||
registryToken, expiresAt, err := g.exchangeTokenForRegistry(ctx, githubToken)
|
||||
registryToken, _, err := g.exchangeTokenForRegistry(ctx, g.githubToken)
|
||||
// Clear the GitHub token from memory after exchange
|
||||
g.githubToken = ""
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("failed to exchange token: %w", err)
|
||||
}
|
||||
|
||||
// Store the registry token
|
||||
err = saveRegistryToken(registryToken, expiresAt)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("failed to save registry token: %w", err)
|
||||
}
|
||||
|
||||
return registryToken, nil
|
||||
}
|
||||
|
||||
// NeedsLogin checks if a new login is required
|
||||
func (g *GitHubATProvider) NeedsLogin() bool {
|
||||
// If a token was provided via --token or MCP_GITHUB_TOKEN, no login needed
|
||||
if g.providedToken != "" {
|
||||
return false
|
||||
}
|
||||
|
||||
if g.forceLogin {
|
||||
return true
|
||||
}
|
||||
|
||||
// Check if GitHub token exists
|
||||
_, statErr := os.Stat(gitHubTokenFilePath)
|
||||
if os.IsNotExist(statErr) {
|
||||
return true
|
||||
}
|
||||
|
||||
// Check if valid registry token exists
|
||||
_, err := readRegistryToken()
|
||||
if err != nil {
|
||||
// No valid registry token, but we have GitHub token
|
||||
// We don't need to login, just exchange tokens
|
||||
return false
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
// Login performs the GitHub device flow authentication
|
||||
func (g *GitHubATProvider) Login(ctx context.Context) error {
|
||||
// If a token was provided via --token or MCP_GITHUB_TOKEN, save it and skip device flow
|
||||
// If a token was provided via --token or MCP_GITHUB_TOKEN, store it in memory and skip device flow
|
||||
if g.providedToken != "" {
|
||||
err := saveToken(g.providedToken)
|
||||
if err != nil {
|
||||
return fmt.Errorf("error saving provided token: %w", err)
|
||||
}
|
||||
g.githubToken = g.providedToken
|
||||
return nil
|
||||
}
|
||||
|
||||
|
|
@ -173,11 +121,8 @@ func (g *GitHubATProvider) Login(ctx context.Context) error {
|
|||
return fmt.Errorf("error polling for token: %w", err)
|
||||
}
|
||||
|
||||
// Store the token locally
|
||||
err = saveToken(token)
|
||||
if err != nil {
|
||||
return fmt.Errorf("error saving token: %w", err)
|
||||
}
|
||||
// Store the token in memory
|
||||
g.githubToken = token
|
||||
|
||||
_, _ = fmt.Fprintln(os.Stdout, "Successfully authenticated!")
|
||||
return nil
|
||||
|
|
@ -305,20 +250,6 @@ func (g *GitHubATProvider) pollForToken(ctx context.Context, deviceCode string)
|
|||
return "", fmt.Errorf("device code authorization timed out")
|
||||
}
|
||||
|
||||
// saveToken saves the GitHub access token to a local file
|
||||
func saveToken(token string) error {
|
||||
return os.WriteFile(gitHubTokenFilePath, []byte(token), 0600)
|
||||
}
|
||||
|
||||
// readToken reads the GitHub access token from a local file
|
||||
func readToken() (string, error) {
|
||||
tokenData, err := os.ReadFile(gitHubTokenFilePath)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
return string(tokenData), nil
|
||||
}
|
||||
|
||||
func getClientID(ctx context.Context, registryURL string) (string, error) {
|
||||
// This function should retrieve the GitHub Client ID from the registry URL
|
||||
// For now, we will return a placeholder value
|
||||
|
|
@ -407,41 +338,3 @@ func (g *GitHubATProvider) exchangeTokenForRegistry(ctx context.Context, githubT
|
|||
|
||||
return tokenResp.RegistryToken, tokenResp.ExpiresAt, nil
|
||||
}
|
||||
|
||||
// saveRegistryToken saves the registry JWT token to a local file with expiration
|
||||
func saveRegistryToken(token string, expiresAt int64) error {
|
||||
storedToken := StoredRegistryToken{
|
||||
Token: token,
|
||||
ExpiresAt: expiresAt,
|
||||
}
|
||||
|
||||
data, err := json.Marshal(storedToken)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to marshal token: %w", err)
|
||||
}
|
||||
|
||||
return os.WriteFile(registryTokenFilePath, data, 0600)
|
||||
}
|
||||
|
||||
// readRegistryToken reads the registry JWT token from a local file
|
||||
func readRegistryToken() (string, error) {
|
||||
data, err := os.ReadFile(registryTokenFilePath)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
|
||||
var storedToken StoredRegistryToken
|
||||
err = json.Unmarshal(data, &storedToken)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("failed to unmarshal token: %w", err)
|
||||
}
|
||||
|
||||
// Check if token has expired
|
||||
if time.Now().Unix() >= storedToken.ExpiresAt {
|
||||
// Token has expired, remove the file
|
||||
os.Remove(registryTokenFilePath)
|
||||
return "", fmt.Errorf("registry token has expired")
|
||||
}
|
||||
|
||||
return storedToken.Token, nil
|
||||
}
|
||||
|
|
|
|||
|
|
@ -38,12 +38,6 @@ func (o *GitHubOIDCProvider) GetToken(ctx context.Context) (string, error) {
|
|||
return registryToken, nil
|
||||
}
|
||||
|
||||
// NeedsLogin always returns false for OIDC since the token is provided by GitHub Actions
|
||||
func (o *GitHubOIDCProvider) NeedsLogin() bool {
|
||||
// OIDC tokens are provided by GitHub Actions runtime, no interactive login needed
|
||||
return false
|
||||
}
|
||||
|
||||
// Login is not needed for OIDC since tokens are provided by GitHub Actions
|
||||
func (o *GitHubOIDCProvider) Login(_ context.Context) error {
|
||||
// No interactive login needed for OIDC
|
||||
|
|
|
|||
203
cmd/publisher/auth/github_at_test.go
Normal file
203
cmd/publisher/auth/github_at_test.go
Normal file
|
|
@ -0,0 +1,203 @@
|
|||
package auth_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"github.com/modelcontextprotocol/registry/cmd/publisher/auth"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestNewGitHubATProvider_Name(t *testing.T) {
|
||||
p := auth.NewGitHubATProvider("https://registry.example.com", "")
|
||||
assert.Equal(t, "github", p.Name())
|
||||
}
|
||||
|
||||
func TestNewGitHubATProvider_WithEnvToken(t *testing.T) {
|
||||
t.Setenv("MCP_GITHUB_TOKEN", "env-token")
|
||||
|
||||
registry := newMockExchangeServer(t, "env-token")
|
||||
|
||||
p := auth.NewGitHubATProvider(registry.URL, "")
|
||||
|
||||
// Login should use the env var token
|
||||
err := p.Login(context.Background())
|
||||
require.NoError(t, err)
|
||||
|
||||
// GetToken should exchange it successfully
|
||||
token, err := p.GetToken(context.Background())
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "registry-jwt", token)
|
||||
}
|
||||
|
||||
func TestNewGitHubATProvider_ExplicitTokenTakesPrecedence(t *testing.T) {
|
||||
t.Setenv("MCP_GITHUB_TOKEN", "env-token")
|
||||
|
||||
registry := newMockExchangeServer(t, "explicit-token")
|
||||
|
||||
p := auth.NewGitHubATProvider(registry.URL, "explicit-token")
|
||||
|
||||
err := p.Login(context.Background())
|
||||
require.NoError(t, err)
|
||||
|
||||
token, err := p.GetToken(context.Background())
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "registry-jwt", token)
|
||||
}
|
||||
|
||||
func TestGitHubATProvider_LoginWithProvidedToken(t *testing.T) {
|
||||
registry := newMockExchangeServer(t, "my-token")
|
||||
|
||||
p := auth.NewGitHubATProvider(registry.URL, "my-token")
|
||||
|
||||
err := p.Login(context.Background())
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify the token exchange works (proves Login stored the token)
|
||||
token, err := p.GetToken(context.Background())
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "registry-jwt", token)
|
||||
}
|
||||
|
||||
func TestGitHubATProvider_LoginDoesNotWriteFiles(t *testing.T) {
|
||||
cwd, err := os.Getwd()
|
||||
require.NoError(t, err)
|
||||
|
||||
p := auth.NewGitHubATProvider("https://registry.example.com", "my-token")
|
||||
|
||||
err = p.Login(context.Background())
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify no .mcpregistry_* files were created in cwd
|
||||
for _, name := range []string{".mcpregistry_github_token", ".mcpregistry_registry_token"} {
|
||||
_, statErr := os.Stat(filepath.Join(cwd, name))
|
||||
assert.True(t, os.IsNotExist(statErr), "Login should not create %s in cwd", name)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGitHubATProvider_GetTokenWithoutLogin(t *testing.T) {
|
||||
p := auth.NewGitHubATProvider("https://registry.example.com", "my-token")
|
||||
|
||||
// GetToken without Login should fail
|
||||
_, err := p.GetToken(context.Background())
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "no GitHub token available")
|
||||
}
|
||||
|
||||
func TestGitHubATProvider_GetTokenClearsTokenAfterExchange(t *testing.T) {
|
||||
registry := newMockExchangeServer(t, "my-token")
|
||||
|
||||
p := auth.NewGitHubATProvider(registry.URL, "my-token")
|
||||
|
||||
err := p.Login(context.Background())
|
||||
require.NoError(t, err)
|
||||
|
||||
// First GetToken should succeed
|
||||
token, err := p.GetToken(context.Background())
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "registry-jwt", token)
|
||||
|
||||
// Second GetToken should fail (GitHub token was cleared after exchange)
|
||||
_, err = p.GetToken(context.Background())
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "no GitHub token available")
|
||||
}
|
||||
|
||||
func TestGitHubATProvider_GetTokenClearsTokenOnError(t *testing.T) {
|
||||
// Mock registry that always returns an error
|
||||
registry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
http.Error(w, "server error", http.StatusInternalServerError)
|
||||
}))
|
||||
t.Cleanup(registry.Close)
|
||||
|
||||
p := auth.NewGitHubATProvider(registry.URL, "my-token")
|
||||
|
||||
err := p.Login(context.Background())
|
||||
require.NoError(t, err)
|
||||
|
||||
// GetToken should fail
|
||||
_, err = p.GetToken(context.Background())
|
||||
require.Error(t, err)
|
||||
|
||||
// Token should be cleared — second call also fails with "no GitHub token"
|
||||
_, err = p.GetToken(context.Background())
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "no GitHub token available")
|
||||
}
|
||||
|
||||
func TestGitHubATProvider_GetTokenExchangeFailure_NoURL(t *testing.T) {
|
||||
p := auth.NewGitHubATProvider("", "my-token")
|
||||
|
||||
err := p.Login(context.Background())
|
||||
require.NoError(t, err)
|
||||
|
||||
_, err = p.GetToken(context.Background())
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "failed to exchange token")
|
||||
}
|
||||
|
||||
func TestGitHubATProvider_LoginDeviceFlow_FetchesClientID(t *testing.T) {
|
||||
// Mock registry health endpoint
|
||||
registry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path == "/v0/health" {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(map[string]string{
|
||||
"status": "ok",
|
||||
"github_client_id": "test-client-id",
|
||||
})
|
||||
return
|
||||
}
|
||||
http.Error(w, "not found", http.StatusNotFound)
|
||||
}))
|
||||
t.Cleanup(registry.Close)
|
||||
|
||||
p := auth.NewGitHubATProvider(registry.URL, "")
|
||||
|
||||
// Login without a provided token triggers device flow, which calls
|
||||
// the real GitHub device code URL. This will fail, but only after
|
||||
// fetching the client ID from our mock health endpoint.
|
||||
err := p.Login(context.Background())
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "error requesting device code")
|
||||
}
|
||||
|
||||
func TestGitHubATProvider_ImplementsProviderInterface(_ *testing.T) {
|
||||
// Compile-time check
|
||||
var _ = auth.NewGitHubATProvider("https://example.com", "token")
|
||||
}
|
||||
|
||||
// newMockExchangeServer creates an httptest.Server that mocks the registry's
|
||||
// token exchange endpoint. It accepts a specific GitHub token and returns
|
||||
// "registry-jwt" as the registry token.
|
||||
func newMockExchangeServer(t *testing.T, expectedGitHubToken string) *httptest.Server {
|
||||
t.Helper()
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path == "/v0/auth/github-at" && r.Method == http.MethodPost {
|
||||
var body map[string]string
|
||||
if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
|
||||
http.Error(w, "bad request", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
if body["github_token"] == expectedGitHubToken {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
resp := map[string]interface{}{
|
||||
"registry_token": "registry-jwt",
|
||||
"expires_at": 9999999999,
|
||||
}
|
||||
_ = json.NewEncoder(w).Encode(resp)
|
||||
return
|
||||
}
|
||||
http.Error(w, "unauthorized", http.StatusUnauthorized)
|
||||
return
|
||||
}
|
||||
http.Error(w, "not found", http.StatusNotFound)
|
||||
}))
|
||||
t.Cleanup(server.Close)
|
||||
return server
|
||||
}
|
||||
|
|
@ -8,10 +8,6 @@ type Provider interface {
|
|||
// It returns the token string and any error encountered
|
||||
GetToken(ctx context.Context) (string, error)
|
||||
|
||||
// NeedsLogin checks if a new login is required
|
||||
// This can check for existing tokens, expiry, etc.
|
||||
NeedsLogin() bool
|
||||
|
||||
// Login performs the authentication flow
|
||||
// This might involve user interaction, device flows, etc.
|
||||
Login(ctx context.Context) error
|
||||
|
|
|
|||
|
|
@ -62,10 +62,6 @@ func (p *NoneProvider) GetToken(ctx context.Context) (string, error) {
|
|||
return p.token, nil
|
||||
}
|
||||
|
||||
func (p *NoneProvider) NeedsLogin() bool {
|
||||
return false
|
||||
}
|
||||
|
||||
func (p *NoneProvider) Login(_ context.Context) error {
|
||||
// No login needed for anonymous auth
|
||||
return nil
|
||||
|
|
|
|||
Loading…
Reference in a new issue