Desktop: Fixes #5080: Ensure resources are decrypted when sharing a notebook with Joplin Server

.eslintignore

@@ -980,6 +980,9 @@ packages/lib/models/dateTimeFormats.test.js.map
 packages/lib/models/settings/FileHandler.d.ts
 packages/lib/models/settings/FileHandler.js
 packages/lib/models/settings/FileHandler.js.map
+packages/lib/models/utils/itemCanBeEncrypted.d.ts
+packages/lib/models/utils/itemCanBeEncrypted.js
+packages/lib/models/utils/itemCanBeEncrypted.js.map
 packages/lib/models/utils/paginatedFeed.d.ts
 packages/lib/models/utils/paginatedFeed.js
 packages/lib/models/utils/paginatedFeed.js.map

.gitignore

@@ -966,6 +966,9 @@ packages/lib/models/dateTimeFormats.test.js.map
 packages/lib/models/settings/FileHandler.d.ts
 packages/lib/models/settings/FileHandler.js
 packages/lib/models/settings/FileHandler.js.map
+packages/lib/models/utils/itemCanBeEncrypted.d.ts
+packages/lib/models/utils/itemCanBeEncrypted.js
+packages/lib/models/utils/itemCanBeEncrypted.js.map
 packages/lib/models/utils/paginatedFeed.d.ts
 packages/lib/models/utils/paginatedFeed.js
 packages/lib/models/utils/paginatedFeed.js.map

packages/app-desktop/runForSharing.sh

@@ -1,48 +0,0 @@
-#!/bin/bash
-
-# Setup the sync parameters for user X and create a few folders and notes to
-# allow sharing. Also calls the API to create the test users and clear the data.
-
-set -e
-
-SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
-ROOT_DIR="$SCRIPT_DIR/../.."
-
-if [ "$1" == "" ]; then
-	echo "User number is required"
-	exit 1
-fi
-
-USER_NUM=$1
-RESET_ALL=$2
-PROFILE_DIR=~/.config/joplindev-desktop-$USER_NUM
-
-if [ "$RESET_ALL" == "1" ]; then
-	CMD_FILE="$SCRIPT_DIR/runForSharingCommands-$USER_NUM.txt"
-	rm -f "$CMD_FILE"
-
-	USER_EMAIL="user$USER_NUM@example.com"
-	rm -rf "$PROFILE_DIR"
-
-	echo "config keychain.supported 0" >> "$CMD_FILE" 
-	echo "config sync.target 10" >> "$CMD_FILE" 
-	# echo "config sync.10.path http://api.joplincloud.local:22300" >> "$CMD_FILE" 
-	echo "config sync.10.username $USER_EMAIL" >> "$CMD_FILE" 
-	echo "config sync.10.password 123456" >> "$CMD_FILE" 
-
-	if [ "$USER_NUM" == "1" ]; then
-		curl --data '{"action": "createTestUsers"}' -H 'Content-Type: application/json' http://api.joplincloud.local:22300/api/debug
-
-		echo 'mkbook "shared"' >> "$CMD_FILE"
-		echo 'mkbook "other"' >> "$CMD_FILE"
-		echo 'use "shared"' >> "$CMD_FILE"
-		echo 'mknote "note 1"' >> "$CMD_FILE"
-		echo 'mknote "note 2"' >> "$CMD_FILE"
-	fi
-
-	cd "$ROOT_DIR/packages/app-cli"
-	npm start -- --profile "$PROFILE_DIR" batch "$CMD_FILE"
-fi
-
-cd "$ROOT_DIR/packages/app-desktop"
-npm start -- --env dev --profile "$PROFILE_DIR"

packages/app-desktop/runForTesting.sh

