Merge branch 'master' into delete-route-for-account-deletion

server/forcessl.mw.spec.js

@@ -0,0 +1,66 @@
+import forceSSL from './forcessl.mw';
+
+describe('Tests for ForceSSL middleware', ()=>{
+	let originalEnv;
+	let nextFn;
+
+	let req = {};
+	let res = {};
+
+	beforeEach(()=>{
+		originalEnv = process.env.NODE_ENV;
+		nextFn = jest.fn();
+
+		req = {
+			header : ()=>{ return 'http'; },
+			get    : ()=>{ return 'test'; },
+			url    : 'URL'
+		};
+
+		res = {
+			redirect : jest.fn()
+		};
+	});
+	afterEach(()=>{
+		process.env.NODE_ENV = originalEnv;
+		jest.clearAllMocks();
+	});
+
+	it('should not redirect when NODE_ENV is set to local', ()=>{
+		process.env.NODE_ENV = 'local';
+
+		forceSSL(null, null, nextFn);
+
+		expect(res.redirect).not.toHaveBeenCalled();
+		expect(nextFn).toHaveBeenCalled();
+	});
+
+	it('should not redirect when NODE_ENV is set to docker', ()=>{
+		process.env.NODE_ENV = 'docker';
+
+		forceSSL(null, null, nextFn);
+
+		expect(res.redirect).not.toHaveBeenCalled();
+		expect(nextFn).toHaveBeenCalled();
+	});
+
+	it('should redirect with 302 when header is not HTTPS and NODE_ENV is not local or docker', ()=>{
+		process.env.NODE_ENV = 'test';
+
+		forceSSL(req, res, nextFn);
+
+		expect(res.redirect).toHaveBeenCalledWith(302, 'https://testURL');
+		expect(nextFn).not.toHaveBeenCalled();
+	});
+
+	it('should not redirect when header is HTTPS and NODE_ENV is not local or docker', ()=>{
+		process.env.NODE_ENV = 'test';
+		req.header = ()=>{ return 'https'; };
+
+		forceSSL(req, res, nextFn);
+
+		expect(res.redirect).not.toHaveBeenCalled();
+		expect(nextFn).toHaveBeenCalled();
+	});
+
+});

server/homebrew.api.js

@@ -8,8 +8,10 @@ import Markdown                      from '../shared/naturalcrit/markdown.js';
 import yaml                          from 'js-yaml';
 import asyncHandler                  from 'express-async-handler';
 import { nanoid }                    from 'nanoid';
+import {makePatches, applyPatches, stringifyPatches, parsePatch} from '@sanity/diff-match-patch';
+import { md5 }                       from 'hash-wasm';
 import { splitTextStyleAndMetadata, 
-		 brewSnippetsToJSON }        from '../shared/helpers.js';
+		 brewSnippetsToJSON, debugTextMismatch }        from '../shared/helpers.js';
 import checkClientVersion            from './middleware/check-client-version.js';
 
 
@@ -46,6 +48,20 @@ const api = {
 			}
 			id = id.slice(googleId.length);
 		}
+
+		// ID Validation Checks
+		// Homebrewery ID
+		// Typically 12 characters, but the DB shows a range of 7 to 14 characters
+		if(!id.match(/^[A-Za-z0-9_-]{7,14}$/)){
+			throw { name: 'ID Error', message: 'Invalid ID', status: 404, HBErrorCode: '11', brewId: id };
+		}
+		// Google ID
+		// Typically 33 characters, old format is 44 - always starts with a 1
+		// Managed by Google, may change outside of our control, so any length between 33 and 44 is acceptable
+		if(googleId && !googleId.match(/^1(?:[A-Za-z0-9+\/]{32,43})$/)){
+			throw { name: 'Google ID Error', message: 'Invalid ID', status: 404, HBErrorCode: '12', brewId: id };
+		}
+
 		return { id, googleId };
 	},
 	//Get array of any of this user's brews tagged with `meta:theme`
