diff --git a/cmd/publisher/auth/common.go b/cmd/publisher/auth/common.go index 28829e8..053b543 100644 --- a/cmd/publisher/auth/common.go +++ b/cmd/publisher/auth/common.go @@ -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 diff --git a/cmd/publisher/auth/github-at.go b/cmd/publisher/auth/github-at.go index 602e116..3e53529 100644 --- a/cmd/publisher/auth/github-at.go +++ b/cmd/publisher/auth/github-at.go @@ -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 -} diff --git a/cmd/publisher/auth/github-oidc.go b/cmd/publisher/auth/github-oidc.go index 55fa350..e4b51ed 100644 --- a/cmd/publisher/auth/github-oidc.go +++ b/cmd/publisher/auth/github-oidc.go @@ -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 diff --git a/cmd/publisher/auth/github_at_test.go b/cmd/publisher/auth/github_at_test.go new file mode 100644 index 0000000..06add95 --- /dev/null +++ b/cmd/publisher/auth/github_at_test.go @@ -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 +} diff --git a/cmd/publisher/auth/interface.go b/cmd/publisher/auth/interface.go index a230429..1e3561f 100644 --- a/cmd/publisher/auth/interface.go +++ b/cmd/publisher/auth/interface.go @@ -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 diff --git a/cmd/publisher/auth/none.go b/cmd/publisher/auth/none.go index a822cba..967db24 100644 --- a/cmd/publisher/auth/none.go +++ b/cmd/publisher/auth/none.go @@ -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 diff --git a/cmd/publisher/commands/login.go b/cmd/publisher/commands/login.go index 091a700..3b97211 100644 --- a/cmd/publisher/commands/login.go +++ b/cmd/publisher/commands/login.go @@ -16,15 +16,45 @@ import ( ) const ( - DefaultRegistryURL = "https://registry.modelcontextprotocol.io" - TokenFileName = ".mcp_publisher_token" //nolint:gosec // Not a credential, just a filename - MethodGitHub = "github" - MethodGitHubOIDC = "github-oidc" - MethodDNS = "dns" - MethodHTTP = "http" - MethodNone = "none" + DefaultRegistryURL = "https://registry.modelcontextprotocol.io" + LegacyTokenFileName = ".mcp_publisher_token" //nolint:gosec // Not a credential, just a filename + MethodGitHub = "github" + MethodGitHubOIDC = "github-oidc" + MethodDNS = "dns" + MethodHTTP = "http" + MethodNone = "none" ) +// tokenFilePath returns the path to the token file in the user's config directory +// (~/.config/mcp-publisher/token.json). It does not create the directory. +func tokenFilePath() (string, error) { + homeDir, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("failed to get home directory: %w", err) + } + return filepath.Join(homeDir, ".config", "mcp-publisher", "token.json"), nil +} + +// notAuthenticatedError returns an error guiding the user to log in, +// with a hint about the token location change for users upgrading. +func notAuthenticatedError() error { + _, _ = fmt.Fprintln(os.Stderr, "hint: token storage moved to ~/.config/mcp-publisher/. If you recently upgraded, please re-login.") + return errors.New("not authenticated, run 'mcp-publisher login ' first") +} + +// ensureTokenDir creates the token directory (~/.config/mcp-publisher/) if needed. +func ensureTokenDir() error { + homeDir, err := os.UserHomeDir() + if err != nil { + return fmt.Errorf("failed to get home directory: %w", err) + } + dir := filepath.Join(homeDir, ".config", "mcp-publisher") + if err := os.MkdirAll(dir, 0700); err != nil { + return fmt.Errorf("failed to create config directory: %w", err) + } + return nil +} + type CryptoAlgorithm auth.CryptoAlgorithm type SignerType string @@ -130,7 +160,7 @@ func createSigner(flags LoginFlags) (auth.Signer, error) { func createAuthProvider(method, registryURL, domain string, token Token, signer auth.Signer) (auth.Provider, error) { switch method { case MethodGitHub: - return auth.NewGitHubATProvider(true, registryURL, string(token)), nil + return auth.NewGitHubATProvider(registryURL, string(token)), nil case MethodGitHubOIDC: return auth.NewGitHubOIDCProvider(registryURL), nil case MethodDNS: @@ -226,12 +256,15 @@ Examples: return fmt.Errorf("failed to get token: %w", err) } - homeDir, err := os.UserHomeDir() + if err := ensureTokenDir(); err != nil { + return err + } + + tokenPath, err := tokenFilePath() if err != nil { - return fmt.Errorf("failed to get home directory: %w", err) + return err } - tokenPath := filepath.Join(homeDir, TokenFileName) tokenData := map[string]string{ "token": token, "method": method, diff --git a/cmd/publisher/commands/login_test.go b/cmd/publisher/commands/login_test.go new file mode 100644 index 0000000..1f6a1b1 --- /dev/null +++ b/cmd/publisher/commands/login_test.go @@ -0,0 +1,97 @@ +package commands_test + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "github.com/modelcontextprotocol/registry/cmd/publisher/commands" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLoginCommand_WritesTokenToConfigDir(t *testing.T) { + tempHome := t.TempDir() + t.Setenv("HOME", tempHome) + + // Mock registry that returns a token for "none" auth + server := setupNoneAuthServer(t) + + err := commands.LoginCommand([]string{"none", "--registry", server.URL}) + require.NoError(t, err) + + // Verify token written to new location + tokenPath := filepath.Join(tempHome, ".config", "mcp-publisher", "token.json") + data, err := os.ReadFile(tokenPath) + require.NoError(t, err) + + var tokenInfo map[string]string + require.NoError(t, json.Unmarshal(data, &tokenInfo)) + + assert.Equal(t, "none", tokenInfo["method"]) + assert.Equal(t, server.URL, tokenInfo["registry"]) + assert.NotEmpty(t, tokenInfo["token"]) +} + +func TestLoginCommand_CreatesDirectoryWithCorrectPerms(t *testing.T) { + tempHome := t.TempDir() + t.Setenv("HOME", tempHome) + + server := setupNoneAuthServer(t) + + err := commands.LoginCommand([]string{"none", "--registry", server.URL}) + require.NoError(t, err) + + // Verify directory permissions + dir := filepath.Join(tempHome, ".config", "mcp-publisher") + info, err := os.Stat(dir) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0700), info.Mode().Perm()) + + // Verify file permissions + tokenPath := filepath.Join(dir, "token.json") + info, err = os.Stat(tokenPath) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0600), info.Mode().Perm()) +} + +func TestLoginCommand_DoesNotWriteLegacyFiles(t *testing.T) { + tempHome := t.TempDir() + t.Setenv("HOME", tempHome) + + server := setupNoneAuthServer(t) + + err := commands.LoginCommand([]string{"none", "--registry", server.URL}) + require.NoError(t, err) + + // Verify no legacy file at $HOME + _, err = os.Stat(filepath.Join(tempHome, ".mcp_publisher_token")) + assert.True(t, os.IsNotExist(err), "should not create legacy ~/.mcp_publisher_token") + + // Verify no .mcpregistry_* files in cwd + cwd, err := os.Getwd() + require.NoError(t, err) + for _, name := range []string{".mcpregistry_github_token", ".mcpregistry_registry_token"} { + _, err := os.Stat(filepath.Join(cwd, name)) + assert.True(t, os.IsNotExist(err), "should not create %s in cwd", name) + } +} + +// setupNoneAuthServer creates a mock registry that handles the "none" auth flow. +func setupNoneAuthServer(t *testing.T) *httptest.Server { + t.Helper() + mux := http.NewServeMux() + mux.HandleFunc("/v0/auth/none", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]interface{}{ + "registry_token": "test-registry-jwt", + "expires_at": 9999999999, + }) + }) + server := httptest.NewServer(mux) + t.Cleanup(server.Close) + return server +} diff --git a/cmd/publisher/commands/logout.go b/cmd/publisher/commands/logout.go index 3ac9abd..25adfd8 100644 --- a/cmd/publisher/commands/logout.go +++ b/cmd/publisher/commands/logout.go @@ -7,37 +7,57 @@ import ( ) func LogoutCommand() error { - homeDir, err := os.UserHomeDir() + tokenPath, err := tokenFilePath() if err != nil { - return fmt.Errorf("failed to get home directory: %w", err) + return err } - tokenPath := filepath.Join(homeDir, TokenFileName) + // Check if token file exists at new location or legacy location + homeDir, err := os.UserHomeDir() + if err != nil { + homeDir = "" // degrade gracefully for legacy cleanup + } + legacyTokenPath := "" + if homeDir != "" { + legacyTokenPath = filepath.Join(homeDir, LegacyTokenFileName) + } - // Check if token file exists - if _, err := os.Stat(tokenPath); os.IsNotExist(err) { + newExists := fileExists(tokenPath) + legacyExists := legacyTokenPath != "" && fileExists(legacyTokenPath) + + if !newExists && !legacyExists { _, _ = fmt.Fprintln(os.Stdout, "Not logged in") return nil } - // Remove token file - if err := os.Remove(tokenPath); err != nil { - return fmt.Errorf("failed to remove token: %w", err) + // Remove token file from new location + os.Remove(tokenPath) + + // Remove from legacy location + if legacyTokenPath != "" { + os.Remove(legacyTokenPath) } - // Also clean up legacy token files if they exist - legacyFiles := []string{ + // Clean up legacy intermediate token files from $HOME and cwd + legacyIntermediateFiles := []string{ ".mcpregistry_github_token", ".mcpregistry_registry_token", } - for _, file := range legacyFiles { - path := filepath.Join(homeDir, file) - if _, err := os.Stat(path); err == nil { - os.Remove(path) // Ignore errors for legacy files + for _, file := range legacyIntermediateFiles { + // Clean from $HOME + if homeDir != "" { + os.Remove(filepath.Join(homeDir, file)) } + // Clean from cwd (the original bug location) + os.Remove(file) } _, _ = fmt.Fprintln(os.Stdout, "✓ Successfully logged out") return nil } + +func fileExists(path string) bool { + _, err := os.Stat(path) + return err == nil +} diff --git a/cmd/publisher/commands/logout_test.go b/cmd/publisher/commands/logout_test.go new file mode 100644 index 0000000..ce0439f --- /dev/null +++ b/cmd/publisher/commands/logout_test.go @@ -0,0 +1,123 @@ +package commands_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/modelcontextprotocol/registry/cmd/publisher/commands" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLogoutCommand_RemovesNewToken(t *testing.T) { + tempHome := t.TempDir() + t.Setenv("HOME", tempHome) + + // Create token at new location + dir := filepath.Join(tempHome, ".config", "mcp-publisher") + require.NoError(t, os.MkdirAll(dir, 0700)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "token.json"), []byte(`{"token":"t"}`), 0600)) + + err := commands.LogoutCommand() + require.NoError(t, err) + + _, err = os.Stat(filepath.Join(dir, "token.json")) + assert.True(t, os.IsNotExist(err)) +} + +func TestLogoutCommand_RemovesLegacyToken(t *testing.T) { + tempHome := t.TempDir() + t.Setenv("HOME", tempHome) + + // Create token at legacy location only + require.NoError(t, os.WriteFile(filepath.Join(tempHome, ".mcp_publisher_token"), []byte(`{"token":"t"}`), 0600)) + + err := commands.LogoutCommand() + require.NoError(t, err) + + _, err = os.Stat(filepath.Join(tempHome, ".mcp_publisher_token")) + assert.True(t, os.IsNotExist(err)) +} + +func TestLogoutCommand_RemovesBothOldAndNew(t *testing.T) { + tempHome := t.TempDir() + t.Setenv("HOME", tempHome) + + // Create tokens at both locations + dir := filepath.Join(tempHome, ".config", "mcp-publisher") + require.NoError(t, os.MkdirAll(dir, 0700)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "token.json"), []byte(`{"token":"new"}`), 0600)) + require.NoError(t, os.WriteFile(filepath.Join(tempHome, ".mcp_publisher_token"), []byte(`{"token":"old"}`), 0600)) + + err := commands.LogoutCommand() + require.NoError(t, err) + + _, err = os.Stat(filepath.Join(dir, "token.json")) + assert.True(t, os.IsNotExist(err), "new token should be removed") + + _, err = os.Stat(filepath.Join(tempHome, ".mcp_publisher_token")) + assert.True(t, os.IsNotExist(err), "legacy token should be removed") +} + +func TestLogoutCommand_CleansUpCwdLegacyFiles(t *testing.T) { + tempHome := t.TempDir() + t.Setenv("HOME", tempHome) + + // Create a token so logout doesn't say "Not logged in" + dir := filepath.Join(tempHome, ".config", "mcp-publisher") + require.NoError(t, os.MkdirAll(dir, 0700)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "token.json"), []byte(`{"token":"t"}`), 0600)) + + // Create legacy intermediate files in a temp cwd + tempCwd := t.TempDir() + origDir, err := os.Getwd() + require.NoError(t, err) + t.Cleanup(func() { _ = os.Chdir(origDir) }) + require.NoError(t, os.Chdir(tempCwd)) + + require.NoError(t, os.WriteFile(".mcpregistry_github_token", []byte("gh-token"), 0600)) + require.NoError(t, os.WriteFile(".mcpregistry_registry_token", []byte("reg-token"), 0600)) + + err = commands.LogoutCommand() + require.NoError(t, err) + + _, err = os.Stat(filepath.Join(tempCwd, ".mcpregistry_github_token")) + assert.True(t, os.IsNotExist(err), ".mcpregistry_github_token should be removed from cwd") + + _, err = os.Stat(filepath.Join(tempCwd, ".mcpregistry_registry_token")) + assert.True(t, os.IsNotExist(err), ".mcpregistry_registry_token should be removed from cwd") +} + +func TestLogoutCommand_CleansUpHomeLegacyFiles(t *testing.T) { + tempHome := t.TempDir() + t.Setenv("HOME", tempHome) + + // Create a token so logout doesn't say "Not logged in" + dir := filepath.Join(tempHome, ".config", "mcp-publisher") + require.NoError(t, os.MkdirAll(dir, 0700)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "token.json"), []byte(`{"token":"t"}`), 0600)) + + // Create legacy intermediate files in $HOME + require.NoError(t, os.WriteFile(filepath.Join(tempHome, ".mcpregistry_github_token"), []byte("gh-token"), 0600)) + require.NoError(t, os.WriteFile(filepath.Join(tempHome, ".mcpregistry_registry_token"), []byte("reg-token"), 0600)) + + err := commands.LogoutCommand() + require.NoError(t, err) + + _, err = os.Stat(filepath.Join(tempHome, ".mcpregistry_github_token")) + assert.True(t, os.IsNotExist(err), ".mcpregistry_github_token should be removed from $HOME") + + _, err = os.Stat(filepath.Join(tempHome, ".mcpregistry_registry_token")) + assert.True(t, os.IsNotExist(err), ".mcpregistry_registry_token should be removed from $HOME") +} + +func TestLogoutCommand_NotLoggedIn(t *testing.T) { + tempHome := t.TempDir() + t.Setenv("HOME", tempHome) + + // No token files exist anywhere + err := commands.LogoutCommand() + // Should not error, just print "Not logged in" + require.NoError(t, err) +} diff --git a/cmd/publisher/commands/publish.go b/cmd/publisher/commands/publish.go index da7de7d..c93621a 100644 --- a/cmd/publisher/commands/publish.go +++ b/cmd/publisher/commands/publish.go @@ -4,12 +4,10 @@ import ( "bytes" "context" "encoding/json" - "errors" "fmt" "io" "net/http" "os" - "path/filepath" "strings" apiv0 "github.com/modelcontextprotocol/registry/pkg/api/v0" @@ -38,16 +36,15 @@ func PublishCommand(args []string) error { } // Load saved token - homeDir, err := os.UserHomeDir() + tokenPath, err := tokenFilePath() if err != nil { - return fmt.Errorf("failed to get home directory: %w", err) + return err } - tokenPath := filepath.Join(homeDir, TokenFileName) tokenData, err := os.ReadFile(tokenPath) if err != nil { if os.IsNotExist(err) { - return errors.New("not authenticated. Run 'mcp-publisher login ' first") + return notAuthenticatedError() } return fmt.Errorf("failed to read token: %w", err) } diff --git a/cmd/publisher/commands/status.go b/cmd/publisher/commands/status.go index bcfacf5..8132fa2 100644 --- a/cmd/publisher/commands/status.go +++ b/cmd/publisher/commands/status.go @@ -12,7 +12,6 @@ import ( "net/http" "net/url" "os" - "path/filepath" "strings" ) @@ -95,16 +94,15 @@ func StatusCommand(args []string) error { } // Load saved token - homeDir, err := os.UserHomeDir() + tokenPath, err := tokenFilePath() if err != nil { - return fmt.Errorf("failed to get home directory: %w", err) + return err } - tokenPath := filepath.Join(homeDir, TokenFileName) tokenData, err := os.ReadFile(tokenPath) if err != nil { if os.IsNotExist(err) { - return errors.New("not authenticated. Run 'mcp-publisher login ' first") + return notAuthenticatedError() } return fmt.Errorf("failed to read token: %w", err) } diff --git a/cmd/publisher/commands/testutil_test.go b/cmd/publisher/commands/testutil_test.go index 51c6687..e69027b 100644 --- a/cmd/publisher/commands/testutil_test.go +++ b/cmd/publisher/commands/testutil_test.go @@ -8,7 +8,6 @@ import ( "path/filepath" "testing" - "github.com/modelcontextprotocol/registry/cmd/publisher/commands" "github.com/modelcontextprotocol/registry/internal/validators" apiv0 "github.com/modelcontextprotocol/registry/pkg/api/v0" "github.com/stretchr/testify/require" @@ -64,14 +63,19 @@ func SetupMockRegistryServer(t *testing.T, publishHandler func(w http.ResponseWr return server } -// SetupTestToken creates a token file pointing to the test server +// SetupTestToken creates a token file pointing to the test server. +// It overrides $HOME to a temp directory so tests don't touch real credentials. func SetupTestToken(t *testing.T, registryURL, token string) string { t.Helper() - homeDir, err := os.UserHomeDir() - require.NoError(t, err) + // Override $HOME so tokenFilePath() resolves to a temp directory + tempHome := t.TempDir() + t.Setenv("HOME", tempHome) - tokenPath := filepath.Join(homeDir, commands.TokenFileName) + dir := filepath.Join(tempHome, ".config", "mcp-publisher") + require.NoError(t, os.MkdirAll(dir, 0700)) + + tokenPath := filepath.Join(dir, "token.json") tokenData := map[string]string{ "token": token, "registry": registryURL, @@ -83,10 +87,6 @@ func SetupTestToken(t *testing.T, registryURL, token string) string { err = os.WriteFile(tokenPath, data, 0600) require.NoError(t, err) - t.Cleanup(func() { - _ = os.Remove(tokenPath) - }) - return tokenPath } diff --git a/cmd/publisher/commands/validate.go b/cmd/publisher/commands/validate.go index b68c2b4..1e360ae 100644 --- a/cmd/publisher/commands/validate.go +++ b/cmd/publisher/commands/validate.go @@ -8,7 +8,6 @@ import ( "io" "net/http" "os" - "path/filepath" "strings" "github.com/modelcontextprotocol/registry/internal/validators" @@ -151,19 +150,15 @@ func ValidateCommand(args []string) error { } // Get registry URL (same pattern as publish) - homeDir, err := os.UserHomeDir() - if err != nil { - return fmt.Errorf("failed to get home directory: %w", err) - } - - tokenPath := filepath.Join(homeDir, TokenFileName) registryURL := DefaultRegistryURL // Try to read registry URL from token file (if it exists) - if tokenData, err := os.ReadFile(tokenPath); err == nil { - var tokenInfo map[string]string - if err := json.Unmarshal(tokenData, &tokenInfo); err == nil { - if url := tokenInfo["registry"]; url != "" { - registryURL = url + if tokenPath, err := tokenFilePath(); err == nil { + if tokenData, err := os.ReadFile(tokenPath); err == nil { + var tokenInfo map[string]string + if err := json.Unmarshal(tokenData, &tokenInfo); err == nil { + if url := tokenInfo["registry"]; url != "" { + registryURL = url + } } } } diff --git a/complete.md b/complete.md index 045e797..2355e4b 100644 --- a/complete.md +++ b/complete.md @@ -2027,21 +2027,24 @@ mcp-publisher logout ``` **Behavior:** -- Removes `~/.mcp_publisher_token` +- Removes `~/.config/mcp-publisher/token.json` +- Also cleans up legacy token files (`~/.mcp_publisher_token`, `.mcpregistry_*`) - Does not revoke tokens on server side ## Configuration ### Token Storage -Authentication tokens stored in `~/.mcp_publisher_token` as JSON: +Authentication tokens are stored in `~/.config/mcp-publisher/token.json` as JSON: ```json { "token": "jwt-token-here", - "registry_url": "https://registry.modelcontextprotocol.io", - "expires_at": "2024-12-31T23:59:59Z" + "method": "github", + "registry": "https://registry.modelcontextprotocol.io" } ``` +> **Note:** Tokens were previously stored in `~/.mcp_publisher_token`. If you are upgrading, run `mcp-publisher logout` followed by `mcp-publisher login` to migrate to the new location. + --- ## File: docs/reference/faq.md diff --git a/docs/reference/cli/commands.md b/docs/reference/cli/commands.md index 4d7d71a..0f44b7b 100644 --- a/docs/reference/cli/commands.md +++ b/docs/reference/cli/commands.md @@ -331,17 +331,20 @@ mcp-publisher logout ``` **Behavior:** -- Removes `~/.mcp_publisher_token` +- Removes `~/.config/mcp-publisher/token.json` +- Also cleans up legacy token files (`~/.mcp_publisher_token`, `.mcpregistry_*`) - Does not revoke tokens on server side ## Configuration ### Token Storage -Authentication tokens stored in `~/.mcp_publisher_token` as JSON: +Authentication tokens are stored in `~/.config/mcp-publisher/token.json` as JSON: ```json { "token": "jwt-token-here", - "registry_url": "https://registry.modelcontextprotocol.io", - "expires_at": "2024-12-31T23:59:59Z" + "method": "github", + "registry": "https://registry.modelcontextprotocol.io" } ``` + +> **Note:** Tokens were previously stored in `~/.mcp_publisher_token`. If you are upgrading, run `mcp-publisher logout` followed by `mcp-publisher login` to migrate to the new location. diff --git a/tests/integration/main.go b/tests/integration/main.go index 889780f..4187ccf 100644 --- a/tests/integration/main.go +++ b/tests/integration/main.go @@ -70,8 +70,15 @@ func cleanupPublisherAuth() { if err != nil { return } - tokenPath := filepath.Join(homeDir, ".mcp_publisher_token") - os.Remove(tokenPath) + // Remove from new config directory location + newTokenPath := filepath.Join(homeDir, ".config", "mcp-publisher", "token.json") + os.Remove(newTokenPath) + // Also remove from legacy location + legacyTokenPath := filepath.Join(homeDir, ".mcp_publisher_token") + os.Remove(legacyTokenPath) + // Clean up legacy intermediate token files from cwd + os.Remove(".mcpregistry_github_token") + os.Remove(".mcpregistry_registry_token") } func publish(examples []example) error {