@@ -0,0 +1,68 @@
+#!/bin/bash
+
+# Setup the sync parameters for user X and create a few folders and notes to
+# allow sharing. Also calls the API to create the test users and clear the data.
+
+set -e
+
+SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
+ROOT_DIR="$SCRIPT_DIR/../.."
+
+if [ "$1" == "" ]; then
+	echo "User number is required"
+	exit 1
+fi
+
+USER_NUM=$1
+COMMANDS=($(echo $2 | tr "," "\n"))
+PROFILE_DIR=~/.config/joplindev-desktop-$USER_NUM
+
+CMD_FILE="$SCRIPT_DIR/runForSharingCommands-$USER_NUM.txt"
+rm -f "$CMD_FILE"
+touch "$CMD_FILE"
+
+for CMD in "${COMMANDS[@]}"
+do
+    if [[ $CMD == "createUsers" ]]; then
+
+		curl --data '{"action": "createTestUsers"}' -H 'Content-Type: application/json' http://api.joplincloud.local:22300/api/debug
+
+	elif [[ $CMD == "createData" ]]; then
+		
+		echo 'mkbook "shared"' >> "$CMD_FILE"
+		echo 'mkbook "other"' >> "$CMD_FILE"
+		echo 'use "shared"' >> "$CMD_FILE"
+		echo 'mknote "note 1"' >> "$CMD_FILE"
+		echo 'mknote "note 2"' >> "$CMD_FILE"
+	
+	elif [[ $CMD == "reset" ]]; then
+	
+		USER_EMAIL="user$USER_NUM@example.com"
+		rm -rf "$PROFILE_DIR"
+		echo "config keychain.supported 0" >> "$CMD_FILE" 
+		echo "config sync.target 10" >> "$CMD_FILE" 
+		# echo "config sync.10.path http://api.joplincloud.local:22300" >> "$CMD_FILE" 
+		echo "config sync.10.username $USER_EMAIL" >> "$CMD_FILE" 
+		echo "config sync.10.password 123456" >> "$CMD_FILE" 
+	
+	elif [[ $CMD == "e2ee" ]]; then
+	
+		echo "e2ee enable --password 111111" >> "$CMD_FILE" 
+	
+	else
+	
+		echo "Unknown command: $CMD"
+		exit 1
+	
+	fi
+done
+
+cd "$ROOT_DIR/packages/app-cli"
+npm start -- --profile "$PROFILE_DIR" batch "$CMD_FILE"
+
+if [[ $COMMANDS != "" ]]; then
+	exit 0
+fi
+
+cd "$ROOT_DIR/packages/app-desktop"
+npm start -- --env dev --profile "$PROFILE_DIR"

packages/lib/models/BaseItem.ts

@@ -1,26 +1,18 @@
 import { ModelType } from '../BaseModel';
-import { NoteEntity } from '../services/database/types';
+import { BaseItemEntity, NoteEntity } from '../services/database/types';
 import Setting from './Setting';
 import BaseModel from '../BaseModel';
 import time from '../time';
 import markdownUtils from '../markdownUtils';
 import { _ } from '../locale';
-
 import Database from '../database';
 import ItemChange from './ItemChange';
 import ShareService from '../services/share/ShareService';
+import itemCanBeEncrypted from './utils/itemCanBeEncrypted';
 const JoplinError = require('../JoplinError.js');
 const { sprintf } = require('sprintf-js');
 const moment = require('moment');
 