@@ -337,21 +353,52 @@ const api = {
 		// Initialize brew from request and body, destructure query params, and set the initial value for the after-save method
 		const brewFromClient = api.excludePropsFromUpdate(req.body);
 		const brewFromServer = req.brew;
-		if(brewFromServer.version && brewFromClient.version && brewFromServer.version > brewFromClient.version) {
+		splitTextStyleAndMetadata(brewFromServer);
+
+		if(brewFromServer?.version !== brewFromClient?.version){
 			console.log(`Version mismatch on brew ${brewFromClient.editId}`);
+
 			res.setHeader('Content-Type', 'application/json');
-			return res.status(409).send(JSON.stringify({ message: `The brew has been changed on a different device. Please save your changes elsewhere, refresh, and try again.` }));
+			return res.status(409).send(JSON.stringify({ message: `The server version is out of sync with the saved brew. Please save your changes elsewhere, refresh, and try again.` }));
 		}
 
-		let brew = _.assign(brewFromServer, brewFromClient);
+		brewFromServer.text  = brewFromServer.text.normalize('NFC');
+		brewFromServer.hash  = await md5(brewFromServer.text);
+
+		if(brewFromServer?.hash !== brewFromClient?.hash) {
+			console.log(`Hash mismatch on brew ${brewFromClient.editId}`);
+			//debugTextMismatch(brewFromClient.text, brewFromServer.text, `edit/${brewFromClient.editId}`);
+			res.setHeader('Content-Type', 'application/json');
+			return res.status(409).send(JSON.stringify({ message: `The server copy is out of sync with the saved brew. Please save your changes elsewhere, refresh, and try again.` }));
+		}
+
+		try {
+			const patches = parsePatch(brewFromClient.patches);
+			// Patch to a throwaway variable while parallelizing - we're more concerned with error/no error.
+			const patchedResult = applyPatches(patches, brewFromServer.text, { allowExceedingIndices: true })[0];
+			if(patchedResult != brewFromClient.text)
+				throw("Patches did not apply cleanly, text mismatch detected");
+			// brew.text = applyPatches(patches, brewFromServer.text)[0];
+		} catch (err) {
+			//debugTextMismatch(brewFromClient.text, brewFromServer.text, `edit/${brewFromClient.editId}`);
+			console.error('Failed to apply patches:', {
+				patches : brewFromClient.patches,
+				brewId  : brewFromClient.editId || 'unknown',
+				error   : err
+			});
+			// While running in parallel, don't throw the error upstream.
+			// throw err; // rethrow to preserve the 500 behavior
+		}
+
+		let brew         = _.assign(brewFromServer, brewFromClient);
+		brew.title       = brew.title.trim();
+		brew.description = brew.description.trim() || '';
+		brew.text        = api.mergeBrewText(brew);
+
 		const googleId = brew.googleId;
 		const { saveToGoogle, removeFromGoogle } = req.query;
 		let afterSave = async ()=>true;
 
-		brew.title = brew.title.trim();
-		brew.description = brew.description.trim() || '';
-		brew.text = api.mergeBrewText(brew);
-
 		if(brew.googleId && removeFromGoogle) {
 			// If the google id exists and we're removing it from google, set afterSave to delete the google brew and mark the brew's google id as undefined
 			afterSave = async ()=>{
@@ -412,6 +459,8 @@ const api = {
 		const after = await afterSave();
 		if(!after) return;
 
+		saved.textBin = undefined; // Remove textBin from the saved object to save bandwidth
+
 		res.status(200).send(saved);
 	},
 	deleteGoogleBrew : async (account, id, editId, res)=>{
@@ -482,8 +531,8 @@ const api = {
 };
 
 router.post('/api', checkClientVersion, asyncHandler(api.newBrew));
-router.put('/api/:id', checkClientVersion, asyncHandler(api.getBrew('edit', true)), asyncHandler(api.updateBrew));
-router.put('/api/update/:id', checkClientVersion, asyncHandler(api.getBrew('edit', true)), asyncHandler(api.updateBrew));
+router.put('/api/:id', checkClientVersion, asyncHandler(api.getBrew('edit', false)), asyncHandler(api.updateBrew));
+router.put('/api/update/:id', checkClientVersion, asyncHandler(api.getBrew('edit', false)), asyncHandler(api.updateBrew));
 router.delete('/api/:id', checkClientVersion, asyncHandler(api.deleteBrew));
 router.get('/api/remove/:id', checkClientVersion, asyncHandler(api.deleteBrew));
 router.get('/api/theme/:renderer/:id', asyncHandler(api.getThemeBundle));

server/homebrew.api.spec.js

@@ -99,18 +99,87 @@ describe('Tests for api', ()=>{
 			expect(googleId).toBeUndefined();
 		});
 
+		it('should throw if id is too short', ()=>{
+			let err;
+			try {
+				api.getId({
+					params : {
+						id : 'abcd'
+					}
+				});
+			} catch (e) {
+				err = e;
+			};
+
+			expect(err).toEqual({ HBErrorCode: '11', brewId: 'abcd', message: 'Invalid ID', name: 'ID Error', status: 404 });
+		});
+
 		it('should return id and google id from request body', ()=>{
 			const { id, googleId } = api.getId({
 				params : {
-					id : 'abcdefgh'
+					id : 'abcdefghijkl'
 				},
 				body : {
-					googleId : '12345'
+					googleId : '123456789012345678901234567890123'
 				}
 			});
 
-			expect(id).toEqual('abcdefgh');
-			expect(googleId).toEqual('12345');
+			expect(id).toEqual('abcdefghijkl');
+			expect(googleId).toEqual('123456789012345678901234567890123');
+		});
+
+		it('should throw invalid - google id right length but does not match pattern', ()=>{
+			let err;
+			try {
+				api.getId({
+					params : {
+						id : 'abcdefghijkl'
+					},
+					body : {
+						googleId : '012345678901234567890123456789012'
+					}
+				});
+			} catch (e) {
+				err = e;
+			}
+
+			expect(err).toEqual({ HBErrorCode: '12', brewId: 'abcdefghijkl', message: 'Invalid ID', name: 'Google ID Error', status: 404 });
+		});
+
+		it('should throw invalid - google id too short (32 char)', ()=>{
+			let err;
+			try {
+				api.getId({
+					params : {
+						id : 'abcdefghijkl'
+					},
+					body : {
+						googleId : '12345678901234567890123456789012'
+					}
+				});
+			} catch (e) {
+				err = e;
+			}
+
+			expect(err).toEqual({ HBErrorCode: '12', brewId: 'abcdefghijkl', message: 'Invalid ID', name: 'Google ID Error', status: 404 });
+		});
+
+		it('should throw invalid - google id too long (45 char)', ()=>{
+			let err;
+			try {
+				api.getId({
+					params : {
+						id : 'abcdefghijkl'
+					},
+					body : {
+						googleId : '123456789012345678901234567890123456789012345'
+					}
+				});
+			} catch (e) {
+				err = e;
+			}
+
+			expect(err).toEqual({ HBErrorCode: '12', brewId: 'abcdefghijkl', message: 'Invalid ID', name: 'Google ID Error', status: 404 });
 		});
 
 		it('should return 12-char id and google id from params', ()=>{
@@ -1052,4 +1121,83 @@ brew`);
 			expect(testBrew.tags).toEqual(['tag a']);
 		});
 	});
+
+	describe('updateBrew', ()=>{
+		it('should return error on version mismatch', async ()=>{
+			const brewFromClient = { version: 1 };
+			const brewFromServer = { version: 1000, text: '' };
+
+			const req = {
+				brew : brewFromServer,
+				body : brewFromClient
+			};
+
+			await api.updateBrew(req, res);
+
+			expect(res.status).toHaveBeenCalledWith(409);
+			expect(res.send).toHaveBeenCalledWith('{\"message\":\"The server version is out of sync with the saved brew. Please save your changes elsewhere, refresh, and try again.\"}');
+		});
+
+		it('should return error on hash mismatch', async ()=>{
+			const brewFromClient = { version: 1, hash: '1234' };
+			const brewFromServer = { version: 1, text: 'test' };
+
+			const req = {
+				brew : brewFromServer,
+				body : brewFromClient
+			};
+
+			await api.updateBrew(req, res);
+
+			expect(req.brew.hash).toBe('098f6bcd4621d373cade4e832627b4f6');
+			expect(res.status).toHaveBeenCalledWith(409);
+			expect(res.send).toHaveBeenCalledWith('{\"message\":\"The server copy is out of sync with the saved brew. Please save your changes elsewhere, refresh, and try again.\"}');
+		});
+
+		// Commenting this one out for now, since we are no longer throwing this error while we monitor
+		// it('should return error on applying patches', async ()=>{
+		// 	const brewFromClient = { version: 1, hash: '098f6bcd4621d373cade4e832627b4f6', patches: 'not a valid patch string' };
+		// 	const brewFromServer = { version: 1, text: 'test', title: 'Test Title', description: 'Test Description' };
+
+		// 	const req = {
+		// 		brew  : brewFromServer,
+		// 		body  : brewFromClient,
+		// 	};
+
+		// 	let err;
+		// 	try {
+		// 		await api.updateBrew(req, res);
+		// 	} catch (e) {
+		// 		err = e;
+		// 	}
+
+		// 	expect(err).toEqual(Error('Invalid patch string: not a valid patch string'));
+		// });
+
+		it('should save brew, no ID', async ()=>{
+			const brewFromClient = { version: 1, hash: '098f6bcd4621d373cade4e832627b4f6', patches: '' };
+			const brewFromServer = { version: 1, text: 'test', title: 'Test Title', description: 'Test Description' };
+
+			model.save = jest.fn((brew)=>{return brew;});
+
+			const req = {
+				brew  : brewFromServer,
+				body  : brewFromClient,
+				query : { saveToGoogle: false, removeFromGoogle: false }
+			};
+
+			await api.updateBrew(req, res);
+
+			expect(res.status).toHaveBeenCalledWith(200);
+			expect(res.send).toHaveBeenCalledWith(
+				expect.objectContaining({
+					_id         : '1',
+					description : 'Test Description',
+					hash        : '098f6bcd4621d373cade4e832627b4f6',
+					title       : 'Test Title',
+					version     : 2
+				})
+			);
+		});
+	});
 });

server/token.js

@@ -5,21 +5,16 @@ import config from './config.js';
 const generateAccessToken = (account)=>{
 	const payload = account;
 
-	// When the token was issued
-	payload.issued = (new Date());
-	// Which service issued the Token
-	payload.issuer = config.get('authentication_token_issuer');
-	// Which service is the token intended for
-	payload.audience = config.get('authentication_token_audience');
-	// The signing key for signing the token
+	payload.issued = (new Date());                              	// When the token was issued
+	payload.issuer = config.get('authentication_token_issuer');     // Which service issued the Token
+	payload.audience = config.get('authentication_token_audience'); // Which service is the token intended for
+	const secret = config.get('authentication_token_secret');       // The signing key for signing the token
+
 	delete payload.password;
 	delete payload._id;
 
-	const secret = config.get('authentication_token_secret');
-
 	const token = jwt.encode(payload, secret);
-
 	return token;
 };
 
-export default generateAccessToken;
+export default generateAccessToken;

server/token.spec.js

@@ -0,0 +1,27 @@
+import { expect, jest } from '@jest/globals';
+import config from './config.js';
+
+import generateAccessToken from './token';
+
+describe('Tests for Token', ()=>{
+	it('Get token', ()=>{
+
+		// Mock the Config module, so we aren't grabbing actual secrets for testing
+		jest.mock('./config.js');
+		config.get = jest.fn((param)=>{
+			// The requested key name will be reflected to the output
+			return param;
+		});
+
+		const account = {};
+
+		const token = generateAccessToken(account);
+
+		// If these tests fail, the config mock has failed
+		expect(account).toHaveProperty('issuer', 'authentication_token_issuer');
+		expect(account).toHaveProperty('audience', 'authentication_token_audience');
+
+		// Because the inputs are fixed, this JWT key should be static
+		expect(typeof token).toBe('string');
+	});
+});