etapi improvements and more tests

src/app.js

@@ -3,7 +3,6 @@ const express = require('express');
 const path = require('path');
 const favicon = require('serve-favicon');
 const cookieParser = require('cookie-parser');
-const bodyParser = require('body-parser');
 const helmet = require('helmet');
 const session = require('express-session');
 const FileStore = require('session-file-store')(session);
@@ -20,13 +19,13 @@ app.set('views', path.join(__dirname, 'views'));
 app.set('view engine', 'ejs');
 
 app.use(helmet({
-    hidePoweredBy: false, // deactivated because electron 4.0 crashes on this right after startup
+    hidePoweredBy: false, // errors out in electron
     contentSecurityPolicy: false
 }));
 
-app.use(bodyParser.text({limit: '500mb'}));
-app.use(bodyParser.json({limit: '500mb'}));
-app.use(bodyParser.urlencoded({extended: false}));
+app.use(express.text({limit: '500mb'}));
+app.use(express.json({limit: '500mb'}));
+app.use(express.urlencoded({extended: false}));
 app.use(cookieParser());
 app.use(express.static(path.join(__dirname, 'public')));
 app.use('/libraries', express.static(path.join(__dirname, '..', 'libraries')));

src/becca/becca.js

@@ -2,7 +2,6 @@
 
 const sql = require("../services/sql");
 const NoteSet = require("../services/search/note_set");
-const EtapiToken = require("./entities/etapi_token");
 
 /**
  * Becca is a backend cache of all notes, branches and attributes. There's a similar frontend cache Froca.

src/becca/entities/attribute.js

@@ -51,7 +51,7 @@ class Attribute extends AbstractEntity {
         /** @type {int} */
         this.position = position;
         /** @type {string} */
-        this.value = value;
+        this.value = value || "";
         /** @type {boolean} */
         this.isInheritable = !!isInheritable;
         /** @type {string} */

src/becca/entities/etapi_token.js

@@ -9,6 +9,9 @@ const sql = require("../../services/sql.js");
  * Used by:
  * - Trilium Sender
  * - ETAPI clients