-export interface BaseItemEntity {
-	id?: string;
-	encryption_applied?: boolean;
-	is_shared?: number;
-	share_id?: string;
-	type_?: ModelType;
-}
-
 export interface ItemsThatNeedDecryptionResult {
 	hasMore: boolean;
 	items: any[];
@@ -404,7 +396,7 @@ export default class BaseItem extends BaseModel {
 
 		const serialized = await ItemClass.serialize(item, shownKeys);
 
-		if (!Setting.value('encryption.enabled') || !ItemClass.encryptionSupported() || item.is_shared || item.share_id) {
+		if (!Setting.value('encryption.enabled') || !ItemClass.encryptionSupported() || !itemCanBeEncrypted(item)) {
 			// Normally not possible since itemsThatNeedSync should only return decrypted items
 			if (item.encryption_applied) throw new JoplinError('Item is encrypted but encryption is currently disabled', 'cannotSyncEncrypted');
 			return serialized;

packages/lib/models/Note.ts

@@ -127,7 +127,7 @@ export default class Note extends BaseItem {
 
 	static async linkedItemIdsByType(type: ModelType, body: string) {
 		const items = await this.linkedItems(body);
-		const output = [];
+		const output: string[] = [];
 
 		for (let i = 0; i < items.length; i++) {
 			const item = items[i];

packages/lib/models/Resource.ts

@@ -12,6 +12,7 @@ const { mime } = require('../mime-utils.js');
 const { filename, safeFilename } = require('../path-utils');
 const { FsDriverDummy } = require('../fs-driver-dummy.js');
 import JoplinError from '../JoplinError';
+import itemCanBeEncrypted from './utils/itemCanBeEncrypted';
 
 export default class Resource extends BaseItem {
 
@@ -192,10 +193,10 @@ export default class Resource extends BaseItem {
 	// as it should be uploaded to the sync target. Note that this may be different from what is stored
 	// in the database. In particular, the flag encryption_blob_encrypted might be 1 on the sync target
 	// if the resource is encrypted, but will be 0 locally because the device has the decrypted resource.
-	static async fullPathForSyncUpload(resource: ResourceEntity) {
+	public static async fullPathForSyncUpload(resource: ResourceEntity) {
 		const plainTextPath = this.fullPath(resource);
 
-		if (!Setting.value('encryption.enabled')) {
+		if (!Setting.value('encryption.enabled') || !itemCanBeEncrypted(resource as any)) {
 			// Normally not possible since itemsThatNeedSync should only return decrypted items
 			if (resource.encryption_blob_encrypted) throw new Error('Trying to access encrypted resource but encryption is currently disabled');
 			return { path: plainTextPath, resource: resource };

packages/lib/models/utils/itemCanBeEncrypted.ts

@@ -0,0 +1,5 @@
+import { BaseItemEntity } from '../../services/database/types';
+
+export default function(resource: BaseItemEntity): boolean {
+	return !resource.is_shared && !resource.share_id;
+}

packages/lib/services/database/types.ts

@@ -1,3 +1,17 @@
+import { ModelType } from "../../BaseModel";
+
+export interface BaseItemEntity {
+  id?: string;
+  encryption_applied?: boolean;
+  is_shared?: number;
+  share_id?: string;
+  type_?: ModelType;
+}
+
+
+
+
+
 // AUTO-GENERATED BY packages/tools/generate-database-types.js
 
 /*
@@ -29,8 +43,6 @@ export interface FolderEntity {
   "encryption_applied"?: number 
   "parent_id"?: string 
   "is_shared"?: number 
-  "is_linked_folder"?: number 
-  "source_folder_owner_id"?: string 
   "share_id"?: string 
   "type_"?: number 
 }
@@ -98,7 +110,6 @@ export interface NoteEntity {
   "created_time"?: number 
   "updated_time"?: number 
   "is_conflict"?: number 
-  "conflict_original_id"?: string
   "latitude"?: number 
   "longitude"?: number 
   "altitude"?: number 
@@ -118,6 +129,7 @@ export interface NoteEntity {
   "markup_language"?: number 
   "is_shared"?: number 
   "share_id"?: string 
+  "conflict_original_id"?: string 
   "type_"?: number 
 }
 export interface NotesNormalizedEntity {
@@ -227,6 +239,7 @@ export interface TagsWithNoteCountEntity {
   "created_time"?: number | null 
   "updated_time"?: number | null 
   "note_count"?: any | null 
+  "todo_completed_count"?: any | null 
   "type_"?: number 
 }
 export interface VersionEntity {

packages/lib/services/synchronizer/Synchronizer.e2ee.test.ts

@@ -8,9 +8,15 @@ import Resource from '../../models/Resource';
 import ResourceFetcher from '../../services/ResourceFetcher';
 import MasterKey from '../../models/MasterKey';
 import BaseItem from '../../models/BaseItem';
+import { ResourceEntity } from '../database/types';
+import Synchronizer from '../../Synchronizer';
 
 let insideBeforeEach = false;
 
+function newResourceFetcher(synchronizer: Synchronizer) {
+	return new ResourceFetcher(() => { return synchronizer.api(); });
+}
+
 describe('Synchronizer.e2ee', function() {
 
 	beforeEach(async (done) => {
@@ -223,7 +229,7 @@ describe('Synchronizer.e2ee', function() {
 		Setting.setObjectValue('encryption.passwordCache', masterKey.id, '123456');
 		await encryptionService().loadMasterKeysFromSettings();
 
-		const fetcher = new ResourceFetcher(() => { return synchronizer().api(); });
+		const fetcher = newResourceFetcher(synchronizer());
 		fetcher.queueDownload_(resource1.id);
 		await fetcher.waitForAllFinished();
 		await decryptionWorker().start();
@@ -287,7 +293,7 @@ describe('Synchronizer.e2ee', function() {
 		expect(!!resource.encryption_applied).toBe(false);
 		expect(!!resource.encryption_blob_encrypted).toBe(true);
 
-		const resourceFetcher = new ResourceFetcher(() => { return synchronizer().api(); });
+		const resourceFetcher = newResourceFetcher(synchronizer());
 		await resourceFetcher.start();
 		await resourceFetcher.waitForAllFinished();
 
@@ -378,8 +384,15 @@ describe('Synchronizer.e2ee', function() {
 			},
 		]);
 
-		const note1 = await Note.loadByTitle('un');
+		let note1 = await Note.loadByTitle('un');
 		let note2 = await Note.loadByTitle('deux');
+		await shim.attachFileToNote(note1, `${supportDir}/photo.jpg`);
+		await shim.attachFileToNote(note2, `${supportDir}/photo.jpg`);
+		note1 = await Note.loadByTitle('un');
+		note2 = await Note.loadByTitle('deux');
+		const resourceId1 = (await Note.linkedResourceIds(note1.body))[0];
+		const resourceId2 = (await Note.linkedResourceIds(note2.body))[0];
+
 		await synchronizerStart();
 
 		await switchClient(2);
@@ -405,11 +418,37 @@ describe('Synchronizer.e2ee', function() {
 		// The shared note should be decrypted
 		const note2_2 = await Note.load(note2.id);
 		expect(note2_2.title).toBe('deux');
+		expect(note2_2.encryption_applied).toBe(0);
 		expect(note2_2.is_shared).toBe(1);
 
+		// The resource linked to the shared note should also be decrypted
+		const resource2: ResourceEntity = await Resource.load(resourceId2);
+		expect(resource2.is_shared).toBe(1);
+		expect(resource2.encryption_applied).toBe(0);
+
+		const fetcher = newResourceFetcher(synchronizer());
+		await fetcher.start();
+		await fetcher.waitForAllFinished();
+
+		// Because the resource is decrypted, the encrypted blob file should not
+		// exist, but the plain text one should.
+		expect(await shim.fsDriver().exists(Resource.fullPath(resource2, true))).toBe(false);
+		expect(await shim.fsDriver().exists(Resource.fullPath(resource2))).toBe(true);
+
 		// The non-shared note should be encrypted
 		const note1_2 = await Note.load(note1.id);
 		expect(note1_2.title).toBe('');
+
+		// The linked resource should also be encrypted
+		const resource1: ResourceEntity = await Resource.load(resourceId1);
+		expect(resource1.is_shared).toBe(0);
+		expect(resource1.encryption_applied).toBe(1);
+
+		// And the plain text blob should not be present. The encrypted one
+		// shouldn't either because it can only be downloaded once the metadata
+		// has been decrypted.
+		expect(await shim.fsDriver().exists(Resource.fullPath(resource1, true))).toBe(false);
+		expect(await shim.fsDriver().exists(Resource.fullPath(resource1))).toBe(false);
 	}));
 
 	it('should not encrypt items that are shared by folder', (async () => {

packages/tools/generate-database-types.ts

@@ -47,7 +47,12 @@ async function main() {
 
 	const targetFile = `${rootDir}/packages/lib/services/database/types.ts`;
 	console.info(`Writing type definitions to ${targetFile}...`);
-	await fs.writeFile(targetFile, `${header}\n\n${tsString}`, 'utf8');
+
+	const existingContent = (await fs.pathExists(targetFile)) ? await fs.readFile(targetFile, 'utf8') : '';
+	const splitted = existingContent.split('// AUTO-GENERATED BY');
+	const staticContent = splitted[0];
+
+	await fs.writeFile(targetFile, `${staticContent}\n\n${header}\n\n${tsString}`, 'utf8');
 }
 
 main().catch((error) => {