test(oauth): add comprehensive Manager tests

- Add TestNewManager, TestManagerHasToken, TestManagerGetAccessToken
- Add TestManagerSetToken, TestGenerateState, TestGenerateElicitationID
- Add test cases for GHEC (ghe.com) and host without scheme
- Improve test documentation with comments

Author: SamMorrowDrums

docs/oauth-authentication.md

@@ -0,0 +1,144 @@
+# OAuth Authentication
+
+The GitHub MCP Server supports OAuth authentication for stdio mode, enabling interactive authentication when no Personal Access Token (PAT) is configured.
+
+## Overview
+
+OAuth authentication allows users to authenticate with GitHub through their browser without pre-configuring a token. This is useful for:
+
+- **Interactive sessions** where users want to authenticate on-demand
+- **Docker deployments** where tokens shouldn't be baked into images
+- **Multi-user scenarios** where each user authenticates individually
+
+## Configuration
+
+### Required Environment Variables
+
+| Variable | Description | Required |
+|----------|-------------|----------|
+| `GITHUB_OAUTH_CLIENT_ID` | OAuth app client ID | Yes |
+| `GITHUB_OAUTH_CLIENT_SECRET` | OAuth app client secret | Recommended |
+
+### Optional Flags
+
+| Flag | Environment Variable | Description |
+|------|---------------------|-------------|
+| `--oauth-callback-port` | `GITHUB_OAUTH_CALLBACK_PORT` | Fixed port for OAuth callback (required for Docker with `-p` flag) |
+| `--oauth-scopes` | `GITHUB_OAUTH_SCOPES` | Custom OAuth scopes (comma-separated) |
+
+## Authentication Flows
+
+The server automatically selects the appropriate OAuth flow based on the environment:
+
+### 1. PKCE Flow (Browser-based)
+
+Used for local binary execution where a browser can be opened:
+
+1. Server starts a local callback server
+2. Browser opens to GitHub authorization page
+3. User authorizes the application
+4. GitHub redirects to local callback with authorization code
+5. Server exchanges code for access token
+
+### 2. Device Flow (Docker/Headless)
+
+Used when running in Docker or when a browser cannot be opened:
+
+1. Server requests a device code from GitHub
+2. User is shown a URL and code to enter
+3. User visits `github.com/login/device` and enters the code
+4. Server polls GitHub until authorization is complete
+5. Access token is retrieved
+
+## Usage Examples
+
+### Local Binary
+
+```bash
+# Set OAuth credentials
+export GITHUB_OAUTH_CLIENT_ID="your-client-id"
+export GITHUB_OAUTH_CLIENT_SECRET="your-client-secret"
+
+# Run without PAT - OAuth will trigger when tools are called
+./github-mcp-server stdio
+```
+
+### Docker (with Device Flow)
+
+```bash
+docker run -i --rm \
+  -e GITHUB_OAUTH_CLIENT_ID="your-client-id" \
+  -e GITHUB_OAUTH_CLIENT_SECRET="your-client-secret" \
+  ghcr.io/github/github-mcp-server stdio
+```
+
+### Docker (with PKCE Flow via port mapping)
+
+```bash
+docker run -i --rm \
+  --network=host \
+  -e GITHUB_OAUTH_CLIENT_ID="your-client-id" \
+  -e GITHUB_OAUTH_CLIENT_SECRET="your-client-secret" \
+  ghcr.io/github/github-mcp-server stdio --oauth-callback-port=8085
+```
+
+### VS Code MCP Configuration
+
+```jsonc
+{
+  "mcpServers": {
+    "github": {
+      "command": "docker",
+      "args": [
+        "run", "-i", "--rm",
+        "-e", "GITHUB_OAUTH_CLIENT_ID=your-client-id",
+        "-e", "GITHUB_OAUTH_CLIENT_SECRET=your-client-secret",
+        "ghcr.io/github/github-mcp-server",
+        "stdio"
+      ],
+      "type": "stdio"
+    }
+  }
+}
+```
+
+## Creating an OAuth App
+
+1. Go to **GitHub Settings** → **Developer settings** → **OAuth Apps**
+2. Click **New OAuth App**
+3. Fill in the details:
+   - **Application name**: Your app name (e.g., "GitHub MCP Server")
+   - **Homepage URL**: Your homepage or `https://github.com/github/github-mcp-server`
+   - **Authorization callback URL**: `http://localhost:8085/callback` (or your chosen port)
+4. Click **Register application**
+5. Copy the **Client ID**
+6. Generate and copy the **Client Secret**
+
+## Scope Computation
+
+The server automatically computes the required OAuth scopes based on enabled tools:
+
+- If `--toolsets` or `--tools` are specified, only scopes for those tools are requested
+- If no tools are specified, default scopes are used: `repo`, `user`, `gist`, `notifications`, `read:org`, `project`
+- Custom scopes can be specified with `--oauth-scopes`
+
+## Security Considerations
+
+1. **Client Secret**: While optional for public OAuth apps, using a client secret is recommended for better security
+2. **Token Storage**: OAuth tokens are stored in memory only and not persisted to disk
+3. **Scope Minimization**: Request only the scopes needed for your use case
+4. **PKCE**: The PKCE flow provides protection against authorization code interception attacks
+
+## Troubleshooting
+
+### "redirect_uri not associated with this client"
+
+Ensure the callback port matches your OAuth app's registered callback URL. Use `--oauth-callback-port` to specify the exact port.
+
+### Browser doesn't open automatically
+
+The server will fall back to displaying the authorization URL. In Docker, the device flow is used automatically.
+
+### Token not being used
+
+Verify that `GITHUB_PERSONAL_ACCESS_TOKEN` is not set, as it takes precedence over OAuth.

