diff --git a/internal/documentation/docs/pages/Overview.md b/internal/documentation/docs/pages/Overview.md index 6baa008bcea..77d89f510a6 100644 --- a/internal/documentation/docs/pages/Overview.md +++ b/internal/documentation/docs/pages/Overview.md @@ -119,7 +119,7 @@ The UI5 CLI contains a web server to serve the project via HTTPS protocol. ui5 serve --https ``` -This requires an SSL certificate. You are guided through the automatic generation process. Also see the [UI5 Server documentation](./Server.md#ssl-certificates) +This requires an SSL certificate. Generate one up front with `ui5 certificate generate`. Also see the [UI5 Server documentation](./Server.md#ssl-certificates) ## Integration in Other Tools One of the key features of the UI5 CLI is its modularization. Single parts of UI5 CLI can easily be integrated in other `Node.js`-based tools and frameworks like [Grunt](https://gruntjs.com/) or [Gulp](https://gulpjs.com/). diff --git a/internal/documentation/docs/pages/Server.md b/internal/documentation/docs/pages/Server.md index e8051e65d1d..a5ac78e428a 100644 --- a/internal/documentation/docs/pages/Server.md +++ b/internal/documentation/docs/pages/Server.md @@ -228,9 +228,15 @@ A project graph can be served only once. Do not call both `serveMiddleware` and ::: ## SSL Certificates -When starting the UI5 Server in HTTPS mode, for example by using UI5 CLI parameter `--https`, you will be prompted for the automatic generation of a local SSL certificate if necessary. +Starting the UI5 Server in HTTPS mode, for example by using the UI5 CLI parameter `--https`, requires an SSL certificate. Generate and install a self-signed certificate with: -Follow the given instructions and enter your password to install the generated certificate as trusted. You can find the generated certificate and corresponding private key under `.ui5/server` in your user's home directory. +```sh +ui5 certificate generate +``` + +Follow the given instructions and enter your password (or confirm the dialog on Windows) to install the generated certificate as trusted. By default the certificate and its private key are written to the UI5 data directory (resolved from `UI5_DATA_DIR`, your UI5 config, or `~/.ui5` by default). Use `--force` to overwrite an existing certificate, or `--key`/`--cert` to write to custom paths. + +`ui5 serve --https` picks up the certificate from the same default location. If no certificate is found, the server fails with guidance to run `ui5 certificate generate` or to provide existing files via `--key` and `--cert`. ::: tip If Chrome unintentionally redirects an HTTP-URL to HTTPS, you need to delete the HSTS mapping in [chrome://net-internals/#hsts](chrome://net-internals/#hsts) by entering the domain name (e.g. localhost) and pressing "delete". diff --git a/internal/documentation/docs/pages/Troubleshooting.md b/internal/documentation/docs/pages/Troubleshooting.md index 738176d59dd..eda8d356299 100644 --- a/internal/documentation/docs/pages/Troubleshooting.md +++ b/internal/documentation/docs/pages/Troubleshooting.md @@ -18,7 +18,7 @@ UI5 CLI stores several kinds of data under your user's home directory in `~/.ui5 | ---- | ---- | ---- | | `~/.ui5/framework/` | Downloaded UI5 framework dependencies (one copy per version) | Yes — re-downloaded on next invocation | | `~/.ui5/buildCache/` | Build cache used by `ui5 build` and `ui5 serve` (see [Build Cache Control](./Builder.md#build-cache-control)) | Yes — rebuilt on next `ui5 build` / `ui5 serve` | -| `~/.ui5/server/` | Locally generated SSL certificate and private key for HTTPS mode | Yes — regenerated on next HTTPS server start; the new certificate must be re-trusted | +| `~/.ui5/server/` | Locally generated SSL certificate and private key for HTTPS mode | Yes — recreate it with `ui5 certificate generate`; the new certificate must be re-trusted | #### Resolution diff --git a/internal/documentation/docs/updates/migrate-v5.md b/internal/documentation/docs/updates/migrate-v5.md index 661b634869a..4239832fa51 100644 --- a/internal/documentation/docs/updates/migrate-v5.md +++ b/internal/documentation/docs/updates/migrate-v5.md @@ -27,6 +27,8 @@ Or update your global install via: `npm i --global @ui5/cli@next` - **@ui5/cli: `ui5 serve` renders a status banner in interactive terminals** +- **@ui5/cli: New `ui5 certificate generate` command; `ui5 serve --https` no longer generates a certificate on the fly** + - **@ui5/cli: Option `--experimental-css-variables` has been removed** - **@ui5/server: The internal Express server has been upgraded from Express 4 to Express 5** @@ -383,10 +385,20 @@ The `--h2` option for the `ui5 serve` command has therefore been removed. Use th ui5 serve --https ``` -Like the previous `--h2` option, `--https` requires an SSL certificate and guides you through the automatic generation process on first use. See the [UI5 Server documentation](../pages/Server.md#ssl-certificates) for details. +Like the previous `--h2` option, `--https` requires an SSL certificate. See [SSL Certificate Generation](#ssl-certificate-generation) below to learn how to generate a certificate or provide an existing one. If you need HTTP/2 for local development, manually set up a reverse proxy, for example, [nginx](https://nginx.org/), in front of the UI5 Server to handle the HTTP/2 connection with the browser. +## SSL Certificate Generation + +In previous versions, UI5 CLI generated and installed a self-signed SSL certificate proactively. In UI5 CLI v5, certificate generation is an explicit step: + +```sh +ui5 certificate generate +``` + +For certificate locations, custom paths, and server behavior when no certificate is found, see [SSL Certificates](../pages/Server#ssl-certificates). + ## `sap-ui-version.json` When you run `ui5 build`, the standard task [`generateVersionInfo`](../api/module-@ui5_builder_tasks_generateVersionInfo) now runs by default. This task generates an `sap-ui-version.json` file in the `resources/` directory. The task runs for all build types (default, jsdoc, and self-contained) in projects of type `application`. For other project types (such as `library`), the behavior remains unchanged: [`generateVersionInfo`](../api/module-@ui5_builder_tasks_generateVersionInfo) does not run. diff --git a/package-lock.json b/package-lock.json index 2eba411774e..e1e2d96eba8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -19259,8 +19259,7 @@ "parseurl": "^1.3.3", "portscanner": "^2.2.0", "router": "^2.2.0", - "ws": "^8.21.3", - "yesno": "^0.4.0" + "ws": "^8.21.3" }, "devDependencies": { "@istanbuljs/esm-loader-hook": "^0.3.0", diff --git a/packages/cli/lib/cli/commands/certificate.js b/packages/cli/lib/cli/commands/certificate.js new file mode 100644 index 00000000000..13dd8f7e64a --- /dev/null +++ b/packages/cli/lib/cli/commands/certificate.js @@ -0,0 +1,104 @@ +import chalk from "chalk"; +import process from "node:process"; +import baseMiddleware from "../middlewares/base.js"; +import {getUi5DataDirOrDefault, resolveServerCertificatePaths, formatPath} from "../../dataDir.js"; +import {exists} from "../../utils/fsHelper.js"; + +const certificateCommand = { + command: "certificate", + describe: "Manage the UI5 CLI server certificate", + middlewares: [baseMiddleware], +}; + +certificateCommand.builder = function(cli) { + return cli + .demandCommand(1, "Command required. Available command is 'generate'") + .command("generate", "Generate a self-signed server certificate and install it into the trust store", { + handler: handleGenerate, + builder: function(yargs) { + return yargs + .option("key", { + describe: "Path the private key is written to", + defaultDescription: "~/.ui5/server/server.key", + type: "string" + }) + .option("cert", { + describe: "Path the certificate is written to", + defaultDescription: "~/.ui5/server/server.crt", + type: "string" + }) + .option("force", { + alias: "f", + describe: "Generate a new certificate even if one already exists at the target path", + default: false, + type: "boolean" + }) + .example("$0 certificate generate", + "Generate a server certificate in the default UI5 data directory") + .example("$0 certificate generate --force", + "Regenerate the server certificate, overwriting an existing one") + .example("UI5_DATA_DIR=/custom/path $0 certificate generate", + "Generate a server certificate in a non-default UI5 data directory"); + }, + middlewares: [baseMiddleware], + }); +}; + +async function handleGenerate(argv) { + const ui5DataDir = await getUi5DataDirOrDefault({cwd: process.cwd()}); + const {keyPath, certPath} = resolveServerCertificatePaths(ui5DataDir, { + keyPath: argv.key, + certPath: argv.cert, + }); + + if (!argv.force) { + let keyExists; + let certExists; + try { + [keyExists, certExists] = await Promise.all([exists(keyPath), exists(certPath)]); + } catch (err) { + throw new Error( + `Failed to check for an existing server certificate at ${formatPath(keyPath)} ` + + `and ${formatPath(certPath)}: ${err.message}`, {cause: err}); + } + // Only a complete pair counts as "already existing". A partial state (just the key or just the + // certificate) is a broken pair that the user cannot otherwise repair without --force, so fall + // through to regeneration, which overwrites any leftover file. + if (keyExists && certExists) { + process.stderr.write( + `A server certificate already exists at the target location:\n` + + ` Private key: ${chalk.bold(formatPath(keyPath))}\n` + + ` Certificate: ${chalk.bold(formatPath(certPath))}\n\n` + + `Use ${chalk.bold("--force")} to generate a new certificate and overwrite the existing one.\n` + ); + return; + } + } + + // Inform the user before triggering the trust-store installation, which requires elevated + // privileges and therefore prompts for the root password (or shows a confirmation dialog on Windows). + if (process.platform === "win32") { + process.stderr.write("Please press allow in the opened dialog to confirm importing the newly created " + + "SSL certificate into the operating system and browsers.\n"); + } else { + process.stderr.write("Please enter your root password to allow importing the newly created " + + "SSL certificate into the operating system and browsers.\n"); + } + + const {generateSslCertificate} = await import("@ui5/server/internal/sslUtil"); + await generateSslCertificate(keyPath, certPath); + + process.stderr.write( + `\nServer certificate written:\n` + + ` Private key: ${chalk.bold(formatPath(keyPath))}\n` + + ` Certificate: ${chalk.bold(formatPath(certPath))}\n` + ); + + // devcert-sanscache leaves handles open that keep the event loop alive: it resumes stdin to wait + // for the user to confirm the browser import without pausing it again, and its Firefox flow starts + // an HTTP server that is never closed. The latter runs unconditionally on Windows, so the process + // would otherwise hang here on every run. All work is done at this point, so exit explicitly. + process.exit(0); +} + +export default certificateCommand; diff --git a/packages/cli/lib/cli/commands/serve.js b/packages/cli/lib/cli/commands/serve.js index ffc7fdb0cce..92f5dcabcce 100644 --- a/packages/cli/lib/cli/commands/serve.js +++ b/packages/cli/lib/cli/commands/serve.js @@ -1,8 +1,7 @@ -import path from "node:path"; -import os from "node:os"; import process from "node:process"; import baseMiddleware from "../middlewares/base.js"; import {applyProjectConfigOptions, applyWorkspaceOptions, applyBuildOptions, dedupeArray} from "../options.js"; +import {getUi5DataDirOrDefault, resolveServerCertificatePaths, formatPath} from "../../dataDir.js"; import {getLogger} from "@ui5/logger"; const log = getLogger("cli:commands:serve"); @@ -54,12 +53,12 @@ serve.builder = function(cli) { }) .option("key", { describe: "Path to the private key", - default: path.join(os.homedir(), ".ui5", "server", "server.key"), + defaultDescription: "~/.ui5/server/server.key", type: "string" }) .option("cert", { describe: "Path to the certificate", - default: path.join(os.homedir(), ".ui5", "server", "server.crt"), + defaultDescription: "~/.ui5/server/server.crt", type: "string" }) .option("sap-csp-policies", { @@ -231,10 +230,35 @@ serve.handler = async function(argv) { }; if (serverConfig.https) { - const {getSslCertificate} = await import("@ui5/server/internal/sslUtil"); - const {key, cert} = await getSslCertificate(serverConfig.key, serverConfig.cert); - serverConfig.key = key; - serverConfig.cert = cert; + // A default certificate path is only needed for HTTPS, so the UI5 data directory is + // resolved once here rather than for every serve invocation. + const ui5DataDir = await getUi5DataDirOrDefault({cwd: process.cwd()}); + const {keyPath, certPath} = resolveServerCertificatePaths(ui5DataDir, { + keyPath: serverConfig.key, + certPath: serverConfig.cert, + }); + + const {getSslCertificate, SslCertificateNotFoundError} = await import("@ui5/server/internal/sslUtil"); + try { + const {key, cert} = await getSslCertificate(keyPath, certPath); + serverConfig.key = key; + serverConfig.cert = cert; + } catch (err) { + if (err instanceof SslCertificateNotFoundError) { + const keyOrigin = serverConfig.key ? "--key" : "default location"; + const certOrigin = serverConfig.cert ? "--cert" : "default location"; + throw new Error( + `Failed to find required SSL certificate for launching the HTTPS server.\n` + + `Looked for:\n` + + ` Private key: ${formatPath(keyPath)} (${keyOrigin})\n` + + ` Certificate: ${formatPath(certPath)} (${certOrigin})\n` + + `To fix this, either:\n` + + ` • Run "ui5 certificate generate" to create and install one (recommended), or\n` + + ` • Pass existing files with --key and --cert` + ); + } + throw err; + } } const {promise: pOnError, reject} = Promise.withResolvers(); diff --git a/packages/cli/lib/dataDir.js b/packages/cli/lib/dataDir.js index 11b9c2ea1e3..fae3a322e51 100644 --- a/packages/cli/lib/dataDir.js +++ b/packages/cli/lib/dataDir.js @@ -36,6 +36,23 @@ export async function getUi5DataDirOrDefault({cwd}) { return (await getUi5DataDir({cwd})) ?? path.join(os.homedir(), ".ui5"); } +/** + * Resolves the paths of the server's private key and certificate. Explicit paths take precedence + * over the default paths within the given UI5 data directory. + * + * @param {string} ui5DataDir Absolute path to the UI5 data directory + * @param {object} [options] + * @param {string} [options.keyPath] Explicit private-key path + * @param {string} [options.certPath] Explicit certificate path + * @returns {{keyPath: string, certPath: string}} Private-key and certificate paths + */ +export function resolveServerCertificatePaths(ui5DataDir, {keyPath, certPath} = {}) { + return { + keyPath: keyPath ?? path.join(ui5DataDir, "server", "server.key"), + certPath: certPath ?? path.join(ui5DataDir, "server", "server.crt"), + }; +} + /** * Shortens an absolute path for display by replacing the user's home directory with * ~ (e.g. ~/.ui5). Intended for console and diff --git a/packages/cli/test/lib/cli/commands/cache.js b/packages/cli/test/lib/cli/commands/cache.js index 713cd4c789d..c5ac660e9c8 100644 --- a/packages/cli/test/lib/cli/commands/cache.js +++ b/packages/cli/test/lib/cli/commands/cache.js @@ -43,7 +43,8 @@ test.beforeEach(async (t) => { t.context.argv = getDefaultArgv(); t.context.stderrWriteStub = sinon.stub(process.stderr, "write"); - // Prevent real env var from leaking into tests + // Tests rely on not having UI5_DATA_DIR defined + t.context.originalUi5DataDirEnv = process.env.UI5_DATA_DIR; delete process.env.UI5_DATA_DIR; t.context.getUi5DataDirOrDefaultStub = sinon.stub().resolves(TEST_UI5_DATA_DIR); @@ -91,7 +92,11 @@ test.afterEach.always((t) => { sinon.restore(); esmock.purge(t.context.cache); process.exitCode = undefined; - delete process.env.UI5_DATA_DIR; + if (typeof t.context.originalUi5DataDirEnv === "undefined") { + delete process.env.UI5_DATA_DIR; + } else { + process.env.UI5_DATA_DIR = t.context.originalUi5DataDirEnv; + } }); // ─── Command structure ────────────────────────────────────────────────────── diff --git a/packages/cli/test/lib/cli/commands/certificate.js b/packages/cli/test/lib/cli/commands/certificate.js new file mode 100644 index 00000000000..2c50262bed7 --- /dev/null +++ b/packages/cli/test/lib/cli/commands/certificate.js @@ -0,0 +1,259 @@ +import test from "ava"; +import path from "node:path"; +import os from "node:os"; +import sinon from "sinon"; +import esmock from "esmock"; + +function getDefaultArgv() { + return { + "_": ["certificate", "generate"], + "loglevel": "info", + "log-level": "info", + "logLevel": "info", + "perf": false, + "silent": false, + "force": false, + "$0": "ui5" + }; +} + +// Anchored outside the home directory so the plain-path assertions below are not affected by +// the ~ shortening applied to home-dir paths. See the dedicated shortening test for that behavior. +const TEST_UI5_DATA_DIR = path.join(path.resolve(path.sep), "test-ui5-home"); +const TEST_KEY_PATH = path.join(TEST_UI5_DATA_DIR, "server", "server.key"); +const TEST_CERT_PATH = path.join(TEST_UI5_DATA_DIR, "server", "server.crt"); + +test.beforeEach(async (t) => { + t.context.argv = getDefaultArgv(); + + t.context.consoleOutput = ""; + t.context.stderrWriteStub = sinon.stub(process.stderr, "write").callsFake((message) => { + t.context.consoleOutput += message; + return true; + }); + + // generateSslCertificate leaves event-loop handles open (see handleGenerate); the handler exits + // explicitly. Stub process.exit so the test runner is not terminated. + t.context.processExitStub = sinon.stub(process, "exit"); + + // Tests rely on not having UI5_DATA_DIR defined + t.context.originalUi5DataDirEnv = process.env.UI5_DATA_DIR; + delete process.env.UI5_DATA_DIR; + + t.context.getUi5DataDirStub = sinon.stub().resolves(TEST_UI5_DATA_DIR); + t.context.generateSslCertificateStub = sinon.stub().resolves({ + key: "generated-key", + cert: "generated-cert", + keyPath: TEST_KEY_PATH, + certPath: TEST_CERT_PATH + }); + // By default, no certificate exists yet + t.context.existsStub = sinon.stub().resolves(false); + + t.context.certificate = await esmock.p("../../../../lib/cli/commands/certificate.js", { + "@ui5/server/internal/sslUtil": { + generateSslCertificate: t.context.generateSslCertificateStub + } + }, { + "../../../../lib/dataDir.js": { + getUi5DataDirOrDefault: t.context.getUi5DataDirStub + }, + "../../../../lib/utils/fsHelper.js": { + exists: t.context.existsStub + } + }); +}); + +test.afterEach.always((t) => { + sinon.restore(); + esmock.purge(t.context.certificate); + if (typeof t.context.originalUi5DataDirEnv === "undefined") { + delete process.env.UI5_DATA_DIR; + } else { + process.env.UI5_DATA_DIR = t.context.originalUi5DataDirEnv; + } +}); + +test.serial("certificate command structure", (t) => { + const {certificate} = t.context; + t.is(certificate.command, "certificate"); + t.truthy(certificate.describe); + t.is(typeof certificate.builder, "function"); +}); + +test.serial("ui5 certificate generate: creates certificate in resolved data dir", async (t) => { + const {argv, generateSslCertificateStub} = t.context; + + await runGenerate(t, argv); + + t.is(generateSslCertificateStub.callCount, 1); + t.deepEqual(generateSslCertificateStub.getCall(0).args, [TEST_KEY_PATH, TEST_CERT_PATH]); + t.true(t.context.consoleOutput.includes(TEST_UI5_DATA_DIR), + "Resolved UI5 data dir is printed"); + t.true(t.context.consoleOutput.includes(TEST_KEY_PATH), "Private key path is printed"); + t.true(t.context.consoleOutput.includes(TEST_CERT_PATH), "Certificate path is printed"); +}); + +test.serial("ui5 certificate generate: honors data dir from getUi5DataDirOrDefault", async (t) => { + const {argv, getUi5DataDirStub, generateSslCertificateStub} = t.context; + getUi5DataDirStub.resolves(path.resolve("custom-data-dir")); + + await runGenerate(t, argv); + + const expectedKey = path.join(path.resolve("custom-data-dir"), "server", "server.key"); + const expectedCert = path.join(path.resolve("custom-data-dir"), "server", "server.crt"); + t.deepEqual(generateSslCertificateStub.getCall(0).args, [expectedKey, expectedCert]); +}); + +test.serial("ui5 certificate generate: uses ~/.ui5 fallback provided by getUi5DataDirOrDefault", async (t) => { + const {argv, getUi5DataDirStub, generateSslCertificateStub} = t.context; + getUi5DataDirStub.resolves(path.join(os.homedir(), ".ui5")); + + await runGenerate(t, argv); + + const expectedKey = path.join(os.homedir(), ".ui5", "server", "server.key"); + const expectedCert = path.join(os.homedir(), ".ui5", "server", "server.crt"); + t.deepEqual(generateSslCertificateStub.getCall(0).args, [expectedKey, expectedCert]); +}); + +test.serial("ui5 certificate generate: shortens home-dir paths in output with ~", async (t) => { + const {argv, getUi5DataDirStub, generateSslCertificateStub} = t.context; + const dataDir = path.join(os.homedir(), ".ui5"); + const keyPath = path.join(dataDir, "server", "server.key"); + const certPath = path.join(dataDir, "server", "server.crt"); + getUi5DataDirStub.resolves(dataDir); + generateSslCertificateStub.resolves({key: "k", cert: "c", keyPath, certPath}); + + await runGenerate(t, argv); + + const shortenedDataDir = "~" + dataDir.slice(os.homedir().length); + const shortenedKey = "~" + keyPath.slice(os.homedir().length); + const shortenedCert = "~" + certPath.slice(os.homedir().length); + t.true(t.context.consoleOutput.includes(shortenedDataDir), "Data dir is printed with ~"); + t.true(t.context.consoleOutput.includes(shortenedKey), "Key path is printed with ~"); + t.true(t.context.consoleOutput.includes(shortenedCert), "Certificate path is printed with ~"); + t.false(t.context.consoleOutput.includes(os.homedir()), "Full home directory is not printed"); +}); + +test.serial("ui5 certificate generate: uses --key and --cert options", async (t) => { + const {argv, generateSslCertificateStub} = t.context; + argv.key = "/custom/my.key"; + argv.cert = "/custom/my.crt"; + + await runGenerate(t, argv); + + t.deepEqual(generateSslCertificateStub.getCall(0).args, ["/custom/my.key", "/custom/my.crt"]); +}); + +test.serial("ui5 certificate generate: exits explicitly so leaked handles cannot hang the process", async (t) => { + const {argv, processExitStub} = t.context; + + await runGenerate(t, argv); + + // devcert-sanscache leaves stdin resumed and an HTTP server open (unconditionally on Windows), + // keeping the event loop alive. The handler must force a clean exit after generation. + t.is(processExitStub.callCount, 1, "process.exit is called after generation"); + t.deepEqual(processExitStub.getCall(0).args, [0], "Exits with code 0"); +}); + +test.serial("ui5 certificate generate: skips generation when certificate exists", async (t) => { + const {argv, existsStub, generateSslCertificateStub, processExitStub} = t.context; + existsStub.resolves(true); // both key and cert exist + + await runGenerate(t, argv); + + t.is(generateSslCertificateStub.callCount, 0, "Does not generate a new certificate"); + t.is(processExitStub.callCount, 0, "Does not force an exit when nothing was generated"); + t.true(t.context.consoleOutput.includes("already exists"), + "Reports that a certificate already exists"); + t.true(t.context.consoleOutput.includes("--force"), "Suggests --force to overwrite"); + t.true(t.context.consoleOutput.includes(TEST_KEY_PATH), "Prints existing key path"); +}); + +test.serial("ui5 certificate generate: regenerates when only the key exists", async (t) => { + const {argv, existsStub, generateSslCertificateStub} = t.context; + existsStub.withArgs(TEST_KEY_PATH).resolves(true); // only the key exists + + await runGenerate(t, argv); + + // A lone key is a broken pair. Without --force the user could otherwise never complete it via + // the plain command, so generation proceeds (writeCertificateFile overwrites the leftover key). + t.is(generateSslCertificateStub.callCount, 1, + "Regenerates to complete a broken pair"); + t.false(t.context.consoleOutput.includes("already exists"), + "Does not report an existing certificate for a partial pair"); +}); + +test.serial("ui5 certificate generate: regenerates when only the certificate exists", async (t) => { + const {argv, existsStub, generateSslCertificateStub} = t.context; + existsStub.withArgs(TEST_CERT_PATH).resolves(true); // only the cert exists + + await runGenerate(t, argv); + + t.is(generateSslCertificateStub.callCount, 1, + "Regenerates to complete a broken pair"); + t.false(t.context.consoleOutput.includes("already exists"), + "Does not report an existing certificate for a partial pair"); +}); + +test.serial("ui5 certificate generate --force: regenerates despite existing certificate", async (t) => { + const {argv, existsStub, generateSslCertificateStub} = t.context; + existsStub.resolves(true); // certificate exists + argv.force = true; + + await runGenerate(t, argv); + + t.is(generateSslCertificateStub.callCount, 1, "Generates a new certificate"); + t.deepEqual(generateSslCertificateStub.getCall(0).args, [TEST_KEY_PATH, TEST_CERT_PATH]); +}); + +test.serial("ui5 certificate generate: wraps errors from the existence check with context", async (t) => { + const {argv, existsStub, generateSslCertificateStub} = t.context; + existsStub.rejects(Object.assign(new Error("permission denied"), {code: "EACCES"})); + + const err = await t.throwsAsync(runGenerate(t, argv)); + t.true(err.message.includes("Failed to check for an existing server certificate"), + "Error explains it originated from the existence pre-check"); + t.true(err.message.includes("permission denied"), "Error preserves the underlying message"); + t.is(err.cause.code, "EACCES", "Original error is retained as cause"); + t.is(generateSslCertificateStub.callCount, 0, "Does not attempt generation after a failed check"); +}); + +test.serial("ui5 certificate generate: prints trust-store notice before generating", async (t) => { + const {argv} = t.context; + + await runGenerate(t, argv); + + t.true( + t.context.consoleOutput.includes("importing the newly created") && + t.context.consoleOutput.includes("operating system and browsers"), + "Prints notice about installing the certificate into the trust store" + ); +}); + +/** + * Resolves and invokes the "generate" subcommand handler by driving the command's builder + * with a fake yargs instance that captures the registered subcommand. + * + * @param {object} t AVA test context + * @param {object} argv Arguments to pass to the handler + * @returns {Promise} + */ +async function runGenerate(t, argv) { + const {certificate} = t.context; + let generateConfig; + const fakeYargs = { + demandCommand() { + return this; + }, + command(name, describe, config) { + if (name === "generate") { + generateConfig = config; + } + return this; + } + }; + certificate.builder(fakeYargs); + t.truthy(generateConfig, "generate subcommand is registered"); + await generateConfig.handler(argv); +} diff --git a/packages/cli/test/lib/cli/commands/serve.js b/packages/cli/test/lib/cli/commands/serve.js index b4506c4b6b8..c541dda288c 100644 --- a/packages/cli/test/lib/cli/commands/serve.js +++ b/packages/cli/test/lib/cli/commands/serve.js @@ -4,6 +4,10 @@ import sinon from "sinon"; import esmock from "esmock"; import yargs from "yargs"; +const DEFAULT_UI5_DATA_DIR = path.join(path.resolve(path.sep), "home", ".ui5"); +const DEFAULT_SERVER_KEY_PATH = path.join(DEFAULT_UI5_DATA_DIR, "server", "server.key"); +const DEFAULT_SERVER_CERT_PATH = path.join(DEFAULT_UI5_DATA_DIR, "server", "server.crt"); + function getDefaultArgv() { // This has been taken from the actual argv object yargs provides return { @@ -18,8 +22,10 @@ function getDefaultArgv() { "simpleIndex": false, "accept-remote-connections": false, "acceptRemoteConnections": false, - "key": "/home/.ui5/server/server.key", - "cert": "/home/.ui5/server/server.crt", + // yargs leaves these undefined unless the user passes --key/--cert + // (the options declare only a defaultDescription, not a default value) + "key": undefined, + "cert": undefined, "sap-csp-policies": false, "sapCspPolicies": false, "serve-csp-reports": false, @@ -52,7 +58,16 @@ test.beforeEach(async (t) => { }) }; t.context.sslUtil = { - getSslCertificate: sinon.stub().resolves() + getSslCertificate: sinon.stub().resolves(), + SslCertificateNotFoundError: class SslCertificateNotFoundError extends Error { + constructor(keyPath, certPath) { + super(`No SSL certificate found at ${keyPath} and ${certPath}`); + this.name = "SslCertificateNotFoundError"; + this.code = "SSL_CERTIFICATE_NOT_FOUND"; + this.keyPath = keyPath; + this.certPath = certPath; + } + } }; t.context.getServerSettings = sinon.stub().returns({}); @@ -84,12 +99,18 @@ test.beforeEach(async (t) => { t.context.open = sinon.stub(); + t.context.getUi5DataDir = sinon.stub().resolves(DEFAULT_UI5_DATA_DIR); + t.context.serve = await esmock.p("../../../../lib/cli/commands/serve.js", { "@ui5/server": t.context.server, "@ui5/server/internal/sslUtil": t.context.sslUtil, "@ui5/project/graph": t.context.graph, "@ui5/project/internal/graph/ProjectDefinitionWatcher": t.context.projectWatcher, "open": t.context.open + }, { + "../../../../lib/dataDir.js": { + getUi5DataDirOrDefault: t.context.getUi5DataDir + } }); }); @@ -195,11 +216,84 @@ test.serial("ui5 serve --https", async (t) => { t.is(sslUtil.getSslCertificate.callCount, 1); t.deepEqual(sslUtil.getSslCertificate.getCall(0).args, [ - "/home/.ui5/server/server.key", - "/home/.ui5/server/server.crt" + DEFAULT_SERVER_KEY_PATH, + DEFAULT_SERVER_CERT_PATH ]); }); +test.serial("ui5 serve --https without existing certificate", async (t) => { + const {argv, serve, server, sslUtil} = t.context; + + sslUtil.getSslCertificate.rejects( + new sslUtil.SslCertificateNotFoundError( + DEFAULT_SERVER_KEY_PATH, + DEFAULT_SERVER_CERT_PATH + ) + ); + + argv.https = true; + + const err = await t.throwsAsync(serve.handler(argv)); + t.regex(err.message, /Failed to find required SSL certificate for launching the HTTPS server/); + t.regex(err.message, /Private key: .*server\.key \(default location\)/); + t.regex(err.message, /Certificate: .*server\.crt \(default location\)/); + t.regex(err.message, /ui5 certificate generate/); + t.regex(err.message, /--key and --cert/); + + // The server must not be started when no certificate is available + t.is(server.serve.callCount, 0); +}); + +test.serial("ui5 serve --https without existing certificate at custom --key/--cert paths", async (t) => { + const {argv, serve, server, sslUtil} = t.context; + + sslUtil.getSslCertificate.rejects( + new sslUtil.SslCertificateNotFoundError( + "/custom/my.key", + "/custom/my.crt" + ) + ); + + argv.https = true; + argv.key = "/custom/my.key"; + argv.cert = "/custom/my.crt"; + + const err = await t.throwsAsync(serve.handler(argv)); + t.regex(err.message, /Failed to find required SSL certificate for launching the HTTPS server/); + t.regex(err.message, /Private key: \/custom\/my\.key \(--key\)/); + t.regex(err.message, /Certificate: \/custom\/my\.crt \(--cert\)/); + t.regex(err.message, /ui5 certificate generate/); + + // A path the user did not specify must not be attributed to them + t.notRegex(err.message, /\(default\)/); + + // The server must not be started when no certificate is available + t.is(server.serve.callCount, 0); +}); + +test.serial("ui5 serve --https without existing certificate at mixed custom/default paths", async (t) => { + const {argv, serve, server, sslUtil} = t.context; + + sslUtil.getSslCertificate.rejects( + new sslUtil.SslCertificateNotFoundError( + "/custom/my.key", + DEFAULT_SERVER_CERT_PATH + ) + ); + + argv.https = true; + argv.key = "/custom/my.key"; + + const err = await t.throwsAsync(serve.handler(argv)); + // The user-supplied key is attributed to --key; the fallback cert path is marked as a default, + // never misattributed as something the user specified. + t.regex(err.message, /Private key: \/custom\/my\.key \(--key\)/); + t.regex(err.message, /Certificate: .*server\.crt \(default location\)/); + + // The server must not be started when no certificate is available + t.is(server.serve.callCount, 0); +}); + test.serial("ui5 serve --accept-remote-connections", async (t) => { const {argv, serve, server, fakeGraph} = t.context; diff --git a/packages/cli/test/lib/dataDir.js b/packages/cli/test/lib/dataDir.js index a8ca6c40136..25420b18205 100644 --- a/packages/cli/test/lib/dataDir.js +++ b/packages/cli/test/lib/dataDir.js @@ -97,6 +97,32 @@ test.serial("getUi5DataDirOrDefault: falls back to ~/.ui5 when no value defined" t.is(result, path.join(os.homedir(), ".ui5")); }); +test.serial("resolveServerCertificatePaths: returns default paths", (t) => { + const {dataDir} = t.context; + const ui5DataDir = path.join(path.resolve(path.sep), "ui5-data-dir"); + + const result = dataDir.resolveServerCertificatePaths(ui5DataDir); + + t.deepEqual(result, { + keyPath: path.join(ui5DataDir, "server", "server.key"), + certPath: path.join(ui5DataDir, "server", "server.crt"), + }); +}); + +test.serial("resolveServerCertificatePaths: explicit paths override defaults", (t) => { + const {dataDir} = t.context; + const ui5DataDir = path.join(path.resolve(path.sep), "ui5-data-dir"); + + const result = dataDir.resolveServerCertificatePaths(ui5DataDir, { + keyPath: path.join(path.resolve(path.sep), "custom", "server.key"), + }); + + t.deepEqual(result, { + keyPath: path.join(path.resolve(path.sep), "custom", "server.key"), + certPath: path.join(ui5DataDir, "server", "server.crt"), + }); +}); + test.serial("formatPath: replaces home directory with ~", (t) => { const {dataDir} = t.context; diff --git a/packages/server/lib/sslUtil.js b/packages/server/lib/sslUtil.js index d17a6c8ffda..c9c1115b004 100644 --- a/packages/server/lib/sslUtil.js +++ b/packages/server/lib/sslUtil.js @@ -1,5 +1,4 @@ -import os from "node:os"; -import {stat, readFile, writeFile, mkdir, chmod, constants} from "node:fs/promises"; +import {stat, readFile, writeFile, mkdir, chmod, rm, constants} from "node:fs/promises"; import path from "node:path"; import {getLogger} from "@ui5/logger"; @@ -11,20 +10,36 @@ const log = getLogger("server:sslUtil"); */ /** - * Creates a new SSL certificate or validates an existing one. + * Error thrown by {@link getSslCertificate} when no SSL certificate could be found. + * The offending paths are exposed via keyPath and certPath so that + * callers can render actionable guidance. + * + * @private + */ +export class SslCertificateNotFoundError extends Error { + constructor(keyPath, certPath) { + super(`No SSL certificate found at ${keyPath} and ${certPath}`); + this.name = "SslCertificateNotFoundError"; + this.code = "SSL_CERTIFICATE_NOT_FOUND"; + this.keyPath = keyPath; + this.certPath = certPath; + } +} + +/** + * Reads and validates an existing SSL certificate. + * + * Does not create a certificate if none is found. Use + * {@link generateSslCertificate} to create one. * * @private * @static - * @param {string} [keyPath=$HOME/.ui5/server/server.key] Path to private key to be used for https. - * Defaults to $HOME/.ui5/server/server.key - * @param {string} [certPath=$HOME/.ui5/server/server.crt] Path to certificate to be used for for https. - * Defaults to $HOME/.ui5/server/server.crt + * @param {string} keyPath Path to the private key to be used for https + * @param {string} certPath Path to the certificate to be used for https * @returns {Promise} Resolves with an sslObject containing cert and key + * @throws {SslCertificateNotFoundError} If the private key or certificate is missing */ -export function getSslCertificate( - keyPath = path.join(os.homedir(), ".ui5/server/server.key"), - certPath = path.join(os.homedir(), ".ui5/server/server.crt") -) { +export function getSslCertificate(keyPath, certPath) { // checks the certificates if they are present return Promise.all([ fileExists(keyPath).then(async (statsOrFalse) => { @@ -51,7 +66,7 @@ export function getSslCertificate( } if (statsOrFalse.mode & constants.S_IWUSR || statsOrFalse.mode & constants.S_IROTH) { - log.verbose(`Detected outdated file permissions for certificate file at ${keyPath}. ` + + log.verbose(`Detected outdated file permissions for certificate file at ${certPath}. ` + `Fixing permissions...`); await chmod(certPath, 0o400).catch((err) => { log.error(`Failed to update permissions of certificate file at ${certPath}: ${err}`); @@ -60,52 +75,53 @@ export function getSslCertificate( return readFile(certPath); }) ]).then(function([key, cert]) { - if (key && cert) { + // A leftover empty file (e.g. from an interrupted previous write) reads as a truthy but + // zero-length Buffer. Treat it as missing so callers get actionable guidance instead of + // starting HTTPS with empty TLS material. + if (key?.length && cert?.length) { return {key, cert}; } - return createAndInstallCertificate(keyPath, certPath); + throw new SslCertificateNotFoundError(keyPath, certPath); }); } - -async function createAndInstallCertificate(keyPath, certPath) { - const {default: yesno} = await import("yesno"); - - const ok = await yesno({ - question: "No SSL certificates found. " + - "Do you want to create new SSL certificates and install them locally? (yes)", - defaultValue: true - }); - - if (!ok) { - throw new Error("Certificate installation aborted! Please install the SSL certificate manually."); - } - - // In case certificate is not found, create a self-signed one and put it into the user's trust store +/** + * Creates a new self-signed SSL certificate, installs it into the operating system's + * trust store and writes the key and certificate to the given paths. + * + * Installing the certificate into the trust store requires elevated privileges. On most + * platforms this triggers a system password prompt; on Windows a confirmation dialog is + * shown. Callers are responsible for informing the user about this beforehand. + * + * @private + * @static + * @param {string} keyPath Path the private key is written to + * @param {string} certPath Path the certificate is written to + * @returns {Promise} Resolves with an object containing the created key and + * cert as well as the keyPath and certPath + * they were written to + */ +export async function generateSslCertificate(keyPath, certPath) { + // Create a self-signed certificate and put it into the user's trust store const {default: devCert} = await import("devcert-sanscache"); - // Inform end user about entering his root password (needed for importing - // the created certificate into the system) - // TODO: Prompt and logging should happen in CLI module rather than server - if (process.platform === "win32") { - process.stderr.write("Please press allow in the opened dialog to confirm importing the newly created " + - "SSL certificate into the operating system and browsers."); - process.stderr.write("\n"); - } else { - process.stderr.write("Please enter your root password to allow importing the newly created " + - "SSL certificate into the operating system and browsers."); - process.stderr.write("\n"); - } - - const {key, cert} = await devCert("UI5Tooling"); + const {key, cert} = await devCert("UI5CLI"); await Promise.all([ // Write certificates to the ui5 certificate folder // such that they are used by default upon next startup - mkdir(path.dirname(keyPath), {recursive: true}).then(() => writeFile(keyPath, key, {mode: 0o400})), - mkdir(path.dirname(certPath), {recursive: true}).then(() => writeFile(certPath, cert, {mode: 0o400})) + writeCertificateFile(keyPath, key), + writeCertificateFile(certPath, cert) ]); - return {key, cert}; + return {key, cert, keyPath, certPath}; +} + +// Files are written with read-only permissions (0o400), so an existing file from a previous run +// cannot be opened for writing. Remove it first to allow regeneration (e.g. via --force). +async function writeCertificateFile(filePath, content) { + await mkdir(path.dirname(filePath), {recursive: true}); + await rm(filePath, {force: true}); + await writeFile(filePath, content, {mode: 0o400}); } function fileExists(filePath) { diff --git a/packages/server/package.json b/packages/server/package.json index cc61c669946..e1743ecfcfa 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -101,8 +101,7 @@ "parseurl": "^1.3.3", "portscanner": "^2.2.0", "router": "^2.2.0", - "ws": "^8.21.3", - "yesno": "^0.4.0" + "ws": "^8.21.3" }, "devDependencies": { "@istanbuljs/esm-loader-hook": "^0.3.0", diff --git a/packages/server/test/lib/server/sslUtil.js b/packages/server/test/lib/server/sslUtil.js index cb151365172..ebea8c91470 100644 --- a/packages/server/test/lib/server/sslUtil.js +++ b/packages/server/test/lib/server/sslUtil.js @@ -18,13 +18,11 @@ function fileExists(filePath) { } test.beforeEach(async (t) => { - t.context.yesno = sinon.stub(); t.context.devcertSanscache = sinon.stub(); t.context.mkdir = sinon.stub().resolves(); t.context.createSslUtilMock = async (mockMkdir = false) => { const mocks = { - "yesno": t.context.yesno, "devcert-sanscache": t.context.devcertSanscache }; if (mockMkdir) { @@ -38,7 +36,9 @@ test.beforeEach(async (t) => { }); test.afterEach.always((t) => { - esmock.purge(t.context.sslUtil); + if (t.context.sslUtil) { + esmock.purge(t.context.sslUtil); + } }); test("Get existing certificate", async (t) => { @@ -78,27 +78,54 @@ test("Get existing certificate with outdated permissions triggers chmod error ha t.true(chmodStub.calledTwice, "chmod was called for both key and cert"); }); -test.serial("Create new certificate and install it", async (t) => { - const {createSslUtilMock, yesno, devcertSanscache} = t.context; +test("Get missing certificate throws SslCertificateNotFoundError", async (t) => { + const sslUtil = await esmock("../../../lib/sslUtil.js"); + + const keyPath = "/does/not/exist/server.key"; + const certPath = "/does/not/exist/server.crt"; + const err = await t.throwsAsync(sslUtil.getSslCertificate(keyPath, certPath), { + instanceOf: sslUtil.SslCertificateNotFoundError + }); + t.is(err.code, "SSL_CERTIFICATE_NOT_FOUND", "Error carries a recognizable code"); + t.is(err.keyPath, keyPath, "Error exposes the key path"); + t.is(err.certPath, certPath, "Error exposes the cert path"); +}); + +test("Get certificate with empty key or cert file throws SslCertificateNotFoundError", async (t) => { + const constants = await import("node:fs").then((m) => m.constants); + const statStub = sinon.stub().resolves({mode: constants.S_IRUSR}); + // A leftover file from an interrupted write reads as a truthy but zero-length Buffer + const readFileStub = sinon.stub(); + readFileStub.withArgs("/fake/path.key").resolves(Buffer.alloc(0)); + readFileStub.withArgs("/fake/path.crt").resolves(Buffer.from("cert-content")); + + const sslUtil = await esmock("../../../lib/sslUtil.js", { + "node:fs/promises": { + stat: statStub, + readFile: readFileStub, + writeFile: sinon.stub(), + mkdir: sinon.stub(), + chmod: sinon.stub().resolves(), + constants, + } + }); + + await t.throwsAsync(sslUtil.getSslCertificate("/fake/path.key", "/fake/path.crt"), { + instanceOf: sslUtil.SslCertificateNotFoundError + }, "An empty key file is treated as missing"); +}); + +test.serial("Generate new certificate and install it", async (t) => { + const {createSslUtilMock, devcertSanscache} = t.context; const sslUtil = await createSslUtilMock(); - t.plan(6); + t.plan(5); const sslKey = "abcd"; const sslCert = "defg"; - yesno.callsFake(async function(options) { - t.deepEqual(options, { - question: "No SSL certificates found. " + - "Do you want to create new SSL certificates and install them locally? (yes)", - defaultValue: true - }, "Pass options to yesno"); - - return true; - }); - devcertSanscache.callsFake(function(name) { - t.is(name, "UI5Tooling", "Create certificate for UI5Tooling."); + t.is(name, "UI5CLI", "Create certificate for UI5CLI."); return Promise.resolve({ key: sslKey, cert: sslCert @@ -110,7 +137,7 @@ test.serial("Create new certificate and install it", async (t) => { const sslPathKey = path.join(sslPath, "someOtherServer1.key"); const sslPathCert = path.join(sslPath, "someOtherServer1.crt"); - const result = await sslUtil.getSslCertificate(sslPathKey, sslPathCert); + const result = await sslUtil.generateSslCertificate(sslPathKey, sslPathCert); t.deepEqual(result.key, sslKey, "Key should be returned"); t.deepEqual(result.cert, sslCert, "Cert should be returned"); @@ -123,68 +150,60 @@ test.serial("Create new certificate and install it", async (t) => { t.is(fileExistsResult[1], true, "Cert was created."); }); -test.serial("Create new certificate and do not install it", async (t) => { - const {createSslUtilMock, yesno} = t.context; +test.serial("Generate new certificate overwrites an existing read-only certificate", async (t) => { + const {createSslUtilMock, devcertSanscache} = t.context; const sslUtil = await createSslUtilMock(); - t.plan(2); + devcertSanscache.resolves({key: "new-key", cert: "new-cert"}); - yesno.callsFake(async function(options) { - t.deepEqual(options, { - question: "No SSL certificates found. " + - "Do you want to create new SSL certificates and install them locally? (yes)", - defaultValue: true - }, "Pass options to yesno"); + const sslPath = path.join(process.cwd(), "./test/tmp/ssl/"); + const sslPathKey = path.join(sslPath, "existingServer.key"); + const sslPathCert = path.join(sslPath, "existingServer.crt"); - return false; - }); + // Simulate a certificate from a previous run: written with read-only permissions (0o400), + // which would otherwise cause EACCES when opened for writing. + await promisify(fs.mkdir)(sslPath, {recursive: true}); + await promisify(fs.writeFile)(sslPathKey, "old-key", {mode: 0o400}); + await promisify(fs.writeFile)(sslPathCert, "old-cert", {mode: 0o400}); - const sslPath = path.join(process.cwd(), "./test/tmp/ssl/"); - const sslPathKey = path.join(sslPath, "someOtherServer2.key"); - const sslPathCert = path.join(sslPath, "someOtherServer2.crt"); - const result = sslUtil.getSslCertificate(sslPathKey, sslPathCert); - return result.catch((error) => { - t.is( - error.message, - "Certificate installation aborted! Please install the SSL certificate manually.", - "Certificate install aborted." - ); - }); + await t.notThrowsAsync(sslUtil.generateSslCertificate(sslPathKey, sslPathCert), + "Regeneration succeeds despite read-only existing files"); + + const readFile = promisify(fs.readFile); + t.is((await readFile(sslPathKey)).toString(), "new-key", "Key was overwritten"); + t.is((await readFile(sslPathCert)).toString(), "new-cert", "Cert was overwritten"); }); -test.serial("Create new certificate not succeeded", async (t) => { - const {createSslUtilMock, yesno, devcertSanscache, mkdir} = t.context; - const sslUtil = await createSslUtilMock(true); +test.serial("Generate new certificate reports the written paths", async (t) => { + const {createSslUtilMock, devcertSanscache} = t.context; + const sslUtil = await createSslUtilMock(); - t.plan(6); + devcertSanscache.resolves({key: "k", cert: "c"}); - yesno.callsFake(async function(options) { - t.deepEqual(options, { - question: "No SSL certificates found. " + - "Do you want to create new SSL certificates and install them locally? (yes)", - defaultValue: true - }, "Pass options to yesno"); + const sslPath = path.join(process.cwd(), "./test/tmp/ssl/"); + const sslPathKey = path.join(sslPath, "someOtherServer4.key"); + const sslPathCert = path.join(sslPath, "someOtherServer4.crt"); + const result = await sslUtil.generateSslCertificate(sslPathKey, sslPathCert); - return true; - }); + t.is(result.keyPath, sslPathKey, "Returned key path matches"); + t.is(result.certPath, sslPathCert, "Returned cert path matches"); +}); - devcertSanscache.callsFake(async function(name) { - t.is(name, "UI5Tooling", "Create certificate for UI5Tooling."); - return { - key: "aaa", - cert: "bbb" - }; - }); - mkdir.callsFake(async function(dirName) { - t.pass("mkdir mock reached."); +test.serial("Generate new certificate not succeeded", async (t) => { + const {createSslUtilMock, devcertSanscache, mkdir} = t.context; + const sslUtil = await createSslUtilMock(true); - throw new Error("some error"); + devcertSanscache.resolves({ + key: "aaa", + cert: "bbb" }); + mkdir.rejects(new Error("some error")); const sslPath = path.join(process.cwd(), "./test/tmp/ssl/"); const sslPathKey = path.join(sslPath, "someOtherServer3.key"); const sslPathCert = path.join(sslPath, "someOtherServer3.crt"); - const err = await t.throwsAsync(sslUtil.getSslCertificate(sslPathKey, sslPathCert)); + const err = await t.throwsAsync(sslUtil.generateSslCertificate(sslPathKey, sslPathCert)); t.is(err.message, "some error", "Correct error thrown"); + t.is(devcertSanscache.firstCall.args[0], "UI5CLI", "Certificate created for UI5CLI"); + t.true(mkdir.called, "mkdir was attempted"); }); -