From 445f54e0128ad7a7c13088d3deb3e5c3803d247b Mon Sep 17 00:00:00 2001 From: baldo Date: Thu, 28 Jul 2022 11:25:53 +0200 Subject: [PATCH 01/13] Update server dependencies. --- package.json | 4 ++-- yarn.lock | 26 +++++++++++++------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index 43ceeb4..4f6b64d 100644 --- a/package.json +++ b/package.json @@ -68,7 +68,7 @@ "@types/html-to-text": "^8.0.1", "@types/jest": "^28.1.6", "@types/lodash": "^4.14.178", - "@types/node": "^18.0.6", + "@types/node": "^18.6.2", "@types/node-cron": "^3.0.2", "@types/nodemailer": "^6.4.4", "@types/request": "^2.48.8", @@ -101,7 +101,7 @@ "time-grunt": "^2.0.0", "ts-jest": "^28.0.7", "typescript": "^4.7.4", - "yarn-audit-fix": "^9.3.2" + "yarn-audit-fix": "^9.3.3" }, "resolutions": { "grunt-connect-proxy/**/http-proxy": "~1.18.1", diff --git a/yarn.lock b/yarn.lock index a9f2f56..0a3b0cc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -935,10 +935,10 @@ resolved "https://registry.yarnpkg.com/@types/node/-/node-17.0.34.tgz#3b0b6a50ff797280b8d000c6281d229f9c538cef" integrity sha512-XImEz7XwTvDBtzlTnm8YvMqGW/ErMWBsKZ+hMTvnDIjGCKxwK5Xpc+c/oQjOauwq8M4OS11hEkpjX8rrI/eEgA== -"@types/node@^18.0.6": - version "18.0.6" - resolved "https://registry.yarnpkg.com/@types/node/-/node-18.0.6.tgz#0ba49ac517ad69abe7a1508bc9b3a5483df9d5d7" - integrity sha512-/xUq6H2aQm261exT6iZTMifUySEt4GR5KX8eYyY+C4MSNPqSh9oNIP7tz2GLKTlFaiBbgZNxffoR3CVRG+cljw== +"@types/node@^18.6.2": + version "18.6.2" + resolved "https://registry.yarnpkg.com/@types/node/-/node-18.6.2.tgz#ffc5f0f099d27887c8d9067b54e55090fcd54126" + integrity sha512-KcfkBq9H4PI6Vpu5B/KoPeuVDAbmi+2mDBqGPGUgoL7yXQtcWGu2vJWmmRkneWK3Rh0nIAX192Aa87AqKHYChQ== "@types/nodemailer@^6.4.4": version "6.4.4" @@ -1962,10 +1962,10 @@ commander@^7.2.0: resolved "https://registry.yarnpkg.com/commander/-/commander-7.2.0.tgz#a36cb57d0b501ce108e4d20559a150a391d97ab7" integrity sha512-QrWXB+ZQSVPmIWIhtEO9H+gwHaMGYiF5ChvoJ+K9ZGHG/sVsa6yiesAD1GC/x46sET00Xlwo1u49RVVVzvcSkw== -commander@^9.3.0: - version "9.3.0" - resolved "https://registry.yarnpkg.com/commander/-/commander-9.3.0.tgz#f619114a5a2d2054e0d9ff1b31d5ccf89255e26b" - integrity sha512-hv95iU5uXPbK83mjrJKuZyFM/LBAoCV/XhVGkS5Je6tl7sxr6A0ITMw5WoRV46/UaJ46Nllm3Xt7IaJhXTIkzw== +commander@^9.4.0: + version "9.4.0" + resolved "https://registry.yarnpkg.com/commander/-/commander-9.4.0.tgz#bc4a40918fefe52e22450c111ecd6b7acce6f11c" + integrity sha512-sRPT+umqkz90UA8M1yqYfnHlZA7fF6nSphDtxeywPZ49ysjxDQybzk13CL+mXekDRG92skbcqCLVovuCusNmFw== commondir@^1.0.1: version "1.0.1" @@ -7287,10 +7287,10 @@ yargs@^17.3.1: y18n "^5.0.5" yargs-parser "^21.0.0" -yarn-audit-fix@^9.3.2: - version "9.3.2" - resolved "https://registry.yarnpkg.com/yarn-audit-fix/-/yarn-audit-fix-9.3.2.tgz#9268aeaf70faafd6d8b8a71d0b8c8d97d6b809ec" - integrity sha512-hRPu2FRTLF5kL+fgq6NZDVgvGV7zEO6ghgfXoFmseDtDzqBIfKbGVNL+XqJ1fIil70x6XyrQwyARyyrMZtxpaw== +yarn-audit-fix@^9.3.3: + version "9.3.3" + resolved "https://registry.yarnpkg.com/yarn-audit-fix/-/yarn-audit-fix-9.3.3.tgz#05e1fab4fb6dd137db6c31006d569dc7144d41c3" + integrity sha512-EFKcjEi3GSQ3QL7dV835ovfcw8of6RbiRdyeA2n67r7dcJYctxOqqmFHQPBoEvQSDXuYH/Zwk/8J6QUT4R8E5A== dependencies: "@types/find-cache-dir" "^3.2.1" "@types/fs-extra" "^9.0.13" @@ -7299,7 +7299,7 @@ yarn-audit-fix@^9.3.2: "@types/yarnpkg__lockfile" "^1.1.5" "@yarnpkg/lockfile" "^1.1.0" chalk "^5.0.1" - commander "^9.3.0" + commander "^9.4.0" find-cache-dir "^3.3.2" find-up "^6.3.0" fs-extra "^10.1.0" From 077cb2ee21196909e654811260cb7119d10fba3d Mon Sep 17 00:00:00 2001 From: baldo Date: Thu, 28 Jul 2022 11:28:15 +0200 Subject: [PATCH 02/13] Removed already done TODO. --- REFACTOR.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/REFACTOR.md b/REFACTOR.md index ca5baae..1f6a845 100644 --- a/REFACTOR.md +++ b/REFACTOR.md @@ -2,9 +2,6 @@ ## TODO -* Different user accounts for admin panel: - * Username + password hash in config - * Commandline tool to generate hash * Test email rendering! ## Short term From f296c6fe266be2593cc86f205d0dbdc065f86142 Mon Sep 17 00:00:00 2001 From: baldo Date: Thu, 28 Jul 2022 12:19:40 +0200 Subject: [PATCH 03/13] Got rid of some warnings. --- server/logger.ts | 16 +++++++++------- server/validation/constraints.ts | 1 + 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/server/logger.ts b/server/logger.ts index d3740ed..edaadc7 100644 --- a/server/logger.ts +++ b/server/logger.ts @@ -5,24 +5,26 @@ import _ from 'lodash'; export type LoggingFunction = (...args: any[]) => void; const noopTaggedLogger: TaggedLogger = { - log(level: LogLevel, ...args: any[]): void {}, - debug(...args: any[]): void {}, - info(...args: any[]): void {}, - warn(...args: any[]): void {}, - error(...args: any[]): void {}, - profile(...args: any[]): void {}, + log(_level: LogLevel, ..._args: any[]): void {}, + debug(..._args: any[]): void {}, + info(..._args: any[]): void {}, + warn(..._args: any[]): void {}, + error(..._args: any[]): void {}, + profile(..._args: any[]): void {}, }; export interface ActivatableLogger extends Logger { init(enabled: boolean, loggingFunction?: LoggingFunction): void; } +/** + * TODO: Check if LoggingConfig.debug and LoggingConfig.profile are handled. + */ export class ActivatableLoggerImpl implements ActivatableLogger { private enabled: boolean = false; private loggingFunction: LoggingFunction = console.info; init(enabled: boolean, loggingFunction?: LoggingFunction): void { - const config = require('./config').config; this.enabled = enabled; this.loggingFunction = loggingFunction || console.info; } diff --git a/server/validation/constraints.ts b/server/validation/constraints.ts index 587faf3..e90cfe7 100644 --- a/server/validation/constraints.ts +++ b/server/validation/constraints.ts @@ -1,6 +1,7 @@ // ATTENTION: Those constraints are no longer the same file as for the client / admin interface. // Make sure changes are also reflected in /shared/validation/constraints.js. +// noinspection RegExpSimplifiable const CONSTRAINTS = { id:{ type: 'string', From 72543b95ee579e86e5b304e2a94a3b93886a7e9d Mon Sep 17 00:00:00 2001 From: baldo Date: Thu, 28 Jul 2022 12:22:57 +0200 Subject: [PATCH 04/13] Make typeguards for newtypes match the type instead of isString or isNumber. --- server/types/config.ts | 9 ++++++--- server/types/index.ts | 5 ++++- server/types/shared.ts | 41 ++++++++++++++++++++++++----------------- 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/server/types/config.ts b/server/types/config.ts index d25ef3d..46585a3 100644 --- a/server/types/config.ts +++ b/server/types/config.ts @@ -1,11 +1,14 @@ import {ArrayField, Field, RawJsonField} from "sparkson" -import {ClientConfig, JSONObject, Url} from "./shared"; - -// TODO: Replace string types by more specific types like URL, Password, etc. +import {ClientConfig, isSite, isString, JSONObject, toIsNewtype, Url} from "./shared"; export type Username = string & { readonly __tag: unique symbol }; +export const isUsername = toIsNewtype(isString, "" as Username); + export type CleartextPassword = string & { readonly __tag: unique symbol }; +export const isCleartextPassword = toIsNewtype(isString, "" as CleartextPassword); + export type PasswordHash = string & { readonly __tag: unique symbol }; +export const isPasswordHash = toIsNewtype(isString, "" as PasswordHash); export class UsersConfig { constructor( diff --git a/server/types/index.ts b/server/types/index.ts index e8e9647..f409f04 100644 --- a/server/types/index.ts +++ b/server/types/index.ts @@ -3,6 +3,7 @@ import { Domain, DomainSpecificNodeResponse, EmailAddress, + isNumber, JSONObject, MonitoringResponse, MonitoringState, @@ -13,6 +14,7 @@ import { Site, StoredNode, toIsEnum, + toIsNewtype, } from "./shared"; export * from "./config"; @@ -90,12 +92,13 @@ export function toMonitoringResponse(node: StoredNode): MonitoringResponse { }; } -// TODO: Complete interface / class declaration. export type NodeSecrets = { monitoringToken?: MonitoringToken, }; export type MailId = number & { readonly __tag: unique symbol }; +export const isMailId = toIsNewtype(isNumber, NaN as MailId); + export type MailData = JSONObject; export enum MailType { diff --git a/server/types/shared.ts b/server/types/shared.ts index 6e39b42..62c1a58 100644 --- a/server/types/shared.ts +++ b/server/types/shared.ts @@ -85,6 +85,13 @@ export function isString(arg: unknown): arg is string { return typeof arg === "string" } +export function toIsNewtype< + Type extends Value & { readonly __tag: symbol }, + Value, +>(isValue: TypeGuard, _example: Type): TypeGuard { + return (arg: unknown): arg is Type => isValue(arg); +} + export function isNumber(arg: unknown): arg is number { return typeof arg === "number" } @@ -114,13 +121,13 @@ export function isOptional(arg: unknown, isT: TypeGuard): arg is (T | unde } export type Url = string & { readonly __tag: unique symbol }; -export const isUrl = isString; +export const isUrl = toIsNewtype(isString, "" as Url); export type Version = string & { readonly __tag: unique symbol }; -export const isVersion = isString; +export const isVersion = toIsNewtype(isString, "" as Version); export type EmailAddress = string & { readonly __tag: unique symbol }; -export const isEmailAddress = isString; +export const isEmailAddress = toIsNewtype(isString, "" as EmailAddress); export type NodeStatistics = { registered: number; @@ -321,31 +328,30 @@ export function isClientConfig(arg: unknown): arg is ClientConfig { ); } -// TODO: Token type. export type Token = string & { readonly __tag: unique symbol }; -export const isToken = isString; +export const isToken = toIsNewtype(isString, "" as Token); export type FastdKey = string & { readonly __tag: unique symbol }; -export const isFastdKey = isString; +export const isFastdKey = toIsNewtype(isString, "" as FastdKey); export type MAC = string & { readonly __tag: unique symbol }; -export const isMAC = isString; +export const isMAC = toIsNewtype(isString, "" as MAC); export type DurationSeconds = number & { readonly __tag: unique symbol }; -export const isDurationSeconds = isNumber; +export const isDurationSeconds = toIsNewtype(isNumber, NaN as DurationSeconds); export type UnixTimestampSeconds = number & { readonly __tag: unique symbol }; -export const isUnixTimestampSeconds = isNumber; +export const isUnixTimestampSeconds = toIsNewtype(isNumber, NaN as UnixTimestampSeconds); export type UnixTimestampMilliseconds = number & { readonly __tag: unique symbol }; -export const isUnixTimestampMilliseconds = isNumber; +export const isUnixTimestampMilliseconds = toIsNewtype(isNumber, NaN as UnixTimestampMilliseconds); export function toUnixTimestampSeconds(ms: UnixTimestampMilliseconds): UnixTimestampSeconds { return Math.floor(ms) as UnixTimestampSeconds; } export type MonitoringToken = string & { readonly __tag: unique symbol }; -export const isMonitoringToken = isString; +export const isMonitoringToken = toIsNewtype(isString, "" as MonitoringToken); export enum MonitoringState { ACTIVE = "active", @@ -356,15 +362,16 @@ export enum MonitoringState { export const isMonitoringState = toIsEnum(MonitoringState); export type NodeId = string & { readonly __tag: unique symbol }; +export const isNodeId = toIsNewtype(isString, "" as NodeId); -export type Hostname = string & { readonly __tag: unique symbol }; -export const isHostname = isString; +export type Hostname = string & { readonly __tag: unique symbol } +export const isHostname = toIsNewtype(isString, "" as Hostname); export type Nickname = string & { readonly __tag: unique symbol }; -export const isNickname = isString; +export const isNickname = toIsNewtype(isString, "" as Nickname); export type Coordinates = string & { readonly __tag: unique symbol }; -export const isCoordinates = isString; +export const isCoordinates = toIsNewtype(isString, "" as Coordinates); /** * Basic node data. @@ -473,10 +480,10 @@ export enum OnlineState { export const isOnlineState = toIsEnum(OnlineState); export type Site = string & { readonly __tag: unique symbol }; -export const isSite = isString; +export const isSite = toIsNewtype(isString, "" as Site); export type Domain = string & { readonly __tag: unique symbol }; -export const isDomain = isString; +export const isDomain = toIsNewtype(isString, "" as Domain); /** * Represents a node in the context of a Freifunk site and domain. From 2d83326b7890edda064c207124a5b46a4289c873 Mon Sep 17 00:00:00 2001 From: baldo Date: Thu, 28 Jul 2022 12:24:07 +0200 Subject: [PATCH 05/13] Make site and domain optional and get rid of "" values. --- server/services/monitoringService.ts | 22 ++++++++++++---------- server/types/index.ts | 4 ++-- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/server/services/monitoringService.ts b/server/services/monitoringService.ts index e162fe6..65d0a37 100644 --- a/server/services/monitoringService.ts +++ b/server/services/monitoringService.ts @@ -19,8 +19,10 @@ import { Domain, DurationSeconds, Hostname, + isDomain, isMonitoringSortField, isOnlineState, + isSite, MAC, MailType, MonitoringSortField, @@ -71,8 +73,8 @@ export type ParsedNode = { importTimestamp: UnixTimestampSeconds, state: OnlineState, lastSeen: UnixTimestampSeconds, - site: Site, - domain: Domain, + site?: Site, + domain?: Domain, }; export type NodesParsingResult = { @@ -224,14 +226,14 @@ export function parseNode(importTimestamp: UnixTimestampSeconds, nodeData: any): ); } - let site = "" as Site; // FIXME: Handle this - if (_.isPlainObject(nodeData.nodeinfo.system) && _.isString(nodeData.nodeinfo.system.site_code)) { - site = nodeData.nodeinfo.system.site_code as Site; + let site: Site | undefined; + if (_.isPlainObject(nodeData.nodeinfo.system) && isSite(nodeData.nodeinfo.system.site_code)) { + site = nodeData.nodeinfo.system.site_code; } - let domain = "" as Domain; // FIXME: Handle this - if (_.isPlainObject(nodeData.nodeinfo.system) && _.isString(nodeData.nodeinfo.system.domain_code)) { - domain = nodeData.nodeinfo.system.domain_code as Domain; + let domain: Domain | undefined; + if (_.isPlainObject(nodeData.nodeinfo.system) && isDomain(nodeData.nodeinfo.system.domain_code)) { + domain = nodeData.nodeinfo.system.domain_code; } return { @@ -611,8 +613,8 @@ export async function getByMacs(macs: MAC[]): Promise } nodeStateByMac[row.mac] = { - site: row.site || "" as Site, // FIXME: Handle this - domain: row.domain || "" as Domain, // FIXME: Handle this + site: row.site || undefined, + domain: row.domain || undefined, state: onlineState, }; } diff --git a/server/types/index.ts b/server/types/index.ts index f409f04..04b39d8 100644 --- a/server/types/index.ts +++ b/server/types/index.ts @@ -23,8 +23,8 @@ export * from "./logger"; export * from "./shared"; export type NodeStateData = { - site: Site, - domain: Domain, + site?: Site, + domain?: Domain, state: OnlineState, } From b734a422a71aac040f7999caff87f7a08e2c8781 Mon Sep 17 00:00:00 2001 From: baldo Date: Thu, 28 Jul 2022 12:24:45 +0200 Subject: [PATCH 06/13] Make hostname typed in NodeFilter. --- server/services/nodeService.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/services/nodeService.ts b/server/services/nodeService.ts index 74adeaa..8807f8d 100644 --- a/server/services/nodeService.ts +++ b/server/services/nodeService.ts @@ -38,8 +38,7 @@ import util from "util"; const pglob = util.promisify(glob); type NodeFilter = { - // TODO: Newtype - hostname?: string, + hostname?: Hostname, mac?: MAC, key?: FastdKey, token?: Token, From 5592892f0d9f49c1f83f917743da8ab2badeac0c Mon Sep 17 00:00:00 2001 From: baldo Date: Thu, 28 Jul 2022 13:16:13 +0200 Subject: [PATCH 07/13] Get rid of lots of unnecessary lodash calls. --- server/logger.test.ts | 17 +++---- server/logger.ts | 9 ++-- server/resources/monitoringResource.ts | 6 +-- server/resources/nodeResource.ts | 21 ++++---- server/resources/taskResource.ts | 10 ++-- server/services/mailService.ts | 2 +- server/services/mailTemplateService.ts | 2 +- server/services/monitoringService.ts | 25 +++++----- server/services/nodeService.ts | 10 ++-- server/types/shared.ts | 4 ++ server/utils/databaseUtil.ts | 2 +- server/utils/resources.ts | 16 +++--- server/utils/strings.ts | 4 +- server/utils/time.ts | 5 +- server/utils/urlBuilder.ts | 14 ++---- server/validation/validator.ts | 67 ++++++++++++-------------- 16 files changed, 99 insertions(+), 115 deletions(-) diff --git a/server/logger.test.ts b/server/logger.test.ts index d3c946c..db93643 100644 --- a/server/logger.test.ts +++ b/server/logger.test.ts @@ -1,6 +1,5 @@ -import {LogLevel, LogLevels, isLogLevel} from "./types"; +import {isLogLevel, LogLevel, LogLevels} from "./types"; import {ActivatableLoggerImpl} from "./logger"; -import _ from "lodash"; class TestableLogger extends ActivatableLoggerImpl { private logs: any[][] = []; @@ -35,13 +34,13 @@ function parseLogEntry(logEntry: any[]): ParsedLogEntry { `Empty log entry. Should always start with log message: ${logEntry}` ); } - + const logMessage = logEntry[0]; if (typeof logMessage !== "string") { throw new Error( `Expected log entry to start with string, but got: ${logMessage}` ); - } + } const regexp = /^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2} ([A-Z]+) - (\[[^\]]*\])? *(.*)$/; const groups = logMessage.match(regexp); @@ -59,13 +58,13 @@ function parseLogEntry(logEntry: any[]): ParsedLogEntry { } const tagsStr = groups[2].substring(1, groups[2].length - 1); - const tags = tagsStr ? _.split(tagsStr, ", ") : []; + const tags = tagsStr ? tagsStr.split(", "): []; const message = groups[3]; const args = logEntry.slice(1); return { level: level as LogLevel, - tags, + tags, message, args, }; @@ -158,7 +157,7 @@ for (const level of LogLevels) { message: "%s %d %f %o %%", args: [], }]); - }); + }); test(`should not escape ${level} message arguments`, () => { // given @@ -174,7 +173,7 @@ for (const level of LogLevels) { message: "message", args: [1, "%s", "%d", "%f", "%o", "%"], }]); - }); + }); test(`should not log ${level} message on disabled logger`, () => { // given @@ -185,6 +184,6 @@ for (const level of LogLevels) { // then expect(parseLogs(logger.getLogs())).toEqual([]); - }); + }); } diff --git a/server/logger.ts b/server/logger.ts index edaadc7..f787c8a 100644 --- a/server/logger.ts +++ b/server/logger.ts @@ -1,6 +1,5 @@ -import {Logger, TaggedLogger, LogLevel} from './types'; +import {isString, Logger, LogLevel, TaggedLogger} from './types'; import moment from 'moment'; -import _ from 'lodash'; export type LoggingFunction = (...args: any[]) => void; @@ -36,15 +35,15 @@ export class ActivatableLoggerImpl implements ActivatableLogger { log(level: LogLevel, ...args: any[]): void { const timeStr = moment().format('YYYY-MM-DD HH:mm:ss'); const levelStr = level.toUpperCase(); - const tagsStr = tags ? '[' + _.join(tags, ', ') + ']' : ''; + const tagsStr = tags ? '[' + tags.join(', ') + ']' : ''; const messagePrefix = `${timeStr} ${levelStr} - ${tagsStr}`; // Make sure to only replace %s, etc. in real log message // but not in tags. const escapedMessagePrefix = messagePrefix.replace(/%/g, '%%'); - + let message = ''; - if (args && _.isString(args[0])) { + if (args && isString(args[0])) { message = args[0]; args.shift(); } diff --git a/server/resources/monitoringResource.ts b/server/resources/monitoringResource.ts index ec54a3f..4b44c0c 100644 --- a/server/resources/monitoringResource.ts +++ b/server/resources/monitoringResource.ts @@ -1,5 +1,3 @@ -import _ from "lodash"; - import CONSTRAINTS from "../validation/constraints"; import ErrorTypes from "../utils/errorTypes"; import * as MonitoringService from "../services/monitoringService"; @@ -17,8 +15,8 @@ async function doGetAll(req: Request): Promise<{ total: number, result: any }> { const {monitoringStates, total} = await MonitoringService.getAll(restParams); return { total, - result: _.map(monitoringStates, function (state) { - state.mapId = _.toLower(state.mac).replace(/:/g, ''); + result: monitoringStates.map(state => { + state.mapId = state.mac.toLowerCase().replace(/:/g, ""); return state; }) }; diff --git a/server/resources/nodeResource.ts b/server/resources/nodeResource.ts index 92d9d14..afb754a 100644 --- a/server/resources/nodeResource.ts +++ b/server/resources/nodeResource.ts @@ -1,5 +1,3 @@ -import _ from "lodash"; - import Constraints from "../validation/constraints"; import ErrorTypes from "../utils/errorTypes"; import * as MonitoringService from "../services/monitoringService"; @@ -13,12 +11,12 @@ import { CreateOrUpdateNode, DomainSpecificNodeResponse, isNodeSortField, - isToken, JSONObject, + isToken, + JSONObject, MAC, NodeResponse, NodeStateData, NodeTokenResponse, - StoredNode, toDomainSpecificNodeResponse, Token, toNodeResponse, @@ -27,15 +25,18 @@ import { const nodeFields = ['hostname', 'key', 'email', 'nickname', 'mac', 'coords', 'monitoring']; +// TODO: Rename function getNormalizedNodeData(reqData: any): CreateOrUpdateNode { const node: { [key: string]: any } = {}; - _.each(nodeFields, function (field) { + for (const field of nodeFields) { let value = normalizeString(reqData[field]); if (field === 'mac') { value = normalizeMac(value as MAC); } node[field] = value; - }); + } + + // TODO: Add typeguard before cast. return node as CreateOrUpdateNode; } @@ -90,15 +91,15 @@ async function doGetAll(req: Request): Promise<{ total: number; pageNodes: any } const nodes = await NodeService.getAllNodes(); - const realNodes = _.filter(nodes, node => + const realNodes = nodes.filter(node => // We ignore nodes without tokens as those are only manually added ones like gateways. - !!node.token + !!node.token // FIXME: As node.token may not be undefined or null here, handle this when loading! ); - const macs: MAC[] = _.map(realNodes, (node: StoredNode): MAC => node.mac); + const macs: MAC[] = realNodes.map(node => node.mac); const nodeStateByMac = await MonitoringService.getByMacs(macs); - const domainSpecificNodes: DomainSpecificNodeResponse[] = _.map(realNodes, (node: StoredNode): DomainSpecificNodeResponse => { + const domainSpecificNodes: DomainSpecificNodeResponse[] = realNodes.map(node => { const nodeState: NodeStateData = nodeStateByMac[node.mac] || {}; return toDomainSpecificNodeResponse(node, nodeState); }); diff --git a/server/resources/taskResource.ts b/server/resources/taskResource.ts index e598bcb..6020d05 100644 --- a/server/resources/taskResource.ts +++ b/server/resources/taskResource.ts @@ -1,9 +1,7 @@ -import _ from "lodash"; - import CONSTRAINTS from "../validation/constraints"; import ErrorTypes from "../utils/errorTypes"; import * as Resources from "../utils/resources"; -import {Entity, handleJSONWithData, RequestData} from "../utils/resources"; +import {handleJSONWithData, RequestData} from "../utils/resources"; import {getTasks, Task, TaskState} from "../jobs/scheduler"; import {normalizeString} from "../utils/strings"; import {forConstraint} from "../validation/validator"; @@ -77,11 +75,11 @@ async function setTaskEnabled(data: RequestData, enable: boolean): Promise { +async function doGetAll(req: Request): Promise<{ total: number, pageTasks: Task[] }> { const restParams = await Resources.getValidRestParams('list', null, req); const tasks = Resources.sort( - _.values(getTasks()), + Object.values(getTasks()), isTaskSortField, restParams ); @@ -104,7 +102,7 @@ export function getAll(req: Request, res: Response): void { doGetAll(req) .then(({total, pageTasks}) => { res.set('X-Total-Count', total.toString(10)); - Resources.success(res, _.map(pageTasks, toTaskResponse)); + Resources.success(res, pageTasks.map(toTaskResponse)); }) .catch(err => Resources.error(res, err)); } diff --git a/server/services/mailService.ts b/server/services/mailService.ts index 1068375..1060c62 100644 --- a/server/services/mailService.ts +++ b/server/services/mailService.ts @@ -170,7 +170,7 @@ export async function getPendingMails(restParams: RestParams): Promise<{ mails: const mails = await db.all( 'SELECT * FROM email_queue WHERE ' + filter.query, - _.concat([], filter.params), + filter.params, ); return { diff --git a/server/services/mailTemplateService.ts b/server/services/mailTemplateService.ts index e9c617f..d2289c4 100644 --- a/server/services/mailTemplateService.ts +++ b/server/services/mailTemplateService.ts @@ -97,7 +97,7 @@ export async function render(mailOptions: Mail): Promise<{subject: string, body: try { return { - subject: _.trim(_.template(subject.toString())(data)), + subject: _.template(subject.toString())(data).trim(), body: _.template(body.toString())(data) }; } catch (error) { diff --git a/server/services/monitoringService.ts b/server/services/monitoringService.ts index 65d0a37..5f6c2d5 100644 --- a/server/services/monitoringService.ts +++ b/server/services/monitoringService.ts @@ -19,10 +19,13 @@ import { Domain, DurationSeconds, Hostname, + isBoolean, isDomain, isMonitoringSortField, isOnlineState, isSite, + isString, + isUndefined, MAC, MailType, MonitoringSortField, @@ -164,7 +167,7 @@ async function storeNodeInformation(nodeData: ParsedNode, node: StoredNode): Pro const row = await db.get('SELECT * FROM node_state WHERE mac = ?', [node.mac]); - if (_.isUndefined(row)) { + if (isUndefined(row)) { return await insertNodeInformation(nodeData, node); } else { return await updateNodeInformation(nodeData, node, row); @@ -188,7 +191,7 @@ export function parseNode(importTimestamp: UnixTimestampSeconds, nodeData: any): } const nodeId = nodeData.nodeinfo.node_id; - if (!nodeId || !_.isString(nodeId)) { + if (!nodeId || !isString(nodeId)) { throw new Error( `Invalid node id of type "${typeof nodeId}": ${nodeId}` ); @@ -212,7 +215,7 @@ export function parseNode(importTimestamp: UnixTimestampSeconds, nodeData: any): 'Node ' + nodeId + ': Unexpected flags type: ' + (typeof nodeData.flags) ); } - if (!_.isBoolean(nodeData.flags.online)) { + if (!isBoolean(nodeData.flags.online)) { throw new Error( 'Node ' + nodeId + ': Unexpected flags.online type: ' + (typeof nodeData.flags.online) ); @@ -558,6 +561,7 @@ async function retrieveNodeInformationForUrls(urls: string[]): Promise { const filterFields = [ 'hostname', @@ -571,7 +575,7 @@ export async function getAll(restParams: RestParams): Promise<{ total: number, m const row = await db.get<{ total: number }>( 'SELECT count(*) AS total FROM node_state WHERE ' + where.query, - _.concat([], where.params), + where.params, ); const total = row?.total || 0; @@ -585,7 +589,7 @@ export async function getAll(restParams: RestParams): Promise<{ total: number, m const monitoringStates = await db.all( 'SELECT * FROM node_state WHERE ' + filter.query, - _.concat([], filter.params), + filter.params, ); return {monitoringStates, total}; @@ -603,7 +607,7 @@ export async function getByMacs(macs: MAC[]): Promise const rows = await db.all( 'SELECT * FROM node_state WHERE ' + inCondition.query, - _.concat([], inCondition.params), + inCondition.params, ); for (const row of rows) { @@ -734,7 +738,7 @@ async function deleteNeverOnlineNodesBefore(deleteBefore: UnixTimestampSeconds): deletionCandidates.length ); - const deletionCandidateMacs: MAC[] = _.map(deletionCandidates, node => node.mac); + const deletionCandidateMacs: MAC[] = deletionCandidates.map(node => node.mac); const chunks: MAC[][] = _.chunk(deletionCandidateMacs, NEVER_ONLINE_NODES_DELETION_CHUNK_SIZE); Logger @@ -753,10 +757,7 @@ async function deleteNeverOnlineNodesBefore(deleteBefore: UnixTimestampSeconds): ' MACs for deletion.' ); - const placeholders = _.join( - _.map(macs, () => '?'), - ',' - ); + const placeholders = macs.map(() => '?').join(','); const rows: { mac: MAC }[] = await db.all( `SELECT * FROM node_state WHERE mac IN (${placeholders})`, @@ -773,7 +774,7 @@ async function deleteNeverOnlineNodesBefore(deleteBefore: UnixTimestampSeconds): ' nodes found in monitoring database. Those should be skipped.' ); - const seenMacs: MAC[] = _.map(rows, (row: { mac: MAC }) => row.mac as MAC); + const seenMacs: MAC[] = rows.map(row => row.mac); const neverSeenMacs = _.difference(macs, seenMacs); Logger diff --git a/server/services/nodeService.ts b/server/services/nodeService.ts index 8807f8d..7a12def 100644 --- a/server/services/nodeService.ts +++ b/server/services/nodeService.ts @@ -108,15 +108,15 @@ async function findFilesInPeersPath(): Promise { } function parseNodeFilename(filename: string): NodeFilenameParsed { - const parts = _.split(filename, '@', filenameParts.length); + const parts = filename.split('@', filenameParts.length); const parsed: { [key: string]: string | undefined } = {}; const zippedParts = _.zip(filenameParts, parts); - _.each(zippedParts, part => { + for (const part of zippedParts) { const key = part[0]; if (key) { parsed[key] = part[1]; } - }); + } return parsed; } @@ -565,14 +565,14 @@ export async function fixNodeFilenames(): Promise { export async function findNodesModifiedBefore(timestamp: UnixTimestampSeconds): Promise { const nodes = await getAllNodes(); - return _.filter(nodes, node => node.modifiedAt < timestamp); + return nodes.filter(node => node.modifiedAt < timestamp); } export async function getNodeStatistics(): Promise { const nodes = await getAllNodes(); const nodeStatistics: NodeStatistics = { - registered: _.size(nodes), + registered: nodes.length, withVPN: 0, withCoords: 0, monitoring: { diff --git a/server/types/shared.ts b/server/types/shared.ts index 62c1a58..e1b31e0 100644 --- a/server/types/shared.ts +++ b/server/types/shared.ts @@ -116,6 +116,10 @@ export function toIsEnum(enumDef: E): EnumTypeGuard { return (arg): arg is EnumValue => Object.values(enumDef).includes(arg as [keyof E]); } +export function isRegExp(arg: unknown): arg is RegExp { + return isObject(arg) && arg instanceof RegExp; +} + export function isOptional(arg: unknown, isT: TypeGuard): arg is (T | undefined) { return arg === undefined || isT(arg); } diff --git a/server/utils/databaseUtil.ts b/server/utils/databaseUtil.ts index 0865577..aad4b66 100644 --- a/server/utils/databaseUtil.ts +++ b/server/utils/databaseUtil.ts @@ -2,7 +2,7 @@ import _ from "lodash"; export function inCondition(field: string, list: T[]): {query: string, params: T[]} { return { - query: '(' + field + ' IN (' + _.join(_.times(list.length, _.constant('?')), ', ') + '))', + query: '(' + field + ' IN (' + _.times(list.length, () =>'?').join(', ') + '))', params: list, } } diff --git a/server/utils/resources.ts b/server/utils/resources.ts index 94722fc..3d459c5 100644 --- a/server/utils/resources.ts +++ b/server/utils/resources.ts @@ -184,7 +184,7 @@ export function filter(entities: E[], allowedFilterFields: string[], restPara if (!query) { return true; } - return _.some(allowedFilterFields, (field: string): boolean => { + return allowedFilterFields.some((field: string): boolean => { if (!query) { return true; } @@ -209,15 +209,15 @@ export function filter(entities: E[], allowedFilterFields: string[], restPara const filters = restParams.filters; function filtersMatch(entity: Entity): boolean { - if (_.isEmpty(filters)) { + if (isUndefined(filters) || _.isEmpty(filters)) { return true; } - return _.every(filters, (value: any, key: string): boolean => { + return Object.entries(filters).every(([key, value]) => { if (isUndefined(value)) { return true; } - if (_.startsWith(key, 'has')) { + if (key.startsWith('has')) { const entityKey = key.substring(3, 4).toLowerCase() + key.substring(4); return _.isEmpty(entity[entityKey]).toString() !== value; } @@ -225,9 +225,7 @@ export function filter(entities: E[], allowedFilterFields: string[], restPara }); } - return _.filter(entities, function (entity) { - return queryMatches(entity) && filtersMatch(entity); - }); + return entities.filter(entity => queryMatches(entity) && filtersMatch(entity)); } export function sort, S extends string>(entities: T[], isSortField: TypeGuard, restParams: RestParams): T[] { @@ -262,7 +260,7 @@ export function sort, S extends string>(entities: T[], return sorted; } -export function getPageEntities(entities: Entity[], restParams: RestParams) { +export function getPageEntities(entities: Entity[], restParams: RestParams): Entity[] { const page = restParams._page; const perPage = restParams._perPage; @@ -291,7 +289,7 @@ export function filterClause( return { query: filter.query + ' ' + orderBy.query + ' ' + limitOffset.query, - params: _.concat(filter.params, orderBy.params, limitOffset.params) + params: [...filter.params, ...orderBy.params, ...limitOffset.params] }; } diff --git a/server/utils/strings.ts b/server/utils/strings.ts index 12ba84d..b19f155 100644 --- a/server/utils/strings.ts +++ b/server/utils/strings.ts @@ -1,8 +1,8 @@ import _ from "lodash" -import {MAC} from "../types"; +import {isString, MAC} from "../types"; export function normalizeString(str: string): string { - return _.isString(str) ? str.trim().replace(/\s+/g, ' ') : str; + return isString(str) ? str.trim().replace(/\s+/g, ' ') : str; } export function normalizeMac(mac: MAC): MAC { diff --git a/server/utils/time.ts b/server/utils/time.ts index 2dd4a42..83bcdf8 100644 --- a/server/utils/time.ts +++ b/server/utils/time.ts @@ -1,5 +1,4 @@ -import {DurationSeconds, UnixTimestampSeconds} from "../types"; -import _ from "lodash"; +import {DurationSeconds, isString, UnixTimestampSeconds} from "../types"; import moment, {Moment} from "moment"; export function now(): UnixTimestampSeconds { @@ -45,7 +44,7 @@ export function formatTimestamp(timestamp: UnixTimestampSeconds): string { } export function parseTimestamp(timestamp: any): UnixTimestampSeconds | null { - if (!_.isString(timestamp)) { + if (!isString(timestamp)) { return null; } const parsed = moment.utc(timestamp); diff --git a/server/utils/urlBuilder.ts b/server/utils/urlBuilder.ts index 79b037e..384537c 100644 --- a/server/utils/urlBuilder.ts +++ b/server/utils/urlBuilder.ts @@ -1,4 +1,3 @@ -import _ from "lodash" import {config} from "../config" import {MonitoringToken, Url} from "../types" @@ -12,15 +11,10 @@ function formUrl(route: string, queryParams?: { [key: string]: string }): Url { } if (queryParams) { url += '?'; - url += _.join( - _.map( - queryParams, - function (value, key) { - return encodeURIComponent(key) + '=' + encodeURIComponent(value); - } - ), - '&' - ); + url += + Object.entries(queryParams) + .map(([key, value]) => encodeURIComponent(key) + '=' + encodeURIComponent(value)) + .join("&"); } return url as Url; } diff --git a/server/validation/validator.ts b/server/validation/validator.ts index 1879f0e..523ac14 100644 --- a/server/validation/validator.ts +++ b/server/validation/validator.ts @@ -1,7 +1,6 @@ -import _ from "lodash"; - import {parseInteger} from "../utils/strings"; import Logger from "../logger"; +import {isArray, isBoolean, isNumber, isObject, isRegExp, isString, isUndefined} from "../types"; export interface Constraint { type: string, @@ -18,52 +17,48 @@ export interface Constraint { regex?: RegExp, } -export type Constraints = {[key: string]: Constraint}; -export type Values = {[key: string]: any}; - -function isStringArray(arr: any): arr is string[] { - return _.isArray(arr) && _.every(arr, (val: any) => _.isString(val)); -} +export type Constraints = { [key: string]: Constraint }; +export type Values = { [key: string]: any }; export function isConstraint(val: any): val is Constraint { - if (!_.isObject(val)) { + if (!isObject(val)) { return false; } - const constraint = val as {[key: string]: any}; + const constraint = val as { [key: string]: any }; - if (!("type" in constraint) || !_.isString(constraint.type)) { + if (!("type" in constraint) || !isString(constraint.type)) { return false; } if ("optional" in constraint - && !_.isUndefined(constraint.optional) - && !_.isBoolean(constraint.optional)) { + && !isUndefined(constraint.optional) + && !isBoolean(constraint.optional)) { return false; } if ("allowed" in constraint - && !_.isUndefined(constraint.allowed) - && !isStringArray(constraint.allowed)) { + && !isUndefined(constraint.allowed) + && !isArray(constraint.allowed, isString)) { return false; } if ("min" in constraint - && !_.isUndefined(constraint.min) - && !_.isNumber(constraint.min)) { + && !isUndefined(constraint.min) + && !isNumber(constraint.min)) { return false; } if ("max" in constraint - && !_.isUndefined(constraint.max) - && !_.isNumber(constraint.max)) { + && !isUndefined(constraint.max) + && !isNumber(constraint.max)) { return false; } // noinspection RedundantIfStatementJS if ("regex" in constraint - && !_.isUndefined(constraint.regex) - && !_.isRegExp(constraint.regex)) { + && !isUndefined(constraint.regex) + && !isRegExp(constraint.regex)) { return false; } @@ -71,41 +66,38 @@ export function isConstraint(val: any): val is Constraint { } export function isConstraints(constraints: any): constraints is Constraints { - if (!_.isObject(constraints)) { + if (!isObject(constraints)) { return false; } - return _.every( - constraints, - (constraint: any, key: any) => _.isString(key) && isConstraint(constraint) - ); + return Object.entries(constraints).every(([key, constraint]) => isString(key) && isConstraint(constraint)); } // TODO: sanitize input for further processing as specified by constraints (correct types, trimming, etc.) function isValidBoolean(value: any): boolean { - return _.isBoolean(value) || value === 'true' || value === 'false'; + return isBoolean(value) || value === 'true' || value === 'false'; } function isValidNumber(constraint: Constraint, value: any): boolean { - if (_.isString(value)) { + if (isString(value)) { value = parseInteger(value); } - if (!_.isNumber(value)) { + if (!isNumber(value)) { return false; } - if (_.isNaN(value) || !_.isFinite(value)) { + if (isNaN(value) || !isFinite(value)) { return false; } - if (_.isNumber(constraint.min) && value < constraint.min) { + if (isNumber(constraint.min) && value < constraint.min) { return false; } // noinspection RedundantIfStatementJS - if (_.isNumber(constraint.max) && value > constraint.max) { + if (isNumber(constraint.max) && value > constraint.max) { return false; } @@ -113,11 +105,12 @@ function isValidNumber(constraint: Constraint, value: any): boolean { } function isValidEnum(constraint: Constraint, value: any): boolean { - if (!_.isString(value)) { + if (!isString(value)) { return false; } - return _.indexOf(constraint.allowed, value) >= 0; + const allowed = constraint.allowed || []; + return allowed.indexOf(value) >= 0; } function isValidString(constraint: Constraint, value: any): boolean { @@ -125,7 +118,7 @@ function isValidString(constraint: Constraint, value: any): boolean { throw new Error("String constraints must have regex set: " + constraint); } - if (!_.isString(value)) { + if (!isString(value)) { return false; } @@ -174,10 +167,10 @@ function areValid(constraints: Constraints, acceptUndefined: boolean, values: Va return true; } -export function forConstraint (constraint: Constraint, acceptUndefined: boolean): (value: any) => boolean { +export function forConstraint(constraint: Constraint, acceptUndefined: boolean): (value: any) => boolean { return ((value: any): boolean => isValid(constraint, acceptUndefined, value)); } -export function forConstraints (constraints: Constraints, acceptUndefined: boolean): (values: Values) => boolean { +export function forConstraints(constraints: Constraints, acceptUndefined: boolean): (values: Values) => boolean { return ((values: Values): boolean => areValid(constraints, acceptUndefined, values)); } From d783bb634f32844933fd31e42ebf7be7f8f77c46 Mon Sep 17 00:00:00 2001 From: baldo Date: Thu, 28 Jul 2022 13:49:22 +0200 Subject: [PATCH 08/13] Get rid of some anys. --- server/jobs/scheduler.ts | 2 +- server/resources/monitoringResource.ts | 1 + server/resources/taskResource.ts | 24 ++++----- server/types/index.ts | 15 +++--- server/utils/resources.ts | 17 ++++--- server/validation/validator.ts | 70 ++++++++------------------ 6 files changed, 53 insertions(+), 76 deletions(-) diff --git a/server/jobs/scheduler.ts b/server/jobs/scheduler.ts index a66c0a1..0fafb5c 100644 --- a/server/jobs/scheduler.ts +++ b/server/jobs/scheduler.ts @@ -85,7 +85,7 @@ export class Task { this.job.run().then(result => { done(TaskState.IDLE, result); - }).catch((err: any) => { + }).catch(err => { Logger.tag('jobs').error("Job %s failed: %s", this.name, err); done(TaskState.FAILED, null); }); diff --git a/server/resources/monitoringResource.ts b/server/resources/monitoringResource.ts index 4b44c0c..1a78bb3 100644 --- a/server/resources/monitoringResource.ts +++ b/server/resources/monitoringResource.ts @@ -10,6 +10,7 @@ import {isMonitoringToken, JSONObject, MonitoringResponse, MonitoringToken, toMo const isValidToken = forConstraint(CONSTRAINTS.token, false); +// FIXME: Get rid of any async function doGetAll(req: Request): Promise<{ total: number, result: any }> { const restParams = await Resources.getValidRestParams('list', null, req); const {monitoringStates, total} = await MonitoringService.getAll(restParams); diff --git a/server/resources/taskResource.ts b/server/resources/taskResource.ts index 6020d05..8f2e939 100644 --- a/server/resources/taskResource.ts +++ b/server/resources/taskResource.ts @@ -10,18 +10,18 @@ import {isString, isTaskSortField} from "../types"; const isValidId = forConstraint(CONSTRAINTS.id, false); -interface TaskResponse { - id: number, - name: string, - description: string, - schedule: string, - runningSince: number | null, - lastRunStarted: number | null, - lastRunDuration: number | null, - state: string, - result: string | null, - message: string | null, - enabled: boolean, +type TaskResponse = { + id: number; + name: string; + description: string; + schedule: string; + runningSince: number | null; + lastRunStarted: number | null; + lastRunDuration: number | null; + state: string; + result: string | null; + message: string | null; + enabled: boolean; } function toTaskResponse(task: Task): TaskResponse { diff --git a/server/types/index.ts b/server/types/index.ts index 04b39d8..5324418 100644 --- a/server/types/index.ts +++ b/server/types/index.ts @@ -111,12 +111,11 @@ export enum MailType { export const isMailType = toIsEnum(MailType); -export interface Mail { - id: MailId, - email: MailType, - sender: EmailAddress, - recipient: EmailAddress, - data: MailData, - failures: number, +export type Mail = { + id: MailId; + email: MailType; + sender: EmailAddress; + recipient: EmailAddress; + data: MailData; + failures: number; } - diff --git a/server/utils/resources.ts b/server/utils/resources.ts index 3d459c5..89dbd95 100644 --- a/server/utils/resources.ts +++ b/server/utils/resources.ts @@ -9,8 +9,12 @@ import { EnumTypeGuard, EnumValue, type GenericSortField, - isJSONObject, isNumber, isString, isUndefined, + isJSONObject, + isNumber, + isString, + isUndefined, JSONObject, + JSONValue, SortDirection, TypeGuard } from "../types"; @@ -36,7 +40,9 @@ export type OrderByClause = { query: string, params: any[] }; export type LimitOffsetClause = { query: string, params: any[] }; export type FilterClause = { query: string, params: any[] }; -function respond(res: Response, httpCode: number, data: any, type: string): void { +function respond(res: Response, httpCode: number, data: string, type: "html"): void; +function respond(res: Response, httpCode: number, data: JSONValue, type: "json"): void; +function respond(res: Response, httpCode: number, data: JSONValue, type: "html" | "json"): void { switch (type) { case 'html': res.writeHead(httpCode, {'Content-Type': 'text/html'}); @@ -249,8 +255,7 @@ export function sort, S extends string>(entities: T[], let order = 0; if (as < bs) { order = -1; - } - else if (bs > as) { + } else if (bs > as) { order = 1; } @@ -293,7 +298,7 @@ export function filterClause( }; } -export function success(res: Response, data: any) { +export function success(res: Response, data: JSONValue) { respond(res, 200, data, 'json'); } @@ -301,7 +306,7 @@ export function successHtml(res: Response, html: string) { respond(res, 200, html, 'html'); } -export function error(res: Response, err: { data: any, type: { code: number } }) { +export function error(res: Response, err: { data: JSONValue, type: { code: number } }) { respond(res, err.type.code, err.data, 'json'); } diff --git a/server/validation/validator.ts b/server/validation/validator.ts index 523ac14..a771acf 100644 --- a/server/validation/validator.ts +++ b/server/validation/validator.ts @@ -1,6 +1,6 @@ import {parseInteger} from "../utils/strings"; import Logger from "../logger"; -import {isArray, isBoolean, isNumber, isObject, isRegExp, isString, isUndefined} from "../types"; +import {isBoolean, isNumber, isObject, isOptional, isRegExp, isString, toIsArray} from "../types"; export interface Constraint { type: string, @@ -20,52 +20,24 @@ export interface Constraint { export type Constraints = { [key: string]: Constraint }; export type Values = { [key: string]: any }; -export function isConstraint(val: any): val is Constraint { - if (!isObject(val)) { +export function isConstraint(arg: unknown): arg is Constraint { + if (!isObject(arg)) { return false; } - const constraint = val as { [key: string]: any }; - - if (!("type" in constraint) || !isString(constraint.type)) { - return false; - } - - if ("optional" in constraint - && !isUndefined(constraint.optional) - && !isBoolean(constraint.optional)) { - return false; - } - - if ("allowed" in constraint - && !isUndefined(constraint.allowed) - && !isArray(constraint.allowed, isString)) { - return false; - } - - if ("min" in constraint - && !isUndefined(constraint.min) - && !isNumber(constraint.min)) { - return false; - } - - if ("max" in constraint - && !isUndefined(constraint.max) - && !isNumber(constraint.max)) { - return false; - } - - // noinspection RedundantIfStatementJS - if ("regex" in constraint - && !isUndefined(constraint.regex) - && !isRegExp(constraint.regex)) { - return false; - } - - return true; + const constraint = arg as Constraint; + return ( + isString(constraint.type) && + // default?: any + isOptional(constraint.optional, isBoolean) && + isOptional(constraint.allowed, toIsArray(isString)) && + isOptional(constraint.min, isNumber) && + isOptional(constraint.max, isNumber) && + isOptional(constraint.regex, isRegExp) + ); } -export function isConstraints(constraints: any): constraints is Constraints { +export function isConstraints(constraints: unknown): constraints is Constraints { if (!isObject(constraints)) { return false; } @@ -75,11 +47,11 @@ export function isConstraints(constraints: any): constraints is Constraints { // TODO: sanitize input for further processing as specified by constraints (correct types, trimming, etc.) -function isValidBoolean(value: any): boolean { +function isValidBoolean(value: unknown): boolean { return isBoolean(value) || value === 'true' || value === 'false'; } -function isValidNumber(constraint: Constraint, value: any): boolean { +function isValidNumber(constraint: Constraint, value: unknown): boolean { if (isString(value)) { value = parseInteger(value); } @@ -104,7 +76,7 @@ function isValidNumber(constraint: Constraint, value: any): boolean { return true; } -function isValidEnum(constraint: Constraint, value: any): boolean { +function isValidEnum(constraint: Constraint, value: unknown): boolean { if (!isString(value)) { return false; } @@ -113,7 +85,7 @@ function isValidEnum(constraint: Constraint, value: any): boolean { return allowed.indexOf(value) >= 0; } -function isValidString(constraint: Constraint, value: any): boolean { +function isValidString(constraint: Constraint, value: unknown): boolean { if (!constraint.regex) { throw new Error("String constraints must have regex set: " + constraint); } @@ -126,7 +98,7 @@ function isValidString(constraint: Constraint, value: any): boolean { return (trimmed === '' && constraint.optional) || constraint.regex.test(trimmed); } -function isValid(constraint: Constraint, acceptUndefined: boolean, value: any): boolean { +function isValid(constraint: Constraint, acceptUndefined: boolean, value: unknown): boolean { if (value === undefined) { return acceptUndefined || constraint.optional === true; } @@ -167,8 +139,8 @@ function areValid(constraints: Constraints, acceptUndefined: boolean, values: Va return true; } -export function forConstraint(constraint: Constraint, acceptUndefined: boolean): (value: any) => boolean { - return ((value: any): boolean => isValid(constraint, acceptUndefined, value)); +export function forConstraint(constraint: Constraint, acceptUndefined: boolean): (value: unknown) => boolean { + return ((value: unknown): boolean => isValid(constraint, acceptUndefined, value)); } export function forConstraints(constraints: Constraints, acceptUndefined: boolean): (values: Values) => boolean { From 779c072ac783c7b78187cff6c3226e072e50cd27 Mon Sep 17 00:00:00 2001 From: baldo Date: Thu, 28 Jul 2022 13:56:47 +0200 Subject: [PATCH 09/13] Add typeguard for node data passed in requests. --- server/resources/nodeResource.ts | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/server/resources/nodeResource.ts b/server/resources/nodeResource.ts index afb754a..7b0cee6 100644 --- a/server/resources/nodeResource.ts +++ b/server/resources/nodeResource.ts @@ -10,9 +10,13 @@ import {Request, Response} from "express"; import { CreateOrUpdateNode, DomainSpecificNodeResponse, + isCreateOrUpdateNode, isNodeSortField, + isString, isToken, + isUndefined, JSONObject, + JSONValue, MAC, NodeResponse, NodeStateData, @@ -25,19 +29,27 @@ import { const nodeFields = ['hostname', 'key', 'email', 'nickname', 'mac', 'coords', 'monitoring']; -// TODO: Rename -function getNormalizedNodeData(reqData: any): CreateOrUpdateNode { +function getNormalizedNodeData(reqData: JSONObject): CreateOrUpdateNode { const node: { [key: string]: any } = {}; for (const field of nodeFields) { - let value = normalizeString(reqData[field]); - if (field === 'mac') { - value = normalizeMac(value as MAC); + let value: JSONValue | undefined = reqData[field]; + if (isString(value)) { + value = normalizeString(value); + if (field === 'mac') { + value = normalizeMac(value as MAC); + } + } + + if (!isUndefined(value)) { + node[field] = value; } - node[field] = value; } - // TODO: Add typeguard before cast. - return node as CreateOrUpdateNode; + if (isCreateOrUpdateNode(node)) { + return node; + } + + throw {data: "Invalid node data.", type: ErrorTypes.badRequest}; } const isValidNode = forConstraints(Constraints.node, false); From d2ca8ed55b3ece4e2613773f606b3d7e13ed06e6 Mon Sep 17 00:00:00 2001 From: baldo Date: Thu, 28 Jul 2022 14:09:46 +0200 Subject: [PATCH 10/13] Make node filename parsing more explicit and add stronger typing. --- server/services/monitoringService.ts | 2 -- server/services/nodeService.ts | 45 ++++++++++++++++------------ 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/server/services/monitoringService.ts b/server/services/monitoringService.ts index 5f6c2d5..11c9231 100644 --- a/server/services/monitoringService.ts +++ b/server/services/monitoringService.ts @@ -176,7 +176,6 @@ async function storeNodeInformation(nodeData: ParsedNode, node: StoredNode): Pro const isValidMac = forConstraint(CONSTRAINTS.node.mac, false); -// TODO: Use sparkson for JSON parsing. export function parseNode(importTimestamp: UnixTimestampSeconds, nodeData: any): ParsedNode { if (!_.isPlainObject(nodeData)) { throw new Error( @@ -249,7 +248,6 @@ export function parseNode(importTimestamp: UnixTimestampSeconds, nodeData: any): }; } -// TODO: Use sparkson for JSON parsing. export function parseNodesJson(body: string): NodesParsingResult { Logger.tag('monitoring', 'information-retrieval').debug('Parsing nodes.json...'); diff --git a/server/services/nodeService.ts b/server/services/nodeService.ts index 7a12def..8347640 100644 --- a/server/services/nodeService.ts +++ b/server/services/nodeService.ts @@ -1,4 +1,3 @@ -import _ from "lodash"; import async from "async"; import crypto from "crypto"; import oldFs, {promises as fs} from "graceful-fs"; @@ -18,7 +17,12 @@ import { EmailAddress, FastdKey, Hostname, + isFastdKey, + isHostname, + isMAC, + isMonitoringToken, isStoredNode, + isToken, MAC, MailType, MonitoringState, @@ -29,6 +33,7 @@ import { StoredNode, Token, toUnixTimestampSeconds, + TypeGuard, unhandledEnumField, UnixTimestampMilliseconds, UnixTimestampSeconds @@ -45,13 +50,12 @@ type NodeFilter = { monitoringToken?: MonitoringToken, } -// TODO: Newtypes? type NodeFilenameParsed = { - hostname?: string, - mac?: string, - key?: string, - token?: string, - monitoringToken?: string, + hostname?: Hostname, + mac?: MAC, + key?: FastdKey, + token?: Token, + monitoringToken?: MonitoringToken, } enum LINE_PREFIX { @@ -65,7 +69,6 @@ enum LINE_PREFIX { MONITORING_TOKEN = "# Monitoring-Token: ", } -const filenameParts = ['hostname', 'mac', 'key', 'token', 'monitoringToken']; function generateToken(): Type { return crypto.randomBytes(8).toString('hex') as Type; @@ -108,16 +111,20 @@ async function findFilesInPeersPath(): Promise { } function parseNodeFilename(filename: string): NodeFilenameParsed { - const parts = filename.split('@', filenameParts.length); - const parsed: { [key: string]: string | undefined } = {}; - const zippedParts = _.zip(filenameParts, parts); - for (const part of zippedParts) { - const key = part[0]; - if (key) { - parsed[key] = part[1]; - } + const parts = filename.split('@', 5); + + function get(isT: TypeGuard, index: number): T | undefined { + const value = index >= 0 && index < parts.length ? parts[index] : undefined; + return isT(value) ? value : undefined; } - return parsed; + + return { + hostname: get(isHostname, 0), + mac: get(isMAC, 1), + key: get(isFastdKey, 2), + token: get(isToken, 3), + monitoringToken: get(isMonitoringToken, 4), + }; } function isDuplicate(filter: NodeFilter, token?: Token): boolean { @@ -446,7 +453,7 @@ export async function updateNode(token: Token, node: CreateOrUpdateNode): Promis // monitoring just has been enabled monitoringState = MonitoringState.PENDING; monitoringToken = generateToken(); - break; + break; case MonitoringState.PENDING: case MonitoringState.ACTIVE: @@ -460,7 +467,7 @@ export async function updateNode(token: Token, node: CreateOrUpdateNode): Promis monitoringState = currentNode.monitoringState; monitoringToken = nodeSecrets.monitoringToken || generateToken(); } - break; + break; default: unhandledEnumField(currentNode.monitoringState); From 13e4895b8171cf14349982687b265e6b515c26f0 Mon Sep 17 00:00:00 2001 From: baldo Date: Thu, 28 Jul 2022 14:45:28 +0200 Subject: [PATCH 11/13] Added more explicit type for SMTP config and refactored transport initialization. --- README.md | 2 +- server/mail/index.ts | 29 +++++++++++++++++ server/main.ts | 3 +- server/services/mailService.ts | 33 ++++---------------- server/types/config.ts | 57 +++++++++++++++++++++++++--------- server/types/shared.ts | 3 ++ 6 files changed, 84 insertions(+), 43 deletions(-) create mode 100644 server/mail/index.ts diff --git a/README.md b/README.md index b82a0cf..11bb454 100644 --- a/README.md +++ b/README.md @@ -146,7 +146,7 @@ Dann die `config.json` anpassen nach belieben. Es gibt die folgenden Konfigurati * **`server.email.from`** Absender für versendete E-Mails, z. B.: `"Freifunk Knotenformular "` * **`server.email.smtp`** Konfiguration des SMTP-Servers für den E-Mail-Versand entsprechend der Dokumentation unter - [https://nodemailer.com/2-0-0-beta/setup-smtp/](https://nodemailer.com/2-0-0-beta/setup-smtp/), z. B.: + [https://nodemailer.com/smtp/](https://nodemailer.com/smtp/), z. B.: ``` { diff --git a/server/mail/index.ts b/server/mail/index.ts new file mode 100644 index 0000000..f47fec5 --- /dev/null +++ b/server/mail/index.ts @@ -0,0 +1,29 @@ +import {createTransport, Transporter} from "nodemailer"; +import {config} from "../config"; +import * as MailTemplateService from "../services/mailTemplateService"; +import Mail from "nodemailer/lib/mailer"; +import SMTPTransport from "nodemailer/lib/smtp-transport"; + +let transporterSingleton: Transporter | null = null; + +function transporter() { + if (!transporterSingleton) { + const options = { + ...config.server.email.smtp, + pool: true, + }; + transporterSingleton = createTransport(new SMTPTransport(options)); + + MailTemplateService.configureTransporter(transporterSingleton); + } + + return transporterSingleton; +} + +export function init(): void { + transporter(); +} + +export async function send(options: Mail.Options): Promise { + await transporter().sendMail(options); +} diff --git a/server/main.ts b/server/main.ts index 657fd3f..dbd3884 100755 --- a/server/main.ts +++ b/server/main.ts @@ -5,6 +5,7 @@ import * as db from "./db/database" import * as scheduler from "./jobs/scheduler" import * as router from "./router" import * as app from "./app" +import * as mail from "./mail"; app.init(); Logger.init(config.server.logging.enabled); @@ -14,8 +15,8 @@ async function main() { Logger.tag('main').info('Initializing...'); await db.init(); + mail.init(); scheduler.init(); - router.init(); app.app.listen(config.server.port, '::'); diff --git a/server/services/mailService.ts b/server/services/mailService.ts index 1060c62..d698902 100644 --- a/server/services/mailService.ts +++ b/server/services/mailService.ts @@ -1,17 +1,15 @@ import _ from "lodash"; -import deepExtend from "deep-extend"; import moment, {Moment} from "moment"; -import {createTransport, Transporter} from "nodemailer"; - -import {config} from "../config"; import {db} from "../db/database"; import Logger from "../logger"; import * as MailTemplateService from "./mailTemplateService"; import * as Resources from "../utils/resources"; import {RestParams} from "../utils/resources"; import { - EmailAddress, isJSONObject, - isMailSortField, isMailType, JSONObject, + EmailAddress, + isJSONObject, + isMailSortField, + isMailType, Mail, MailData, MailId, @@ -21,6 +19,7 @@ import { UnixTimestampSeconds } from "../types"; import ErrorTypes from "../utils/errorTypes"; +import {send} from "../mail"; type EmaiQueueRow = { id: MailId, @@ -35,26 +34,6 @@ type EmaiQueueRow = { const MAIL_QUEUE_DB_BATCH_SIZE = 50; -// TODO: Extract transporter into own module and initialize during main(). -let transporterSingleton: Transporter | null = null; - -function transporter() { - if (!transporterSingleton) { - transporterSingleton = createTransport(deepExtend( - {}, - config.server.email.smtp, - { - transport: 'smtp', - pool: true - } as JSONObject - )); - - MailTemplateService.configureTransporter(transporterSingleton); - } - - return transporterSingleton; -} - async function sendMail(options: Mail): Promise { Logger .tag('mail', 'queue') @@ -73,7 +52,7 @@ async function sendMail(options: Mail): Promise { html: renderedTemplate.body }; - await transporter().sendMail(mailOptions); + await send(mailOptions); Logger.tag('mail', 'queue').info('Mail[%d] has been send.', options.id); } diff --git a/server/types/config.ts b/server/types/config.ts index 46585a3..7710b39 100644 --- a/server/types/config.ts +++ b/server/types/config.ts @@ -1,5 +1,5 @@ import {ArrayField, Field, RawJsonField} from "sparkson" -import {ClientConfig, isSite, isString, JSONObject, toIsNewtype, Url} from "./shared"; +import {ClientConfig, DurationMilliseconds, isString, toIsNewtype, Url} from "./shared"; export type Username = string & { readonly __tag: unique symbol }; export const isUsername = toIsNewtype(isString, "" as Username); @@ -14,7 +14,8 @@ export class UsersConfig { constructor( @Field("user") public username: Username, @Field("passwordHash") public passwordHash: PasswordHash, - ) {} + ) { + } } export class LoggingConfig { @@ -22,51 +23,79 @@ export class LoggingConfig { @Field("enabled") public enabled: boolean, @Field("debug") public debug: boolean, @Field("profile") public profile: boolean, - ) {} + ) { + } } export class InternalConfig { constructor( @Field("active") public active: boolean, @ArrayField("users", UsersConfig) public users: UsersConfig[], - ) {} + ) { + } +} + +export class SMTPAuthConfig { + constructor( + @Field("user") public user: Username, + @Field("pass") public pass: CleartextPassword, + ) { + } +} + +// For details see: https://nodemailer.com/smtp/ +export class SMTPConfig { + constructor( + @Field("host") public host?: string, + @Field("port") public port?: number, + @Field("auth") public auth?: SMTPAuthConfig, + @Field("secure") public secure?: boolean, + @Field("ignoreTLS") public ignoreTLS?: boolean, + @Field("requireTLS") public requireTLS?: boolean, + @Field("opportunisticTLS") public opportunisticTLS?: boolean, + @Field("name") public name?: string, + @Field("localAddress") public localAddress?: string, + @Field("connectionTimeout") public connectionTimeout?: DurationMilliseconds, + @Field("greetingTimeout") public greetingTimeout?: DurationMilliseconds, + @Field("socketTimeout") public socketTimeout?: DurationMilliseconds, + ) { + } } export class EmailConfig { constructor( @Field("from") public from: string, - - // For details see: https://nodemailer.com/2-0-0-beta/setup-smtp/ - @RawJsonField("smtp") public smtp: JSONObject, - ) {} + @RawJsonField("smtp") public smtp: SMTPConfig, + ) { + } } export class ServerMapConfig { constructor( @ArrayField("nodesJsonUrl", String) public nodesJsonUrl: Url[], - ) {} + ) { + } } export class ServerConfig { constructor( @Field("baseUrl") public baseUrl: Url, @Field("port") public port: number, - @Field("databaseFile") public databaseFile: string, @Field("peersPath") public peersPath: string, - @Field("logging") public logging: LoggingConfig, @Field("internal") public internal: InternalConfig, @Field("email") public email: EmailConfig, @Field("map") public map: ServerMapConfig, - @Field("rootPath", true, undefined, "/") public rootPath: string, - ) {} + ) { + } } export class Config { constructor( @Field("server") public server: ServerConfig, @Field("client") public client: ClientConfig, - ) {} + ) { + } } diff --git a/server/types/shared.ts b/server/types/shared.ts index e1b31e0..b1b050a 100644 --- a/server/types/shared.ts +++ b/server/types/shared.ts @@ -344,6 +344,9 @@ export const isMAC = toIsNewtype(isString, "" as MAC); export type DurationSeconds = number & { readonly __tag: unique symbol }; export const isDurationSeconds = toIsNewtype(isNumber, NaN as DurationSeconds); +export type DurationMilliseconds = number & { readonly __tag: unique symbol }; +export const isDurationMilliseconds = toIsNewtype(isNumber, NaN as DurationMilliseconds); + export type UnixTimestampSeconds = number & { readonly __tag: unique symbol }; export const isUnixTimestampSeconds = toIsNewtype(isNumber, NaN as UnixTimestampSeconds); From 111fac6e47f7c50dd457f833d7dd80b4bdfdeb96 Mon Sep 17 00:00:00 2001 From: baldo Date: Thu, 28 Jul 2022 15:07:35 +0200 Subject: [PATCH 12/13] Fix logging: Config options for debugging and profiling had no effect. --- server/logger.test.ts | 72 +++++++++++++++++++++++++++++++++++++++++-- server/logger.ts | 22 ++++++++----- server/main.ts | 2 +- 3 files changed, 84 insertions(+), 12 deletions(-) diff --git a/server/logger.test.ts b/server/logger.test.ts index db93643..7103807 100644 --- a/server/logger.test.ts +++ b/server/logger.test.ts @@ -1,13 +1,25 @@ -import {isLogLevel, LogLevel, LogLevels} from "./types"; +import {isLogLevel, isUndefined, LoggingConfig, LogLevel, LogLevels} from "./types"; import {ActivatableLoggerImpl} from "./logger"; +function withDefault(value: T | undefined, defaultValue: T): T { + return isUndefined(value) ? defaultValue : value; +} + class TestableLogger extends ActivatableLoggerImpl { private logs: any[][] = []; - constructor(enabled?: boolean) { + constructor( + enabled?: boolean, + debug?: boolean, + profile?: boolean, + ) { super(); this.init( - enabled === false ? false : true, // default is true + new LoggingConfig( + withDefault(enabled, true), + withDefault(debug, true), + withDefault(profile, true), + ), (...args: any[]): void => this.doLog(...args) ); } @@ -42,6 +54,7 @@ function parseLogEntry(logEntry: any[]): ParsedLogEntry { ); } + // noinspection RegExpRedundantEscape const regexp = /^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2} ([A-Z]+) - (\[[^\]]*\])? *(.*)$/; const groups = logMessage.match(regexp); if (groups === null || groups.length < 4) { @@ -187,3 +200,56 @@ for (const level of LogLevels) { }); } +test(`should not log debug message with disabled debugging`, () => { + // given + const logger = new TestableLogger(true, false, true); + + // when + logger.tag("tag").debug("message"); + + // then + expect(parseLogs(logger.getLogs())).toEqual([]); +}); + +test(`should log profile message with disabled debugging`, () => { + // given + const logger = new TestableLogger(true, false, true); + + // when + logger.tag("tag").profile("message"); + + // then + expect(parseLogs(logger.getLogs())).toEqual([{ + level: "profile", + tags: ["tag"], + message: "message", + args: [], + }]); +}); + +test(`should not log profile message with disabled profiling`, () => { + // given + const logger = new TestableLogger(true, true, false); + + // when + logger.tag("tag").profile("message"); + + // then + expect(parseLogs(logger.getLogs())).toEqual([]); +}); + +test(`should log debug message with disabled profiling`, () => { + // given + const logger = new TestableLogger(true, true, false); + + // when + logger.tag("tag").debug("message"); + + // then + expect(parseLogs(logger.getLogs())).toEqual([{ + level: "debug", + tags: ["tag"], + message: "message", + args: [], + }]); +}); diff --git a/server/logger.ts b/server/logger.ts index f787c8a..d3dee8e 100644 --- a/server/logger.ts +++ b/server/logger.ts @@ -1,4 +1,4 @@ -import {isString, Logger, LogLevel, TaggedLogger} from './types'; +import {isString, Logger, LoggingConfig, LogLevel, TaggedLogger} from './types'; import moment from 'moment'; export type LoggingFunction = (...args: any[]) => void; @@ -13,23 +13,25 @@ const noopTaggedLogger: TaggedLogger = { }; export interface ActivatableLogger extends Logger { - init(enabled: boolean, loggingFunction?: LoggingFunction): void; + init(config: LoggingConfig, loggingFunction?: LoggingFunction): void; } /** * TODO: Check if LoggingConfig.debug and LoggingConfig.profile are handled. */ export class ActivatableLoggerImpl implements ActivatableLogger { - private enabled: boolean = false; + private config: LoggingConfig = new LoggingConfig(false, false, false); private loggingFunction: LoggingFunction = console.info; - init(enabled: boolean, loggingFunction?: LoggingFunction): void { - this.enabled = enabled; + init(config: LoggingConfig, loggingFunction?: LoggingFunction): void { + this.config = config; this.loggingFunction = loggingFunction || console.info; } tag(...tags: string[]): TaggedLogger { - if (this.enabled) { + if (this.config.enabled) { + const debug = this.config.debug; + const profile = this.config.profile; const loggingFunction = this.loggingFunction; return { log(level: LogLevel, ...args: any[]): void { @@ -54,7 +56,9 @@ export class ActivatableLoggerImpl implements ActivatableLogger { loggingFunction(logStr, ...args); }, debug(...args: any[]): void { - this.log('debug', ...args); + if (debug) { + this.log('debug', ...args); + } }, info(...args: any[]): void { this.log('info', ...args); @@ -66,7 +70,9 @@ export class ActivatableLoggerImpl implements ActivatableLogger { this.log('error', ...args); }, profile(...args: any[]): void { - this.log('profile', ...args); + if (profile) { + this.log('profile', ...args); + } }, } } else { diff --git a/server/main.ts b/server/main.ts index dbd3884..e5c2121 100755 --- a/server/main.ts +++ b/server/main.ts @@ -8,7 +8,7 @@ import * as app from "./app" import * as mail from "./mail"; app.init(); -Logger.init(config.server.logging.enabled); +Logger.init(config.server.logging); Logger.tag('main', 'startup').info('Server starting up...'); async function main() { From 92c61c57c1868f1bd3d7295503af4589b645d5c0 Mon Sep 17 00:00:00 2001 From: baldo Date: Thu, 28 Jul 2022 15:08:41 +0200 Subject: [PATCH 13/13] Fix test --- server/services/monitoringService.test.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/server/services/monitoringService.test.ts b/server/services/monitoringService.test.ts index 92486fe..81769f4 100644 --- a/server/services/monitoringService.test.ts +++ b/server/services/monitoringService.test.ts @@ -1,4 +1,3 @@ -import moment from 'moment'; import {ParsedNode, parseNode, parseNodesJson} from "./monitoringService"; import {Domain, MAC, OnlineState, Site, UnixTimestampSeconds} from "../types"; import Logger from '../logger'; @@ -205,8 +204,8 @@ test('parseNode() should succeed parsing node without site and domain', () => { importTimestamp: importTimestamp, state: OnlineState.ONLINE, lastSeen: TIMESTAMP_VALID, - site: "" as Site, - domain: "" as Domain, + site: undefined, + domain: undefined, }; expect(parseNode(importTimestamp, nodeData)).toEqual(expectedParsedNode); });