internal/oauth/manager.go

@@ -93,7 +93,9 @@ func (m *Manager) RequestAuthentication(ctx context.Context, session *mcp.Server
 	return m.startPKCEFlowWithElicitation(ctx, session)
 }
 
-// startDeviceFlowWithElicitation initiates device flow and uses session elicitation
+// startDeviceFlowWithElicitation initiates device flow and uses session elicitation.
+// Device flow is used when a callback server cannot be started (e.g., in Docker containers).
+// It displays a code that the user must enter at the verification URL.
 func (m *Manager) startDeviceFlowWithElicitation(ctx context.Context, session *mcp.ServerSession) error {
 	oauth2Cfg := &oauth2.Config{
 		ClientID:     m.config.ClientID,
@@ -112,11 +114,14 @@ func (m *Manager) startDeviceFlowWithElicitation(ctx context.Context, session *m
 		return fmt.Errorf("failed to get device authorization: %w", err)
 	}
 
-	// Use session elicitation if available (blocks until user responds)
+	// Use session elicitation if available to show the user the verification URL and code
 	if session != nil {
-		// Generate elicitation ID for tracking
-		elicitID, _ := generateElicitationID()
-		// Use the base verification URI and show the code in the message for the user to paste
+		elicitID, err := generateElicitationID()
+		if err != nil {
+			// Log warning but continue - elicitation ID is for tracking only
+			elicitID = "fallback-id"
+		}
+		// Elicitation result is not critical - device flow polls independently
 		_, _ = session.Elicit(ctx, &mcp.ElicitParams{
 			Mode:          "url",
 			URL:           deviceAuth.VerificationURI,

internal/oauth/oauth_test.go

@@ -2,6 +2,7 @@ package oauth
 
 import (
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -57,6 +58,21 @@ func TestGetGitHubOAuthConfig(t *testing.T) {
 		assert.Equal(t, "https://github.enterprise.com", cfg.Host)
 		assert.Equal(t, 8080, cfg.CallbackPort)
 	})
+
+	t.Run("GHEC host (ghe.com)", func(t *testing.T) {
+		cfg := GetGitHubOAuthConfig(clientID, clientSecret, scopes, "https://mycompany.ghe.com", 0)
+
+		assert.Equal(t, "https://mycompany.ghe.com/login/oauth/authorize", cfg.AuthURL)
+		assert.Equal(t, "https://mycompany.ghe.com/login/oauth/access_token", cfg.TokenURL)
+		assert.Equal(t, "https://mycompany.ghe.com/login/device/code", cfg.DeviceAuthURL)
+	})
+
+	t.Run("host without scheme", func(t *testing.T) {
+		cfg := GetGitHubOAuthConfig(clientID, clientSecret, scopes, "github.enterprise.com", 0)
+
+		// Should default to https
+		assert.Equal(t, "https://github.enterprise.com/login/oauth/authorize", cfg.AuthURL)
+	})
 }
 
 func TestStartLocalServer(t *testing.T) {
@@ -81,3 +97,111 @@ func TestStartLocalServer(t *testing.T) {
 		assert.Equal(t, fixedPort, port)
 	})
 }
+
+// Manager tests
+
+func TestNewManager(t *testing.T) {
+	cfg := Config{
+		ClientID:     "test-client-id",
+		ClientSecret: "test-secret",
+		Scopes:       []string{"repo"},
+	}
+
+	mgr := NewManager(cfg)
+
+	assert.NotNil(t, mgr)
+	assert.Equal(t, cfg.ClientID, mgr.config.ClientID)
+	assert.Equal(t, cfg.ClientSecret, mgr.config.ClientSecret)
+	assert.Equal(t, cfg.Scopes, mgr.config.Scopes)
+	assert.False(t, mgr.HasToken())
+	assert.Empty(t, mgr.GetAccessToken())
+}
+
+func TestManagerHasToken(t *testing.T) {
+	mgr := NewManager(Config{})
+
+	t.Run("no token initially", func(t *testing.T) {
+		assert.False(t, mgr.HasToken())
+	})
+
+	t.Run("has token after setting", func(t *testing.T) {
+		mgr.setToken(&Result{
+			AccessToken: "test-token",
+			TokenType:   "Bearer",
+		})
+
+		assert.True(t, mgr.HasToken())
+	})
+
+	t.Run("no token if empty access token", func(t *testing.T) {
+		mgr.setToken(&Result{
+			AccessToken: "",
+			TokenType:   "Bearer",
+		})
+
+		assert.False(t, mgr.HasToken())
+	})
+}
+
+func TestManagerGetAccessToken(t *testing.T) {
+	mgr := NewManager(Config{})
+
+	t.Run("empty initially", func(t *testing.T) {
+		assert.Empty(t, mgr.GetAccessToken())
+	})
+
+	t.Run("returns token after setting", func(t *testing.T) {
+		expectedToken := "gho_test123456"
+		mgr.setToken(&Result{
+			AccessToken:  expectedToken,
+			TokenType:    "Bearer",
+			RefreshToken: "refresh-token",
+			Expiry:       time.Now().Add(time.Hour),
+		})
+
+		assert.Equal(t, expectedToken, mgr.GetAccessToken())
+	})
+}
+
+func TestManagerSetToken(t *testing.T) {
+	mgr := NewManager(Config{})
+
+	token := &Result{
+		AccessToken:  "test-access-token",
+		RefreshToken: "test-refresh-token",
+		TokenType:    "Bearer",
+		Expiry:       time.Now().Add(time.Hour),
+	}
+
+	mgr.setToken(token)
+
+	// Verify token is stored correctly
+	assert.Equal(t, token.AccessToken, mgr.GetAccessToken())
+	assert.True(t, mgr.HasToken())
+}
+
+func TestGenerateState(t *testing.T) {
+	state1, err := generateState()
+	require.NoError(t, err)
+	require.NotEmpty(t, state1)
+
+	// State should be URL-safe base64 encoded
+	// 16 bytes of random data = ~22 chars in base64url
+	assert.GreaterOrEqual(t, len(state1), 20)
+
+	// Each call should produce unique state
+	state2, err := generateState()
+	require.NoError(t, err)
+	assert.NotEqual(t, state1, state2)
+}
+
+func TestGenerateElicitationID(t *testing.T) {
+	id1, err := generateElicitationID()
+	require.NoError(t, err)
+	require.NotEmpty(t, id1)
+
+	// Each call should produce unique ID
+	id2, err := generateElicitationID()
+	require.NoError(t, err)
+	assert.NotEqual(t, id1, id2)
+}