diff --git a/lib/vscode/.eslintignore b/lib/vscode/.eslintignore index be2d47d3..b67a8161 100644 --- a/lib/vscode/.eslintignore +++ b/lib/vscode/.eslintignore @@ -19,3 +19,4 @@ # These are code-server code symlinks. src/vs/base/node/proxy_agent.ts src/vs/ipc.d.ts +src/vs/server/common/util.ts diff --git a/lib/vscode/src/vs/server/browser/client.ts b/lib/vscode/src/vs/server/browser/client.ts index 26b709fa..c1c5b450 100644 --- a/lib/vscode/src/vs/server/browser/client.ts +++ b/lib/vscode/src/vs/server/browser/client.ts @@ -1,8 +1,10 @@ import * as path from 'vs/base/common/path'; -import { URI } from 'vs/base/common/uri'; import { Options } from 'vs/ipc'; import { localize } from 'vs/nls'; +import { MenuId, MenuRegistry } from 'vs/platform/actions/common/actions'; +import { CommandsRegistry } from 'vs/platform/commands/common/commands'; import { Extensions, IConfigurationRegistry } from 'vs/platform/configuration/common/configurationRegistry'; +import { ContextKeyExpr, IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; import { registerSingleton } from 'vs/platform/instantiation/common/extensions'; import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection'; import { ILogService } from 'vs/platform/log/common/log'; @@ -11,10 +13,18 @@ import { Registry } from 'vs/platform/registry/common/platform'; import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; import { TelemetryChannelClient } from 'vs/server/common/telemetry'; +import { getOptions } from 'vs/server/common/util'; import 'vs/workbench/contrib/localizations/browser/localizations.contribution'; import 'vs/workbench/services/localizations/browser/localizationsService'; import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService'; +/** + * All client-side customization to VS Code should live in this file when + * possible. + */ + +const options = getOptions(); + class TelemetryService extends TelemetryChannelClient { public constructor( @IRemoteAgentService remoteAgentService: IRemoteAgentService, @@ -23,26 +33,6 @@ class TelemetryService extends TelemetryChannelClient { } } -/** - * Remove extra slashes in a URL. - */ -export const normalize = (url: string, keepTrailing = false): string => { - return url.replace(/\/\/+/g, '/').replace(/\/+$/, keepTrailing ? '/' : ''); -}; - -/** - * Get options embedded in the HTML. - */ -export const getOptions = (): T => { - try { - return JSON.parse(document.getElementById('coder-options')!.getAttribute('data-settings')!); - } catch (error) { - return {} as T; - } -}; - -const options = getOptions(); - const TELEMETRY_SECTION_ID = 'telemetry'; Registry.as(Extensions.Configuration).registerConfiguration({ 'id': TELEMETRY_SECTION_ID, @@ -173,38 +163,36 @@ export const initialize = async (services: ServiceCollection): Promise => if (theme) { localStorage.setItem('colorThemeData', theme); } -}; -export interface Query { - [key: string]: string | undefined; -} + // Use to show or hide logout commands and menu options. + const contextKeyService = (services.get(IContextKeyService) as IContextKeyService); + contextKeyService.createKey('code-server.authed', options.authed); -/** - * Split a string up to the delimiter. If the delimiter doesn't exist the first - * item will have all the text and the second item will be an empty string. - */ -export const split = (str: string, delimiter: string): [string, string] => { - const index = str.indexOf(delimiter); - return index !== -1 ? [str.substring(0, index).trim(), str.substring(index + 1)] : [str, '']; -}; + // Add a logout command. + const logoutEndpoint = path.join(options.base, '/logout') + `?base=${options.base}`; + const LOGOUT_COMMAND_ID = 'code-server.logout'; + CommandsRegistry.registerCommand( + LOGOUT_COMMAND_ID, + () => { + window.location.href = logoutEndpoint; + }, + ); -/** - * Return the URL modified with the specified query variables. It's pretty - * stupid so it probably doesn't cover any edge cases. Undefined values will - * unset existing values. Doesn't allow duplicates. - */ -export const withQuery = (url: string, replace: Query): string => { - const uri = URI.parse(url); - const query = { ...replace }; - uri.query.split('&').forEach((kv) => { - const [key, value] = split(kv, '='); - if (!(key in query)) { - query[key] = value; - } + // Add logout to command palette. + MenuRegistry.appendMenuItem(MenuId.CommandPalette, { + command: { + id: LOGOUT_COMMAND_ID, + title: localize('logout', "Log out") + }, + when: ContextKeyExpr.has('code-server.authed') + }); + + // Add logout to the (web-only) home menu. + MenuRegistry.appendMenuItem(MenuId.MenubarHomeMenu, { + command: { + id: LOGOUT_COMMAND_ID, + title: localize('logout', "Log out") + }, + when: ContextKeyExpr.has('code-server.authed') }); - return uri.with({ - query: Object.keys(query) - .filter((k) => typeof query[k] !== 'undefined') - .map((k) => `${k}=${query[k]}`).join('&'), - }).toString(true); }; diff --git a/lib/vscode/src/vs/server/common/cookie.ts b/lib/vscode/src/vs/server/common/cookie.ts deleted file mode 100644 index e2720a04..00000000 --- a/lib/vscode/src/vs/server/common/cookie.ts +++ /dev/null @@ -1,3 +0,0 @@ -export enum Cookie { - Key = 'key', -} diff --git a/lib/vscode/src/vs/server/common/util.ts b/lib/vscode/src/vs/server/common/util.ts new file mode 120000 index 00000000..625cfa1c --- /dev/null +++ b/lib/vscode/src/vs/server/common/util.ts @@ -0,0 +1 @@ +../../../../../../src/common/util.ts \ No newline at end of file diff --git a/lib/vscode/src/vs/workbench/browser/parts/titlebar/menubarControl.ts b/lib/vscode/src/vs/workbench/browser/parts/titlebar/menubarControl.ts index 397211f0..bc20eca2 100644 --- a/lib/vscode/src/vs/workbench/browser/parts/titlebar/menubarControl.ts +++ b/lib/vscode/src/vs/workbench/browser/parts/titlebar/menubarControl.ts @@ -9,7 +9,7 @@ import { registerThemingParticipant, IThemeService } from 'vs/platform/theme/com import { MenuBarVisibility, getTitleBarStyle, IWindowOpenable, getMenuBarVisibility } from 'vs/platform/windows/common/windows'; import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; import { IAction, Action, SubmenuAction, Separator } from 'vs/base/common/actions'; -import { addDisposableListener, Dimension, EventType, getCookieValue } from 'vs/base/browser/dom'; +import { addDisposableListener, Dimension, EventType } from 'vs/base/browser/dom'; import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; import { isMacintosh, isWeb, isIOS, isNative } from 'vs/base/common/platform'; import { IConfigurationService, IConfigurationChangeEvent } from 'vs/platform/configuration/common/configuration'; @@ -38,8 +38,6 @@ import { KeyCode } from 'vs/base/common/keyCodes'; import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry'; import { IsWebContext } from 'vs/platform/contextkey/common/contextkeys'; import { ICommandService } from 'vs/platform/commands/common/commands'; -import { ILogService } from 'vs/platform/log/common/log'; -import { Cookie } from 'vs/server/common/cookie'; export type IOpenRecentAction = IAction & { uri: URI, remoteAuthority?: string }; @@ -318,8 +316,7 @@ export class CustomMenubarControl extends MenubarControl { @IThemeService private readonly themeService: IThemeService, @IWorkbenchLayoutService private readonly layoutService: IWorkbenchLayoutService, @IHostService protected readonly hostService: IHostService, - @ICommandService commandService: ICommandService, - @ILogService private readonly logService: ILogService + @ICommandService commandService: ICommandService ) { super(menuService, workspacesService, contextKeyService, keybindingService, configurationService, labelService, updateService, storageService, notificationService, preferencesService, environmentService, accessibilityService, hostService, commandService); @@ -721,28 +718,6 @@ export class CustomMenubarControl extends MenubarControl { webNavigationActions.pop(); } - webNavigationActions.push(new Action('logout', localize('logout', "Log out"), undefined, true, - async (event?: MouseEvent) => { - const COOKIE_KEY = Cookie.Key; - const loginCookie = getCookieValue(COOKIE_KEY); - - this.logService.info('Logging out of code-server'); - - if(loginCookie) { - this.logService.info(`Removing cookie under ${COOKIE_KEY}`); - - if (document && document.cookie) { - // We delete the cookie by setting the expiration to a date/time in the past - document.cookie = COOKIE_KEY +'=; Path=/; Expires=Thu, 01 Jan 1970 00:00:01 GMT;'; - window.location.href = '/login'; - } else { - this.logService.warn('Could not delete cookie because document and/or document.cookie is undefined'); - } - } else { - this.logService.warn('Could not log out because we could not find cookie'); - } - })); - return webNavigationActions; } diff --git a/src/browser/register.ts b/src/browser/register.ts index e99d64c6..b4f4dc6c 100644 --- a/src/browser/register.ts +++ b/src/browser/register.ts @@ -1,3 +1,4 @@ +import { logger } from "@coder/logger" import { getOptions, normalize, logError } from "../common/util" import "./pages/error.css" @@ -6,19 +7,21 @@ import "./pages/login.css" export async function registerServiceWorker(): Promise { const options = getOptions() + logger.level = options.logLevel + const path = normalize(`${options.csStaticBase}/dist/serviceWorker.js`) try { await navigator.serviceWorker.register(path, { scope: options.base + "/", }) - console.log("[Service Worker] registered") + logger.info(`[Service Worker] registered`) } catch (error) { - logError(`[Service Worker] registration`, error) + logError(logger, `[Service Worker] registration`, error) } } if (typeof navigator !== "undefined" && "serviceWorker" in navigator) { registerServiceWorker() } else { - console.error(`[Service Worker] navigator is undefined`) + logger.error(`[Service Worker] navigator is undefined`) } diff --git a/src/common/util.ts b/src/common/util.ts index 87ca6f59..4e4f23cf 100644 --- a/src/common/util.ts +++ b/src/common/util.ts @@ -1,5 +1,13 @@ -import { logger, field } from "@coder/logger" +/* + * This file exists in two locations: + * - src/common/util.ts + * - lib/vscode/src/vs/server/common/util.ts + * The second is a symlink to the first. + */ +/** + * Base options included on every page. + */ export interface Options { base: string csStaticBase: string @@ -69,6 +77,9 @@ export const getOptions = (): T => { options = {} as T } + // You can also pass options in stringified form to the options query + // variable. Options provided here will override the ones in the options + // element. const params = new URLSearchParams(location.search) const queryOpts = params.get("options") if (queryOpts) { @@ -78,13 +89,9 @@ export const getOptions = (): T => { } } - logger.level = options.logLevel - options.base = resolveBase(options.base) options.csStaticBase = resolveBase(options.csStaticBase) - logger.debug("got options", field("options", options)) - return options } @@ -113,7 +120,8 @@ export const getFirstString = (value: string | string[] | object | undefined): s return typeof value === "string" ? value : undefined } -export function logError(prefix: string, err: any): void { +// TODO: Might make sense to add Error handling to the logger itself. +export function logError(logger: { error: (msg: string) => void }, prefix: string, err: Error | string): void { if (err instanceof Error) { logger.error(`${prefix}: ${err.message} ${err.stack}`) } else { diff --git a/src/node/app.ts b/src/node/app.ts index b487dd8f..97ad62c3 100644 --- a/src/node/app.ts +++ b/src/node/app.ts @@ -37,7 +37,7 @@ export const createApp = async (args: DefaultedArgs): Promise<[Express, Express, reject(err) } else { // Promise resolved earlier so this is an unrelated error. - util.logError("http server error", err) + util.logError(logger, "http server error", err) } }) diff --git a/src/node/routes/index.ts b/src/node/routes/index.ts index 5bc9f10b..c183b42b 100644 --- a/src/node/routes/index.ts +++ b/src/node/routes/index.ts @@ -20,6 +20,7 @@ import * as apps from "./apps" import * as domainProxy from "./domainProxy" import * as health from "./health" import * as login from "./login" +import * as logout from "./logout" import * as pathProxy from "./pathProxy" // static is a reserved keyword. import * as _static from "./static" @@ -136,10 +137,10 @@ export const register = async ( if (args.auth === AuthType.Password) { app.use("/login", login.router) + app.use("/logout", logout.router) } else { - app.all("/login", (req, res) => { - redirect(req, res, "/", {}) - }) + app.all("/login", (req, res) => redirect(req, res, "/", {})) + app.all("/logout", (req, res) => redirect(req, res, "/", {})) } app.use("/static", _static.router) diff --git a/src/node/routes/logout.ts b/src/node/routes/logout.ts new file mode 100644 index 00000000..e42789b4 --- /dev/null +++ b/src/node/routes/logout.ts @@ -0,0 +1,17 @@ +import { Router } from "express" +import { getCookieDomain, redirect } from "../http" +import { Cookie } from "./login" + +export const router = Router() + +router.get("/", async (req, res) => { + // Must use the *identical* properties used to set the cookie. + res.clearCookie(Cookie.Key, { + domain: getCookieDomain(req.headers.host || "", req.args["proxy-domain"]), + path: req.body.base || "/", + sameSite: "lax", + }) + + const to = (typeof req.query.to === "string" && req.query.to) || "/" + return redirect(req, res, to, { to: undefined, base: undefined }) +}) diff --git a/src/node/routes/vscode.ts b/src/node/routes/vscode.ts index aee4cacd..0d9c6309 100644 --- a/src/node/routes/vscode.ts +++ b/src/node/routes/vscode.ts @@ -3,6 +3,7 @@ import { Request, Router } from "express" import { promises as fs } from "fs" import * as path from "path" import qs from "qs" +import * as ipc from "../../../typings/ipc" import { Emitter } from "../../common/emitter" import { HttpCode, HttpError } from "../../common/http" import { getFirstString } from "../../common/util" @@ -39,12 +40,13 @@ router.get("/", async (req, res) => { options.productConfiguration.codeServerVersion = version res.send( - replaceTemplates( + replaceTemplates( req, // Uncomment prod blocks if not in development. TODO: Would this be // better as a build step? Or maintain two HTML files again? commit !== "development" ? content.replace(//g, "") : content, { + authed: req.args.auth !== "none", disableTelemetry: !!req.args["disable-telemetry"], disableUpdateCheck: !!req.args["disable-update-check"], }, diff --git a/test/unit/register.test.ts b/test/unit/register.test.ts index 106b9487..4aa2006f 100644 --- a/test/unit/register.test.ts +++ b/test/unit/register.test.ts @@ -22,11 +22,11 @@ describe("register", () => { }) beforeEach(() => { + jest.clearAllMocks() jest.mock("@coder/logger", () => loggerModule) }) afterEach(() => { - mockRegisterFn.mockClear() jest.resetModules() }) @@ -39,6 +39,7 @@ describe("register", () => { global.navigator = (undefined as unknown) as Navigator & typeof globalThis global.location = (undefined as unknown) as Location & typeof globalThis }) + it("test should have access to browser globals from beforeAll", () => { expect(typeof global.window).not.toBeFalsy() expect(typeof global.document).not.toBeFalsy() @@ -74,24 +75,24 @@ describe("register", () => { }) describe("when navigator and serviceWorker are NOT defined", () => { - let spy: jest.SpyInstance - beforeEach(() => { - spy = jest.spyOn(console, "error") + jest.clearAllMocks() + jest.mock("@coder/logger", () => loggerModule) }) afterAll(() => { jest.restoreAllMocks() }) - it("should log an error to the console", () => { + it("should log an error", () => { // Load service worker like you would in the browser require("../../src/browser/register") - expect(spy).toHaveBeenCalled() - expect(spy).toHaveBeenCalledTimes(1) - expect(spy).toHaveBeenCalledWith("[Service Worker] navigator is undefined") + expect(loggerModule.logger.error).toHaveBeenCalled() + expect(loggerModule.logger.error).toHaveBeenCalledTimes(1) + expect(loggerModule.logger.error).toHaveBeenCalledWith("[Service Worker] navigator is undefined") }) }) + describe("registerServiceWorker", () => { let serviceWorkerPath: string let serviceWorkerScope: string diff --git a/test/unit/update.test.ts b/test/unit/update.test.ts index 39437120..e76c87d3 100644 --- a/test/unit/update.test.ts +++ b/test/unit/update.test.ts @@ -5,7 +5,7 @@ import { tmpdir } from "../../src/node/constants" import { SettingsProvider, UpdateSettings } from "../../src/node/settings" import { LatestResponse, UpdateProvider } from "../../src/node/update" -describe.skip("update", () => { +describe("update", () => { let version = "1.0.0" let spy: string[] = [] const server = http.createServer((request: http.IncomingMessage, response: http.ServerResponse) => { @@ -75,7 +75,7 @@ describe.skip("update", () => { await expect(settings.read()).resolves.toEqual({ update }) expect(isNaN(update.checked)).toEqual(false) expect(update.checked < Date.now() && update.checked >= now).toEqual(true) - expect(update.version).toBe("2.1.0") + expect(update.version).toStrictEqual("2.1.0") expect(spy).toEqual(["/latest"]) }) @@ -87,9 +87,9 @@ describe.skip("update", () => { const update = await p.getUpdate() await expect(settings.read()).resolves.toEqual({ update }) - expect(isNaN(update.checked)).toBe(false) + expect(isNaN(update.checked)).toStrictEqual(false) expect(update.checked < now).toBe(true) - expect(update.version).toBe("2.1.0") + expect(update.version).toStrictEqual("2.1.0") expect(spy).toEqual([]) }) @@ -101,10 +101,10 @@ describe.skip("update", () => { const update = await p.getUpdate(true) await expect(settings.read()).resolves.toEqual({ update }) - expect(isNaN(update.checked)).toBe(false) - expect(update.checked < Date.now() && update.checked >= now).toBe(true) - expect(update.version).toBe("4.1.1") - expect(spy).toBe(["/latest"]) + expect(isNaN(update.checked)).toStrictEqual(false) + expect(update.checked < Date.now() && update.checked >= now).toStrictEqual(true) + expect(update.version).toStrictEqual("4.1.1") + expect(spy).toStrictEqual(["/latest"]) }) it("should get latest after interval passes", async () => { @@ -121,8 +121,8 @@ describe.skip("update", () => { await settings.write({ update: { checked, version } }) const update = await p.getUpdate() - expect(update.checked).not.toBe(checked) - expect(spy).toBe(["/latest"]) + expect(update.checked).not.toStrictEqual(checked) + expect(spy).toStrictEqual(["/latest"]) }) it("should check if it's the current version", async () => { @@ -130,24 +130,31 @@ describe.skip("update", () => { const p = provider() let update = await p.getUpdate(true) - expect(p.isLatestVersion(update)).toBe(false) + expect(p.isLatestVersion(update)).toStrictEqual(false) version = "0.0.0" update = await p.getUpdate(true) - expect(p.isLatestVersion(update)).toBe(true) + expect(p.isLatestVersion(update)).toStrictEqual(true) // Old version format; make sure it doesn't report as being later. version = "999999.9999-invalid999.99.9" update = await p.getUpdate(true) - expect(p.isLatestVersion(update)).toBe(true) + expect(p.isLatestVersion(update)).toStrictEqual(true) }) it("should not reject if unable to fetch", async () => { - expect.assertions(2) let provider = new UpdateProvider("invalid", settings) - await expect(() => provider.getUpdate(true)).resolves.toBe(undefined) + let now = Date.now() + let update = await provider.getUpdate(true) + expect(isNaN(update.checked)).toStrictEqual(false) + expect(update.checked < Date.now() && update.checked >= now).toEqual(true) + expect(update.version).toStrictEqual("unknown") provider = new UpdateProvider("http://probably.invalid.dev.localhost/latest", settings) - await expect(() => provider.getUpdate(true)).resolves.toBe(undefined) + now = Date.now() + update = await provider.getUpdate(true) + expect(isNaN(update.checked)).toStrictEqual(false) + expect(update.checked < Date.now() && update.checked >= now).toEqual(true) + expect(update.version).toStrictEqual("unknown") }) }) diff --git a/test/unit/util.test.ts b/test/unit/util.test.ts index e4d6349a..66ea0ce2 100644 --- a/test/unit/util.test.ts +++ b/test/unit/util.test.ts @@ -18,9 +18,6 @@ global.document = dom.window.document export type LocationLike = Pick -// jest.mock is hoisted above the imports so we must use `require` here. -jest.mock("@coder/logger", () => require("../utils/helpers").loggerModule) - describe("util", () => { describe("normalize", () => { it("should remove multiple slashes", () => { @@ -236,14 +233,14 @@ describe("util", () => { const message = "You don't have access to that folder." const error = new Error(message) - logError("ui", error) + logError(loggerModule.logger, "ui", error) expect(loggerModule.logger.error).toHaveBeenCalled() expect(loggerModule.logger.error).toHaveBeenCalledWith(`ui: ${error.message} ${error.stack}`) }) it("should log an error, even if not an instance of error", () => { - logError("api", "oh no") + logError(loggerModule.logger, "api", "oh no") expect(loggerModule.logger.error).toHaveBeenCalled() expect(loggerModule.logger.error).toHaveBeenCalledWith("api: oh no") diff --git a/test/utils/httpserver.ts b/test/utils/httpserver.ts index a66bbbff..bbd25a6c 100644 --- a/test/utils/httpserver.ts +++ b/test/utils/httpserver.ts @@ -1,3 +1,4 @@ +import { logger } from "@coder/logger" import * as express from "express" import * as http from "http" import * as net from "net" @@ -45,7 +46,7 @@ export class HttpServer { rej(err) } else { // Promise resolved earlier so this is some other error. - util.logError("http server error", err) + util.logError(logger, "http server error", err) } }) }) diff --git a/typings/ipc.d.ts b/typings/ipc.d.ts index f31e1d45..e93e3ab1 100644 --- a/typings/ipc.d.ts +++ b/typings/ipc.d.ts @@ -6,9 +6,12 @@ * The second is a symlink to the first. */ export interface Options { + authed: boolean base: string + csStaticBase: string disableTelemetry: boolean disableUpdateCheck: boolean + logLevel: number } export interface InitMessage {