From d0ef981fd23b06c4e616cf161e5354f3d224add7 Mon Sep 17 00:00:00 2001 From: Maddy Guthridge Date: Tue, 1 Oct 2024 22:38:53 +1000 Subject: [PATCH 01/15] Split auth code into multiple files --- src/endpoints/admin/firstrun.ts | 2 +- src/lib/server/auth/index.ts | 14 +++ src/lib/server/auth/passwords.ts | 64 ++++++++++ src/lib/server/auth/secret.ts | 38 ++++++ src/lib/server/auth/setup.ts | 63 ++++++++++ src/lib/server/{auth.ts => auth/tokens.ts} | 138 +++++---------------- 6 files changed, 214 insertions(+), 105 deletions(-) create mode 100644 src/lib/server/auth/index.ts create mode 100644 src/lib/server/auth/passwords.ts create mode 100644 src/lib/server/auth/secret.ts create mode 100644 src/lib/server/auth/setup.ts rename src/lib/server/{auth.ts => auth/tokens.ts} (58%) diff --git a/src/endpoints/admin/firstrun.ts b/src/endpoints/admin/firstrun.ts index f7ffd7d9..fceae3e7 100644 --- a/src/endpoints/admin/firstrun.ts +++ b/src/endpoints/admin/firstrun.ts @@ -1,5 +1,5 @@ /** Git repository endpoints */ -import type { FirstRunCredentials } from '$lib/server/auth'; +import type { FirstRunCredentials } from '$lib/server/auth/tokens'; import { apiFetch, json } from '../fetch'; /** diff --git a/src/lib/server/auth/index.ts b/src/lib/server/auth/index.ts new file mode 100644 index 00000000..c409bb45 --- /dev/null +++ b/src/lib/server/auth/index.ts @@ -0,0 +1,14 @@ +/** + * Minifolio Auth + * + * Code for performing authorization in Minifolio. + * + * If you discover a security vulnerability, please disclose it responsibly. + */ +export { validateCredentials } from './passwords'; +export { + generateToken, + validateTokenFromRequest, + isRequestAuthorized, + redirectOnInvalidToken, +} from './tokens'; diff --git a/src/lib/server/auth/passwords.ts b/src/lib/server/auth/passwords.ts new file mode 100644 index 00000000..3bece05c --- /dev/null +++ b/src/lib/server/auth/passwords.ts @@ -0,0 +1,64 @@ + +import { hash } from 'crypto'; +import { getLocalConfig } from '../data/localConfig'; +import { error } from '@sveltejs/kit'; + +/** + * How long to wait (in ms) before notifying the user that their login attempt + * was rejected. + */ +const FAIL_DURATION = 100; + +/** + * Promise that resolves in a random amount of time, used to get some timing + * invariance. + */ +const sleepRandom = () => new Promise((r) => setTimeout(r, Math.random() * FAIL_DURATION)); + +/** + * Throw a 401 after a random (small) amount of time, so that timing attacks + * cannot be used reliably. + */ +async function fail(timer: Promise) { + await timer; + return error(401, 'The username or password is incorrect'); +} + +/** Hash a password with the given salt, returning the result */ +export function hashAndSalt(salt: string, password: string): string { + // TODO: Thoroughly check this against the OWASP guidelines -- it might + // not match the requirements. + // https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html + return hash('SHA256', salt + password); +} + +/** + * Validate the user's credentials. + * + * If validation is successful, the user's userId is returned. + * + * If the credentials fail to validate, a random amount of time is waited + * before sending the response, in order to prevent timing attacks. + */ +export async function validateCredentials( + username: string, + password: string, +): Promise { + const local = await getLocalConfig(); + + const failTimer = sleepRandom(); + + // Find a user with a matching username + const userId = Object.keys(local.auth).find(id => local.auth[id].username === username); + + if (!userId) { + return fail(failTimer); + } + + const hashResult = hashAndSalt(local.auth[userId].password.salt, password); + + if (hashResult !== local.auth[userId].password.hash) { + return fail(failTimer); + } + return userId; +} diff --git a/src/lib/server/auth/secret.ts b/src/lib/server/auth/secret.ts new file mode 100644 index 00000000..0ccaf3d2 --- /dev/null +++ b/src/lib/server/auth/secret.ts @@ -0,0 +1,38 @@ +/** + * Code for managing the authentication secret. + */ +import fs from 'fs/promises'; +import { getPrivateDataDir } from '../data/dataDir'; +import { nanoid } from 'nanoid'; + +/** Returns the path to the auth secret file */ +export function getAuthSecretPath(): string { + return `${getPrivateDataDir()}/auth.secret`; +} + +/** Cache for token secret */ +let authSecret: string | undefined; + +/** Returns the secret value used to validate JWTs */ +export async function getAuthSecret(): Promise { + if (authSecret) { + return authSecret; + } + return fs.readFile(getAuthSecretPath(), { encoding: 'utf-8' }); +} + +/** Generate and store a new auth secret, returning its value */ +export async function generateAuthSecret(): Promise { + const secret = nanoid(); + authSecret = secret; + await fs.writeFile(getAuthSecretPath(), secret, { encoding: 'utf-8' }); + return secret; +} + +/** + * Destroy the auth secret, removing it from memory and from the file system. + */ +export async function destroyAuthSecret() { + authSecret = undefined; + await fs.unlink(getAuthSecretPath()); +} diff --git a/src/lib/server/auth/setup.ts b/src/lib/server/auth/setup.ts new file mode 100644 index 00000000..9c367054 --- /dev/null +++ b/src/lib/server/auth/setup.ts @@ -0,0 +1,63 @@ +/** + * Code managing the setup of the auth system of Minifolio. + */ +import { setLocalConfig, type ConfigLocalJson } from '../data/localConfig'; +import { version } from '$app/environment'; +import { hashAndSalt } from './passwords'; +import { nanoid } from 'nanoid'; +import { unixTime } from '$lib/util'; +import { generateToken } from './tokens'; +import type { Cookies } from '@sveltejs/kit'; +import { generateAuthSecret } from './secret'; + +/** Info returned during first-run */ +export interface FirstRunCredentials { + token: string, +} + +/** + * Set up auth information. + * + * This is responsible for: + * + * 1. Setting up the auth secret + * 2. Creating the first account + * 3. Storing the auth info info the local config. + */ +export async function authSetup( + username: string, + password: string, + cookies?: Cookies, +): Promise { + // 1. Set up the auth secret + await generateAuthSecret(); + + // 2. Create the user + const userId = nanoid(); + + // Generate a salt for the password + // Using nanoid for secure generation + const salt = nanoid(); + const passwordHash = hashAndSalt(salt, password); + + // Set up auth config + const config: ConfigLocalJson = { + auth: { + [userId]: { + username, + password: { + hash: passwordHash, + salt: salt, + }, + sessions: { + notBefore: unixTime(), + revokedSessions: {}, + } + }, + }, + version, + }; + await setLocalConfig(config); + + return { token: await generateToken(userId, cookies) }; +} diff --git a/src/lib/server/auth.ts b/src/lib/server/auth/tokens.ts similarity index 58% rename from src/lib/server/auth.ts rename to src/lib/server/auth/tokens.ts index 13856831..3ba655f1 100644 --- a/src/lib/server/auth.ts +++ b/src/lib/server/auth/tokens.ts @@ -2,11 +2,9 @@ import { nanoid } from 'nanoid'; import { validate, number, string, type, type Infer } from 'superstruct'; import jwt, { type Algorithm as JwtAlgorithm } from 'jsonwebtoken'; import { unixTime } from '$lib/util'; -import { hash } from 'crypto'; -import { generate as generateWords } from 'random-words'; -import { getLocalConfig, setLocalConfig, type ConfigLocalJson } from './data/localConfig'; -import { dev, version } from '$app/environment'; +import { getLocalConfig, setLocalConfig } from '../data/localConfig'; import { error, redirect, type Cookies } from '@sveltejs/kit'; +import { getAuthSecret } from './secret'; /** Maximum lifetime of a session -- 14 days */ const sessionLifetime = 60 * 60 * 24 * 14; @@ -23,36 +21,26 @@ const algorithm: JwtAlgorithm = 'HS256'; export const JwtPayload = type({ /** Session ID */ sessionId: string(), + /** User ID of the user who owns this token */ + uid: string(), /** Expiry time (as UNIX timestamp) */ exp: number(), /** Initialization time (as UNIX timestamp) */ iat: number(), }); -/** Returns the secret value used to validate JWTs */ -function getTokenSecret(): string { - const secret = process.env.AUTH_SECRET; - if (!secret) { - throw new Error('AUTH_SECRET environment variable must be set to a value'); - } - if (!dev && secret === 'CHANGE ME') { - throw new Error('AUTH_SECRET must be changed when running in production'); - } - return secret; -} - /** * Generate a token. * * If cookies is provided, the token will also be stored to the cookies. */ -export function generateToken(cookies?: Cookies): string { +export async function generateToken(userId: string, cookies?: Cookies): Promise { const sessionId = nanoid(); const iat = unixTime(); const token = jwt.sign( - { sessionId, iat }, - getTokenSecret(), + { sessionId, iat, uid: userId }, + await getAuthSecret(), { expiresIn: sessionLifetime, algorithm } ); const expires = iat + sessionLifetime; @@ -83,7 +71,7 @@ export async function validateToken(token: string): Promise error(400, e))` them const [err, data] = validate(payload, JwtPayload); if (err) { // Token data format is incorrect throw Error('Token data is in incorrect format'); } // Ensure that the session isn't in our revoked list - if (data.sessionId in config.auth.sessions.revokedSessions) { + if (data.sessionId in config.auth[data.uid].sessions.revokedSessions) { throw Error('This session has been revoked'); } // And also that it wasn't issued before our notBefore time - if (data.iat < config.auth.sessions.notBefore) { + if (data.iat < config.auth[data.uid].sessions.notBefore) { throw Error('This session was created too long ago'); } return data; } +/** Revoke the session of the given token */ +export async function revokeSession(token: string): Promise { + const config = await getLocalConfig(); + if (!config.auth) { + // Can't invalidate tokens if there is not auth + throw Error('Authentication is disabled'); + } + const sessionData = await validateToken(token); + // Add to the revoked sessions + config.auth[sessionData.uid].sessions.revokedSessions[sessionData.sessionId] + = sessionData.exp; + await setLocalConfig(config); +} + /** * Validates and returns the token from the given request. * @@ -135,91 +139,17 @@ export async function validateTokenFromRequest(req: { request: Request, cookies: return token; } -/** Return whether the request is authorized (has a token) */ +/** Return whether the request is authorized (has a valid token) */ export async function isRequestAuthorized(req: { request: Request, cookies: Cookies }): Promise { - try { - await validateTokenFromRequest(req); - return true; - } catch { - return false; - } -} - -/** If the given token is invalid, throw a redirect to the given page */ -export async function redirectOnInvalidToken(req: { request: Request, cookies: Cookies }, url: string) { - await validateTokenFromRequest(req).catch(() => redirect(303, url)); -} - -/** Revoke the session of the given token */ -export async function revokeSession(token: string): Promise { - const config = await getLocalConfig(); - if (!config.auth) { - // Can't invalidate tokens if there is not auth - throw Error('Authentication is disabled'); - } - const sessionData = await validateToken(token); - // Add to the revoked sessions - config.auth.sessions.revokedSessions[sessionData.sessionId] = sessionData.exp; - await setLocalConfig(config); -} - -/** Credentials provided after first run */ -export interface FirstRunCredentials { - username: string, - password: string, - token: string, -} - -/** Hash a password with the given salt, returning the result */ -export function hashAndSalt(salt: string, password: string): string { - // TODO: Thoroughly check this against the OWASP guidelines -- it probably - // doesn't match the requirements. - // https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html - return hash('SHA256', salt + password); + // This feels kinda like Rust's Result type and I like that + return validateTokenFromRequest(req) + .then(() => true) + .catch(() => false); } /** - * Set up auth information. - * - * This is responsible for generating and storing a secure password, thereby - * creating the default "admin" account. + * If the given request's token is invalid, throw a redirect to the given URL. */ -export async function authSetup(cookies?: Cookies): Promise { - const username = 'admin'; - - // generate password using 4 random dictionary words - // as per tradition, https://xkcd.com/936/ - // TODO: Can this package be used securely? - // If not maybe just use a nanoid, even though that's much less fun - const password = (generateWords({ exactly: 4, minLength: 5 }) as string[]).join('-'); - - // Generate a salt for the password - // Using nanoid for secure generation - const salt = nanoid(); - - const passwordHash = hashAndSalt(salt, password); - - // Set up auth config - const config: ConfigLocalJson = { - auth: { - username, - password: { - hash: passwordHash, - salt: salt, - }, - sessions: { - notBefore: unixTime(), - revokedSessions: {}, - } - }, - version, - }; - - await setLocalConfig(config); - - return { - username, - password, - token: generateToken(cookies), - }; +export async function redirectOnInvalidToken(req: { request: Request, cookies: Cookies }, url: string) { + await validateTokenFromRequest(req).catch(() => redirect(303, url)); } From f2bab9b04566a9a3302cc610dc680630eeb4545c Mon Sep 17 00:00:00 2001 From: Maddy Guthridge Date: Tue, 1 Oct 2024 22:39:50 +1000 Subject: [PATCH 02/15] Split data dir into public and private data --- .gitignore | 2 ++ .vscode/settings.json | 1 + docs/Files.md | 31 ++++++++++++++++++++ src/lib/server/data/dataDir.ts | 37 +++++++++++++++++------- src/lib/server/data/localConfig.ts | 15 +++++----- src/lib/server/data/migrations/index.ts | 32 ++++++++++---------- src/lib/server/data/migrations/shared.ts | 24 +++++++++++++++ src/lib/server/data/migrations/v0.6.0.ts | 7 +++++ src/lib/server/git.ts | 4 +-- src/lib/server/keys.ts | 12 ++++++++ 10 files changed, 128 insertions(+), 37 deletions(-) create mode 100644 docs/Files.md create mode 100644 src/lib/server/data/migrations/shared.ts create mode 100644 src/lib/server/data/migrations/v0.6.0.ts create mode 100644 src/lib/server/keys.ts diff --git a/.gitignore b/.gitignore index a193bab5..9a42208f 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,8 @@ # Data directory /data/ +# Private data directory +/private_data/ # Server logs *.log diff --git a/.vscode/settings.json b/.vscode/settings.json index 936bd2d1..687c772e 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -2,6 +2,7 @@ "cSpell.words": [ "Asciinema", "firstrun", + "Minifolio", "superstruct" ] } diff --git a/docs/Files.md b/docs/Files.md new file mode 100644 index 00000000..e48aa5f4 --- /dev/null +++ b/docs/Files.md @@ -0,0 +1,31 @@ +# File locations in Minifolio + +## Data directory + +Determined using environment variable `DATA_REPO_PATH`. + +Main portfolio data. Should be backed up using a `git` repo. + +### `config.json` + +Main site configuration. + +## Private data directory + +Determined using environment variable `PRIVATE_DATA_PATH`. + +Contains private data, including credentials and authentication secrets. + +### `config.local.json` + +Contains the local configuration of the server, including credentials and token +info. + +### `id_ed25519`, `id_ed25519.pub` + +SSH key used by the server. These are used to perform git operations over SSH. + +### `auth.secret` + +Contains the authentication secret used by the server. This is used to validate +JWTs. diff --git a/src/lib/server/data/dataDir.ts b/src/lib/server/data/dataDir.ts index 08c116df..d7644151 100644 --- a/src/lib/server/data/dataDir.ts +++ b/src/lib/server/data/dataDir.ts @@ -11,9 +11,24 @@ export function getDataDir(): string { return repoPath; } +/** + * Returns the path to the private data directory, which contains data such as + * `config.local.json` and the server's SSH keys. + */ +export function getPrivateDataDir(): string { + const privateDataPath = process.env.PRIVATE_DATA_PATH; + if (!privateDataPath) { + throw new Error('PRIVATE_DATA_PATH environment variable is not set'); + } + return privateDataPath; +} + /** * Returns whether the data directory contains data that can conceivably - * be used by the portfolio website. + * be used by Minifolio. + * + * This should be used when setting up a git repo to ensure that its data is + * valid enough to be used by us. * * Currently, this checks for the existence of a config.json, but in the future * I may force it to check more thoroughly for data validity. @@ -23,19 +38,19 @@ export async function dataDirContainsData(): Promise { return await fileExists(path.join(repoPath, 'config.json')); } +/** Returns whether the data directory is a git repo */ +export async function dataDirUsesGit(): Promise { + const repoPath = getDataDir(); + const repo = simpleGit(repoPath); + return repo.checkIsRepo(CheckRepoActions.IS_REPO_ROOT); +} + /** - * Returns whether the data directory has been initialized. + * Returns whether the private data has been initialized. * * This checks for the existence of a config.local.json. */ -export async function dataDirIsInit(): Promise { - const repoPath = getDataDir(); +export async function serverIsSetUp(): Promise { + const repoPath = getPrivateDataDir(); return await fileExists(path.join(repoPath, 'config.local.json')); } - -/** Returns whether the data directory is backed by git */ -export async function dataDirUsesGit(): Promise { - const repoPath = getDataDir(); - const repo = simpleGit(repoPath); - return repo.checkIsRepo(CheckRepoActions.IS_REPO_ROOT); -} diff --git a/src/lib/server/data/localConfig.ts b/src/lib/server/data/localConfig.ts index cb882583..11e81d6d 100644 --- a/src/lib/server/data/localConfig.ts +++ b/src/lib/server/data/localConfig.ts @@ -1,21 +1,20 @@ import { readFile, writeFile } from 'fs/promises'; -import { nullable, number, object, record, string, validate, type Infer } from 'superstruct'; -import { getDataDir } from './dataDir'; +import { number, object, record, string, validate, type Infer } from 'superstruct'; +import { getPrivateDataDir } from './dataDir'; /** Path to config.local.json */ -const CONFIG_LOCAL_JSON = () => `${getDataDir()}/config.local.json`; +const CONFIG_LOCAL_JSON = () => `${getPrivateDataDir()}/config.local.json`; /** * Validator for config.local.json file */ export const ConfigLocalJsonStruct = object({ /** - * Authentication data. - * - * If null, then authentication is disabled, and logging in is not allowed. + * Authentication data, as a record between user IDs and their + * authentication info. */ - auth: nullable(object({ - /** Username of account */ + auth: record(string(), object({ + /** The user's username, used for logging in */ username: string(), /** Information about the user's password */ password: object({ diff --git a/src/lib/server/data/migrations/index.ts b/src/lib/server/data/migrations/index.ts index fb90636e..d8d6bffd 100644 --- a/src/lib/server/data/migrations/index.ts +++ b/src/lib/server/data/migrations/index.ts @@ -2,24 +2,11 @@ import { version } from '$app/environment'; import { getConfig, setConfig } from '../config'; import { getDataDir } from '../dataDir'; import { getLocalConfig, setLocalConfig } from '../localConfig'; -import migrateV010 from './v0.1.0'; -import migrateV020 from './v0.2.0'; -import migrateV030 from './v0.3.0'; +import migrateFromV010 from './v0.1.0'; +import migrateFromV020 from './v0.2.0'; +import migrateFromV030 from './v0.3.0'; import semver from 'semver'; -export type MigrationFunction = (dataDir: string) => Promise; - -/** Lookup table of migrations */ -const migrations: Record = { - '~0.1.0': migrateV010, - '~0.2.0': migrateV020, - '~0.3.0': migrateV030, - // No major changes to data format this release - '~0.4.0': updateConfigVersions, - // Pre-empt future releases - '~0.5.0': updateConfigVersions, -}; - /** Update config versions (only for minor, non-breaking changes to config.json) */ export async function updateConfigVersions(_dataDir: string) { const config = await getConfig(); @@ -30,6 +17,19 @@ export async function updateConfigVersions(_dataDir: string) { await setLocalConfig(configLocal); } +export type MigrationFunction = (dataDir: string) => Promise; + +/** Lookup table of migrations */ +const migrations: Record = { + '~0.1.0': migrateFromV010, + '~0.2.0': migrateFromV020, + '~0.3.0': migrateFromV030, + // No major changes to data format this release + '~0.4.0': updateConfigVersions, + // Pre-empt future releases + '~0.5.0': updateConfigVersions, +}; + /** Perform a migration from the given version */ export default async function migrate(oldVersion: string) { console.log(`Data directory uses version ${oldVersion}. Migration needed`); diff --git a/src/lib/server/data/migrations/shared.ts b/src/lib/server/data/migrations/shared.ts new file mode 100644 index 00000000..d420c01a --- /dev/null +++ b/src/lib/server/data/migrations/shared.ts @@ -0,0 +1,24 @@ +/** + * Shared helper functions for common data migration actions. + */ +import { dev } from '$app/environment'; +import fs from 'fs/promises'; + +/** Move config.local.json to the private data directory */ +export async function moveLocalConfig(dataDir: string, privateDataDir: string) { + const originalPath = `${dataDir}/config.local.json`; + const newPath = `${privateDataDir}/config.local.json`; + await fs.rename(originalPath, newPath); +} + +/** Write auth secret from environment variable */ +export async function writeAuthSecret(privateDataDir: string) { + const secret = process.env.AUTH_SECRET; + if (!secret) { + throw new Error('AUTH_SECRET environment variable must be set to a value'); + } + if (!dev && secret === 'CHANGE ME') { + throw new Error('AUTH_SECRET must be changed when running in production'); + } + await fs.writeFile(`${privateDataDir}/auth.secret`, secret, { encoding: 'utf-8' }); +} diff --git a/src/lib/server/data/migrations/v0.6.0.ts b/src/lib/server/data/migrations/v0.6.0.ts new file mode 100644 index 00000000..fc0dba17 --- /dev/null +++ b/src/lib/server/data/migrations/v0.6.0.ts @@ -0,0 +1,7 @@ +/** + * Migrate to v0.6.0 + * + * Primary changes: + * + * * Move `config.local.json` to the new private data directory. + */ diff --git a/src/lib/server/git.ts b/src/lib/server/git.ts index b88c70db..5a04733c 100644 --- a/src/lib/server/git.ts +++ b/src/lib/server/git.ts @@ -1,5 +1,5 @@ import { error } from '@sveltejs/kit'; -import { dataDirContainsData, dataDirIsInit, getDataDir } from './data/dataDir'; +import { dataDirContainsData, serverIsSetUp, getDataDir } from './data/dataDir'; import simpleGit, { type FileStatusResult } from 'simple-git'; import fs from 'fs/promises'; import { rimraf } from 'rimraf'; @@ -102,7 +102,7 @@ export async function getRepoStatus(): Promise { /** Set up the data dir given a git repo URL and branch name */ export async function setupGitRepo(repo: string, branch: string | null) { // Check whether the data repo is set up - if (await dataDirIsInit()) { + if (await serverIsSetUp()) { throw error(403, 'Data repo is already set up'); } diff --git a/src/lib/server/keys.ts b/src/lib/server/keys.ts new file mode 100644 index 00000000..b13e7f36 --- /dev/null +++ b/src/lib/server/keys.ts @@ -0,0 +1,12 @@ +/** Code for managing the server's SSH keys */ +import fs from 'fs/promises'; + +/** Generate an SSH key */ +export async function sshKeygen() { + +} + +export async function getPublicKey(): Promise { + const data + return fs.readFile() +} From cfbb6cb083205d3a09425a99ef469345c95541f9 Mon Sep 17 00:00:00 2001 From: Maddy Guthridge Date: Tue, 1 Oct 2024 22:42:04 +1000 Subject: [PATCH 03/15] Rename core functions to make them more clear --- src/routes/+page.server.ts | 6 +-- src/routes/[group]/+page.server.ts | 2 +- src/routes/[group]/[item]/+page.server.ts | 2 +- src/routes/[group]/[item]/[file]/+server.ts | 2 +- src/routes/about/+page.server.ts | 6 +-- src/routes/admin/+page.server.ts | 6 +-- src/routes/admin/firstrun/+page.server.ts | 4 +- src/routes/admin/login/+page.server.ts | 2 +- src/routes/api/admin/auth/change/+server.ts | 6 +-- src/routes/api/admin/auth/disable/+server.ts | 6 +-- src/routes/api/admin/auth/login/+server.ts | 41 +++---------------- src/routes/api/admin/auth/logout/+server.ts | 4 +- src/routes/api/admin/auth/revoke/+server.ts | 8 ++-- src/routes/api/admin/config/+server.ts | 8 ++-- src/routes/api/admin/data/refresh/+server.ts | 4 +- src/routes/api/admin/firstrun/+server.ts | 7 ++-- src/routes/api/admin/git/+server.ts | 6 +-- src/routes/api/admin/git/commit/+server.ts | 8 ++-- src/routes/api/admin/git/init/+server.ts | 8 ++-- src/routes/api/admin/git/pull/+server.ts | 8 ++-- src/routes/api/admin/git/push/+server.ts | 8 ++-- src/routes/api/admin/publickey/+server.ts | 12 ++++++ src/routes/api/debug/clear/+server.ts | 4 +- src/routes/api/group/+server.ts | 2 +- src/routes/api/group/[groupId]/+server.ts | 6 +-- .../group/[groupId]/item/[itemId]/+server.ts | 10 ++--- .../[groupId]/item/[itemId]/link/+server.ts | 8 ++-- .../[groupId]/item/[itemId]/readme/+server.ts | 6 +-- .../api/group/[groupId]/readme/+server.ts | 4 +- src/routes/api/readme/+server.ts | 6 +-- src/routes/favicon/+server.ts | 4 +- tests/backend/admin/auth/login.test.ts | 2 +- tests/backend/admin/firstrun.test.ts | 2 +- tests/backend/admin/publickey/get.test.ts | 0 tests/backend/helpers.ts | 3 +- 35 files changed, 102 insertions(+), 119 deletions(-) create mode 100644 src/routes/api/admin/publickey/+server.ts create mode 100644 tests/backend/admin/publickey/get.test.ts diff --git a/src/routes/+page.server.ts b/src/routes/+page.server.ts index 37a04270..cf90ea84 100644 --- a/src/routes/+page.server.ts +++ b/src/routes/+page.server.ts @@ -1,10 +1,10 @@ import { redirect } from '@sveltejs/kit'; import { getPortfolioGlobals } from '$lib/server'; -import { dataDirIsInit } from '$lib/server/data/dataDir.js'; -import { isRequestAuthorized } from '$lib/server/auth.js'; +import { serverIsSetUp } from '$lib/server/data/dataDir'; +import { isRequestAuthorized } from '$lib/server/auth/tokens'; export async function load(req) { - if (!await dataDirIsInit()) { + if (!await serverIsSetUp()) { redirect(303, '/admin/firstrun'); } const globals = await getPortfolioGlobals(); diff --git a/src/routes/[group]/+page.server.ts b/src/routes/[group]/+page.server.ts index 3f88f94b..90cb0198 100644 --- a/src/routes/[group]/+page.server.ts +++ b/src/routes/[group]/+page.server.ts @@ -1,6 +1,6 @@ import { error } from '@sveltejs/kit'; import { getPortfolioGlobals } from '$lib/server'; -import { isRequestAuthorized } from '$lib/server/auth.js'; +import { isRequestAuthorized } from '$lib/server/auth/tokens'; export async function load(req) { const globals = await getPortfolioGlobals(); diff --git a/src/routes/[group]/[item]/+page.server.ts b/src/routes/[group]/[item]/+page.server.ts index 12f58805..0ab9af3a 100644 --- a/src/routes/[group]/[item]/+page.server.ts +++ b/src/routes/[group]/[item]/+page.server.ts @@ -1,6 +1,6 @@ import { error } from '@sveltejs/kit'; import { getPortfolioGlobals } from '$lib/server'; -import { isRequestAuthorized } from '$lib/server/auth.js'; +import { isRequestAuthorized } from '$lib/server/auth/tokens'; export async function load(req) { const globals = await getPortfolioGlobals(); diff --git a/src/routes/[group]/[item]/[file]/+server.ts b/src/routes/[group]/[item]/[file]/+server.ts index b0c1789b..6eb09928 100644 --- a/src/routes/[group]/[item]/[file]/+server.ts +++ b/src/routes/[group]/[item]/[file]/+server.ts @@ -2,7 +2,7 @@ import sanitize from 'sanitize-filename'; import fs from 'fs/promises'; import { error } from '@sveltejs/kit'; import mime from 'mime-types'; -import { getDataDir } from '$lib/server/data/dataDir.js'; +import { getDataDir } from '$lib/server/data/dataDir'; export async function GET({ params, setHeaders }) { const { group, item, file } = params; diff --git a/src/routes/about/+page.server.ts b/src/routes/about/+page.server.ts index 67e84198..4c31cdaa 100644 --- a/src/routes/about/+page.server.ts +++ b/src/routes/about/+page.server.ts @@ -1,7 +1,7 @@ import { getPortfolioGlobals } from '$lib/server'; -import { isRequestAuthorized } from '$lib/server/auth.js'; -import blankConfig from '$lib/blankConfig.js'; -import type { ConfigJson } from '$lib/server/data/config.js'; +import { isRequestAuthorized } from '$lib/server/auth/tokens'; +import blankConfig from '$lib/blankConfig'; +import type { ConfigJson } from '$lib/server/data/config'; import { version } from '$app/environment'; // import { VERSION as SVELTE_VERSION } from 'svelte/compiler'; // import { VERSION as SVELTEKIT_VERSION } from '@sveltejs/kit'; diff --git a/src/routes/admin/+page.server.ts b/src/routes/admin/+page.server.ts index ce35cdd3..a1b446ab 100644 --- a/src/routes/admin/+page.server.ts +++ b/src/routes/admin/+page.server.ts @@ -1,7 +1,7 @@ import { getPortfolioGlobals } from '$lib/server'; -import { redirectOnInvalidToken } from '$lib/server/auth.js'; -import { dataDirUsesGit } from '$lib/server/data/dataDir.js'; -import { getRepoStatus } from '$lib/server/git.js'; +import { redirectOnInvalidToken } from '$lib/server/auth/tokens'; +import { dataDirUsesGit } from '$lib/server/data/dataDir'; +import { getRepoStatus } from '$lib/server/git'; export async function load(req) { const globals = await getPortfolioGlobals(); diff --git a/src/routes/admin/firstrun/+page.server.ts b/src/routes/admin/firstrun/+page.server.ts index 34b2923c..8366ab11 100644 --- a/src/routes/admin/firstrun/+page.server.ts +++ b/src/routes/admin/firstrun/+page.server.ts @@ -1,8 +1,8 @@ import { redirect } from '@sveltejs/kit'; -import { dataDirIsInit } from '$lib/server/data/dataDir.js'; +import { serverIsSetUp } from '$lib/server/data/dataDir'; export async function load() { - if (await dataDirIsInit()) { + if (await serverIsSetUp()) { redirect(303, '/'); } return {}; diff --git a/src/routes/admin/login/+page.server.ts b/src/routes/admin/login/+page.server.ts index a28b8f4b..c2703146 100644 --- a/src/routes/admin/login/+page.server.ts +++ b/src/routes/admin/login/+page.server.ts @@ -1,5 +1,5 @@ import { getPortfolioGlobals } from '$lib/server'; -import { validateTokenFromRequest } from '$lib/server/auth.js'; +import { validateTokenFromRequest } from '$lib/server/auth/tokens'; import { redirect } from '@sveltejs/kit'; export async function load(req) { diff --git a/src/routes/api/admin/auth/change/+server.ts b/src/routes/api/admin/auth/change/+server.ts index fe919a7c..18b9106e 100644 --- a/src/routes/api/admin/auth/change/+server.ts +++ b/src/routes/api/admin/auth/change/+server.ts @@ -1,6 +1,6 @@ -import { hashAndSalt, validateTokenFromRequest } from '$lib/server/auth'; -import { getPortfolioGlobals } from '$lib/server/data/index.js'; -import { getLocalConfig, setLocalConfig } from '$lib/server/data/localConfig.js'; +import { hashAndSalt, validateTokenFromRequest } from '$lib/server/auth/tokens'; +import { getPortfolioGlobals } from '$lib/server/data/index'; +import { getLocalConfig, setLocalConfig } from '$lib/server/data/localConfig'; import { error, json } from '@sveltejs/kit'; import { nanoid } from 'nanoid'; import { object, string, validate } from 'superstruct'; diff --git a/src/routes/api/admin/auth/disable/+server.ts b/src/routes/api/admin/auth/disable/+server.ts index e3bf9fe2..5e639207 100644 --- a/src/routes/api/admin/auth/disable/+server.ts +++ b/src/routes/api/admin/auth/disable/+server.ts @@ -1,6 +1,6 @@ -import { hashAndSalt, validateTokenFromRequest } from '$lib/server/auth'; -import { getPortfolioGlobals } from '$lib/server/data/index.js'; -import { getLocalConfig, setLocalConfig } from '$lib/server/data/localConfig.js'; +import { hashAndSalt, validateTokenFromRequest } from '$lib/server/auth/tokens'; +import { getPortfolioGlobals } from '$lib/server/data/index'; +import { getLocalConfig, setLocalConfig } from '$lib/server/data/localConfig'; import { error, json } from '@sveltejs/kit'; export async function POST({ request, cookies }) { diff --git a/src/routes/api/admin/auth/login/+server.ts b/src/routes/api/admin/auth/login/+server.ts index a36be297..d0686208 100644 --- a/src/routes/api/admin/auth/login/+server.ts +++ b/src/routes/api/admin/auth/login/+server.ts @@ -1,46 +1,15 @@ -import { generateToken, hashAndSalt } from '$lib/server/auth.js'; -import { getPortfolioGlobals } from '$lib/server/data/index.js'; -import { getLocalConfig } from '$lib/server/data/localConfig.js'; +import { validateCredentials } from '$lib/server/auth/passwords'; +import { generateToken } from '$lib/server/auth/tokens'; +import { getPortfolioGlobals } from '$lib/server/data/index'; import { error, json } from '@sveltejs/kit'; -const FAIL_DURATION = 50; - -/** - * Promise that resolves in a random amount of time, used to get some timing - * invariance. - */ -const sleepRandom = () => new Promise((r) => setTimeout(r, Math.random() * FAIL_DURATION)); - -/** - * Throw a 401 after a random (small) amount of time, so that timing attacks - * cannot be used reliably. - */ -async function fail(timer: Promise) { - await timer; - return error(401, 'The username or password is incorrect'); -} export async function POST({ request, cookies }) { await getPortfolioGlobals().catch(e => error(400, e)); - const local = await getLocalConfig(); - - if (!local.auth) { - error(403, 'Logging in has been disabled by the administrator'); - } - const { username, password } = await request.json(); - const failTimer = sleepRandom(); - - if (username !== local.auth.username) { - return fail(failTimer); - } - - if (hashAndSalt(local.auth.password.salt, password) !== local.auth.password.hash) { - return fail(failTimer); - } + const uid = await validateCredentials(username, password); - const token = generateToken(cookies); - return json({ token }, { status: 200 }); + return json({ token: generateToken(uid, cookies) }, { status: 200 }); } diff --git a/src/routes/api/admin/auth/logout/+server.ts b/src/routes/api/admin/auth/logout/+server.ts index 9526355b..e00a5681 100644 --- a/src/routes/api/admin/auth/logout/+server.ts +++ b/src/routes/api/admin/auth/logout/+server.ts @@ -1,5 +1,5 @@ -import { revokeSession, validateTokenFromRequest } from '$lib/server/auth'; -import { getPortfolioGlobals } from '$lib/server/data/index.js'; +import { revokeSession, validateTokenFromRequest } from '$lib/server/auth/tokens'; +import { getPortfolioGlobals } from '$lib/server/data/index'; import { error, json } from '@sveltejs/kit'; export async function POST(req) { diff --git a/src/routes/api/admin/auth/revoke/+server.ts b/src/routes/api/admin/auth/revoke/+server.ts index 4c391e21..dc773811 100644 --- a/src/routes/api/admin/auth/revoke/+server.ts +++ b/src/routes/api/admin/auth/revoke/+server.ts @@ -1,7 +1,7 @@ -import { validateTokenFromRequest } from '$lib/server/auth'; -import { getPortfolioGlobals } from '$lib/server/data/index.js'; -import { getLocalConfig, setLocalConfig } from '$lib/server/data/localConfig.js'; -import { unixTime } from '$lib/util.js'; +import { validateTokenFromRequest } from '$lib/server/auth/tokens'; +import { getPortfolioGlobals } from '$lib/server/data/index'; +import { getLocalConfig, setLocalConfig } from '$lib/server/data/localConfig'; +import { unixTime } from '$lib/util'; import { error, json } from '@sveltejs/kit'; export async function POST({ request, cookies }) { diff --git a/src/routes/api/admin/config/+server.ts b/src/routes/api/admin/config/+server.ts index acb616aa..f13f6a41 100644 --- a/src/routes/api/admin/config/+server.ts +++ b/src/routes/api/admin/config/+server.ts @@ -1,11 +1,11 @@ import { error, json } from '@sveltejs/kit'; -import { validateTokenFromRequest } from '$lib/server/auth.js'; -import { ConfigJsonStruct, setConfig } from '$lib/server/data/config.js'; +import { validateTokenFromRequest } from '$lib/server/auth/tokens'; +import { ConfigJsonStruct, setConfig } from '$lib/server/data/config'; import { validate } from 'superstruct'; import { version } from '$app/environment'; -import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/data/index.js'; +import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/data/index'; import fs from 'fs/promises'; -import { getDataDir } from '$lib/server/data/dataDir.js'; +import { getDataDir } from '$lib/server/data/dataDir'; export async function GET({ request, cookies }) { const data = await getPortfolioGlobals().catch(e => error(400, e)); diff --git a/src/routes/api/admin/data/refresh/+server.ts b/src/routes/api/admin/data/refresh/+server.ts index 2126ed70..c575108e 100644 --- a/src/routes/api/admin/data/refresh/+server.ts +++ b/src/routes/api/admin/data/refresh/+server.ts @@ -1,5 +1,5 @@ -import { validateTokenFromRequest } from '$lib/server/auth.js'; -import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/index.js'; +import { validateTokenFromRequest } from '$lib/server/auth/tokens'; +import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/index'; import { error, json } from '@sveltejs/kit'; export async function POST({ request, cookies }) { diff --git a/src/routes/api/admin/firstrun/+server.ts b/src/routes/api/admin/firstrun/+server.ts index 6610667b..ab30f4f4 100644 --- a/src/routes/api/admin/firstrun/+server.ts +++ b/src/routes/api/admin/firstrun/+server.ts @@ -1,17 +1,18 @@ import { error, json } from '@sveltejs/kit'; -import { dataDirContainsData, dataDirIsInit, getDataDir } from '$lib/server/data/dataDir'; +import { dataDirContainsData, serverIsSetUp, getDataDir } from '$lib/server/data/dataDir'; import { mkdir } from 'fs/promises'; import { runSshKeyscan, setupGitignore, setupGitRepo, urlRequiresSsh } from '$lib/server/git'; -import { authSetup } from '$lib/server/auth'; +import { authSetup } from '$lib/server/auth/setup'; import { initConfig } from '$lib/server/data/config'; import { initReadme } from '$lib/server/data/readme'; import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/data/index'; +// FIXME: Make this accept a username and password export async function POST({ request, cookies }) { const { repoUrl, branch }: { repoUrl: string | null, branch: string | null } = await request.json(); - if (await dataDirIsInit()) { + if (await serverIsSetUp()) { error(403); } diff --git a/src/routes/api/admin/git/+server.ts b/src/routes/api/admin/git/+server.ts index 2858ea35..eba607ab 100644 --- a/src/routes/api/admin/git/+server.ts +++ b/src/routes/api/admin/git/+server.ts @@ -1,8 +1,8 @@ import { error, json } from '@sveltejs/kit'; import { dataDirUsesGit } from '$lib/server/data/dataDir'; -import { validateTokenFromRequest } from '$lib/server/auth.js'; -import { getPortfolioGlobals } from '$lib/server/data/index.js'; -import { getRepoStatus } from '$lib/server/git.js'; +import { validateTokenFromRequest } from '$lib/server/auth/tokens'; +import { getPortfolioGlobals } from '$lib/server/data/index'; +import { getRepoStatus } from '$lib/server/git'; export async function GET({ request, cookies }) { await getPortfolioGlobals().catch(e => error(400, e)); diff --git a/src/routes/api/admin/git/commit/+server.ts b/src/routes/api/admin/git/commit/+server.ts index df573b63..4ea6ff7c 100644 --- a/src/routes/api/admin/git/commit/+server.ts +++ b/src/routes/api/admin/git/commit/+server.ts @@ -1,7 +1,7 @@ -import { validateTokenFromRequest } from '$lib/server/auth.js'; -import { dataDirUsesGit, getDataDir } from '$lib/server/data/dataDir.js'; -import { getRepoStatus } from '$lib/server/git.js'; -import { getPortfolioGlobals } from '$lib/server/index.js'; +import { validateTokenFromRequest } from '$lib/server/auth/tokens'; +import { dataDirUsesGit, getDataDir } from '$lib/server/data/dataDir'; +import { getRepoStatus } from '$lib/server/git'; +import { getPortfolioGlobals } from '$lib/server/index'; import { error, json } from '@sveltejs/kit'; import simpleGit from 'simple-git'; import { object, string, validate } from 'superstruct'; diff --git a/src/routes/api/admin/git/init/+server.ts b/src/routes/api/admin/git/init/+server.ts index d40d6d99..2e1530bd 100644 --- a/src/routes/api/admin/git/init/+server.ts +++ b/src/routes/api/admin/git/init/+server.ts @@ -1,7 +1,7 @@ -import { validateTokenFromRequest } from '$lib/server/auth.js'; -import { dataDirUsesGit, getDataDir } from '$lib/server/data/dataDir.js'; -import { getRepoStatus, runSshKeyscan, urlRequiresSsh } from '$lib/server/git.js'; -import { getPortfolioGlobals } from '$lib/server/index.js'; +import { validateTokenFromRequest } from '$lib/server/auth/tokens'; +import { dataDirUsesGit, getDataDir } from '$lib/server/data/dataDir'; +import { getRepoStatus, runSshKeyscan, urlRequiresSsh } from '$lib/server/git'; +import { getPortfolioGlobals } from '$lib/server/index'; import { error, json } from '@sveltejs/kit'; import simpleGit from 'simple-git'; import { object, string, validate } from 'superstruct'; diff --git a/src/routes/api/admin/git/pull/+server.ts b/src/routes/api/admin/git/pull/+server.ts index b41f66f9..0da5fced 100644 --- a/src/routes/api/admin/git/pull/+server.ts +++ b/src/routes/api/admin/git/pull/+server.ts @@ -1,7 +1,7 @@ -import { validateTokenFromRequest } from '$lib/server/auth.js'; -import { dataDirUsesGit, getDataDir } from '$lib/server/data/dataDir.js'; -import { getRepoStatus } from '$lib/server/git.js'; -import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/index.js'; +import { validateTokenFromRequest } from '$lib/server/auth/tokens'; +import { dataDirUsesGit, getDataDir } from '$lib/server/data/dataDir'; +import { getRepoStatus } from '$lib/server/git'; +import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/index'; import { error, json } from '@sveltejs/kit'; import simpleGit from 'simple-git'; diff --git a/src/routes/api/admin/git/push/+server.ts b/src/routes/api/admin/git/push/+server.ts index d93557f5..4e805f1e 100644 --- a/src/routes/api/admin/git/push/+server.ts +++ b/src/routes/api/admin/git/push/+server.ts @@ -1,7 +1,7 @@ -import { validateTokenFromRequest } from '$lib/server/auth.js'; -import { dataDirUsesGit, getDataDir } from '$lib/server/data/dataDir.js'; -import { getRepoStatus } from '$lib/server/git.js'; -import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/index.js'; +import { validateTokenFromRequest } from '$lib/server/auth/tokens'; +import { dataDirUsesGit, getDataDir } from '$lib/server/data/dataDir'; +import { getRepoStatus } from '$lib/server/git'; +import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/index'; import { error, json } from '@sveltejs/kit'; import simpleGit from 'simple-git'; diff --git a/src/routes/api/admin/publickey/+server.ts b/src/routes/api/admin/publickey/+server.ts new file mode 100644 index 00000000..01cb15bd --- /dev/null +++ b/src/routes/api/admin/publickey/+server.ts @@ -0,0 +1,12 @@ +import { validateTokenFromRequest } from '$lib/server/auth/tokens'; +import { serverIsSetUp } from '$lib/server/data/dataDir'; +import { json } from '@sveltejs/kit'; + +export async function GET(req) { + // Auth needed when set up, but otherwise not + if (await serverIsSetUp()) { + await validateTokenFromRequest(req); + } + + return json({ key: '' }, { status: 200 }); +} diff --git a/src/routes/api/debug/clear/+server.ts b/src/routes/api/debug/clear/+server.ts index 7ffd06cb..519418d0 100644 --- a/src/routes/api/debug/clear/+server.ts +++ b/src/routes/api/debug/clear/+server.ts @@ -1,6 +1,6 @@ import { dev } from '$app/environment'; -import { getDataDir } from '$lib/server/data/dataDir.js'; -import { invalidatePortfolioGlobals } from '$lib/server/data/index.js'; +import { getDataDir } from '$lib/server/data/dataDir'; +import { invalidatePortfolioGlobals } from '$lib/server/data/index'; import { error, json } from '@sveltejs/kit'; import { rimraf } from 'rimraf'; diff --git a/src/routes/api/group/+server.ts b/src/routes/api/group/+server.ts index 2d1b7b0d..657f90e8 100644 --- a/src/routes/api/group/+server.ts +++ b/src/routes/api/group/+server.ts @@ -1,5 +1,5 @@ import { error, json } from '@sveltejs/kit'; -import { getPortfolioGlobals } from '$lib/server/data/index.js'; +import { getPortfolioGlobals } from '$lib/server/data/index'; export async function GET() { const data = await getPortfolioGlobals().catch(e => error(400, e)); diff --git a/src/routes/api/group/[groupId]/+server.ts b/src/routes/api/group/[groupId]/+server.ts index ee7ce98d..0c1d2556 100644 --- a/src/routes/api/group/[groupId]/+server.ts +++ b/src/routes/api/group/[groupId]/+server.ts @@ -1,11 +1,11 @@ import { error, json } from '@sveltejs/kit'; import { createGroup, deleteGroup, GroupInfoStruct, setGroupInfo } from '$lib/server/data/group'; -import { validateTokenFromRequest } from '$lib/server/auth'; +import { validateTokenFromRequest } from '$lib/server/auth/tokens'; import { object, string, validate } from 'superstruct'; import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/data/index'; import { validateId, validateName } from '$lib/validators'; -import { removeAllLinksToItem } from '$lib/server/links.js'; -import { setConfig } from '$lib/server/data/config.js'; +import { removeAllLinksToItem } from '$lib/server/links'; +import { setConfig } from '$lib/server/data/config'; export async function GET({ params }) { const groupId = params.groupId; diff --git a/src/routes/api/group/[groupId]/item/[itemId]/+server.ts b/src/routes/api/group/[groupId]/item/[itemId]/+server.ts index 1d45edca..cc18cda3 100644 --- a/src/routes/api/group/[groupId]/item/[itemId]/+server.ts +++ b/src/routes/api/group/[groupId]/item/[itemId]/+server.ts @@ -1,11 +1,11 @@ import { error, json } from '@sveltejs/kit'; import { getGroupInfo, setGroupInfo } from '$lib/server/data/group'; -import { validateTokenFromRequest } from '$lib/server/auth'; +import { validateTokenFromRequest } from '$lib/server/auth/tokens'; import { object, string, validate } from 'superstruct'; -import { createItem, setItemInfo, ItemInfoFullStruct, deleteItem } from '$lib/server/data/item.js'; -import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/data/index.js'; -import { validateId, validateName } from '$lib/validators.js'; -import { removeAllLinksToItem } from '$lib/server/links.js'; +import { createItem, setItemInfo, ItemInfoFullStruct, deleteItem } from '$lib/server/data/item'; +import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/data/index'; +import { validateId, validateName } from '$lib/validators'; +import { removeAllLinksToItem } from '$lib/server/links'; export async function GET({ params }) { const data = await getPortfolioGlobals().catch(e => error(400, e)); diff --git a/src/routes/api/group/[groupId]/item/[itemId]/link/+server.ts b/src/routes/api/group/[groupId]/item/[itemId]/link/+server.ts index 94076a72..e91d779b 100644 --- a/src/routes/api/group/[groupId]/item/[itemId]/link/+server.ts +++ b/src/routes/api/group/[groupId]/item/[itemId]/link/+server.ts @@ -1,9 +1,9 @@ import { error, json } from '@sveltejs/kit'; -import { validateTokenFromRequest } from '$lib/server/auth'; -import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/data/index.js'; +import { validateTokenFromRequest } from '$lib/server/auth/tokens'; +import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/data/index'; import { object, string, validate } from 'superstruct'; -import { LinkStyleStruct } from '$lib/server/data/item.js'; -import { changeLinkStyle, createLink, removeLinkFromItem } from '$lib/server/links.js'; +import { LinkStyleStruct } from '$lib/server/data/item'; +import { changeLinkStyle, createLink, removeLinkFromItem } from '$lib/server/links'; export async function POST({ params, request, cookies }) { const data = await getPortfolioGlobals().catch(e => error(400, e)); diff --git a/src/routes/api/group/[groupId]/item/[itemId]/readme/+server.ts b/src/routes/api/group/[groupId]/item/[itemId]/readme/+server.ts index c5a1e8c4..fa0cfd81 100644 --- a/src/routes/api/group/[groupId]/item/[itemId]/readme/+server.ts +++ b/src/routes/api/group/[groupId]/item/[itemId]/readme/+server.ts @@ -1,8 +1,8 @@ import { error, json } from '@sveltejs/kit'; -import { validateTokenFromRequest } from '$lib/server/auth'; +import { validateTokenFromRequest } from '$lib/server/auth/tokens'; import { assert, string } from 'superstruct'; -import { getItemInfo, setItemReadme } from '$lib/server/data/item.js'; -import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/data/index.js'; +import { getItemInfo, setItemReadme } from '$lib/server/data/item'; +import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/data/index'; export async function GET({ params }) { const data = await getPortfolioGlobals().catch(e => error(400, e)); diff --git a/src/routes/api/group/[groupId]/readme/+server.ts b/src/routes/api/group/[groupId]/readme/+server.ts index d7200e87..030d666e 100644 --- a/src/routes/api/group/[groupId]/readme/+server.ts +++ b/src/routes/api/group/[groupId]/readme/+server.ts @@ -1,8 +1,8 @@ import { error, json } from '@sveltejs/kit'; import { setGroupReadme } from '$lib/server/data/group'; -import { validateTokenFromRequest } from '$lib/server/auth'; +import { validateTokenFromRequest } from '$lib/server/auth/tokens'; import { assert, string } from 'superstruct'; -import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/data/index.js'; +import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/data/index'; export async function GET({ params }) { const groupId = params.groupId; diff --git a/src/routes/api/readme/+server.ts b/src/routes/api/readme/+server.ts index 545b12cd..348b26df 100644 --- a/src/routes/api/readme/+server.ts +++ b/src/routes/api/readme/+server.ts @@ -1,10 +1,10 @@ /** Endpoint for get/setting the README */ import { error, json } from '@sveltejs/kit'; -import { validateTokenFromRequest } from '$lib/server/auth.js'; +import { validateTokenFromRequest } from '$lib/server/auth/tokens'; import { object, string, validate } from 'superstruct'; -import { setReadme } from '$lib/server/data/readme.js'; -import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/data/index.js'; +import { setReadme } from '$lib/server/data/readme'; +import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/data/index'; export async function GET() { const data = await getPortfolioGlobals().catch(e => error(400, e)); diff --git a/src/routes/favicon/+server.ts b/src/routes/favicon/+server.ts index bb728fb0..e78ba31b 100644 --- a/src/routes/favicon/+server.ts +++ b/src/routes/favicon/+server.ts @@ -1,8 +1,8 @@ import fs from 'fs/promises'; import { error } from '@sveltejs/kit'; import mime from 'mime-types'; -import { getDataDir } from '$lib/server/data/dataDir.js'; -import { getPortfolioGlobals } from '$lib/server/index.js'; +import { getDataDir } from '$lib/server/data/dataDir'; +import { getPortfolioGlobals } from '$lib/server/index'; export async function GET({ setHeaders }) { const globals = await getPortfolioGlobals(); diff --git a/tests/backend/admin/auth/login.test.ts b/tests/backend/admin/auth/login.test.ts index 17f7d2a1..9f05c398 100644 --- a/tests/backend/admin/auth/login.test.ts +++ b/tests/backend/admin/auth/login.test.ts @@ -6,7 +6,7 @@ import { it, expect, beforeEach } from 'vitest'; import { setup } from '../../helpers'; import api from '$endpoints'; -import type { FirstRunCredentials } from '$lib/server/auth'; +import type { FirstRunCredentials } from '$lib/server/auth/tokens'; let credentials: FirstRunCredentials; diff --git a/tests/backend/admin/firstrun.test.ts b/tests/backend/admin/firstrun.test.ts index e2757c17..336c8216 100644 --- a/tests/backend/admin/firstrun.test.ts +++ b/tests/backend/admin/firstrun.test.ts @@ -8,7 +8,7 @@ import simpleGit, { CheckRepoActions } from 'simple-git'; import { readFile } from 'node:fs/promises'; import path from 'node:path'; import { version } from '$app/environment'; -import type { FirstRunCredentials } from '$lib/server/auth'; +import type { FirstRunCredentials } from '$lib/server/auth/tokens'; // Git clone takes a while, increase the test timeout vi.setConfig({ testTimeout: 15_000 }); diff --git a/tests/backend/admin/publickey/get.test.ts b/tests/backend/admin/publickey/get.test.ts new file mode 100644 index 00000000..e69de29b diff --git a/tests/backend/helpers.ts b/tests/backend/helpers.ts index 071805f5..ce858cb3 100644 --- a/tests/backend/helpers.ts +++ b/tests/backend/helpers.ts @@ -85,11 +85,12 @@ export function makeItemInfo(options: Partial = {}): ItemInfoFull /** Rewind the data repo's git repo to an earlier commit */ export async function forceRewindDataRepoGit(api: ApiClient) { + // A commit hash within MaddyGuthridge/portfolio-data const OLD_COMMIT_HASH = 'd7ef6fd7ef9bac4c24f5634e6b1e76d201507498'; // Forcefully move back a number of commits, then invalidate the data const git = simpleGit(getDataDir()); await git.reset(['--hard', OLD_COMMIT_HASH]); await api.admin.data.refresh(); // Attempt to make a commit just in case of data migrations - await api.admin.git.commit('Migrate data').catch(e => {void e}); + await api.admin.git.commit('Migrate data').catch(e => { void e }); } From 1679be5fec88cd155979b9d0f0c6dc9712485d26 Mon Sep 17 00:00:00 2001 From: Maddy Guthridge Date: Tue, 1 Oct 2024 22:43:23 +1000 Subject: [PATCH 04/15] Destroy auth secret on clear --- src/routes/api/debug/clear/+server.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/routes/api/debug/clear/+server.ts b/src/routes/api/debug/clear/+server.ts index 519418d0..3663b415 100644 --- a/src/routes/api/debug/clear/+server.ts +++ b/src/routes/api/debug/clear/+server.ts @@ -1,4 +1,5 @@ import { dev } from '$app/environment'; +import { destroyAuthSecret } from '$lib/server/auth/secret.js'; import { getDataDir } from '$lib/server/data/dataDir'; import { invalidatePortfolioGlobals } from '$lib/server/data/index'; import { error, json } from '@sveltejs/kit'; @@ -10,6 +11,9 @@ export async function DELETE({ cookies }) { await rimraf(getDataDir()); invalidatePortfolioGlobals(); + // Destroy the auth secret + await destroyAuthSecret(); + // Also remove token from their cookies cookies.delete('token', { path: '/' }); From 24b68a1ac4fa54eddc24d2edd72bf1382f21a97f Mon Sep 17 00:00:00 2001 From: Maddy Guthridge Date: Tue, 1 Oct 2024 23:14:51 +1000 Subject: [PATCH 05/15] Modify firstrun endpoint to use new design --- src/endpoints/admin/firstrun.ts | 10 ++-- src/lib/server/data/setup.ts | 59 +++++++++++++++++++ src/lib/server/git.ts | 6 +- src/lib/server/util.ts | 20 +++++++ src/lib/validators.ts | 16 +++--- src/routes/api/admin/firstrun/+server.ts | 72 ++++++++---------------- 6 files changed, 120 insertions(+), 63 deletions(-) create mode 100644 src/lib/server/data/setup.ts create mode 100644 src/lib/server/util.ts diff --git a/src/endpoints/admin/firstrun.ts b/src/endpoints/admin/firstrun.ts index fceae3e7..ebe3e255 100644 --- a/src/endpoints/admin/firstrun.ts +++ b/src/endpoints/admin/firstrun.ts @@ -1,5 +1,5 @@ /** Git repository endpoints */ -import type { FirstRunCredentials } from '$lib/server/auth/tokens'; +import type { FirstRunCredentials } from '$lib/server/auth/setup'; import { apiFetch, json } from '../fetch'; /** @@ -9,13 +9,15 @@ import { apiFetch, json } from '../fetch'; * @param branch The branch to check-out */ export default async function ( - repoUrl: string | null, - branch: string | null, + username: string, + password: string, + repoUrl?: string | undefined, + branch?: string | undefined, ) { return json(apiFetch( 'POST', '/api/admin/firstrun', undefined, - { repoUrl, branch }, + { username, password, repoUrl, branch }, )) as Promise<{ credentials: FirstRunCredentials, firstTime: boolean }>; } diff --git a/src/lib/server/data/setup.ts b/src/lib/server/data/setup.ts new file mode 100644 index 00000000..284bf56c --- /dev/null +++ b/src/lib/server/data/setup.ts @@ -0,0 +1,59 @@ +/** + * Code for setting up public data. + */ + +import { mkdir } from 'fs/promises'; +import { runSshKeyscan, setupGitRepo, urlRequiresSsh } from '../git'; +import { dataDirContainsData, getDataDir } from './dataDir'; +import { initConfig } from './config'; +import { initReadme } from './readme'; +import { getPortfolioGlobals, invalidatePortfolioGlobals } from '.'; +import { error } from '@sveltejs/kit'; + +/** + * Set up the data directory. + * + * This returns whether it is the data is new (ie the git repo is empty, or git + * isn't in use). + */ +export async function setupData(repoUrl?: string, branch?: string): Promise { + // If we were given a repoUrl, set it up + if (repoUrl) { + if (urlRequiresSsh(repoUrl)) { + await runSshKeyscan(repoUrl); + } + await setupGitRepo(repoUrl, branch); + } else { + // Otherwise, just create the data dir empty + // Discard errors for this, as the dir may already exist + await mkdir(getDataDir()).catch(() => { }); + // Currently, gitignore is not needed, since private data is now stored + // separately + // await setupGitignore(); + } + + /** + * Whether the data repo is empty -- true if data dir was empty before + * firstrun. + */ + let firstTime = false; + + // If data dir is empty, set up default configuration + if (!await dataDirContainsData()) { + firstTime = true; + await initConfig(); + // Also set up a default README + await initReadme(); + } + + // Attempt to forcefully load global data + invalidatePortfolioGlobals(); + try { + await getPortfolioGlobals(); + } catch (e) { + console.log(e); + error(400, `Error loading data: ${e}`); + } + + return firstTime; +} diff --git a/src/lib/server/git.ts b/src/lib/server/git.ts index 5a04733c..5f0d35ae 100644 --- a/src/lib/server/git.ts +++ b/src/lib/server/git.ts @@ -100,7 +100,7 @@ export async function getRepoStatus(): Promise { } /** Set up the data dir given a git repo URL and branch name */ -export async function setupGitRepo(repo: string, branch: string | null) { +export async function setupGitRepo(repo: string, branch?: string | null) { // Check whether the data repo is set up if (await serverIsSetUp()) { throw error(403, 'Data repo is already set up'); @@ -109,7 +109,9 @@ export async function setupGitRepo(repo: string, branch: string | null) { const dir = getDataDir(); // Set up branch options - const options: Record = branch === null ? {} : { '--branch': branch }; + // FIXME: This may cause git to only track that branch on the remote, making + // switching branches impossible. + const options: Record = branch ? { '--branch': branch } : {}; try { await simpleGit().clone(repo, dir, options); diff --git a/src/lib/server/util.ts b/src/lib/server/util.ts new file mode 100644 index 00000000..f6eace2e --- /dev/null +++ b/src/lib/server/util.ts @@ -0,0 +1,20 @@ +import { error } from "@sveltejs/kit"; +import { create, type Struct } from "superstruct"; + +/** + * A wrapper around superstruct's assert, making it async to make error + * handling easier. + * + * By default this throws an error 400. + */ +export async function applyStruct( + value: unknown, + struct: Struct, + message?: string, +): Promise { + try { + return create(value, struct, message); + } catch (e) { + error(400, `${e}`); + } +} diff --git a/src/lib/validators.ts b/src/lib/validators.ts index e1386653..1c1d4e8e 100644 --- a/src/lib/validators.ts +++ b/src/lib/validators.ts @@ -1,27 +1,27 @@ import { error } from '@sveltejs/kit'; -/** Regex for matching group IDs */ +/** Regex for matching ID strings */ const idValidatorRegex = /^[a-z0-9-.]+$/; /** Ensure that the given ID string is valid */ -export function validateId(id: string): string { +export function validateId(type: string, id: string): string { if (!id.trim().length) { - return error(400, `Group ID '${id}' is empty`); + return error(400, `${type} '${id}' is empty`); } if (!idValidatorRegex.test(id)) { - return error(400, `Group ID '${id}' is contains illegal characters`); + return error(400, `${type} '${id}' is contains illegal characters`); } if (id.startsWith('.')) { - return error(400, `Group ID '${id}' has a leading dot`); + return error(400, `${type} '${id}' has a leading dot`); } if (id.endsWith('.')) { - return error(400, `Group ID '${id}' has a trailing dot`); + return error(400, `${type} '${id}' has a trailing dot`); } if (id.startsWith('-')) { - return error(400, `Group ID '${id}' has a leading dash`); + return error(400, `${type} '${id}' has a leading dash`); } if (id.endsWith('-')) { - return error(400, `Group ID '${id}' has a trailing dash`); + return error(400, `${type} '${id}' has a trailing dash`); } return id; } diff --git a/src/routes/api/admin/firstrun/+server.ts b/src/routes/api/admin/firstrun/+server.ts index ab30f4f4..ec2ea5c4 100644 --- a/src/routes/api/admin/firstrun/+server.ts +++ b/src/routes/api/admin/firstrun/+server.ts @@ -1,67 +1,41 @@ import { error, json } from '@sveltejs/kit'; -import { dataDirContainsData, serverIsSetUp, getDataDir } from '$lib/server/data/dataDir'; -import { mkdir } from 'fs/promises'; -import { runSshKeyscan, setupGitignore, setupGitRepo, urlRequiresSsh } from '$lib/server/git'; +import { serverIsSetUp } from '$lib/server/data/dataDir'; import { authSetup } from '$lib/server/auth/setup'; -import { initConfig } from '$lib/server/data/config'; -import { initReadme } from '$lib/server/data/readme'; -import { getPortfolioGlobals, invalidatePortfolioGlobals } from '$lib/server/data/index'; +import { object, optional, string } from 'superstruct'; +import { applyStruct } from '$lib/server/util'; +import { validateId } from '$lib/validators'; +import * as validator from 'validator'; +import { setupData } from '$lib/server/data/setup.js'; + +const FirstRunOptions = object({ + username: string(), + password: string(), + repoUrl: optional(string()), + branch: optional(string()), +}); // FIXME: Make this accept a username and password export async function POST({ request, cookies }) { - const { repoUrl, branch }: { repoUrl: string | null, branch: string | null } - = await request.json(); + const options = await applyStruct(await request.json(), FirstRunOptions); if (await serverIsSetUp()) { error(403); } - if (repoUrl === '') { - error(400, 'Repo URL must be a non-empty string or `null`'); - } - if (branch === '') { - error(400, 'Branch must be a non-empty string or `null`'); + // Validate username and password + validateId('username', options.username); + if (!validator.isStrongPassword(options.password)) { + error(400, 'Password is not strong enough'); } - // If we were given a repoUrl, set it up - if (repoUrl) { - if (urlRequiresSsh(repoUrl)) { - await runSshKeyscan(repoUrl); - } - await setupGitRepo(repoUrl, branch); - } else { - // Otherwise, just create the data dir empty - // Discard errors for this, as the dir may already exist - await mkdir(getDataDir()).catch(() => { }); - // Setup a gitignore just in case - await setupGitignore(); + if (options.branch && !options.repoUrl) { + error(400, 'Branch must not be given if repo URL is not provided'); } - // Now set up auth - const credentials = await authSetup(cookies); - - /** - * Whether the data repo is empty -- true if data dir was empty before - * firstrun. - */ - let firstTime = false; + const firstTime = await setupData(options.repoUrl, options.branch); - // If data dir is empty, set up default configuration - if (!await dataDirContainsData()) { - firstTime = true; - await initConfig(); - // Also set up a default README - await initReadme(); - } - - // Attempt to forcefully load global data - invalidatePortfolioGlobals(); - try { - await getPortfolioGlobals(); - } catch (e) { - console.log(e); - error(400, `Error loading data: ${e}`); - } + // Now set up auth + const credentials = await authSetup(options.username, options.password, cookies); return json({ credentials, firstTime }, { status: 200 }); } From 0511b92886d6ee12e8bb58a6e5b882c035af5924 Mon Sep 17 00:00:00 2001 From: Maddy Guthridge Date: Tue, 1 Oct 2024 23:56:07 +1000 Subject: [PATCH 06/15] Fix rerun test cases --- package-lock.json | 4 +- src/lib/server/auth/secret.ts | 6 +- src/lib/server/data/index.ts | 2 + src/routes/api/admin/firstrun/+server.ts | 11 +- src/routes/api/debug/clear/+server.ts | 7 +- tests/backend/admin/firstrun.test.ts | 140 +++++++++++------------ tests/backend/helpers.ts | 2 +- 7 files changed, 81 insertions(+), 91 deletions(-) diff --git a/package-lock.json b/package-lock.json index 6b3feb66..42a76739 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "minifolio", - "version": "0.4.0", + "version": "0.5.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "minifolio", - "version": "0.4.0", + "version": "0.5.0", "license": "GPL-3.0-only", "dependencies": { "child-process-promise": "^2.2.1", diff --git a/src/lib/server/auth/secret.ts b/src/lib/server/auth/secret.ts index 0ccaf3d2..0e87326a 100644 --- a/src/lib/server/auth/secret.ts +++ b/src/lib/server/auth/secret.ts @@ -23,6 +23,7 @@ export async function getAuthSecret(): Promise { /** Generate and store a new auth secret, returning its value */ export async function generateAuthSecret(): Promise { + await fs.mkdir(getPrivateDataDir()).catch(() => { }); const secret = nanoid(); authSecret = secret; await fs.writeFile(getAuthSecretPath(), secret, { encoding: 'utf-8' }); @@ -30,9 +31,8 @@ export async function generateAuthSecret(): Promise { } /** - * Destroy the auth secret, removing it from memory and from the file system. + * Invalidate the in-memory copy of the auth secret. */ -export async function destroyAuthSecret() { +export function invalidateAuthSecret() { authSecret = undefined; - await fs.unlink(getAuthSecretPath()); } diff --git a/src/lib/server/data/index.ts b/src/lib/server/data/index.ts index fe97687f..92e4fa2e 100644 --- a/src/lib/server/data/index.ts +++ b/src/lib/server/data/index.ts @@ -17,6 +17,7 @@ import { getItemData, listItems, type ItemData } from './item'; import { invalidateLocalConfigCache } from './localConfig'; import migrate from './migrations'; import { getReadme } from './readme'; +import { invalidateAuthSecret } from '../auth/secret'; /** Public global data for the portfolio */ export interface PortfolioGlobals { @@ -97,5 +98,6 @@ export async function getPortfolioGlobals(): Promise { export function invalidatePortfolioGlobals() { portfolioGlobals = undefined; invalidateLocalConfigCache(); + invalidateAuthSecret(); // console.log('Data invalidated'); } diff --git a/src/routes/api/admin/firstrun/+server.ts b/src/routes/api/admin/firstrun/+server.ts index ec2ea5c4..2c2e2d76 100644 --- a/src/routes/api/admin/firstrun/+server.ts +++ b/src/routes/api/admin/firstrun/+server.ts @@ -1,22 +1,23 @@ import { error, json } from '@sveltejs/kit'; import { serverIsSetUp } from '$lib/server/data/dataDir'; import { authSetup } from '$lib/server/auth/setup'; -import { object, optional, string } from 'superstruct'; +import { object, optional, string, type Infer } from 'superstruct'; import { applyStruct } from '$lib/server/util'; import { validateId } from '$lib/validators'; -import * as validator from 'validator'; +import validator from 'validator'; import { setupData } from '$lib/server/data/setup.js'; -const FirstRunOptions = object({ +const FirstRunOptionsStruct = object({ username: string(), password: string(), repoUrl: optional(string()), branch: optional(string()), }); -// FIXME: Make this accept a username and password +export type FirstRunOptions = Infer; + export async function POST({ request, cookies }) { - const options = await applyStruct(await request.json(), FirstRunOptions); + const options = await applyStruct(await request.json(), FirstRunOptionsStruct); if (await serverIsSetUp()) { error(403); diff --git a/src/routes/api/debug/clear/+server.ts b/src/routes/api/debug/clear/+server.ts index 3663b415..6d398013 100644 --- a/src/routes/api/debug/clear/+server.ts +++ b/src/routes/api/debug/clear/+server.ts @@ -1,6 +1,5 @@ import { dev } from '$app/environment'; -import { destroyAuthSecret } from '$lib/server/auth/secret.js'; -import { getDataDir } from '$lib/server/data/dataDir'; +import { getDataDir, getPrivateDataDir } from '$lib/server/data/dataDir'; import { invalidatePortfolioGlobals } from '$lib/server/data/index'; import { error, json } from '@sveltejs/kit'; import { rimraf } from 'rimraf'; @@ -9,11 +8,9 @@ export async function DELETE({ cookies }) { if (!dev) throw error(404); // Delete data directory await rimraf(getDataDir()); + await rimraf(getPrivateDataDir()); invalidatePortfolioGlobals(); - // Destroy the auth secret - await destroyAuthSecret(); - // Also remove token from their cookies cookies.delete('token', { path: '/' }); diff --git a/tests/backend/admin/firstrun.test.ts b/tests/backend/admin/firstrun.test.ts index 336c8216..740fb49f 100644 --- a/tests/backend/admin/firstrun.test.ts +++ b/tests/backend/admin/firstrun.test.ts @@ -3,115 +3,105 @@ */ import api from '$endpoints'; import gitRepos from '../gitRepos'; -import { it, test, describe, expect, vi, beforeEach } from 'vitest'; +import { it, describe, expect, vi } from 'vitest'; import simpleGit, { CheckRepoActions } from 'simple-git'; -import { readFile } from 'node:fs/promises'; -import path from 'node:path'; -import { version } from '$app/environment'; -import type { FirstRunCredentials } from '$lib/server/auth/tokens'; +// Yucky import +import type { FirstRunOptions } from '../../../src/routes/api/admin/firstrun/+server'; +import { invalidIds, validIds } from '../consts'; +import { getDataDir } from '$lib/server/data/dataDir'; // Git clone takes a while, increase the test timeout vi.setConfig({ testTimeout: 15_000 }); -const REPO_PATH = process.env.DATA_REPO_PATH as string; +const REPO_PATH = getDataDir(); +/** Make a simpleGit for the repo */ const repo = () => simpleGit(REPO_PATH); +/** Helper function for firstrun testing */ +async function firstrun(options: Partial = {}) { + const defaults: FirstRunOptions = { + username: 'admin', + password: 'abc123ABC!', + repoUrl: undefined, + branch: undefined, + }; + + const combined = { ...defaults, ...options }; + + return api().admin.firstrun( + combined.username, + combined.password, + combined.repoUrl, + combined.branch, + ); +} + +describe('username', () => { + // Only ID characters are allowed as usernames + it.each(invalidIds)('Rejects invalid usernames ($case)', async ({ id }) => { + await expect(firstrun({ username: id })) + .rejects.toMatchObject({ code: 400 }); + }); + + it.each(validIds)('Accepts valid usernames ($case)', async ({ id }) => { + await expect(firstrun({ username: id })).toResolve(); + }); +}); + +describe('password', () => { + it('Rejects insecure passwords', async () => { + await expect(firstrun({ password: 'insecure' })) + .rejects.toMatchObject({ code: 400 }); + }); +}); + describe('git', () => { it('Clones repo to the default branch when URL is provided', async () => { - await api().admin.firstrun(gitRepos.TEST_REPO_RW, null); - await expect(repo().checkIsRepo(CheckRepoActions.IS_REPO_ROOT)).resolves.toStrictEqual(true); + await firstrun({ repoUrl: gitRepos.TEST_REPO_RW }); + await expect(repo().checkIsRepo(CheckRepoActions.IS_REPO_ROOT)) + .resolves.toStrictEqual(true); // Default branch for this repo is 'main' await expect(repo().status()).resolves.toMatchObject({ current: 'main' }); }); it("Gives an error if the repo doesn't contain a config.json, but isn't empty", async () => { - await expect( - api().admin.firstrun(gitRepos.NON_PORTFOLIO, null) - ).rejects.toMatchObject({ code: 400 }); + await expect(firstrun({ repoUrl: gitRepos.NON_PORTFOLIO })) + .rejects.toMatchObject({ code: 400 }); }, 10000); it("Doesn't give an error if the repository is entirely empty", async () => { - await api().admin.firstrun(gitRepos.EMPTY, null); - await expect(repo().checkIsRepo(CheckRepoActions.IS_REPO_ROOT)).resolves.toStrictEqual(true); + await firstrun({ repoUrl: gitRepos.EMPTY }); + await expect(repo().checkIsRepo(CheckRepoActions.IS_REPO_ROOT)) + .resolves.toStrictEqual(true); }); - it('Checks out a branch when one is given', async () => { - await api().admin.firstrun(gitRepos.TEST_REPO_RW, 'example'); + it.only('Checks out a branch when one is given', async () => { + await firstrun({ repoUrl: gitRepos.TEST_REPO_RW, branch: 'example' }); // Check branch name matches await expect(repo().status()).resolves.toMatchObject({ current: 'example' }); }); it('Gives an error if the repo URL cannot be cloned', async () => { - await expect( - api().admin.firstrun(gitRepos.INVALID, null) - ).rejects.toMatchObject({ code: 400 }); - }); - - it('Gives an error if repo URL is an empty string', async () => { - await expect( - api().admin.firstrun('', null) - ).rejects.toMatchObject({ code: 400 }); - }); - - it('Gives an error if branch is an empty string', async () => { - await expect( - api().admin.firstrun(gitRepos.EMPTY, '') - ).rejects.toMatchObject({ code: 400 }); + await expect(firstrun({ repoUrl: gitRepos.INVALID })) + .rejects.toMatchObject({ code: 400 }); }); }); -it('Gives auth credentials on success', async () => { - await expect(api().admin.firstrun(null, null)) +it('Gives a token on success', async () => { + await expect(firstrun()) .resolves.toMatchObject({ - credentials: { - username: 'admin', - password: expect.any(String), - token: expect.any(String), - } + credentials: { token: expect.any(String) } }); }); it('Blocks access if data is already set up', async () => { - await api().admin.firstrun(null, null); - await expect( - api().admin.firstrun(gitRepos.TEST_REPO_RW, null) - ).rejects.toMatchObject({ code: 403 }); + await firstrun(); + await expect(firstrun()).rejects.toMatchObject({ code: 403 }); }); it("Doesn't clone repo when no URL provided", async () => { - await api().admin.firstrun(null, null); - await expect(repo().checkIsRepo(CheckRepoActions.IS_REPO_ROOT)).resolves.toStrictEqual(false); -}); - -describe('Sets up required starting files', () => { - let firstRun: FirstRunCredentials; - beforeEach(async () => { - firstRun = (await api().admin.firstrun(null, null)).credentials; - }); - - test('config.local.json', async () => { - const now = Math.floor(Date.now() / 1000); - const config = await readFile(path.join(REPO_PATH, 'config.local.json'), { encoding: 'utf-8' }); - - expect(JSON.parse(config)).toStrictEqual({ - auth: { - username: firstRun.username, - password: { - // Don't expect anything specific here - hash: expect.any(String), - salt: expect.any(String), - }, - sessions: { - notBefore: expect.toBeWithin(now - 1, now + 1), - revokedSessions: {}, - }, - }, - version, - }); - }); - - test.todo('config.json'); + await firstrun(); + await expect(repo().checkIsRepo(CheckRepoActions.IS_REPO_ROOT)) + .resolves.toStrictEqual(false); }); - -it.todo('Leaves the .gitignore as-is if config.local.json is already there'); diff --git a/tests/backend/helpers.ts b/tests/backend/helpers.ts index ce858cb3..387dc104 100644 --- a/tests/backend/helpers.ts +++ b/tests/backend/helpers.ts @@ -9,7 +9,7 @@ import { getDataDir } from '$lib/server/data/dataDir'; /** Set up the server, returning (amongst other things) an API client */ export async function setup(repoUrl?: string, branch?: string) { const credentials - = (await api(undefined).admin.firstrun(repoUrl ?? null, branch ?? null)) + = (await api(undefined).admin.firstrun('admin', 'abc123ABC!', repoUrl, branch)) .credentials; return { api: api(credentials.token), From 0cf16e8e5ad160c0e01f0f0cd529e553e26173ec Mon Sep 17 00:00:00 2001 From: Maddy Guthridge Date: Wed, 2 Oct 2024 00:22:51 +1000 Subject: [PATCH 07/15] Add migration to v0.6.0 --- src/lib/server/data/localConfig.ts | 2 +- src/lib/server/data/migrations/index.ts | 30 +++++++--------- src/lib/server/data/migrations/shared.ts | 22 ++++++++++-- src/lib/server/data/migrations/unsafeLoad.ts | 5 +++ src/lib/server/data/migrations/v0.2.0.ts | 4 +-- src/lib/server/data/migrations/v0.3.0.ts | 8 +++-- src/lib/server/data/migrations/v0.5.0.ts | 37 ++++++++++++++++++++ src/lib/server/data/migrations/v0.6.0.ts | 7 ---- 8 files changed, 83 insertions(+), 32 deletions(-) create mode 100644 src/lib/server/data/migrations/v0.5.0.ts delete mode 100644 src/lib/server/data/migrations/v0.6.0.ts diff --git a/src/lib/server/data/localConfig.ts b/src/lib/server/data/localConfig.ts index 11e81d6d..5d52ca42 100644 --- a/src/lib/server/data/localConfig.ts +++ b/src/lib/server/data/localConfig.ts @@ -57,7 +57,7 @@ export type ConfigLocalJson = Infer; /** Cache of the local config to speed up operations */ let localConfigCache: ConfigLocalJson | undefined; -/** Return the local configuration, stored in `/data/config.local.json` */ +/** Return the local configuration, stored in `/private-data/config.local.json` */ export async function getLocalConfig(): Promise { if (localConfigCache) { return localConfigCache; diff --git a/src/lib/server/data/migrations/index.ts b/src/lib/server/data/migrations/index.ts index d8d6bffd..0c70fea0 100644 --- a/src/lib/server/data/migrations/index.ts +++ b/src/lib/server/data/migrations/index.ts @@ -1,33 +1,29 @@ import { version } from '$app/environment'; -import { getConfig, setConfig } from '../config'; -import { getDataDir } from '../dataDir'; -import { getLocalConfig, setLocalConfig } from '../localConfig'; +import { getDataDir, getPrivateDataDir } from '../dataDir'; +import { updateConfigVersions } from './shared'; import migrateFromV010 from './v0.1.0'; import migrateFromV020 from './v0.2.0'; import migrateFromV030 from './v0.3.0'; +import migrateFromV050 from './v0.5.0'; import semver from 'semver'; -/** Update config versions (only for minor, non-breaking changes to config.json) */ -export async function updateConfigVersions(_dataDir: string) { - const config = await getConfig(); - config.version = version; - await setConfig(config); - const configLocal = await getLocalConfig(); - configLocal.version = version; - await setLocalConfig(configLocal); -} -export type MigrationFunction = (dataDir: string) => Promise; +export type MigrationFunction = ( + dataDir: string, + privateDataDir: string, +) => Promise; /** Lookup table of migrations */ const migrations: Record = { '~0.1.0': migrateFromV010, '~0.2.0': migrateFromV020, '~0.3.0': migrateFromV030, - // No major changes to data format this release - '~0.4.0': updateConfigVersions, + // No major changes to data format between 0.4 and 0.5, so just use the 0.5 + // function + '~0.4.0': migrateFromV050, + '~0.5.0': migrateFromV050, // Pre-empt future releases - '~0.5.0': updateConfigVersions, + '~0.6.0': updateConfigVersions, }; /** Perform a migration from the given version */ @@ -44,7 +40,7 @@ export default async function migrate(oldVersion: string) { // calls getConfig and getLocalConfig without specifying the desired // directory to load from). try { - await migrateFunction(getDataDir()); + await migrateFunction(getDataDir(), getPrivateDataDir()); console.log('Migration success'); return; } catch (e) { diff --git a/src/lib/server/data/migrations/shared.ts b/src/lib/server/data/migrations/shared.ts index d420c01a..a65a5ef3 100644 --- a/src/lib/server/data/migrations/shared.ts +++ b/src/lib/server/data/migrations/shared.ts @@ -1,14 +1,32 @@ /** * Shared helper functions for common data migration actions. */ -import { dev } from '$app/environment'; +import { dev, version } from '$app/environment'; import fs from 'fs/promises'; +import { getConfig, setConfig } from '../config'; +import { getLocalConfig, setLocalConfig } from '../localConfig'; +import { serverIsSetUp } from '../dataDir'; + +/** Update config versions (only for minor, non-breaking changes to config.json) */ +export async function updateConfigVersions() { + const config = await getConfig(); + config.version = version; + await setConfig(config); + // Only migrate local config if it is created + if (await serverIsSetUp()) { + const configLocal = await getLocalConfig(); + configLocal.version = version; + await setLocalConfig(configLocal); + } +} /** Move config.local.json to the private data directory */ export async function moveLocalConfig(dataDir: string, privateDataDir: string) { const originalPath = `${dataDir}/config.local.json`; const newPath = `${privateDataDir}/config.local.json`; - await fs.rename(originalPath, newPath); + // Discard error (file not found), which occurs when cloning a new repo, + // since the auth info isn't set up yet + await fs.rename(originalPath, newPath).catch(() => { }); } /** Write auth secret from environment variable */ diff --git a/src/lib/server/data/migrations/unsafeLoad.ts b/src/lib/server/data/migrations/unsafeLoad.ts index 50bb6b65..e5f36dfb 100644 --- a/src/lib/server/data/migrations/unsafeLoad.ts +++ b/src/lib/server/data/migrations/unsafeLoad.ts @@ -6,6 +6,11 @@ export async function unsafeLoadConfig(dataDir: string): Promise { return JSON.parse(await fs.readFile(`${dataDir}/config.json`, { encoding: 'utf-8' })); } +/** Load old config.local.json */ +export async function unsafeLoadLocalConfig(privateDataDir: string): Promise { + return JSON.parse(await fs.readFile(`${privateDataDir}/config.local.json`, { encoding: 'utf-8' })); +} + /** Load item info in old format */ export async function unsafeLoadItemInfo(dataDir: string, groupId: string, itemId: string): Promise { return JSON.parse(await fs.readFile(`${dataDir}/${groupId}/${itemId}/info.json`, { encoding: 'utf-8' })); diff --git a/src/lib/server/data/migrations/v0.2.0.ts b/src/lib/server/data/migrations/v0.2.0.ts index e6bef6cf..7d74728d 100644 --- a/src/lib/server/data/migrations/v0.2.0.ts +++ b/src/lib/server/data/migrations/v0.2.0.ts @@ -8,7 +8,7 @@ import { listGroups, setGroupInfo, type GroupInfo } from '../group'; import { unsafeLoadGroupInfo } from './unsafeLoad'; import migrateV030 from './v0.3.0'; -export default async function migrate(dataDir: string) { +export default async function migrate(dataDir: string, privateDataDir: string) { console.log(`Migrate from v0.2.x -> ${version}`); // For each group @@ -22,5 +22,5 @@ export default async function migrate(dataDir: string) { } // Also perform v0.3.0 migration - await migrateV030(dataDir); + await migrateV030(dataDir, privateDataDir); } diff --git a/src/lib/server/data/migrations/v0.3.0.ts b/src/lib/server/data/migrations/v0.3.0.ts index a0f79d08..f65122c0 100644 --- a/src/lib/server/data/migrations/v0.3.0.ts +++ b/src/lib/server/data/migrations/v0.3.0.ts @@ -1,12 +1,13 @@ /** Migration from 0.3.0 -> current */ -import { updateConfigVersions } from '.'; +import { updateConfigVersions } from './shared'; import { setConfig, type ConfigJson } from '../config'; import { listGroups, setGroupInfo, type GroupInfo } from '../group'; import { listItems, setItemInfo, type ItemInfoFull } from '../item'; +import { moveLocalConfig } from './shared'; import { unsafeLoadConfig, unsafeLoadGroupInfo, unsafeLoadItemInfo } from './unsafeLoad'; -export default async function migrate(dataDir: string) { +export default async function migrate(dataDir: string, privateDataDir: string) { await migrateConfig(dataDir); for (const groupId of await listGroups()) { await migrateGroup(dataDir, groupId); @@ -15,7 +16,8 @@ export default async function migrate(dataDir: string) { } } - await updateConfigVersions(dataDir); + await moveLocalConfig(dataDir, privateDataDir); + await updateConfigVersions(); } async function migrateConfig(dataDir: string) { diff --git a/src/lib/server/data/migrations/v0.5.0.ts b/src/lib/server/data/migrations/v0.5.0.ts new file mode 100644 index 00000000..0de2b899 --- /dev/null +++ b/src/lib/server/data/migrations/v0.5.0.ts @@ -0,0 +1,37 @@ +/** + * Migrate to v0.6.0 + * + * Primary changes: + * + * * Move `config.local.json` to the new private data directory. + */ + +import { nanoid } from "nanoid"; +import { moveLocalConfig, updateConfigVersions } from "./shared"; +import { unsafeLoadLocalConfig } from "./unsafeLoad"; +import { setLocalConfig } from "../localConfig"; + +export default async function migrate(dataDir: string, privateDataDir: string) { + await moveLocalConfig(dataDir, privateDataDir); + await updateLocalConfig(privateDataDir); + await updateConfigVersions(); +} + +async function updateLocalConfig(privateDataDir: string) { + // Too lazy to make this type-safe + const config = await unsafeLoadLocalConfig(privateDataDir) as any; + + const userInfo = config.auth; + + if (userInfo === null) { + // Auth disabled, register no users + config.auth = null; + } else { + // Generate user ID + const userId = nanoid(); + config.auth = { + [userId]: userInfo + }; + } + await setLocalConfig(config); +} diff --git a/src/lib/server/data/migrations/v0.6.0.ts b/src/lib/server/data/migrations/v0.6.0.ts deleted file mode 100644 index fc0dba17..00000000 --- a/src/lib/server/data/migrations/v0.6.0.ts +++ /dev/null @@ -1,7 +0,0 @@ -/** - * Migrate to v0.6.0 - * - * Primary changes: - * - * * Move `config.local.json` to the new private data directory. - */ From 7604ed4b87414a19009ef04e60239b9533ef9169 Mon Sep 17 00:00:00 2001 From: Maddy Guthridge Date: Wed, 2 Oct 2024 00:23:04 +1000 Subject: [PATCH 08/15] Bump version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 2879d380..ba10ef7d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "minifolio", - "version": "0.5.0", + "version": "0.6.0", "private": true, "license": "GPL-3.0-only", "scripts": { From c5f94126617c652585f7292e62694a15cd120579 Mon Sep 17 00:00:00 2001 From: Maddy Guthridge Date: Wed, 2 Oct 2024 00:37:46 +1000 Subject: [PATCH 09/15] Fix firstrun tests --- src/lib/server/data/migrations/v0.5.0.ts | 5 ++++- src/routes/api/group/[groupId]/+server.ts | 2 +- src/routes/api/group/[groupId]/item/[itemId]/+server.ts | 2 +- tests/backend/admin/firstrun.test.ts | 2 +- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/lib/server/data/migrations/v0.5.0.ts b/src/lib/server/data/migrations/v0.5.0.ts index 0de2b899..22604868 100644 --- a/src/lib/server/data/migrations/v0.5.0.ts +++ b/src/lib/server/data/migrations/v0.5.0.ts @@ -10,10 +10,13 @@ import { nanoid } from "nanoid"; import { moveLocalConfig, updateConfigVersions } from "./shared"; import { unsafeLoadLocalConfig } from "./unsafeLoad"; import { setLocalConfig } from "../localConfig"; +import { serverIsSetUp } from "../dataDir"; export default async function migrate(dataDir: string, privateDataDir: string) { await moveLocalConfig(dataDir, privateDataDir); - await updateLocalConfig(privateDataDir); + if (await serverIsSetUp()) { + await updateLocalConfig(privateDataDir); + } await updateConfigVersions(); } diff --git a/src/routes/api/group/[groupId]/+server.ts b/src/routes/api/group/[groupId]/+server.ts index 0c1d2556..9c6574cf 100644 --- a/src/routes/api/group/[groupId]/+server.ts +++ b/src/routes/api/group/[groupId]/+server.ts @@ -25,7 +25,7 @@ export async function POST({ params, request, cookies }) { await validateTokenFromRequest({ request, cookies }); // Validate group ID - const groupId = validateId(params.groupId); + const groupId = validateId('Group ID', params.groupId); const [err, body] = validate(await request.json(), object({ name: string(), description: string() })); if (err) { diff --git a/src/routes/api/group/[groupId]/item/[itemId]/+server.ts b/src/routes/api/group/[groupId]/item/[itemId]/+server.ts index cc18cda3..394574c8 100644 --- a/src/routes/api/group/[groupId]/item/[itemId]/+server.ts +++ b/src/routes/api/group/[groupId]/item/[itemId]/+server.ts @@ -29,7 +29,7 @@ export async function POST({ params, request, cookies }) { // Ensure group exists await getGroupInfo(groupId).catch(e => error(404, e)); - validateId(itemId); + validateId('Item ID', itemId); const [err, body] = validate(await request.json(), object({ name: string(), description: string() })); if (err) { diff --git a/tests/backend/admin/firstrun.test.ts b/tests/backend/admin/firstrun.test.ts index 740fb49f..cdb397ce 100644 --- a/tests/backend/admin/firstrun.test.ts +++ b/tests/backend/admin/firstrun.test.ts @@ -76,7 +76,7 @@ describe('git', () => { .resolves.toStrictEqual(true); }); - it.only('Checks out a branch when one is given', async () => { + it('Checks out a branch when one is given', async () => { await firstrun({ repoUrl: gitRepos.TEST_REPO_RW, branch: 'example' }); // Check branch name matches await expect(repo().status()).resolves.toMatchObject({ current: 'example' }); From c63d167b97e9348876ca99ebbb82134bcd53e8f4 Mon Sep 17 00:00:00 2001 From: Maddy Guthridge Date: Wed, 2 Oct 2024 00:42:50 +1000 Subject: [PATCH 10/15] Fix failing login tests --- src/endpoints/admin/firstrun.ts | 3 +-- src/lib/server/auth/setup.ts | 9 ++------- src/routes/api/admin/auth/login/+server.ts | 2 +- src/routes/api/admin/firstrun/+server.ts | 4 ++-- tests/backend/admin/auth/login.test.ts | 5 ++--- tests/backend/helpers.ts | 14 +++++++++----- 6 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/endpoints/admin/firstrun.ts b/src/endpoints/admin/firstrun.ts index ebe3e255..a9ebda3a 100644 --- a/src/endpoints/admin/firstrun.ts +++ b/src/endpoints/admin/firstrun.ts @@ -1,5 +1,4 @@ /** Git repository endpoints */ -import type { FirstRunCredentials } from '$lib/server/auth/setup'; import { apiFetch, json } from '../fetch'; /** @@ -19,5 +18,5 @@ export default async function ( '/api/admin/firstrun', undefined, { username, password, repoUrl, branch }, - )) as Promise<{ credentials: FirstRunCredentials, firstTime: boolean }>; + )) as Promise<{ token: string, firstTime: boolean }>; } diff --git a/src/lib/server/auth/setup.ts b/src/lib/server/auth/setup.ts index 9c367054..c1313f1f 100644 --- a/src/lib/server/auth/setup.ts +++ b/src/lib/server/auth/setup.ts @@ -10,11 +10,6 @@ import { generateToken } from './tokens'; import type { Cookies } from '@sveltejs/kit'; import { generateAuthSecret } from './secret'; -/** Info returned during first-run */ -export interface FirstRunCredentials { - token: string, -} - /** * Set up auth information. * @@ -28,7 +23,7 @@ export async function authSetup( username: string, password: string, cookies?: Cookies, -): Promise { +): Promise { // 1. Set up the auth secret await generateAuthSecret(); @@ -59,5 +54,5 @@ export async function authSetup( }; await setLocalConfig(config); - return { token: await generateToken(userId, cookies) }; + return generateToken(userId, cookies); } diff --git a/src/routes/api/admin/auth/login/+server.ts b/src/routes/api/admin/auth/login/+server.ts index d0686208..f30fc1ea 100644 --- a/src/routes/api/admin/auth/login/+server.ts +++ b/src/routes/api/admin/auth/login/+server.ts @@ -11,5 +11,5 @@ export async function POST({ request, cookies }) { const uid = await validateCredentials(username, password); - return json({ token: generateToken(uid, cookies) }, { status: 200 }); + return json({ token: await generateToken(uid, cookies) }, { status: 200 }); } diff --git a/src/routes/api/admin/firstrun/+server.ts b/src/routes/api/admin/firstrun/+server.ts index 2c2e2d76..7bfdef0a 100644 --- a/src/routes/api/admin/firstrun/+server.ts +++ b/src/routes/api/admin/firstrun/+server.ts @@ -36,7 +36,7 @@ export async function POST({ request, cookies }) { const firstTime = await setupData(options.repoUrl, options.branch); // Now set up auth - const credentials = await authSetup(options.username, options.password, cookies); + const token = await authSetup(options.username, options.password, cookies); - return json({ credentials, firstTime }, { status: 200 }); + return json({ token, firstTime }, { status: 200 }); } diff --git a/tests/backend/admin/auth/login.test.ts b/tests/backend/admin/auth/login.test.ts index 9f05c398..0f82d2c1 100644 --- a/tests/backend/admin/auth/login.test.ts +++ b/tests/backend/admin/auth/login.test.ts @@ -6,12 +6,11 @@ import { it, expect, beforeEach } from 'vitest'; import { setup } from '../../helpers'; import api from '$endpoints'; -import type { FirstRunCredentials } from '$lib/server/auth/tokens'; -let credentials: FirstRunCredentials; +let credentials: Awaited>; beforeEach(async () => { - credentials = (await setup()).credentials; + credentials = (await setup()); }); it("Gives an error when the server isn't setup", async () => { diff --git a/tests/backend/helpers.ts b/tests/backend/helpers.ts index 387dc104..2e8556cc 100644 --- a/tests/backend/helpers.ts +++ b/tests/backend/helpers.ts @@ -8,12 +8,16 @@ import { getDataDir } from '$lib/server/data/dataDir'; /** Set up the server, returning (amongst other things) an API client */ export async function setup(repoUrl?: string, branch?: string) { - const credentials - = (await api(undefined).admin.firstrun('admin', 'abc123ABC!', repoUrl, branch)) - .credentials; + const username = 'admin'; + const password = 'abc123ABC!'; + const token + = (await api(undefined).admin.firstrun(username, password, repoUrl, branch)) + .token; return { - api: api(credentials.token), - credentials, + api: api(token), + token, + username, + password, }; } From bdc9b8d8c136397d7e27550b59e8e6a834e8b183 Mon Sep 17 00:00:00 2001 From: Maddy Guthridge Date: Wed, 2 Oct 2024 01:33:53 +1000 Subject: [PATCH 11/15] Fix all tests --- src/endpoints/admin/auth.ts | 6 ++--- src/endpoints/fetch.ts | 2 +- src/lib/server/auth/passwords.ts | 9 ++++---- src/lib/server/auth/tokens.ts | 21 ++++++++++------- src/routes/api/admin/auth/change/+server.ts | 24 ++++++++------------ src/routes/api/admin/auth/disable/+server.ts | 16 ++++++------- src/routes/api/admin/auth/logout/+server.ts | 11 +++++++-- src/routes/api/admin/auth/revoke/+server.ts | 4 ++-- tests/backend/admin/auth/change.test.ts | 10 ++++---- tests/backend/admin/auth/disable.test.ts | 16 ++++++------- tests/backend/admin/auth/login.test.ts | 4 ++-- tests/backend/admin/auth/revoke.test.ts | 2 +- tests/backend/admin/firstrun.test.ts | 2 +- tests/backend/admin/git/get.test.ts | 2 +- tests/backend/debug/clear.test.ts | 2 +- tests/backend/group/all.test.ts | 2 +- 16 files changed, 71 insertions(+), 62 deletions(-) diff --git a/src/endpoints/admin/auth.ts b/src/endpoints/admin/auth.ts index 5d3b9e1d..b099816b 100644 --- a/src/endpoints/admin/auth.ts +++ b/src/endpoints/admin/auth.ts @@ -27,7 +27,7 @@ export default function auth(token: string | undefined) { 'POST', '/api/admin/auth/logout', token, - )) as Promise<{ token: string }>; + )); }; /** @@ -66,12 +66,12 @@ export default function auth(token: string | undefined) { * @param token The auth token * @param password The password to the admin account */ - const disable = async (password: string) => { + const disable = async (username: string, password: string) => { return json(apiFetch( 'POST', '/api/admin/auth/disable', token, - { password } + { username, password } )) as Promise>; }; diff --git a/src/endpoints/fetch.ts b/src/endpoints/fetch.ts index a95a4397..fce1092e 100644 --- a/src/endpoints/fetch.ts +++ b/src/endpoints/fetch.ts @@ -131,7 +131,7 @@ export async function json(response: Promise): Promise { } if ([400, 401, 403].includes(res.status)) { - // All 400 and 403 errors have an error message + // All 400, 401 and 403 errors have an error message const message = (json as { message: string }).message; throw new ApiError(res.status, message); } diff --git a/src/lib/server/auth/passwords.ts b/src/lib/server/auth/passwords.ts index 3bece05c..6defd0bd 100644 --- a/src/lib/server/auth/passwords.ts +++ b/src/lib/server/auth/passwords.ts @@ -19,9 +19,9 @@ const sleepRandom = () => new Promise((r) => setTimeout(r, Math.random() * * Throw a 401 after a random (small) amount of time, so that timing attacks * cannot be used reliably. */ -async function fail(timer: Promise) { +async function fail(timer: Promise, code: number) { await timer; - return error(401, 'The username or password is incorrect'); + return error(code, 'The username or password is incorrect'); } /** Hash a password with the given salt, returning the result */ @@ -43,6 +43,7 @@ export function hashAndSalt(salt: string, password: string): string { export async function validateCredentials( username: string, password: string, + code: number = 403, ): Promise { const local = await getLocalConfig(); @@ -52,13 +53,13 @@ export async function validateCredentials( const userId = Object.keys(local.auth).find(id => local.auth[id].username === username); if (!userId) { - return fail(failTimer); + return fail(failTimer, code); } const hashResult = hashAndSalt(local.auth[userId].password.salt, password); if (hashResult !== local.auth[userId].password.hash) { - return fail(failTimer); + return fail(failTimer, code); } return userId; } diff --git a/src/lib/server/auth/tokens.ts b/src/lib/server/auth/tokens.ts index 3ba655f1..f6ea3bd5 100644 --- a/src/lib/server/auth/tokens.ts +++ b/src/lib/server/auth/tokens.ts @@ -100,6 +100,13 @@ export async function validateToken(token: string): Promise { const config = await getLocalConfig(); @@ -115,7 +122,9 @@ export async function revokeSession(token: string): Promise { } /** - * Validates and returns the token from the given request. + * Validates the token from the given request. + * + * Returns the user ID of the token's owner. * * First attempts to get the token from the "Authorization" header, but if that * isn't present, the "token" property is checked within the cookies. @@ -123,20 +132,16 @@ export async function revokeSession(token: string): Promise { * If the token is invalid, it is removed from the cookies. */ export async function validateTokenFromRequest(req: { request: Request, cookies: Cookies }): Promise { - const tokenFromHeader = req.request.headers.get('Authorization'); - const tokenFromCookie = req.cookies.get('token'); - - const token = tokenFromHeader || tokenFromCookie; - + const token = getTokenFromRequest(req); if (!token) { throw error(401, 'A token is required to access this endpoint'); } - await validateToken(token).catch(e => { + const data = await validateToken(token).catch(e => { // Remove token from cookies, as it is invalid req.cookies.delete('token', { path: '/' }); error(401, `${e}`); }); - return token; + return data.uid; } /** Return whether the request is authorized (has a valid token) */ diff --git a/src/routes/api/admin/auth/change/+server.ts b/src/routes/api/admin/auth/change/+server.ts index 18b9106e..738a06fd 100644 --- a/src/routes/api/admin/auth/change/+server.ts +++ b/src/routes/api/admin/auth/change/+server.ts @@ -1,9 +1,11 @@ -import { hashAndSalt, validateTokenFromRequest } from '$lib/server/auth/tokens'; +import { hashAndSalt } from '$lib/server/auth/passwords.js'; +import { validateTokenFromRequest } from '$lib/server/auth/tokens'; import { getPortfolioGlobals } from '$lib/server/data/index'; import { getLocalConfig, setLocalConfig } from '$lib/server/data/localConfig'; +import { applyStruct } from '$lib/server/util.js'; import { error, json } from '@sveltejs/kit'; import { nanoid } from 'nanoid'; -import { object, string, validate } from 'superstruct'; +import { object, string } from 'superstruct'; import validator from 'validator'; const NewCredentials = object({ @@ -14,7 +16,7 @@ const NewCredentials = object({ export async function POST({ request, cookies }) { await getPortfolioGlobals().catch(e => error(400, e)); - await validateTokenFromRequest({ request, cookies }); + const uid = await validateTokenFromRequest({ request, cookies }); const local = await getLocalConfig(); @@ -22,20 +24,14 @@ export async function POST({ request, cookies }) { throw Error('Unreachable'); } - const [err, newCredentials] - = validate(await request.json(), NewCredentials); - - if (err) { - return error(400, `${err}`); - } - - const { newUsername, oldPassword, newPassword } = newCredentials; + const { newUsername, oldPassword, newPassword } + = await applyStruct(await request.json(), NewCredentials); if (!newUsername) { return error(400, 'New username is empty'); } - if (hashAndSalt(local.auth.password.salt, oldPassword) !== local.auth.password.hash) { + if (hashAndSalt(local.auth[uid].password.salt, oldPassword) !== local.auth[uid].password.hash) { return error(403, 'Old password is incorrect'); } @@ -46,12 +42,12 @@ export async function POST({ request, cookies }) { // Hash and salt new password const salt = nanoid(); const hash = hashAndSalt(salt, newPassword); - local.auth.password = { + local.auth[uid].password = { hash, salt, }; // Change the username - local.auth.username = newUsername; + local.auth[uid].username = newUsername; await setLocalConfig(local); return json({}, { status: 200 }); diff --git a/src/routes/api/admin/auth/disable/+server.ts b/src/routes/api/admin/auth/disable/+server.ts index 5e639207..ba2206bf 100644 --- a/src/routes/api/admin/auth/disable/+server.ts +++ b/src/routes/api/admin/auth/disable/+server.ts @@ -1,11 +1,13 @@ -import { hashAndSalt, validateTokenFromRequest } from '$lib/server/auth/tokens'; +import { validateCredentials } from '$lib/server/auth/passwords'; +import { validateTokenFromRequest } from '$lib/server/auth/tokens'; import { getPortfolioGlobals } from '$lib/server/data/index'; import { getLocalConfig, setLocalConfig } from '$lib/server/data/localConfig'; import { error, json } from '@sveltejs/kit'; +// TODO: Remove this when setting up user management export async function POST({ request, cookies }) { await getPortfolioGlobals().catch(e => error(400, e)); - await validateTokenFromRequest({ request, cookies }); + const uid = await validateTokenFromRequest({ request, cookies }); const local = await getLocalConfig(); @@ -13,14 +15,12 @@ export async function POST({ request, cookies }) { throw Error('Unreachable'); } - const { password } = await request.json(); + const { username, password } = await request.json(); - if (hashAndSalt(local.auth.password.salt, password) !== local.auth.password.hash) { - return error(403, 'Incorrect password'); - } + await validateCredentials(username, password, 403); - // Disable authentication - local.auth = null; + // Delete this user + delete local.auth[uid]; await setLocalConfig(local); return json({}, { status: 200 }); diff --git a/src/routes/api/admin/auth/logout/+server.ts b/src/routes/api/admin/auth/logout/+server.ts index e00a5681..91981f70 100644 --- a/src/routes/api/admin/auth/logout/+server.ts +++ b/src/routes/api/admin/auth/logout/+server.ts @@ -1,11 +1,18 @@ -import { revokeSession, validateTokenFromRequest } from '$lib/server/auth/tokens'; +import { getTokenFromRequest, revokeSession } from '$lib/server/auth/tokens'; import { getPortfolioGlobals } from '$lib/server/data/index'; import { error, json } from '@sveltejs/kit'; export async function POST(req) { await getPortfolioGlobals().catch(e => error(400, e)); - await revokeSession(await validateTokenFromRequest(req)); + const token = getTokenFromRequest(req); + + if (!token) { + error(401, 'A token is required to access this endpoint'); + } + + await revokeSession(token) + .catch(e => error(401, `${e}`)); return json({}, { status: 200 }); } diff --git a/src/routes/api/admin/auth/revoke/+server.ts b/src/routes/api/admin/auth/revoke/+server.ts index dc773811..a538a0f1 100644 --- a/src/routes/api/admin/auth/revoke/+server.ts +++ b/src/routes/api/admin/auth/revoke/+server.ts @@ -13,9 +13,9 @@ export async function POST({ request, cookies }) { return error(403, 'Authentication was disabled'); } - await validateTokenFromRequest({ request, cookies }); + const uid = await validateTokenFromRequest({ request, cookies }); - local.auth.sessions.notBefore = unixTime(); + local.auth[uid].sessions.notBefore = unixTime(); await setLocalConfig(local); diff --git a/tests/backend/admin/auth/change.test.ts b/tests/backend/admin/auth/change.test.ts index 4bcf3b70..d80a8dc1 100644 --- a/tests/backend/admin/auth/change.test.ts +++ b/tests/backend/admin/auth/change.test.ts @@ -7,13 +7,13 @@ import { it, expect } from 'vitest'; import { setup } from '../../helpers'; it('Blocks unauthorized users', async () => { - const { api, credentials: { password } } = await setup(); + const { api, password } = await setup(); await expect(api.withToken(undefined).admin.auth.change('foo', password, 'abc123ABC$')) .rejects.toMatchObject({ code: 401 }); }); it('Allows users to reset their password', async () => { - const { api, credentials: { password } } = await setup(); + const { api, password } = await setup(); const newPassword = 'abc123ABC$'; await expect(api.admin.auth.change('foo', password, newPassword)) .resolves.toStrictEqual({}); @@ -30,19 +30,19 @@ it('Rejects incorrect previous passwords', async () => { }); it('Rejects insecure new passwords', async () => { - const { api, credentials: { password } } = await setup(); + const { api, password } = await setup(); await expect(api.admin.auth.change('foo', password, 'insecure')) .rejects.toMatchObject({ code: 400 }); }); it('Rejects empty new usernames', async () => { - const { api, credentials: { password } } = await setup(); + const { api, password } = await setup(); await expect(api.admin.auth.change('', password, 'abc123ABC$')) .rejects.toMatchObject({ code: 400 }); }); it('Errors if the data is not set up', async () => { - const { api, credentials: { password } } = await setup(); + const { api, password } = await setup(); await api.debug.clear(); await expect(api.admin.auth.change('foo', password, 'abc123ABC$')) .rejects.toMatchObject({ code: 400 }); diff --git a/tests/backend/admin/auth/disable.test.ts b/tests/backend/admin/auth/disable.test.ts index 239ef25e..bcc9d054 100644 --- a/tests/backend/admin/auth/disable.test.ts +++ b/tests/backend/admin/auth/disable.test.ts @@ -5,8 +5,8 @@ import { it, expect } from 'vitest'; import { setup } from '../../helpers'; it('Disables authentication', async () => { - const { api, credentials: { username, password } } = await setup(); - await expect(api.admin.auth.disable(password)).resolves.toStrictEqual({}); + const { api, username, password } = await setup(); + await expect(api.admin.auth.disable(username, password)).resolves.toStrictEqual({}); // Logging in should fail await expect(api.admin.auth.login(username, password)) .rejects.toMatchObject({ code: 403 }); @@ -17,20 +17,20 @@ it('Disables authentication', async () => { }); it('Errors for invalid tokens', async () => { - const { api, credentials: { password } } = await setup(); - await expect(api.withToken('invalid').admin.auth.disable(password)) + const { api, username, password } = await setup(); + await expect(api.withToken('invalid').admin.auth.disable(username, password)) .rejects.toMatchObject({ code: 401 }); }); it('Errors for incorrect passwords', async () => { - const { api } = await setup(); - await expect(api.admin.auth.disable('incorrect')) + const { api, username } = await setup(); + await expect(api.admin.auth.disable(username, 'incorrect')) .rejects.toMatchObject({ code: 403 }); }); it('Errors if the data is not set up', async () => { - const { api, credentials: { password } } = await setup(); + const { api, username, password } = await setup(); await api.debug.clear(); - await expect(api.admin.auth.disable(password)) + await expect(api.admin.auth.disable(username, password)) .rejects.toMatchObject({ code: 400 }); }); diff --git a/tests/backend/admin/auth/login.test.ts b/tests/backend/admin/auth/login.test.ts index 0f82d2c1..276e2180 100644 --- a/tests/backend/admin/auth/login.test.ts +++ b/tests/backend/admin/auth/login.test.ts @@ -26,12 +26,12 @@ it('Returns a token when correct credentials are provided', async () => { it('Blocks logins with non-existent usernames', async () => { await expect(api().admin.auth.login(credentials.username + 'hi', credentials.password)) - .rejects.toMatchObject({ code: 401 }); + .rejects.toMatchObject({ code: 403 }); }); it('Blocks logins with incorrect passwords', async () => { await expect(api().admin.auth.login(credentials.username, credentials.password + 'hi')) - .rejects.toMatchObject({ code: 401 }); + .rejects.toMatchObject({ code: 403 }); }); /** diff --git a/tests/backend/admin/auth/revoke.test.ts b/tests/backend/admin/auth/revoke.test.ts index 34059803..5711e80b 100644 --- a/tests/backend/admin/auth/revoke.test.ts +++ b/tests/backend/admin/auth/revoke.test.ts @@ -8,7 +8,7 @@ import api from '$endpoints'; const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); it('Revokes the current token', async () => { - const { api, credentials: { username, password } } = await setup(); + const { api, username, password } = await setup(); const { token: token2 } = await api.admin.auth.login(username, password); // Wait a second, since otherwise the token will have been created at the new // notBefore time diff --git a/tests/backend/admin/firstrun.test.ts b/tests/backend/admin/firstrun.test.ts index cdb397ce..24057249 100644 --- a/tests/backend/admin/firstrun.test.ts +++ b/tests/backend/admin/firstrun.test.ts @@ -91,7 +91,7 @@ describe('git', () => { it('Gives a token on success', async () => { await expect(firstrun()) .resolves.toMatchObject({ - credentials: { token: expect.any(String) } + token: expect.any(String) }); }); diff --git a/tests/backend/admin/git/get.test.ts b/tests/backend/admin/git/get.test.ts index 861847fc..d02e1e8c 100644 --- a/tests/backend/admin/git/get.test.ts +++ b/tests/backend/admin/git/get.test.ts @@ -20,7 +20,7 @@ it('Gives a 400 when no data directory is set up', async () => { }); it('Correctly returns repo info when a repo is set up', { timeout: 15_000 }, async () => { - const { token } = (await api().admin.firstrun(gitRepos.TEST_REPO_RW, null)).credentials; + const { token } = await api().admin.firstrun('admin', 'abc123ABC!', gitRepos.TEST_REPO_RW); const repo = simpleGit(getDataDir()); await expect(api(token).admin.git.status()).resolves.toStrictEqual({ repo: { diff --git a/tests/backend/debug/clear.test.ts b/tests/backend/debug/clear.test.ts index 776a6422..40bf4215 100644 --- a/tests/backend/debug/clear.test.ts +++ b/tests/backend/debug/clear.test.ts @@ -14,7 +14,7 @@ it('Removes all content from the data directory', async () => { }); it('Removes the login credentials', async () => { - const credentials = (await setup()).credentials; + const credentials = (await setup()); await api().debug.clear(); // Logging in should fail await expect(api().admin.auth.login(credentials.username, credentials.password)).toReject(); diff --git a/tests/backend/group/all.test.ts b/tests/backend/group/all.test.ts index c159fb42..85161b9a 100644 --- a/tests/backend/group/all.test.ts +++ b/tests/backend/group/all.test.ts @@ -21,7 +21,7 @@ it('Lists existing groups', async () => { }); it('Ignores the .git directory', { timeout: 15_000 }, async () => { - await api().admin.firstrun(gitRepos.EMPTY, null); + await api().admin.firstrun('admin', 'abc123ABC!', gitRepos.EMPTY); await expect(api().group.all()).resolves.toStrictEqual({}); }); From a2bcbf1e171d0e0cde7191888a2e6a1a71bdc7ce Mon Sep 17 00:00:00 2001 From: Maddy Guthridge Date: Wed, 2 Oct 2024 01:36:07 +1000 Subject: [PATCH 12/15] Add test cases for public key endpoint --- tests/backend/admin/publickey/get.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/backend/admin/publickey/get.test.ts b/tests/backend/admin/publickey/get.test.ts index e69de29b..f2eda1ef 100644 --- a/tests/backend/admin/publickey/get.test.ts +++ b/tests/backend/admin/publickey/get.test.ts @@ -0,0 +1,12 @@ +/** + * Test cases for public key requests + */ + +import { describe, test } from "vitest"; + + +test.todo('Public key can be requested without token when server is not set up'); + +test.todo('Public key can be requested with token when server is set up'); + +describe.todo('Token tests'); From 09f86a18dcba3212e6fcd59bc3aaf1cef5c45610 Mon Sep 17 00:00:00 2001 From: Maddy Guthridge Date: Wed, 2 Oct 2024 16:34:04 +1000 Subject: [PATCH 13/15] Update .env example --- .env.example | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.env.example b/.env.example index 1234d1bb..0b9fa2f5 100644 --- a/.env.example +++ b/.env.example @@ -1,4 +1,4 @@ -HOST=localhost # the hostname to use -PORT=5096 # the port number to use -DATA_REPO_PATH="./data" # the path to the data repository -AUTH_SECRET="CHANGE ME" # the secret key to validate tokens +HOST=localhost # the hostname to use +PORT=5096 # the port number to use +DATA_REPO_PATH="./data" # the path to the data volume +PRIVATE_DATA_PATH="./private_data" # the path to the private data volume From 0f5ecb0f791ab0bc3e4e0fcb884589f8b9d0f5bb Mon Sep 17 00:00:00 2001 From: Maddy Guthridge Date: Wed, 2 Oct 2024 16:59:39 +1000 Subject: [PATCH 14/15] Update firstrun page --- src/lib/validators.ts | 2 +- src/routes/admin/firstrun/+page.svelte | 94 ++++++++++++++++---------- 2 files changed, 58 insertions(+), 38 deletions(-) diff --git a/src/lib/validators.ts b/src/lib/validators.ts index 1c1d4e8e..64fcce06 100644 --- a/src/lib/validators.ts +++ b/src/lib/validators.ts @@ -1,7 +1,7 @@ import { error } from '@sveltejs/kit'; /** Regex for matching ID strings */ -const idValidatorRegex = /^[a-z0-9-.]+$/; +export const idValidatorRegex = /^[a-z0-9-.]+$/; /** Ensure that the given ID string is valid */ export function validateId(type: string, id: string): string { diff --git a/src/routes/admin/firstrun/+page.svelte b/src/routes/admin/firstrun/+page.svelte index e309f43f..aac4ae62 100644 --- a/src/routes/admin/firstrun/+page.svelte +++ b/src/routes/admin/firstrun/+page.svelte @@ -1,28 +1,33 @@