fix(cli): login over HTTPS + suggest last-used email
Some checks failed
CI/CD / typecheck (pull_request) Successful in 1m19s
CI/CD / lint (pull_request) Successful in 2m13s
CI/CD / test (pull_request) Successful in 1m18s
CI/CD / smoke (pull_request) Failing after 2m35s
CI/CD / build (pull_request) Successful in 2m25s
CI/CD / publish (pull_request) Has been skipped

auth.ts talked to mcpd over node:http unconditionally with port:url.port,
so an https:// mcpdUrl connected to the ingress on :80, got a 301 redirect
with an empty body (status < 400), and JSON.parse('') crashed login with
"Unexpected end of JSON input". api-client.ts already handled this; the auth
commands did not.

- All 4 auth request fns now pick https/443 and the TLS driver from the URL
  protocol, mirroring api-client.ts.
- parseJsonResponse() guard turns any non-JSON <400 body into a clear
  "Invalid <ctx> response from mcpd" error instead of a raw SyntaxError.
- login now suggests the previously-used email (when a session for the same
  mcpd exists) so pressing Enter reuses it after a session expires.
- Regression tests drive the real default* request fns against a local
  HTTP server (valid JSON, empty body, HTML redirect body, https driver
  selection) and cover the email-suggestion behaviour.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Michal
2026-07-17 21:06:29 +01:00
parent 76470f4f8c
commit 6ef16f12b6
2 changed files with 155 additions and 19 deletions

View File

@@ -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 });