Merge pull request 'fix(cli): login over HTTPS + suggest last-used email' (#78) from fix/login-https-and-email-suggest into main

src/cli/src/commands/auth.ts

@@ -1,12 +1,13 @@
 import { Command } from 'commander';
 import http from 'node:http';
+import https from 'node:https';
 import { loadConfig } from '../config/index.js';
 import type { ConfigLoaderDeps } from '../config/index.js';
 import { saveCredentials, loadCredentials, deleteCredentials } from '../auth/index.js';
 import type { CredentialsDeps } from '../auth/index.js';
 
 export interface PromptDeps {
-  input(message: string): Promise<string>;
+  input(message: string, defaultValue?: string): Promise<string>;
   password(message: string): Promise<string>;
 }
 
@@ -30,19 +31,35 @@ interface LoginResponse {
   user: { email: string };
 }
 
-function defaultLoginRequest(mcpdUrl: string, email: string, password: string): Promise<LoginResponse> {
+// mcpd always answers these endpoints with JSON. A non-JSON body on a <400
+// response means we didn't actually reach mcpd (e.g. an ingress redirect landing
+// on the wrong protocol/port), so surface a clear error instead of a raw
+// SyntaxError stack trace.
+function parseJsonResponse<T>(raw: string, context: string): T {
+  try {
+    return JSON.parse(raw) as T;
+  } catch {
+    const preview = raw.trim() ? `: ${raw.slice(0, 200)}` : ' (empty body)';
+    throw new Error(
+      `Invalid ${context} response from mcpd. Check that mcpdUrl points to an mcpd server${preview}`,
+    );
+  }
+}
+
+export function defaultLoginRequest(mcpdUrl: string, email: string, password: string): Promise<LoginResponse> {
   return new Promise((resolve, reject) => {
     const url = new URL('/api/v1/auth/login', mcpdUrl);
+    const isHttps = url.protocol === 'https:';
     const body = JSON.stringify({ email, password });
     const opts: http.RequestOptions = {
       hostname: url.hostname,
-      port: url.port,
+      port: url.port || (isHttps ? 443 : 80),
       path: url.pathname,
       method: 'POST',
       timeout: 10000,
       headers: { 'Content-Type': 'application/json', 'Content-Length': Buffer.byteLength(body) },
     };
-    const req = http.request(opts, (res) => {
+    const req = (isHttps ? https : http).request(opts, (res) => {
       const chunks: Buffer[] = [];
       res.on('data', (chunk: Buffer) => chunks.push(chunk));
       res.on('end', () => {
@@ -55,7 +72,11 @@ function defaultLoginRequest(mcpdUrl: string, email: string, password: string):
           reject(new Error(`Login failed (${res.statusCode}): ${raw}`));
           return;
         }
-        resolve(JSON.parse(raw) as LoginResponse);
+        try {
+          resolve(parseJsonResponse<LoginResponse>(raw, 'login'));
+        } catch (err) {
+          reject(err as Error);
+        }
       });
     });
     req.on('error', (err) => reject(new Error(`Cannot reach mcpd: ${err.message}`)));
@@ -68,15 +89,16 @@ function defaultLoginRequest(mcpdUrl: string, email: string, password: string):
 function defaultLogoutRequest(mcpdUrl: string, token: string): Promise<void> {
   return new Promise((resolve) => {
     const url = new URL('/api/v1/auth/logout', mcpdUrl);
+    const isHttps = url.protocol === 'https:';
     const opts: http.RequestOptions = {
       hostname: url.hostname,
-      port: url.port,
+      port: url.port || (isHttps ? 443 : 80),
       path: url.pathname,
       method: 'POST',
       timeout: 10000,
       headers: { 'Authorization': `Bearer ${token}` },
     };
-    const req = http.request(opts, (res) => {
+    const req = (isHttps ? https : http).request(opts, (res) => {
       res.resume();
       res.on('end', () => resolve());
     });
@@ -86,18 +108,19 @@ function defaultLogoutRequest(mcpdUrl: string, token: string): Promise<void> {
   });
 }
 
-function defaultStatusRequest(mcpdUrl: string): Promise<StatusResponse> {
+export function defaultStatusRequest(mcpdUrl: string): Promise<StatusResponse> {
   return new Promise((resolve, reject) => {
     const url = new URL('/api/v1/auth/status', mcpdUrl);
+    const isHttps = url.protocol === 'https:';
     const opts: http.RequestOptions = {
       hostname: url.hostname,
-      port: url.port,
+      port: url.port || (isHttps ? 443 : 80),
       path: url.pathname,
       method: 'GET',
       timeout: 10000,
       headers: { 'Content-Type': 'application/json' },
     };
-    const req = http.request(opts, (res) => {
+    const req = (isHttps ? https : http).request(opts, (res) => {
       const chunks: Buffer[] = [];
       res.on('data', (chunk: Buffer) => chunks.push(chunk));
       res.on('end', () => {
@@ -106,7 +129,11 @@ function defaultStatusRequest(mcpdUrl: string): Promise<StatusResponse> {
           reject(new Error(`Status check failed (${res.statusCode}): ${raw}`));
           return;
         }
-        resolve(JSON.parse(raw) as StatusResponse);
+        try {
+          resolve(parseJsonResponse<StatusResponse>(raw, 'status'));
+        } catch (err) {
+          reject(err as Error);
+        }
       });
     });
     req.on('error', (err) => reject(new Error(`Cannot reach mcpd: ${err.message}`)));
@@ -115,9 +142,10 @@ function defaultStatusRequest(mcpdUrl: string): Promise<StatusResponse> {
   });
 }
 
-function defaultBootstrapRequest(mcpdUrl: string, email: string, password: string, name?: string): Promise<LoginResponse> {
+export function defaultBootstrapRequest(mcpdUrl: string, email: string, password: string, name?: string): Promise<LoginResponse> {
   return new Promise((resolve, reject) => {
     const url = new URL('/api/v1/auth/bootstrap', mcpdUrl);
+    const isHttps = url.protocol === 'https:';
     const payload: Record<string, string> = { email, password };
     if (name) {
       payload['name'] = name;
@@ -125,13 +153,13 @@ function defaultBootstrapRequest(mcpdUrl: string, email: string, password: strin
     const body = JSON.stringify(payload);
     const opts: http.RequestOptions = {
       hostname: url.hostname,
-      port: url.port,
+      port: url.port || (isHttps ? 443 : 80),
       path: url.pathname,
       method: 'POST',
       timeout: 10000,
       headers: { 'Content-Type': 'application/json', 'Content-Length': Buffer.byteLength(body) },
     };
-    const req = http.request(opts, (res) => {
+    const req = (isHttps ? https : http).request(opts, (res) => {
       const chunks: Buffer[] = [];
       res.on('data', (chunk: Buffer) => chunks.push(chunk));
       res.on('end', () => {
@@ -140,7 +168,11 @@ function defaultBootstrapRequest(mcpdUrl: string, email: string, password: strin
           reject(new Error(`Bootstrap failed (${res.statusCode}): ${raw}`));
           return;
         }
-        resolve(JSON.parse(raw) as LoginResponse);
+        try {
+          resolve(parseJsonResponse<LoginResponse>(raw, 'bootstrap'));
+        } catch (err) {
+          reject(err as Error);
+        }
       });
     });
     req.on('error', (err) => reject(new Error(`Cannot reach mcpd: ${err.message}`)));
@@ -150,9 +182,15 @@ function defaultBootstrapRequest(mcpdUrl: string, email: string, password: strin
   });
 }
 
-async function defaultInput(message: string): Promise<string> {
+async function defaultInput(message: string, defaultValue?: string): Promise<string> {
   const { default: inquirer } = await import('inquirer');
-  const { answer } = await inquirer.prompt([{ type: 'input', name: 'answer', message }]);
+  const question: { type: 'input'; name: 'answer'; message: string; default?: string } = {
+    type: 'input', name: 'answer', message,
+  };
+  if (defaultValue) {
+    question.default = defaultValue;
+  }
+  const { answer } = await inquirer.prompt([question]);
   return answer as string;
 }
 
@@ -202,7 +240,11 @@ export function createLoginCommand(deps?: Partial<AuthCommandDeps>): Command {
           }, credentialsDeps);
           log(`Logged in as ${result.user.email} (admin)`);
         } else {
-          const email = await prompt.input('Email:');
+          // If we were previously logged into this same mcpd (e.g. the session
+          // just expired), suggest the last-used email so Enter reuses it.
+          const existing = loadCredentials(credentialsDeps);
+          const suggestedEmail = existing && existing.mcpdUrl === mcpdUrl ? existing.user : undefined;
+          const email = await prompt.input('Email:', suggestedEmail);
           const password = await prompt.password('Password:');
 
           const result = await loginRequest(mcpdUrl, email, password);

src/cli/tests/commands/auth.test.ts

@@ -2,7 +2,9 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest';
 import { mkdtempSync, rmSync } from 'node:fs';
 import { join } from 'node:path';
 import { tmpdir } from 'node:os';
-import { createLoginCommand, createLogoutCommand } from '../../src/commands/auth.js';
+import http from 'node:http';
+import type { AddressInfo } from 'node:net';
+import { createLoginCommand, createLogoutCommand, defaultStatusRequest } from '../../src/commands/auth.js';
 import { saveCredentials, loadCredentials } from '../../src/auth/index.js';
 import { saveConfig, DEFAULT_CONFIG } from '../../src/config/index.js';
 
@@ -94,6 +96,53 @@ describe('login command', () => {
     expect(capturedUrl).toBe('http://custom:3100');
   });
 
+  it('suggests the previously-used email when a session for the same mcpd exists', async () => {
+    saveConfig({ ...DEFAULT_CONFIG, mcpdUrl: 'http://custom:3100' }, { configDir: tempDir });
+    saveCredentials({ token: 'stale', mcpdUrl: 'http://custom:3100', user: 'returning@test.com' }, { configDir: tempDir });
+    let suggestedDefault: string | undefined = 'UNSET';
+    const cmd = createLoginCommand({
+      configDeps: { configDir: tempDir },
+      credentialsDeps: { configDir: tempDir },
+      prompt: {
+        // Simulate the user pressing Enter: return the suggested default unchanged.
+        input: async (_msg, def) => { suggestedDefault = def; return def ?? ''; },
+        password: async () => 'newpass',
+      },
+      log,
+      loginRequest: async (_url, email) => ({ token: 'fresh', user: { email } }),
+      logoutRequest: async () => {},
+      statusRequest: async () => ({ hasUsers: true }),
+      bootstrapRequest: async () => ({ token: '', user: { email: '' } }),
+    });
+    await cmd.parseAsync([], { from: 'user' });
+
+    expect(suggestedDefault).toBe('returning@test.com');
+    const creds = loadCredentials({ configDir: tempDir });
+    expect(creds!.user).toBe('returning@test.com');
+  });
+
+  it('does not suggest an email from a different mcpd', async () => {
+    saveConfig({ ...DEFAULT_CONFIG, mcpdUrl: 'http://server-a:3100' }, { configDir: tempDir });
+    saveCredentials({ token: 'stale', mcpdUrl: 'http://server-b:3100', user: 'elsewhere@test.com' }, { configDir: tempDir });
+    let suggestedDefault: string | undefined = 'UNSET';
+    const cmd = createLoginCommand({
+      configDeps: { configDir: tempDir },
+      credentialsDeps: { configDir: tempDir },
+      prompt: {
+        input: async (_msg, def) => { suggestedDefault = def; return 'typed@test.com'; },
+        password: async () => 'pass',
+      },
+      log,
+      loginRequest: async (_url, email) => ({ token: 'tok', user: { email } }),
+      logoutRequest: async () => {},
+      statusRequest: async () => ({ hasUsers: true }),
+      bootstrapRequest: async () => ({ token: '', user: { email: '' } }),
+    });
+    await cmd.parseAsync([], { from: 'user' });
+
+    expect(suggestedDefault).toBeUndefined();
+  });
+
   it('allows --mcpd-url flag override', async () => {
     let capturedUrl = '';
     const cmd = createLoginCommand({
@@ -177,6 +226,51 @@ describe('login bootstrap flow', () => {
   });
 });
 
+// Regression: `mcpctl login` crashed with "JSON Parse error: Unexpected EOF"
+// when mcpdUrl used https. defaultStatusRequest hardcoded the plain-http driver
+// and port 80, so an https mcpdUrl hit the ingress on :80, got a redirect with an
+// empty body (status < 400), and JSON.parse('') threw a raw SyntaxError.
+describe('defaultStatusRequest (real HTTP)', () => {
+  let server: http.Server;
+  let baseUrl: string;
+  let handler: (req: http.IncomingMessage, res: http.ServerResponse) => void;
+
+  beforeEach(async () => {
+    handler = (_req, res) => { res.statusCode = 200; res.end(JSON.stringify({ hasUsers: true })); };
+    server = http.createServer((req, res) => handler(req, res));
+    await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
+    const { port } = server.address() as AddressInfo;
+    baseUrl = `http://127.0.0.1:${port}`;
+  });
+
+  afterEach(async () => {
+    await new Promise<void>((resolve) => server.close(() => resolve()));
+  });
+
+  it('parses a valid JSON status response', async () => {
+    const status = await defaultStatusRequest(baseUrl);
+    expect(status.hasUsers).toBe(true);
+  });
+
+  it('rejects with a clear error (not a raw SyntaxError) on an empty body', async () => {
+    handler = (_req, res) => { res.statusCode = 200; res.end(''); };
+    await expect(defaultStatusRequest(baseUrl)).rejects.toThrow(/Invalid status response from mcpd.*empty body/);
+  });
+
+  it('rejects with a clear error on a non-JSON (HTML redirect) body', async () => {
+    handler = (_req, res) => { res.statusCode = 200; res.end('<html>301 Moved</html>'); };
+    await expect(defaultStatusRequest(baseUrl)).rejects.toThrow(/Invalid status response from mcpd/);
+  });
+
+  it('uses the TLS driver for an https URL (would silently succeed over plain http before the fix)', async () => {
+    const { port } = server.address() as AddressInfo;
+    // Point an https URL at the plain-http test server. The https driver cannot
+    // complete a TLS handshake with it, so this must fail to connect. The old
+    // code always used the http driver and would have resolved instead.
+    await expect(defaultStatusRequest(`https://127.0.0.1:${port}`)).rejects.toThrow(/Cannot reach mcpd/);
+  });
+});
+
 describe('logout command', () => {
   it('removes credentials on logout', async () => {
     saveCredentials({ token: 'tok', mcpdUrl: 'http://x:3100', user: 'alice' }, { configDir: tempDir });