+ * 
+ * The format user is presented with is "<etapiTokenId>_<tokenHash>". This is also called "authToken" to distinguish it
+ * from tokenHash and token.
  */
 class EtapiToken extends AbstractEntity {
     static get entityName() { return "etapi_tokens"; }

src/etapi/attributes.js

@@ -2,7 +2,7 @@ const becca = require("../becca/becca");
 const eu = require("./etapi_utils");
 const mappers = require("./mappers");
 const attributeService = require("../services/attributes");
-const validators = require("./validators");
+const v = require("./validators");
 
 function register(router) {
     eu.route(router, 'get', '/etapi/attributes/:attributeId', (req, res, next) => {
@@ -11,18 +11,23 @@ function register(router) {
         res.json(mappers.mapAttributeToPojo(attribute));
     });
 
+    const ALLOWED_PROPERTIES_FOR_CREATE_ATTRIBUTE = {
+        'attributeId': [v.mandatory, v.notNull, v.isValidEntityId],
+        'noteId': [v.mandatory, v.notNull, v.isNoteId],
+        'type': [v.mandatory, v.notNull, v.isAttributeType],
+        'name': [v.mandatory, v.notNull, v.isString],
+        'value': [v.notNull, v.isString],
+        'isInheritable': [v.notNull, v.isBoolean]
+    };
+    
     eu.route(router, 'post' ,'/etapi/attributes', (req, res, next) => {
-        const params = req.body;
-
-        eu.getAndCheckNote(params.noteId);
-
-        if (params.type === 'relation') {
-            eu.getAndCheckNote(params.value);
-        }
-
-        if (params.type !== 'relation' && params.type !== 'label') {
-            throw new eu.EtapiError(400, eu.GENERIC_CODE, `Only "relation" and "label" are supported attribute types, "${params.type}" given.`);
+        if (req.body.type === 'relation') {
+            eu.getAndCheckNote(req.body.value);
         }
+        
+        const params = {};
+        
+        eu.validateAndPatch(params, req.body, ALLOWED_PROPERTIES_FOR_CREATE_ATTRIBUTE);
 
         try {
             const attr = attributeService.createAttribute(params);
@@ -30,18 +35,24 @@ function register(router) {
             res.json(mappers.mapAttributeToPojo(attr));
         }
         catch (e) {
-            throw new eu.EtapiError(400, eu.GENERIC_CODE, e.message);
+            throw new eu.EtapiError(500, eu.GENERIC_CODE, e.message);
         }
     });
 
     const ALLOWED_PROPERTIES_FOR_PATCH = {
-        'value': validators.isString
+        'value': [v.notNull, v.isString]
     };
 
     eu.route(router, 'patch' ,'/etapi/attributes/:attributeId', (req, res, next) => {
         const attribute = eu.getAndCheckAttribute(req.params.attributeId);
 
+        if (attribute.type === 'relation') {
+            eu.getAndCheckNote(req.body.value);
+        }
+        
         eu.validateAndPatch(attribute, req.body, ALLOWED_PROPERTIES_FOR_PATCH);
+        
+        attribute.save();
 
         res.json(mappers.mapAttributeToPojo(attribute));
     });

src/etapi/branches.js

@@ -5,7 +5,7 @@ const Branch = require("../becca/entities/branch");
 const noteService = require("../services/notes");
 const TaskContext = require("../services/task_context");
 const entityChangesService = require("../services/entity_changes");
-const validators = require("./validators");
+const v = require("./validators");
 
 function register(router) {
     eu.route(router, 'get', '/etapi/branches/:branchId', (req, res, next) => {
@@ -14,11 +14,19 @@ function register(router) {
         res.json(mappers.mapBranchToPojo(branch));
     });
 
+    const ALLOWED_PROPERTIES_FOR_CREATE_BRANCH = {
+        'branchId': [v.mandatory, v.notNull, v.isValidEntityId],
+        'noteId': [v.mandatory, v.notNull, v.isNoteId],
+        'parentNoteId': [v.mandatory, v.notNull, v.isNoteId],
+        'notePosition': [v.notNull, v.isInteger],
+        'prefix': [v.isString],
+        'isExpanded': [v.notNull, v.isBoolean]
+    };
+    
     eu.route(router, 'post' ,'/etapi/branches', (req, res, next) => {
-        const params = req.body;
-
-        eu.getAndCheckNote(params.noteId);
-        eu.getAndCheckNote(params.parentNoteId);
+        const params = {};
+        
+        eu.validateAndPatch(params, req.body, ALLOWED_PROPERTIES_FOR_CREATE_BRANCH);
 
         const existing = becca.getBranchFromChildAndParent(params.noteId, params.parentNoteId);
 
@@ -41,15 +49,16 @@ function register(router) {
     });
 
     const ALLOWED_PROPERTIES_FOR_PATCH = {
-        'notePosition': validators.isInteger,
-        'prefix': validators.isStringOrNull,
-        'isExpanded': validators.isBoolean
+        'notePosition': [v.notNull, v.isInteger],
+        'prefix': [v.isString],
+        'isExpanded': [v.notNull, v.isBoolean]
     };
 
     eu.route(router, 'patch' ,'/etapi/branches/:branchId', (req, res, next) => {
         const branch = eu.getAndCheckBranch(req.params.branchId);
 
         eu.validateAndPatch(branch, req.body, ALLOWED_PROPERTIES_FOR_PATCH);
+        branch.save();
 
         res.json(mappers.mapBranchToPojo(branch));
     });

src/etapi/etapi.openapi.yaml

@@ -591,13 +591,15 @@ components:
       type: object
       required:
         - parentNoteId
-        - type
         - title
+        - type
         - content
       properties:
         parentNoteId:
           $ref: '#/components/schemas/EntityId'
           description: Note ID of the parent note in the tree
+        title:
+          type: string
         type:
           type: string
           enum: 
@@ -613,8 +615,6 @@ components:
           type: string
           description: this needs to be specified only for note types 'code', 'file', 'image'.
           example: application/json
-        title:
-          type: string
         content:
           type: string
         notePosition:
@@ -628,6 +628,9 @@ components:
             Prefix is branch (placement) specific title prefix for the note. 
             Let's say you have your note placed into two different places in the tree, 
             but you want to change the title a bit in one of the placements. For this you can use prefix.
+        isExpanded:
+          type: boolean
+          description: true if this note (as a folder) should appear expanded
         noteId:
           $ref: '#/components/schemas/EntityId'
           description: DON'T specify unless you want to force a specific noteId
@@ -644,7 +647,7 @@ components:
           type: string
         type:
           type: string
-          enum: [text, code, book, image, file, mermaid, relation-map, render, search, note-map]
+          enum: [text, code, render, file, image, search, relation-map, book, note-map, mermaid]
         mime:
           type: string
         isProtected:
@@ -686,7 +689,6 @@ components:
       properties:
         branchId:
           $ref: '#/components/schemas/EntityId'
-          readOnly: true
         noteId:
           $ref: '#/components/schemas/EntityId'
           readOnly: true
@@ -700,7 +702,7 @@ components:
         notePosition:
           type: integer
           format: int32
-        isExanded:
+        isExpanded:
           type: boolean
         utcDateModified:
           $ref: '#/components/schemas/UtcDateTime'
@@ -713,7 +715,6 @@ components:
       properties:
         attributeId:
           $ref: '#/components/schemas/EntityId'
-          readOnly: true
         noteId:
           $ref: '#/components/schemas/EntityId'
           readOnly: true
@@ -753,7 +754,7 @@ components:
           description: debugging info on parsing the search query enabled with &debug=true parameter
     EntityId:
       type: string
-      pattern: '[a-zA-Z0-9]{4,12}'
+      pattern: '[a-zA-Z0-9]{4,32}'
       example: evnnmvHTCgIn
     EntityIdList:
       type: array

src/etapi/etapi_utils.js

@@ -103,27 +103,26 @@ function getAndCheckAttribute(attributeId) {
     }
 }
 
-function validateAndPatch(entity, props, allowedProperties) {
-    for (const key of Object.keys(props)) {
+function validateAndPatch(target, source, allowedProperties) {
+    for (const key of Object.keys(source)) {
         if (!(key in allowedProperties)) {
-            throw new EtapiError(400, "PROPERTY_NOT_ALLOWED_FOR_PATCH", `Property '${key}' is not allowed for PATCH.`);
+            throw new EtapiError(400, "PROPERTY_NOT_ALLOWED", `Property '${key}' is not allowed for PATCH.`);
         }
         else {
-            const validator = allowedProperties[key];
-            const validationResult = validator(props[key]);
-            
-            if (validationResult) {
-                throw new EtapiError(400, "PROPERTY_VALIDATION_ERROR", `Validation failed on property '${key}': ${validationResult}`);
+            for (const validator of allowedProperties[key]) {
+                const validationResult = validator(source[key]);
+
+                if (validationResult) {
+                    throw new EtapiError(400, "PROPERTY_VALIDATION_ERROR", `Validation failed on property '${key}': ${validationResult}`);
+                }
             }
         }
     }
     
     // validation passed, let's patch
-    for (const propName of Object.keys(props)) {
-        entity[propName] = props[propName];
+    for (const propName of Object.keys(source)) {
+        target[propName] = source[propName];
     }
-    
-    entity.save();
 }
 
 module.exports = {

src/etapi/notes.js

@@ -4,7 +4,7 @@ const eu = require("./etapi_utils");
 const mappers = require("./mappers");
 const noteService = require("../services/notes");
 const TaskContext = require("../services/task_context");
-const validators = require("./validators");
+const v = require("./validators");
 const searchService = require("../services/search/services/search");
 const SearchContext = require("../services/search/search_context");
 
@@ -39,10 +39,23 @@ function register(router) {
         res.json(mappers.mapNoteToPojo(note));
     });
 
+    const ALLOWED_PROPERTIES_FOR_CREATE_NOTE = {
+        'parentNoteId': [v.mandatory, v.notNull, v.isNoteId],
+        'title': [v.mandatory, v.notNull, v.isString],
+        'type': [v.mandatory, v.notNull, v.isNoteType],
+        'mime': [v.notNull, v.isString],
+        'content': [v.notNull, v.isString],
+        'notePosition': [v.notNull, v.isInteger],
+        'prefix': [v.notNull, v.isInteger],
+        'isExpanded': [v.notNull, v.isBoolean],
+        'noteId': [v.notNull, v.isValidEntityId],
+        'branchId': [v.notNull, v.isValidEntityId],
+    };
+    
     eu.route(router, 'post' ,'/etapi/create-note', (req, res, next) => {
-        const params = req.body;
-
-        eu.getAndCheckNote(params.parentNoteId);
+        const params = {};
+        
+        eu.validateAndPatch(params, req.body, ALLOWED_PROPERTIES_FOR_CREATE_NOTE);
 
         try {
             const resp = noteService.createNewNote(params);
@@ -53,14 +66,14 @@ function register(router) {
             });
         }
         catch (e) {
-            return eu.sendError(res, 400, eu.GENERIC_CODE, e.message);
+            return eu.sendError(res, 500, eu.GENERIC_CODE, e.message);
         }
     });
 
     const ALLOWED_PROPERTIES_FOR_PATCH = {
-        'title': validators.isString,
-        'type': validators.isString,
-        'mime': validators.isString
+        'title': [v.notNull, v.isString],
+        'type': [v.notNull, v.isString],
+        'mime': [v.notNull, v.isString]
     };
 
     eu.route(router, 'patch' ,'/etapi/notes/:noteId', (req, res, next) => {
@@ -71,6 +84,7 @@ function register(router) {
         }
 
         eu.validateAndPatch(note, req.body, ALLOWED_PROPERTIES_FOR_PATCH);
+        note.save();
 
         res.json(mappers.mapNoteToPojo(note));
     });

src/etapi/validators.js

@@ -1,30 +1,101 @@
+const noteTypes = require("../services/note_types");
+
+function mandatory(obj) {
+    if (obj === undefined ) {
+        return `mandatory, but not set`;
+    }
+}
+
+function notNull(obj) {
+    if (obj === null) {
+        return `cannot be null`;
+    }
+}
+
 function isString(obj) {
+    if (obj === undefined || obj === null) {
+        return;
+    }
+    
     if (typeof obj !== 'string') {
         return `'${obj}' is not a string`;
     }
 }
 
-function isStringOrNull(obj) {
-    if (obj) {
-        return isString(obj);
-    }
-}
-
 function isBoolean(obj) {
+    if (obj === undefined || obj === null) {
+        return;
+    }
+    
     if (typeof obj !== 'boolean') {
         return `'${obj}' is not a boolean`;
     }
 }
 
 function isInteger(obj) {
+    if (obj === undefined || obj === null) {
+        return;
+    }
+    
     if (!Number.isInteger(obj)) {
         return `'${obj}' is not an integer`;
     }
 }
 
+function isNoteId(obj) {
+    if (obj === undefined || obj === null) {
+        return;
+    }
+    
+    const becca = require('../becca/becca');
+    
+    if (typeof obj !== 'string') {
+        return `'${obj}' is not a valid noteId`;
+    }
+    
+    if (!(obj in becca.notes)) {
+        return `Note '${obj}' does not exist`;
+    }
+}
+
+function isNoteType(obj) {
+    if (obj === undefined || obj === null) {
+        return;
+    }
+
+    if (!noteTypes.includes(obj)) {
+        return `'${obj}' is not a valid note type, allowed types are: ` + noteTypes.join(", ");
+    }
+}
+
+function isAttributeType(obj) {
+    if (obj === undefined || obj === null) {
+        return;
+    }
+
+    if (!['label', 'relation'].includes(obj)) {
+        return `'${obj}' is not a valid attribute type, allowed types are: label, relation`;
+    }
+}
+
+function isValidEntityId(obj) {
+    if (obj === undefined || obj === null) {
+        return;
+    }
+    
+    if (typeof obj !== 'string' || !/^[A-Za-z0-9]{4,32}$/.test(obj)) {
+        return `'${obj}' is not a valid entityId. Only alphanumeric characters are allowed of length 4 to 32.`;
+    }
+}
+
 module.exports = {
+    mandatory,
+    notNull,
     isString,
-    isStringOrNull,
     isBoolean,
-    isInteger
+    isInteger,
+    isNoteId,
+    isNoteType,
+    isAttributeType,
+    isValidEntityId
 };

src/public/app/services/load_results.js

@@ -91,7 +91,7 @@ export default class LoadResults {
         }
 
         const componentIds = this.noteIdToComponentId[noteId];
-        return componentIds && !!componentIds.find(sId => sId !== componentId);
+        return componentIds && componentIds.find(sId => sId !== componentId) !== undefined;
     }
 
     addNoteContent(noteId, componentId) {

src/routes/routes.js

@@ -190,7 +190,7 @@ const uploadMiddleware = multer.single('upload');
 function register(app) {
     route(GET, '/', [auth.checkAuth, csrfMiddleware], indexRoute.index);
     route(GET, '/login', [auth.checkAppInitialized, auth.checkPasswordSet], loginRoute.loginPage);
-    route(GET, '/set-password', [auth.checkAppInitialized], loginRoute.setPasswordPage);
+    route(GET, '/set-password', [auth.checkAppInitialized, auth.checkPasswordNotSet], loginRoute.setPasswordPage);
 
     const loginRateLimiter = rateLimit({
         windowMs: 15 * 60 * 1000, // 15 minutes
@@ -199,7 +199,7 @@ function register(app) {
 
     route(POST, '/login', [loginRateLimiter], loginRoute.login);
     route(POST, '/logout', [csrfMiddleware, auth.checkAuth], loginRoute.logout);
-    route(POST, '/set-password', [auth.checkAppInitialized], loginRoute.setPassword);
+    route(POST, '/set-password', [auth.checkAppInitialized, auth.checkPasswordNotSet], loginRoute.setPassword);
     route(GET, '/setup', [], setupRoute.setupPage);
 
     apiRoute(GET, '/api/tree', treeApiRoute.getTree);

src/routes/setup.js

@@ -14,6 +14,8 @@ function setupPage(req, res) {
         else {
             res.redirect('/');
         }
+
+        return;
     }
 
     // we got here because DB is not completely initialized so if schema exists

src/services/auth.js

@@ -15,11 +15,7 @@ function checkAuth(req, res, next) {
         res.redirect("setup");
     }
     else if (!req.session.loggedIn && !utils.isElectron() && !noAuthentication) {
-        if (passwordService.isPasswordSet()) {
-            res.redirect("login");
-        } else {
-            res.redirect("set-password");
-        }
+        res.redirect("login");
     }
     else {
         next();
@@ -63,6 +59,14 @@ function checkPasswordSet(req, res, next) {
     }
 }
 
+function checkPasswordNotSet(req, res, next) {
+    if (!utils.isElectron() && passwordService.isPasswordSet()) {
+        res.redirect("login");
+    } else {
+        next();
+    }
+}
+
 function checkAppNotInitialized(req, res, next) {
     if (sqlInit.isDbInitialized()) {
         reject(req, res, "App already initialized.");
@@ -111,6 +115,7 @@ module.exports = {
     checkApiAuth,
     checkAppInitialized,
     checkPasswordSet,
+    checkPasswordNotSet,
     checkAppNotInitialized,
     checkApiAuthOrElectron,
     checkEtapiToken,

src/services/consistency_checks.js

@@ -13,6 +13,7 @@ const attributeService = require('./attributes');
 const noteRevisionService = require('./note_revisions');
 const becca = require("../becca/becca");
 const utils = require("../services/utils");
+const noteTypes = require("../services/note_types");
 
 class ConsistencyChecks {
     constructor(autoFix) {
@@ -281,11 +282,13 @@ class ConsistencyChecks {
     }
 
     findLogicIssues() {
+        const noteTypesStr = noteTypes.map(nt => `'${nt}'`).join(", ");
+        
         this.findAndFixIssues(`
                     SELECT noteId, type
                     FROM notes
                     WHERE isDeleted = 0
-                      AND type NOT IN ('text', 'code', 'render', 'file', 'image', 'search', 'relation-map', 'book', 'note-map', 'mermaid')`,
+                      AND type NOT IN (${noteTypesStr})`,
             ({noteId, type}) => {
                 if (this.autoFix) {
                     const note = becca.getNote(noteId);

src/services/entity_changes.js

@@ -23,7 +23,7 @@ function addEntityChange(origEntityChange) {
         ec.changeId = utils.randomString(12);
     }
 
-    ec.componentId = ec.componentId || cls.getComponentId() || "";
+    ec.componentId = ec.componentId || cls.getComponentId() || "NA"; // NA = not available
     ec.instanceId = ec.instanceId || instanceId;
     ec.isSynced = ec.isSynced ? 1 : 0;
     ec.isErased = ec.isErased ? 1 : 0;
@@ -43,7 +43,7 @@ function addNoteReorderingEntityChange(parentNoteId, componentId) {
         utcDateChanged: dateUtils.utcNowDateTime(),
         isSynced: true,
         componentId,
-        instanceId: instanceId
+        instanceId
     });
 
     const eventService = require('./events');

src/services/etapi_tokens.js

@@ -12,7 +12,7 @@ function getTokenHash(token) {
 }
 
 function createToken(tokenName) {
-    const token = utils.randomSecureToken();
+    const token = utils.randomSecureToken(32);
     const tokenHash = getTokenHash(token);
 
     const etapiToken = new EtapiToken({

src/services/notes.js

@@ -18,8 +18,6 @@ const Branch = require('../becca/entities/branch');
 const Note = require('../becca/entities/note');
 const Attribute = require('../becca/entities/attribute');
 
-// TODO: patch/put note content
-
 function getNewNotePosition(parentNoteId) {
     const note = becca.notes[parentNoteId];
 

src/services/options.js

@@ -1,22 +1,27 @@
 const becca = require('../becca/becca');
 const sql = require("./sql");
 
-function getOption(name) {
+function getOptionOrNull(name) {
     let option;
 
     if (becca.loaded) {
         option = becca.getOption(name);
-    }
-    else {
+    } else {
         // e.g. in initial sync becca is not loaded because DB is not initialized
         option = sql.getRow("SELECT * FROM options WHERE name = ?", name);
     }
+    
+    return option ? option.value : null;
+}
 
-    if (!option) {
+function getOption(name) {
+    const val = getOptionOrNull(name);
+
+    if (val === null) {
         throw new Error(`Option "${name}" doesn't exist`);
     }
 
-    return option.value;
+    return val;
 }
 
 /**
@@ -96,5 +101,6 @@ module.exports = {
     setOption,
     createOption,
     getOptions,
-    getOptionsMap
+    getOptionsMap,
+    getOptionOrNull
 };

src/services/password_encryption.js

@@ -6,8 +6,12 @@ const dataEncryptionService = require('./data_encryption');
 function verifyPassword(password) {
     const givenPasswordHash = utils.toBase64(myScryptService.getVerificationHash(password));
 
-    const dbPasswordHash = optionService.getOption('passwordVerificationHash');
+    const dbPasswordHash = optionService.getOptionOrNull('passwordVerificationHash');
 
+    if (!dbPasswordHash) {
+        return false;
+    }
+    
     return givenPasswordHash === dbPasswordHash;
 }
 

src/services/tree.js

@@ -154,10 +154,6 @@ function sortNotes(parentNoteId, customSortBy = 'title', reverse = false, folder
             const topAEl = fetchValue(a, 'top');
             const topBEl = fetchValue(b, 'top');
 
-            console.log(a.title, topAEl);
-            console.log(b.title, topBEl);
-            console.log("comp", compare(topAEl, topBEl) && !reverse);
-
             if (topAEl !== topBEl) {
                 // since "top" should not be reversible, we'll reverse it once more to nullify this effect
                 return compare(topAEl, topBEl) * (reverse ? -1 : 1);